Skip to content

Commit

Permalink
[Security][Detections] Unskip failing modal tests (elastic#71969)
Browse files Browse the repository at this point in the history
* Revert "Skip jest tests that timeout waiting for react"

This reverts commit dd9b0b3.

* Unmount async effectful components instead of waiting for them

A previous commit introduced waitForUpdates as a solution to the
warnings introduced by enzymejs/enzyme#2073:
by waiting for the effects to complete we avoid the warning.

However, waiting for the effects to complete could occasionally be very
costly, especially on an overtasked CI machine, and I've been seeing
these tests fail on occasion due to timeouts.

Since a warning message is preferable to a false negative, I'm removing
waitForUpdates and allowing the warnings to occur, as this should be
fixed on a subsequent update of enzyme/react-adapter.

I've also fixed warnings in a few particularly problematic/noisy tests
by simply unmounting the component at the end of the test (this does
not work in an afterEach).
  • Loading branch information
rylnd committed Jul 16, 2020
1 parent 68d7f58 commit 47a5ab4
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 55 deletions.
16 changes: 0 additions & 16 deletions x-pack/plugins/security_solution/public/common/utils/test_utils.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import React, { FormEvent } from 'react';
import { mount, ReactWrapper } from 'enzyme';
import { act } from 'react-dom/test-utils';

import { waitForUpdates } from '../../../common/utils/test_utils';
import { TestProviders } from '../../../common/mock';
import { ValueListsForm } from './form';
import { useImportList } from '../../../shared_imports';
Expand All @@ -30,10 +29,6 @@ const mockSelectFile: <P>(container: ReactWrapper<P>, file: File) => Promise<voi
fileChange(([file] as unknown) as FormEvent);
}
});
await waitForUpdates(container);
expect(
container.find('button[data-test-subj="value-lists-form-import-action"]').prop('disabled')
).not.toEqual(true);
};

describe('ValueListsForm', () => {
Expand Down Expand Up @@ -68,7 +63,6 @@ describe('ValueListsForm', () => {
await mockSelectFile(container, mockFile);

container.find('button[data-test-subj="value-lists-form-import-action"]').simulate('click');
await waitForUpdates(container);

expect(mockImportList).toHaveBeenCalledWith(expect.objectContaining({ file: mockFile }));
});
Expand All @@ -80,12 +74,11 @@ describe('ValueListsForm', () => {
}));

const onError = jest.fn();
const container = mount(
mount(
<TestProviders>
<ValueListsForm onError={onError} onSuccess={jest.fn()} />
</TestProviders>
);
await waitForUpdates(container);

expect(onError).toHaveBeenCalledWith('whoops');
});
Expand All @@ -97,12 +90,11 @@ describe('ValueListsForm', () => {
}));

