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] Importing stories from separate packages in a monorepo no longer works with v0.14 - testMatch returns 0 matches #381

Closed
aislinn-hayes-wd opened this issue Nov 9, 2023 · 8 comments
Assignees
Labels
bug Something isn't working sev:S2

Comments

@aislinn-hayes-wd
Copy link

Describe the bug
#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 #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
Copy link

ahayes91 commented Nov 9, 2023

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

@yannbf
Copy link
Member

yannbf commented Nov 10, 2023

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 yannbf added the bug Something isn't working label Nov 10, 2023
@yannbf yannbf moved this to Empathy Backlog in Core Team Projects Nov 13, 2023
@vanessayuenn vanessayuenn moved this from Empathy Backlog to Ready for work in Core Team Projects Nov 13, 2023
@yannbf
Copy link
Member

yannbf commented Nov 13, 2023

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:

  • both files are .stories.*
  • both files are .story.*
  • one file is .story.* and the other .stories.*

and every scenario worked correctly.

@ahayes91
Copy link

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
Copy link
Member

yannbf commented Nov 13, 2023

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

@yannbf
Copy link
Member

yannbf commented Nov 13, 2023

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

@ahayes91
Copy link

ahayes91 commented Nov 13, 2023

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
Copy link
Member

yannbf commented Nov 13, 2023

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!

@yannbf yannbf closed this as completed Nov 13, 2023
@github-project-automation github-project-automation bot moved this from Ready for work to Done in Core Team Projects Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sev:S2
Projects
Archived in project
Development

No branches or pull requests

4 participants