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

[Chore] upgrade Jest to v29 #7821

Merged
merged 20 commits into from
Jun 27, 2024
Merged

Conversation

mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Jun 10, 2024

Summary

closes #6813
closes #7322

Note

This PR upgrades jest and all related dependencies (including jsdom) to their latest versions. With that EUI will be using jest version ^29.7.0 and jsdom version 24.1.0 now.

Changes

Note

The upgrade resulted in 226 failed unit tests and 84 failed snapshot tests

Dependencies

1. 📦 Updated main jest testing dependencies

  • jest - ^29.7.0
  • jest-cli - ^29.7.0
  • @types/jest - ^29.5.12
  • babel-jest: ^29.7.0
  • eslint-plugin-jest: ^28.5.0

🔗 See the change here

2. 📦 added jest-environment-jsdom dependency

Since version 28 jest does not include the environment by default anymore. We're adding it manually now.

  • jest-environment-jsdom - ^29.7.0

🔗 See the change here

It's enabled in the jest config (packages/eui/scripts/jest/config.js) via testEnvironment option:

testEnvironment: 'jsdom'

🔗 See the change here

Setup

1. 🧪 update getCacheDirectory import

We can't import from the file directly anymore from the jest-config package subdirectory as it's not covered by the exports option defined in its package.json file. We can import the config and use the defined cacheDirectory default option instead (jest-config).

// old
const getCacheDirectory =
  require('jest-config/build/getCacheDirectory').default;

// new
const jestConfig = require('jest-config');
const getCacheDirectory = () => jestConfig.defaults.cacheDirectory;

🔗 See the change here

2. 🧪 add modulenameMapper option for uuid as it causes syntax erros due to using es6 syntax

moduleNameMapper: {
    '^uuid$': require.resolve('uuid'),
}

🔗 See the change here

3. 📦 fix jsdom version to a newer version than used by jest-environment-jsdom

With the update there were tests failing on color value changes and some of these showed that hsl() inputs are a) no longer output as is and b) not output correctly. (hsl values would show up as rgb(0, 0, 0)

ℹ️ It's expected that jsdom converts color values to rgb values and it uses cssstyle for that. There was a fix for the wrongly converted hsl values from version 4.0.0 (changelog) which is available from jsdom version 23.1.0 (changelog).
The latest jest-environment-jsdom version 29.7.0 however still uses jsdom version 20.0.0 and the most recent alpha version 30.0.0-alpha.1 only uses 22.0.0 (changelog)

💡 Instead we can use resolutions to enforce jsdom version 24.0.0 for now which includes the hsl conversion fix and did not seem to affect any other tests.

"resolutions": {
    "jsdom": "24.1.0"
}

🔗 See change here

Important

After this upgrade all color values are output as converted rgb for testing (unit and snapshot) which means we need to assert color checks using rgb values.

Broken tests

1. 🎨 Updates to test and snapshots as general side effects due to changes in the versions

There were a few generic reasons tests needed updating, which are:

  • color value conversion changes (mentioned in point 5 above)
    • this caused some snapshot updates as well as updates to test assertions (changes 1, 2, 3, 4, 5, 6)
  • newly supported css features (e.g. aspect-ratio, cal(), css properties) (changes 1, 2, 3)
  • js syntax changes
    • lots of snapshot updates are only related to using shorthand syntax (Object {} vs {} or Array [] vs [])

2. 💥 Further updates due to broken test functionality

There were some tests that broke in functionality and the main point of friction was handling focus in enzyme tests.
As we're migrating to RTL (testing-library/react) this was a good opportunity to update failing tests to use RTL which generally also fixed the issues.

Note

This PR only updates the min. required parts of tests to RTL to ensure tests are passing
This is to keep within the scope of the tasks. For some larger tests (I'm looking at you, EuiDataGrid) this still means that many updates to the code were done as there where many usages of updated functionality.

QA

  • passing pipeline should give us confidence that the upgrade works
  • review value changes in snapshots and test assertions if nothing's off or unexpected

@mgadewoll mgadewoll added testing Issues or PRs that only affect tests - will not need changelog entries skip-changelog labels Jun 10, 2024
@mgadewoll mgadewoll force-pushed the jest/6813-upgrade-v29 branch from c898270 to 74f17da Compare June 11, 2024 09:18
@mgadewoll mgadewoll changed the title [Jest] upgrade to v29 [Chore] upgrade Jest to v29 Jun 11, 2024
@mgadewoll mgadewoll marked this pull request as ready for review June 11, 2024 12:17
@mgadewoll mgadewoll requested a review from a team as a code owner June 11, 2024 12:17
@mgadewoll mgadewoll force-pushed the jest/6813-upgrade-v29 branch from 8868bb3 to d2d12cc Compare June 13, 2024 17:48
@tkajtoch tkajtoch self-requested a review June 17, 2024 14:14
@mgadewoll mgadewoll force-pushed the jest/6813-upgrade-v29 branch from d2d12cc to 9ee49dd Compare June 18, 2024 11:41
@tkajtoch
Copy link
Member

I'm in the process of reviewing this PR!

);

mockCell.append(mockPopoverAnchor);
// NOTE: [issue for React 16/17] we need to append the cell to the body instead of the container
Copy link
Member

Choose a reason for hiding this comment

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

Is this only related to jest v29?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It did show up in the migration but I can't fully answer this, as I did not convert this to RTL with jest v24 but did the migration to jest v29 and conversion to RTL at the same time 😅
It might be a difference in how/when RTL and Enzyme render/mount things to the DOM also? 🤷‍♀️

@mgadewoll mgadewoll mentioned this pull request Jun 25, 2024
27 tasks
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

The changes look and work great! Thank you for putting effort into converting more tests to RTL 😄

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@mgadewoll mgadewoll merged commit c76492c into elastic:main Jun 27, 2024
5 checks passed
@mgadewoll mgadewoll deleted the jest/6813-upgrade-v29 branch June 27, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog testing Issues or PRs that only affect tests - will not need changelog entries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade jsdom to v15+ Upgrade jest and all related dependencies to latest 29.x
4 participants