Merged
Conversation
c898270 to
74f17da
Compare
1 task
8868bb3 to
d2d12cc
Compare
fix config related issues
- updates are related to newly supported css, underlying color conversion changes and syntax changes of the snapshot generation
updates EuiPopover handleStrandedFocus condition as on mount with isOpen the stranded focus is on the panel
- updates all instances of openPopover and closePopover to RTL to not mix testing approaches between them
d2d12cc to
9ee49dd
Compare
Member
|
I'm in the process of reviewing this PR! |
tkajtoch
reviewed
Jun 24, 2024
| ); | ||
|
|
||
| mockCell.append(mockPopoverAnchor); | ||
| // NOTE: [issue for React 16/17] we need to append the cell to the body instead of the container |
Member
There was a problem hiding this comment.
Is this only related to jest v29?
Contributor
Author
There was a problem hiding this comment.
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? 🤷♀️
27 tasks
tkajtoch
reviewed
Jun 26, 2024
tkajtoch
reviewed
Jun 26, 2024
tkajtoch
approved these changes
Jun 26, 2024
Member
tkajtoch
left a comment
There was a problem hiding this comment.
The changes look and work great! Thank you for putting effort into converting more tests to RTL 😄
|
Preview staging links for this PR:
|
Collaborator
💚 Build Succeeded
History
|
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
closes #6813
closes #7322
Note
This PR upgrades
jestand all related dependencies (includingjsdom) to their latest versions. With that EUI will be usingjestversion^29.7.0andjsdomversion24.1.0now.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.0jest-cli-^29.7.0@types/jest-^29.5.12babel-jest:^29.7.0eslint-plugin-jest:^28.5.0🔗 See the change here
2. 📦 added
jest-environment-jsdomdependencySince version 28
jestdoes 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) viatestEnvironmentoption:testEnvironment: 'jsdom'🔗 See the change here
Setup
1. 🧪 update
getCacheDirectoryimportWe can't import from the file directly anymore from the
jest-configpackage subdirectory as it's not covered by theexportsoption defined in itspackage.jsonfile. We can import the config and use the definedcacheDirectorydefault option instead (jest-config).🔗 See the change here
2. 🧪 add
modulenameMapperoption foruuidas it causes syntax erros due to using es6 syntax🔗 See the change here
3. 📦 fix
jsdomversion to a newer version than used byjest-environment-jsdomWith 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 asrgb(0, 0, 0)ℹ️ It's expected that
jsdomconverts color values torgbvalues and it usescssstylefor that. There was a fix for the wrongly convertedhslvalues from version4.0.0(changelog) which is available fromjsdomversion23.1.0(changelog).The latest
jest-environment-jsdomversion29.7.0however still usesjsdomversion20.0.0and the most recent alpha version30.0.0-alpha.1only uses22.0.0(changelog)💡 Instead we can use
resolutionsto enforcejsdomversion24.0.0for now which includes thehslconversion fix and did not seem to affect any other tests.🔗 See change here
Important
After this upgrade all color values are output as converted
rgbfor testing (unit and snapshot) which means we need to assert color checks usingrgbvalues.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:
aspect-ratio,cal(), css properties) (changes 1, 2, 3)Object {}vs{}orArray []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