storybookjs / test-runner

πŸš• Turn stories into executable tests
https://storybook.js.org/docs/writing-tests/interaction-testing
MIT License
231 stars 72 forks source link

[bug] Importing stories from separate packages in a monorepo no longer works with v0.14 - testMatch returns 0 matches #381

Closed aislinn-hayes-wd closed 11 months ago

aislinn-hayes-wd commented 11 months ago

Describe the bug https://github.com/storybookjs/test-runner/pull/339 added capability to import stories in a monorepo with a regex, but this is broken for me now in v0.14.

I worked upwards from v0.13 and identified that the breakage occurs from 0.14.0-next.3 - so it must have been introduced with the code in https://github.com/storybookjs/test-runner/pull/376.

To Reproduce I can work on a reproduction repo for this but it might take a bit 😬 If there's a CodeSandbox template available to give me a head start that would be great πŸ™‡

I've redacted some info from the output below, you can see we're using Nx to run the task from within a storybook package. This is working fine to display the stories on Storybook and the interaction addon works fine in the browser too.

aislinn.hayes@XXXXXXXXXXXX my-repo % yarn nx run storybook:test-storybook
yarn run v1.22.19
$ /Users/aislinn.hayes/code/my-repo/node_modules/.bin/nx run storybook:test-storybook

> nx run storybook:test-storybook

$ test-storybook
No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/aislinn.hayes/code/my-repo
  5 files checked.
  testMatch: /Users/aislinn.hayes/code/my-repo/packages/**/*.stories.@(js|jsx|ts|tsx|mdx) - 0 matches
  testPathIgnorePatterns: /node_modules/ - 5 matches
  testRegex:  - 0 matches
Pattern:  - 0 matches
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

 β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”β€”

 >  NX   Ran target test-storybook for project storybook (3s)

    βœ–    1/1 failed
    βœ”    0/1 succeeded [0 read from cache]

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Expected behavior Test runner should have found a few tests!

Screenshots N/A

System

Storybook Environment Info:

  System:
    OS: macOS 13.6
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 16.18.1 - ~/.nvm/versions/node/v16.18.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.18.1/bin/yarn <----- active
    npm: 8.19.2 - ~/.nvm/versions/node/v16.18.1/bin/npm
    pnpm: 8.6.7 - ~/.nvm/versions/node/v16.18.1/bin/pnpm
  Browsers:
    Chrome: 119.0.6045.105
    Safari: 17.0
  npmPackages:
    @storybook/addon-a11y: 7.5.3 => 7.5.3 
    @storybook/addon-essentials: 7.5.3 => 7.5.3 
    @storybook/addon-interactions: 7.5.3 => 7.5.3 
    @storybook/jest: 0.2.3 => 0.2.3 
    @storybook/react: 7.5.3 => 7.5.3 
    @storybook/react-vite: 7.5.3 => 7.5.3 
    @storybook/storybook-deployer: 2.8.16 => 2.8.16 
    @storybook/test-runner: 0.14.0-next.3 => 0.14.0-next.3 
    @storybook/testing-library: 0.2.2 => 0.2.2 
    eslint-plugin-storybook: 0.6.13 => 0.6.13 
    msw-storybook-addon: 1.9.0 => 1.9.0 
    storybook: 7.5.3 => 7.5.3 

Additional context Versions below 0.14.0-next.3 work fine.

ahayes91 commented 11 months ago

Opened this under a test Github user I had, my bad! Just to let you folks know I'm not a bot πŸ˜‚

yannbf commented 11 months ago

Hey @ahayes91 and πŸ€–, thank you for reporting the issue!

The only actual change from that PR is this: https://github.com/storybookjs/test-runner/pull/376/files#diff-192b7f2e275030badbef39a08e82d3b8836a81600ea3fae116511507c676da77R80

could you please try to undo that change, in your local node_modules, and see if the issue is fixed?

yannbf commented 11 months ago

Hey there @ahayes91 I tried to make a reproduction but couldn't get it to fail. In fact, I noticed you're using NX so I set a workspace up here: https://github.com/yannbf/nx-test-runner-repro

yarn nx run org:storybook to run Storybook yarn nx run org:test-storybook to test it

And that works correctly:

image

I renamed the story files so that:

and every scenario worked correctly.

ahayes91 commented 11 months ago

Hey @ahayes91 and πŸ€–, thank you for reporting the issue!

The only actual change from that PR is this: https://github.com/storybookjs/test-runner/pull/376/files#diff-192b7f2e275030badbef39a08e82d3b8836a81600ea3fae116511507c676da77R80

could you please try to undo that change, in your local node_modules, and see if the issue is fixed?

No joy when I made the changes in node_modules, still failing:

Screenshot 2023-11-13 at 15 10 36 Screenshot 2023-11-13 at 15 10 54

Anything else you could think of that I can fiddle with to try and debug this a bit? (Maybe point me towards the file internally that finds the Storybook files?)

I really appreciate this definitely needs a proper reproduction, so I'll work on that now - we're using package-based Nx (a very annoying restriction on us right now due to other internal blockers) and have Storybook in its own package (so that I can put some task management around the storybook commands), so it wasn't generated in using @nx/storybook.

yannbf commented 11 months ago

@ahayes91 you can reach me out on discord at @yannbf, happy to assist!

yannbf commented 11 months ago

In the meantime, can you remove this line of code as well? It must be the culprit

ahayes91 commented 11 months ago

In the meantime, can you remove this line of code as well? It must be the culprit

Yes this fixes it! πŸ’ͺ

Edit: Sticking a debug of process.cwd() gives me: /Users/aislinn.hayes/code/my-repo/packages/storybook

This bug is very specific to folks who whack Storybook into its own package in a monorepo, rather than a .storybook folder at the root. Just to give a bit of background, the reason I did that was so that we could make nx run the build tasks for the other packages in the monorepo before running Storybook (https://nx.dev/core-features/run-tasks#defining-a-task-pipeline) - so I've got things like @my-repo/package-a as a dependency in the package.json of the packages/storybook package, meaning the build for @my-repo/package-a will always run before Storybook starts up.

Is that a valid use case? I'm undecided - I really don't like our package-based architecture, but we have unrelated constraints and it works for us for now πŸ˜‚

The workaround in this case would be to eject the test-runner-jest.config.js with yarn test-storybook --eject and override the roots attribute in my project through test-runner-jest.config.js, which works:

const {getJestConfig} = require('@storybook/test-runner');

// The default configuration comes from @storybook/test-runner
const testRunnerConfig = getJestConfig();

/**
 * @type {import('@jest/types').Config.InitialOptions}
 */
module.exports = {
  ...testRunnerConfig,
  /** Add your own overrides below, and make sure
   *  to merge testRunnerConfig properties with your own
   * @see https://jestjs.io/docs/configuration
   */
  // Required with bug https://github.com/storybookjs/test-runner/issues/381 in v0.14.0
  roots: undefined,
};

Wondering what you folks are thinking about this - is this setting of roots something we could / should revert in @storybook/test-runner, or is my use-case so niche that the workaround is a better option? Thank you!

yannbf commented 11 months ago

Hey @ahayes91!

Your use case is valid, and I've seen other people do it, mostly because they do not want dependencies from Storybook to somehow get in the way from their application's dependencies. We do not recommend this as de-facto Storybook configuration because we don't use it ourselves, we don't support it as greatly (as you can see) as with basic configurations, and because there could be hoisting and other issues caused by tools like Yarn PnP and pnpm PnP. You are free to use it as you see fit, of course.

Regardless, you have detected a bug from a change I did in a previous PR, so I reverted it on #387 and it's now released as a patch in 0.14.1. Please upgrade and it should all be good now, with no need to eject the test-runner config.

Also, I greatly appreciate the effort you put into getting this fixed! Thank you so much and have a great week!