From 68c406223b3a1d028176fea200ef084fd114eb13 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 20 Nov 2024 22:01:22 +0100 Subject: [PATCH 01/28] add minimal coverage reporting --- code/addons/test/package.json | 10 +++- code/addons/test/src/manager.tsx | 49 +++++++++++++++++-- .../addons/test/src/node/coverage-reporter.ts | 21 ++++++++ code/addons/test/src/node/vitest-manager.ts | 14 +++++- code/yarn.lock | 18 +++++++ 5 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 code/addons/test/src/node/coverage-reporter.ts diff --git a/code/addons/test/package.json b/code/addons/test/package.json index 3d8bbaaa0903..c1f17f258309 100644 --- a/code/addons/test/package.json +++ b/code/addons/test/package.json @@ -48,6 +48,11 @@ "import": "./dist/vitest-plugin/test-utils.mjs", "require": "./dist/vitest-plugin/test-utils.js" }, + "./internal/coverage-reporter": { + "types": "./dist/node/coverage-reporter.d.ts", + "import": "./dist/node/coverage-reporter.mjs", + "require": "./dist/node/coverage-reporter.js" + }, "./preview": { "types": "./dist/preview.d.ts", "import": "./dist/preview.mjs", @@ -88,6 +93,7 @@ "devDependencies": { "@devtools-ds/object-inspector": "^1.1.2", "@storybook/icons": "^1.2.12", + "@types/istanbul-lib-report": "^3.0.3", "@types/node": "^22.0.0", "@types/semver": "^7", "@vitest/browser": "^2.1.3", @@ -98,6 +104,7 @@ "execa": "^8.0.1", "find-up": "^7.0.0", "formik": "^2.2.9", + "istanbul-lib-report": "^3.0.1", "picocolors": "^1.1.0", "react": "^18.2.0", "react-dom": "^18.2.0", @@ -145,7 +152,8 @@ "./src/vitest-plugin/index.ts", "./src/vitest-plugin/global-setup.ts", "./src/postinstall.ts", - "./src/node/vitest.ts" + "./src/node/vitest.ts", + "./src/node/coverage-reporter.ts" ] }, "storybook": { diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index 9b560cdd8cee..8bbbdfc775ad 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useSyncExternalStore } from 'react'; import { AddonPanel, Button, Link as LinkComponent } from 'storybook/internal/components'; import { TESTING_MODULE_RUN_ALL_REQUEST } from 'storybook/internal/core-events'; @@ -14,6 +14,8 @@ import { import { EyeIcon, PlayHollowIcon, StopAltHollowIcon } from '@storybook/icons'; +import type { ReportNode } from 'istanbul-lib-report'; + import { ContextMenuItem } from './components/ContextMenuItem'; import { GlobalErrorModal } from './components/GlobalErrorModal'; import { Panel } from './components/Panel'; @@ -77,6 +79,38 @@ addons.register(ADDON_ID, (api) => { api.togglePanel(true); }; + type CoverageSummaryData = ReturnType['data']; + const coverageStore = { + data: undefined as CoverageSummaryData | undefined, + subscribe: () => { + const listener = (data: CoverageSummaryData) => { + console.log('LOG: got coverage data on channel', data); + coverageStore.data = data; + }; + const channel = api.getChannel(); + channel.on('storybook/coverage', listener); + return () => channel.off('storybook/coverage', listener); + }, + getSnapshot: () => coverageStore.data, + }; + const useCoverage = () => { + const coverageSummaryData = useSyncExternalStore( + coverageStore.subscribe, + coverageStore.getSnapshot + ); + console.log('LOG: coverageSummaryData', coverageSummaryData); + if (!coverageSummaryData) { + return; + } + + let total = 0; + let covered = 0; + for (const metric of Object.values(coverageSummaryData)) { + total += metric.total; + covered += metric.covered; + } + return covered / total; + }; addons.add(TEST_PROVIDER_ID, { type: Addon_TypesEnum.experimental_TEST_PROVIDER, runnable: true, @@ -97,6 +131,8 @@ addons.register(ADDON_ID, (api) => { render: (state) => { const [isModalOpen, setIsModalOpen] = useState(false); + const coverage = useCoverage(); + const title = state.crashed || state.failed ? 'Component tests failed' : 'Component tests'; const errorMessage = state.error?.message; let description: string | React.ReactNode = 'Not run'; @@ -122,10 +158,13 @@ addons.register(ADDON_ID, (api) => { ); } else if (state.progress?.finishedAt) { description = ( - + <> + + {coverage ? ` (${Math.round(coverage * 100)}% coverage)` : ''} + ); } else if (state.watching) { description = 'Watching for file changes'; diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts new file mode 100644 index 000000000000..a4b70b947db3 --- /dev/null +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -0,0 +1,21 @@ +import type { Channel } from 'storybook/internal/channels'; + +import type { ReportNode, Visitor } from 'istanbul-lib-report'; +import { ReportBase } from 'istanbul-lib-report'; + +export default class StorybookCoverageReporter extends ReportBase implements Partial { + #channel: Channel; + + constructor(opts: { channel: Channel }) { + super(); + this.#channel = opts.channel; + } + + onSummary(node: ReportNode) { + if (!node.isRoot()) { + return; + } + const coverageSummary = node.getCoverageSummary(false); + this.#channel.emit('storybook/coverage', coverageSummary); + } +} diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index f69f065aa42a..eedb5ba20aaa 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -35,9 +35,19 @@ export class VitestManager { // find a way to just show errors and warnings for example // Otherwise it might be hard for the user to discover Storybook related logs reporters: ['default', new StorybookReporter(this.testManager)], - // @ts-expect-error we just want to disable coverage, not specify a provider coverage: { - enabled: false, + provider: 'v8', + reporter: [ + // 'html', + [ + // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? + '@storybook/experimental-addon-test/internal/coverage-reporter', + { channel: this.channel }, + ], + ], + reportOnFailure: true, + enabled: true, + cleanOnRerun: false, }, }); diff --git a/code/yarn.lock b/code/yarn.lock index fcf45a9df6f6..6de7c7ca57f3 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -6602,6 +6602,7 @@ __metadata: "@storybook/instrumenter": "workspace:*" "@storybook/test": "workspace:*" "@storybook/theming": "workspace:*" + "@types/istanbul-lib-report": "npm:^3.0.3" "@types/node": "npm:^22.0.0" "@types/semver": "npm:^7" "@vitest/browser": "npm:^2.1.3" @@ -6612,6 +6613,7 @@ __metadata: execa: "npm:^8.0.1" find-up: "npm:^7.0.0" formik: "npm:^2.2.9" + istanbul-lib-report: "npm:^3.0.1" picocolors: "npm:^1.1.0" polished: "npm:^4.2.2" prompts: "npm:^2.4.0" @@ -8359,6 +8361,13 @@ __metadata: languageName: node linkType: hard +"@types/istanbul-lib-coverage@npm:*": + version: 2.0.6 + resolution: "@types/istanbul-lib-coverage@npm:2.0.6" + checksum: 10c0/3948088654f3eeb45363f1db158354fb013b362dba2a5c2c18c559484d5eb9f6fd85b23d66c0a7c2fcfab7308d0a585b14dadaca6cc8bf89ebfdc7f8f5102fb7 + languageName: node + linkType: hard + "@types/istanbul-lib-coverage@npm:^2.0.1": version: 2.0.4 resolution: "@types/istanbul-lib-coverage@npm:2.0.4" @@ -8366,6 +8375,15 @@ __metadata: languageName: node linkType: hard +"@types/istanbul-lib-report@npm:^3.0.3": + version: 3.0.3 + resolution: "@types/istanbul-lib-report@npm:3.0.3" + dependencies: + "@types/istanbul-lib-coverage": "npm:*" + checksum: 10c0/247e477bbc1a77248f3c6de5dadaae85ff86ac2d76c5fc6ab1776f54512a745ff2a5f791d22b942e3990ddbd40f3ef5289317c4fca5741bedfaa4f01df89051c + languageName: node + linkType: hard + "@types/js-yaml@npm:^4.0.5": version: 4.0.9 resolution: "@types/js-yaml@npm:4.0.9" From 1e0869b6bae746320a134826378c68dfedeedce2 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 21 Nov 2024 15:53:55 +0100 Subject: [PATCH 02/28] fix allTestsRun --- code/addons/test/src/node/vitest-manager.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index eedb5ba20aaa..eb957f2749b6 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -38,16 +38,13 @@ export class VitestManager { coverage: { provider: 'v8', reporter: [ - // 'html', + 'html', [ // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? '@storybook/experimental-addon-test/internal/coverage-reporter', { channel: this.channel }, ], ], - reportOnFailure: true, - enabled: true, - cleanOnRerun: false, }, }); @@ -142,7 +139,14 @@ export class VitestManager { this.vitest!.configOverride.testNamePattern = testNamePattern; } - await this.vitest!.runFiles(testList, true); + // console.log('LOG: conf', this.vitest.config.coverage); + // console.log('LOG: confOver', this.vitest.configOverride.coverage); + // this.vitest.configOverride.coverage = { + // enabled: Math.random() > 0.5, + // }; + + await this.vitest!.runFiles(testList, false); + this.resetTestNamePattern(); } @@ -226,7 +230,7 @@ export class VitestManager { if (triggerAffectedTests.length) { await this.vitest.cancelCurrentRun('keyboard-input'); await this.vitest.runningPromise; - await this.vitest.runFiles(triggerAffectedTests, true); + await this.vitest.runFiles(triggerAffectedTests, false); } } From 5aff4bea9d6881ce56ccc784a748a27569f6ba7f Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 21 Nov 2024 15:55:55 +0100 Subject: [PATCH 03/28] static link to coverage --- code/addons/test/src/manager.tsx | 24 ++++++++++++++----- .../addons/test/src/node/coverage-reporter.ts | 6 ++++- code/addons/test/src/preset.ts | 12 +++++++++- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index 8bbbdfc775ad..b7a72e979ad9 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -79,11 +79,13 @@ addons.register(ADDON_ID, (api) => { api.togglePanel(true); }; - type CoverageSummaryData = ReturnType['data']; + type CoverageSummary = { + coverageSummary: ReturnType['data']; + }; const coverageStore = { - data: undefined as CoverageSummaryData | undefined, + data: undefined as CoverageSummary | undefined, subscribe: () => { - const listener = (data: CoverageSummaryData) => { + const listener = (data: CoverageSummary) => { console.log('LOG: got coverage data on channel', data); coverageStore.data = data; }; @@ -105,11 +107,13 @@ addons.register(ADDON_ID, (api) => { let total = 0; let covered = 0; - for (const metric of Object.values(coverageSummaryData)) { + for (const metric of Object.values(coverageSummaryData.coverageSummary)) { total += metric.total; covered += metric.covered; } - return covered / total; + return { + percentage: covered / total, + }; }; addons.add(TEST_PROVIDER_ID, { type: Addon_TypesEnum.experimental_TEST_PROVIDER, @@ -163,7 +167,15 @@ addons.register(ADDON_ID, (api) => { timestamp={new Date(state.progress.finishedAt)} testCount={state.progress.numTotalTests} /> - {coverage ? ` (${Math.round(coverage * 100)}% coverage)` : ''} + {coverage ? ( + {`(${Math.round(coverage.percentage * 100)}% coverage)`} + ) : ( + '' + )} ); } else if (state.watching) { diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index a4b70b947db3..0f0027307943 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -1,3 +1,5 @@ +import { join } from 'node:path'; + import type { Channel } from 'storybook/internal/channels'; import type { ReportNode, Visitor } from 'istanbul-lib-report'; @@ -16,6 +18,8 @@ export default class StorybookCoverageReporter extends ReportBase implements Par return; } const coverageSummary = node.getCoverageSummary(false); - this.#channel.emit('storybook/coverage', coverageSummary); + this.#channel.emit('storybook/coverage', { + coverageSummary, + }); } } diff --git a/code/addons/test/src/preset.ts b/code/addons/test/src/preset.ts index 685a56e2baf4..96164dcfe303 100644 --- a/code/addons/test/src/preset.ts +++ b/code/addons/test/src/preset.ts @@ -9,7 +9,7 @@ import { TESTING_MODULE_WATCH_MODE_REQUEST, } from 'storybook/internal/core-events'; import { oneWayHash, telemetry } from 'storybook/internal/telemetry'; -import type { Options, PresetProperty, StoryId } from 'storybook/internal/types'; +import type { Options, PresetProperty, PresetPropertyFn, StoryId } from 'storybook/internal/types'; import picocolors from 'picocolors'; import { dedent } from 'ts-dedent'; @@ -128,3 +128,13 @@ export const managerEntries: PresetProperty<'managerEntries'> = async (entry = [ // for whatever reason seems like the return type of managerEntries is not correct (it expects never instead of string[]) return entry as never; }; + +export const staticDirs: PresetPropertyFn<'staticDirs'> = async (values = []) => { + return [ + { + from: join(process.cwd(), 'coverage'), + to: '/coverage', + }, + ...values, + ]; +}; From 50c9292341966f6585ab0ece32b04d2e58895fb8 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 26 Nov 2024 11:22:08 +0100 Subject: [PATCH 04/28] use configOverrides for coverage config --- code/addons/test/src/node/vitest-manager.ts | 31 +++++++++++++-------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index eb957f2749b6..0ea3779b495a 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -35,17 +35,6 @@ export class VitestManager { // find a way to just show errors and warnings for example // Otherwise it might be hard for the user to discover Storybook related logs reporters: ['default', new StorybookReporter(this.testManager)], - coverage: { - provider: 'v8', - reporter: [ - 'html', - [ - // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? - '@storybook/experimental-addon-test/internal/coverage-reporter', - { channel: this.channel }, - ], - ], - }, }); if (this.vitest) { @@ -59,6 +48,8 @@ export class VitestManager { if (watchMode) { await this.setupWatchers(); } + + this.setCoverageConfig(); } async runAllTests() { @@ -269,6 +260,24 @@ export class VitestManager { } } + setCoverageConfig() { + if (!this.vitest) { + return; + } + this.vitest.configOverride.coverage = { + // TODO: merge with existing coverage config? (if it exists) + reporter: [ + //@ts-expect-error wrong upstream types + 'html', + [ + // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? + '@storybook/experimental-addon-test/internal/coverage-reporter', + { channel: this.channel }, + ], + ], + }; + } + isStorybookProject(project: TestProject | WorkspaceProject) { // eslint-disable-next-line no-underscore-dangle return !!project.config.env?.__STORYBOOK_URL__; From dcd84836a9740544d150fc3bc57efffaeb2ff1c0 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 26 Nov 2024 13:40:56 +0100 Subject: [PATCH 05/28] use cache dir for coverage staticDir --- code/addons/test/src/constants.ts | 2 ++ code/addons/test/src/manager.tsx | 24 +++---------------- .../addons/test/src/node/coverage-reporter.ts | 2 -- code/addons/test/src/node/vitest-manager.ts | 3 +++ code/addons/test/src/preset.ts | 20 ++++++++++++---- 5 files changed, 24 insertions(+), 27 deletions(-) diff --git a/code/addons/test/src/constants.ts b/code/addons/test/src/constants.ts index 838594e212a3..bd0637e98700 100644 --- a/code/addons/test/src/constants.ts +++ b/code/addons/test/src/constants.ts @@ -10,6 +10,8 @@ export const DOCUMENTATION_LINK = 'writing-tests/test-addon'; export const DOCUMENTATION_DISCREPANCY_LINK = `${DOCUMENTATION_LINK}#what-happens-when-there-are-different-test-results-in-multiple-environments`; export const DOCUMENTATION_FATAL_ERROR_LINK = `${DOCUMENTATION_LINK}#what-happens-if-vitest-itself-has-an-error`; +export const COVERAGE_DIRECTORY = 'coverage'; + export interface Config { coverage: boolean; a11y: boolean; diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index cedee3695167..16968b9dcd6a 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useSyncExternalStore } from 'react'; import { AddonPanel } from 'storybook/internal/components'; import type { Combo } from 'storybook/internal/manager-api'; @@ -10,6 +10,8 @@ import { Addon_TypesEnum, } from 'storybook/internal/types'; +import type { ReportNode } from 'istanbul-lib-report'; + import { ContextMenuItem } from './components/ContextMenuItem'; import { Panel } from './components/Panel'; import { PanelTitle } from './components/PanelTitle'; @@ -46,26 +48,6 @@ addons.register(ADDON_ID, (api) => { }, getSnapshot: () => coverageStore.data, }; - const useCoverage = () => { - const coverageSummaryData = useSyncExternalStore( - coverageStore.subscribe, - coverageStore.getSnapshot - ); - console.log('LOG: coverageSummaryData', coverageSummaryData); - if (!coverageSummaryData) { - return; - } - - let total = 0; - let covered = 0; - for (const metric of Object.values(coverageSummaryData.coverageSummary)) { - total += metric.total; - covered += metric.covered; - } - return { - percentage: covered / total, - }; - }; addons.add(TEST_PROVIDER_ID, { type: Addon_TypesEnum.experimental_TEST_PROVIDER, runnable: true, diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index 0f0027307943..e240eb6889c2 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -1,5 +1,3 @@ -import { join } from 'node:path'; - import type { Channel } from 'storybook/internal/channels'; import type { ReportNode, Visitor } from 'istanbul-lib-report'; diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 57cd12ec3e15..6f5ae58437e6 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -3,6 +3,7 @@ import { existsSync } from 'node:fs'; import type { TestProject, TestSpecification, Vitest, WorkspaceProject } from 'vitest/node'; import type { Channel } from 'storybook/internal/channels'; +import { resolvePathInStorybookCache } from 'storybook/internal/common'; import type { TestingModuleRunRequestPayload } from 'storybook/internal/core-events'; import type { DocsIndexEntry, StoryIndex, StoryIndexEntry } from '@storybook/types'; @@ -10,6 +11,7 @@ import type { DocsIndexEntry, StoryIndex, StoryIndexEntry } from '@storybook/typ import path, { normalize } from 'pathe'; import slash from 'slash'; +import { COVERAGE_DIRECTORY } from '../constants'; import { log } from '../logger'; import { StorybookReporter } from './reporter'; import type { TestManager } from './test-manager'; @@ -295,6 +297,7 @@ export class VitestManager { { channel: this.channel }, ], ], + reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), }; } diff --git a/code/addons/test/src/preset.ts b/code/addons/test/src/preset.ts index 17fef0364769..15b8238c8241 100644 --- a/code/addons/test/src/preset.ts +++ b/code/addons/test/src/preset.ts @@ -1,7 +1,13 @@ import { readFileSync } from 'node:fs'; +import { mkdir } from 'node:fs/promises'; import type { Channel } from 'storybook/internal/channels'; -import { checkAddonOrder, getFrameworkName, serverRequire } from 'storybook/internal/common'; +import { + checkAddonOrder, + getFrameworkName, + resolvePathInStorybookCache, + serverRequire, +} from 'storybook/internal/common'; import { TESTING_MODULE_RUN_REQUEST, TESTING_MODULE_WATCH_MODE_REQUEST, @@ -13,7 +19,7 @@ import { isAbsolute, join } from 'pathe'; import picocolors from 'picocolors'; import { dedent } from 'ts-dedent'; -import { STORYBOOK_ADDON_TEST_CHANNEL } from './constants'; +import { COVERAGE_DIRECTORY, STORYBOOK_ADDON_TEST_CHANNEL } from './constants'; import { log } from './logger'; import { runTestRunner } from './node/boot-test-runner'; @@ -127,10 +133,16 @@ export const managerEntries: PresetProperty<'managerEntries'> = async (entry = [ return entry as never; }; -export const staticDirs: PresetPropertyFn<'staticDirs'> = async (values = []) => { +export const staticDirs: PresetPropertyFn<'staticDirs'> = async (values = [], options) => { + if (options.configType === 'PRODUCTION') { + return values; + } + + const coverageDirectory = resolvePathInStorybookCache(COVERAGE_DIRECTORY); + await mkdir(coverageDirectory, { recursive: true }); return [ { - from: join(process.cwd(), 'coverage'), + from: coverageDirectory, to: '/coverage', }, ...values, From 7a2bf5adb93de2b867c382cfbe777d0ae3f23f15 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Tue, 26 Nov 2024 14:25:55 +0100 Subject: [PATCH 06/28] share testManager with the coverage reporter --- code/addons/test/src/node/coverage-reporter.ts | 18 +++++++++++------- code/addons/test/src/node/vitest-manager.ts | 2 +- .../src/core-events/data/testing-module.ts | 4 ++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index e240eb6889c2..b35239adda94 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -1,14 +1,15 @@ -import type { Channel } from 'storybook/internal/channels'; - import type { ReportNode, Visitor } from 'istanbul-lib-report'; import { ReportBase } from 'istanbul-lib-report'; +import { TEST_PROVIDER_ID } from '../constants'; +import type { TestManager } from './test-manager'; + export default class StorybookCoverageReporter extends ReportBase implements Partial { - #channel: Channel; + #testManager: TestManager; - constructor(opts: { channel: Channel }) { + constructor(opts: { getTestManager: () => TestManager }) { super(); - this.#channel = opts.channel; + this.#testManager = opts.getTestManager(); } onSummary(node: ReportNode) { @@ -16,8 +17,11 @@ export default class StorybookCoverageReporter extends ReportBase implements Par return; } const coverageSummary = node.getCoverageSummary(false); - this.#channel.emit('storybook/coverage', { - coverageSummary, + this.#testManager.sendProgressReport({ + providerId: TEST_PROVIDER_ID, + details: { + coverageSummary: coverageSummary.data, + }, }); } } diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 6f5ae58437e6..e470e9f2a607 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -294,7 +294,7 @@ export class VitestManager { [ // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? '@storybook/experimental-addon-test/internal/coverage-reporter', - { channel: this.channel }, + { getTestManager: () => this.testManager }, ], ], reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), diff --git a/code/core/src/core-events/data/testing-module.ts b/code/core/src/core-events/data/testing-module.ts index 80edea66aa64..b02a07d966c9 100644 --- a/code/core/src/core-events/data/testing-module.ts +++ b/code/core/src/core-events/data/testing-module.ts @@ -37,6 +37,10 @@ export type TestingModuleProgressReportPayload = message: string; stack?: string; }; + } + | { + providerId: TestProviderId; + details: { [key: string]: any }; }; export type TestingModuleCrashReportPayload = { From 19ec3f63270917a3752c99dddd25e2bf6006034b Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Tue, 26 Nov 2024 15:49:11 +0100 Subject: [PATCH 07/28] trying to set coverage override vitest config --- .../addons/test/src/node/coverage-reporter.ts | 5 ++ code/addons/test/src/node/vitest-manager.ts | 61 ++++++++++++------- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index b35239adda94..9c23d8c852b6 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -10,9 +10,12 @@ export default class StorybookCoverageReporter extends ReportBase implements Par constructor(opts: { getTestManager: () => TestManager }) { super(); this.#testManager = opts.getTestManager(); + + console.log('StorybookCoverageReporter created'); } onSummary(node: ReportNode) { + console.log(node.isRoot(), node.getRelativeName()); if (!node.isRoot()) { return; } @@ -23,5 +26,7 @@ export default class StorybookCoverageReporter extends ReportBase implements Par coverageSummary: coverageSummary.data, }, }); + + console.log('StorybookCoverageReporter onSummary', Object.keys(coverageSummary)); } } diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index e470e9f2a607..8329b091e70f 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -37,6 +37,8 @@ export class VitestManager { async startVitest(watchMode = false) { const { createVitest } = await import('vitest/node'); + console.log('STARTING VITEST WITH COVERAGE REPORTING'); + this.vitest = await createVitest('test', { watch: watchMode, passWithNoTests: false, @@ -47,6 +49,22 @@ export class VitestManager { // find a way to just show errors and warnings for example // Otherwise it might be hard for the user to discover Storybook related logs reporters: ['default', new StorybookReporter(this.testManager)], + // @ts-expect-error (no provider needed) + coverage: { + enabled: true, // @JReinhold we want to disable this, but then it doesn't work + cleanOnRerun: !watchMode, + reportOnFailure: true, + // TODO: merge with existing coverage config? (if it exists) + reporter: [ + ['html', {}], + [ + // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? + '@storybook/experimental-addon-test/internal/coverage-reporter', + { getTestManager: () => this.testManager }, + ], + ], + reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), + }, }); if (this.vitest) { @@ -60,8 +78,6 @@ export class VitestManager { if (watchMode) { await this.setupWatchers(); } - - this.setCoverageConfig(); } private updateLastChanged(filepath: string) { @@ -158,6 +174,28 @@ export class VitestManager { ); } + // TODO based on the event payload config / state + if (true) { + console.log('SETTING UP COVERAGE REPORTING'); + this.vitest.config.coverage.enabled = true; + // // @ts-expect-error (no provider needed) + // this.vitest.configOverride.coverage = { + // enabled: true, + // cleanOnRerun: true, + // reportOnFailure: true, + // // TODO: merge with existing coverage config? (if it exists) + // reporter: [ + // ['html', {}], + // [ + // // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? + // '@storybook/experimental-addon-test/internal/coverage-reporter', + // { getTestManager: () => this.testManager }, + // ], + // ], + // reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), + // }; + } + await this.vitest!.runFiles(filteredTestFiles, true); this.resetTestNamePattern(); } @@ -282,25 +320,6 @@ export class VitestManager { } } - setCoverageConfig() { - if (!this.vitest) { - return; - } - this.vitest.configOverride.coverage = { - // TODO: merge with existing coverage config? (if it exists) - reporter: [ - //@ts-expect-error wrong upstream types - 'html', - [ - // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? - '@storybook/experimental-addon-test/internal/coverage-reporter', - { getTestManager: () => this.testManager }, - ], - ], - reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), - }; - } - isStorybookProject(project: TestProject | WorkspaceProject) { // eslint-disable-next-line no-underscore-dangle return !!project.config.env?.__STORYBOOK_URL__; From 426ecaf8e7e8cad3d2895a35dac0f35c0b452ea2 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Tue, 26 Nov 2024 16:02:15 +0100 Subject: [PATCH 08/28] This seems like the right thing to do to me @JReinhold --- .../manager-api/modules/experimental_testmodule.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/code/core/src/manager-api/modules/experimental_testmodule.ts b/code/core/src/manager-api/modules/experimental_testmodule.ts index a6a0eff1d376..e96680a28f23 100644 --- a/code/core/src/manager-api/modules/experimental_testmodule.ts +++ b/code/core/src/manager-api/modules/experimental_testmodule.ts @@ -57,7 +57,19 @@ export const init: ModuleFn = ({ store, fullAPI }) => { updateTestProviderState(id, update) { return store.setState( ({ testProviders }) => { - return { testProviders: { ...testProviders, [id]: { ...testProviders[id], ...update } } }; + return { + testProviders: { + ...testProviders, + [id]: { + ...testProviders[id], + ...update, + details: { + ...(testProviders[id].details || {}), + ...(update.details || {}), + }, + }, + }, + }; }, { persistence: 'session' } ); From f51b127bd201139aad69d6693b4cbae8105f714d Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Tue, 26 Nov 2024 17:05:19 +0100 Subject: [PATCH 09/28] add to UI; I'm sure either Jeppe or Gert will change --- code/addons/test/src/components/TestProviderRender.tsx | 6 ++++++ code/addons/test/src/constants.ts | 3 +++ 2 files changed, 9 insertions(+) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 9e7534472774..2a1f66ef56ed 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -116,6 +116,12 @@ export const TestProviderRender: FC<{ + {state.details?.coverageSummary ? ( +
{state.details?.coverageSummary?.lines.pct || 0}% coverage
+ ) : ( +
nothing
+ )} + {!isEditing ? ( {Object.entries(config).map(([key, value]) => ( diff --git a/code/addons/test/src/constants.ts b/code/addons/test/src/constants.ts index bd0637e98700..db08e9e8699b 100644 --- a/code/addons/test/src/constants.ts +++ b/code/addons/test/src/constants.ts @@ -1,3 +1,5 @@ +import type { CoverageSummaryData } from 'istanbul-lib-coverage'; + import type { TestResult } from './node/reporter'; export const ADDON_ID = 'storybook/test'; @@ -19,4 +21,5 @@ export interface Config { export type Details = { testResults: TestResult[]; + coverageSummary: CoverageSummaryData; }; From 7a1e4b92274872c158b1164d31fdc7a388189121 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 26 Nov 2024 22:56:30 +0100 Subject: [PATCH 10/28] restart vitest on coverage change --- code/addons/test/src/node/boot-test-runner.ts | 2 +- .../addons/test/src/node/coverage-reporter.ts | 9 +---- code/addons/test/src/node/test-manager.ts | 39 +++++++++++++++---- code/addons/test/src/node/vitest-manager.ts | 37 +++--------------- code/addons/test/src/node/vitest.ts | 26 ++++++++++++- 5 files changed, 64 insertions(+), 49 deletions(-) diff --git a/code/addons/test/src/node/boot-test-runner.ts b/code/addons/test/src/node/boot-test-runner.ts index 0771604861d9..2f6ae8d70e7f 100644 --- a/code/addons/test/src/node/boot-test-runner.ts +++ b/code/addons/test/src/node/boot-test-runner.ts @@ -63,7 +63,7 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a const startChildProcess = () => new Promise((resolve, reject) => { - child = execaNode(vitestModulePath); + child = execaNode(vitestModulePath, undefined, { stdio: 'inherit' }); stderr = []; child.stdout?.on('data', log); diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index 9c23d8c852b6..ffe962ff0ae0 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -7,15 +7,12 @@ import type { TestManager } from './test-manager'; export default class StorybookCoverageReporter extends ReportBase implements Partial { #testManager: TestManager; - constructor(opts: { getTestManager: () => TestManager }) { + constructor(opts: { testManager: TestManager }) { super(); - this.#testManager = opts.getTestManager(); - - console.log('StorybookCoverageReporter created'); + this.#testManager = opts.testManager; } onSummary(node: ReportNode) { - console.log(node.isRoot(), node.getRelativeName()); if (!node.isRoot()) { return; } @@ -26,7 +23,5 @@ export default class StorybookCoverageReporter extends ReportBase implements Par coverageSummary: coverageSummary.data, }, }); - - console.log('StorybookCoverageReporter onSummary', Object.keys(coverageSummary)); } } diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index 624916772056..369f1859d58f 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -20,6 +20,8 @@ export class TestManager { watchMode = false; + coverage = false; + constructor( private channel: Channel, private options: { @@ -27,7 +29,7 @@ export class TestManager { onReady?: () => void; } = {} ) { - this.vitestManager = new VitestManager(channel, this); + this.vitestManager = new VitestManager(this); this.channel.on(TESTING_MODULE_RUN_REQUEST, this.handleRunRequest.bind(this)); this.channel.on(TESTING_MODULE_CONFIG_CHANGE, this.handleConfigChange.bind(this)); @@ -37,21 +39,31 @@ export class TestManager { this.vitestManager.startVitest().then(() => options.onReady?.()); } - async restartVitest(watchMode = false) { + async restartVitest({ watchMode, coverage }: { watchMode: boolean; coverage: boolean }) { await this.vitestManager.vitest?.runningPromise; await this.vitestManager.closeVitest(); - await this.vitestManager.startVitest(watchMode); + await this.vitestManager.startVitest({ watchMode, coverage }); } async handleConfigChange(payload: TestingModuleConfigChangePayload) { - // TODO do something with the config const config = payload.config; + console.log('LOG: handleConfigChange', config); + + try { + if (payload.providerId !== TEST_PROVIDER_ID) { + return; + } + //@ts-expect-error - TODO: fix types, should allow a generic Config type + if (this.coverage !== payload.config.coverage) { + this.coverage = payload.config.coverage; + await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage }); + } + } catch (e) { + this.reportFatalError('Failed to change coverage mode', e); + } } async handleWatchModeRequest(payload: TestingModuleWatchModeRequestPayload) { - // TODO do something with the config - const config = payload.config; - try { if (payload.providerId !== TEST_PROVIDER_ID) { return; @@ -59,7 +71,7 @@ export class TestManager { if (this.watchMode !== payload.watchMode) { this.watchMode = payload.watchMode; - await this.restartVitest(this.watchMode); + await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage }); } } catch (e) { this.reportFatalError('Failed to change watch mode', e); @@ -71,8 +83,19 @@ export class TestManager { if (payload.providerId !== TEST_PROVIDER_ID) { return; } + // If we have coverage enabled and we're running a subset of stories, we need 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: this.watchMode, coverage: false }); + } await this.vitestManager.runTests(payload); + + if (temporarilyDisableCoverage) { + await this.restartVitest({ watchMode: this.watchMode, 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 8329b091e70f..20a249406c9d 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -29,16 +29,11 @@ export class VitestManager { storyCountForCurrentRun: number = 0; - constructor( - private channel: Channel, - private testManager: TestManager - ) {} + constructor(private testManager: TestManager) {} - async startVitest(watchMode = false) { + async startVitest({ watchMode = false, coverage = false } = {}) { const { createVitest } = await import('vitest/node'); - console.log('STARTING VITEST WITH COVERAGE REPORTING'); - this.vitest = await createVitest('test', { watch: watchMode, passWithNoTests: false, @@ -51,16 +46,16 @@ export class VitestManager { reporters: ['default', new StorybookReporter(this.testManager)], // @ts-expect-error (no provider needed) coverage: { - enabled: true, // @JReinhold we want to disable this, but then it doesn't work + enabled: coverage, cleanOnRerun: !watchMode, reportOnFailure: true, // TODO: merge with existing coverage config? (if it exists) reporter: [ - ['html', {}], + 'html', [ // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? '@storybook/experimental-addon-test/internal/coverage-reporter', - { getTestManager: () => this.testManager }, + { testManager: this.testManager }, ], ], reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), @@ -174,28 +169,6 @@ export class VitestManager { ); } - // TODO based on the event payload config / state - if (true) { - console.log('SETTING UP COVERAGE REPORTING'); - this.vitest.config.coverage.enabled = true; - // // @ts-expect-error (no provider needed) - // this.vitest.configOverride.coverage = { - // enabled: true, - // cleanOnRerun: true, - // reportOnFailure: true, - // // TODO: merge with existing coverage config? (if it exists) - // reporter: [ - // ['html', {}], - // [ - // // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? - // '@storybook/experimental-addon-test/internal/coverage-reporter', - // { getTestManager: () => this.testManager }, - // ], - // ], - // reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), - // }; - } - await this.vitest!.runFiles(filteredTestFiles, true); this.resetTestNamePattern(); } diff --git a/code/addons/test/src/node/vitest.ts b/code/addons/test/src/node/vitest.ts index 5ba46113b7a9..7a962bc39b84 100644 --- a/code/addons/test/src/node/vitest.ts +++ b/code/addons/test/src/node/vitest.ts @@ -2,6 +2,7 @@ import process from 'node:process'; import { Channel } from 'storybook/internal/channels'; +import { TEST_PROVIDER_ID } from '../constants'; import { TestManager } from './test-manager'; process.env.TEST = 'true'; @@ -20,7 +21,7 @@ const channel: Channel = new Channel({ }, }); -new TestManager(channel, { +const testManager = new TestManager(channel, { onError: (message, error) => { process.send?.({ type: 'error', message, error: error.stack ?? error }); }, @@ -29,6 +30,29 @@ new TestManager(channel, { }, }); +// Enable raw mode to get keystrokes +process.stdin.setRawMode(true); +process.stdin.resume(); +process.stdin.setEncoding('utf8'); + +// Listen for keyboard input +process.stdin.on('data', (key) => { + // Press 'C' to trigger + if (key === 'c' || key === 'C') { + console.log('C was pressed, enabling coverage!'); + testManager.handleConfigChange({ config: { coverage: true }, providerId: TEST_PROVIDER_ID }); + } + if (key === 'v' || key === 'V') { + console.log('V was pressed, disabling coverage!'); + testManager.handleConfigChange({ config: { coverage: false }, providerId: TEST_PROVIDER_ID }); + } + + // Press ctrl+c to exit + if (key === '\u0003') { + process.exit(); + } +}); + const exit = (code = 0) => { channel?.removeAllListeners(); process.exit(code); From 39fdd979500024801d0e53988e0c47153ecce231 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 27 Nov 2024 14:06:41 +0100 Subject: [PATCH 11/28] only set coverage options when coveage is enabled --- code/addons/test/src/node/test-manager.ts | 1 - code/addons/test/src/node/vitest-manager.ts | 44 ++++++++++++--------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index 369f1859d58f..6cb3b70fef08 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -47,7 +47,6 @@ export class TestManager { async handleConfigChange(payload: TestingModuleConfigChangePayload) { const config = payload.config; - console.log('LOG: handleConfigChange', config); try { if (payload.providerId !== TEST_PROVIDER_ID) { diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 20a249406c9d..3bdc95061518 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -1,6 +1,12 @@ import { existsSync } from 'node:fs'; -import type { TestProject, TestSpecification, Vitest, WorkspaceProject } from 'vitest/node'; +import type { + CoverageOptions, + TestProject, + TestSpecification, + Vitest, + WorkspaceProject, +} from 'vitest/node'; import type { Channel } from 'storybook/internal/channels'; import { resolvePathInStorybookCache } from 'storybook/internal/common'; @@ -34,6 +40,23 @@ export class VitestManager { async startVitest({ watchMode = false, coverage = false } = {}) { const { createVitest } = await import('vitest/node'); + const coverageOptions: CoverageOptions = coverage + ? ({ + enabled: true, + clean: false, + cleanOnRerun: false, + reportOnFailure: true, + reporter: [ + ['html', {}], + [ + '@storybook/experimental-addon-test/internal/coverage-reporter', + { testManager: this.testManager }, + ], + ], + reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), + } as CoverageOptions) + : ({ enabled: false } as CoverageOptions); + this.vitest = await createVitest('test', { watch: watchMode, passWithNoTests: false, @@ -44,27 +67,12 @@ export class VitestManager { // find a way to just show errors and warnings for example // Otherwise it might be hard for the user to discover Storybook related logs reporters: ['default', new StorybookReporter(this.testManager)], - // @ts-expect-error (no provider needed) - coverage: { - enabled: coverage, - cleanOnRerun: !watchMode, - reportOnFailure: true, - // TODO: merge with existing coverage config? (if it exists) - reporter: [ - 'html', - [ - // TODO: use require.resolve here instead? (or import.meta.resolve) how does this behave in monorepos? - '@storybook/experimental-addon-test/internal/coverage-reporter', - { testManager: this.testManager }, - ], - ], - reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), - }, + coverage: coverageOptions, }); if (this.vitest) { this.vitest.onCancel(() => { - // TODO: handle cancelation + // TODO: handle cancellation }); } From c5c3cf67d2dc05afd17300967e54a1cd0d2c39cc Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 27 Nov 2024 14:07:44 +0100 Subject: [PATCH 12/28] cleanup --- code/addons/test/src/node/vitest-manager.ts | 32 +++++++++++---------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 3bdc95061518..68141a15daa7 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -40,22 +40,24 @@ export class VitestManager { async startVitest({ watchMode = false, coverage = false } = {}) { const { createVitest } = await import('vitest/node'); - const coverageOptions: CoverageOptions = coverage - ? ({ - enabled: true, - clean: false, - cleanOnRerun: false, - reportOnFailure: true, - reporter: [ - ['html', {}], - [ - '@storybook/experimental-addon-test/internal/coverage-reporter', - { testManager: this.testManager }, + const coverageOptions = ( + coverage + ? { + enabled: true, + clean: false, + cleanOnRerun: false, + reportOnFailure: true, + reporter: [ + ['html', {}], + [ + '@storybook/experimental-addon-test/internal/coverage-reporter', + { testManager: this.testManager }, + ], ], - ], - reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), - } as CoverageOptions) - : ({ enabled: false } as CoverageOptions); + reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), + } + : { enabled: false } + ) as CoverageOptions; this.vitest = await createVitest('test', { watch: watchMode, From f0fdfb1642a91c2ca9d1fae106ab629740dc97af Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 27 Nov 2024 14:16:53 +0100 Subject: [PATCH 13/28] fix config types --- code/addons/test/src/node/test-manager.ts | 5 +++-- code/core/src/core-events/data/testing-module.ts | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index 6cb3b70fef08..8aa9f1243dc7 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -45,14 +45,15 @@ export class TestManager { await this.vitestManager.startVitest({ watchMode, coverage }); } - async handleConfigChange(payload: TestingModuleConfigChangePayload) { + async handleConfigChange( + payload: TestingModuleConfigChangePayload + ) { const config = payload.config; try { if (payload.providerId !== TEST_PROVIDER_ID) { return; } - //@ts-expect-error - TODO: fix types, should allow a generic Config type if (this.coverage !== payload.config.coverage) { this.coverage = payload.config.coverage; await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage }); diff --git a/code/core/src/core-events/data/testing-module.ts b/code/core/src/core-events/data/testing-module.ts index b02a07d966c9..488d66673dd9 100644 --- a/code/core/src/core-events/data/testing-module.ts +++ b/code/core/src/core-events/data/testing-module.ts @@ -81,7 +81,10 @@ export type TestingModuleWatchModeRequestPayload = { config?: TestProviderState['config']; }; -export type TestingModuleConfigChangePayload = { +export type TestingModuleConfigChangePayload< + Details extends { [key: string]: any } = NonNullable, + Config extends { [key: string]: any } = NonNullable, +> = { providerId: TestProviderId; - config: TestProviderState['config']; + config: TestProviderState['config']; }; From 7b001def82450228717ed760b7506687fcac93a5 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 10:28:57 +0100 Subject: [PATCH 14/28] integrate coverage backend with UI --- .../src/components/TestProviderRender.tsx | 26 ++++++++++++++----- code/addons/test/src/constants.ts | 5 +++- code/addons/test/src/node/boot-test-runner.ts | 5 ++++ .../addons/test/src/node/coverage-reporter.ts | 13 +++++++++- code/addons/test/src/node/test-manager.ts | 2 -- 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 588523f89ad9..20869fc6e38e 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -77,6 +77,7 @@ export const TestProviderRender: FC<{ const title = state.crashed || state.failed ? 'Local tests failed' : 'Run local tests'; const errorMessage = state.error?.message; + const coverage = state.details?.coverage; const [config, updateConfig] = useConfig( api, @@ -159,7 +160,6 @@ export const TestProviderRender: FC<{ right={ updateConfig({ coverage: !config.coverage })} /> @@ -185,11 +185,24 @@ export const TestProviderRender: FC<{ title="Component tests" icon={} /> - } - right={`60%`} - /> + {coverage ? ( + + } + right={`${coverage.percentage}%`} + /> + ) : ( + } + /> + )} } @@ -221,6 +234,7 @@ function useConfig(api: API, providerId: string, initialConfig: Config) { debounce((config: Config) => { if (!isEqual(config, lastConfig.current)) { api.updateTestProviderState(providerId, { config }); + console.log('LOG: saveConfig', { providerId, config }); api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); lastConfig.current = config; } diff --git a/code/addons/test/src/constants.ts b/code/addons/test/src/constants.ts index db08e9e8699b..0ee35f70dd8e 100644 --- a/code/addons/test/src/constants.ts +++ b/code/addons/test/src/constants.ts @@ -21,5 +21,8 @@ export interface Config { export type Details = { testResults: TestResult[]; - coverageSummary: CoverageSummaryData; + coverage?: { + status: 'positive' | 'warning' | 'negative' | 'unknown'; + percentage: number; + }; }; diff --git a/code/addons/test/src/node/boot-test-runner.ts b/code/addons/test/src/node/boot-test-runner.ts index 2f6ae8d70e7f..6acc53d19182 100644 --- a/code/addons/test/src/node/boot-test-runner.ts +++ b/code/addons/test/src/node/boot-test-runner.ts @@ -3,6 +3,7 @@ import { type ChildProcess } from 'node:child_process'; import type { Channel } from 'storybook/internal/channels'; import { TESTING_MODULE_CANCEL_TEST_RUN_REQUEST, + TESTING_MODULE_CONFIG_CHANGE, TESTING_MODULE_CRASH_REPORT, TESTING_MODULE_RUN_REQUEST, TESTING_MODULE_WATCH_MODE_REQUEST, @@ -43,11 +44,14 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a child?.send({ args, from: 'server', type: TESTING_MODULE_WATCH_MODE_REQUEST }); const forwardCancel = (...args: any[]) => child?.send({ args, from: 'server', type: TESTING_MODULE_CANCEL_TEST_RUN_REQUEST }); + const forwardConfigChange = (...args: any[]) => + child?.send({ args, from: 'server', type: TESTING_MODULE_CONFIG_CHANGE }); const killChild = () => { channel.off(TESTING_MODULE_RUN_REQUEST, forwardRun); channel.off(TESTING_MODULE_WATCH_MODE_REQUEST, forwardWatchMode); channel.off(TESTING_MODULE_CANCEL_TEST_RUN_REQUEST, forwardCancel); + channel.off(TESTING_MODULE_CONFIG_CHANGE, forwardConfigChange); child?.kill(); child = null; }; @@ -86,6 +90,7 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a channel.on(TESTING_MODULE_RUN_REQUEST, forwardRun); channel.on(TESTING_MODULE_WATCH_MODE_REQUEST, forwardWatchMode); channel.on(TESTING_MODULE_CANCEL_TEST_RUN_REQUEST, forwardCancel); + channel.on(TESTING_MODULE_CONFIG_CHANGE, forwardConfigChange); resolve(); } else if (result.type === 'error') { diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index ffe962ff0ae0..9458e872e3f4 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -17,10 +17,21 @@ export default class StorybookCoverageReporter extends ReportBase implements Par return; } const coverageSummary = node.getCoverageSummary(false); + let total = 0; + let covered = 0; + + for (const metric of Object.values(coverageSummary.data)) { + total += metric.total; + covered += metric.covered; + } + this.#testManager.sendProgressReport({ providerId: TEST_PROVIDER_ID, details: { - coverageSummary: coverageSummary.data, + coverage: { + status: 'warning', // TODO: determine status based on thresholds/watermarks + percentage: Math.round((covered / total) * 100), + }, }, }); } diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index 8aa9f1243dc7..ce5f368569ce 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -48,8 +48,6 @@ export class TestManager { async handleConfigChange( payload: TestingModuleConfigChangePayload ) { - const config = payload.config; - try { if (payload.providerId !== TEST_PROVIDER_ID) { return; From 2cf1d42013a9fffff7e3063eae29ac284e863c2b Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 13:48:15 +0100 Subject: [PATCH 15/28] configure coverage color based on watermark config --- .../addons/test/src/node/coverage-reporter.ts | 36 +++++++++++++++---- code/addons/test/src/node/vitest-manager.ts | 20 ++++++----- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index 9458e872e3f4..24294e4e667e 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -1,15 +1,25 @@ +import type { ResolvedCoverageOptions } from 'vitest/node'; + import type { ReportNode, Visitor } from 'istanbul-lib-report'; import { ReportBase } from 'istanbul-lib-report'; -import { TEST_PROVIDER_ID } from '../constants'; +import { type Details, TEST_PROVIDER_ID } from '../constants'; import type { TestManager } from './test-manager'; +export type StorybookCoverageReporterOptions = { + testManager: TestManager; + getCoverageOptions: () => ResolvedCoverageOptions<'v8'>; +}; + export default class StorybookCoverageReporter extends ReportBase implements Partial { - #testManager: TestManager; + #testManager: StorybookCoverageReporterOptions['testManager']; + + #getCoverageOptions: StorybookCoverageReporterOptions['getCoverageOptions']; - constructor(opts: { testManager: TestManager }) { + constructor(opts: StorybookCoverageReporterOptions) { super(); this.#testManager = opts.testManager; + this.#getCoverageOptions = opts.getCoverageOptions; } onSummary(node: ReportNode) { @@ -25,13 +35,25 @@ export default class StorybookCoverageReporter extends ReportBase implements Par covered += metric.covered; } + const percentage = Math.round((covered / total) * 100); + + // Fallback to Vitest's default watermarks https://vitest.dev/config/#coverage-watermarks + const [lowWatermark = 50, highWatermark = 80] = + this.#getCoverageOptions().watermarks?.statements ?? []; + + const coverageDetails: Details['coverage'] = { + percentage, + status: + percentage < lowWatermark + ? 'negative' + : percentage < highWatermark + ? 'warning' + : 'positive', + }; this.#testManager.sendProgressReport({ providerId: TEST_PROVIDER_ID, details: { - coverage: { - status: 'warning', // TODO: determine status based on thresholds/watermarks - percentage: Math.round((covered / total) * 100), - }, + coverage: coverageDetails, }, }); } diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 68141a15daa7..42a15a45c022 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -2,13 +2,13 @@ import { existsSync } from 'node:fs'; import type { CoverageOptions, + ResolvedCoverageOptions, TestProject, TestSpecification, Vitest, WorkspaceProject, } from 'vitest/node'; -import type { Channel } from 'storybook/internal/channels'; import { resolvePathInStorybookCache } from 'storybook/internal/common'; import type { TestingModuleRunRequestPayload } from 'storybook/internal/core-events'; @@ -19,6 +19,7 @@ import slash from 'slash'; import { COVERAGE_DIRECTORY } from '../constants'; import { log } from '../logger'; +import type { StorybookCoverageReporterOptions } from './coverage-reporter'; import { StorybookReporter } from './reporter'; import type { TestManager } from './test-manager'; @@ -40,20 +41,21 @@ export class VitestManager { async startVitest({ watchMode = false, coverage = false } = {}) { const { createVitest } = await import('vitest/node'); + const storybookCoverageReporter: [string, StorybookCoverageReporterOptions] = [ + '@storybook/experimental-addon-test/internal/coverage-reporter', + { + testManager: this.testManager, + getCoverageOptions: () => this.vitest?.config?.coverage as ResolvedCoverageOptions<'v8'>, + }, + ]; const coverageOptions = ( coverage ? { enabled: true, clean: false, - cleanOnRerun: false, + cleanOnRerun: !watchMode, reportOnFailure: true, - reporter: [ - ['html', {}], - [ - '@storybook/experimental-addon-test/internal/coverage-reporter', - { testManager: this.testManager }, - ], - ], + reporter: [['html', {}], storybookCoverageReporter], reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY), } : { enabled: false } From 69884ada5e3f3dfe2c8d2e21ec84a48bf6bb4ac6 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 15:19:03 +0100 Subject: [PATCH 16/28] cleanup --- code/addons/test/src/node/vitest.ts | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/code/addons/test/src/node/vitest.ts b/code/addons/test/src/node/vitest.ts index 7a962bc39b84..753f7f06b9dc 100644 --- a/code/addons/test/src/node/vitest.ts +++ b/code/addons/test/src/node/vitest.ts @@ -21,7 +21,7 @@ const channel: Channel = new Channel({ }, }); -const testManager = new TestManager(channel, { +new TestManager(channel, { onError: (message, error) => { process.send?.({ type: 'error', message, error: error.stack ?? error }); }, @@ -35,24 +35,6 @@ process.stdin.setRawMode(true); process.stdin.resume(); process.stdin.setEncoding('utf8'); -// Listen for keyboard input -process.stdin.on('data', (key) => { - // Press 'C' to trigger - if (key === 'c' || key === 'C') { - console.log('C was pressed, enabling coverage!'); - testManager.handleConfigChange({ config: { coverage: true }, providerId: TEST_PROVIDER_ID }); - } - if (key === 'v' || key === 'V') { - console.log('V was pressed, disabling coverage!'); - testManager.handleConfigChange({ config: { coverage: false }, providerId: TEST_PROVIDER_ID }); - } - - // Press ctrl+c to exit - if (key === '\u0003') { - process.exit(); - } -}); - const exit = (code = 0) => { channel?.removeAllListeners(); process.exit(code); From 172f64c80251962a31c7b6b51ce68d43c1ee723b Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 15:19:23 +0100 Subject: [PATCH 17/28] use coverage.statement as metric --- code/addons/test/src/node/coverage-reporter.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index 24294e4e667e..922b364f3fd5 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -27,15 +27,8 @@ export default class StorybookCoverageReporter extends ReportBase implements Par return; } const coverageSummary = node.getCoverageSummary(false); - let total = 0; - let covered = 0; - for (const metric of Object.values(coverageSummary.data)) { - total += metric.total; - covered += metric.covered; - } - - const percentage = Math.round((covered / total) * 100); + const percentage = Math.round(coverageSummary.data.statements.pct); // Fallback to Vitest's default watermarks https://vitest.dev/config/#coverage-watermarks const [lowWatermark = 50, highWatermark = 80] = From 13f59b2bcbfb1414e0585c54ee3a1daf638f74c2 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 15:19:52 +0100 Subject: [PATCH 18/28] only boot vitest child process on addon-test-specific events --- code/addons/test/src/preset.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/code/addons/test/src/preset.ts b/code/addons/test/src/preset.ts index 15b8238c8241..30b0e9da7b3d 100644 --- a/code/addons/test/src/preset.ts +++ b/code/addons/test/src/preset.ts @@ -9,6 +9,7 @@ import { serverRequire, } from 'storybook/internal/common'; import { + TESTING_MODULE_CONFIG_CHANGE, TESTING_MODULE_RUN_REQUEST, TESTING_MODULE_WATCH_MODE_REQUEST, } from 'storybook/internal/core-events'; @@ -19,7 +20,7 @@ import { isAbsolute, join } from 'pathe'; import picocolors from 'picocolors'; import { dedent } from 'ts-dedent'; -import { COVERAGE_DIRECTORY, STORYBOOK_ADDON_TEST_CHANNEL } from './constants'; +import { COVERAGE_DIRECTORY, STORYBOOK_ADDON_TEST_CHANNEL, TEST_PROVIDER_ID } from './constants'; import { log } from './logger'; import { runTestRunner } from './node/boot-test-runner'; @@ -70,7 +71,9 @@ export const experimental_serverChannel = async (channel: Channel, options: Opti const execute = (eventName: string) => (...args: any[]) => { - runTestRunner(channel, eventName, args); + if (args[0]?.providerId === TEST_PROVIDER_ID) { + runTestRunner(channel, eventName, args); + } }; channel.on(TESTING_MODULE_RUN_REQUEST, execute(TESTING_MODULE_RUN_REQUEST)); @@ -79,6 +82,7 @@ export const experimental_serverChannel = async (channel: Channel, options: Opti execute(TESTING_MODULE_WATCH_MODE_REQUEST)(payload); } }); + channel.on(TESTING_MODULE_CONFIG_CHANGE, execute(TESTING_MODULE_CONFIG_CHANGE)); if (!core.disableTelemetry) { const packageJsonPath = require.resolve('@storybook/experimental-addon-test/package.json'); From d8948f018416a01772811471aec9b097c87bebfa Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 22:04:41 +0100 Subject: [PATCH 19/28] restart vitest before every coverage run --- code/addons/test/src/node/boot-test-runner.ts | 2 +- code/addons/test/src/node/test-manager.ts | 4 ++-- code/addons/test/src/node/vitest-manager.ts | 13 ++++++++++--- code/addons/test/src/node/vitest.ts | 5 ----- code/core/src/core-events/data/testing-module.ts | 6 ++++-- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/code/addons/test/src/node/boot-test-runner.ts b/code/addons/test/src/node/boot-test-runner.ts index 6acc53d19182..9cf4880ec35e 100644 --- a/code/addons/test/src/node/boot-test-runner.ts +++ b/code/addons/test/src/node/boot-test-runner.ts @@ -67,7 +67,7 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a const startChildProcess = () => new Promise((resolve, reject) => { - child = execaNode(vitestModulePath, undefined, { stdio: 'inherit' }); + child = execaNode(vitestModulePath); stderr = []; child.stdout?.on('data', log); diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index ce5f368569ce..bb258301cdaa 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -12,7 +12,7 @@ import { type TestingModuleWatchModeRequestPayload, } from 'storybook/internal/core-events'; -import { TEST_PROVIDER_ID } from '../constants'; +import { type Config, TEST_PROVIDER_ID } from '../constants'; import { VitestManager } from './vitest-manager'; export class TestManager { @@ -76,7 +76,7 @@ export class TestManager { } } - async handleRunRequest(payload: TestingModuleRunRequestPayload) { + async handleRunRequest(payload: TestingModuleRunRequestPayload) { try { if (payload.providerId !== TEST_PROVIDER_ID) { return; diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 42a15a45c022..39c4fe59aa97 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -17,7 +17,7 @@ import type { DocsIndexEntry, StoryIndex, StoryIndexEntry } from '@storybook/typ import path, { normalize } from 'pathe'; import slash from 'slash'; -import { COVERAGE_DIRECTORY } from '../constants'; +import { COVERAGE_DIRECTORY, type Config } from '../constants'; import { log } from '../logger'; import type { StorybookCoverageReporterOptions } from './coverage-reporter'; import { StorybookReporter } from './reporter'; @@ -133,10 +133,17 @@ export class VitestManager { return true; } - async runTests(requestPayload: TestingModuleRunRequestPayload) { - if (!this.vitest) { + async runTests(requestPayload: TestingModuleRunRequestPayload) { + if (requestPayload.config?.coverage && (requestPayload.storyIds ?? []).length === 0) { + // For some reason we need to restart Vitest between every coverage run, + // otherwise the coverage is not updated correctly + await this.vitest?.runningPromise; + await this.closeVitest(); + await this.startVitest({ watchMode: false, coverage: true }); + } else if (!this.vitest) { await this.startVitest(); } + this.resetTestNamePattern(); const stories = await this.fetchStories(requestPayload.indexUrl, requestPayload.storyIds); diff --git a/code/addons/test/src/node/vitest.ts b/code/addons/test/src/node/vitest.ts index 753f7f06b9dc..83f7b06816ab 100644 --- a/code/addons/test/src/node/vitest.ts +++ b/code/addons/test/src/node/vitest.ts @@ -30,11 +30,6 @@ new TestManager(channel, { }, }); -// Enable raw mode to get keystrokes -process.stdin.setRawMode(true); -process.stdin.resume(); -process.stdin.setEncoding('utf8'); - const exit = (code = 0) => { channel?.removeAllListeners(); process.exit(code); diff --git a/code/core/src/core-events/data/testing-module.ts b/code/core/src/core-events/data/testing-module.ts index 488d66673dd9..184139626e7f 100644 --- a/code/core/src/core-events/data/testing-module.ts +++ b/code/core/src/core-events/data/testing-module.ts @@ -11,12 +11,14 @@ export type TestProviderState< export type TestProviders = Record; -export type TestingModuleRunRequestPayload = { +export type TestingModuleRunRequestPayload< + Config extends { [key: string]: any } = NonNullable, +> = { providerId: TestProviderId; // TODO: Avoid needing to do a fetch request server-side to retrieve the index indexUrl: string; // e.g. http://localhost:6006/index.json storyIds?: string[]; // ['button--primary', 'button--secondary'] - config?: TestProviderState['config']; + config?: TestProviderState, Config>['config']; }; export type TestingModuleProgressReportPayload = From 1563d50e07747d1ebbe27e51e457f2a6717425c4 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 22:53:32 +0100 Subject: [PATCH 20/28] cleanup --- .../src/components/TestProviderRender.tsx | 1 - code/addons/test/src/constants.ts | 2 -- code/addons/test/src/manager.tsx | 20 +---------- code/addons/test/src/node/test-manager.ts | 34 ++++++++++++------- code/addons/test/src/node/vitest-manager.ts | 8 +---- 5 files changed, 23 insertions(+), 42 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 20869fc6e38e..3bdbfb642360 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -234,7 +234,6 @@ function useConfig(api: API, providerId: string, initialConfig: Config) { debounce((config: Config) => { if (!isEqual(config, lastConfig.current)) { api.updateTestProviderState(providerId, { config }); - console.log('LOG: saveConfig', { providerId, config }); api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); lastConfig.current = config; } diff --git a/code/addons/test/src/constants.ts b/code/addons/test/src/constants.ts index 0ee35f70dd8e..08431f1e7923 100644 --- a/code/addons/test/src/constants.ts +++ b/code/addons/test/src/constants.ts @@ -1,5 +1,3 @@ -import type { CoverageSummaryData } from 'istanbul-lib-coverage'; - import type { TestResult } from './node/reporter'; export const ADDON_ID = 'storybook/test'; diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index 1b10c1ef9a25..b0b2eeef2402 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -1,4 +1,4 @@ -import React, { useSyncExternalStore } from 'react'; +import React from 'react'; import { AddonPanel } from 'storybook/internal/components'; import type { Combo } from 'storybook/internal/manager-api'; @@ -10,8 +10,6 @@ import { Addon_TypesEnum, } from 'storybook/internal/types'; -import type { ReportNode } from 'istanbul-lib-report'; - import { ContextMenuItem } from './components/ContextMenuItem'; import { Panel } from './components/Panel'; import { PanelTitle } from './components/PanelTitle'; @@ -32,22 +30,6 @@ addons.register(ADDON_ID, (api) => { api.togglePanel(true); }; - type CoverageSummary = { - coverageSummary: ReturnType['data']; - }; - const coverageStore = { - data: undefined as CoverageSummary | undefined, - subscribe: () => { - const listener = (data: CoverageSummary) => { - console.log('LOG: got coverage data on channel', data); - coverageStore.data = data; - }; - const channel = api.getChannel(); - channel.on('storybook/coverage', listener); - return () => channel.off('storybook/coverage', listener); - }, - getSnapshot: () => coverageStore.data, - }; addons.add(TEST_PROVIDER_ID, { type: Addon_TypesEnum.experimental_TEST_PROVIDER, runnable: true, diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index bb258301cdaa..5e2eae310f31 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -48,16 +48,16 @@ export class TestManager { async handleConfigChange( payload: TestingModuleConfigChangePayload ) { - try { - if (payload.providerId !== TEST_PROVIDER_ID) { - return; - } - if (this.coverage !== payload.config.coverage) { + if (payload.providerId !== TEST_PROVIDER_ID) { + return; + } + if (this.coverage !== payload.config.coverage) { + try { this.coverage = payload.config.coverage; await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage }); + } catch (e) { + this.reportFatalError('Failed to change coverage mode', e); } - } catch (e) { - this.reportFatalError('Failed to change coverage mode', e); } } @@ -81,17 +81,25 @@ export class TestManager { if (payload.providerId !== TEST_PROVIDER_ID) { return; } - // If we have coverage enabled and we're running a subset of stories, we need 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: this.watchMode, coverage: false }); + 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. + + // 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. + await this.restartVitest({ + watchMode: allTestsRun ? false : this.watchMode, + coverage: allTestsRun, + }); } await this.vitestManager.runTests(payload); - if (temporarilyDisableCoverage) { + if (this.coverage && !allTestsRun) { + // 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 }); } } catch (e) { diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 39c4fe59aa97..5d3a66fad412 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -134,13 +134,7 @@ export class VitestManager { } async runTests(requestPayload: TestingModuleRunRequestPayload) { - if (requestPayload.config?.coverage && (requestPayload.storyIds ?? []).length === 0) { - // For some reason we need to restart Vitest between every coverage run, - // otherwise the coverage is not updated correctly - await this.vitest?.runningPromise; - await this.closeVitest(); - await this.startVitest({ watchMode: false, coverage: true }); - } else if (!this.vitest) { + if (!this.vitest) { await this.startVitest(); } From 979f974bd22500756e92eb4d593ee73149e84334 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 23:07:48 +0100 Subject: [PATCH 21/28] fix types --- .../src/core-server/presets/common-preset.ts | 23 ++++++++++--------- .../components/sidebar/SidebarBottom.tsx | 6 +++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/code/core/src/core-server/presets/common-preset.ts b/code/core/src/core-server/presets/common-preset.ts index 7bd893d0daf1..fe5f7db4f53a 100644 --- a/code/core/src/core-server/presets/common-preset.ts +++ b/code/core/src/core-server/presets/common-preset.ts @@ -303,26 +303,27 @@ export const experimental_serverChannel = async ( channel.on( TESTING_MODULE_PROGRESS_REPORT, async (payload: TestingModuleProgressReportPayload) => { - if ( - (payload.status === 'success' || payload.status === 'cancelled') && - payload.progress?.finishedAt - ) { + const status = 'status' in payload ? payload.status : undefined; + const progress = 'progress' in payload ? payload.progress : undefined; + const error = 'error' in payload ? payload.error : undefined; + + if ((status === 'success' || status === 'cancelled') && progress?.finishedAt) { await telemetry('testing-module-completed-report', { provider: payload.providerId, - duration: payload.progress.finishedAt - payload.progress.startedAt, - numTotalTests: payload.progress.numTotalTests, - numFailedTests: payload.progress.numFailedTests, - numPassedTests: payload.progress.numPassedTests, - status: payload.status, + duration: progress?.finishedAt - progress?.startedAt, + numTotalTests: progress?.numTotalTests, + numFailedTests: progress?.numFailedTests, + numPassedTests: progress?.numPassedTests, + status, }); } - if (payload.status === 'failed') { + if (status === 'failed') { await telemetry('testing-module-completed-report', { provider: payload.providerId, status: 'failed', ...(options.enableCrashReports && { - error: sanitizeError(payload.error), + error: error && sanitizeError(error), }), }); } diff --git a/code/core/src/manager/components/sidebar/SidebarBottom.tsx b/code/core/src/manager/components/sidebar/SidebarBottom.tsx index ba64b4e500a5..e39f7595188d 100644 --- a/code/core/src/manager/components/sidebar/SidebarBottom.tsx +++ b/code/core/src/manager/components/sidebar/SidebarBottom.tsx @@ -130,10 +130,12 @@ export const SidebarBottomBase = ({ }; const onProgressReport = ({ providerId, ...result }: TestingModuleProgressReportPayload) => { - if (result.status === 'failed') { + const statusResult = 'status' in result ? result.status : undefined; + + if (statusResult === 'failed') { api.updateTestProviderState(providerId, { ...result, running: false, failed: true }); } else { - const update = { ...result, running: result.status === 'pending' }; + const update = { ...result, running: statusResult === 'pending' }; api.updateTestProviderState(providerId, update); const { mapStatusUpdate, ...state } = testProviders[providerId]; From 4a6999395e458ad5d07dac41ff358beff7592a68 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 28 Nov 2024 23:16:43 +0100 Subject: [PATCH 22/28] disable coverage in watch mode --- code/addons/test/src/components/TestProviderRender.tsx | 3 ++- code/addons/test/src/node/test-manager.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 3bdbfb642360..d1c879d60318 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -160,7 +160,8 @@ export const TestProviderRender: FC<{ right={ updateConfig({ coverage: !config.coverage })} /> } diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index 5e2eae310f31..5de508cca3cc 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -69,7 +69,7 @@ export class TestManager { if (this.watchMode !== payload.watchMode) { this.watchMode = payload.watchMode; - await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage }); + await this.restartVitest({ watchMode: this.watchMode, coverage: false }); } } catch (e) { this.reportFatalError('Failed to change watch mode', e); From 37fb3f710c1da8b6be5e996ecd59ec1ee8ecae08 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Fri, 29 Nov 2024 09:07:02 +0100 Subject: [PATCH 23/28] improve vitest restart comment --- code/addons/test/src/node/test-manager.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index 5de508cca3cc..b9f1c95f38a6 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -84,12 +84,15 @@ export class TestManager { 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. - - // 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 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. + */ await this.restartVitest({ watchMode: allTestsRun ? false : this.watchMode, coverage: allTestsRun, From 56666f0ec25529e0a5b8d97bfd7a420851a171ed Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Fri, 29 Nov 2024 09:58:43 +0100 Subject: [PATCH 24/28] add coverage html link --- code/addons/test/src/components/TestProviderRender.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index d1c879d60318..f87f260a496b 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -189,6 +189,9 @@ export const TestProviderRender: FC<{ {coverage ? ( Date: Fri, 29 Nov 2024 11:02:41 +0100 Subject: [PATCH 25/28] rename details.coverage to details.coverageSummary, to not confuse with config.coverage --- .../test/src/components/TestProviderRender.tsx | 12 ++++++------ code/addons/test/src/constants.ts | 2 +- code/addons/test/src/node/coverage-reporter.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index f87f260a496b..ca6be687c88d 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -77,7 +77,7 @@ export const TestProviderRender: FC<{ const title = state.crashed || state.failed ? 'Local tests failed' : 'Run local tests'; const errorMessage = state.error?.message; - const coverage = state.details?.coverage; + const coverageSummary = state.details?.coverageSummary; const [config, updateConfig] = useConfig( api, @@ -186,7 +186,7 @@ export const TestProviderRender: FC<{ title="Component tests" icon={} /> - {coverage ? ( + {coverageSummary ? ( } - right={`${coverage.percentage}%`} + right={`${coverageSummary.percentage}%`} /> ) : ( Date: Fri, 29 Nov 2024 11:21:18 +0100 Subject: [PATCH 26/28] simplify --- code/addons/test/package.json | 1 - code/addons/test/src/node/coverage-reporter.ts | 8 ++++---- code/addons/test/src/node/test-manager.ts | 2 +- code/addons/test/src/node/vitest-manager.ts | 2 +- code/addons/test/src/node/vitest.ts | 1 - code/core/src/core-events/data/testing-module.ts | 11 ++++++----- 6 files changed, 12 insertions(+), 13 deletions(-) diff --git a/code/addons/test/package.json b/code/addons/test/package.json index f6861121429f..4fa638c7ff8d 100644 --- a/code/addons/test/package.json +++ b/code/addons/test/package.json @@ -92,7 +92,6 @@ }, "devDependencies": { "@devtools-ds/object-inspector": "^1.1.2", - "@storybook/icons": "^1.2.12", "@types/istanbul-lib-report": "^3.0.3", "@types/node": "^22.0.0", "@types/semver": "^7", diff --git a/code/addons/test/src/node/coverage-reporter.ts b/code/addons/test/src/node/coverage-reporter.ts index bd5c8bde07aa..452643cd9d60 100644 --- a/code/addons/test/src/node/coverage-reporter.ts +++ b/code/addons/test/src/node/coverage-reporter.ts @@ -8,18 +8,18 @@ import type { TestManager } from './test-manager'; export type StorybookCoverageReporterOptions = { testManager: TestManager; - getCoverageOptions: () => ResolvedCoverageOptions<'v8'>; + coverageOptions: ResolvedCoverageOptions<'v8'>; }; export default class StorybookCoverageReporter extends ReportBase implements Partial { #testManager: StorybookCoverageReporterOptions['testManager']; - #getCoverageOptions: StorybookCoverageReporterOptions['getCoverageOptions']; + #coverageOptions: StorybookCoverageReporterOptions['coverageOptions']; constructor(opts: StorybookCoverageReporterOptions) { super(); this.#testManager = opts.testManager; - this.#getCoverageOptions = opts.getCoverageOptions; + this.#coverageOptions = opts.coverageOptions; } onSummary(node: ReportNode) { @@ -32,7 +32,7 @@ export default class StorybookCoverageReporter extends ReportBase implements Par // Fallback to Vitest's default watermarks https://vitest.dev/config/#coverage-watermarks const [lowWatermark = 50, highWatermark = 80] = - this.#getCoverageOptions().watermarks?.statements ?? []; + this.#coverageOptions.watermarks?.statements ?? []; const coverageDetails: Details['coverageSummary'] = { percentage, diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index b9f1c95f38a6..f424408b7bc3 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -46,7 +46,7 @@ export class TestManager { } async handleConfigChange( - payload: TestingModuleConfigChangePayload + payload: TestingModuleConfigChangePayload<{ coverage: boolean; a11y: boolean }> ) { if (payload.providerId !== TEST_PROVIDER_ID) { return; diff --git a/code/addons/test/src/node/vitest-manager.ts b/code/addons/test/src/node/vitest-manager.ts index 5d3a66fad412..37e6e0588aa1 100644 --- a/code/addons/test/src/node/vitest-manager.ts +++ b/code/addons/test/src/node/vitest-manager.ts @@ -45,7 +45,7 @@ export class VitestManager { '@storybook/experimental-addon-test/internal/coverage-reporter', { testManager: this.testManager, - getCoverageOptions: () => this.vitest?.config?.coverage as ResolvedCoverageOptions<'v8'>, + coverageOptions: this.vitest?.config?.coverage as ResolvedCoverageOptions<'v8'>, }, ]; const coverageOptions = ( diff --git a/code/addons/test/src/node/vitest.ts b/code/addons/test/src/node/vitest.ts index 83f7b06816ab..5ba46113b7a9 100644 --- a/code/addons/test/src/node/vitest.ts +++ b/code/addons/test/src/node/vitest.ts @@ -2,7 +2,6 @@ import process from 'node:process'; import { Channel } from 'storybook/internal/channels'; -import { TEST_PROVIDER_ID } from '../constants'; import { TestManager } from './test-manager'; process.env.TEST = 'true'; diff --git a/code/core/src/core-events/data/testing-module.ts b/code/core/src/core-events/data/testing-module.ts index 184139626e7f..ad843450723b 100644 --- a/code/core/src/core-events/data/testing-module.ts +++ b/code/core/src/core-events/data/testing-module.ts @@ -18,7 +18,7 @@ export type TestingModuleRunRequestPayload< // TODO: Avoid needing to do a fetch request server-side to retrieve the index indexUrl: string; // e.g. http://localhost:6006/index.json storyIds?: string[]; // ['button--primary', 'button--secondary'] - config?: TestProviderState, Config>['config']; + config?: Config; }; export type TestingModuleProgressReportPayload = @@ -77,16 +77,17 @@ export type TestingModuleCancelTestRunResponsePayload = message: string; }; -export type TestingModuleWatchModeRequestPayload = { +export type TestingModuleWatchModeRequestPayload< + Config extends { [key: string]: any } = NonNullable, +> = { providerId: TestProviderId; watchMode: boolean; - config?: TestProviderState['config']; + config?: Config; }; export type TestingModuleConfigChangePayload< - Details extends { [key: string]: any } = NonNullable, Config extends { [key: string]: any } = NonNullable, > = { providerId: TestProviderId; - config: TestProviderState['config']; + config: Config; }; From 04f9ca060293f762cec7cffd34680dd3ccf3a581 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Fri, 29 Nov 2024 13:37:05 +0100 Subject: [PATCH 27/28] add coverage states to stories --- .../components/TestProviderRender.stories.tsx | 82 +++++++++++++++++-- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.stories.tsx b/code/addons/test/src/components/TestProviderRender.stories.tsx index 13a9e9bd3f35..fbdb3d488c7c 100644 --- a/code/addons/test/src/components/TestProviderRender.stories.tsx +++ b/code/addons/test/src/components/TestProviderRender.stories.tsx @@ -124,29 +124,83 @@ export const Running: Story = { }, }; -export const EnableA11y: Story = { +export const Watching: Story = { + args: { + state: { + ...config, + ...baseState, + watching: true, + }, + }, +}; + +export const WithCoverageNegative: Story = { args: { state: { ...config, ...baseState, details: { testResults: [], + coverageSummary: { + percentage: 20, + status: 'negative', + }, }, config: { - a11y: true, - coverage: false, + a11y: false, + coverage: true, }, }, }, }; -export const EnableEditing: Story = { +export const WithCoverageWarning: Story = { args: { state: { ...config, ...baseState, + details: { + testResults: [], + coverageSummary: { + percentage: 50, + status: 'warning', + }, + }, config: { - a11y: true, + a11y: false, + coverage: true, + }, + }, + }, +}; + +export const WithCoveragePositive: Story = { + args: { + state: { + ...config, + ...baseState, + details: { + testResults: [], + coverageSummary: { + percentage: 80, + status: 'positive', + }, + }, + config: { + a11y: false, + coverage: true, + }, + }, + }, +}; + +export const Editing: Story = { + args: { + state: { + ...config, + ...baseState, + config: { + a11y: false, coverage: false, }, details: { @@ -161,3 +215,21 @@ export const EnableEditing: Story = { screen.getByLabelText(/Open settings/).click(); }, }; + +export const EditingAndWatching: Story = { + args: { + state: { + ...config, + ...baseState, + watching: true, + config: { + a11y: true, + coverage: true, // should be automatically disabled in the UI + }, + details: { + testResults: [], + }, + }, + }, + play: Editing.play, +}; From d4d566a58dc01f89a83fce65b34650655bd8926e Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Fri, 29 Nov 2024 13:50:35 +0100 Subject: [PATCH 28/28] sync coverage state on run request --- code/addons/test/src/node/test-manager.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts index f424408b7bc3..3660081de58c 100644 --- a/code/addons/test/src/node/test-manager.ts +++ b/code/addons/test/src/node/test-manager.ts @@ -81,6 +81,9 @@ export class TestManager { if (payload.providerId !== TEST_PROVIDER_ID) { return; } + if (payload.config && this.coverage !== payload.config.coverage) { + this.coverage = payload.config.coverage; + } const allTestsRun = (payload.storyIds ?? []).length === 0; if (this.coverage) {