Skip to content

Commit 704cfae

Browse files
valentinpalkovicstorybook-bot
valentinpalkovic
authored and
storybook-bot
committed
Merge pull request #33229 from storybookjs/valentin/add-init-step-playwright-telemetry
Telemetry: Add playwright-prompt (cherry picked from commit 8db6c43)
1 parent dd5a891 commit 704cfae

6 files changed

Lines changed: 134 additions & 47 deletions

File tree

‎code/core/src/cli/AddonVitestService.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,16 @@ export class AddonVitestService {
114114
* @param options - Installation options
115115
* @returns Array of error messages if installation fails
116116
*/
117-
async installPlaywright(options: { yes?: boolean } = {}): Promise<{ errors: string[] }> {
117+
async installPlaywright(
118+
options: { yes?: boolean } = {}
119+
): Promise<{ errors: string[]; result: 'installed' | 'skipped' | 'aborted' | 'failed' }> {
118120
const errors: string[] = [];
119121

120122
const playwrightCommand = ['playwright', 'install', 'chromium', '--with-deps'];
121123
const playwrightCommandString = this.packageManager.getPackageCommand(playwrightCommand);
122124

125+
let result: 'installed' | 'skipped' | 'aborted' | 'failed';
126+
123127
try {
124128
const shouldBeInstalled = options.yes
125129
? true
@@ -135,7 +139,7 @@ export class AddonVitestService {
135139
})();
136140

137141
if (shouldBeInstalled) {
138-
await prompt.executeTaskWithSpinner(
142+
const processAborted = await prompt.executeTaskWithSpinner(
139143
(signal) =>
140144
this.packageManager.runPackageCommand({
141145
args: playwrightCommand,
@@ -150,10 +154,17 @@ export class AddonVitestService {
150154
abortable: true,
151155
}
152156
);
157+
if (processAborted) {
158+
result = 'aborted';
159+
} else {
160+
result = 'installed';
161+
}
153162
} else {
154163
logger.warn('Playwright installation skipped');
164+
result = 'skipped';
155165
}
156166
} catch (e) {
167+
result = 'failed';
157168
ErrorCollector.addError(e);
158169
if (e instanceof Error) {
159170
errors.push(e.stack ?? e.message);
@@ -162,7 +173,7 @@ export class AddonVitestService {
162173
}
163174
}
164175

165-
return { errors };
176+
return { errors, result };
166177
}
167178

168179
/**

‎code/core/src/node-logger/tasks.test.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
3+
// eslint-disable-next-line depend/ban-dependencies
4+
import type { ExecaChildProcess } from 'execa';
5+
6+
import { executeTaskWithSpinner } from './tasks';
7+
8+
// Create a minimal fake ExecaChildProcess
9+
const makeChild = (onStart?: (cp: Partial<ExecaChildProcess>) => void): ExecaChildProcess => {
10+
const listeners: Record<string, Function[]> = {};
11+
const stdout = {
12+
on: vi.fn((event: string, cb: (data: Buffer) => void) => {
13+
listeners[event] ||= [];
14+
listeners[event].push(cb);
15+
}),
16+
} as any;
17+
18+
const cp: Partial<ExecaChildProcess> = {
19+
stdout: stdout as any,
20+
then: undefined as any,
21+
catch: undefined as any,
22+
finally: undefined as any,
23+
};
24+
25+
const promise = Promise.resolve() as any;
26+
Object.setPrototypeOf(cp, promise);
27+
(cp as any).then = promise.then.bind(promise);
28+
(cp as any).catch = promise.catch.bind(promise);
29+
(cp as any).finally = promise.finally.bind(promise);
30+
31+
onStart?.(cp);
32+
return cp as ExecaChildProcess;
33+
};
34+
35+
describe('executeTaskWithSpinner', () => {
36+
it('returns "aborted" when the child process rejects with an abort error', async () => {
37+
const outcome = await executeTaskWithSpinner(() => makeChild(), {
38+
id: 'test',
39+
intro: 'Intro',
40+
error: 'Error',
41+
success: 'Success',
42+
abortable: true,
43+
});
44+
45+
// Non-abort path returns undefined
46+
expect(outcome).toBeUndefined();
47+
48+
// Simulate an aborted child process by rejecting with an abort-like error message
49+
const outcome2 = await executeTaskWithSpinner(
50+
() => {
51+
const err = new Error('The operation was aborted');
52+
const p = Promise.reject(err);
53+
// Avoid unhandled rejection warnings
54+
p.catch(() => {});
55+
const cp: any = makeChild();
56+
// Make the thenable reject
57+
const rejected = p as any;
58+
Object.setPrototypeOf(cp, rejected);
59+
cp.then = rejected.then.bind(rejected);
60+
cp.catch = rejected.catch.bind(rejected);
61+
cp.finally = rejected.finally.bind(rejected);
62+
return cp;
63+
},
64+
{
65+
id: 'test2',
66+
intro: 'Intro',
67+
error: 'Error',
68+
success: 'Success',
69+
abortable: true,
70+
}
71+
);
72+
73+
expect(outcome2).toBe('aborted');
74+
});
75+
});

‎code/core/src/node-logger/tasks.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export const executeTask = async (
6161
success,
6262
abortable = false,
6363
}: { intro: string; error: string; success: string; abortable?: boolean }
64-
) => {
64+
): Promise<'aborted' | void> => {
6565
logTracker.addLog('info', intro);
6666
log(intro);
6767

@@ -99,7 +99,7 @@ export const executeTask = async (
9999
if (isAborted) {
100100
logTracker.addLog('info', `${intro} aborted`);
101101
log(CLI_COLORS.error(`${intro} aborted`));
102-
return;
102+
return 'aborted';
103103
}
104104
const errorMessage = err instanceof Error ? (err.stack ?? err.message) : String(err);
105105
logTracker.addLog('error', error, { error: errorMessage });
@@ -108,6 +108,7 @@ export const executeTask = async (
108108
} finally {
109109
cleanup?.();
110110
}
111+
return undefined;
111112
};
112113

113114
export const executeTaskWithSpinner = async (
@@ -119,7 +120,7 @@ export const executeTaskWithSpinner = async (
119120
success,
120121
abortable = false,
121122
}: { id: string; intro: string; error: string; success: string; abortable?: boolean }
122-
) => {
123+
): Promise<'aborted' | void> => {
123124
logTracker.addLog('info', intro);
124125

125126
let abortController: AbortController | undefined;
@@ -159,7 +160,7 @@ export const executeTaskWithSpinner = async (
159160
if (isAborted) {
160161
logTracker.addLog('info', `${intro} aborted`);
161162
task.cancel(CLI_COLORS.warning(`${intro} aborted`));
162-
return;
163+
return 'aborted';
163164
}
164165
const errorMessage = err instanceof Error ? (err.stack ?? err.message) : String(err);
165166
logTracker.addLog('error', error, { error: errorMessage });
@@ -168,4 +169,5 @@ export const executeTaskWithSpinner = async (
168169
} finally {
169170
cleanup?.();
170171
}
172+
return undefined;
171173
};

‎code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import { AddonConfigurationCommand } from './AddonConfigurationCommand';
77

88
vi.mock('storybook/internal/node-logger', { spy: true });
99

10-
vi.mock('storybook/internal/cli', () => ({
10+
vi.mock('storybook/internal/cli', async (actualImport) => ({
11+
...(await actualImport()),
1112
AddonVitestService: vi.fn().mockImplementation(() => ({
12-
installPlaywright: vi.fn().mockResolvedValue([]),
13+
installPlaywright: vi.fn().mockResolvedValue({ errors: [] }),
1314
})),
1415
}));
1516

@@ -42,11 +43,14 @@ describe('AddonConfigurationCommand', () => {
4243
const { AddonVitestService } = await import('storybook/internal/cli');
4344
mockAddonVitestService = vi.mocked(AddonVitestService as any);
4445
const mockInstance = {
45-
installPlaywright: vi.fn().mockResolvedValue([]),
46+
installPlaywright: vi.fn().mockResolvedValue({ errors: [] }),
4647
};
47-
mockAddonVitestService.mockImplementation(() => mockInstance);
48+
mockAddonVitestService.mockImplementation(() => mockInstance as any);
4849

49-
command = new AddonConfigurationCommand(mockPackageManager);
50+
command = new AddonConfigurationCommand(mockPackageManager, {
51+
yes: true,
52+
disableTelemetry: true,
53+
} as any);
5054

5155
mockTask = {
5256
success: vi.fn(),
@@ -64,16 +68,11 @@ describe('AddonConfigurationCommand', () => {
6468
describe('execute', () => {
6569
it('should skip configuration when no addons are provided', async () => {
6670
const addons: string[] = [];
67-
const options = {
68-
packageManager: PackageManagerName.NPM,
69-
features: [],
70-
};
7171

7272
const result = await command.execute({
7373
dependencyInstallationResult: { status: 'success' },
7474
addons,
7575
configDir: '.storybook',
76-
options,
7776
});
7877

7978
expect(result.status).toBe('success');
@@ -83,17 +82,11 @@ describe('AddonConfigurationCommand', () => {
8382

8483
it('should configure test addons when test feature is enabled', async () => {
8584
const addons = ['@storybook/addon-a11y', '@storybook/addon-vitest'];
86-
const options = {
87-
packageManager: PackageManagerName.NPM,
88-
features: [],
89-
yes: true,
90-
};
9185

9286
const result = await command.execute({
9387
dependencyInstallationResult: { status: 'success' },
9488
addons,
9589
configDir: '.storybook',
96-
options,
9790
});
9891

9992
expect(result.status).toBe('success');
@@ -105,10 +98,6 @@ describe('AddonConfigurationCommand', () => {
10598

10699
it('should handle configuration errors gracefully', async () => {
107100
const addons = ['@storybook/addon-a11y', '@storybook/addon-vitest'];
108-
const options = {
109-
packageManager: PackageManagerName.NPM,
110-
features: [],
111-
};
112101
const error = new Error('Configuration failed');
113102

114103
mockPostinstallAddon.mockRejectedValue(error);
@@ -117,7 +106,6 @@ describe('AddonConfigurationCommand', () => {
117106
dependencyInstallationResult: { status: 'success' },
118107
addons,
119108
configDir: '.storybook',
120-
options,
121109
});
122110

123111
expect(result.status).toBe('failed');
@@ -128,17 +116,11 @@ describe('AddonConfigurationCommand', () => {
128116

129117
it('should complete successfully with valid configuration', async () => {
130118
const addons = ['@storybook/addon-a11y', '@storybook/addon-vitest'];
131-
const options = {
132-
packageManager: PackageManagerName.NPM,
133-
features: [],
134-
yes: true,
135-
};
136119

137120
const result = await command.execute({
138121
dependencyInstallationResult: { status: 'success' },
139122
addons,
140123
configDir: '.storybook',
141-
options,
142124
});
143125

144126
expect(result.status).toBe('success');

‎code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { ErrorCollector } from 'storybook/internal/telemetry';
66
import { dedent } from 'ts-dedent';
77

88
import type { CommandOptions } from '../generators/types';
9+
import { TelemetryService } from '../services';
910

1011
const ADDON_INSTALLATION_INSTRUCTIONS = {
1112
'@storybook/addon-vitest':
@@ -14,7 +15,6 @@ const ADDON_INSTALLATION_INSTRUCTIONS = {
1415

1516
type ExecuteAddonConfigurationParams = {
1617
addons: string[];
17-
options: CommandOptions;
1818
configDir?: string;
1919
dependencyInstallationResult: { status: 'success' | 'failed' };
2020
};
@@ -35,18 +35,19 @@ export type ExecuteAddonConfigurationResult = {
3535
export class AddonConfigurationCommand {
3636
constructor(
3737
readonly packageManager: JsPackageManager,
38-
private readonly addonVitestService = new AddonVitestService(packageManager)
38+
private readonly commandOptions: CommandOptions,
39+
private readonly addonVitestService = new AddonVitestService(packageManager),
40+
private readonly telemetryService = new TelemetryService(commandOptions.disableTelemetry)
3941
) {}
4042

4143
/** Execute addon configuration */
4244
async execute({
43-
options,
4445
addons,
4546
configDir,
4647
dependencyInstallationResult,
4748
}: ExecuteAddonConfigurationParams): Promise<ExecuteAddonConfigurationResult> {
4849
const areDependenciesInstalled =
49-
dependencyInstallationResult.status === 'success' && !options.skipInstall;
50+
dependencyInstallationResult.status === 'success' && !this.commandOptions.skipInstall;
5051

5152
if (!areDependenciesInstalled && this.getAddonsWithInstructions(addons).length > 0) {
5253
this.logManualAddonInstructions(addons);
@@ -58,12 +59,14 @@ export class AddonConfigurationCommand {
5859
}
5960

6061
try {
61-
const { hasFailures, addonResults } = await this.configureAddons(configDir, addons, options);
62+
const { hasFailures, addonResults } = await this.configureAddons(configDir, addons);
6263

6364
if (addonResults.has('@storybook/addon-vitest')) {
64-
await this.addonVitestService.installPlaywright({
65-
yes: options.yes,
65+
const { result } = await this.addonVitestService.installPlaywright({
66+
yes: this.commandOptions.yes,
6667
});
68+
// Map outcome to telemetry decision
69+
await this.telemetryService.trackPlaywrightPromptDecision(result);
6770
}
6871

6972
return { status: hasFailures ? 'failed' : 'success' };
@@ -103,7 +106,7 @@ export class AddonConfigurationCommand {
103106
}
104107

105108
/** Configure test addons (a11y and vitest) */
106-
private async configureAddons(configDir: string, addons: string[], options: CommandOptions) {
109+
private async configureAddons(configDir: string, addons: string[]) {
107110
// Import postinstallAddon from cli-storybook package
108111
const { postinstallAddon } = await import('../../../cli-storybook/src/postinstallAddon');
109112

@@ -123,7 +126,7 @@ export class AddonConfigurationCommand {
123126
await postinstallAddon(addon, {
124127
packageManager: this.packageManager.type,
125128
configDir,
126-
yes: options.yes,
129+
yes: this.commandOptions.yes,
127130
skipInstall: true,
128131
skipDependencyManagement: true,
129132
logger,
@@ -161,7 +164,11 @@ export class AddonConfigurationCommand {
161164

162165
export const executeAddonConfiguration = ({
163166
packageManager,
164-
...options
165-
}: ExecuteAddonConfigurationParams & { packageManager: JsPackageManager }) => {
166-
return new AddonConfigurationCommand(packageManager).execute(options);
167+
options,
168+
...rest
169+
}: ExecuteAddonConfigurationParams & {
170+
packageManager: JsPackageManager;
171+
options: CommandOptions;
172+
}) => {
173+
return new AddonConfigurationCommand(packageManager, options).execute(rest);
167174
};

‎code/lib/create-storybook/src/services/TelemetryService.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export class TelemetryService {
1818

1919
/** Track a new user check step */
2020
async trackNewUserCheck(newUser: boolean): Promise<void> {
21-
this.runTelemetryIfEnabled('init-step', {
21+
await this.runTelemetryIfEnabled('init-step', {
2222
step: 'new-user-check',
2323
newUser,
2424
});
@@ -32,6 +32,16 @@ export class TelemetryService {
3232
});
3333
}
3434

35+
/** Track Playwright prompt decision (install | skip | aborted) */
36+
async trackPlaywrightPromptDecision(
37+
result: 'installed' | 'skipped' | 'aborted' | 'failed'
38+
): Promise<void> {
39+
await this.runTelemetryIfEnabled('init-step', {
40+
step: 'playwright-install',
41+
result,
42+
});
43+
}
44+
3545
/** Track the main init event with all metadata */
3646
async trackInit(data: {
3747
projectType: ProjectType;

0 commit comments

Comments
 (0)