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

feat: jest 28 support #154

Merged
merged 9 commits into from Aug 5, 2022
Merged

feat: jest 28 support #154

merged 9 commits into from Aug 5, 2022

Conversation

andykenward
Copy link
Contributor

@andykenward andykenward commented Jul 24, 2022

CLOSES #99

A first pass at upgrading to Jest 28 only support.

As jest-playwright 2.0.0 has been upgraded to now support Jest 28. But it has a breaking change of no longer supporting older Jest versions.

Release Notes

Features

This release updates jest-playwright to version 2.0.0 which adds support for Jest 28. In order to maintain backwards compatibility with Jest 27, you might have to take a few steps in case you are installing the test runner for the first time, or if you don't keep package locks in your project.

You can find more info at https://github.com/storybookjs/test-runner#jest-27-support

Version

Published prerelease version: v0.6.0-next.0

Changelog

Release Notes

feat: jest 28 support (#154)

Features

This release updates jest-playwright to version 2.0.0 which adds support for Jest 28. In order to maintain backwards compatibility with Jest 27, you might have to take a few steps in case you are installing the test runner for the first time, or if you don't keep package locks in your project.

You can find more info at https://github.com/storybookjs/test-runner#jest-27-support


🚀 Enhancement

🐛 Bug Fix

  • docs(README): add coverage recipes (@yannbf)

Authors: 2

@andykenward andykenward marked this pull request as draft July 24, 2022 11:25
@andykenward
Copy link
Contributor Author

The yarn test-storybook was failing locally for me.

Had to update the playwright/transform.hs due to jest 28 change https://jestjs.io/docs/upgrading-to-jest28#transformer

@@ -15,6 +15,6 @@ module.exports = {
'@babel/preset-react',
],
});
return result ? result.code : src;
return result ? { code: result.code } : src;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Hey @andykenward thank you so much for your contribution! I added a few comments.

I am hoping we can still support both Jest versions, and the way to use Jest < 28 would be to just downgrade jest-playwright-preset. Please check if my comments make sense, I'd love to see if there is a way to make this less breaking for the users!

Also, feel free to turn this PR into non-draft, so that it releases canary versions that you can test in your project!

README.md Outdated Show resolved Hide resolved
bin/test-storybook.js Outdated Show resolved Hide resolved
bin/test-storybook.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
playwright/transform.js Outdated Show resolved Hide resolved
@andykenward andykenward marked this pull request as ready for review July 24, 2022 15:44
@andykenward andykenward changed the title feat: jest 28 only support feat: jest 28 support Jul 24, 2022
@andykenward
Copy link
Contributor Author

Reverted some changes so we still support older Jest and Jest 28 at the same time.

@yannbf yannbf added the minor Increment the minor version when merged label Jul 24, 2022
@IanVS
Copy link
Member

IanVS commented Jul 25, 2022

@yannbf was a canary version published? I can't find it, but I'd love to test this out in my own project.

@IanVS IanVS requested a review from yannbf July 25, 2022 15:52
@yannbf
Copy link
Member

yannbf commented Jul 26, 2022

Hey @andykenward @IanVS I manually released this as @storybook/test-runner@0.6.0--canary.154.5e2f62a.0. Give it a go! I'm currently testing it out

@yannbf
Copy link
Member

yannbf commented Jul 26, 2022

A few updates from my testing rounds:

0.5.0 version + Jest 27
image

Canary 0.6.0 version + Jest 28, first time running.
For some reason it took a loooong time to run:
image

But running again, it worked fine, same time as before ~3-4s.

Canary 0.6.0 version + Jest 27
image

Canary 0.6.0 version + yarn resolutions + Jest 27
image


Overall it's looking great! Thank you so much @andykenward <3

@IanVS
Copy link
Member

IanVS commented Jul 26, 2022

I updated to the canary without changing jest, and it worked fine. But when I updated jest, I now get:

● Validation Error:

  Preset jest-playwright-preset not found.

  Configuration Documentation:
  https://jestjs.io/docs/configuration
