From 47a5ab47cf04ab7e6e19eb5bceebc5716cfcbf96 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 15 Jul 2020 19:07:02 -0500 Subject: [PATCH] [Security][Detections] Unskip failing modal tests (#71969) * Revert "Skip jest tests that timeout waiting for react" This reverts commit dd9b0b3274eaa37a55c651d2c10d37f5a523da14. * Unmount async effectful components instead of waiting for them A previous commit introduced waitForUpdates as a solution to the warnings introduced by https://github.com/enzymejs/enzyme/issues/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). --- .../public/common/utils/test_utils.ts | 16 --------- .../form.test.tsx | 12 ++----- .../modal.test.tsx | 19 +++++------ .../public/overview/pages/overview.test.tsx | 33 +++++++++---------- 4 files changed, 25 insertions(+), 55 deletions(-) delete mode 100644 x-pack/plugins/security_solution/public/common/utils/test_utils.ts diff --git a/x-pack/plugins/security_solution/public/common/utils/test_utils.ts b/x-pack/plugins/security_solution/public/common/utils/test_utils.ts deleted file mode 100644 index 5a3cddb74657..000000000000 --- a/x-pack/plugins/security_solution/public/common/utils/test_utils.ts +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { ReactWrapper } from 'enzyme'; -import { act } from 'react-dom/test-utils'; - -// Temporary fix for https://github.com/enzymejs/enzyme/issues/2073 -export const waitForUpdates = async

(wrapper: ReactWrapper

) => { - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 0)); - wrapper.update(); - }); -}; diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx index ce5d19259e9e..e2e793b34eaf 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx @@ -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'; @@ -30,10 +29,6 @@ const mockSelectFile:

(container: ReactWrapper

, file: File) => Promise { @@ -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 })); }); @@ -80,12 +74,11 @@ describe('ValueListsForm', () => { })); const onError = jest.fn(); - const container = mount( + mount( ); - await waitForUpdates(container); expect(onError).toHaveBeenCalledWith('whoops'); }); @@ -97,12 +90,11 @@ describe('ValueListsForm', () => { })); const onSuccess = jest.fn(); - const container = mount( + mount( ); - await waitForUpdates(container); expect(onSuccess).toHaveBeenCalledWith({ mockResult: true }); }); diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx index ab2bc9b2e90e..175882de551c 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx @@ -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( @@ -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( ); - 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( @@ -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( ); - await waitForUpdates(container); - expect(container.find('ValueListsForm')).toHaveLength(1); expect(container.find('ValueListsTable')).toHaveLength(1); + container.unmount(); }); }); diff --git a/x-pack/plugins/security_solution/public/overview/pages/overview.test.tsx b/x-pack/plugins/security_solution/public/overview/pages/overview.test.tsx index f7c77bc2dfdf..286cc870378e 100644 --- a/x-pack/plugins/security_solution/public/overview/pages/overview.test.tsx +++ b/x-pack/plugins/security_solution/public/overview/pages/overview.test.tsx @@ -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 { @@ -65,7 +64,7 @@ describe('Overview', () => { mockuseMessagesStorage.mockImplementation(() => endpointNoticeMessage(false)); }); - it('renders the Setup Instructions text', async () => { + it('renders the Setup Instructions text', () => { const wrapper = mount( @@ -73,11 +72,10 @@ describe('Overview', () => { ); - 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( @@ -85,11 +83,10 @@ describe('Overview', () => { ); - 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( @@ -98,12 +95,11 @@ describe('Overview', () => { ); - 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: {}, @@ -120,12 +116,12 @@ describe('Overview', () => { ); - 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: {}, @@ -147,12 +143,12 @@ describe('Overview', () => { ); - 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: {}, @@ -174,12 +170,12 @@ describe('Overview', () => { ); - 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: {}, @@ -196,12 +192,12 @@ describe('Overview', () => { ); - 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: {}, @@ -219,9 +215,10 @@ describe('Overview', () => { ); 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: {}, @@ -238,9 +235,9 @@ describe('Overview', () => { ); - await waitForUpdates(wrapper); expect(wrapper.find('[data-test-subj="endpoint-prompt-banner"]').exists()).toBe(false); + wrapper.unmount(); }); }); });