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

[Bug]: default --test build configuration omits addon-coverage, which breaks test-runner runs #25205

Open
vicrep opened this issue Dec 12, 2023 · 2 comments

Comments

@vicrep
Copy link

vicrep commented Dec 12, 2023

Describe the bug

Hi, posting this as a bug but it very might well be intended behaviour, so please let me know if that is the case. If not I'd be happy to contribute a PR to possibly fix this.

There was the recent new addition of the --test build mode for storybook, which IIUC is intended to be used for speedier builds to use with Chromatic and @storybook/test-runner. However, while trying it out, I noticed that if you use the @storybook/addon-coverage add-on and try to build using the --test flag with no custom configuration, that the builder seems to remove coverage instrumentation, which in turn causes the test-runner to fail when run with the --coverage flag.

The reason this happens is that this is how the --test build mode is configured by default:

? ['@storybook/addon-docs', '@storybook/addon-essentials/docs', '@storybook/addon-coverage']

To fix this in an existing project, you just need to undo that default as such in your config file:

// main.ts|js
export default {
  // ...
  build: {
    test: {
      disabledAddons: [
        '@storybook/addon-docs',
        '@storybook/addon-essentials/docs',
      ],
    },
  },
}

I'm more wondering if that's intentional as it feels a little odd to me to have coverage be disabled from a "test" build of your storybook, given that this is one of the primary use-cases where you would indeed collect coverage.

Related issue: storybookjs/addon-coverage#31

To Reproduce

  • configure your storybook to use @addon-coverage and to run using @storybook/test-runner
  • run npx storybook build --test with no additional test config in your storybook/main.js|ts
  • run npm run test-storybook -- --coverage
  • run fails due to missing instrumentation

System

No response

Additional context

No response

@shilman
Copy link
Member

shilman commented Dec 18, 2023

This is the intended behavior but I can see how it would feel like a bug. What do you think about the following explanation for the docs.

The --test flag is designed to be as fast as possible, so it removes addons that are known to slow down the build and are not needed for functional testing. One of these addons is @storybook/addon-coverage, which is used in conjunction with the Storybook Test runner to collect coverage information for your stories.

If you are using addon-coverage AND running the test runner against your built Storybook, the --test flag will strip out the coverage information. To configure the --test build to keep coverage information (at the expense of a slightly slower build), update .storbyook/main.js:

// main.ts|js
export default {
  // ...
  build: {
    test: {
      disabledAddons: [
        '@storybook/addon-docs',
        '@storybook/addon-essentials/docs',
      ],
    },
  },
}

@vicrep
Copy link
Author

vicrep commented Dec 18, 2023

Thanks for the answer @shilman , very much appreciated.

I think adding a little note to the docs to make that extra-clear for people who use coverage + test-runner would be a great start!

Thinking about this a little, my only caveat with this being intended and "disabled" this way is that by forcing consumers to overwrite the disabled list by effectively hardcoding all the SB provided defaults minus the coverage addons, is that consumers will then not benefit from any additional add-ons that SB might decide isn't needed by the --test builds in the future.

I know it doesn't really fit the generic pattern you have in place for test build configuration, but I was wondering if it could be worth it to consider adding a specific flag to disableCoverage, where the behaviour of this flag would be to strip out addon-coverage from the list of disabled add-ons. That way interested consumers can ensure their coverage remains but still benefit form any additional add-ons being added in future releases to the default ignore-list.

Alternatively or in addition, you could leverage a COVERAGE=true / STORYBOOK_COVERAGE=true env var to do this determination automatically, which would fit alongside how some tools like vite-istanbul can be instructed to instrument code.

Just food for thought, would be curious what you think :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: No status
Development

No branches or pull requests

2 participants