▶ npm ls jest
nebula-web@0.0.0 /Users/ianvs/code/defined/webclient
├─┬ @storybook/test-runner@0.6.0--canary.154.5e2f62a.0
│ ├─┬ jest-playwright-preset@2.0.0
│ │ └── jest@28.1.3 deduped
│ ├─┬ jest-watch-typeahead@2.0.0
│ │ └── jest@28.1.3 deduped
│ └── jest@28.1.3 deduped
└── jest@28.1.3

@andykenward
Copy link
Contributor Author

node: v16.16.0
npm: 8.11.0

I upgraded to this canary build and jest in my project. Did an npm i. But the older version of jest-playwright-preset 1.7.2 is still in my package-lock.json file. So I would need to regenerate my package-lock.json or install jest-playwright-preset latest to my project.

npm ls jest
CleanShot 2022-07-26 at 16 23 38@2x

npm ls jest-playwright-preset
CleanShot 2022-07-26 at 16 48 46@2x

@IanVS try doing npm ls jest-playwright-preset

@IanVS
Copy link
Member

IanVS commented Jul 26, 2022

@andykenward for some reason I can't determine, npm is putting jest-playwright-preset inside of node_modules/@storybook/test-runner/node_modules/jest-playwright-preset. When that happens, jest, which is running from the root node_modules, can't find it. If I install jest-playwright-preset directly, it works. Trying to figure out why npm is not hoisting it...

@andykenward
Copy link
Contributor Author

@andykenward for some reason I can't determine, npm is putting jest-playwright-preset inside of node_modules/@storybook/test-runner/node_modules/jest-playwright-preset. When that happens, jest, which is running from the root node_modules, can't find it. If I install jest-playwright-preset directly, it works. Trying to figure out why npm is not hoisting it...

@IanVS I've set in my .npmrc file legacy-peer-deps=true. Due to how storybook and it's add-ons resolve certain npm packages, see storybookjs/storybook#18298 . It's been an issue for me on multiple projects when adding storybook to them.

@IanVS
Copy link
Member

IanVS commented Jul 26, 2022

But jest-playwright-preset is an internal dependency of the test-runner. How would the peer dep resolution impact where npm installs it? Normally it should hoist all packages to the top level unless there are conflicting versions in different packages.

@IanVS
Copy link
Member

IanVS commented Jul 26, 2022

I just noticed that the jest-playwright-preset moved into the subdirectory when I ran npm i --save-dev jest@28 jest-mock@28. Prior to that (and after updating to the canary), the preset was the older version and was in the root directory. After updating jest, it got shoved into the subdirectory. 😕 Regenerating my lockfile also works. I guess my package-lock just must have been in some weird state or something. Hopefully not a common situation.

@IanVS
Copy link
Member

IanVS commented Jul 28, 2022

I'd say don't hold this up for my weird issue. There's at least a workaround for it.

@yannbf
Copy link
Member

yannbf commented Jul 29, 2022

Hey peeps! Sorry I'll get to this as soon as I can. The week has been pretty tough so I didn't have time. Thank you so much for your patience!!

yen-tt added a commit to yext/search-ui-react that referenced this pull request Aug 4, 2022
Use storybook test-runner and `@storybook/addon-coverage` to get visual coverage report. The setup requires spinning up storybook before generating the coverage numbers. Now, we have three test scripts:
- test => combined test coverage
- test:unit => jest test coverage only
- test:visual => story book coverage only (note: this only collect coverage on src/components)

Updated coverage github action to also create/update comment which display code coverage percentage for unit, visual, and combined tests

