From 86e4c9654c7178770320fb233396496a3f7bd786 Mon Sep 17 00:00:00 2001
From: Jeppe Reinhold <jeppe@reinhold.is>
Date: Fri, 29 Nov 2024 13:08:57 +0100
Subject: [PATCH 1/4] always run vitest in watch mode
---
code/addons/test/src/node/test-manager.ts | 50 ++++++++++-----------
code/addons/test/src/node/vitest-manager.ts | 14 +++---
2 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts
index f424408b7bc3..2be5356ef118 100644
--- a/code/addons/test/src/node/test-manager.ts
+++ b/code/addons/test/src/node/test-manager.ts
@@ -39,10 +39,10 @@ export class TestManager {
this.vitestManager.startVitest().then(() => options.onReady?.());
}
- async restartVitest({ watchMode, coverage }: { watchMode: boolean; coverage: boolean }) {
+ async restartVitest({ coverage }: { coverage: boolean }) {
await this.vitestManager.vitest?.runningPromise;
await this.vitestManager.closeVitest();
- await this.vitestManager.startVitest({ watchMode, coverage });
+ await this.vitestManager.startVitest({ coverage });
}
async handleConfigChange(
@@ -52,9 +52,9 @@ export class TestManager {
return;
}
if (this.coverage !== payload.config.coverage) {
+ this.coverage = payload.config.coverage;
try {
- this.coverage = payload.config.coverage;
- await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage });
+ await this.restartVitest({ coverage: this.coverage });
} catch (e) {
this.reportFatalError('Failed to change coverage mode', e);
}
@@ -62,14 +62,18 @@ export class TestManager {
}
async handleWatchModeRequest(payload: TestingModuleWatchModeRequestPayload) {
- try {
- if (payload.providerId !== TEST_PROVIDER_ID) {
- return;
- }
+ if (payload.providerId !== TEST_PROVIDER_ID) {
+ return;
+ }
+ this.watchMode = payload.watchMode;
- if (this.watchMode !== payload.watchMode) {
- this.watchMode = payload.watchMode;
- await this.restartVitest({ watchMode: this.watchMode, coverage: false });
+ try {
+ if (payload.watchMode && this.coverage) {
+ // if watch mode is toggled on and coverage is already enabled, restart vitest without coverage to automatically disable it
+ this.restartVitest({ coverage: false });
+ } else if (!payload.watchMode && this.coverage) {
+ // if watch mode is toggled off and coverage is already enabled, restart vitest with coverage to automatically re-enable it
+ this.restartVitest({ coverage: this.coverage });
}
} catch (e) {
this.reportFatalError('Failed to change watch mode', e);
@@ -82,28 +86,22 @@ export class TestManager {
return;
}
- const allTestsRun = (payload.storyIds ?? []).length === 0;
- 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 && (payload.storyIds ?? []).length === 0;
+ if (temporarilyDisableCoverage) {
await this.restartVitest({
- watchMode: allTestsRun ? false : this.watchMode,
- coverage: allTestsRun,
+ coverage: false,
});
}
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.restartVitest({ watchMode: this.watchMode, coverage: this.coverage });
+ await this.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 37e6e0588aa1..8d4abc854695 100644
--- a/code/addons/test/src/node/vitest-manager.ts
+++ b/code/addons/test/src/node/vitest-manager.ts
@@ -38,7 +38,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] = [
@@ -53,7 +53,7 @@ export class VitestManager {
? {
enabled: true,
clean: false,
- cleanOnRerun: !watchMode,
+ cleanOnRerun: false,
reportOnFailure: true,
reporter: [['html', {}], storybookCoverageReporter],
reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY),
@@ -62,9 +62,8 @@ export class VitestManager {
) as CoverageOptions;
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
@@ -82,9 +81,7 @@ export class VitestManager {
await this.vitest.init();
- if (watchMode) {
- await this.setupWatchers();
- }
+ await this.setupWatchers();
}
private updateLastChanged(filepath: string) {
@@ -276,6 +273,9 @@ export class VitestManager {
this.updateLastChanged(id);
this.storyCountForCurrentRun = 0;
+ if (!this.testManager.watchMode) {
+ return;
+ }
await this.runAffectedTests(file);
}
From e5fb87d51924cbc9d1b183f03304e413cdab460d Mon Sep 17 00:00:00 2001
From: Jeppe Reinhold <jeppe@reinhold.is>
Date: Mon, 2 Dec 2024 15:40:29 +0100
Subject: [PATCH 2/4] fix coverage mode toggling
---
code/addons/test/src/node/test-manager.ts | 7 ++++---
code/addons/test/src/node/vitest-manager.ts | 2 ++
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts
index a9b558c66acc..06cbe7d008f2 100644
--- a/code/addons/test/src/node/test-manager.ts
+++ b/code/addons/test/src/node/test-manager.ts
@@ -70,10 +70,10 @@ export class TestManager {
try {
if (payload.watchMode && this.coverage) {
// if watch mode is toggled on and coverage is already enabled, restart vitest without coverage to automatically disable it
- this.restartVitest({ coverage: false });
+ await this.restartVitest({ coverage: false });
} else if (!payload.watchMode && this.coverage) {
// if watch mode is toggled off and coverage is already enabled, restart vitest with coverage to automatically re-enable it
- this.restartVitest({ coverage: this.coverage });
+ await this.restartVitest({ coverage: this.coverage });
}
} catch (e) {
this.reportFatalError('Failed to change watch mode', e);
@@ -93,7 +93,8 @@ export class TestManager {
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 && (payload.storyIds ?? []).length === 0;
+ const temporarilyDisableCoverage =
+ this.coverage && !this.watchMode && (payload.storyIds ?? []).length > 0;
if (temporarilyDisableCoverage) {
await this.restartVitest({
coverage: false,
diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts
index 8d4abc854695..7404b94cd315 100644
--- a/code/addons/test/src/node/vitest-manager.ts
+++ b/code/addons/test/src/node/vitest-manager.ts
@@ -273,6 +273,8 @@ 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;
}
From 47101c6f96e289e0e7100873c702c17eb545691f Mon Sep 17 00:00:00 2001
From: Jeppe Reinhold <jeppe@reinhold.is>
Date: Fri, 6 Dec 2024 11:27:15 +0100
Subject: [PATCH 3/4] add coverage unit tests
---
.../addons/test/src/node/test-manager.test.ts | 58 ++++++++++++++++++-
1 file changed, 56 insertions(+), 2 deletions(-)
diff --git a/code/addons/test/src/node/test-manager.test.ts b/code/addons/test/src/node/test-manager.test.ts
index b674c1d786f7..db77ab2f3e2e 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);
+ });
});
From 57cb78bfee77b8b8b6c17b1cd541a760bb517653 Mon Sep 17 00:00:00 2001
From: Jeppe Reinhold <jeppe@reinhold.is>
Date: Tue, 10 Dec 2024 10:21:20 +0100
Subject: [PATCH 4/4] update yarn.lock
---
test-storybooks/portable-stories-kitchen-sink/react/yarn.lock | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/test-storybooks/portable-stories-kitchen-sink/react/yarn.lock b/test-storybooks/portable-stories-kitchen-sink/react/yarn.lock
index b6be56469636..e3484dc01721 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