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:storybook command 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/storybook directory
    • A broken story is simply one that results in a sb-show-errordisplay class on the body element 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-puppeteer package as dev-dependency along with puppeteer as per the documentation. https://storybook.js.org/addons/@storybook/addon-storyshots-puppeteer
  • Create .storybook/Stories.test.js which contains the code sample in the “Specifying the storybook URL” section of the documentation, using the local static build of storybook found in /dist via file://.
  • The setupTimeout and testTimeout values are 15s by default and this can be reduced to 5s as per the documentation.
  • Update the parameters object in .storybook/preview.js to include the asynchronous puppeteerTest function (refer to documentation), where the function checks whether the body of the rendered story contains the sb-show-errordisplay class name. For e.g:
    expect( await page.$eval( 'body', ( el ) => el.classList.contains( 'sb-show-errordisplay' ) ) ).toBe( false );
    
  • Update package.json to include a new command, test:storybook which runs:
    npm run test:js -- .storybook/Stories.test.js
    
  • Using .github/workflows/storybook.yml,
    • Add a new job test-storybook which uses the code from the existing build-storybook job with the following changes:
      • Instead of running npm run build:storybook, run the npm run test:storybook command.
      • Remove Upload artifacts section.

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)

Most upvoted comments

@danielgent it fails to download the artifact because the job needs to depend on the build job for that to be available. The job.needs is 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.