site-kit-wp: Add tests for broken stories
Feature Description
We use Storybook as the foundation of our visual regression tests but we also have many more stories than we have VRT scenarios. Currently VRT is the only test we have in place to prevent Storybook from being broken, but it only catches stories that have corresponding VRT scenarios which are almost entirely all legacy stories. It doesn’t make sense to enable VRT for every story, but we should at least still be able to require that no stories are broken which is a problem that keeps coming up.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- A new
test:storybookcommand should be created, which when run will check every story to see if it’s broken or not- The command should list all broken stories by default although an option to fail on the first one found would be nice
- If a broken story is found, the command should return with a non-zero exit code
- Any new test scripts/config/files should go in a new
tests/storybookdirectory - A broken story is simply one that results in a
sb-show-errordisplayclass on thebodyelement in the preview iframe – this frame should always be loaded directly (like it is in VRT) rather than in the larger view we normally use
- The new command should be integrated into our current GitHub actions so that broken stories cause the Storybook workflow to fail
Implementation Brief
- Install the
@storybook/addon-storyshots-puppeteerpackage as dev-dependency along withpuppeteeras per the documentation. https://storybook.js.org/addons/@storybook/addon-storyshots-puppeteer - Create
.storybook/Stories.test.jswhich contains the code sample in the “Specifying the storybook URL” section of the documentation, using the local static build of storybook found in/distviafile://. - The
setupTimeoutandtestTimeoutvalues are15sby default and this can be reduced to5sas per the documentation. - Update the
parametersobject in.storybook/preview.jsto include the asynchronouspuppeteerTestfunction (refer to documentation), where the function checks whether thebodyof the rendered story contains thesb-show-errordisplayclass name. For e.g:expect( await page.$eval( 'body', ( el ) => el.classList.contains( 'sb-show-errordisplay' ) ) ).toBe( false ); - Update
package.jsonto include a new command,test:storybookwhich runs:npm run test:js -- .storybook/Stories.test.js - Using
.github/workflows/storybook.yml,- Add a new job
test-storybookwhich uses the code from the existingbuild-storybookjob with the following changes:- Instead of running
npm run build:storybook, run thenpm run test:storybookcommand. - Remove
Upload artifactssection.
- Instead of running
- Add a new job
Test Coverage
- No further tests to be added.
Visual Regression Changes
- N/A
QA Brief
This should maybe be a QA:Eng ticket as there are no user-facing changes
- Check that all checks are passing on GitHub
Changelog entry
- N/A
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 2
- Comments: 22 (12 by maintainers)
@danielgent it fails to download the artifact because the job needs to depend on the
buildjob for that to be available. Thejob.needsis missing.Regarding the broken stories, I mentioned on the others that it didn’t make sense to fix them again and again until this issue was done so, it’s somewhat out of scope but unless they’re substantially too much effort let’s just fix them as part of this PR.