Skip to content

Commit 03fe8c3

Browse files
[SIEM] Uses faster wait from testing-library and removes duplicate older wait idiom (#72509)
## Summary * Removes the older wait pattern that does a block no matter what * Utilizes the improved and better pattern for test-library's waitFor which will test immediately and then poll for results * Changes everything to put their expect statement within the waitFor * Once the waitFor is in TypeScript/JS we can change the import statement to use that If you get a timeout or error this is what it looks like now which improves the developer experience in some ways but does degrade things in others as it suggests that everything is timeout related. However, developers should inspect the values and remove the waitFor() and re-run their tests if they think that they have a real problem during development. <img width="990" alt="Screen Shot 2020-07-20 at 12 40 39 PM" src="https://user-images.githubusercontent.com/1151048/87975739-4084d980-ca89-11ea-83c9-ba3fb932a175.png"> See the API for more information: https://testing-library.com/docs/dom-testing-library/api-async#waitfor But in short we should be using: ```ts await waitFor(() => expect(...)); ``` throughout our code at this point and the waitFor will loop quickly and efficiently until it either times out or gets the condition expected. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
1 parent 709e0a0 commit 03fe8c3

File tree

22 files changed

+578
-491
lines changed

22 files changed

+578
-491
lines changed

x-pack/plugins/security_solution/public/cases/components/add_comment/index.test.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import { Router, routeData, mockHistory, mockLocation } from '../__mock__/router
1515
import { useInsertTimeline } from '../../../timelines/components/timeline/insert_timeline_popover/use_insert_timeline';
1616
import { usePostComment } from '../../containers/use_post_comment';
1717
import { useForm } from '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form';
18-
import { wait } from '../../../common/lib/helpers';
18+
19+
// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
20+
import { wait as waitFor } from '@testing-library/react';
1921

2022
jest.mock(
2123
'../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form'
@@ -84,10 +86,11 @@ describe('AddComment ', () => {
8486
expect(wrapper.find(`[data-test-subj="loading-spinner"]`).exists()).toBeFalsy();
8587

8688
wrapper.find(`[data-test-subj="submit-comment"]`).first().simulate('click');
87-
await wait();
88-
expect(onCommentSaving).toBeCalled();
89-
expect(postComment).toBeCalledWith(sampleData, onCommentPosted);
90-
expect(formHookMock.reset).toBeCalled();
89+
await waitFor(() => {
90+
expect(onCommentSaving).toBeCalled();
91+
expect(postComment).toBeCalledWith(sampleData, onCommentPosted);
92+
expect(formHookMock.reset).toBeCalled();
93+
});
9194
});
9295

9396
it('should render spinner and disable submit when loading', () => {

x-pack/plugins/security_solution/public/cases/components/case_view/index.test.tsx

Lines changed: 67 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import { TestProviders } from '../../../common/mock';
1515
import { useUpdateCase } from '../../containers/use_update_case';
1616
import { useGetCase } from '../../containers/use_get_case';
1717
import { useGetCaseUserActions } from '../../containers/use_get_case_user_actions';
18-
import { wait } from '../../../common/lib/helpers';
18+
19+
// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
20+
import { wait as waitFor } from '@testing-library/react';
1921

2022
import { useConnectors } from '../../containers/configure/use_connectors';
2123
import { connectorsMock } from '../../containers/configure/mock';
@@ -108,30 +110,33 @@ describe('CaseView ', () => {
108110
</Router>
109111
</TestProviders>
110112
);
111-
await wait();
112-
expect(wrapper.find(`[data-test-subj="case-view-title"]`).first().prop('title')).toEqual(
113-
data.title
114-
);
115-
expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(data.status);
116-
expect(
117-
wrapper
118-
.find(`[data-test-subj="case-view-tag-list"] [data-test-subj="case-tag"]`)
119-
.first()
120-
.text()
121-
).toEqual(data.tags[0]);
122-
expect(wrapper.find(`[data-test-subj="case-view-username"]`).first().text()).toEqual(
123-
data.createdBy.username
124-
);
125-
expect(wrapper.contains(`[data-test-subj="case-view-closedAt"]`)).toBe(false);
126-
expect(wrapper.find(`[data-test-subj="case-view-createdAt"]`).first().prop('value')).toEqual(
127-
data.createdAt
128-
);
129-
expect(
130-
wrapper
131-
.find(`[data-test-subj="description-action"] [data-test-subj="user-action-markdown"]`)
132-
.first()
133-
.prop('raw')
134-
).toEqual(data.description);
113+
await waitFor(() => {
114+
expect(wrapper.find(`[data-test-subj="case-view-title"]`).first().prop('title')).toEqual(
115+
data.title
116+
);
117+
expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(
118+
data.status
119+
);
120+
expect(
121+
wrapper
122+
.find(`[data-test-subj="case-view-tag-list"] [data-test-subj="case-tag"]`)
123+
.first()
124+
.text()
125+
).toEqual(data.tags[0]);
126+
expect(wrapper.find(`[data-test-subj="case-view-username"]`).first().text()).toEqual(
127+
data.createdBy.username
128+
);
129+
expect(wrapper.contains(`[data-test-subj="case-view-closedAt"]`)).toBe(false);
130+
expect(wrapper.find(`[data-test-subj="case-view-createdAt"]`).first().prop('value')).toEqual(
131+
data.createdAt
132+
);
133+
expect(
134+
wrapper
135+
.find(`[data-test-subj="description-action"] [data-test-subj="user-action-markdown"]`)
136+
.first()
137+
.prop('raw')
138+
).toEqual(data.description);
139+
});
135140
});
136141

137142
it('should show closed indicators in header when case is closed', async () => {
@@ -146,14 +151,15 @@ describe('CaseView ', () => {
146151
</Router>
147152
</TestProviders>
148153
);
149-
await wait();
150-
expect(wrapper.contains(`[data-test-subj="case-view-createdAt"]`)).toBe(false);
151-
expect(wrapper.find(`[data-test-subj="case-view-closedAt"]`).first().prop('value')).toEqual(
152-
basicCaseClosed.closedAt
153-
);
154-
expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(
155-
basicCaseClosed.status
156-
);
154+
await waitFor(() => {
155+
expect(wrapper.contains(`[data-test-subj="case-view-createdAt"]`)).toBe(false);
156+
expect(wrapper.find(`[data-test-subj="case-view-closedAt"]`).first().prop('value')).toEqual(
157+
basicCaseClosed.closedAt
158+
);
159+
expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(
160+
basicCaseClosed.status
161+
);
162+
});
157163
});
158164

159165
it('should dispatch update state when button is toggled', async () => {
@@ -164,11 +170,12 @@ describe('CaseView ', () => {
164170
</Router>
165171
</TestProviders>
166172
);
167-
await wait();
168-
wrapper
169-
.find('input[data-test-subj="toggle-case-status"]')
170-
.simulate('change', { target: { checked: true } });
171-
expect(updateCaseProperty).toHaveBeenCalled();
173+
await waitFor(() => {
174+
wrapper
175+
.find('input[data-test-subj="toggle-case-status"]')
176+
.simulate('change', { target: { checked: true } });
177+
expect(updateCaseProperty).toHaveBeenCalled();
178+
});
172179
});
173180

174181
it('should display EditableTitle isLoading', () => {
@@ -296,17 +303,17 @@ describe('CaseView ', () => {
296303
</TestProviders>
297304
);
298305

299-
await wait();
306+
await waitFor(() => {
307+
expect(
308+
wrapper.find('[data-test-subj="has-data-to-push-button"]').first().exists()
309+
).toBeTruthy();
300310

301-
expect(
302-
wrapper.find('[data-test-subj="has-data-to-push-button"]').first().exists()
303-
).toBeTruthy();
311+
wrapper.find('[data-test-subj="push-to-external-service"]').first().simulate('click');
304312

305-
wrapper.find('[data-test-subj="push-to-external-service"]').first().simulate('click');
313+
wrapper.update();
306314

307-
wrapper.update();
308-
309-
expect(postPushToService).toHaveBeenCalled();
315+
expect(postPushToService).toHaveBeenCalled();
316+
});
310317
});
311318

312319
it('should return null if error', () => {
@@ -424,17 +431,19 @@ describe('CaseView ', () => {
424431
</Router>
425432
</TestProviders>
426433
);
427-
await wait();
428-
wrapper.find('button[data-test-subj="dropdown-connectors"]').simulate('click');
429-
wrapper.update();
430-
wrapper.find('button[data-test-subj="dropdown-connector-servicenow-2"]').simulate('click');
431-
wrapper.update();
432-
wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
433-
wrapper.update();
434-
await wait();
435-
wrapper.update();
436-
expect(
437-
wrapper.find('[data-test-subj="dropdown-connectors"]').at(0).prop('valueOfSelected')
438-
).toBe('servicenow-1');
434+
await waitFor(() => {
435+
wrapper.find('button[data-test-subj="dropdown-connectors"]').simulate('click');
436+
wrapper.update();
437+
wrapper.find('button[data-test-subj="dropdown-connector-servicenow-2"]').simulate('click');
438+
wrapper.update();
439+
wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
440+
wrapper.update();
441+
});
442+
await waitFor(() => {
443+
wrapper.update();
444+
expect(
445+
wrapper.find('[data-test-subj="dropdown-connectors"]').at(0).prop('valueOfSelected')
446+
).toBe('servicenow-1');
447+
});
439448
});
440449
});

x-pack/plugins/security_solution/public/cases/components/create/index.test.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import { useGetTags } from '../../containers/use_get_tags';
1919
jest.mock('../../../timelines/components/timeline/insert_timeline_popover/use_insert_timeline');
2020
jest.mock('../../containers/use_post_case');
2121
import { useForm } from '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form';
22-
import { wait } from '../../../common/lib/helpers';
22+
23+
// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
24+
import { wait as waitFor } from '@testing-library/react';
2325

2426
jest.mock(
2527
'../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form'
@@ -97,8 +99,7 @@ describe('Create case', () => {
9799
</TestProviders>
98100
);
99101
wrapper.find(`[data-test-subj="create-case-submit"]`).first().simulate('click');
100-
await wait();
101-
expect(postCase).toBeCalledWith(sampleData);
102+
await waitFor(() => expect(postCase).toBeCalledWith(sampleData));
102103
});
103104

104105
it('should redirect to all cases on cancel click', () => {

x-pack/plugins/security_solution/public/cases/components/edit_connector/index.test.tsx

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import { EditConnector } from './index';
1111
import { getFormMock, useFormMock } from '../__mock__/form';
1212
import { TestProviders } from '../../../common/mock';
1313
import { connectorsMock } from '../../containers/configure/mock';
14-
import { wait } from '../../../common/lib/helpers';
14+
// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
15+
import { wait as waitFor } from '@testing-library/react';
1516
import { act } from 'react-dom/test-utils';
1617
jest.mock(
1718
'../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form'
@@ -68,8 +69,7 @@ describe('EditConnector ', () => {
6869

6970
await act(async () => {
7071
wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
71-
await wait();
72-
expect(onSubmit.mock.calls[0][0]).toBe(sampleConnector);
72+
await waitFor(() => expect(onSubmit.mock.calls[0][0]).toBe(sampleConnector));
7373
});
7474
});
7575

@@ -92,10 +92,11 @@ describe('EditConnector ', () => {
9292

9393
await act(async () => {
9494
wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
95-
await wait();
96-
wrapper.update();
95+
await waitFor(() => {
96+
wrapper.update();
97+
expect(formHookMock.setFieldValue).toHaveBeenCalledWith('connector', 'none');
98+
});
9799
});
98-
expect(formHookMock.setFieldValue).toHaveBeenCalledWith('connector', 'none');
99100
});
100101

101102
it('Resets selector on cancel', async () => {
@@ -115,12 +116,13 @@ describe('EditConnector ', () => {
115116

116117
await act(async () => {
117118
wrapper.find(`[data-test-subj="edit-connectors-cancel"]`).last().simulate('click');
118-
await wait();
119-
wrapper.update();
120-
expect(formHookMock.setFieldValue).toBeCalledWith(
121-
'connector',
122-
defaultProps.selectedConnector
123-
);
119+
await waitFor(() => {
120+
wrapper.update();
121+
expect(formHookMock.setFieldValue).toBeCalledWith(
122+
'connector',
123+
defaultProps.selectedConnector
124+
);
125+
});
124126
});
125127
});
126128

x-pack/plugins/security_solution/public/cases/components/tag_list/index.test.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import { act } from 'react-dom/test-utils';
1111
import { TagList } from '.';
1212
import { getFormMock } from '../__mock__/form';
1313
import { TestProviders } from '../../../common/mock';
14-
import { wait } from '../../../common/lib/helpers';
14+
// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
15+
import { wait as waitFor } from '@testing-library/react';
1516
import { useForm } from '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form';
1617
import { useGetTags } from '../../containers/use_get_tags';
1718

@@ -77,8 +78,7 @@ describe('TagList ', () => {
7778
wrapper.find(`[data-test-subj="tag-list-edit-button"]`).last().simulate('click');
7879
await act(async () => {
7980
wrapper.find(`[data-test-subj="edit-tags-submit"]`).last().simulate('click');
80-
await wait();
81-
expect(onSubmit).toBeCalledWith(sampleTags);
81+
await waitFor(() => expect(onSubmit).toBeCalledWith(sampleTags));
8282
});
8383
});
8484
it('Tag options render with new tags added', () => {
@@ -107,9 +107,10 @@ describe('TagList ', () => {
107107
await act(async () => {
108108
expect(wrapper.find(`[data-test-subj="case-tag"]`).last().exists()).toBeFalsy();
109109
wrapper.find(`[data-test-subj="edit-tags-cancel"]`).last().simulate('click');
110-
await wait();
111-
wrapper.update();
112-
expect(wrapper.find(`[data-test-subj="case-tag"]`).last().exists()).toBeTruthy();
110+
await waitFor(() => {
111+
wrapper.update();
112+
expect(wrapper.find(`[data-test-subj="case-tag"]`).last().exists()).toBeTruthy();
113+
});
113114
});
114115
});
115116
it('Renders disabled button', () => {

x-pack/plugins/security_solution/public/cases/components/user_action_tree/index.test.tsx

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import { useUpdateComment } from '../../containers/use_update_comment';
1313
import { basicCase, basicPush, getUserAction } from '../../containers/mock';
1414
import { UserActionTree } from '.';
1515
import { TestProviders } from '../../../common/mock';
16-
import { wait } from '../../../common/lib/helpers';
16+
// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
17+
import { wait as waitFor } from '@testing-library/react';
1718
import { act } from 'react-dom/test-utils';
1819

1920
const fetchUserActions = jest.fn();
@@ -225,22 +226,23 @@ describe('UserActionTree ', () => {
225226
.first()
226227
.simulate('click');
227228
await act(async () => {
228-
await wait();
229-
wrapper.update();
230-
expect(
231-
wrapper
232-
.find(
233-
`[data-test-subj="user-action-${props.data.comments[0].id}"] [data-test-subj="user-action-markdown-form"]`
234-
)
235-
.exists()
236-
).toEqual(false);
237-
expect(patchComment).toBeCalledWith({
238-
commentUpdate: sampleData.content,
239-
caseId: props.data.id,
240-
commentId: props.data.comments[0].id,
241-
fetchUserActions,
242-
updateCase,
243-
version: props.data.comments[0].version,
229+
await waitFor(() => {
230+
wrapper.update();
231+
expect(
232+
wrapper
233+
.find(
234+
`[data-test-subj="user-action-${props.data.comments[0].id}"] [data-test-subj="user-action-markdown-form"]`
235+
)
236+
.exists()
237+
).toEqual(false);
238+
expect(patchComment).toBeCalledWith({
239+
commentUpdate: sampleData.content,
240+
caseId: props.data.id,
241+
commentId: props.data.comments[0].id,
242+
fetchUserActions,
243+
updateCase,
244+
version: props.data.comments[0].version,
245+
});
244246
});
245247
});
246248
});
@@ -269,15 +271,16 @@ describe('UserActionTree ', () => {
269271
.first()
270272
.simulate('click');
271273
await act(async () => {
272-
await wait();
273-
expect(
274-
wrapper
275-
.find(
276-
`[data-test-subj="user-action-${props.data.id}"] [data-test-subj="user-action-markdown-form"]`
277-
)
278-
.exists()
279-
).toEqual(false);
280-
expect(onUpdateField).toBeCalledWith({ key: 'description', value: sampleData.content });
274+
await waitFor(() => {
275+
expect(
276+
wrapper
277+
.find(
278+
`[data-test-subj="user-action-${props.data.id}"] [data-test-subj="user-action-markdown-form"]`
279+
)
280+
.exists()
281+
).toEqual(false);
282+
expect(onUpdateField).toBeCalledWith({ key: 'description', value: sampleData.content });
283+
});
281284
});
282285
});
283286

0 commit comments

Comments
 (0)