Update Jest to latest version, and use optimized JSDOM #53736
Merged
Conversation
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
jsnajdr
added
[Type] Enhancement
jsnajdr
requested review from
ajitbohra,
ellatrix,
kevin940726,
nerrad,
ntwb and
ryanwelcher
as code owners
|
Size Change: 0 B Total Size: 1.51 MB ℹ️ View Unchanged
|
jsnajdr
force-pushed
the
update/jest-jsdom-optimized
branch
from
0c35d84 to
e18beb9
Compare
Member
Author
|
Peformance tests are repeatedly failing, the only explanation is that we screwed some packages in the lockfile. I now rebased onto the latest trunk, hoping it helps. |
tyxla
approved these changes
Aug 17, 2023
Member
|
I remember times before when we could execute unit tests in less than 30 seconds 😄 Happy to see that you keep checking how performance can get improved. |
Member
Author
That can always be fixed by removing some tests 😉 When running the unit test suite locally, I see that many warnings and errors are scrolling by as the tests run. That's expected for tests that specifically test for error conditions, but many other warnings probably shouldn't be there. I suspect that many of our tests are half-broken. But not broken enough to make them fail. That could also have some infuence on running times. |
Member
|
In fact, printing to the console might have a bigger impact on the execution time than people would think. We maintain hundreds of test files, and thousands of tests so that takes some time to even log all the test files. |
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.
This PR adds an
overridesfield topackage.jsonto force JSDOM v22 that includes a.getComputedStyleoptimization. This should speed up unit test runs. Although for some reason I don't see convincing results locally, the improvement is from like 50 seconds to 48 seconds, very small. Let's see if CI results are better.I'm also upgrading Jest from 29.5.0 to 29.6.2 in this PR.
npm installfor the changedpackage.jsonwas installing some 29.6.2 dependencies anyway, so I decided to do a proper upgrade.Updating JSDOM to v22 has two other consequences that needed some tests to be fixed:
http://\u001f!\"$&'()*+,-.;=_{}~/"There are very technical reasons, related to TR46 localized domain names, why this URL used to be valid and now no longer is.window.getComputedStylein JSDOM now returns "resolved" color values, i.e.,color: blackis resolved tocolor: rgb(0,0,0). This is the correct behavior, and instances of the.toHaveStylesJest helper need to be updated accordingly.How to test:
The changes affect only unit tests, both web and mobile. There should be no impact on production code.