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] Fix remaining unit tests #207195

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Jan 20, 2025

Summary

Part of #206952

Fixes remaining unit tests that are failing with React@18

  • [job] [logs] Jest Tests / OpenTimelineButton should open the modal after clicking on the button
  • [job] [logs] Jest Tests / useTimelineTypes timelineFilters set timelineTypes correctly
  • [job] [logs] Jest Tests / When on the trusted applications page should search using expected exception item fields

The idea is that tests should pass with both React 17 and 18. To run a unit tests with React@18:

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

return {
...origin,
useDispatch: jest.fn(),
useDispatch: jest.fn(() => mockDispatch),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes

  • [job] [logs] Jest Tests / OpenTimelineButton should open the modal after clicking on the button

The problem was that with React@18 the test started getting to this dispatch call and it failed because instead of a function we mock returned undefined.

}
return prevTimelineTypes;
});
setTimelineTypes(tabId);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes

  • [job] [logs] Jest Tests / useTimelineTypes timelineFilters set timelineTypes correctly

Given the failed test, it seems there was a genuine runtime bug with React@18. It appears that the behavior of this pattern has changed in React@18 compared to React@17. I suspect that the original logic was lost over several refactorings and there is no need to nest setState in the same setState anymore, but please doublecheck

@Dosant Dosant added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Jan 20, 2025
@Dosant Dosant marked this pull request as ready for review January 20, 2025 13:34
@Dosant Dosant requested a review from a team as a code owner January 20, 2025 13:34
@elasticmachine
Copy link
Contributor

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

@Dosant Dosant requested a review from a team as a code owner January 20, 2025 14:02
@Dosant Dosant requested review from pzl and szwarckonrad January 20, 2025 14:02
@Dosant Dosant changed the title [React@18] Fix remaining unit tests security-threat-hunting-investigations [React@18] Fix remaining unit tests Jan 20, 2025
await waitFor(async () => {
await expect(findAllByTestId('trustedAppsListPage-card')).resolves.toHaveLength(10);
expect(getAllByTestId('trustedAppsListPage-card')).toHaveLength(10);
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 / When on the trusted applications page should search using expected exception item fields

Copy link
Contributor

@szwarckonrad szwarckonrad left a comment

Choose a reason for hiding this comment

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

DW changes LGTM, thanks for addressing this!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 21.2MB 21.2MB -114.0B

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) React@18 release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants