[React@18] fix outstanding easy unit tests #206917

Dosant · 2025-01-16T10:16:53Z

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 (legacy mode):
To run with react@18:

REACT_18=true yarn test:jest src/platform/plugins/private/kibana_overview/public/components/overview/overview.test.tsx

Also to run the app with React@18 please use this pr #196673 or from main:

REACT_18=true yarn bootstrap
REACT_18=true yarn start

Dosant · 2025-01-16T10:30:07Z

packages/kbn-test/src/jest/setup/enzyme.js

+ * 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() {


I described the problem in the comment, but here is the thread with investigation for even more context https://elastic.slack.com/archives/C7QC1JV6F/p1736790569444059

The problem is that snapshot serialization didn't work with React@18 and import {render} from 'enzyme' .

- class="euiText emotion-euiText-m" + class="euiText css-1py2xv4-euiText-m"

Longer term we should get rid of enzyme. I plan to start this conversation post react@18 work

Dosant · 2025-01-16T10:30:53Z

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) {


just wanted to reduce noise on ci

Dosant · 2025-01-16T11:10:33Z

...latform/packages/shared/kbn-alerts-ui-shared/src/alert_filter_controls/filter_group.test.tsx

+      const componentErrors = consoleErrorSpy.mock.calls.filter(
+        (c) =>
+          !c[0]?.startsWith?.(
+            'Warning: ReactDOM.render'


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. Since this is the only place that got broken because of it, I haven't come up with anything better then just adjusting in place :)

This test is so weird. Ideally we should not check the console log but this is a conversation for another day 🙂.

...gins/shared/lens/public/datasources/form_based/operations/definitions/ranges/ranges.test.tsx

Dosant · 2025-01-16T11:13:16Z

...ggers_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();


fixing [job] [logs] Jest Tests #4 / ConnectorForm calls onChange when the form is invalid

I assumed that runAllTimers caused tooltip to close before we checked that it is present 🤷‍♂️

Dosant · 2025-01-16T11:14:46Z

...ility/plugins/infra/public/components/asset_details/tabs/dashboards/actions/actions.test.tsx

-    expect(screen.getByTestId('infraUnLinkDashboardMenu')).toBeDisabled();
-    expect(screen.getByTestId('infraUnLinkDashboardMenu')).toHaveTextContent('Unlink dashboard');
+
+  describe('UnlinkDashboard', () => {


@eokoneyo fixed [job] [logs] Jest Tests #2 / Custom Dashboards Actions should render the unlink dashboard action when the user can unlink a dashboard

[job] [logs] Jest Tests #2 / Custom Dashboards Actions should render the unlink dashboard action when the user cannot unlink a dashboard

Added this in to fix the issue here, the mocked hook here requires another hook that would have required a mock, hence the need to mock this.

Dosant · 2025-01-16T11:16:20Z

...on/public/common/components/filter_by_assignees_popover/filter_by_assignees_popover.test.tsx

@@ -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));


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

Looks like with an update the last event can be missed before the assertion. By using fireEvent we wrap each event in act allowing tests to process pending events.

Dosant · 2025-01-16T11:16:53Z

...plugins/security_solution/public/flyout/document_details/right/components/assignees.test.tsx

-    getByTestId(ASSIGNEES_ADD_BUTTON_TEST_ID).click();
-    getByText('User 1').click();
-    getByText('User 3').click();
+    fireEvent.click(getByTestId(ASSIGNEES_ADD_BUTTON_TEST_ID));


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

Looks like with an update the last event can be missed before the assertion. By using fireEvent we wrap each event in act allowing tests to process pending events.

Dosant · 2025-01-16T11:19:08Z

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


fix [job] [logs] Jest Tests #9 / useOnExpandableFlyoutClose should run the callback function and remove the event listener from the window

I just assume this test wasn't testing what was intended. It might be that removeEventListener was called on the initial render during effect re-rerun and that is what the assertion was cathing.

elasticmachine · 2025-01-16T12:42:38Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

dmlemeshko

kbn-test changes lgtm

cnasikas · 2025-01-16T14:33:26Z

...latform/packages/shared/kbn-alerts-ui-shared/src/alert_filter_controls/filter_group.test.tsx

+      const componentErrors = consoleErrorSpy.mock.calls.filter(
+        (c) =>
+          !c[0]?.startsWith?.(
+            'Warning: ReactDOM.render'


This test is so weird. Ideally we should not check the console log but this is a conversation for another day 🙂.

rmyz

LGTM

Dosant · 2025-01-17T10:29:33Z

@elasticmachine merge upstream

kibanamachine · 2025-01-17T11:55:28Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12828554923

elasticmachine · 2025-01-17T11:56:02Z

💚 Build Succeeded

Buildkite Build
Commit: e5c9136

Metrics [docs]

✅ unchanged

History

💛 Build #267684 was flaky 31b9908

## Summary Extracted remaining easy backward-compatible unit test fixes that fail with React@18 from elastic#206411 The idea is that the tests should pass for both React@17 and React@18 (cherry picked from commit 3ca02b3)

kibanamachine · 2025-01-17T12:01:33Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.x`: - [[React@18] fix outstanding easy unit tests (#206917)](#206917)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Anton Dosov <[email protected]>

## Summary Extracted remaining easy backward-compatible unit test fixes that fail with React@18 from elastic#206411 The idea is that the tests should pass for both React@17 and React@18

fix easy jest test with react@18

31b9908

Dosant commented Jan 16, 2025

View reviewed changes

Dosant changed the title ~~fix easy jest test with react@18~~ [React@18] fix outstanding easy unit tests Jan 16, 2025

Dosant added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) React@18 labels Jan 16, 2025

Dosant marked this pull request as ready for review January 16, 2025 12:42

Dosant requested review from a team as code owners January 16, 2025 12:42

dmlemeshko approved these changes Jan 16, 2025

View reviewed changes

Dosant mentioned this pull request Jan 16, 2025

[NEEDS ACTION] Fix remaining unit tests for React@18 #206952

Open

5 tasks

cnasikas approved these changes Jan 16, 2025

View reviewed changes

jbudz approved these changes Jan 16, 2025

View reviewed changes

nickofthyme approved these changes Jan 16, 2025

View reviewed changes

PhilippeOberti approved these changes Jan 17, 2025

View reviewed changes

rmyz approved these changes Jan 17, 2025

View reviewed changes

Merge branch 'main' into d/2025-01-16-react-18-remaining-easy-jest-tests

e5c9136

Dosant enabled auto-merge (squash) January 17, 2025 10:30

Dosant merged commit 3ca02b3 into elastic:main Jan 17, 2025
8 checks passed

kibanamachine added the v9.0.0 label Jan 17, 2025

kibanamachine mentioned this pull request Jan 17, 2025

[8.x] [React@18] fix outstanding easy unit tests (#206917) #207060

Merged

kibanamachine added the v8.18.0 label Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[React@18] fix outstanding easy unit tests #206917

[React@18] fix outstanding easy unit tests #206917

Dosant commented Jan 16, 2025 •

edited by kibanamachine

Loading

Dosant Jan 16, 2025

Dosant Jan 16, 2025

Dosant Jan 16, 2025

cnasikas Jan 16, 2025

Dosant Jan 16, 2025

Dosant Jan 16, 2025

Dosant Jan 16, 2025

Dosant Jan 16, 2025

Dosant Jan 16, 2025

elasticmachine commented Jan 16, 2025

dmlemeshko left a comment

cnasikas Jan 16, 2025

rmyz left a comment

Dosant commented Jan 17, 2025

kibanamachine commented Jan 17, 2025

elasticmachine commented Jan 17, 2025

kibanamachine commented Jan 17, 2025

[React@18] fix outstanding easy unit tests #206917

[React@18] fix outstanding easy unit tests #206917

Conversation

Dosant commented Jan 16, 2025 • edited by kibanamachine Loading

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jan 16, 2025

dmlemeshko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmyz left a comment

Choose a reason for hiding this comment

Dosant commented Jan 17, 2025

kibanamachine commented Jan 17, 2025

elasticmachine commented Jan 17, 2025

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Jan 17, 2025

💚 All backports created successfully

Questions ?

Dosant commented Jan 16, 2025 •

edited by kibanamachine

Loading