Skip to content

Commit 52b9097

Browse files
committed
[Security][Detections] Unskip failing modal tests (#71969)
* 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).
1 parent 8566491 commit 52b9097

File tree

4 files changed

+25
-55
lines changed

4 files changed

+25
-55
lines changed

x-pack/plugins/security_solution/public/common/utils/test_utils.ts

Lines changed: 0 additions & 16 deletions
This file was deleted.

x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import React, { FormEvent } from 'react';
77
import { mount, ReactWrapper } from 'enzyme';
88
import { act } from 'react-dom/test-utils';
99

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

3934
describe('ValueListsForm', () => {
@@ -68,7 +63,6 @@ describe('ValueListsForm', () => {
6863
await mockSelectFile(container, mockFile);
6964

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

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

8276
const onError = jest.fn();
83-
const container = mount(
77+
mount(
8478
<TestProviders>
8579
<ValueListsForm onError={onError} onSuccess={jest.fn()} />
8680
</TestProviders>
8781
);
88-
await waitForUpdates(container);
8982

9083
expect(onError).toHaveBeenCalledWith('whoops');
9184
});
@@ -97,12 +90,11 @@ describe('ValueListsForm', () => {
9790
}));
9891

9992
const onSuccess = jest.fn();
100-
const container = mount(
93+
mount(
10194
<TestProviders>
10295
<ValueListsForm onSuccess={onSuccess} onError={jest.fn()} />
10396
</TestProviders>
10497
);
105-
await waitForUpdates(container);
10698

10799
expect(onSuccess).toHaveBeenCalledWith({ mockResult: true });
108100
});

x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ import { mount } from 'enzyme';
99

1010
import { TestProviders } from '../../../common/mock';
1111
import { ValueListsModal } from './modal';
12-
import { waitForUpdates } from '../../../common/utils/test_utils';
1312

14-
// TODO: These are occasionally timing out
15-
describe.skip('ValueListsModal', () => {
13+
describe('ValueListsModal', () => {
1614
it('renders nothing if showModal is false', () => {
1715
const container = mount(
1816
<TestProviders>
@@ -21,20 +19,21 @@ describe.skip('ValueListsModal', () => {
2119
);
2220

2321
expect(container.find('EuiModal')).toHaveLength(0);
22+
container.unmount();
2423
});
2524

26-
it('renders modal if showModal is true', async () => {
25+
it('renders modal if showModal is true', () => {
2726
const container = mount(
2827
<TestProviders>
2928
<ValueListsModal showModal={true} onClose={jest.fn()} />
3029
</TestProviders>
3130
);
32-
await waitForUpdates(container);
3331

3432
expect(container.find('EuiModal')).toHaveLength(1);
33+
container.unmount();
3534
});
3635

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

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

47-
await waitForUpdates(container);
48-
4946
expect(onClose).toHaveBeenCalled();
47+
container.unmount();
5048
});
5149

52-
it('renders ValueListsForm and ValueListsTable', async () => {
50+
it('renders ValueListsForm and ValueListsTable', () => {
5351
const container = mount(
5452
<TestProviders>
5553
<ValueListsModal showModal={true} onClose={jest.fn()} />
5654
</TestProviders>
5755
);
5856

59-
await waitForUpdates(container);
60-
6157
expect(container.find('ValueListsForm')).toHaveLength(1);
6258
expect(container.find('ValueListsTable')).toHaveLength(1);
59+
container.unmount();
6360
});
6461
});

x-pack/plugins/security_solution/public/overview/pages/overview.test.tsx

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import React from 'react';
99
import { MemoryRouter } from 'react-router-dom';
1010

1111
import '../../common/mock/match_media';
12-
import { waitForUpdates } from '../../common/utils/test_utils';
1312
import { TestProviders } from '../../common/mock';
1413
import { useWithSource } from '../../common/containers/source';
1514
import {
@@ -65,31 +64,29 @@ describe('Overview', () => {
6564
mockuseMessagesStorage.mockImplementation(() => endpointNoticeMessage(false));
6665
});
6766

68-
it('renders the Setup Instructions text', async () => {
67+
it('renders the Setup Instructions text', () => {
6968
const wrapper = mount(
7069
<TestProviders>
7170
<MemoryRouter>
7271
<Overview />
7372
</MemoryRouter>
7473
</TestProviders>
7574
);
76-
await waitForUpdates(wrapper);
7775
expect(wrapper.find('[data-test-subj="empty-page"]').exists()).toBe(true);
7876
});
7977

80-
it('does not show Endpoint get ready button when ingest is not enabled', async () => {
78+
it('does not show Endpoint get ready button when ingest is not enabled', () => {
8179
const wrapper = mount(
8280
<TestProviders>
8381
<MemoryRouter>
8482
<Overview />
8583
</MemoryRouter>
8684
</TestProviders>
8785
);
88-
await waitForUpdates(wrapper);
8986
expect(wrapper.find('[data-test-subj="empty-page-secondary-action"]').exists()).toBe(false);
9087
});
9188

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

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

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

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

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

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

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

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

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

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

224-
test('it does NOT render the Endpoint banner when Ingest is NOT available', async () => {
221+
test('it does NOT render the Endpoint banner when Ingest is NOT available', () => {
225222
(useWithSource as jest.Mock).mockReturnValue({
226223
indicesExist: true,
227224
indexPattern: {},
@@ -238,9 +235,9 @@ describe('Overview', () => {
238235
</MemoryRouter>
239236
</TestProviders>
240237
);
241-
await waitForUpdates(wrapper);
242238

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

0 commit comments

Comments
 (0)