Build: Upgrade to Vitest 4#33086
Conversation
|
View your CI Pipeline Execution ↗ for commit a62905a
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThe PR upgrades the codebase to Vitest 4.0.0, updating dependency versions across package.json files, migrating import paths from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
code/vitest.config.storybook.mts (2)
8-19: Optional: Consider documenting INSPECT usage.The
INSPECTenvironment variable enables the vite-plugin-inspect for debugging transformations. Consider documenting this in the repository's testing documentation for developers who need to debug Storybook test transformations.
39-40: TODO: Track UseState test support.The testNamePattern excludes tests with "UseState" until portable stories support storybook/preview-api hooks. Consider creating a tracking issue if one doesn't exist to ensure this limitation is revisited.
Would you like me to generate a tracking issue for enabling UseState tests once portable stories support is complete?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
code/.storybook/storybook.setup.ts(1 hunks)code/addons/a11y/src/preview.test.tsx(1 hunks)code/addons/vitest/src/node/test-manager.test.ts(2 hunks)code/core/src/instrumenter/instrumenter.test.ts(1 hunks)code/core/src/preview-api/modules/preview-web/PreviewWeb.test.ts(3 hunks)code/package.json(2 hunks)code/renderers/react/vitest.setup.ts(0 hunks)code/vitest-storybook.config.mts(0 hunks)code/vitest.config.storybook.mts(1 hunks)code/vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- code/renderers/react/vitest.setup.ts
- code/vitest-storybook.config.mts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/.storybook/storybook.setup.tscode/package.json
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/.storybook/storybook.setup.tscode/package.json
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
code/package.json
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/package.json
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/package.json
🧬 Code graph analysis (1)
code/vitest.config.storybook.mts (1)
code/addons/vitest/src/vitest-plugin/index.ts (1)
storybookTest(100-448)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (7)
code/core/src/preview-api/modules/preview-web/PreviewWeb.test.ts (1)
3908-3909: LGTM! Snapshot updates reflect Vitest v4 mock serialization.The changes from
[MockFunction spy]to[MockFunction]across all three snapshot locations are confirmed and expected when upgrading from Vitest v3 to v4. The mock functions (docs.containeranddocs.page) remain correctly mocked, with Vitest v4 serializing them in the updated format.All three occurrences are updated consistently:
- Lines 3908-3909 and 3982-3983: Both update
containerandpage- Line 4034: Correctly updates only
page(nocontainerin this story)The changes are snapshot-only with no logic modifications.
Please confirm the test suite passes with these updated snapshots in your local environment, as automated verification is not available in this sandbox.
code/core/src/instrumenter/instrumenter.test.ts (1)
697-703: LGTM: Async function interception test.The change to make the test function async aligns with the test's purpose of verifying that the instrumenter correctly steps through async interceptable functions. The test properly awaits the results and verifies the expected call counts.
code/addons/vitest/src/node/test-manager.test.ts (1)
1-1: LGTM: Improved mock setup pattern.The refactoring to use a dedicated
mockCreateVitestfunction with abeforeEachhook makes the test setup more explicit and maintainable. This pattern ensures consistent mock behavior across all tests.Also applies to: 45-59
code/vitest.config.ts (1)
10-10: LGTM: Config file renamed to follow conventions.The rename from
vitest-storybook.config.mtstovitest.config.storybook.mtsfollows Vitest 4 naming conventions and aligns with the new configuration file structure.code/vitest.config.storybook.mts (1)
21-55: LGTM: Well-structured Vitest project configuration.The configuration properly sets up browser testing with Playwright, appropriate excludes, and integration with Storybook's test plugin. The happy-dom environment and setupFiles are correctly configured for Storybook testing.
code/.storybook/storybook.setup.ts (1)
18-22: Import path verified as correct.The import path
vitest/browseris correct for Vitest 4 according to the release notes and Interactivity API documentation. The code properly accesses the browser API from this location. No changes needed.code/package.json (1)
160-163: Vitest 4.0.0 packages verified as stable with no known security issues.All specified packages (vitest, @vitest/browser, @vitest/browser-playwright, @vitest/coverage-istanbul, @vitest/coverage-v8) exist at version 4.0.0 on npm. @vitest/browser-playwright is officially stable as part of Vitest's Browser Mode (no longer experimental) and shows no known security vulnerabilities. The package is actively maintained.
Note: Vitest 4.0.0 includes breaking changes (API removals, worker-pool rewrite, browser package reorganization), which are documented in the migration guide but expected for a major version.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/vitest/src/vitest-plugin/test-utils.ts (1)
6-6: Update import path for Vitest 4 compatibility.Line 6 imports from the deprecated
@vitest/browser/contextpath, which was consolidated tovitest/browserin Vitest 4. The module augmentation on line 11 correctly targetsvitest/browser, but line 6 must also use this path for consistency and compatibility. Changeimport { server } from '@vitest/browser/context'toimport { server } from 'vitest/browser'.
🧹 Nitpick comments (4)
code/core/src/preview-api/modules/store/csf/prepareStory.test.ts (1)
579-585:vi.fn() as anychange is fine; consider avoidinganyin these two spots (optional).Switching to
render: vi.fn() as anyis a reasonable TS workaround for Vitest 4’s typings and doesn’t change test behavior. Given these two tests don’t assert onrenderat all, you could optionally avoid theanycast by reusing the existing top‑levelrenderstub instead of a mock here, for example:// at top of file, already present: const render = () => {}; // in the two mapping tests: const story = prepareStory( { /* ... */ }, { id, title }, { render } // no cast needed );Not blocking, but this would keep these cases aligned with the simpler stub used elsewhere in the file and slightly reduce
anyusage.Also applies to: 593-605
code/core/src/manager-api/tests/refs.test.ts (3)
16-16: Consider usingspy: trueoption for the mock.The mock of
@storybook/globaldoesn't include thespy: trueoption. According to coding guidelines, all package mocks should use this option.If a complete module replacement is necessary, the current approach is acceptable. However, if you only need to mock specific properties, consider:
-vi.mock('@storybook/global', () => { +vi.mock('@storybook/global', { spy: true }, () => {Based on coding guidelines: "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests."
35-40: Useconstinstead ofletforlocationMock.The
locationMockvariable is never reassigned, so it should be declared withconst.Apply this diff:
- let locationMock = vi.fn(); + const locationMock = vi.fn();Additionally, consider adding a comment to document the complex mock behavior. The current implementation returns different values based on call order, which creates an implicit dependency between test execution order and mock state. Per coding guidelines: "Document complex mock behaviors in Vitest tests."
For example:
// locationMock returns different values on successive calls: // - First call: edgecaseLocations[0] // - Subsequent calls: lastLocation const locationMock = vi.fn();
142-156: Test order dependency may cause issues.The edge case tests on lines 143-149 must run first and in a specific order to match the
edgecaseLocationsmock setup. This creates a fragile dependency that could break if:
- Tests are run in isolation (e.g., using
.only)- Vitest runs tests in parallel
- Tests are reordered
Consider refactoring to make each test independent, perhaps by resetting the mock in a
beforeEachblock or using test-specific mocks instead of relying on global call order.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
code/addons/vitest/src/node/test-manager.ts(1 hunks)code/addons/vitest/src/vitest-plugin/test-utils.ts(1 hunks)code/core/src/channels/index.test.ts(0 hunks)code/core/src/common/js-package-manager/JsPackageManager.test.ts(0 hunks)code/core/src/manager-api/tests/refs.test.ts(1 hunks)code/core/src/preview-api/modules/store/csf/prepareStory.test.ts(2 hunks)code/core/src/telemetry/sanitize.test.ts(1 hunks)code/vitest.shared.ts(1 hunks)scripts/vitest.config.ts(2 hunks)
💤 Files with no reviewable changes (2)
- code/core/src/channels/index.test.ts
- code/core/src/common/js-package-manager/JsPackageManager.test.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.tsscripts/vitest.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.tsscripts/vitest.config.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/telemetry/sanitize.test.ts
🧠 Learnings (20)
📓 Common learnings
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tsscripts/vitest.config.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tsscripts/vitest.config.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/vitest.shared.tsscripts/vitest.config.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tsscripts/vitest.config.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/vitest.shared.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.tsscripts/vitest.config.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with `vi.mocked()` in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/vitest.shared.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-28T14:50:24.872Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.872Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mocked()` to type and access the mocked functions in Vitest tests
Applied to files:
code/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Applied to files:
code/core/src/preview-api/modules/store/csf/prepareStory.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Use `NODE_OPTIONS=--max_old_space_size=4096` for memory optimization when running tests
Applied to files:
code/vitest.shared.tsscripts/vitest.config.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/vitest.shared.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/manager-api/tests/refs.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (4)
code/addons/vitest/src/node/test-manager.ts (1)
1-1: LGTM: Import path modernized for Vitest 4.The change from
'vitest/dist/node.js'to'vitest/node'uses the public API export instead of internal dist paths, which is the correct approach for Vitest 4.scripts/vitest.config.ts (1)
17-18: LGTM: Worker configuration updated for Vitest 4.The explicit
pool: 'threads'setting and simplifiedmaxWorkersconfiguration align with the same changes incode/vitest.shared.ts, ensuring consistency across Vitest configs.code/vitest.shared.ts (1)
19-19: ThemaxWorkersconfiguration is correct for Vitest 4. The code has already migrated from the deprecatedpoolOptions.threads.{minThreads, maxThreads}pattern to use the unifiedmaxWorkersoption, which is the standard approach in Vitest 4's pool architecture.code/core/src/telemetry/sanitize.test.ts (1)
45-45: Good refactoring to use the pre-defined variable.Using the
cwdvariable improves readability compared to the inline escaping expression.
yannbf
added
ci:merged
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
code/core/src/telemetry/sanitize.test.ts (1)
23-49: Path-sanitizing test setup looks good; consider guarding spy cleanupThe new setup with
mockCwd,vi.spyOn(process, 'cwd'), and a stack string that embeds${mockCwd}exercisessanitizeError(e, '/')exactly as intended: you verify the real stack includes the cwd, then assert the sanitized stack drops it and adds$SNIP, while preserving the message. This aligns with the updated sanitize API and the mocking guidelines. Based on learnings, ...One small robustness tweak: if
sanitizeErrorever throws or an assertion fails,mockCwdSpy.mockRestore()won’t run and could leak into later tests. You can guard the cleanup withtry/finally:- const sanitizedError = sanitizeError(e as Error, '/'); - - expect(sanitizedError.message).toEqual(errorMessage); - expect(sanitizedError.stack).toEqual(expect.not.stringContaining(mockCwd)); - expect(sanitizedError.stack).toEqual(expect.stringContaining('$SNIP')); - - mockCwdSpy.mockRestore(); + try { + const sanitizedError = sanitizeError(e as Error, '/'); + + expect(sanitizedError.message).toEqual(errorMessage); + expect(sanitizedError.stack).toEqual(expect.not.stringContaining(mockCwd)); + expect(sanitizedError.stack).toEqual(expect.stringContaining('$SNIP')); + } finally { + mockCwdSpy.mockRestore(); + }code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts (1)
99-162: Add test coverage for telemetry tracking.The tests configure the
@storybook/addon-vitestaddon, which triggerstrackPlaywrightPromptDecision(seeAddonConfigurationCommand.tsline 62), but neither test verifies this method is called. Consider adding an assertion to ensure telemetry is tracked correctly.For example, add this assertion in the "should configure test addons when test feature is enabled" test:
const result = await command.execute({ dependencyInstallationResult: { status: 'success' }, addons, configDir: '.storybook', }); expect(result.status).toBe('success'); expect(prompt.taskLog).toHaveBeenCalledWith({ id: 'configure-addons', title: 'Configuring addons...', }); + + // Verify telemetry tracking for vitest addon + const mockTelemetryService = (command as any).telemetryService; + expect(mockTelemetryService.trackPlaywrightPromptDecision).toHaveBeenCalledWith('installed'); });As per coding guidelines, ensure adequate test coverage for business logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
code/core/src/manager-api/tests/refs.test.ts(1 hunks)code/core/src/telemetry/sanitize.test.ts(1 hunks)code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts(3 hunks)code/lib/eslint-plugin/vitest.config.ts(1 hunks)code/vitest.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/lib/eslint-plugin/vitest.config.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
🧠 Learnings (24)
📓 Common learnings
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/vitest.config.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/vitest.config.tscode/core/src/telemetry/sanitize.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/vitest.config.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Use `NODE_OPTIONS=--max_old_space_size=4096` for memory optimization when running tests
Applied to files:
code/vitest.config.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts
📚 Learning: 2025-11-28T14:50:24.872Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.872Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-28T14:50:24.872Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.872Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mocked()` to type and access the mocked functions in Vitest tests
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with `vi.mocked()` in Vitest tests
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-28T14:50:24.872Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.872Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/core/src/telemetry/sanitize.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/telemetry/sanitize.test.tscode/core/src/manager-api/tests/refs.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/manager-api/tests/refs.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: merged
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/core/src/manager-api/tests/refs.test.ts (1)
35-40: Location mock refactor is clear and preserves call-order semanticsUsing a dedicated
locationMockwithmockReturnValueOnceplus a finalmockReturnValuemakes theglobal.locationsequencing much easier to follow and extend (just append toedgecaseLocations). This keeps the existing test reliance on call order explicit without changing behavior.code/vitest.config.ts (2)
23-23: The new Storybook config filevitest.config.storybook.mtsexists and is properly configured with appropriate Vitest project settings, storybookTest plugin, browser configuration, and exclusion patterns. The old file has been successfully removed. No action needed.
20-21: No action needed. Thepool: 'threads'andmaxWorkers: threadCountconfiguration correctly follows Vitest v4 API. The maxWorkers option properly handles undefined values (using Vitest defaults locally) and conditional numeric values in CI environments, which aligns with v4 specifications.
…book into yann/vitest-v4-upgrade
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/vitest.config.storybook.ts (1)
8-8: Consider using a more specific type for plugin array.The
any[]type reduces type safety. Consider importingPluginfrom'vite'or'vitest/config'and usingPlugin[]instead.Apply this diff:
+import type { Plugin } from 'vite'; import { defaultExclude, defineProject } from 'vitest/config'; import { storybookTest } from '@storybook/addon-vitest/vitest-plugin'; import { playwright } from '@vitest/browser-playwright'; import Inspect from 'vite-plugin-inspect'; -const extraPlugins: any[] = []; +const extraPlugins: Plugin[] = [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
code/lib/eslint-plugin/vitest-setup.ts(1 hunks)code/lib/eslint-plugin/vitest.config.ts(1 hunks)code/vitest-setup.ts(1 hunks)code/vitest.config.storybook.ts(1 hunks)code/vitest.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- code/vitest-setup.ts
- code/vitest.config.ts
- code/lib/eslint-plugin/vitest.config.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.tscode/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/lib/eslint-plugin/vitest-setup.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Applied to files:
code/vitest.config.storybook.ts
🧬 Code graph analysis (1)
code/vitest.config.storybook.ts (1)
code/addons/vitest/src/vitest-plugin/index.ts (1)
storybookTest(100-452)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: merged
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (6)
code/lib/eslint-plugin/vitest-setup.ts (1)
1-9: LGTM! RuleTester configuration follows the documented pattern.The setup correctly aliases RuleTester methods to Vitest's test runner functions, enabling ESLint rule testing with Vitest as documented in the typescript-eslint guide.
code/vitest.config.storybook.ts (5)
1-7: LGTM! Imports are correctly updated for Vitest 4.The imports use the correct packages for Vitest 4, including
@vitest/browser-playwrightfor browser testing support.
23-27: LGTM! storybookTest plugin configuration is correct.The tag filtering with
include: ['vitest']appropriately limits test execution to Storybook stories tagged for Vitest.
30-40: LGTM! Test configuration and exclusions are well-documented.The exclude patterns and testNamePattern are clearly explained with comments, making it easy to understand why certain tests are skipped.
53-54: LGTM! Environment configuration is appropriate.Using
happy-domas the test environment is suitable for Storybook UI component testing.
41-51: > Likely an incorrect or invalid review comment.
2 participants
Closes #
What I did
Migrating internally Vitest from v3 to v4
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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.