the test-runner only works with jest v27 right now. As nidhi noted in her [pr](#274), storybook will be adding support for v28 shortly (storybookjs/test-runner#154).

[playwright](https://www.npmjs.com/package/playwright) library is needed in the github action for test-storybook.
<img width="600" alt="Screen Shot 2022-08-03 at 4 51 03 PM" src="https://user-images.githubusercontent.com/36055303/182709211-0189bad6-e978-4f82-9ee4-ba62be350283.png">

NOTE: The report generated from `nyc` merge command may not be super accurate after some comparison between to the unit vs visual vs merge report (ex: [github issue](istanbuljs/nyc#1302)).

J=SLAP-2269 & SLAP-2270
TEST=manual&auto

- See that running `npm run test:visual` generates lcov report in coverage/visual folder. And the report is printed out in terminal.
- See that a comment is made to the PR about the three coverage percentages.
- See that coverall percentage increase without changes to tests (increased due to combined test coverage)
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM! 💯 Perhaps we can figure out what's going wrong with Ian's setup if more users encounter this

@IanVS
Copy link
Member

IanVS commented Aug 4, 2022

I agree. And the solution / workaround is simple if they do. They just need to install jest-playwright-preset along with jest.

@yannbf yannbf merged commit 561bc7a into storybookjs:next Aug 5, 2022
nmanu1 added a commit to yext/search-ui-react that referenced this pull request Aug 5, 2022
Add a GH workflow to check for WCAG violations. If the check fails, the PR will be blocked from merging into main. There are some WCAG violations that we ignore, namely color contrast failures on stories where components are in a "loading" state. Using config options, that rule is disabled for those particular stories. To get the automated WCAG checks, Storybook has to be running while the `npm run wcag` command is run.

Note, Jest had to be downgraded to v27 to work with `@storybook/test-runner`. But, it looks like they will be adding support for v28 shortly (storybookjs/test-runner#154), so we should be able to upgrade again soon. In the meantime, we haven't been using any features from v28, so the downgrade didn't affect any tests.

Also, the Snyk checks are failing, but the new license and one of new vulnerabilities are both approved by legal in Juliann's [PR](#264 (comment)). The other new vulnerability is also introduced only through Storybook, so we wouldn't be susceptible to it.

J=SLAP-2289
TEST=auto

See that the check runs as expected on this PR and passes.
@andykenward andykenward deleted the feat/jest-28 branch August 9, 2022 08:51
@github-actions
Copy link

🚀 PR was released in v0.6.0 🚀

yen-tt added a commit to yext/search-ui-react that referenced this pull request Sep 28, 2022
Use storybook test-runner and `@storybook/addon-coverage` to get visual coverage report. The setup requires spinning up storybook before generating the coverage numbers. Now, we have three test scripts:
- test => combined test coverage
- test:unit => jest test coverage only
- test:visual => story book coverage only (note: this only collect coverage on src/components)

Updated coverage github action to also create/update comment which display code coverage percentage for unit, visual, and combined tests

the test-runner only works with jest v27 right now. As nidhi noted in her [pr](#274), storybook will be adding support for v28 shortly (storybookjs/test-runner#154).

[playwright](https://www.npmjs.com/package/playwright) library is needed in the github action for test-storybook.
<img width="600" alt="Screen Shot 2022-08-03 at 4 51 03 PM" src="https://user-images.githubusercontent.com/36055303/182709211-0189bad6-e978-4f82-9ee4-ba62be350283.png">

NOTE: The report generated from `nyc` merge command may not be super accurate after some comparison between to the unit vs visual vs merge report (ex: [github issue](istanbuljs/nyc#1302)).

J=SLAP-2269 & SLAP-2270
TEST=manual&auto

- See that running `npm run test:visual` generates lcov report in coverage/visual folder. And the report is printed out in terminal.
- See that a comment is made to the PR about the three coverage percentages.
- See that coverall percentage increase without changes to tests (increased due to combined test coverage)
yen-tt pushed a commit to yext/search-ui-react that referenced this pull request Sep 28, 2022
Add a GH workflow to check for WCAG violations. If the check fails, the PR will be blocked from merging into main. There are some WCAG violations that we ignore, namely color contrast failures on stories where components are in a "loading" state. Using config options, that rule is disabled for those particular stories. To get the automated WCAG checks, Storybook has to be running while the `npm run wcag` command is run.

Note, Jest had to be downgraded to v27 to work with `@storybook/test-runner`. But, it looks like they will be adding support for v28 shortly (storybookjs/test-runner#154), so we should be able to upgrade again soon. In the meantime, we haven't been using any features from v28, so the downgrade didn't affect any tests.

Also, the Snyk checks are failing, but the new license and one of new vulnerabilities are both approved by legal in Juliann's [PR](#264 (comment)). The other new vulnerability is also introduced only through Storybook, so we wouldn't be susceptible to it.

J=SLAP-2289
TEST=auto

See that the check runs as expected on this PR and passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility with Jest 28: Got error running globalSetup
4 participants