Addon Test: Refactor test addon to include stories automatically #29367
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 5c9c675. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 61 | 61 | 0 |
| Self size | 695 KB | 916 KB | 🚨 +221 KB 🚨 |
| Dependency size | 13.86 MB | 13.86 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -6,3 +6,4 @@ export * from './build-static'; | |||
| export * from './build-dev'; | |||
| export * from './withTelemetry'; | |||
| export { default as build } from './standalone'; | |||
| export { StoryIndexGenerator } from './utils/StoryIndexGenerator'; | |||
There was a problem hiding this comment.
style: Exposing internal utilities like StoryIndexGenerator could make it harder to refactor internals in the future. Consider creating a dedicated public API for story file discovery instead.
| ${template.expected.renderer === '@storybook/svelte' ? '"**/*.stories.svelte",' : ''} | ||
| ], |
There was a problem hiding this comment.
logic: Svelte stories exclusion should be conditional on both renderer AND framework being Svelte to handle SvelteKit case
ndelangen
added
ci:daily
|
@ghengeveld @ndelangen I wonder if our new way of finding stories here, also fixes #29572. We should try it out in QA. |
It doesn't seem to fix it. Here's an example on windows with two directories: strangely enough if I use the newest alpha and I keep the include which was there originally (the default one with .stories.), then the tests fail because they end up finding the template stories in node_modules |
Closes #29771
What I did
This PR introduces a change in the Vitest plugin where it leverages the
storiesfield ofmain.tsto transform into an array of files forvitest, and adds that automatically asconfig.test.include.I tested this locally in the monorepo's Storybook and it worked without issues. Adding extra
includeorexcludepaths in thevitestconfig will work as expected.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This is tested by our sandboxes, as well as via our
storybook:vitestnpm script; Which is run in CI.You can experiment by adding a new story to the monorepo storybook, and running
storybook:vitestand check if the new story is tested.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
Based on the provided files and context, here's a concise summary of the changes:
Refactored the Storybook test addon to automatically sync story discovery between Storybook and Vitest testing.
storiesfield from main.jsStoryIndexGeneratorexport from core-server for consistent story file discoveryThe changes aim to resolve inconsistencies where stories might be visible in Storybook but not in Vitest tests or vice versa, while maintaining compatibility with different frameworks and build configurations.