From 68c406223b3a1d028176fea200ef084fd114eb13 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 20 Nov 2024 22:01:22 +0100 Subject: [PATCH 01/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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 c327a491d3f020c5978069f504be19840e8e29d3 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 29 Nov 2024 11:57:29 +0100 Subject: [PATCH 27/38] Build: Fix portable stories tests by moving from jsdom to happy-dom --- .../nextjs/jest.config.js | 2 +- .../nextjs/package.json | 4 ++-- .../portable-stories.test.tsx.snap | 24 +++++++++---------- .../svelte/package.json | 4 ++-- .../svelte/vite.config.ts | 5 ++-- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/test-storybooks/portable-stories-kitchen-sink/nextjs/jest.config.js b/test-storybooks/portable-stories-kitchen-sink/nextjs/jest.config.js index 3867a50e7bc7..f427b4763dd6 100644 --- a/test-storybooks/portable-stories-kitchen-sink/nextjs/jest.config.js +++ b/test-storybooks/portable-stories-kitchen-sink/nextjs/jest.config.js @@ -9,7 +9,7 @@ const createJestConfig = nextJest({ /** @type {import('jest').Config} */ const customJestConfig = { coverageProvider: 'v8', - testEnvironment: 'jsdom', + testEnvironment: '@happy-dom/jest-environment', // Add more setup options before each test is run setupFilesAfterEnv: ['./jest.setup.ts'], moduleNameMapper: { diff --git a/test-storybooks/portable-stories-kitchen-sink/nextjs/package.json b/test-storybooks/portable-stories-kitchen-sink/nextjs/package.json index 9e9b8e0285f8..69722cabd5db 100644 --- a/test-storybooks/portable-stories-kitchen-sink/nextjs/package.json +++ b/test-storybooks/portable-stories-kitchen-sink/nextjs/package.json @@ -84,6 +84,7 @@ "react-dom": "^18.2.0" }, "devDependencies": { + "@happy-dom/jest-environment": "^15.11.7", "@jest/globals": "^29.7.0", "@storybook/addon-actions": "^8.0.0", "@storybook/addon-essentials": "^8.0.0", @@ -101,11 +102,10 @@ "eslint": "^8.56.0", "eslint-plugin-storybook": "^0.6.15", "jest": "^29.7.0", - "jest-environment-jsdom": "^29.7.0", "storybook": "^8.0.0", "typescript": "^5.2.2" }, "maintainer_please_read_this": { "_": "we use file protocol to make this setup close to real life scenarios as well as avoid issues with duplicated React instances. When you recompile the SB packages, you need to rerun install." } -} \ No newline at end of file +} diff --git a/test-storybooks/portable-stories-kitchen-sink/nextjs/stories/__snapshots__/portable-stories.test.tsx.snap b/test-storybooks/portable-stories-kitchen-sink/nextjs/stories/__snapshots__/portable-stories.test.tsx.snap index e65b31476609..677e011bb38c 100644 --- a/test-storybooks/portable-stories-kitchen-sink/nextjs/stories/__snapshots__/portable-stories.test.tsx.snap +++ b/test-storybooks/portable-stories-kitchen-sink/nextjs/stories/__snapshots__/portable-stories.test.tsx.snap @@ -158,16 +158,16 @@ exports[`renders imageLegacyStories stories renders BlurredAbsolutePlaceholder 1 Global Decorator