-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Test: Refactor to see stories as tests #32328
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33 files reviewed, 4 comments
| title: ComponentTitle; | ||
| tags?: Tag[]; | ||
| importPath: Path; | ||
| parentId: StoryId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Adding parentId to BaseIndexEntry affects all index entries (stories and docs), but only tests would logically need a parent. Consider adding this only to TestIndexInput or making it optional.
code/core/src/csf/csf-factories.ts
Outdated
| test: testFunction, | ||
| }; | ||
| const play = | ||
| mountDestructured(this.play) || mountDestructured(testFn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Variable testFn is referenced but may be undefined. Should check mountDestructured(testFunction) instead.
| mountDestructured(this.play) || mountDestructured(testFn) | |
| mountDestructured(this.play) || mountDestructured(testFunction) |
| const testId = toTestId(storyMeta.id, testName); | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore We provide the __id parameter because we don't want normalizeStory to calculate the id | ||
| storyTest.input.parameters.__id = testId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Variable storyTest is not defined. Should be story.input.parameters.__id = testId;
| storyTest.input.parameters.__id = testId; | |
| story.input.parameters.__id = testId; |
| getStoryChildren(story).forEach((story) => { | ||
| const testName = story.input.name!; | ||
| const testId = toTestId(storyMeta.id, testName); | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore We provide the __id parameter because we don't want normalizeStory to calculate the id | ||
| storyTest.input.parameters.__id = testId; | ||
|
|
||
| csfFile.stories[testId] = normalizeStory(testName, story.input as any, meta); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Variable shadowing: the inner story parameter shadows the outer story variable from line 62. Consider renaming the inner variable to testStory or childStory
|
View your CI Pipeline Execution ↗ for commit a444184
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 80 KB | 80 KB | 0 B |
| Dependency size | 31.40 MB | 31.43 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 41.77 MB | 41.75 MB | 🎉 -19 KB 🎉 |
| Dependency size | 16.45 MB | 16.59 MB | 🚨 +132 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 199 | 199 | 0 |
| Self size | 191 KB | 191 KB | 0 B |
| Dependency size | 29.73 MB | 29.78 MB | 🚨 +45 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 203 | 0 |
| Self size | 28 KB | 28 KB | 0 B |
| Dependency size | 28.13 MB | 28.17 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 538 | 538 | 0 |
| Self size | 903 KB | 903 KB | 0 B |
| Dependency size | 59.17 MB | 59.21 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 286 | 286 | 0 |
| Self size | 25 KB | 25 KB | 0 B |
| Dependency size | 43.72 MB | 43.75 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 210 | 210 | 0 |
| Self size | 17 KB | 17 KB | 0 B |
| Dependency size | 32.67 MB | 32.70 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 17 | 17 | 0 |
| Self size | 53 KB | 53 KB | 0 B |
| Dependency size | 26.67 MB | 26.66 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 18 | 18 | 0 |
| Self size | 51 KB | 51 KB | 0 B |
| Dependency size | 26.72 MB | 26.71 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 105 | 105 | 0 |
| Self size | 34 KB | 34 KB | 0 B |
| Dependency size | 43.54 MB | 43.53 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
sb
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 51 | 51 | 0 |
| Self size | 1 KB | 1 KB | 0 B |
| Dependency size | 58.22 MB | 58.33 MB | 🚨 +113 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 219 | 219 | 0 |
| Self size | 585 KB | 585 KB | 0 B |
| Dependency size | 102.74 MB | 102.86 MB | 🚨 +116 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 188 | 0 |
| Self size | 31 KB | 31 KB | 0 B |
| Dependency size | 86.81 MB | 86.93 MB | 🚨 +120 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-create-react-app
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 68 | 68 | 0 |
| Self size | 18 KB | 18 KB | 0 B |
| Dependency size | 5.94 MB | 5.97 MB | 🚨 +37 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 177 | 177 | 0 |
| Self size | 24 KB | 24 KB | 0 B |
| Dependency size | 30.56 MB | 30.59 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
Closes #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 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
This PR introduces a comprehensive new testing paradigm for Storybook by enabling stories to have attached test functions through a new
.test()method API. The changes implement what's called "CSF4" (Component Story Format 4) syntax, allowing developers to write tests directly on story objects likeMyStory.test('test name', async (context) => { ... }).The implementation spans multiple layers of the Storybook architecture:
Core Infrastructure: New
TestFunctiontype definitions are added alongside existingPlayFunctiontypes, with support for story contexts and async execution. The CSF factories are extended with atest()method that creates child stories with special 'test-fn' tags, inheriting parent story configuration while allowing overrides.Parsing and Indexing: The CSF parser now recognizes
.test()method calls on story exports and tracks them in a new_storyTestsdata structure. The story indexer generates unique test IDs using atoTestId()function that combines parent story IDs with test names (e.g., "story-id:test-name").Vitest Integration: The Vitest transformer is refactored to generate nested test structures, creating
describeblocks for stories with tests and individualtest()calls for each test function. The vitest plugin supports running tests at different granularities - individual test functions or all tests for a story.UI Support: The Storybook sidebar gains visual support for test nodes with a distinctive green beaker icon and special styling. Test stories are tagged to exclude them from auto-documentation while maintaining full functionality.
Preview and Rendering: The preview system is updated to handle both play functions and test functions in theme selection logic, ensuring proper rendering in visual testing environments like Chromatic.
The architecture maintains backward compatibility with existing CSF1-3 formats while introducing this new testing capability. Test functions inherit story context (canvas, userEvent, args) and can override story configuration for specific test scenarios.
Confidence score: 2/5
processCSFFile.tsthat will cause runtime failures, plus extensive use of temporary "hack" code and TODO comments indicating incomplete implementationcode/core/src/preview-api/modules/store/csf/processCSFFile.tswhich has a critical bug on line 76, and console.log debugging statements inStoryStore.tsthat should be removed before production