-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Telemetry: Add playwright-prompt #33229
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
Telemetry: Add playwright-prompt #33229
Conversation
…orted); expose abort outcome from tasks; wire TelemetryService and fix tests
|
View your CI Pipeline Execution ↗ for commit 1bb3387
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThis PR enhances the Playwright installation flow to track installation outcomes as structured result states ('installed', 'skipped', 'aborted', 'failed'), integrates telemetry to report these results, and refactors AddonConfigurationCommand to accept command options at the constructor level rather than through method parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AddonConfigurationCommand
participant AddonVitestService
participant executeTaskWithSpinner
participant TelemetryService
Client->>AddonConfigurationCommand: execute({ addons, configDir, ... })
Note over AddonConfigurationCommand: Uses constructor-bound commandOptions
AddonConfigurationCommand->>AddonConfigurationCommand: configureAddons()
AddonConfigurationCommand->>AddonVitestService: installPlaywright({ yes })
AddonVitestService->>executeTaskWithSpinner: Run installation task
executeTaskWithSpinner-->>AddonVitestService: Returns 'aborted' | void
alt Installation Success
AddonVitestService-->>AddonConfigurationCommand: { errors: [], result: 'installed' }
else User Aborted
AddonVitestService-->>AddonConfigurationCommand: { errors: [], result: 'aborted' }
else Installation Skipped
AddonVitestService-->>AddonConfigurationCommand: { errors: [], result: 'skipped' }
else Installation Failed
AddonVitestService-->>AddonConfigurationCommand: { errors: [...], result: 'failed' }
end
Note over AddonConfigurationCommand: Capture result
AddonConfigurationCommand->>TelemetryService: trackPlaywrightPromptDecision(result)
TelemetryService-->>AddonConfigurationCommand: Telemetry recorded
AddonConfigurationCommand-->>Client: Execution complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
✨ Finishing touches
Comment |
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.
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/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts (1)
62-66:vi.clearAllMocks()placement clears mock implementations set inbeforeEach.Calling
vi.clearAllMocks()at line 65 after setting up mock implementations in lines 40-63 will clear those implementations. This likely causes tests to use empty/undefined mocks. Either movevi.clearAllMocks()to the start ofbeforeEachor verify this is intentional behavior.beforeEach(async () => { + vi.clearAllMocks(); + const { postinstallAddon } = await import('../../../cli-storybook/src/postinstallAddon'); // ... rest of setup - - vi.clearAllMocks(); });
🧹 Nitpick comments (2)
code/core/src/node-logger/tasks.test.ts (2)
9-33: Biome warnings forthenproperty are expected here.The static analysis warnings about adding
thento an object are intentional in this context—you're simulatingExecaChildProcess's thenable behavior. Consider adding an inline eslint/biome disable comment to suppress the noise:+// biome-ignore lint/suspicious/noThenProperty: Intentionally simulating ExecaChildProcess thenable const cp: Partial<ExecaChildProcess> = { stdout: stdout as any, then: undefined as any,
35-74: Consider splitting into separate test cases for clarity.The single test validates both the success path (returns
undefined) and abort path (returns'aborted'). Splitting these into two distinctit()blocks would improve test readability and failure diagnostics.Additionally, this test doesn't mock the
spinnerorlogTrackerdependencies from./tasks, so it will hit the real implementations. Depending on test isolation requirements, you may want to mock these.describe('executeTaskWithSpinner', () => { - it('returns "aborted" when the child process rejects with an abort error', async () => { + it('returns undefined when child process succeeds', async () => { const outcome = await executeTaskWithSpinner(() => makeChild(), { id: 'test', intro: 'Intro', error: 'Error', success: 'Success', abortable: true, }); - - // Non-abort path returns undefined expect(outcome).toBeUndefined(); + }); - // Simulate an aborted child process by rejecting with an abort-like error message - const outcome2 = await executeTaskWithSpinner( + it('returns "aborted" when the child process rejects with an abort error', async () => { + const outcome = await executeTaskWithSpinner( () => { // ... existing abort simulation code }, { ... } ); - - expect(outcome2).toBe('aborted'); + expect(outcome).toBe('aborted'); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
code/core/src/cli/AddonVitestService.ts(4 hunks)code/core/src/node-logger/tasks.test.ts(1 hunks)code/core/src/node-logger/tasks.ts(6 hunks)code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts(2 hunks)code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts(6 hunks)code/lib/create-storybook/src/services/TelemetryService.ts(2 hunks)
🧰 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/core/src/cli/AddonVitestService.tscode/core/src/node-logger/tasks.test.tscode/lib/create-storybook/src/services/TelemetryService.tscode/core/src/node-logger/tasks.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/cli/AddonVitestService.tscode/core/src/node-logger/tasks.test.tscode/lib/create-storybook/src/services/TelemetryService.tscode/core/src/node-logger/tasks.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.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/core/src/cli/AddonVitestService.tscode/core/src/node-logger/tasks.test.tscode/lib/create-storybook/src/services/TelemetryService.tscode/core/src/node-logger/tasks.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.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/core/src/cli/AddonVitestService.tscode/core/src/node-logger/tasks.test.tscode/lib/create-storybook/src/services/TelemetryService.tscode/core/src/node-logger/tasks.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.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/core/src/cli/AddonVitestService.tscode/core/src/node-logger/tasks.test.tscode/lib/create-storybook/src/services/TelemetryService.tscode/core/src/node-logger/tasks.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.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/node-logger/tasks.test.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.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/node-logger/tasks.test.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.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/node-logger/tasks.test.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts
🧠 Learnings (18)
📚 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/node-logger/tasks.test.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} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/node-logger/tasks.test.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} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/node-logger/tasks.test.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} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/node-logger/tasks.test.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} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/core/src/node-logger/tasks.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/core/src/node-logger/tasks.test.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} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/core/src/node-logger/tasks.test.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} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/node-logger/tasks.test.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.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/node-logger/tasks.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/core/src/node-logger/tasks.test.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} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/node-logger/tasks.test.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} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/core/src/node-logger/tasks.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/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/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.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.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.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.ts
🧬 Code graph analysis (3)
code/core/src/node-logger/tasks.test.ts (1)
code/core/src/node-logger/tasks.ts (1)
executeTaskWithSpinner(114-173)
code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts (3)
code/lib/create-storybook/src/generators/types.ts (1)
CommandOptions(118-137)code/core/src/cli/AddonVitestService.ts (1)
AddonVitestService(39-386)code/lib/create-storybook/src/services/TelemetryService.ts (1)
TelemetryService(10-115)
code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts (1)
code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts (1)
AddonConfigurationCommand(35-163)
🪛 Biome (2.1.2)
code/core/src/node-logger/tasks.test.ts
[error] 20-20: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 27-27: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 59-59: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ 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). (5)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (10)
code/core/src/cli/AddonVitestService.ts (1)
117-176: Well-structured result tracking for Playwright installation outcomes.The explicit result state tracking (
'installed' | 'skipped' | 'aborted' | 'failed') is a clean approach that enables proper telemetry tracking downstream. All control flow paths correctly assign the result before returning.code/lib/create-storybook/src/services/TelemetryService.ts (1)
35-43: Clean telemetry integration for Playwright installation tracking.The method follows the established pattern of other tracking methods in this service. The result type aligns correctly with
AddonVitestService.installPlaywright()return type.code/core/src/node-logger/tasks.ts (2)
56-112: Consistent return type update for abort handling.The updated signature
Promise<'aborted' | void>properly communicates the possible outcomes. The explicitreturn 'aborted'on abort andreturn undefinedon success provide type-safe discrimination for callers.
114-173: LGTM!Mirrors the changes in
executeTaskwith identical abort-handling semantics. The return type update is consistent across both task execution functions.code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts (1)
50-53: Consider adding test coverage for telemetry tracking.The
AddonConfigurationCommandnow callstelemetryService.trackPlaywrightPromptDecision(result)after Playwright installation when@storybook/addon-vitestis configured. The tests mockdisableTelemetry: true, which disables tracking, but there's no explicit verification that telemetry is called when enabled.Consider adding a test case that verifies telemetry is tracked when
disableTelemetry: false, or at minimum verify that the telemetry service is instantiated correctly. Based on learnings, ensure all required dependencies are mocked.code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts (5)
1-9: LGTM!Imports are correctly structured. The TelemetryService import aligns with its usage in the constructor.
35-41: LGTM!Good use of dependency injection with default instantiation. This pattern supports testability while keeping production usage simple. The
readonlymodifiers appropriately enforce immutability.
108-162: LGTM!The refactoring to use
this.commandOptions.yesis consistent with the new constructor pattern. The method logic remains sound.
165-174: LGTM!Clean wrapper function that properly separates constructor dependencies (
packageManager,options) from execution parameters. The type extension is correct.
64-70: I'll verify the review comment by investigating theinstallPlaywrightmethod and error handling patterns in the codebase.
<function_calls>
#!/bin/bashFind AddonVitestService file
fd -t f "AddonVitestService" --extension ts --extension js
</function_calls>
| vi.mock('storybook/internal/cli', async (actualImport) => ({ | ||
| ...(await actualImport()), | ||
| AddonVitestService: vi.fn().mockImplementation(() => ({ | ||
| installPlaywright: vi.fn().mockResolvedValue([]), | ||
| installPlaywright: vi.fn().mockResolvedValue({ errors: [] }), | ||
| })), | ||
| })); |
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.
Mock missing the result field from the updated return type.
The installPlaywright mock returns { errors: [] }, but the actual return type is { errors: string[]; result: 'installed' | 'skipped' | 'aborted' | 'failed' }. The production code accesses result to pass to telemetryService.trackPlaywrightPromptDecision(result). This may cause test failures or mask issues.
vi.mock('storybook/internal/cli', async (actualImport) => ({
...(await actualImport()),
AddonVitestService: vi.fn().mockImplementation(() => ({
- installPlaywright: vi.fn().mockResolvedValue({ errors: [] }),
+ installPlaywright: vi.fn().mockResolvedValue({ errors: [], result: 'installed' }),
})),
}));And in beforeEach:
const mockInstance = {
- installPlaywright: vi.fn().mockResolvedValue({ errors: [] }),
+ installPlaywright: vi.fn().mockResolvedValue({ errors: [], result: 'installed' }),
};Also applies to: 43-48
🤖 Prompt for AI Agents
In code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts
around lines 10-15 (and similarly at lines 43-48), the mocked installPlaywright
return value is missing the required result field; update the mock to return the
full shape { errors: string[]; result: 'installed' | 'skipped' | 'aborted' |
'failed' } (e.g., { errors: [], result: 'installed' }) so the production code
can read result and telemetryService.trackPlaywrightPromptDecision(result) will
receive a valid value.
…p-playwright-telemetry
shilman
left a comment
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.
Great. Thank you!!!
…ywright-telemetry Telemetry: Add playwright-prompt (cherry picked from commit 8db6c43)
Closes #
What I did
I have added telemetry for the playwright prompt.
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
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.