Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[React@18] Run jest tests #206411

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,13 @@ export async function pickTestGroupRunOrder() {
* For example in code coverage pipeline definition, it is "limited"
* to 'unit,integration'. This means FTR tests will not be executed.
*/
const LIMIT_CONFIG_TYPE = process.env.LIMIT_CONFIG_TYPE
? process.env.LIMIT_CONFIG_TYPE.split(',')
.map((t) => t.trim())
.filter(Boolean)
: ['unit', 'integration', 'functional'];
// const LIMIT_CONFIG_TYPE = process.env.LIMIT_CONFIG_TYPE
// ? process.env.LIMIT_CONFIG_TYPE.split(',')
// .map((t) => t.trim())
// .filter(Boolean)
// : ['unit', 'integration', 'functional'];

const LIMIT_CONFIG_TYPE = ['unit'];

const FTR_CONFIG_PATTERNS = process.env.FTR_CONFIG_PATTERNS
? process.env.FTR_CONFIG_PATTERNS.split(',')
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1228,10 +1228,10 @@
"rbush": "^3.0.1",
"re-resizable": "^6.9.9",
"re2js": "0.4.3",
"react": "^17.0.2",
"react": "~18.2.0",
"react-18": "npm:react@~18.2.0",
"react-diff-view": "^3.2.1",
"react-dom": "^17.0.2",
"react-dom": "~18.2.0",
"react-dom-18": "npm:react-dom@~18.2.0",
"react-dropzone": "^4.2.9",
"react-fast-compare": "^2.0.4",
Expand Down Expand Up @@ -1649,7 +1649,7 @@
"@types/react-router": "^5.1.20",
"@types/react-router-config": "^5.0.7",
"@types/react-router-dom": "^5.3.3",
"@types/react-test-renderer": "^17.0.2",
"@types/react-test-renderer": "~18.0.0",
"@types/react-virtualized": "^9.21.30",
"@types/react-window": "^1.8.8",
"@types/react-window-infinite-loader": "^1.0.9",
Expand Down Expand Up @@ -1821,7 +1821,7 @@
"proxy": "^2.1.1",
"raw-loader": "^3.1.0",
"react-is": "^17.0.2",
"react-test-renderer": "^17.0.2",
"react-test-renderer": "~18.2.0",
"recast": "^0.23.9",
"regenerate": "^1.4.0",
"resolve": "^1.22.0",
Expand Down
24 changes: 24 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,27 @@ 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'`
*/
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(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing [job] [logs] Jest Tests #3 / Filter Group Component Filter Changed Banner should ignore url params if there is an error in using them

The problem is that when running React@18 in legacy mode there is an additional log message that warns about running in legacy mode.

(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 @@ -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();
});
eokoneyo marked this conversation as resolved.
Show resolved Hide resolved

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');
});
});
});

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -110,14 +110,14 @@ describe('<FilterByAssigneesPopover />', () => {
const onUsersChangeMock = jest.fn();
const { getByTestId, getByText } = renderFilterByAssigneesPopover([], onUsersChangeMock);

getByTestId(FILTER_BY_ASSIGNEES_BUTTON).click();
fireEvent.click(getByTestId(FILTER_BY_ASSIGNEES_BUTTON));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes [job] [logs] Jest Tests #8 / should call onUsersChange on closing the popover

Looks like with an update to React@18 (even in legacy mode) these events can be skipped. By using fireEvent we wrap each event in act allowing tests to process pending events.


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([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -132,12 +132,12 @@ describe('<Assignees />', () => {
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));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[job] [logs] Jest Tests #9 / should call assignees update functionality with the right arguments

Looks like with an update to React@18 (even in legacy mode) these events can be skipped. By using fireEvent we wrap each event in act allowing tests to process pending events.

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(
{
Expand Down
Loading