Maintenance: Enhance ghost stories internal tests#34707
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughDev-server–spawned Vitest runs are marked with STORYBOOK_INTERNAL_TEST_RUN and the Vitest addon only installs AgentTelemetryReporter when an agent is active and that env var is absent. Tests and ghost-stories runner were updated accordingly. Separately, test-analysis now omits cumulative fields unless cumulativeResults are provided and the TestRunAnalysis cumulative fields are optional. ChangesInternal Test Run Telemetry Gating
Test Result Analysis & Types
Sequence Diagram(s)sequenceDiagram
autonumber
participant GhostRunner as GhostStories Runner
participant DevServer as Dev Server
participant Vitest as Vitest Process
participant VitestPlugin as Vitest Addon (AgentTelemetryReporter)
participant Telemetry as Telemetry System
GhostRunner->>DevServer: request ghost-story test run
DevServer->>Vitest: spawn "npx vitest run" with env (STORYBOOK_INTERNAL_TEST_RUN=1, maybe STORYBOOK_COMPONENT_PATHS)
Vitest->>VitestPlugin: vitest config hook executes (reads process.env)
alt STORYBOOK_INTERNAL_TEST_RUN set
VitestPlugin-->>Vitest: skip AgentTelemetryReporter installation
else not set and agent active
VitestPlugin->>Telemetry: install AgentTelemetryReporter (emit telemetry)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Sidnioulz
approved these changes
May 5, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/shared/utils/analyze-test-results.test.ts`:
- Around line 129-140: The test for analyzeTestResults that "omits all
cumulative fields when no cumulative results are provided" is missing assertions
for cumulativePassedButEmptyRender and cumulativeSuccessRateWithoutEmptyRender;
update the test that calls analyzeTestResults(results) to also assert that
analysis.cumulativePassedButEmptyRender and
analysis.cumulativeSuccessRateWithoutEmptyRender are undefined so all cumulative
properties returned by analyzeTestResults are covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dced9f58-445a-4d27-9778-52d31899b6c3
📒 Files selected for processing (5)
code/core/src/core-server/server-channel/ghost-stories-channel.test.tscode/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.tscode/core/src/shared/utils/analyze-test-results.test.tscode/core/src/shared/utils/analyze-test-results.tscode/core/src/shared/utils/test-result-types.ts
💤 Files with no reviewable changes (1)
- code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
Comment on lines
+129
to
+140
| it('omits all cumulative fields when no cumulative results are provided', () => { | ||
| const results: StoryTestResult[] = [ | ||
| { storyId: 's1', status: 'PASS' }, | ||
| { storyId: 's2', status: 'FAIL', error: 'oops' }, | ||
| ]; | ||
| const analysis = analyzeTestResults(results); | ||
| expect(analysis.cumulativeTotal).toBe(analysis.runTotal); | ||
| expect(analysis.cumulativePassed).toBe(analysis.runPassed); | ||
| expect(analysis.cumulativeSuccessRate).toBe(analysis.runSuccessRate); | ||
| expect(analysis.cumulativeUniqueErrorCount).toBe(analysis.runUniqueErrorCount); | ||
| expect(analysis.cumulativeTotal).toBeUndefined(); | ||
| expect(analysis.cumulativePassed).toBeUndefined(); | ||
| expect(analysis.cumulativeSuccessRate).toBeUndefined(); | ||
| expect(analysis.cumulativeUniqueErrorCount).toBeUndefined(); | ||
| expect(analysis.cumulativeCategorizedErrors).toBeUndefined(); | ||
| expect(analysis.cumulativeCssCheck).toBeUndefined(); |
Contributor
There was a problem hiding this comment.
“All cumulative fields” test misses two cumulative properties.
At Line 129 the test claims full coverage, but cumulativePassedButEmptyRender and cumulativeSuccessRateWithoutEmptyRender are not asserted.
Suggested patch
expect(analysis.cumulativeTotal).toBeUndefined();
expect(analysis.cumulativePassed).toBeUndefined();
+ expect(analysis.cumulativePassedButEmptyRender).toBeUndefined();
expect(analysis.cumulativeSuccessRate).toBeUndefined();
+ expect(analysis.cumulativeSuccessRateWithoutEmptyRender).toBeUndefined();
expect(analysis.cumulativeUniqueErrorCount).toBeUndefined();
expect(analysis.cumulativeCategorizedErrors).toBeUndefined();
expect(analysis.cumulativeCssCheck).toBeUndefined();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('omits all cumulative fields when no cumulative results are provided', () => { | |
| const results: StoryTestResult[] = [ | |
| { storyId: 's1', status: 'PASS' }, | |
| { storyId: 's2', status: 'FAIL', error: 'oops' }, | |
| ]; | |
| const analysis = analyzeTestResults(results); | |
| expect(analysis.cumulativeTotal).toBe(analysis.runTotal); | |
| expect(analysis.cumulativePassed).toBe(analysis.runPassed); | |
| expect(analysis.cumulativeSuccessRate).toBe(analysis.runSuccessRate); | |
| expect(analysis.cumulativeUniqueErrorCount).toBe(analysis.runUniqueErrorCount); | |
| expect(analysis.cumulativeTotal).toBeUndefined(); | |
| expect(analysis.cumulativePassed).toBeUndefined(); | |
| expect(analysis.cumulativeSuccessRate).toBeUndefined(); | |
| expect(analysis.cumulativeUniqueErrorCount).toBeUndefined(); | |
| expect(analysis.cumulativeCategorizedErrors).toBeUndefined(); | |
| expect(analysis.cumulativeCssCheck).toBeUndefined(); | |
| it('omits all cumulative fields when no cumulative results are provided', () => { | |
| const results: StoryTestResult[] = [ | |
| { storyId: 's1', status: 'PASS' }, | |
| { storyId: 's2', status: 'FAIL', error: 'oops' }, | |
| ]; | |
| const analysis = analyzeTestResults(results); | |
| expect(analysis.cumulativeTotal).toBeUndefined(); | |
| expect(analysis.cumulativePassed).toBeUndefined(); | |
| expect(analysis.cumulativePassedButEmptyRender).toBeUndefined(); | |
| expect(analysis.cumulativeSuccessRate).toBeUndefined(); | |
| expect(analysis.cumulativeSuccessRateWithoutEmptyRender).toBeUndefined(); | |
| expect(analysis.cumulativeUniqueErrorCount).toBeUndefined(); | |
| expect(analysis.cumulativeCategorizedErrors).toBeUndefined(); | |
| expect(analysis.cumulativeCssCheck).toBeUndefined(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@code/core/src/shared/utils/analyze-test-results.test.ts` around lines 129 -
140, The test for analyzeTestResults that "omits all cumulative fields when no
cumulative results are provided" is missing assertions for
cumulativePassedButEmptyRender and cumulativeSuccessRateWithoutEmptyRender;
update the test that calls analyzeTestResults(results) to also assert that
analysis.cumulativePassedButEmptyRender and
analysis.cumulativeSuccessRateWithoutEmptyRender are undefined so all cumulative
properties returned by analyzeTestResults are covered.
Sidnioulz
approved these changes
May 5, 2026
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 93 | 93 | 0 |
| Self size | 1.37 MB | 1.12 MB | 🎉 -252 KB 🎉 |
| Dependency size | 24.67 MB | 24.67 MB | 🎉 -30 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 881 KB | 880 KB | 🎉 -2 KB 🎉 |
| Dependency size | 68.30 MB | 68.29 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 51 | 51 | 0 |
| Self size | 1.07 MB | 1.05 MB | 🎉 -19 KB 🎉 |
| Dependency size | 37.15 MB | 37.15 MB | 🎉 -347 B 🎉 |
| Bundle Size Analyzer | node | node |
This was referenced May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
What I did
In an agentic session the ghost stories run shouldn’t trigger the vitest reporter used for self healing
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
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 pull request has been released as version
0.0.0-pr-34707-sha-66dc6ff5. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34707-sha-66dc6ff5 sandboxor in an existing project withnpx storybook@0.0.0-pr-34707-sha-66dc6ff5 upgrade.More information
0.0.0-pr-34707-sha-66dc6ff5yann/fix-observability66dc6ff51777958546)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34707Summary by CodeRabbit
Chores
Refactor
Tests