From 3ca02b324051adfa54965cfc3985abdc2a93b8a3 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Fri, 17 Jan 2025 12:55:08 +0100 Subject: [PATCH] [React@18] fix outstanding easy unit tests (#206917) ## Summary Extracted remaining easy backward-compatible unit test fixes that fail with React@18 from https://github.com/elastic/kibana/pull/206411 The idea is that the tests should pass for both React@17 and React@18 --- packages/kbn-test/src/jest/setup/enzyme.js | 28 +++ .../src/jest/setup/react_testing_library.js | 4 +- .../filter_group.test.tsx | 10 +- .../definitions/ranges/ranges.test.tsx | 165 +++++++++--------- .../action_type_form.test.tsx | 2 +- .../tabs/dashboards/actions/actions.test.tsx | 86 ++++++--- .../filter_by_assignees_popover.test.tsx | 16 +- .../right/components/assignees.test.tsx | 10 +- .../use_on_expandable_flyout_close.test.tsx | 5 +- 9 files changed, 195 insertions(+), 131 deletions(-) diff --git a/packages/kbn-test/src/jest/setup/enzyme.js b/packages/kbn-test/src/jest/setup/enzyme.js index d101358fd5b4a..aa4a629303bfa 100644 --- a/packages/kbn-test/src/jest/setup/enzyme.js +++ b/packages/kbn-test/src/jest/setup/enzyme.js @@ -11,3 +11,31 @@ import { configure } from 'enzyme'; import Adapter from '@wojtekmaj/enzyme-adapter-react-17'; configure({ adapter: new Adapter() }); + +/* eslint-env jest */ + +/** + * This is a workaround to fix snapshot serialization of emotion classes when rendering React@18 using `import { render } from 'enzyme'` + * With React@18 emotion uses `useInsertionEffect` to insert styles into the DOM, which enzyme `render` does not trigger because it is using ReactDomServer.renderToString. + * With React@17 emotion fell back to sync cb, so it was working with enzyme `render`. + * Without those styles in DOM the custom snapshot serializer is not able to replace the emotion class names. + * This workaround ensures a fake emotion style tag is present in the DOM before rendering the component with enzyme making the snapshot serializer work. + */ +function mockEnsureEmotionStyleTag() { + if (!document.head.querySelector('style[data-emotion]')) { + const style = document.createElement('style'); + style.setAttribute('data-emotion', 'css'); + document.head.appendChild(style); + } +} + +jest.mock('enzyme', () => { + const actual = jest.requireActual('enzyme'); + return { + ...actual, + render: (node, options) => { + mockEnsureEmotionStyleTag(); + return actual.render(node, options); + }, + }; +}); diff --git a/packages/kbn-test/src/jest/setup/react_testing_library.js b/packages/kbn-test/src/jest/setup/react_testing_library.js index 7b184485df1cb..ce54bcf8326cb 100644 --- a/packages/kbn-test/src/jest/setup/react_testing_library.js +++ b/packages/kbn-test/src/jest/setup/react_testing_library.js @@ -80,6 +80,8 @@ console.error = (...args) => { * Tracking issue to clean this up https://github.com/elastic/kibana/issues/199100 */ if (REACT_VERSION.startsWith('18.')) { - console.warn('Running with React@18 and muting the legacy ReactDOM.render warning'); + if (!process.env.CI) { + console.warn('Running with React@18 and muting the legacy ReactDOM.render warning'); + } muteLegacyRootWarning(); } diff --git a/src/platform/packages/shared/kbn-alerts-ui-shared/src/alert_filter_controls/filter_group.test.tsx b/src/platform/packages/shared/kbn-alerts-ui-shared/src/alert_filter_controls/filter_group.test.tsx index 3d08697f98652..cfe5442f5473a 100644 --- a/src/platform/packages/shared/kbn-alerts-ui-shared/src/alert_filter_controls/filter_group.test.tsx +++ b/src/platform/packages/shared/kbn-alerts-ui-shared/src/alert_filter_controls/filter_group.test.tsx @@ -589,8 +589,14 @@ describe(' Filter Group Component ', () => { /> ); - expect(consoleErrorSpy.mock.calls.length).toBe(1); - expect(String(consoleErrorSpy.mock.calls[0][0])).toMatch(URL_PARAM_ARRAY_EXCEPTION_MSG); + const componentErrors = consoleErrorSpy.mock.calls.filter( + (c) => + !c[0]?.startsWith?.( + 'Warning: ReactDOM.render' + ) /* exclude react@18 legacy root warnings */ + ); + expect(componentErrors.length).toBe(1); + expect(String(componentErrors[0][0])).toMatch(URL_PARAM_ARRAY_EXCEPTION_MSG); }); }); diff --git a/x-pack/platform/plugins/shared/lens/public/datasources/form_based/operations/definitions/ranges/ranges.test.tsx b/x-pack/platform/plugins/shared/lens/public/datasources/form_based/operations/definitions/ranges/ranges.test.tsx index a7c6e436dcfab..e85f6d6f7d550 100644 --- a/x-pack/platform/plugins/shared/lens/public/datasources/form_based/operations/definitions/ranges/ranges.test.tsx +++ b/x-pack/platform/plugins/shared/lens/public/datasources/form_based/operations/definitions/ranges/ranges.test.tsx @@ -577,45 +577,44 @@ describe('ranges', () => { /> ); - // This series of act closures are made to make it work properly the update flush - act(() => { - instance.find(EuiButtonEmpty).simulate('click'); - }); + instance.find(EuiButtonEmpty).simulate('click'); act(() => { - // need another wrapping for this in order to work instance.update(); + }); - expect(instance.find(RangePopover)).toHaveLength(2); + expect(instance.find(RangePopover)).toHaveLength(2); - // edit the range and check - instance - .find('RangePopover input[type="number"]') - .first() - .simulate('change', { - target: { - value: '50', - }, - }); + // edit the range and check + instance + .find('RangePopover input[type="number"]') + .first() + .simulate('change', { + target: { + value: '50', + }, + }); + jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); - jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); + act(() => { + instance.update(); + }); - expect(updateLayerSpy).toHaveBeenCalledWith({ - ...layer, - columns: { - ...layer.columns, - col1: { - ...layer.columns.col1, - params: { - ...(layer.columns.col1 as RangeIndexPatternColumn).params, - ranges: [ - { from: 0, to: DEFAULT_INTERVAL, label: '' }, - { from: 50, to: Infinity, label: '' }, - ], - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...(layer.columns.col1 as RangeIndexPatternColumn).params, + ranges: [ + { from: 0, to: DEFAULT_INTERVAL, label: '' }, + { from: 50, to: Infinity, label: '' }, + ], }, }, - }); + }, }); }); @@ -632,45 +631,43 @@ describe('ranges', () => { /> ); - // This series of act closures are made to make it work properly the update flush - act(() => { - instance.find(EuiButtonEmpty).simulate('click'); - }); - + instance.find(EuiButtonEmpty).simulate('click'); act(() => { - // need another wrapping for this in order to work instance.update(); + }); + expect(instance.find(RangePopover)).toHaveLength(2); - expect(instance.find(RangePopover)).toHaveLength(2); + // edit the label and check + instance + .find('RangePopover input[type="text"]') + .first() + .simulate('change', { + target: { + value: 'customlabel', + }, + }); - // edit the label and check - instance - .find('RangePopover input[type="text"]') - .first() - .simulate('change', { - target: { - value: 'customlabel', - }, - }); + jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); - jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); + act(() => { + instance.update(); + }); - expect(updateLayerSpy).toHaveBeenCalledWith({ - ...layer, - columns: { - ...layer.columns, - col1: { - ...layer.columns.col1, - params: { - ...(layer.columns.col1 as RangeIndexPatternColumn).params, - ranges: [ - { from: 0, to: DEFAULT_INTERVAL, label: '' }, - { from: DEFAULT_INTERVAL, to: Infinity, label: 'customlabel' }, - ], - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...(layer.columns.col1 as RangeIndexPatternColumn).params, + ranges: [ + { from: 0, to: DEFAULT_INTERVAL, label: '' }, + { from: DEFAULT_INTERVAL, to: Infinity, label: 'customlabel' }, + ], }, }, - }); + }, }); }); @@ -687,37 +684,37 @@ describe('ranges', () => { /> ); - // This series of act closures are made to make it work properly the update flush + instance.find(RangePopover).find(EuiLink).find('button').simulate('click'); act(() => { - instance.find(RangePopover).find(EuiLink).find('button').simulate('click'); + instance.update(); }); + instance + .find('RangePopover input[type="number"]') + .last() + .simulate('change', { + target: { + value: '50', + }, + }); + jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); + act(() => { - // need another wrapping for this in order to work instance.update(); - instance - .find('RangePopover input[type="number"]') - .last() - .simulate('change', { - target: { - value: '50', - }, - }); - jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); + }); - expect(updateLayerSpy).toHaveBeenCalledWith({ - ...layer, - columns: { - ...layer.columns, - col1: { - ...layer.columns.col1, - params: { - ...(layer.columns.col1 as RangeIndexPatternColumn).params, - ranges: [{ from: 0, to: 50, label: '' }], - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...(layer.columns.col1 as RangeIndexPatternColumn).params, + ranges: [{ from: 0, to: 50, label: '' }], }, }, - }); + }, }); }); diff --git a/x-pack/platform/plugins/shared/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.test.tsx b/x-pack/platform/plugins/shared/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.test.tsx index 945ab23c43611..1e795a2cf3dd8 100644 --- a/x-pack/platform/plugins/shared/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.test.tsx +++ b/x-pack/platform/plugins/shared/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.test.tsx @@ -381,7 +381,7 @@ describe('action_type_form', () => { jest.useFakeTimers({ legacyFakeTimers: true }); wrapper.find('[data-test-subj="action-group-error-icon"]').first().simulate('mouseOver'); // Run the timers so the EuiTooltip will be visible - jest.runAllTimers(); + jest.runOnlyPendingTimers(); wrapper.update(); expect(wrapper.find('.euiToolTipPopover').last().text()).toBe('Action contains errors.'); // Clearing all mocks will also reset fake timers. diff --git a/x-pack/solutions/observability/plugins/infra/public/components/asset_details/tabs/dashboards/actions/actions.test.tsx b/x-pack/solutions/observability/plugins/infra/public/components/asset_details/tabs/dashboards/actions/actions.test.tsx index 27ae309078cad..bc1c59b3b8835 100644 --- a/x-pack/solutions/observability/plugins/infra/public/components/asset_details/tabs/dashboards/actions/actions.test.tsx +++ b/x-pack/solutions/observability/plugins/infra/public/components/asset_details/tabs/dashboards/actions/actions.test.tsx @@ -9,6 +9,7 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; import { EditDashboard, UnlinkDashboard, LinkDashboard } from '.'; import { useTabSwitcherContext } from '../../../hooks/use_tab_switcher'; +import * as fetchCustomDashboards from '../../../hooks/use_fetch_custom_dashboards'; import * as hooks from '../../../hooks/use_saved_objects_permissions'; const TEST_CURRENT_DASHBOARD = { @@ -114,34 +115,61 @@ describe('Custom Dashboards Actions', () => { expect(screen.getByTestId('infraLinkDashboardMenu')).toBeDisabled(); expect(screen.getByTestId('infraLinkDashboardMenu')).toHaveTextContent('Link new dashboard'); }); - it('should render the unlink dashboard action when the user can unlink a dashboard', () => { - jest.spyOn(hooks, 'useSavedObjectUserPermissions').mockImplementation(() => ({ - canSave: true, - canDelete: true, - })); - render( - {}} - assetType="host" - currentDashboard={TEST_CURRENT_DASHBOARD} - /> - ); - expect(screen.getByTestId('infraUnLinkDashboardMenu')).not.toBeDisabled(); - expect(screen.getByTestId('infraUnLinkDashboardMenu')).toHaveTextContent('Unlink dashboard'); - }); - it('should render the unlink dashboard action when the user cannot unlink a dashboard', () => { - jest.spyOn(hooks, 'useSavedObjectUserPermissions').mockImplementation(() => ({ - canSave: true, - canDelete: false, - })); - render( - {}} - assetType="host" - currentDashboard={TEST_CURRENT_DASHBOARD} - /> - ); - expect(screen.getByTestId('infraUnLinkDashboardMenu')).toBeDisabled(); - expect(screen.getByTestId('infraUnLinkDashboardMenu')).toHaveTextContent('Unlink dashboard'); + + describe('UnlinkDashboard', () => { + const fetchCustomDashboardsSpy = jest.spyOn(fetchCustomDashboards, 'useFetchCustomDashboards'); + + beforeEach(() => { + // provide mock for invocation to fetch custom dashboards + fetchCustomDashboardsSpy.mockReturnValue({ + dashboards: [ + { + id: 'test-so-id', + dashboardSavedObjectId: 'test-dashboard-id', + assetType: 'host', + dashboardFilterAssetIdEnabled: true, + }, + ], + loading: false, + error: null, + // @ts-expect-error we provide a mock function as we don't need to test the actual implementation + refetch: jest.fn(), + }); + }); + + afterEach(() => { + fetchCustomDashboardsSpy.mockClear(); + }); + + it('should render the unlink dashboard action when the user can unlink a dashboard', () => { + jest.spyOn(hooks, 'useSavedObjectUserPermissions').mockImplementation(() => ({ + canSave: true, + canDelete: true, + })); + render( + {}} + assetType="host" + currentDashboard={TEST_CURRENT_DASHBOARD} + /> + ); + expect(screen.getByTestId('infraUnLinkDashboardMenu')).not.toBeDisabled(); + expect(screen.getByTestId('infraUnLinkDashboardMenu')).toHaveTextContent('Unlink dashboard'); + }); + it('should render the unlink dashboard action when the user cannot unlink a dashboard', () => { + jest.spyOn(hooks, 'useSavedObjectUserPermissions').mockImplementation(() => ({ + canSave: true, + canDelete: false, + })); + render( + {}} + assetType="host" + currentDashboard={TEST_CURRENT_DASHBOARD} + /> + ); + expect(screen.getByTestId('infraUnLinkDashboardMenu')).toBeDisabled(); + expect(screen.getByTestId('infraUnLinkDashboardMenu')).toHaveTextContent('Unlink dashboard'); + }); }); }); diff --git a/x-pack/solutions/security/plugins/security_solution/public/common/components/filter_by_assignees_popover/filter_by_assignees_popover.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/common/components/filter_by_assignees_popover/filter_by_assignees_popover.test.tsx index fbfe0eb705b91..5e912e9e5c71d 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/common/components/filter_by_assignees_popover/filter_by_assignees_popover.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/common/components/filter_by_assignees_popover/filter_by_assignees_popover.test.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { render } from '@testing-library/react'; +import { render, fireEvent } from '@testing-library/react'; import { FilterByAssigneesPopover } from './filter_by_assignees_popover'; import { TestProviders } from '../../mock'; @@ -110,14 +110,14 @@ describe('', () => { const onUsersChangeMock = jest.fn(); const { getByTestId, getByText } = renderFilterByAssigneesPopover([], onUsersChangeMock); - getByTestId(FILTER_BY_ASSIGNEES_BUTTON).click(); + fireEvent.click(getByTestId(FILTER_BY_ASSIGNEES_BUTTON)); - getByText('User 1').click(); - getByText('User 2').click(); - getByText('User 3').click(); - getByText('User 3').click(); - getByText('User 2').click(); - getByText('User 1').click(); + fireEvent.click(getByText('User 1')); + fireEvent.click(getByText('User 2')); + fireEvent.click(getByText('User 3')); + fireEvent.click(getByText('User 3')); + fireEvent.click(getByText('User 2')); + fireEvent.click(getByText('User 1')); expect(onUsersChangeMock).toHaveBeenCalledTimes(6); expect(onUsersChangeMock.mock.calls).toEqual([ diff --git a/x-pack/solutions/security/plugins/security_solution/public/flyout/document_details/right/components/assignees.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/flyout/document_details/right/components/assignees.test.tsx index 3c27165f8cc11..32e27b36e25ac 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/flyout/document_details/right/components/assignees.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/flyout/document_details/right/components/assignees.test.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { render } from '@testing-library/react'; +import { render, fireEvent } from '@testing-library/react'; import { ASSIGNEES_ADD_BUTTON_TEST_ID, @@ -132,12 +132,12 @@ describe('', () => { const { getByTestId, getByText } = renderAssignees('test-event', assignees); // Update assignees - getByTestId(ASSIGNEES_ADD_BUTTON_TEST_ID).click(); - getByText('User 1').click(); - getByText('User 3').click(); + fireEvent.click(getByTestId(ASSIGNEES_ADD_BUTTON_TEST_ID)); + fireEvent.click(getByText('User 1')); + fireEvent.click(getByText('User 3')); // Apply assignees - getByTestId(ASSIGNEES_APPLY_BUTTON_TEST_ID).click(); + fireEvent.click(getByTestId(ASSIGNEES_APPLY_BUTTON_TEST_ID)); expect(setAlertAssigneesMock).toHaveBeenCalledWith( { diff --git a/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx index 2a4e5576e24bc..72039ea06a60b 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx @@ -21,7 +21,7 @@ describe('useOnExpandableFlyoutClose', () => { window.removeEventListener = jest.fn().mockImplementationOnce((event, callback) => {}); - renderHook(() => useOnExpandableFlyoutClose({ callback: callbackFct })); + const { unmount } = renderHook(() => useOnExpandableFlyoutClose({ callback: callbackFct })); window.dispatchEvent( new CustomEvent(TIMELINE_ON_CLOSE_EVENT, { @@ -30,6 +30,9 @@ describe('useOnExpandableFlyoutClose', () => { ); expect(callbackFct).toHaveBeenCalledWith(Flyouts.timeline); + + unmount(); + expect(window.removeEventListener).toBeCalled(); });