diff --git a/code/addons/test/src/node/test-manager.test.ts b/code/addons/test/src/node/test-manager.test.ts index b674c1d786f..db77ab2f3e2 100644 --- a/code/addons/test/src/node/test-manager.test.ts +++ b/code/addons/test/src/node/test-manager.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; -import { createVitest } from 'vitest/node'; +import { createVitest as actualCreateVitest } from 'vitest/node'; import { Channel, type ChannelTransport } from '@storybook/core/channels'; import type { StoryIndex } from '@storybook/types'; @@ -34,6 +34,7 @@ const vitest = vi.hoisted(() => ({ vi.mock('vitest/node', () => ({ createVitest: vi.fn(() => Promise.resolve(vitest)), })); +const createVitest = vi.mocked(actualCreateVitest); const transport = { setHandler: vi.fn(), send: vi.fn() } satisfies ChannelTransport; const mockChannel = new Channel({ transport }); @@ -109,7 +110,7 @@ describe('TestManager', () => { await testManager.handleWatchModeRequest({ providerId: TEST_PROVIDER_ID, watchMode: true }); expect(testManager.watchMode).toBe(true); - expect(createVitest).toHaveBeenCalledTimes(2); + expect(createVitest).toHaveBeenCalledTimes(1); // shouldn't restart vitest }); it('should handle run request', async () => { @@ -145,4 +146,57 @@ describe('TestManager', () => { expect(setTestNamePattern).toHaveBeenCalledWith(/^One$/); expect(vitest.runFiles).toHaveBeenCalledWith(tests.slice(0, 1), true); }); + + it('should handle coverage toggling', async () => { + const testManager = await TestManager.start(mockChannel, options); + expect(testManager.coverage).toBe(false); + expect(createVitest).toHaveBeenCalledTimes(1); + createVitest.mockClear(); + + await testManager.handleConfigChange({ + providerId: TEST_PROVIDER_ID, + config: { coverage: true, a11y: false }, + }); + expect(testManager.coverage).toBe(true); + expect(createVitest).toHaveBeenCalledTimes(1); + createVitest.mockClear(); + + await testManager.handleConfigChange({ + providerId: TEST_PROVIDER_ID, + config: { coverage: false, a11y: false }, + }); + expect(testManager.coverage).toBe(false); + expect(createVitest).toHaveBeenCalledTimes(1); + }); + + it('should temporarily disable coverage on focused tests', async () => { + vitest.globTestSpecs.mockImplementation(() => tests); + const testManager = await TestManager.start(mockChannel, options); + expect(testManager.coverage).toBe(false); + expect(createVitest).toHaveBeenCalledTimes(1); + + await testManager.handleConfigChange({ + providerId: TEST_PROVIDER_ID, + config: { coverage: true, a11y: false }, + }); + expect(testManager.coverage).toBe(true); + expect(createVitest).toHaveBeenCalledTimes(2); + + await testManager.handleRunRequest({ + providerId: TEST_PROVIDER_ID, + indexUrl: 'http://localhost:6006/index.json', + storyIds: ['button--primary', 'button--secondary'], + }); + // expect vitest to be restarted twice, without and with coverage + expect(createVitest).toHaveBeenCalledTimes(4); + expect(vitest.runFiles).toHaveBeenCalledWith([], true); + + await testManager.handleRunRequest({ + providerId: TEST_PROVIDER_ID, + indexUrl: 'http://localhost:6006/index.json', + }); + // don't expect vitest to be restarted, as we're running all tests + expect(createVitest).toHaveBeenCalledTimes(4); + expect(vitest.runFiles).toHaveBeenCalledWith(tests, true); + }); }); diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index 430fd45eb40..4770f2b5a17 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -39,9 +39,7 @@ export class TestManager { this.vitestManager.startVitest().then(() => options.onReady?.()); } - async handleConfigChange( - payload: TestingModuleConfigChangePayload<{ coverage: boolean; a11y: boolean }> - ) { + async handleConfigChange(payload: TestingModuleConfigChangePayload) { if (payload.providerId !== TEST_PROVIDER_ID) { return; } @@ -49,10 +47,9 @@ export class TestManager { process.env.VITEST_STORYBOOK_CONFIG = JSON.stringify(payload.config); if (this.coverage !== payload.config.coverage) { + this.coverage = payload.config.coverage; try { - this.coverage = payload.config.coverage; await this.vitestManager.restartVitest({ - watchMode: this.watchMode, coverage: this.coverage, }); } catch (e) { @@ -68,25 +65,31 @@ export class TestManager { } } - async handleWatchModeRequest(payload: TestingModuleWatchModeRequestPayload) { - try { - if (payload.providerId !== TEST_PROVIDER_ID) { - return; - } + async handleWatchModeRequest(payload: TestingModuleWatchModeRequestPayload) { + if (payload.providerId !== TEST_PROVIDER_ID) { + return; + } + this.watchMode = payload.watchMode; - if (payload.config) { - this.handleConfigChange({ - providerId: payload.providerId, - config: payload.config as any, - }); - } + if (payload.config) { + this.handleConfigChange({ + providerId: payload.providerId, + config: payload.config, + }); + } - if (this.watchMode !== payload.watchMode) { - this.watchMode = payload.watchMode; - await this.vitestManager.restartVitest({ watchMode: this.watchMode, coverage: false }); + if (this.coverage) { + try { + if (payload.watchMode) { + // if watch mode is toggled on and coverage is already enabled, restart vitest without coverage to automatically disable it + await this.vitestManager.restartVitest({ coverage: false }); + } else { + // if watch mode is toggled off and coverage is already enabled, restart vitest with coverage to automatically re-enable it + await this.vitestManager.restartVitest({ coverage: this.coverage }); + } + } catch (e) { + this.reportFatalError('Failed to change watch mode while coverage was enabled', e); } - } catch (e) { - this.reportFatalError('Failed to change watch mode', e); } } @@ -96,25 +99,22 @@ export class TestManager { return; } - const allTestsRun = (payload.storyIds ?? []).length === 0; - - if (payload.config && this.coverage !== payload.config.coverage) { - this.coverage = payload.config.coverage; + if (payload.config) { + this.handleConfigChange({ + providerId: payload.providerId, + config: payload.config, + }); } - if (this.coverage) { - /* - If we have coverage enabled and we're running all stories, - we have to restart Vitest AND disable watch mode otherwise the coverage report will be incorrect, - Vitest behaves wonky when re-using the same Vitest instance but with watch mode disabled, - among other things it causes the coverage report to be incorrect and stale. - - If we're only running a subset of stories, we have to temporarily disable coverage, - as a coverage report for a subset of stories is not useful. - */ + /* + If we're only running a subset of stories, we have to temporarily disable coverage, + as a coverage report for a subset of stories is not useful. + */ + const temporarilyDisableCoverage = + this.coverage && !this.watchMode && (payload.storyIds ?? []).length > 0; + if (temporarilyDisableCoverage) { await this.vitestManager.restartVitest({ - watchMode: allTestsRun ? false : this.watchMode, - coverage: allTestsRun, + coverage: false, }); } else { await this.vitestManager.vitestRestartPromise; @@ -122,12 +122,9 @@ export class TestManager { await this.vitestManager.runTests(payload); - if (this.coverage && !allTestsRun) { + if (temporarilyDisableCoverage) { // Re-enable coverage if it was temporarily disabled because of a subset of stories was run - await this.vitestManager.restartVitest({ - watchMode: this.watchMode, - coverage: this.coverage, - }); + await this.vitestManager.restartVitest({ coverage: this.coverage }); } } catch (e) { this.reportFatalError('Failed to run tests', e); diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 78c929732d7..7267f3dcea3 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -40,7 +40,7 @@ export class VitestManager { constructor(private testManager: TestManager) {} - async startVitest({ watchMode = false, coverage = false } = {}) { + async startVitest({ coverage = false } = {}) { const { createVitest } = await import('vitest/node'); const storybookCoverageReporter: [string, StorybookCoverageReporterOptions] = [ @@ -55,7 +55,7 @@ export class VitestManager { ? { enabled: true, clean: false, - cleanOnRerun: !watchMode, + cleanOnRerun: false, reportOnFailure: true, reporter: [['html', {}], storybookCoverageReporter], reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), @@ -66,9 +66,8 @@ export class VitestManager { this.vitest = await createVitest( 'test', { - watch: watchMode, + watch: true, passWithNoTests: false, - changed: watchMode, // TODO: // Do we want to enable Vite's default reporter? // The output in the terminal might be too spamy and it might be better to @@ -110,18 +109,16 @@ export class VitestManager { this.testManager.reportFatalError('Failed to init Vitest', e); } - if (watchMode) { - await this.setupWatchers(); - } + await this.setupWatchers(); } - async restartVitest({ watchMode, coverage }: { watchMode: boolean; coverage: boolean }) { + async restartVitest({ coverage }: { coverage: boolean }) { await this.vitestRestartPromise; this.vitestRestartPromise = new Promise(async (resolve, reject) => { try { await this.vitest?.runningPromise; await this.closeVitest(); - await this.startVitest({ watchMode, coverage }); + await this.startVitest({ coverage }); resolve(); } catch (e) { reject(e); @@ -324,6 +321,11 @@ export class VitestManager { this.updateLastChanged(id); this.storyCountForCurrentRun = 0; + // when watch mode is disabled, don't trigger any tests (below) + // but still invalidate the cache for the changed file, which is handled above + if (!this.testManager.watchMode) { + return; + } await this.runAffectedTests(file); } diff --git a/test-storybooks/portable-stories-kitchen-sink/react/yarn.lock b/test-storybooks/portable-stories-kitchen-sink/react/yarn.lock index b6be5646963..e3484dc0172 100644 --- a/test-storybooks/portable-stories-kitchen-sink/react/yarn.lock +++ b/test-storybooks/portable-stories-kitchen-sink/react/yarn.lock @@ -1841,7 +1841,7 @@ __metadata: "@storybook/experimental-addon-test@file:../../../code/addons/test::locator=portable-stories-react%40workspace%3A.": version: 8.5.0-alpha.18 - resolution: "@storybook/experimental-addon-test@file:../../../code/addons/test#../../../code/addons/test::hash=f1c849&locator=portable-stories-react%40workspace%3A." + resolution: "@storybook/experimental-addon-test@file:../../../code/addons/test#../../../code/addons/test::hash=21369f&locator=portable-stories-react%40workspace%3A." dependencies: "@storybook/csf": "npm:0.1.12" "@storybook/global": "npm:^5.0.0" @@ -1864,7 +1864,7 @@ __metadata: optional: true vitest: optional: true - checksum: 10/29f2db97577ff0a3a79ae37a8b0eaebd86d7eca43c3ad364abbb5f4a63e5f9d12dab2927723802752a2c2e2d36561ac89db32a7fd770e31c5b6e573e265bdd27 + checksum: 10/2081814e214dc1dd31144870a6a4ea7637c9c241ab02044488be57e19402c206c0037d449197f77bb4262147703f6d0b27f09c9f6cc2ee358c97fd7d1cdfa908 languageName: node linkType: hard