Add CI step for storybook test runner #935
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // COMMENTED OUT for now until we want to actually turn on accessibility tests on CI | ||
|
There was a problem hiding this comment. Do the tests pass for you locally? test-storybook passed for me (which I was surprised by, if it included accessibility tests?) but I'm not 100% confident that I resolved the merge conflicts correctly There was a problem hiding this comment. We can see the accessibility failures both locally and on CI by uncommenting the contents of this file. See this build, for example. Do you want to actually turn this on now and make all our CI tests fail until we fix the a11y issues? I had assumed not yet... But maybe it would be a good idea to turn it on and factor it out into a separate build step, that way when it fails we can immediately recognize the failure is expected, instead of clicking into the gh action log to make sure it's failing on the expected step? There was a problem hiding this comment. Huh okay I wasn't seeing them locally, but I'll try again. No I agree, lets not intentionally have CI fail. |
||
| // See docs for more details including story- and component-level configuration: | ||
| // https://storybook.js.org/docs/writing-tests/accessibility-testing#automate-accessibility-tests-with-test-runner | ||
|
|
||
| // import type { TestRunnerConfig } from '@storybook/test-runner'; | ||
| // import { injectAxe, checkA11y } from 'axe-playwright'; | ||
| // | ||
| // /* | ||
| // * See https://storybook.js.org/docs/writing-tests/test-runner#test-hook-api | ||
| // * to learn more about the test-runner hooks API. | ||
| // */ | ||
| // const config: TestRunnerConfig = { | ||
| // async preVisit(page) { | ||
| // await injectAxe(page); | ||
| // }, | ||
| // async postVisit(page) { | ||
| // await checkA11y(page, '#storybook-root', { | ||
| // detailedReport: true, | ||
| // detailedReportOptions: { | ||
| // html: true, | ||
| // }, | ||
| // }); | ||
| // }, | ||
| // }; | ||
| // | ||
| // export default config; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@martha I am wondering about the choice to user storybook test runner on CI as opposed to running tests via Chromatic on CI. The storybook gives the example that you might want to "Use [test runner] locally and Chromatic on your CI". I would guess that a benefit of Chromatic would be if the tests fail, maybe you can see some more detailed output on chromatic as opposed to just test logs. (Judging from chromatic docs about PR workflow it seems pretty advanced).
Linking https://github.com/open-path/Green-River/issues/6687 for reference too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't read all the way through your description 🙈
That was set up mostly for the purpose of publishing the "release version" of the storybook. I don't have anything against using chromatic to run testing on PRs. (And I guess publishing, maybe that's the norm for chromatic usage).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I updated the PR to run the tests against the Chromatic instance of Storybook.
We do still need the test runner to run the accessibility tests. Chromatic supports running interaction tests, but doesn't support a11y tests as far as I can tell. You can see this in the example build here. The test runner step shows a11y failures, but the Chromatic step succeeds. In the corresponding Chromatic build for that action, there are 51 "unreviewed changes" (bc I had temporarily turned on the "visual tests" setting) but there are no a11y-related failures or warnings.