[SIEM] Uses faster wait from testing-library and removes duplicate ol…

…der wait idiom (#72509) (#72549) ## 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
elastic · Jul 21, 2020 · a376898 · a376898
1 parent dba7479
commit a376898
Show file tree

Hide file tree

Showing 22 changed files with 578 additions and 491 deletions.
diff --git a/x-pack/plugins/security_solution/public/cases/components/add_comment/index.test.tsx b/x-pack/plugins/security_solution/public/cases/components/add_comment/index.test.tsx
@@ -15,7 +15,9 @@ import { Router, routeData, mockHistory, mockLocation } from '../__mock__/router
 import { useInsertTimeline } from '../../../timelines/components/timeline/insert_timeline_popover/use_insert_timeline';
 import { usePostComment } from '../../containers/use_post_comment';
 import { useForm } from '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form';
-import { wait } from '../../../common/lib/helpers';
+
+// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
+import { wait as waitFor } from '@testing-library/react';
 
 jest.mock(
   '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form'
@@ -84,10 +86,11 @@ describe('AddComment ', () => {
     expect(wrapper.find(`[data-test-subj="loading-spinner"]`).exists()).toBeFalsy();
 
     wrapper.find(`[data-test-subj="submit-comment"]`).first().simulate('click');
-    await wait();
-    expect(onCommentSaving).toBeCalled();
-    expect(postComment).toBeCalledWith(sampleData, onCommentPosted);
-    expect(formHookMock.reset).toBeCalled();
+    await waitFor(() => {
+      expect(onCommentSaving).toBeCalled();
+      expect(postComment).toBeCalledWith(sampleData, onCommentPosted);
+      expect(formHookMock.reset).toBeCalled();
+    });
   });
 
   it('should render spinner and disable submit when loading', () => {

diff --git a/x-pack/plugins/security_solution/public/cases/components/case_view/index.test.tsx b/x-pack/plugins/security_solution/public/cases/components/case_view/index.test.tsx
@@ -15,7 +15,9 @@ import { TestProviders } from '../../../common/mock';
 import { useUpdateCase } from '../../containers/use_update_case';
 import { useGetCase } from '../../containers/use_get_case';
 import { useGetCaseUserActions } from '../../containers/use_get_case_user_actions';
-import { wait } from '../../../common/lib/helpers';
+
+// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
+import { wait as waitFor } from '@testing-library/react';
 
 import { useConnectors } from '../../containers/configure/use_connectors';
 import { connectorsMock } from '../../containers/configure/mock';
@@ -108,30 +110,33 @@ describe('CaseView ', () => {
         </Router>
       </TestProviders>
     );
-    await wait();
-    expect(wrapper.find(`[data-test-subj="case-view-title"]`).first().prop('title')).toEqual(
-      data.title
-    );
-    expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(data.status);
-    expect(
-      wrapper
-        .find(`[data-test-subj="case-view-tag-list"] [data-test-subj="case-tag"]`)
-        .first()
-        .text()
-    ).toEqual(data.tags[0]);
-    expect(wrapper.find(`[data-test-subj="case-view-username"]`).first().text()).toEqual(
-      data.createdBy.username
-    );
-    expect(wrapper.contains(`[data-test-subj="case-view-closedAt"]`)).toBe(false);
-    expect(wrapper.find(`[data-test-subj="case-view-createdAt"]`).first().prop('value')).toEqual(
-      data.createdAt
-    );
-    expect(
-      wrapper
-        .find(`[data-test-subj="description-action"] [data-test-subj="user-action-markdown"]`)
-        .first()
-        .prop('raw')
-    ).toEqual(data.description);
+    await waitFor(() => {
+      expect(wrapper.find(`[data-test-subj="case-view-title"]`).first().prop('title')).toEqual(
+        data.title
+      );
+      expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(
+        data.status
+      );
+      expect(
+        wrapper
+          .find(`[data-test-subj="case-view-tag-list"] [data-test-subj="case-tag"]`)
+          .first()
+          .text()
+      ).toEqual(data.tags[0]);
+      expect(wrapper.find(`[data-test-subj="case-view-username"]`).first().text()).toEqual(
+        data.createdBy.username
+      );
+      expect(wrapper.contains(`[data-test-subj="case-view-closedAt"]`)).toBe(false);
+      expect(wrapper.find(`[data-test-subj="case-view-createdAt"]`).first().prop('value')).toEqual(
+        data.createdAt
+      );
+      expect(
+        wrapper
+          .find(`[data-test-subj="description-action"] [data-test-subj="user-action-markdown"]`)
+          .first()
+          .prop('raw')
+      ).toEqual(data.description);
+    });
   });
 
   it('should show closed indicators in header when case is closed', async () => {
@@ -146,14 +151,15 @@ describe('CaseView ', () => {
         </Router>
       </TestProviders>
     );
-    await wait();
-    expect(wrapper.contains(`[data-test-subj="case-view-createdAt"]`)).toBe(false);
-    expect(wrapper.find(`[data-test-subj="case-view-closedAt"]`).first().prop('value')).toEqual(
-      basicCaseClosed.closedAt
-    );
-    expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(
-      basicCaseClosed.status
-    );
+    await waitFor(() => {
+      expect(wrapper.contains(`[data-test-subj="case-view-createdAt"]`)).toBe(false);
+      expect(wrapper.find(`[data-test-subj="case-view-closedAt"]`).first().prop('value')).toEqual(
+        basicCaseClosed.closedAt
+      );
+      expect(wrapper.find(`[data-test-subj="case-view-status"]`).first().text()).toEqual(
+        basicCaseClosed.status
+      );
+    });
   });
 
   it('should dispatch update state when button is toggled', async () => {
@@ -164,11 +170,12 @@ describe('CaseView ', () => {
         </Router>
       </TestProviders>
     );
-    await wait();
-    wrapper
-      .find('input[data-test-subj="toggle-case-status"]')
-      .simulate('change', { target: { checked: true } });
-    expect(updateCaseProperty).toHaveBeenCalled();
+    await waitFor(() => {
+      wrapper
+        .find('input[data-test-subj="toggle-case-status"]')
+        .simulate('change', { target: { checked: true } });
+      expect(updateCaseProperty).toHaveBeenCalled();
+    });
   });
 
   it('should display EditableTitle isLoading', () => {
@@ -296,17 +303,17 @@ describe('CaseView ', () => {
       </TestProviders>
     );
 
-    await wait();
+    await waitFor(() => {
+      expect(
+        wrapper.find('[data-test-subj="has-data-to-push-button"]').first().exists()
+      ).toBeTruthy();
 
-    expect(
-      wrapper.find('[data-test-subj="has-data-to-push-button"]').first().exists()
-    ).toBeTruthy();
+      wrapper.find('[data-test-subj="push-to-external-service"]').first().simulate('click');
 
-    wrapper.find('[data-test-subj="push-to-external-service"]').first().simulate('click');
+      wrapper.update();
 
-    wrapper.update();
-
-    expect(postPushToService).toHaveBeenCalled();
+      expect(postPushToService).toHaveBeenCalled();
+    });
   });
 
   it('should return null if error', () => {
@@ -424,17 +431,19 @@ describe('CaseView ', () => {
         </Router>
       </TestProviders>
     );
-    await wait();
-    wrapper.find('button[data-test-subj="dropdown-connectors"]').simulate('click');
-    wrapper.update();
-    wrapper.find('button[data-test-subj="dropdown-connector-servicenow-2"]').simulate('click');
-    wrapper.update();
-    wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
-    wrapper.update();
-    await wait();
-    wrapper.update();
-    expect(
-      wrapper.find('[data-test-subj="dropdown-connectors"]').at(0).prop('valueOfSelected')
-    ).toBe('servicenow-1');
+    await waitFor(() => {
+      wrapper.find('button[data-test-subj="dropdown-connectors"]').simulate('click');
+      wrapper.update();
+      wrapper.find('button[data-test-subj="dropdown-connector-servicenow-2"]').simulate('click');
+      wrapper.update();
+      wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
+      wrapper.update();
+    });
+    await waitFor(() => {
+      wrapper.update();
+      expect(
+        wrapper.find('[data-test-subj="dropdown-connectors"]').at(0).prop('valueOfSelected')
+      ).toBe('servicenow-1');
+    });
   });
 });
diff --git a/x-pack/plugins/security_solution/public/cases/components/create/index.test.tsx b/x-pack/plugins/security_solution/public/cases/components/create/index.test.tsx
@@ -19,7 +19,9 @@ import { useGetTags } from '../../containers/use_get_tags';
 jest.mock('../../../timelines/components/timeline/insert_timeline_popover/use_insert_timeline');
 jest.mock('../../containers/use_post_case');
 import { useForm } from '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form';
-import { wait } from '../../../common/lib/helpers';
+
+// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
+import { wait as waitFor } from '@testing-library/react';
 
 jest.mock(
   '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form'
@@ -97,8 +99,7 @@ describe('Create case', () => {
       </TestProviders>
     );
     wrapper.find(`[data-test-subj="create-case-submit"]`).first().simulate('click');
-    await wait();
-    expect(postCase).toBeCalledWith(sampleData);
+    await waitFor(() => expect(postCase).toBeCalledWith(sampleData));
   });
 
   it('should redirect to all cases on cancel click', () => {

diff --git a/x-pack/plugins/security_solution/public/cases/components/edit_connector/index.test.tsx b/x-pack/plugins/security_solution/public/cases/components/edit_connector/index.test.tsx
@@ -11,7 +11,8 @@ import { EditConnector } from './index';
 import { getFormMock, useFormMock } from '../__mock__/form';
 import { TestProviders } from '../../../common/mock';
 import { connectorsMock } from '../../containers/configure/mock';
-import { wait } from '../../../common/lib/helpers';
+// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
+import { wait as waitFor } from '@testing-library/react';
 import { act } from 'react-dom/test-utils';
 jest.mock(
   '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form'
@@ -68,8 +69,7 @@ describe('EditConnector ', () => {
 
     await act(async () => {
       wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
-      await wait();
-      expect(onSubmit.mock.calls[0][0]).toBe(sampleConnector);
+      await waitFor(() => expect(onSubmit.mock.calls[0][0]).toBe(sampleConnector));
     });
   });
 
@@ -92,10 +92,11 @@ describe('EditConnector ', () => {
 
     await act(async () => {
       wrapper.find(`[data-test-subj="edit-connectors-submit"]`).last().simulate('click');
-      await wait();
-      wrapper.update();
+      await waitFor(() => {
+        wrapper.update();
+        expect(formHookMock.setFieldValue).toHaveBeenCalledWith('connector', 'none');
+      });
     });
-    expect(formHookMock.setFieldValue).toHaveBeenCalledWith('connector', 'none');
   });
 
   it('Resets selector on cancel', async () => {
@@ -115,12 +116,13 @@ describe('EditConnector ', () => {
 
     await act(async () => {
       wrapper.find(`[data-test-subj="edit-connectors-cancel"]`).last().simulate('click');
-      await wait();
-      wrapper.update();
-      expect(formHookMock.setFieldValue).toBeCalledWith(
-        'connector',
-        defaultProps.selectedConnector
-      );
+      await waitFor(() => {
+        wrapper.update();
+        expect(formHookMock.setFieldValue).toBeCalledWith(
+          'connector',
+          defaultProps.selectedConnector
+        );
+      });
     });
   });
 

diff --git a/x-pack/plugins/security_solution/public/cases/components/tag_list/index.test.tsx b/x-pack/plugins/security_solution/public/cases/components/tag_list/index.test.tsx
@@ -11,7 +11,8 @@ import { act } from 'react-dom/test-utils';
 import { TagList } from '.';
 import { getFormMock } from '../__mock__/form';
 import { TestProviders } from '../../../common/mock';
-import { wait } from '../../../common/lib/helpers';
+// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
+import { wait as waitFor } from '@testing-library/react';
 import { useForm } from '../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form';
 import { useGetTags } from '../../containers/use_get_tags';
 
@@ -77,8 +78,7 @@ describe('TagList ', () => {
     wrapper.find(`[data-test-subj="tag-list-edit-button"]`).last().simulate('click');
     await act(async () => {
       wrapper.find(`[data-test-subj="edit-tags-submit"]`).last().simulate('click');
-      await wait();
-      expect(onSubmit).toBeCalledWith(sampleTags);
+      await waitFor(() => expect(onSubmit).toBeCalledWith(sampleTags));
     });
   });
   it('Tag options render with new tags added', () => {
@@ -107,9 +107,10 @@ describe('TagList ', () => {
     await act(async () => {
       expect(wrapper.find(`[data-test-subj="case-tag"]`).last().exists()).toBeFalsy();
       wrapper.find(`[data-test-subj="edit-tags-cancel"]`).last().simulate('click');
-      await wait();
-      wrapper.update();
-      expect(wrapper.find(`[data-test-subj="case-tag"]`).last().exists()).toBeTruthy();
+      await waitFor(() => {
+        wrapper.update();
+        expect(wrapper.find(`[data-test-subj="case-tag"]`).last().exists()).toBeTruthy();
+      });
     });
   });
   it('Renders disabled button', () => {

diff --git a/x-pack/plugins/security_solution/public/cases/components/user_action_tree/index.test.tsx b/x-pack/plugins/security_solution/public/cases/components/user_action_tree/index.test.tsx
@@ -13,7 +13,8 @@ import { useUpdateComment } from '../../containers/use_update_comment';
 import { basicCase, basicPush, getUserAction } from '../../containers/mock';
 import { UserActionTree } from '.';
 import { TestProviders } from '../../../common/mock';
-import { wait } from '../../../common/lib/helpers';
+// we don't have the types for waitFor just yet, so using "as waitFor" until when we do
+import { wait as waitFor } from '@testing-library/react';
 import { act } from 'react-dom/test-utils';
 
 const fetchUserActions = jest.fn();
@@ -225,22 +226,23 @@ describe('UserActionTree ', () => {
       .first()
       .simulate('click');
     await act(async () => {
-      await wait();
-      wrapper.update();
-      expect(
-        wrapper
-          .find(
-            `[data-test-subj="user-action-${props.data.comments[0].id}"] [data-test-subj="user-action-markdown-form"]`
-          )
-          .exists()
-      ).toEqual(false);
-      expect(patchComment).toBeCalledWith({
-        commentUpdate: sampleData.content,
-        caseId: props.data.id,
-        commentId: props.data.comments[0].id,
-        fetchUserActions,
-        updateCase,
-        version: props.data.comments[0].version,
+      await waitFor(() => {
+        wrapper.update();
+        expect(
+          wrapper
+            .find(
+              `[data-test-subj="user-action-${props.data.comments[0].id}"] [data-test-subj="user-action-markdown-form"]`
+            )
+            .exists()
+        ).toEqual(false);
+        expect(patchComment).toBeCalledWith({
+          commentUpdate: sampleData.content,
+          caseId: props.data.id,
+          commentId: props.data.comments[0].id,
+          fetchUserActions,
+          updateCase,
+          version: props.data.comments[0].version,
+        });
       });
     });
   });
@@ -269,15 +271,16 @@ describe('UserActionTree ', () => {
       .first()
       .simulate('click');
     await act(async () => {
-      await wait();
-      expect(
-        wrapper
-          .find(
-            `[data-test-subj="user-action-${props.data.id}"] [data-test-subj="user-action-markdown-form"]`
-          )
-          .exists()
-      ).toEqual(false);
-      expect(onUpdateField).toBeCalledWith({ key: 'description', value: sampleData.content });
+      await waitFor(() => {
+        expect(
+          wrapper
+            .find(
+              `[data-test-subj="user-action-${props.data.id}"] [data-test-subj="user-action-markdown-form"]`
+            )
+            .exists()
+        ).toEqual(false);
+        expect(onUpdateField).toBeCalledWith({ key: 'description', value: sampleData.content });
+      });
     });
   });