From e23fef96478f2e683794cfe8b531055b6e99a02d Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Thu, 22 Aug 2024 14:35:21 +0200 Subject: [PATCH 001/111] Vite: Don't prefix story import with `@fs` --- code/builders/builder-vite/package.json | 4 +- .../src/codegen-importfn-script.test.ts | 56 +++++++++++++++++++ .../src/codegen-importfn-script.ts | 31 +++++----- code/yarn.lock | 9 +++ 4 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 code/builders/builder-vite/src/codegen-importfn-script.test.ts diff --git a/code/builders/builder-vite/package.json b/code/builders/builder-vite/package.json index 947994ae403..eb030deb396 100644 --- a/code/builders/builder-vite/package.json +++ b/code/builders/builder-vite/package.json @@ -50,14 +50,16 @@ "express": "^4.19.2", "find-cache-dir": "^3.0.0", "fs-extra": "^11.1.0", + "knitwork": "^1.1.0", "magic-string": "^0.30.0", + "pathe": "^1.1.2", + "slash": "^5.0.0", "ts-dedent": "^2.0.0" }, "devDependencies": { "@types/express": "^4.17.21", "@types/node": "^22.0.0", "glob": "^10.0.0", - "slash": "^5.0.0", "typescript": "^5.3.2", "vite": "^4.0.4" }, diff --git a/code/builders/builder-vite/src/codegen-importfn-script.test.ts b/code/builders/builder-vite/src/codegen-importfn-script.test.ts new file mode 100644 index 00000000000..4d3ce5bcb79 --- /dev/null +++ b/code/builders/builder-vite/src/codegen-importfn-script.test.ts @@ -0,0 +1,56 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { toImportFn } from './codegen-importfn-script'; + +describe('toImportFn', () => { + it('should correctly map story paths to import functions for absolute paths on Linux', async () => { + const root = '/absolute/path'; + const stories = ['/absolute/path/to/story1.js', '/absolute/path/to/story2.js']; + + const result = await toImportFn(root, stories); + + expect(result).toMatchInlineSnapshot(` + "const importers = { + "./to/story1.js": () => import("/absolute/path/to/story1.js"), + "./to/story2.js": () => import("/absolute/path/to/story2.js") + }; + + export async function importFn(path) { + return await importers[path](); + }" + `); + }); + + it('should correctly map story paths to import functions for absolute paths on Windows', async () => { + const root = 'C:\\absolute\\path'; + const stories = ['C:\\absolute\\path\\to\\story1.js', 'C:\\absolute\\path\\to\\story2.js']; + + const result = await toImportFn(root, stories); + + expect(result).toMatchInlineSnapshot(` + "const importers = { + "./to/story1.js": () => import("C:/absolute/path/to/story1.js"), + "./to/story2.js": () => import("C:/absolute/path/to/story2.js") + }; + + export async function importFn(path) { + return await importers[path](); + }" + `); + }); + + it('should handle an empty array of stories', async () => { + const root = '/absolute/path'; + const stories: string[] = []; + + const result = await toImportFn(root, stories); + + expect(result).toMatchInlineSnapshot(` + "const importers = {}; + + export async function importFn(path) { + return await importers[path](); + }" + `); + }); +}); diff --git a/code/builders/builder-vite/src/codegen-importfn-script.ts b/code/builders/builder-vite/src/codegen-importfn-script.ts index 0853b054962..92bfe81bc9e 100644 --- a/code/builders/builder-vite/src/codegen-importfn-script.ts +++ b/code/builders/builder-vite/src/codegen-importfn-script.ts @@ -1,7 +1,9 @@ -import { relative } from 'node:path'; - import type { Options } from 'storybook/internal/types'; +import { genDynamicImport, genImport, genObjectFromRawEntries } from 'knitwork'; +import { normalize, relative } from 'pathe'; +import { dedent } from 'ts-dedent'; + import { listStories } from './list-stories'; /** @@ -21,34 +23,33 @@ function toImportPath(relativePath: string) { /** * This function takes an array of stories and creates a mapping between the stories' relative paths * to the working directory and their dynamic imports. The import is done in an asynchronous - * function to delay loading. It then creates a function, `importFn(path)`, which resolves a path to - * an import function and this is called by Storybook to fetch a story dynamically when needed. + * function to delay loading and to allow Vite to split the code into smaller chunks. It then + * creates a function, `importFn(path)`, which resolves a path to an import function and this is + * called by Storybook to fetch a story dynamically when needed. * * @param stories An array of absolute story paths. */ -async function toImportFn(stories: string[]) { - const { normalizePath } = await import('vite'); +export async function toImportFn(root: string, stories: string[]) { const objectEntries = stories.map((file) => { - const relativePath = normalizePath(relative(process.cwd(), file)); + const relativePath = relative(root, file); - return ` '${toImportPath(relativePath)}': async () => import('/@fs/${file}')`; + return [toImportPath(relativePath), genDynamicImport(normalize(file))] as [string, string]; }); - return ` - const importers = { - ${objectEntries.join(',\n')} - }; + return dedent` + const importers = ${genObjectFromRawEntries(objectEntries)}; export async function importFn(path) { - return importers[path](); + return await importers[path](); } `; } -export async function generateImportFnScriptCode(options: Options) { +export async function generateImportFnScriptCode(options: Options): Promise { // First we need to get an array of stories and their absolute paths. const stories = await listStories(options); // We can then call toImportFn to create a function that can be used to load each story dynamically. - return (await toImportFn(stories)).trim(); + // eslint-disable-next-line @typescript-eslint/return-await + return await toImportFn(options.root || process.cwd(), stories); } diff --git a/code/yarn.lock b/code/yarn.lock index dea4e0ea405..d810ea58354 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -5696,7 +5696,9 @@ __metadata: find-cache-dir: "npm:^3.0.0" fs-extra: "npm:^11.1.0" glob: "npm:^10.0.0" + knitwork: "npm:^1.1.0" magic-string: "npm:^0.30.0" + pathe: "npm:^1.1.2" slash: "npm:^5.0.0" ts-dedent: "npm:^2.0.0" typescript: "npm:^5.3.2" @@ -18677,6 +18679,13 @@ __metadata: languageName: node linkType: hard +"knitwork@npm:^1.1.0": + version: 1.1.0 + resolution: "knitwork@npm:1.1.0" + checksum: 10c0/e23c679d4ded01890ab2669ccde2e85e4d7e6ba327b1395ff657f8067c7d73dc134fc8cd8188c653de4a687be7fa9c130bd36c3e2f76d8685e8b97ff8b30779c + languageName: node + linkType: hard + "language-subtag-registry@npm:^0.3.20": version: 0.3.22 resolution: "language-subtag-registry@npm:0.3.22" From c9107a1f0224e070a2865f0badc6a9c0ad95ac50 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Thu, 22 Aug 2024 15:25:15 +0200 Subject: [PATCH 002/111] add projectRoot --- code/builders/builder-vite/src/codegen-importfn-script.ts | 2 +- code/builders/builder-vite/src/vite-config.ts | 6 +++--- code/core/src/types/modules/core-common.ts | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/code/builders/builder-vite/src/codegen-importfn-script.ts b/code/builders/builder-vite/src/codegen-importfn-script.ts index 92bfe81bc9e..4b118ad9cc5 100644 --- a/code/builders/builder-vite/src/codegen-importfn-script.ts +++ b/code/builders/builder-vite/src/codegen-importfn-script.ts @@ -51,5 +51,5 @@ export async function generateImportFnScriptCode(options: Options): Promise(options); - const projectRoot = resolve(options.configDir, '..'); + options.projectRoot = options.projectRoot || resolve(options.configDir, '..'); // I destructure away the `build` property from the user's config object // I do this because I can contain config that breaks storybook, such as we had in a lit project. // If the user needs to configure the `build` they need to do so in the viteFinal function in main.js. const { config: { build: buildProperty = undefined, ...userConfig } = {} } = - (await loadConfigFromFile(configEnv, viteConfigPath, projectRoot)) ?? {}; + (await loadConfigFromFile(configEnv, viteConfigPath, options.projectRoot)) ?? {}; const sbConfig: InlineConfig = { configFile: false, cacheDir: resolvePathInStorybookCache('sb-vite', options.cacheKey), - root: projectRoot, + root: options.projectRoot, // Allow storybook deployed as subfolder. See https://github.com/storybookjs/builder-vite/issues/238 base: './', plugins: await pluginConfig(options), diff --git a/code/core/src/types/modules/core-common.ts b/code/core/src/types/modules/core-common.ts index 8e71a4cadb2..85fcd7284ff 100644 --- a/code/core/src/types/modules/core-common.ts +++ b/code/core/src/types/modules/core-common.ts @@ -194,6 +194,7 @@ export interface BuilderOptions { ignorePreview?: boolean; cache?: FileSystemCache; configDir: string; + projectRoot?: string; docsMode?: boolean; features?: StorybookConfigRaw['features']; versionCheck?: VersionCheck; From 6b90a2a8eb7bea2d1e81840fb211bd57c11bb79b Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Thu, 3 Oct 2024 19:18:55 +0200 Subject: [PATCH 003/111] docs: Add example to the Indexer API doc --- docs/api/main-config/main-config-indexers.mdx | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/api/main-config/main-config-indexers.mdx b/docs/api/main-config/main-config-indexers.mdx index 67a6cf5c45b..18a4e31aa2b 100644 --- a/docs/api/main-config/main-config-indexers.mdx +++ b/docs/api/main-config/main-config-indexers.mdx @@ -349,3 +349,27 @@ Some example usages of custom indexers include: Custom indexers can be used for an advanced purpose: defining stories in any language, including template languages, and converting the files to CSF. To see examples of this in action, you can refer to [`@storybook/addon-svelte-csf`](https://github.com/storybookjs/addon-svelte-csf) for Svelte template syntax and [`storybook-vue-addon`](https://github.com/tobiasdiez/storybook-vue-addon) for Vue template syntax. + +
+ Adding sidebar links from a collection of URLs + + The Indexer API is flexible enough to let you process arbitrary content, so long as your framework tooling is able to transform the exports in that content into actual stories it can run. In this advanced example, we have an EcmaScript module that contains URLs as named exports: + + ```ts + // src/stories/MyLinks.url.js + export default {}; + + export const DesignTokens = 'https://www.designtokens.org/'; + export const CobaltUI = 'https://cobalt-ui.pages.dev/'; + export const MiseEnMode = 'https://mode.place/'; + export const IndexerAPI = 'https://github.com/storybookjs/storybook/discussions/23176'; + ``` + + We'll write an indexer in `main.ts` that generates story entries for each URL. We'll use the export name as a story title, and we'll store the URL in the story ID so we can find it later. + + Because strings aren't valid stories, we'll complement the indexer with a Vite plugin in our `vite.config.ts` that transforms the file into actual stories. We'll use a Svelte framework, and replace each URL with a Svelte component that redirects to the previous page (so that the story loading is cancelled and so that loading the story feels like clicking an actual external link). + + In parallel to loading valid stories, we'll add a `renderLabel` function in `manager.ts` to customise the links in the sidebar, and make them actually open the URL in a new tab when clicked. The URL is fetched from the story ID. We'll add some CSS in `manager-head.html` to make those sidebar entries look unique. + + The code and live demo for this proof-of-concept are [available on StackBlitz](https://stackblitz.com/~/github.com/Sidnioulz/storybook-sidebar-urls). +
\ No newline at end of file From 1bf3b54b3442377b6f418715a944b91d42f0b4fc Mon Sep 17 00:00:00 2001 From: Valerii Sidorenko Date: Tue, 5 Nov 2024 11:03:50 +0100 Subject: [PATCH 004/111] fix(addon-toolbars): suppress deprecation warning --- code/addons/toolbars/src/components/ToolbarMenuListItem.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/code/addons/toolbars/src/components/ToolbarMenuListItem.tsx b/code/addons/toolbars/src/components/ToolbarMenuListItem.tsx index 829829de4b2..37ae26d1aeb 100644 --- a/code/addons/toolbars/src/components/ToolbarMenuListItem.tsx +++ b/code/addons/toolbars/src/components/ToolbarMenuListItem.tsx @@ -21,7 +21,9 @@ export const ToolbarMenuListItem = ({ disabled, currentValue, }: ToolbarMenuListItemProps) => { - const Icon = icon && ; + const Icon = icon && ( + + ); const Item: TooltipLinkListLink = { id: value ?? '_reset', From 0eb203740eb838b2e5ae92934201a02bf8add7b7 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 15 Nov 2024 11:15:22 +0100 Subject: [PATCH 005/111] Core: Evaluate main config when checking 'whats new' notifications --- code/core/src/core-server/utils/whats-new.ts | 23 ++++++++++---------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/code/core/src/core-server/utils/whats-new.ts b/code/core/src/core-server/utils/whats-new.ts index cb523f78318..71c71984cb1 100644 --- a/code/core/src/core-server/utils/whats-new.ts +++ b/code/core/src/core-server/utils/whats-new.ts @@ -1,7 +1,8 @@ +/* eslint-disable no-underscore-dangle */ import { writeFile } from 'node:fs/promises'; import type { Channel } from '@storybook/core/channels'; -import { findConfigFile } from '@storybook/core/common'; +import { findConfigFile, loadMainConfig } from '@storybook/core/common'; import { telemetry } from '@storybook/core/telemetry'; import type { CoreConfig, Options } from '@storybook/core/types'; @@ -58,15 +59,9 @@ export function initializeWhatsNew( throw response; })) as WhatsNewResponse; - const configFileName = findConfigFile('main', options.configDir); - if (!configFileName) { - throw new Error(`unable to find storybook main file in ${options.configDir}`); - } - const main = await readConfig(configFileName); - const disableWhatsNewNotifications = main.getFieldValue([ - 'core', - 'disableWhatsNewNotifications', - ]); + const main = await loadMainConfig({ configDir: options.configDir, noCache: true }); + const disableWhatsNewNotifications = + (main.core as CoreConfig)?.disableWhatsNewNotifications === true; const cache: WhatsNewCache = (await options.cache.get(WHATS_NEW_CACHE)) ?? {}; const data = { @@ -91,8 +86,14 @@ export function initializeWhatsNew( const isTelemetryEnabled = coreOptions.disableTelemetry !== true; try { const mainPath = findConfigFile('main', options.configDir); - invariant(mainPath, `unable to find storybook main file in ${options.configDir}`); + invariant(mainPath, `unable to find Storybook main file in ${options.configDir}`); const main = await readConfig(mainPath); + if (!main._exportsObject) { + // eslint-disable-next-line local-rules/no-uncategorized-errors + throw new Error( + `Unable to parse Storybook main file while trying to read 'core' property` + ); + } main.setFieldValue(['core', 'disableWhatsNewNotifications'], disableWhatsNewNotifications); await writeFile(mainPath, printConfig(main).code); if (isTelemetryEnabled) { From 5f21921ba3a76e9dcd0f948ef402011d692cb226 Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Wed, 20 Nov 2024 11:46:50 +0100 Subject: [PATCH 006/111] refactor: Use better sanitisation for the intersect util --- code/core/src/manager-api/lib/intersect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/core/src/manager-api/lib/intersect.ts b/code/core/src/manager-api/lib/intersect.ts index 84eae122322..764af9ac172 100644 --- a/code/core/src/manager-api/lib/intersect.ts +++ b/code/core/src/manager-api/lib/intersect.ts @@ -1,6 +1,6 @@ export default (a: T[], b: T[]): T[] => { // no point in intersecting if one of the input is ill-defined - if (!a || !b) { + if (!Array.isArray(a) || !Array.isArray(b)) { return []; } From 103c6b24b5bb62f19fdc42af4c7fa1add22e1ff4 Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Wed, 20 Nov 2024 12:24:39 +0100 Subject: [PATCH 007/111] Manager: Add tags property to GroupEntry objects --- code/core/src/manager-api/lib/stories.ts | 2 + .../src/manager-api/tests/stories.test.ts | 69 +++++++++++++++++++ code/core/src/types/modules/api-stories.ts | 1 + 3 files changed, 72 insertions(+) diff --git a/code/core/src/manager-api/lib/stories.ts b/code/core/src/manager-api/lib/stories.ts index eed677371ef..aef21759907 100644 --- a/code/core/src/manager-api/lib/stories.ts +++ b/code/core/src/manager-api/lib/stories.ts @@ -289,6 +289,8 @@ export const transformStoryIndexToStoriesHash = ( children: [childId], }), }); + // same as the merge for the component conditional branch above. + acc[id].tags = intersect(acc[id]?.tags ?? item.tags, item.tags); } }); diff --git a/code/core/src/manager-api/tests/stories.test.ts b/code/core/src/manager-api/tests/stories.test.ts index 9d3c4433fde..ed84ec85740 100644 --- a/code/core/src/manager-api/tests/stories.test.ts +++ b/code/core/src/manager-api/tests/stories.test.ts @@ -332,6 +332,75 @@ describe('stories API', () => { tags: ['shared', 'two-specific'], }); }); + + it('intersects story/docs tags to compute tags for group entries', () => { + const moduleArgs = createMockModuleArgs({}); + const { api } = initStories(moduleArgs as unknown as ModuleArgs); + const { store } = moduleArgs; + api.setIndex({ + v: 5, + entries: { + 'a-sampleone': { + type: 'story', + id: 'a-sampleone', + title: 'A/SampleOne', + name: '1', + tags: ['shared', 'one-specific'], + importPath: './a.ts', + }, + 'a-sampletwo': { + type: 'story', + id: 'a-sampletwo', + title: 'A/SampleTwo', + name: '2', + tags: ['shared', 'two-specific'], + importPath: './a.ts', + }, + 'a-embedded-othertopic': { + type: 'docs', + id: 'a-embedded-othertopic', + title: 'A/Embedded/OtherTopic', + name: '3', + tags: ['shared', 'embedded-docs-specific', 'other'], + storiesImports: [], + importPath: './embedded/other.mdx', + }, + 'a-embedded-extras': { + type: 'docs', + id: 'a-embedded-extras', + title: 'A/Embedded/Extras', + name: '3', + tags: ['shared', 'embedded-docs-specific', 'extras'], + storiesImports: [], + importPath: './embedded/extras.mdx', + }, + }, + }); + const { index } = store.getState(); + // We need exact key ordering, even if in theory JS doesn't guarantee it + expect(Object.keys(index!)).toEqual([ + 'a', + 'a-sampleone', + 'a-sampletwo', + 'a-embedded', + 'a-embedded-othertopic', + 'a-embedded-extras', + ]); + // Acts as the root, so that the next level is a group we're testing. + expect(index!.a).toMatchObject({ + type: 'root', + id: 'a', + children: ['a-sampleone', 'a-sampletwo', 'a-embedded'], + }); + // The object of this test. + expect(index!['a-embedded']).toMatchObject({ + type: 'group', + id: 'a-embedded', + parent: 'a', + name: 'Embedded', + tags: ['shared', 'embedded-docs-specific'], + }); + }); // Stories can get out of order for a few reasons -- see reproductions on // https://github.com/storybookjs/storybook/issues/5518 it('does the right thing for out of order stories', async () => { diff --git a/code/core/src/types/modules/api-stories.ts b/code/core/src/types/modules/api-stories.ts index c53d379b719..79ffb64d4d2 100644 --- a/code/core/src/types/modules/api-stories.ts +++ b/code/core/src/types/modules/api-stories.ts @@ -21,6 +21,7 @@ export interface API_GroupEntry extends API_BaseEntry { type: 'group'; parent?: StoryId; children: StoryId[]; + tags: Tag[]; } export interface API_ComponentEntry extends API_BaseEntry { From 247e65cde81b6a4f5707ed610f4b42cfe1727c43 Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Wed, 20 Nov 2024 14:33:28 +0100 Subject: [PATCH 008/111] refactor: Further improve sanitisation in intersect --- code/core/src/manager-api/lib/intersect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/core/src/manager-api/lib/intersect.ts b/code/core/src/manager-api/lib/intersect.ts index 764af9ac172..5918f7c8bab 100644 --- a/code/core/src/manager-api/lib/intersect.ts +++ b/code/core/src/manager-api/lib/intersect.ts @@ -1,6 +1,6 @@ export default (a: T[], b: T[]): T[] => { // no point in intersecting if one of the input is ill-defined - if (!Array.isArray(a) || !Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || !a.length || !b.length) { return []; } From 68c406223b3a1d028176fea200ef084fd114eb13 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 20 Nov 2024 22:01:22 +0100 Subject: [PATCH 009/111] 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 3d8bbaaa090..c1f17f25830 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 9b560cdd8ce..8bbbdfc775a 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 00000000000..a4b70b947db --- /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 f69f065aa42..eedb5ba20aa 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 fcf45a9df6f..6de7c7ca57f 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 3851bb885251c47dec53e7f7ae1d58f93e7579ec Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 21 Nov 2024 11:27:22 +0100 Subject: [PATCH 010/111] Replace mapStatusUpdate with stateUpdater to allow intercepting and modifying the internal test provider state updates --- code/addons/test/src/manager.tsx | 53 ++++++++++--------- .../modules/experimental_testmodule.ts | 8 ++- .../components/sidebar/SidebarBottom.tsx | 18 +++---- code/core/src/types/modules/addons.ts | 7 +-- 4 files changed, 45 insertions(+), 41 deletions(-) diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index 9b560cdd8ce..d17e6ecc560 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -197,30 +197,35 @@ addons.register(ADDON_ID, (api) => { ); }, - mapStatusUpdate: (state) => - Object.fromEntries( - (state.details.testResults || []).flatMap((testResult) => - testResult.results - .map(({ storyId, status, testRunId, ...rest }) => { - if (storyId) { - const statusObject: API_StatusObject = { - title: 'Component tests', - status: statusMap[status], - description: - 'failureMessages' in rest && rest.failureMessages?.length - ? rest.failureMessages.join('\n') - : '', - data: { - testRunId, - }, - onClick: openAddonPanel, - }; - return [storyId, statusObject]; - } - }) - .filter(Boolean) - ) - ), + stateUpdater: (state, update) => { + if (update.details?.testResults) { + const statusUpdate = Object.fromEntries( + update.details.testResults.flatMap((testResult) => + testResult.results + .map(({ storyId, status, testRunId, ...rest }) => { + if (storyId) { + const statusObject: API_StatusObject = { + title: 'Component tests', + status: statusMap[status], + description: + 'failureMessages' in rest && rest.failureMessages?.length + ? rest.failureMessages.join('\n') + : '', + data: { + testRunId, + }, + onClick: openAddonPanel, + }; + return [storyId, statusObject]; + } + }) + .filter(Boolean) + ) + ); + api.experimental_updateStatus(TEST_PROVIDER_ID, statusUpdate); + } + return { ...state, ...update }; + }, } as Addon_TestProviderType<{ testResults: TestResult[]; }>); diff --git a/code/core/src/manager-api/modules/experimental_testmodule.ts b/code/core/src/manager-api/modules/experimental_testmodule.ts index 54bfdc437da..7c34e2c8bae 100644 --- a/code/core/src/manager-api/modules/experimental_testmodule.ts +++ b/code/core/src/manager-api/modules/experimental_testmodule.ts @@ -51,13 +51,17 @@ export const init: ModuleFn = ({ store, fullAPI }) => { const api: SubAPI = { getTestProviderState(id) { const { testProviders } = store.getState(); - return testProviders?.[id]; }, updateTestProviderState(id, update) { return store.setState( ({ testProviders }) => { - return { testProviders: { ...testProviders, [id]: { ...testProviders[id], ...update } } }; + const currentState = testProviders[id]; + const updatedState = currentState.stateUpdater?.(currentState, update) ?? { + ...currentState, + ...update, + }; + return { testProviders: { ...testProviders, [id]: updatedState } }; }, { persistence: 'session' } ); diff --git a/code/core/src/manager/components/sidebar/SidebarBottom.tsx b/code/core/src/manager/components/sidebar/SidebarBottom.tsx index 9eea4a4855b..36d7692ffe2 100644 --- a/code/core/src/manager/components/sidebar/SidebarBottom.tsx +++ b/code/core/src/manager/components/sidebar/SidebarBottom.tsx @@ -153,18 +153,12 @@ export const SidebarBottomBase = ({ }; const onProgressReport = ({ providerId, ...result }: TestingModuleProgressReportPayload) => { - if (result.status === 'failed') { - api.updateTestProviderState(providerId, { ...result, running: false, failed: true }); - } else { - const update = { ...result, running: result.status === 'pending' }; - api.updateTestProviderState(providerId, update); - - const { mapStatusUpdate, ...state } = testProviders[providerId]; - const statusUpdate = mapStatusUpdate?.({ ...state, ...update }); - if (statusUpdate) { - api.experimental_updateStatus(providerId, statusUpdate); - } - } + api.updateTestProviderState( + providerId, + result.status === 'failed' + ? { ...result, running: false, failed: true } + : { ...result, running: result.status === 'pending' } + ); }; api.on(TESTING_MODULE_CRASH_REPORT, onCrashReport); diff --git a/code/core/src/types/modules/addons.ts b/code/core/src/types/modules/addons.ts index d01563530bb..c9275394448 100644 --- a/code/core/src/types/modules/addons.ts +++ b/code/core/src/types/modules/addons.ts @@ -486,9 +486,10 @@ export interface Addon_TestProviderType< }, components: { ListItem: typeof ListItem } ) => ReactNode; - mapStatusUpdate?: ( - state: Addon_TestProviderState
- ) => API_StatusUpdate | ((state: API_StatusState) => API_StatusUpdate); + stateUpdater?: ( + state: TestProviderConfig & Addon_TestProviderState
, + update: Partial> + ) => Partial>; runnable?: boolean; watchable?: boolean; } From 1e0869b6bae746320a134826378c68dfedeedce2 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 21 Nov 2024 15:53:55 +0100 Subject: [PATCH 011/111] 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 eedb5ba20aa..eb957f2749b 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 012/111] 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 8bbbdfc775a..b7a72e979ad 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 a4b70b947db..0f002730794 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 685a56e2baf..96164dcfe30 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 b86bc649b8dd249711722abc1ae27356af4083a2 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 21 Nov 2024 21:21:44 +0100 Subject: [PATCH 013/111] Optional return value --- code/addons/test/src/manager.tsx | 1 - code/core/src/types/modules/addons.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index d17e6ecc560..b2e99e0ef16 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -224,7 +224,6 @@ addons.register(ADDON_ID, (api) => { ); api.experimental_updateStatus(TEST_PROVIDER_ID, statusUpdate); } - return { ...state, ...update }; }, } as Addon_TestProviderType<{ testResults: TestResult[]; diff --git a/code/core/src/types/modules/addons.ts b/code/core/src/types/modules/addons.ts index c9275394448..5554c7f7b0f 100644 --- a/code/core/src/types/modules/addons.ts +++ b/code/core/src/types/modules/addons.ts @@ -489,7 +489,7 @@ export interface Addon_TestProviderType< stateUpdater?: ( state: TestProviderConfig & Addon_TestProviderState
, update: Partial> - ) => Partial>; + ) => void | Partial>; runnable?: boolean; watchable?: boolean; } From f69b7f35ab2c52f2f057fd0cd8b552e2c6adbb43 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 22 Nov 2024 11:12:41 +0100 Subject: [PATCH 014/111] Extract test provider render method to a component --- .../test/src/components/RelativeTime.tsx | 19 +- .../test/src/components/TestProvider.tsx | 145 ++++++++++++++++ code/addons/test/src/manager.tsx | 164 +----------------- 3 files changed, 172 insertions(+), 156 deletions(-) create mode 100644 code/addons/test/src/components/TestProvider.tsx diff --git a/code/addons/test/src/components/RelativeTime.tsx b/code/addons/test/src/components/RelativeTime.tsx index d643960b06e..f819eb2211c 100644 --- a/code/addons/test/src/components/RelativeTime.tsx +++ b/code/addons/test/src/components/RelativeTime.tsx @@ -1,6 +1,23 @@ import { useEffect, useState } from 'react'; -import { getRelativeTimeString } from '../manager'; +function getRelativeTimeString(date: Date): string { + const delta = Math.round((date.getTime() - Date.now()) / 1000); + const cutoffs = [60, 3600, 86400, 86400 * 7, 86400 * 30, 86400 * 365, Infinity]; + const units: Intl.RelativeTimeFormatUnit[] = [ + 'second', + 'minute', + 'hour', + 'day', + 'week', + 'month', + 'year', + ]; + + const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(delta)); + const divisor = unitIndex ? cutoffs[unitIndex - 1] : 1; + const rtf = new Intl.RelativeTimeFormat('en', { numeric: 'auto' }); + return rtf.format(Math.floor(delta / divisor), units[unitIndex]); +} export const RelativeTime = ({ timestamp, testCount }: { timestamp: Date; testCount: number }) => { const [relativeTimeString, setRelativeTimeString] = useState(null); diff --git a/code/addons/test/src/components/TestProvider.tsx b/code/addons/test/src/components/TestProvider.tsx new file mode 100644 index 00000000000..b0095ae454b --- /dev/null +++ b/code/addons/test/src/components/TestProvider.tsx @@ -0,0 +1,145 @@ +import React, { useState } from 'react'; + +import { Button, Link as LinkComponent } from 'storybook/internal/components'; +import { + TESTING_MODULE_RUN_ALL_REQUEST, + type TestProviderConfig, +} from 'storybook/internal/core-events'; +import type { API } from 'storybook/internal/manager-api'; +import { styled } from 'storybook/internal/theming'; + +import { EyeIcon, PlayHollowIcon, StopAltHollowIcon } from '@storybook/icons'; +import type { Addon_TestProviderState } from '@storybook/types'; + +import { TEST_PROVIDER_ID } from '../constants'; +import type { Details } from '../manager'; +import { GlobalErrorModal } from './GlobalErrorModal'; +import { RelativeTime } from './RelativeTime'; + +const Info = styled.div({ + display: 'flex', + flexDirection: 'column', + marginLeft: 6, +}); + +const SidebarContextMenuTitle = styled.div<{ crashed?: boolean }>(({ crashed, theme }) => ({ + fontSize: theme.typography.size.s1, + fontWeight: crashed ? 'bold' : 'normal', + color: crashed ? theme.color.negativeText : theme.color.defaultText, +})); + +const Description = styled.div(({ theme }) => ({ + fontSize: theme.typography.size.s1, + color: theme.barTextColor, +})); + +const Actions = styled.div({ + display: 'flex', + gap: 6, +}); + +interface TestProviderProps { + api: API; + state: TestProviderConfig & Addon_TestProviderState
; +} + +export const TestProvider = ({ api, state }: TestProviderProps) => { + const [isModalOpen, setIsModalOpen] = useState(false); + + const title = state.crashed || state.failed ? 'Component tests failed' : 'Component tests'; + const errorMessage = state.error?.message; + let description: string | React.ReactNode = 'Not run'; + + if (state.running) { + description = state.progress + ? `Testing... ${state.progress.numPassedTests}/${state.progress.numTotalTests}` + : 'Starting...'; + } else if (state.failed && !errorMessage) { + description = ''; + } else if (state.crashed || (state.failed && errorMessage)) { + description = ( + <> + { + setIsModalOpen(true); + }} + > + {state.error?.name || 'View full error'} + + + ); + } else if (state.progress?.finishedAt) { + description = ( + + ); + } else if (state.watching) { + description = 'Watching for file changes'; + } + + return ( + <> + + + {title} + + {description} + + + + {state.watchable && ( + + )} + {state.runnable && ( + <> + {state.running && state.cancellable ? ( + + ) : ( + + )} + + )} + + + { + setIsModalOpen(false); + }} + onRerun={() => { + setIsModalOpen(false); + api.getChannel().emit(TESTING_MODULE_RUN_ALL_REQUEST, { providerId: TEST_PROVIDER_ID }); + }} + /> + + ); +}; diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index 9b560cdd8ce..92b67dbd962 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -1,10 +1,8 @@ -import React, { useState } from 'react'; +import React from 'react'; -import { AddonPanel, Button, Link as LinkComponent } from 'storybook/internal/components'; -import { TESTING_MODULE_RUN_ALL_REQUEST } from 'storybook/internal/core-events'; +import { AddonPanel } from 'storybook/internal/components'; import type { Combo } from 'storybook/internal/manager-api'; import { Consumer, addons, types } from 'storybook/internal/manager-api'; -import { styled } from 'storybook/internal/theming'; import { type API_StatusObject, type API_StatusValue, @@ -12,63 +10,23 @@ import { Addon_TypesEnum, } from 'storybook/internal/types'; -import { EyeIcon, PlayHollowIcon, StopAltHollowIcon } from '@storybook/icons'; - import { ContextMenuItem } from './components/ContextMenuItem'; -import { GlobalErrorModal } from './components/GlobalErrorModal'; import { Panel } from './components/Panel'; import { PanelTitle } from './components/PanelTitle'; -import { RelativeTime } from './components/RelativeTime'; +import { TestProvider } from './components/TestProvider'; import { ADDON_ID, PANEL_ID, TEST_PROVIDER_ID } from './constants'; import type { TestResult } from './node/reporter'; +export type Details = { + testResults: TestResult[]; +}; + const statusMap: Record = { failed: 'error', passed: 'success', pending: 'pending', }; -export function getRelativeTimeString(date: Date): string { - const delta = Math.round((date.getTime() - Date.now()) / 1000); - const cutoffs = [60, 3600, 86400, 86400 * 7, 86400 * 30, 86400 * 365, Infinity]; - const units: Intl.RelativeTimeFormatUnit[] = [ - 'second', - 'minute', - 'hour', - 'day', - 'week', - 'month', - 'year', - ]; - - const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(delta)); - const divisor = unitIndex ? cutoffs[unitIndex - 1] : 1; - const rtf = new Intl.RelativeTimeFormat('en', { numeric: 'auto' }); - return rtf.format(Math.floor(delta / divisor), units[unitIndex]); -} - -const Info = styled.div({ - display: 'flex', - flexDirection: 'column', - marginLeft: 6, -}); - -const SidebarContextMenuTitle = styled.div<{ crashed?: boolean }>(({ crashed, theme }) => ({ - fontSize: theme.typography.size.s1, - fontWeight: crashed ? 'bold' : 'normal', - color: crashed ? theme.color.negativeText : theme.color.defaultText, -})); - -const Description = styled.div(({ theme }) => ({ - fontSize: theme.typography.size.s1, - color: theme.barTextColor, -})); - -const Actions = styled.div({ - display: 'flex', - gap: 6, -}); - addons.register(ADDON_ID, (api) => { const storybookBuilder = (globalThis as any).STORYBOOK_BUILDER || ''; if (storybookBuilder.includes('vite')) { @@ -82,6 +40,7 @@ addons.register(ADDON_ID, (api) => { runnable: true, watchable: true, name: 'Component tests', + render: (state) => , sidebarContextMenu: ({ context, state }, { ListItem }) => { if (context.type === 'docs') { @@ -94,109 +53,6 @@ addons.register(ADDON_ID, (api) => { return ; }, - render: (state) => { - const [isModalOpen, setIsModalOpen] = useState(false); - - const title = state.crashed || state.failed ? 'Component tests failed' : 'Component tests'; - const errorMessage = state.error?.message; - let description: string | React.ReactNode = 'Not run'; - - if (state.running) { - description = state.progress - ? `Testing... ${state.progress.numPassedTests}/${state.progress.numTotalTests}` - : 'Starting...'; - } else if (state.failed && !errorMessage) { - description = ''; - } else if (state.crashed || (state.failed && errorMessage)) { - description = ( - <> - { - setIsModalOpen(true); - }} - > - {state.error?.name || 'View full error'} - - - ); - } else if (state.progress?.finishedAt) { - description = ( - - ); - } else if (state.watching) { - description = 'Watching for file changes'; - } - - return ( - <> - - - {title} - - {description} - - - - {state.watchable && ( - - )} - {state.runnable && ( - <> - {state.running && state.cancellable ? ( - - ) : ( - - )} - - )} - - - { - setIsModalOpen(false); - }} - onRerun={() => { - setIsModalOpen(false); - api - .getChannel() - .emit(TESTING_MODULE_RUN_ALL_REQUEST, { providerId: TEST_PROVIDER_ID }); - }} - /> - - ); - }, - mapStatusUpdate: (state) => Object.fromEntries( (state.details.testResults || []).flatMap((testResult) => @@ -221,9 +77,7 @@ addons.register(ADDON_ID, (api) => { .filter(Boolean) ) ), - } as Addon_TestProviderType<{ - testResults: TestResult[]; - }>); + } as Addon_TestProviderType
); } const filter = ({ state }: Combo) => { From 049cec156914bd9c1983339d9bad7b461fe77189 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 22 Nov 2024 15:03:00 +0100 Subject: [PATCH 015/111] Draw horizontal line between each test provider --- code/addons/test/src/components/TestProvider.tsx | 2 +- .../src/manager/components/sidebar/TestingModule.tsx | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/code/addons/test/src/components/TestProvider.tsx b/code/addons/test/src/components/TestProvider.tsx index b0095ae454b..3588a6a58ef 100644 --- a/code/addons/test/src/components/TestProvider.tsx +++ b/code/addons/test/src/components/TestProvider.tsx @@ -35,7 +35,7 @@ const Description = styled.div(({ theme }) => ({ const Actions = styled.div({ display: 'flex', - gap: 6, + gap: 2, }); interface TestProviderProps { diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index a28a338857b..0385cdcd70e 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -78,10 +78,8 @@ const Collapsible = styled.div(({ theme }) => ({ })); const Content = styled.div({ - padding: '12px 6px', display: 'flex', flexDirection: 'column', - gap: '12px', }); const Bar = styled.div<{ onClick?: (e: SyntheticEvent) => void }>(({ onClick }) => ({ @@ -138,11 +136,16 @@ const StatusButton = styled(Button)<{ status: 'negative' | 'warning' }>( }) ); -const TestProvider = styled.div({ +const TestProvider = styled.div(({ theme }) => ({ display: 'flex', justifyContent: 'space-between', + padding: '12px 6px', gap: 6, -}); + + '&:not(:last-child)': { + boxShadow: `inset 0 -1px 0 ${theme.appBorderColor}`, + }, +})); interface TestingModuleProps { testProviders: TestProviders[keyof TestProviders][]; From 9efed90d22a6a404ff3fc78750e1fd72fb9ebdfd Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 25 Nov 2024 17:07:42 +0100 Subject: [PATCH 016/111] Glow testing module when changing settings --- .../sidebar/TestingModule.stories.tsx | 44 ++++++--- .../components/sidebar/TestingModule.tsx | 89 +++++++++++-------- 2 files changed, 84 insertions(+), 49 deletions(-) diff --git a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx index a13e4ccbad5..fdbfa3db37d 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx @@ -1,5 +1,6 @@ import React from 'react'; +import { styled } from '@storybook/core/theming'; import { Addon_TypesEnum } from '@storybook/core/types'; import type { Meta, StoryObj } from '@storybook/react'; import { fn, userEvent } from '@storybook/test'; @@ -9,6 +10,11 @@ import { ManagerContext } from '@storybook/core/manager-api'; import { TestingModule } from './TestingModule'; +const TestProvider = styled.div({ + padding: 8, + fontSize: 12, +}); + const baseState = { details: {}, cancellable: false, @@ -25,11 +31,11 @@ const testProviders: TestProviders[keyof TestProviders][] = [ id: 'component-tests', name: 'Component tests', render: () => ( - <> + Component tests
Ran 2 seconds ago - +
), runnable: true, watchable: true, @@ -40,11 +46,11 @@ const testProviders: TestProviders[keyof TestProviders][] = [ id: 'visual-tests', name: 'Visual tests', render: () => ( - <> + Visual tests
Not run - +
), runnable: true, ...baseState, @@ -54,11 +60,11 @@ const testProviders: TestProviders[keyof TestProviders][] = [ id: 'linting', name: 'Linting', render: () => ( - <> + Linting
Watching for changes - +
), ...baseState, watching: true, @@ -67,6 +73,8 @@ const testProviders: TestProviders[keyof TestProviders][] = [ const managerContext: any = { api: { + on: fn().mockName('api::on'), + off: fn().mockName('api::off'), runTestProvider: fn().mockName('api::runTestProvider'), cancelTestProvider: fn().mockName('api::cancelTestProvider'), updateTestProviderState: fn().mockName('api::updateTestProviderState'), @@ -104,9 +112,9 @@ type Story = StoryObj; export const Default: Story = {}; -export const Collapsed: Story = { +export const Expanded: Story = { play: async ({ canvas }) => { - const button = await canvas.findByRole('button', { name: /Collapse/ }); + const button = await canvas.findByRole('button', { name: /Expand/ }); await userEvent.click(button); }, }; @@ -116,6 +124,7 @@ export const Statuses: Story = { errorCount: 14, warningCount: 42, }, + play: Expanded.play, }; export const ErrorsActive: Story = { @@ -123,6 +132,7 @@ export const ErrorsActive: Story = { ...Statuses.args, errorsActive: true, }, + play: Expanded.play, }; export const WarningsActive: Story = { @@ -130,6 +140,7 @@ export const WarningsActive: Story = { ...Statuses.args, warningsActive: true, }, + play: Expanded.play, }; export const BothActive: Story = { @@ -138,28 +149,31 @@ export const BothActive: Story = { errorsActive: true, warningsActive: true, }, + play: Expanded.play, }; export const CollapsedStatuses: Story = { args: Statuses.args, - play: Collapsed.play, + play: Expanded.play, }; export const Running: Story = { args: { testProviders: [{ ...testProviders[0], running: true }, ...testProviders.slice(1)], }, + play: Expanded.play, }; export const RunningAll: Story = { args: { testProviders: testProviders.map((tp) => ({ ...tp, running: !!tp.runnable })), }, + play: Expanded.play, }; export const CollapsedRunning: Story = { args: RunningAll.args, - play: Collapsed.play, + play: Expanded.play, }; export const Cancellable: Story = { @@ -169,6 +183,7 @@ export const Cancellable: Story = { ...testProviders.slice(1), ], }, + play: Expanded.play, }; export const Cancelling: Story = { @@ -178,12 +193,14 @@ export const Cancelling: Story = { ...testProviders.slice(1), ], }, + play: Expanded.play, }; export const Watching: Story = { args: { testProviders: [{ ...testProviders[0], watching: true }, ...testProviders.slice(1)], }, + play: Expanded.play, }; export const Failing: Story = { @@ -193,12 +210,14 @@ export const Failing: Story = { ...testProviders.slice(1), ], }, + play: Expanded.play, }; export const Failed: Story = { args: { testProviders: [{ ...testProviders[0], failed: true }, ...testProviders.slice(1)], }, + play: Expanded.play, }; export const Crashed: Story = { @@ -207,15 +226,16 @@ export const Crashed: Story = { { ...testProviders[0], render: () => ( - <> + Component tests didn't complete
Problems! - +
), crashed: true, }, ...testProviders.slice(1), ], }, + play: Expanded.play, }; diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index c07f37e8b02..6724f52d640 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -1,11 +1,11 @@ -import React, { Fragment, type SyntheticEvent, useEffect, useRef, useState } from 'react'; +import React, { type SyntheticEvent, useEffect, useRef, useState } from 'react'; import { Button, TooltipNote } from '@storybook/core/components'; import { WithTooltip } from '@storybook/core/components'; import { keyframes, styled } from '@storybook/core/theming'; import { ChevronSmallUpIcon, PlayAllHollowIcon } from '@storybook/icons'; -import type { TestProviders } from '@storybook/core/core-events'; +import { TESTING_MODULE_CONFIG_CHANGE, type TestProviders } from '@storybook/core/core-events'; import { useStorybookApi } from '@storybook/core/manager-api'; import { LegacyRender } from './LegacyRender'; @@ -22,42 +22,42 @@ const spin = keyframes({ '100%': { transform: 'rotate(360deg)' }, }); -const Outline = styled.div<{ crashed: boolean; failed: boolean; running: boolean }>( - ({ crashed, running, theme, failed }) => ({ - position: 'relative', - lineHeight: '20px', - width: '100%', - padding: 1, - overflow: 'hidden', - background: `var(--sb-sidebar-bottom-card-background, ${theme.background.content})`, - borderRadius: - `var(--sb-sidebar-bottom-card-border-radius, ${theme.appBorderRadius + 1}px)` as any, - boxShadow: `inset 0 0 0 1px ${crashed && !running ? theme.color.negative : theme.appBorderColor}, var(--sb-sidebar-bottom-card-box-shadow, 0 1px 2px 0 rgba(0, 0, 0, 0.05), 0px -5px 20px 10px ${theme.background.app})`, - transitionProperty: - 'color, background-color, border-color, text-decoration-color, fill, stroke', - transitionTimingFunction: 'cubic-bezier(0.4, 0, 0.2, 1)', - transitionDuration: '0.15s', +const Outline = styled.div<{ + crashed: boolean; + failed: boolean; + running: boolean; + updated: boolean; +}>(({ crashed, failed, running, theme, updated }) => ({ + position: 'relative', + lineHeight: '20px', + width: '100%', + padding: 1, + overflow: 'hidden', + backgroundColor: `var(--sb-sidebar-bottom-card-background, ${theme.background.content})`, + borderRadius: + `var(--sb-sidebar-bottom-card-border-radius, ${theme.appBorderRadius + 1}px)` as any, + boxShadow: `inset 0 0 0 1px ${crashed && !running ? theme.color.negative : updated ? theme.color.positive : theme.appBorderColor}, var(--sb-sidebar-bottom-card-box-shadow, 0 1px 2px 0 rgba(0, 0, 0, 0.05), 0px -5px 20px 10px ${theme.background.app})`, + transition: 'box-shadow 1s', - '&:after': { - content: '""', - display: running ? 'block' : 'none', - position: 'absolute', - left: '50%', - top: '50%', - marginLeft: 'calc(max(100vw, 100vh) * -0.5)', - marginTop: 'calc(max(100vw, 100vh) * -0.5)', - height: 'max(100vw, 100vh)', - width: 'max(100vw, 100vh)', - animation: `${spin} 3s linear infinite`, - background: failed - ? // Hardcoded colors to prevent themes from messing with them (orange+gold, secondary+seafoam) - `conic-gradient(transparent 90deg, #FC521F 150deg, #FFAE00 210deg, transparent 270deg)` - : `conic-gradient(transparent 90deg, #029CFD 150deg, #37D5D3 210deg, transparent 270deg)`, - opacity: 1, - willChange: 'auto', - }, - }) -); + '&:after': { + content: '""', + display: running ? 'block' : 'none', + position: 'absolute', + left: '50%', + top: '50%', + marginLeft: 'calc(max(100vw, 100vh) * -0.5)', + marginTop: 'calc(max(100vw, 100vh) * -0.5)', + height: 'max(100vw, 100vh)', + width: 'max(100vw, 100vh)', + animation: `${spin} 3s linear infinite`, + background: failed + ? // Hardcoded colors to prevent themes from messing with them (orange+gold, secondary+seafoam) + `conic-gradient(transparent 90deg, #FC521F 150deg, #FFAE00 210deg, transparent 270deg)` + : `conic-gradient(transparent 90deg, #029CFD 150deg, #37D5D3 210deg, transparent 270deg)`, + opacity: 1, + willChange: 'auto', + }, +})); const Card = styled.div(({ theme }) => ({ position: 'relative', @@ -165,6 +165,7 @@ export const TestingModule = ({ }: TestingModuleProps) => { const api = useStorybookApi(); const contentRef = useRef(null); + const [updated, setUpdated] = useState(false); const [animating, setAnimating] = useState(false); const [collapsed, setCollapsed] = useState(true); const [maxHeight, setMaxHeight] = useState(DEFAULT_HEIGHT); @@ -173,6 +174,19 @@ export const TestingModule = ({ setMaxHeight(contentRef.current?.offsetHeight || DEFAULT_HEIGHT); }, []); + useEffect(() => { + let timeout: NodeJS.Timeout; + const handler = () => { + setUpdated(true); + timeout = setTimeout(setUpdated, 1000, false); + }; + api.on(TESTING_MODULE_CONFIG_CHANGE, handler); + return () => { + api.off(TESTING_MODULE_CONFIG_CHANGE, handler); + clearTimeout(timeout); + }; + }, [api]); + const toggleCollapsed = () => { setAnimating(true); setMaxHeight(contentRef.current?.offsetHeight || DEFAULT_HEIGHT); @@ -191,6 +205,7 @@ export const TestingModule = ({ running={running} crashed={crashed} failed={failed || errorCount > 0} + updated={updated} > Date: Tue, 26 Nov 2024 06:36:45 +0100 Subject: [PATCH 017/111] Manager: Generalise tag intersection to root entries and ensure all entries have tags --- code/core/src/manager-api/lib/stories.ts | 22 ++++++++++++++----- .../src/manager-api/tests/stories.test.ts | 13 ++++++++++- code/core/src/types/modules/api-stories.ts | 5 +---- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/code/core/src/manager-api/lib/stories.ts b/code/core/src/manager-api/lib/stories.ts index aef21759907..59b59f070a1 100644 --- a/code/core/src/manager-api/lib/stories.ts +++ b/code/core/src/manager-api/lib/stories.ts @@ -1,4 +1,5 @@ import type { + API_BaseEntry, API_ComponentEntry, API_DocsEntry, API_GroupEntry, @@ -16,6 +17,7 @@ import type { StoryId, StoryIndexV2, StoryIndexV3, + Tag, } from '@storybook/core/types'; import { sanitize } from '@storybook/csf'; @@ -248,6 +250,7 @@ export const transformStoryIndexToStoriesHash = ( type: 'root', id, name: names[idx], + tags: [], depth: idx, renderLabel, startCollapsed: collapsedRoots.includes(id), @@ -267,6 +270,7 @@ export const transformStoryIndexToStoriesHash = ( type: 'component', id, name: names[idx], + tags: [], parent: paths[idx - 1], depth: idx, renderLabel, @@ -274,14 +278,12 @@ export const transformStoryIndexToStoriesHash = ( children: [childId], }), }); - // merge computes a union of arrays but we want an intersection on this - // specific array property, so it's easier to add it after the merge. - acc[id].tags = intersect(acc[id]?.tags ?? item.tags, item.tags); } else { acc[id] = merge((acc[id] || {}) as API_GroupEntry, { type: 'group', id, name: names[idx], + tags: [], parent: paths[idx - 1], depth: idx, renderLabel, @@ -289,14 +291,13 @@ export const transformStoryIndexToStoriesHash = ( children: [childId], }), }); - // same as the merge for the component conditional branch above. - acc[id].tags = intersect(acc[id]?.tags ?? item.tags, item.tags); } }); // Finally add an entry for the docs/story itself acc[item.id] = { type: 'story', + tags: [], ...item, depth: paths.length, parent: paths[paths.length - 1], @@ -315,9 +316,18 @@ export const transformStoryIndexToStoriesHash = ( } acc[item.id] = item; - // Ensure we add the children depth-first *before* inserting any other entries + // Ensure we add the children depth-first *before* inserting any other entries, + // and compute tags from the children put in the accumulator afterwards, once + // they're all known and we can compute a sound intersection. if (item.type === 'root' || item.type === 'group' || item.type === 'component') { item.children.forEach((childId: any) => addItem(acc, storiesHashOutOfOrder[childId])); + + item.tags = item.children.reduce((currentTags: Tag[] | null, childId: any): Tag[] => { + const child = acc[childId]; + + // On the first child, we have nothing to intersect against so we use it as a source of data. + return currentTags === null ? child.tags : intersect(currentTags, child.tags); + }, null); } return acc; } diff --git a/code/core/src/manager-api/tests/stories.test.ts b/code/core/src/manager-api/tests/stories.test.ts index ed84ec85740..b652c87cd7a 100644 --- a/code/core/src/manager-api/tests/stories.test.ts +++ b/code/core/src/manager-api/tests/stories.test.ts @@ -162,6 +162,7 @@ describe('stories API', () => { expect(index!['design-system']).toMatchObject({ type: 'root', name: 'Design System', // root name originates from `kind`, so it gets trimmed + tags: [], }); expect(index!['design-system-some-component']).toMatchObject({ type: 'component', @@ -186,6 +187,7 @@ describe('stories API', () => { title: 'Root/First', name: 'Story 1', importPath: './path/to/root/first.ts', + tags: [], }, ...mockEntries, }, @@ -207,6 +209,7 @@ describe('stories API', () => { type: 'root', id: 'root', children: ['root-first'], + tags: [], }); }); it('sets roots when showRoots = true', () => { @@ -222,6 +225,7 @@ describe('stories API', () => { id: 'a-b--1', title: 'a/b', name: '1', + tags: [], importPath: './a/b.ts', }, }, @@ -233,6 +237,7 @@ describe('stories API', () => { type: 'root', id: 'a', children: ['a-b'], + tags: [], }); expect(index!['a-b']).toMatchObject({ type: 'component', @@ -333,7 +338,7 @@ describe('stories API', () => { }); }); - it('intersects story/docs tags to compute tags for group entries', () => { + it('intersects story/docs tags to compute tags for root and group entries', () => { const moduleArgs = createMockModuleArgs({}); const { api } = initStories(moduleArgs as unknown as ModuleArgs); const { store } = moduleArgs; @@ -391,6 +396,7 @@ describe('stories API', () => { type: 'root', id: 'a', children: ['a-sampleone', 'a-sampletwo', 'a-embedded'], + tags: ['shared'], }); // The object of this test. expect(index!['a-embedded']).toMatchObject({ @@ -1584,6 +1590,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1595,6 +1602,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1650,6 +1658,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1692,6 +1701,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1703,6 +1713,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, diff --git a/code/core/src/types/modules/api-stories.ts b/code/core/src/types/modules/api-stories.ts index 79ffb64d4d2..5f7b8dfe864 100644 --- a/code/core/src/types/modules/api-stories.ts +++ b/code/core/src/types/modules/api-stories.ts @@ -7,6 +7,7 @@ export interface API_BaseEntry { id: StoryId; depth: number; name: string; + tags: Tag[]; refId?: string; renderLabel?: (item: API_BaseEntry, api: any) => any; } @@ -21,14 +22,12 @@ export interface API_GroupEntry extends API_BaseEntry { type: 'group'; parent?: StoryId; children: StoryId[]; - tags: Tag[]; } export interface API_ComponentEntry extends API_BaseEntry { type: 'component'; parent?: StoryId; children: StoryId[]; - tags: Tag[]; } export interface API_DocsEntry extends API_BaseEntry { @@ -36,7 +35,6 @@ export interface API_DocsEntry extends API_BaseEntry { parent: StoryId; title: ComponentTitle; importPath: Path; - tags: Tag[]; prepared: boolean; parameters?: { [parameterName: string]: any; @@ -48,7 +46,6 @@ export interface API_StoryEntry extends API_BaseEntry { parent: StoryId; title: ComponentTitle; importPath: Path; - tags: Tag[]; prepared: boolean; parameters?: { [parameterName: string]: any; From a71d9012c4ee016231ea44382d8bc5121e5afd68 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 09:34:55 +0100 Subject: [PATCH 018/111] Add story for updated state --- .../sidebar/TestingModule.stories.tsx | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx index fdbfa3db37d..625396064f0 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx @@ -1,12 +1,13 @@ import React from 'react'; +import type { Listener } from '@storybook/core/channels'; import { styled } from '@storybook/core/theming'; import { Addon_TypesEnum } from '@storybook/core/types'; import type { Meta, StoryObj } from '@storybook/react'; -import { fn, userEvent } from '@storybook/test'; +import { fireEvent, fn } from '@storybook/test'; -import type { TestProviders } from '@storybook/core/core-events'; -import { ManagerContext } from '@storybook/core/manager-api'; +import { TESTING_MODULE_CONFIG_CHANGE, type TestProviders } from '@storybook/core/core-events'; +import { ManagerContext, mockChannel } from '@storybook/core/manager-api'; import { TestingModule } from './TestingModule'; @@ -71,10 +72,17 @@ const testProviders: TestProviders[keyof TestProviders][] = [ }, ]; +let triggerUpdate: () => void; +const channel = mockChannel(); const managerContext: any = { api: { - on: fn().mockName('api::on'), - off: fn().mockName('api::off'), + on: (eventName: string, listener: Listener) => { + if (eventName === TESTING_MODULE_CONFIG_CHANGE) { + triggerUpdate = listener; + } + return channel.on(eventName, listener); + }, + off: (eventName: string, listener: Listener) => channel.off(eventName, listener), runTestProvider: fn().mockName('api::runTestProvider'), cancelTestProvider: fn().mockName('api::cancelTestProvider'), updateTestProviderState: fn().mockName('api::updateTestProviderState'), @@ -115,7 +123,8 @@ export const Default: Story = {}; export const Expanded: Story = { play: async ({ canvas }) => { const button = await canvas.findByRole('button', { name: /Expand/ }); - await userEvent.click(button); + await fireEvent.click(button); + await new Promise((resolve) => setTimeout(resolve, 500)); }, }; @@ -239,3 +248,11 @@ export const Crashed: Story = { }, play: Expanded.play, }; + +export const Updated: Story = { + args: {}, + play: async (context) => { + await Expanded.play!(context); + triggerUpdate?.(); + }, +}; From dba14dcdc90d5864748881d7057a0b181ab08aca Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 09:45:24 +0100 Subject: [PATCH 019/111] Fix cursor on checkbox --- 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 cf6f4c81e4c..b6e079dc58b 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -60,6 +60,9 @@ const Extras = styled.div({ const Checkbox = styled.input({ margin: 0, + '&:enabled': { + cursor: 'pointer', + }, }); export const TestProviderRender: FC<{ From 40cce2bf8de84fc6626ebf2f2507b46d22239a67 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 10:10:36 +0100 Subject: [PATCH 020/111] Refactor useConfig to debounce update synchronization --- .../src/components/TestProviderRender.tsx | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index b6e079dc58b..e2ff8c8cbf2 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -1,11 +1,10 @@ -import React, { type FC, Fragment, useCallback, useRef, useState } from 'react'; +import React, { type FC, useCallback, useRef, useState } from 'react'; import { Button, ListItem } from 'storybook/internal/components'; import { TESTING_MODULE_CONFIG_CHANGE, type TestProviderConfig, type TestProviderState, - type TestingModuleConfigChangePayload, } from 'storybook/internal/core-events'; import type { API } from 'storybook/internal/manager-api'; import { styled, useTheme } from 'storybook/internal/theming'; @@ -20,6 +19,8 @@ import { StopAltHollowIcon, } from '@storybook/icons'; +import { debounce } from 'es-toolkit/compat'; + import { type Config, type Details, TEST_PROVIDER_ID } from '../constants'; import { Description } from './Description'; import { GlobalErrorModal } from './GlobalErrorModal'; @@ -76,10 +77,10 @@ export const TestProviderRender: FC<{ const title = state.crashed || state.failed ? 'Local tests failed' : 'Run local tests'; const errorMessage = state.error?.message; - const [config, changeConfig] = useConfig( + const [config, updateConfig] = useConfig( + api, state.id, - state.config || { a11y: false, coverage: false }, - api + state.config || { a11y: false, coverage: false } ); return ( @@ -157,7 +158,7 @@ export const TestProviderRender: FC<{ changeConfig({ coverage: !config.coverage })} + onChange={() => updateConfig({ coverage: !config.coverage })} /> } /> @@ -169,7 +170,7 @@ export const TestProviderRender: FC<{ changeConfig({ a11y: !config.a11y })} + onChange={() => updateConfig({ a11y: !config.a11y })} /> } /> @@ -201,29 +202,27 @@ export const TestProviderRender: FC<{ ); }; -function useConfig(id: string, config: Config, api: API) { - const data = useRef(config); - data.current = config || { - a11y: false, - coverage: false, - }; +function useConfig(api: API, providerId: string, initialConfig: Config) { + const [currentConfig, setConfig] = useState(initialConfig); + + const saveConfig = useCallback( + debounce((config: Config) => { + api.updateTestProviderState(providerId, { config }); + api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); + }, 500), + [api, providerId] + ); - const changeConfig = useCallback( + const updateConfig = useCallback( (update: Partial) => { - const newConfig = { - ...data.current, - ...update, - }; - api.updateTestProviderState(id, { - config: newConfig, + setConfig((value) => { + const updated = { ...value, ...update }; + saveConfig(updated); + return updated; }); - api.emit(TESTING_MODULE_CONFIG_CHANGE, { - providerId: id, - config: newConfig, - } as TestingModuleConfigChangePayload); }, - [api, id] + [saveConfig] ); - return [data.current, changeConfig] as const; + return [currentConfig, updateConfig] as const; } From 8789ac40081cb8909283e238fd74a98b631c374c Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 10:26:51 +0100 Subject: [PATCH 021/111] Fix story --- .../src/manager/components/sidebar/SidebarBottom.stories.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx b/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx index 898b02f2894..22cf6e68ef9 100644 --- a/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx +++ b/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx @@ -155,7 +155,9 @@ export const DynamicHeight: StoryObj = { play: async ({ canvasElement }) => { const screen = await within(canvasElement); - const toggleButton = await screen.getByLabelText('Collapse testing module'); + const toggleButton = await screen.getByLabelText(/Expand/); + await userEvent.click(toggleButton); + const content = await screen.findByText('CUSTOM CONTENT WITH DYNAMIC HEIGHT'); const collapse = await screen.getByTestId('collapse'); From cd203fa05a3c66c55bcff857c7886eb32425f81b Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 10:30:39 +0100 Subject: [PATCH 022/111] Use proper type for timeouts --- code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx | 2 +- code/core/src/manager/components/sidebar/TestingModule.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx b/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx index 56bbb75e7d5..8ba6930efb5 100644 --- a/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx +++ b/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx @@ -23,7 +23,7 @@ export function GuidedTour({ const theme = useTheme(); useEffect(() => { - let timeout: NodeJS.Timeout; + let timeout: ReturnType; setStepIndex((current) => { const index = steps.findIndex(({ key }) => key === step); diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index 09256e100dd..4dcb743934e 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -191,7 +191,7 @@ export const TestingModule = ({ }, [isCollapsed]); useEffect(() => { - let timeout: NodeJS.Timeout; + let timeout: ReturnType; const handler = () => { setUpdated(true); timeout = setTimeout(setUpdated, 1000, false); From c4071cf10935b7c1c93849c8a8d779699a69a7a7 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 11:06:33 +0100 Subject: [PATCH 023/111] Show 'Settings updated' when changing config, and don't update config if it's equal to the previous value --- .../test/src/components/Description.tsx | 21 ++++++++++++++++--- .../src/components/TestProviderRender.tsx | 9 ++++++-- .../modules/experimental_testmodule.ts | 1 - 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/code/addons/test/src/components/Description.tsx b/code/addons/test/src/components/Description.tsx index aa7365a007e..3f125797266 100644 --- a/code/addons/test/src/components/Description.tsx +++ b/code/addons/test/src/components/Description.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useEffect } from 'react'; import { Link as LinkComponent } from 'storybook/internal/components'; import { type TestProviderConfig, type TestProviderState } from 'storybook/internal/core-events'; @@ -11,6 +11,10 @@ export const DescriptionStyle = styled.div(({ theme }) => ({ color: theme.barTextColor, })); +const PositiveText = styled.span(({ theme }) => ({ + color: theme.color.positiveText, +})); + export function Description({ errorMessage, setIsModalOpen, @@ -20,9 +24,20 @@ export function Description({ errorMessage: string; setIsModalOpen: React.Dispatch>; }) { - let description: string | React.ReactNode = 'Not run'; + const [isUpdated, setUpdated] = React.useState(false); - if (state.running) { + useEffect(() => { + setUpdated(true); + const timeout = setTimeout(setUpdated, 2000, false); + return () => { + clearTimeout(timeout); + }; + }, [state.config]); + + let description: string | React.ReactNode = 'Not run'; + if (isUpdated) { + description = Settings updated; + } else if (state.running) { description = state.progress ? `Testing... ${state.progress.numPassedTests}/${state.progress.numTotalTests}` : 'Starting...'; diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index e2ff8c8cbf2..7f085ac2e43 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -19,6 +19,7 @@ import { StopAltHollowIcon, } from '@storybook/icons'; +import { isEqual } from 'es-toolkit'; import { debounce } from 'es-toolkit/compat'; import { type Config, type Details, TEST_PROVIDER_ID } from '../constants'; @@ -204,11 +205,15 @@ export const TestProviderRender: FC<{ function useConfig(api: API, providerId: string, initialConfig: Config) { const [currentConfig, setConfig] = useState(initialConfig); + const lastConfig = useRef(initialConfig); const saveConfig = useCallback( debounce((config: Config) => { - api.updateTestProviderState(providerId, { config }); - api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); + if (!isEqual(config, lastConfig.current)) { + api.updateTestProviderState(providerId, { config }); + api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); + lastConfig.current = config; + } }, 500), [api, providerId] ); diff --git a/code/core/src/manager-api/modules/experimental_testmodule.ts b/code/core/src/manager-api/modules/experimental_testmodule.ts index a6a0eff1d37..52cf4f5042c 100644 --- a/code/core/src/manager-api/modules/experimental_testmodule.ts +++ b/code/core/src/manager-api/modules/experimental_testmodule.ts @@ -20,7 +20,6 @@ export type SubState = { }; const initialTestProviderState: TestProviderState = { - config: {} as { [key: string]: any }, details: {} as { [key: string]: any }, cancellable: false, cancelling: false, From 50c9292341966f6585ab0ece32b04d2e58895fb8 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 26 Nov 2024 11:22:08 +0100 Subject: [PATCH 024/111] 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 eb957f2749b..0ea3779b495 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 84b6a1d7d868697b9122737f11e44f82ff69fa0f Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 11:34:29 +0100 Subject: [PATCH 025/111] Fix stories --- .../components/TestProviderRender.stories.tsx | 2 +- .../sidebar/TestingModule.stories.tsx | 28 ++++--------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.stories.tsx b/code/addons/test/src/components/TestProviderRender.stories.tsx index 522060b4707..13a9e9bd3f3 100644 --- a/code/addons/test/src/components/TestProviderRender.stories.tsx +++ b/code/addons/test/src/components/TestProviderRender.stories.tsx @@ -158,6 +158,6 @@ export const EnableEditing: Story = { play: async ({ canvasElement }) => { const screen = within(canvasElement); - screen.getByLabelText('Edit').click(); + screen.getByLabelText(/Open settings/).click(); }, }; diff --git a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx index 3d16a655015..66fd53c488a 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx @@ -31,13 +31,8 @@ const testProviders: TestProviders[keyof TestProviders][] = [ type: Addon_TypesEnum.experimental_TEST_PROVIDER, id: 'component-tests', name: 'Component tests', - render: () => ( - - Component tests -
- Ran 2 seconds ago -
- ), + title: () => 'Component tests', + description: () => 'Ran 2 seconds ago', runnable: true, watchable: true, ...baseState, @@ -46,13 +41,8 @@ const testProviders: TestProviders[keyof TestProviders][] = [ type: Addon_TypesEnum.experimental_TEST_PROVIDER, id: 'visual-tests', name: 'Visual tests', - render: () => ( - - Visual tests -
- Not run -
- ), + title: () => 'Visual tests', + description: () => 'Not run', runnable: true, ...baseState, }, @@ -60,13 +50,7 @@ const testProviders: TestProviders[keyof TestProviders][] = [ type: Addon_TypesEnum.experimental_TEST_PROVIDER, id: 'linting', name: 'Linting', - render: () => ( - - Linting -
- Watching for changes -
- ), + render: () => Custom render function, ...baseState, watching: true, }, @@ -163,7 +147,6 @@ export const BothActive: Story = { export const CollapsedStatuses: Story = { args: Statuses.args, - play: Expanded.play, }; export const Running: Story = { @@ -182,7 +165,6 @@ export const RunningAll: Story = { export const CollapsedRunning: Story = { args: RunningAll.args, - play: Expanded.play, }; export const Cancellable: Story = { From dcd84836a9740544d150fc3bc57efffaeb2ff1c0 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 26 Nov 2024 13:40:56 +0100 Subject: [PATCH 026/111] 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 838594e212a..bd0637e9870 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 cedee369516..16968b9dcd6 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 0f002730794..e240eb6889c 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 57cd12ec3e1..6f5ae58437e 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 17fef036476..15b8238c824 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 027/111] 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 e240eb6889c..b35239adda9 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 6f5ae58437e..e470e9f2a60 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 80edea66aa6..b02a07d966c 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 155741ec6c7b13e20a28ae18b59342367d6f61b8 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 14:48:52 +0100 Subject: [PATCH 028/111] Add aria labels --- .../test/src/components/TestProviderRender.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 7f085ac2e43..bf665ca191c 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -178,13 +178,20 @@ export const TestProviderRender: FC<{ ) : ( - } /> + } + /> } + icon={} right={`60%`} /> - } right={73} /> + } + right={73} + /> )} From 2f3ec95a46941f36479cc1e621e4edbe12e44651 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 15:06:19 +0100 Subject: [PATCH 029/111] Disable coverage and a11y checkboxes for now --- code/addons/test/src/components/TestProviderRender.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index bf665ca191c..544892e651f 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -158,6 +158,7 @@ export const TestProviderRender: FC<{ right={ updateConfig({ coverage: !config.coverage })} /> @@ -170,6 +171,7 @@ export const TestProviderRender: FC<{ right={ updateConfig({ a11y: !config.a11y })} /> From 19ec3f63270917a3752c99dddd25e2bf6006034b Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Tue, 26 Nov 2024 15:49:11 +0100 Subject: [PATCH 030/111] 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 b35239adda9..9c23d8c852b 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 e470e9f2a60..8329b091e70 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 031/111] 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 a6a0eff1d37..e96680a28f2 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 032/111] 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 9e753447277..2a1f66ef56e 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 bd0637e9870..db08e9e8699 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 f0f2a4c5bd7a05956b37c2b7cd779f4b3d789a98 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 27 Nov 2024 01:32:23 +0800 Subject: [PATCH 033/111] Fix check --- .../src/manager/components/sidebar/__tests__/Sidebar.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx b/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx index 2e1583fdcf8..0840be5fcd7 100644 --- a/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx +++ b/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx @@ -50,6 +50,7 @@ const generateStories = ({ title, refId }: { title: string; refId?: string }): A name: root, children: [componentId], startCollapsed: false, + tags: [], }, { type: 'component', From 7a1e4b92274872c158b1164d31fdc7a388189121 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 26 Nov 2024 22:56:30 +0100 Subject: [PATCH 034/111] 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 0771604861d..2f6ae8d70e7 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 9c23d8c852b..ffe962ff0ae 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 62491677205..369f1859d58 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 8329b091e70..20a249406c9 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 5ba46113b7a..7a962bc39b8 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 a06fb3df344090a3d4391d7df218c80cb0af66b2 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 09:32:57 +0100 Subject: [PATCH 035/111] Small refactor --- code/addons/test/src/manager.tsx | 34 ++++++++++++++------------------ 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index ccfef2d4d42..a82327a5dfe 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -56,26 +56,22 @@ addons.register(ADDON_ID, (api) => { api.experimental_updateStatus( TEST_PROVIDER_ID, Object.fromEntries( - update.details.testResults.flatMap((testResult) => + state.details.testResults.flatMap((testResult) => testResult.results - .map(({ storyId, status, testRunId, ...rest }) => { - if (storyId) { - const statusObject: API_StatusObject = { - title: 'Component tests', - status: statusMap[status], - description: - 'failureMessages' in rest && rest.failureMessages?.length - ? rest.failureMessages.join('\n') - : '', - data: { - testRunId, - }, - onClick: openAddonPanel, - }; - return [storyId, statusObject]; - } - }) - .filter(Boolean) + .filter(({ storyId }) => storyId) + .map(({ storyId, status, testRunId, ...rest }) => [ + storyId, + { + title: 'Component tests', + status: statusMap[status], + description: + 'failureMessages' in rest && rest.failureMessages + ? rest.failureMessages.join('\n') + : '', + data: { testRunId }, + onClick: openAddonPanel, + } as API_StatusObject, + ]) ) ) ); From 9954cd1ca1adf8a37a1bfa445b315ba3ab89b063 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 10:35:43 +0100 Subject: [PATCH 036/111] Collapsed by default --- code/core/src/manager/components/sidebar/TestingModule.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index 09604461ec8..b7d5536ce20 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -169,7 +169,7 @@ export const TestingModule = ({ const contentRef = useRef(null); const [maxHeight, setMaxHeight] = useState(DEFAULT_HEIGHT); const [isUpdated, setUpdated] = useState(false); - const [isCollapsed, setCollapsed] = useState(false); + const [isCollapsed, setCollapsed] = useState(true); const [isChangingCollapse, setChangingCollapse] = useState(false); useEffect(() => { From e2d2e53fe64ca9824a48ecc99efdaab440f55f0b Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 10:54:23 +0100 Subject: [PATCH 037/111] Don't show updated state when mounting --- code/addons/test/src/components/Description.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/code/addons/test/src/components/Description.tsx b/code/addons/test/src/components/Description.tsx index 3f125797266..9e7441aca3d 100644 --- a/code/addons/test/src/components/Description.tsx +++ b/code/addons/test/src/components/Description.tsx @@ -24,14 +24,18 @@ export function Description({ errorMessage: string; setIsModalOpen: React.Dispatch>; }) { + const isMounted = React.useRef(false); const [isUpdated, setUpdated] = React.useState(false); useEffect(() => { - setUpdated(true); - const timeout = setTimeout(setUpdated, 2000, false); - return () => { - clearTimeout(timeout); - }; + if (isMounted.current) { + setUpdated(true); + const timeout = setTimeout(setUpdated, 2000, false); + return () => { + clearTimeout(timeout); + }; + } + isMounted.current = true; }, [state.config]); let description: string | React.ReactNode = 'Not run'; From a74f5115845b21e4ca2e82b4430595d285c07052 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 11:03:30 +0100 Subject: [PATCH 038/111] Only apply hover styling to labels that contain an enabled input --- .../test/src/components/TestProviderRender.tsx | 1 + .../src/components/components/tooltip/ListItem.tsx | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 544892e651f..588523f89ad 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -147,6 +147,7 @@ export const TestProviderRender: FC<{ {isEditing ? ( } right={} diff --git a/code/core/src/components/components/tooltip/ListItem.tsx b/code/core/src/components/components/tooltip/ListItem.tsx index 023914f6f2d..2a93287af50 100644 --- a/code/core/src/components/components/tooltip/ListItem.tsx +++ b/code/core/src/components/components/tooltip/ListItem.tsx @@ -140,8 +140,8 @@ const Item = styled.div( paddingLeft: 10, }, }), - ({ theme, href, onClick, as }) => - (href || onClick || as === 'label') && { + ({ theme, href, onClick }) => + (href || onClick) && { cursor: 'pointer', '&:hover': { background: theme.background.hoverable, @@ -150,6 +150,15 @@ const Item = styled.div( opacity: 1, }, }, + ({ theme, as }) => + as === 'label' && { + '&:has(input:not(:disabled))': { + cursor: 'pointer', + '&:hover': { + background: theme.background.hoverable, + }, + }, + }, ({ disabled }) => disabled && { cursor: 'not-allowed' } ); From bb0fd7ea338a6d3e458771b27d3f1b14a5009b4f Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 27 Nov 2024 11:22:09 +0100 Subject: [PATCH 039/111] temporarily remove react native sandbox --- .circleci/config.yml | 10 +++++----- code/lib/cli-storybook/src/sandbox-templates.ts | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 550dc9e6b99..e2921e6fa20 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -986,22 +986,22 @@ workflows: requires: - build - create-sandboxes: - parallelism: 38 + parallelism: 37 requires: - build # - smoke-test-sandboxes: # disabled for now # requires: # - create-sandboxes - build-sandboxes: - parallelism: 38 + parallelism: 37 requires: - create-sandboxes - chromatic-sandboxes: - parallelism: 35 + parallelism: 34 requires: - build-sandboxes - e2e-production: - parallelism: 33 + parallelism: 32 requires: - build-sandboxes - e2e-dev: @@ -1009,7 +1009,7 @@ workflows: requires: - create-sandboxes - test-runner-production: - parallelism: 33 + parallelism: 32 requires: - build-sandboxes - vitest-integration: diff --git a/code/lib/cli-storybook/src/sandbox-templates.ts b/code/lib/cli-storybook/src/sandbox-templates.ts index fe89f9decae..1a29411a093 100644 --- a/code/lib/cli-storybook/src/sandbox-templates.ts +++ b/code/lib/cli-storybook/src/sandbox-templates.ts @@ -853,7 +853,8 @@ export const daily: TemplateKey[] = [ 'html-vite/default-js', 'internal/react16-webpack', 'internal/react18-webpack-babel', - 'react-native-web-vite/expo-ts', + // TODO: bring this back once ts-docgen supports Vite 6 + // 'react-native-web-vite/expo-ts', // 'react-native-web-vite/rn-cli-ts', ]; From d4ecd0d9a992bb602c317a6dbc37d922904ef74a Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 27 Nov 2024 11:32:15 +0100 Subject: [PATCH 040/111] Revert "temporarily remove react native sandbox" This reverts commit bb0fd7ea338a6d3e458771b27d3f1b14a5009b4f. --- .circleci/config.yml | 10 +++++----- code/lib/cli-storybook/src/sandbox-templates.ts | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e2921e6fa20..550dc9e6b99 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -986,22 +986,22 @@ workflows: requires: - build - create-sandboxes: - parallelism: 37 + parallelism: 38 requires: - build # - smoke-test-sandboxes: # disabled for now # requires: # - create-sandboxes - build-sandboxes: - parallelism: 37 + parallelism: 38 requires: - create-sandboxes - chromatic-sandboxes: - parallelism: 34 + parallelism: 35 requires: - build-sandboxes - e2e-production: - parallelism: 32 + parallelism: 33 requires: - build-sandboxes - e2e-dev: @@ -1009,7 +1009,7 @@ workflows: requires: - create-sandboxes - test-runner-production: - parallelism: 32 + parallelism: 33 requires: - build-sandboxes - vitest-integration: diff --git a/code/lib/cli-storybook/src/sandbox-templates.ts b/code/lib/cli-storybook/src/sandbox-templates.ts index 1a29411a093..fe89f9decae 100644 --- a/code/lib/cli-storybook/src/sandbox-templates.ts +++ b/code/lib/cli-storybook/src/sandbox-templates.ts @@ -853,8 +853,7 @@ export const daily: TemplateKey[] = [ 'html-vite/default-js', 'internal/react16-webpack', 'internal/react18-webpack-babel', - // TODO: bring this back once ts-docgen supports Vite 6 - // 'react-native-web-vite/expo-ts', + 'react-native-web-vite/expo-ts', // 'react-native-web-vite/rn-cli-ts', ]; From e68fd8b59b6cc07224cb49bbaae277bc19d0913b Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 11:38:14 +0100 Subject: [PATCH 041/111] Fix E2E tests for Testing Module --- .../react/e2e-tests/component-testing.spec.ts | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts b/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts index 2b60a9314d5..68276fdb1a2 100644 --- a/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts +++ b/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts @@ -49,11 +49,14 @@ test.describe("component testing", () => { await sbPage.navigateToStory("addons/test", "Mismatch Failure"); + const expandButton = await page.getByLabel('Expand testing module') + await expandButton.click(); + // For whatever reason, sometimes it takes longer for the story to load const storyElement = sbPage .getCanvasBodyElement() .getByRole("button", { name: "test" }); - await expect(storyElement).toBeVisible({ timeout: 10000 }); + await expect(storyElement).toBeVisible({ timeout: 30000 }); await sbPage.viewAddonPanel("Component tests"); @@ -65,13 +68,12 @@ test.describe("component testing", () => { if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { testStoryElement.click(); } - + const testingModuleDescription = await page.locator('#testing-module-description'); await expect(testingModuleDescription).toContainText('Not run'); const runTestsButton = await page.getByLabel('Start component tests') - await runTestsButton.click(); await expect(testingModuleDescription).toContainText('Testing', { timeout: 60000 }); @@ -117,14 +119,17 @@ test.describe("component testing", () => { const sbPage = new SbPage(page, expect); await sbPage.navigateToStory("addons/test", "Expected Failure"); - + + const expandButton = await page.getByLabel('Expand testing module') + await expandButton.click(); + // For whatever reason, sometimes it takes longer for the story to load const storyElement = sbPage .getCanvasBodyElement() .getByRole("button", { name: "test" }); - await expect(storyElement).toBeVisible({ timeout: 10000 }); + await expect(storyElement).toBeVisible({ timeout: 30000 }); - await expect(page.locator('#testing-module-title')).toHaveText('Component tests'); + await expect(page.locator('#testing-module-title')).toHaveText('Run local tests'); const testingModuleDescription = await page.locator('#testing-module-description'); @@ -142,7 +147,7 @@ test.describe("component testing", () => { // Wait for test results to appear await expect(testingModuleDescription).toHaveText(/Ran \d+ tests/, { timeout: 30000 }); - + await expect(runTestsButton).toBeEnabled(); await expect(watchModeButton).toBeEnabled(); @@ -186,11 +191,14 @@ test.describe("component testing", () => { const sbPage = new SbPage(page, expect); await sbPage.navigateToStory("addons/test", "Expected Failure"); + const expandButton = await page.getByLabel('Expand testing module') + await expandButton.click(); + // For whatever reason, sometimes it takes longer for the story to load const storyElement = sbPage .getCanvasBodyElement() .getByRole("button", { name: "test" }); - await expect(storyElement).toBeVisible({ timeout: 10000 }); + await expect(storyElement).toBeVisible({ timeout: 30000 }); await page.getByLabel("Enable watch mode for Component tests").click(); From 792c32430e85b16487a8235494bcb2a7f2ae1636 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 12:20:45 +0100 Subject: [PATCH 042/111] Refactor ContextMenuItem to reuse title and description from TestProviderRender --- .../test/src/components/ContextMenuItem.tsx | 38 ++++--------------- .../test/src/components/Description.tsx | 14 ++++--- .../src/components/TestProviderRender.tsx | 22 ++++------- code/addons/test/src/components/Title.tsx | 21 ++++++++++ code/addons/test/src/manager.tsx | 1 - 5 files changed, 44 insertions(+), 52 deletions(-) create mode 100644 code/addons/test/src/components/Title.tsx diff --git a/code/addons/test/src/components/ContextMenuItem.tsx b/code/addons/test/src/components/ContextMenuItem.tsx index 8fa63f84819..5f2b5195726 100644 --- a/code/addons/test/src/components/ContextMenuItem.tsx +++ b/code/addons/test/src/components/ContextMenuItem.tsx @@ -8,21 +8,20 @@ import React, { } from 'react'; import { Button, ListItem } from 'storybook/internal/components'; +import type { TestProviderConfig } from 'storybook/internal/core-events'; import { useStorybookApi } from 'storybook/internal/manager-api'; import { useTheme } from 'storybook/internal/theming'; import { type API_HashEntry, type Addon_TestProviderState } from 'storybook/internal/types'; import { PlayHollowIcon, StopAltHollowIcon } from '@storybook/icons'; -import { TEST_PROVIDER_ID } from '../constants'; -import type { TestResult } from '../node/reporter'; -import { RelativeTime } from './RelativeTime'; +import { type Config, type Details, TEST_PROVIDER_ID } from '../constants'; +import { Description } from './Description'; +import { Title } from './Title'; export const ContextMenuItem: FC<{ context: API_HashEntry; - state: Addon_TestProviderState<{ - testResults: TestResult[]; - }>; + state: TestProviderConfig & Addon_TestProviderState; }> = ({ context, state }) => { const api = useStorybookApi(); const [isDisabled, setDisabled] = useState(false); @@ -51,29 +50,6 @@ export const ContextMenuItem: FC<{ const theme = useTheme(); - const title = state.crashed || state.failed ? 'Component tests failed' : 'Component tests'; - const errorMessage = state.error?.message; - let description: string | React.ReactNode = 'Not run'; - - if (state.running) { - description = state.progress - ? `Testing... ${state.progress.numPassedTests}/${state.progress.numTotalTests}` - : 'Starting...'; - } else if (state.failed && !errorMessage) { - description = ''; - } else if (state.crashed || (state.failed && errorMessage)) { - description = 'An error occured'; - } else if (state.progress?.finishedAt) { - description = ( - - ); - } else if (state.watching) { - description = 'Watching for file changes'; - } - return (
{ @@ -82,8 +58,8 @@ export const ContextMenuItem: FC<{ }} > } + center={ {}} state={state} />} right={