Build: Upgrade Jest to 29 #19863
Build: Upgrade Jest to 29 #19863
Conversation
|
@IanVS Just did a quick commit, to see if we can remove run in band with jest 29, to see if we can speed things up again in CI. |
Do we know for sure that the flag slowed things down? It seemed like it was finishing in approximately the same time as before, given that we are now running a few more tests as well. |
|
@IanVS Removing runInBand made it crash indeed, but setting maxWorkers=4 seems to be green:
When I initially removed runInBand a couple of weeks ago, I saw a consistent 2.6 improvement. Just comparing those 2 gives 2.4 time improvement. I think by default it will use 7 workers (given 8 cores in xlarge). |
|
We will also want to update igor-dv/jest-specific-snapshot#44 once it's merged and released, but I don't think it needs to block this PR. |
| @@ -82,7 +82,7 @@ | |||
| "@storybook/theming": "7.0.0-alpha.50", | |||
| "@storybook/types": "7.0.0-alpha.50", | |||
| "global": "^4.4.0", | |||
| "jest-mock": "^27.0.6", | |||
| "jest-mock": "^29.3.1", | |||
There was a problem hiding this comment.
I don't think we can update this, since it's an actual dependency that is used by the interactions addon, and so far it only supports jest 27, AFAIK
There was a problem hiding this comment.
Aha, will downgrade, cc @yannbf
|
🎉 Is it on purpose this was skipped? |
|
@SimenB no, thanks for the catch. I think I missed it since I was searching for dependencies with the string |
| @@ -85,7 +85,7 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@babel/plugin-transform-react-jsx": "^7.19.0", | |||
| "@jest/transform": "^28.0.0", | |||
| "@jest/transform": "^29.3.1", | |||
There was a problem hiding this comment.
@kasperpeulen I see that you updated @jest/transform here and in storyshots. Was that intentional? This is a dependency that is used by addon-docs, and my storybook is now crashing with an error message coming from jest-util via @jest/transform.
Uncaught TypeError: Cannot read properties of undefined (reading 'isTTY')
at ./node_modules/jest-util/build/isInteractive.js
Seems to come from https://github.com/facebook/jest/blob/e7edb75f8357090714213f252c5aaaa9b3c29f5f/packages/jest-util/src/isInteractive.ts#L10. We can't run that in the browser, since process isn't defined there.
There was a problem hiding this comment.
Hm... intentional in the sense that I thought it would be nice to upgrade all the jest-related deps from 28 to 29, but I don't know about this package. So downgrading seems fine to me.
There was a problem hiding this comment.
should be trivial to replace process with a fake object for browser usage, no?
There was a problem hiding this comment.
I'm sure we can deal with it, yes. I just like to keep user facing changes separate from internal test updates.
@SimenB since you're here, is this something that changed from 28 to 29? I took a quick look and couldn't immediately figure out why this would start to fail now in 29. Maybe you know offhand?
There was a problem hiding this comment.
Hm, I upgraded to the latest beta (beta.8), and I no longer get this error. I guess we can just keep our eyes out for it again.
There was a problem hiding this comment.
Nothing comes to mind, that code has been the same for years
Issue:
What I did
This bumps jest to the latest version, 29.3.1, along with a few other related jest packages.
I needed to break up the config a little, because I started getting validation messages about the "global" config options that I needed to move to the top level.
How to test