const onSuccess = jest.fn();
const container = mount(
mount(
<TestProviders>
<ValueListsForm onSuccess={onSuccess} onError={jest.fn()} />
</TestProviders>
);
await waitForUpdates(container);

expect(onSuccess).toHaveBeenCalledWith({ mockResult: true });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import { mount } from 'enzyme';

import { TestProviders } from '../../../common/mock';
import { ValueListsModal } from './modal';
import { waitForUpdates } from '../../../common/utils/test_utils';

// TODO: These are occasionally timing out
describe.skip('ValueListsModal', () => {
describe('ValueListsModal', () => {
it('renders nothing if showModal is false', () => {
const container = mount(
<TestProviders>
Expand All @@ -21,20 +19,21 @@ describe.skip('ValueListsModal', () => {
);

expect(container.find('EuiModal')).toHaveLength(0);
container.unmount();
});

it('renders modal if showModal is true', async () => {
it('renders modal if showModal is true', () => {
const container = mount(
<TestProviders>
<ValueListsModal showModal={true} onClose={jest.fn()} />
</TestProviders>
);
await waitForUpdates(container);

expect(container.find('EuiModal')).toHaveLength(1);
container.unmount();
});

it('calls onClose when modal is closed', async () => {
it('calls onClose when modal is closed', () => {
const onClose = jest.fn();
const container = mount(
<TestProviders>
Expand All @@ -44,21 +43,19 @@ describe.skip('ValueListsModal', () => {

container.find('button[data-test-subj="value-lists-modal-close-action"]').simulate('click');

await waitForUpdates(container);

expect(onClose).toHaveBeenCalled();
container.unmount();
});

it('renders ValueListsForm and ValueListsTable', async () => {
it('renders ValueListsForm and ValueListsTable', () => {
const container = mount(
<TestProviders>
<ValueListsModal showModal={true} onClose={jest.fn()} />
</TestProviders>
);

await waitForUpdates(container);

expect(container.find('ValueListsForm')).toHaveLength(1);
expect(container.find('ValueListsTable')).toHaveLength(1);
container.unmount();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import React from 'react';
import { MemoryRouter } from 'react-router-dom';

import '../../common/mock/match_media';
import { waitForUpdates } from '../../common/utils/test_utils';
import { TestProviders } from '../../common/mock';
import { useWithSource } from '../../common/containers/source';
import {
Expand Down Expand Up @@ -65,31 +64,29 @@ describe('Overview', () => {
mockuseMessagesStorage.mockImplementation(() => endpointNoticeMessage(false));
});

it('renders the Setup Instructions text', async () => {
it('renders the Setup Instructions text', () => {
const wrapper = mount(
<TestProviders>
<MemoryRouter>
<Overview />
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);
expect(wrapper.find('[data-test-subj="empty-page"]').exists()).toBe(true);
});

it('does not show Endpoint get ready button when ingest is not enabled', async () => {
it('does not show Endpoint get ready button when ingest is not enabled', () => {
const wrapper = mount(
<TestProviders>
<MemoryRouter>
<Overview />
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);
expect(wrapper.find('[data-test-subj="empty-page-secondary-action"]').exists()).toBe(false);
});

it('shows Endpoint get ready button when ingest is enabled', async () => {
it('shows Endpoint get ready button when ingest is enabled', () => {
(useIngestEnabledCheck as jest.Mock).mockReturnValue({ allEnabled: true });
const wrapper = mount(
<TestProviders>
Expand All @@ -98,12 +95,11 @@ describe('Overview', () => {
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);
expect(wrapper.find('[data-test-subj="empty-page-secondary-action"]').exists()).toBe(true);
});
});

it('it DOES NOT render the Getting started text when an index is available', async () => {
it('it DOES NOT render the Getting started text when an index is available', () => {
(useWithSource as jest.Mock).mockReturnValue({
indicesExist: true,
indexPattern: {},
Expand All @@ -120,12 +116,12 @@ describe('Overview', () => {
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);

expect(wrapper.find('[data-test-subj="empty-page"]').exists()).toBe(false);
wrapper.unmount();
});

test('it DOES render the Endpoint banner when the endpoint index is NOT available AND storage is NOT set', async () => {
test('it DOES render the Endpoint banner when the endpoint index is NOT available AND storage is NOT set', () => {
(useWithSource as jest.Mock).mockReturnValueOnce({
indicesExist: true,
indexPattern: {},
Expand All @@ -147,12 +143,12 @@ describe('Overview', () => {
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);

expect(wrapper.find('[data-test-subj="endpoint-prompt-banner"]').exists()).toBe(true);
wrapper.unmount();
});

test('it does NOT render the Endpoint banner when the endpoint index is NOT available but storage is set', async () => {
test('it does NOT render the Endpoint banner when the endpoint index is NOT available but storage is set', () => {
(useWithSource as jest.Mock).mockReturnValueOnce({
indicesExist: true,
indexPattern: {},
Expand All @@ -174,12 +170,12 @@ describe('Overview', () => {
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);

expect(wrapper.find('[data-test-subj="endpoint-prompt-banner"]').exists()).toBe(false);
wrapper.unmount();
});

test('it does NOT render the Endpoint banner when the endpoint index is available AND storage is set', async () => {
test('it does NOT render the Endpoint banner when the endpoint index is available AND storage is set', () => {
(useWithSource as jest.Mock).mockReturnValue({
indicesExist: true,
indexPattern: {},
Expand All @@ -196,12 +192,12 @@ describe('Overview', () => {
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);

expect(wrapper.find('[data-test-subj="endpoint-prompt-banner"]').exists()).toBe(false);
wrapper.unmount();
});

test('it does NOT render the Endpoint banner when an index IS available but storage is NOT set', async () => {
test('it does NOT render the Endpoint banner when an index IS available but storage is NOT set', () => {
(useWithSource as jest.Mock).mockReturnValue({
indicesExist: true,
indexPattern: {},
Expand All @@ -219,9 +215,10 @@ describe('Overview', () => {
</TestProviders>
);
expect(wrapper.find('[data-test-subj="endpoint-prompt-banner"]').exists()).toBe(false);
wrapper.unmount();
});

test('it does NOT render the Endpoint banner when Ingest is NOT available', async () => {
test('it does NOT render the Endpoint banner when Ingest is NOT available', () => {
(useWithSource as jest.Mock).mockReturnValue({
indicesExist: true,
indexPattern: {},
Expand All @@ -238,9 +235,9 @@ describe('Overview', () => {
</MemoryRouter>
</TestProviders>
);
await waitForUpdates(wrapper);

expect(wrapper.find('[data-test-subj="endpoint-prompt-banner"]').exists()).toBe(false);
wrapper.unmount();
});
});
});

0 comments on commit 47a5ab4

Please sign in to comment.