Add WCAG PR check #274

nmanu1 · 2022-08-02T20:15:46Z

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. 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.

coveralls · 2022-08-02T20:18:19Z

Coverage remained the same at 85.126% when pulling 8e581aa on dev/wcag-pr-check into f3c4ad0 on main.

…y notices check

This reverts commit 9882621.

oshi97 · 2022-08-03T15:11:21Z

.github/workflows/wcag_test.yml

+    secrets:
+      NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
+    with:
+      build_script: 'npm run storybook & sleep 10 && curl http://localhost:6006 -I'


I'm sure you looked into this but can we avoid the sleep 10? Maybe some option in start-storybook?

Yeah, I tried finding a way to not use sleep and checked again in the start-storybook options. The closest option to what we want is --smoke-test, but that stops serving the site once it successfully starts. We need the site to be running when the test-storybook command is run. The only other way I saw to do this as part of a GH workflow was either to host the site somewhere public, or involved setting up service containers. Service containers seemed like they might be overkill, but I can look into it more

is something like this possible? I don't have any experience trying stuff like this https://stackoverflow.com/questions/21475639/wait-until-service-starts-in-bash-script

sure! I updated it to be a while loop instead so it keeps checking every second, instead of having an arbitrary sleep time of 10 seconds

nice! this is very cool

This reverts commit 301c281.

.storybook/test-runner.cjs

.github/workflows/wcag_test.yml

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)

.storybook/test-runner.ts

github-actions · 2022-08-04T18:44:22Z

Current unit coverage is 88.60759493670886%
Current visual coverage is 77.55331088664423%
Current combined coverage is 88.92405063291139%

.storybook/test-runner.ts

This reverts commit 174bcba.

.storybook/wcagConfig.ts

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)

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.

Add WCAG PR check

37874b6

github-actions bot and others added 15 commits August 2, 2022 20:18

Automated update to THIRD-PARTY-NOTICES from github action's 3rd part…

8780aec

…y notices check

Try sleeping

a1c7241

Sleep longer

16c8437

Fix wrong port and update package versions

9882621

Revert "Fix wrong port and update package versions"

4dc074b

This reverts commit 9882621.

Fix port and update some imports

931c341

Remove build

e983c1e

Remove extra steps

8dfe7c7

Bump some package versions

6012358

Update more packages

9fcac7f

Bump a11y

cd16587

Bump actions

fe13233

Bump links to 6.5.0

b7d9c6e

Upgrade percy

ea9fa1c

Bump links to 6.5.9

bfaae61

nmanu1 marked this pull request as ready for review August 3, 2022 00:19

nmanu1 requested a review from a team as a code owner August 3, 2022 00:19

oshi97 reviewed Aug 3, 2022

View reviewed changes

nmanu1 added 2 commits August 3, 2022 11:17

Try smoke-test

301c281

Revert "Try smoke-test"

1123603

This reverts commit 301c281.

alextaing reviewed Aug 3, 2022

View reviewed changes

.storybook/test-runner.cjs Outdated Show resolved Hide resolved

nmanu1 added 6 commits August 3, 2022 15:19

Try bash script

1c0172f

Try wrapping script

6402c37

Try one long line

a7c0dbc

Remove |

8bc9017

Semicolons

4e77d97

Shorten

6110549

nmanu1 requested a review from oshi97 August 3, 2022 20:01

oshi97 reviewed Aug 4, 2022

View reviewed changes

.github/workflows/wcag_test.yml Outdated Show resolved Hide resolved

nmanu1 added 2 commits August 4, 2022 13:22

Format and make quiet

740ae60

Export default

cc9c956

nmanu1 requested a review from oshi97 August 4, 2022 18:05

yen-tt reviewed Aug 4, 2022

View reviewed changes

.github/workflows/wcag_test.yml Outdated Show resolved Hide resolved

Merge branch 'main' into dev/wcag-pr-check

e23d88c

yen-tt reviewed Aug 4, 2022

View reviewed changes

.storybook/test-runner.ts Outdated Show resolved Hide resolved

nmanu1 added 5 commits August 4, 2022 15:06

Reuse script

5043813

Fix paths

ee5e7cf

Try npm run command

7bd00a5

oops

212892a

Try some TS

174bcba

oshi97 reviewed Aug 4, 2022

View reviewed changes

.storybook/test-runner.ts Outdated Show resolved Hide resolved

nmanu1 added 4 commits August 4, 2022 16:49

Revert "Try some TS"

074458d

This reverts commit 174bcba.

Fix spacing

2fadcdd

Add asterisk

14ca6f8

Abstract out wcag config

31f31fa

nmanu1 requested review from yen-tt and oshi97 and removed request for oshi97 August 4, 2022 21:11

Update package-lock

8e581aa

oshi97 reviewed Aug 4, 2022

View reviewed changes

.storybook/wcagConfig.ts Show resolved Hide resolved

nmanu1 requested a review from oshi97 August 5, 2022 13:46

oshi97 approved these changes Aug 5, 2022

View reviewed changes

nmanu1 merged commit d734620 into main Aug 5, 2022

nmanu1 deleted the dev/wcag-pr-check branch August 5, 2022 14:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add WCAG PR check #274

Add WCAG PR check #274

nmanu1 commented Aug 2, 2022 •

edited

coveralls commented Aug 2, 2022 •

edited

oshi97 Aug 3, 2022

nmanu1 Aug 3, 2022

oshi97 Aug 3, 2022

nmanu1 Aug 3, 2022

oshi97 Aug 4, 2022 •

edited

github-actions bot commented Aug 4, 2022

Add WCAG PR check #274

Add WCAG PR check #274

Conversation

nmanu1 commented Aug 2, 2022 • edited

coveralls commented Aug 2, 2022 • edited

oshi97 Aug 3, 2022

Choose a reason for hiding this comment

nmanu1 Aug 3, 2022

Choose a reason for hiding this comment

oshi97 Aug 3, 2022

Choose a reason for hiding this comment

nmanu1 Aug 3, 2022

Choose a reason for hiding this comment

oshi97 Aug 4, 2022 • edited

Choose a reason for hiding this comment

github-actions bot commented Aug 4, 2022

nmanu1 commented Aug 2, 2022 •

edited

coveralls commented Aug 2, 2022 •

edited

oshi97 Aug 4, 2022 •

edited