Skip to content

Commit

Permalink
[React@18] fix outstanding easy unit tests (#206917)
Browse files Browse the repository at this point in the history
## Summary

Extracted remaining easy backward-compatible unit test fixes that fail
with React@18 from #206411

The idea is that the tests should pass for both React@17 and React@18
  • Loading branch information
Dosant authored Jan 17, 2025
1 parent 7bafc0b commit 3ca02b3
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 131 deletions.
28 changes: 28 additions & 0 deletions packages/kbn-test/src/jest/setup/enzyme.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
};
});
4 changes: 3 additions & 1 deletion packages/kbn-test/src/jest/setup/react_testing_library.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: '' },
],
},
},
});
},
});
});

Expand All @@ -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' },
],
},
},
});
},
});
});

Expand All @@ -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: '' }],
},
},
});
},
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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(
<UnlinkDashboard
onRefresh={() => {}}
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(
<UnlinkDashboard
onRefresh={() => {}}
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(
<UnlinkDashboard
onRefresh={() => {}}
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(
<UnlinkDashboard
onRefresh={() => {}}
assetType="host"
currentDashboard={TEST_CURRENT_DASHBOARD}
/>
);
expect(screen.getByTestId('infraUnLinkDashboardMenu')).toBeDisabled();
expect(screen.getByTestId('infraUnLinkDashboardMenu')).toHaveTextContent('Unlink dashboard');
});
});
});
Loading

0 comments on commit 3ca02b3

Please sign in to comment.