From 239c066d89bf7067051b47b2fcfa4d9513a5cb22 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 13 Nov 2024 15:51:45 +0100 Subject: [PATCH 1/2] Core: Add Reporting API --- .../components/InteractionsPanel.stories.tsx | 11 ++++++++ code/addons/test/src/node/reporter.ts | 17 +++++++++++-- .../test/src/vitest-plugin/test-utils.ts | 8 ++++-- code/core/src/core-events/index.ts | 2 ++ code/core/src/manager/globals/exports.ts | 3 +++ code/core/src/preview-api/index.ts | 2 +- .../preview-web/render/StoryRender.test.ts | 6 +++-- .../modules/preview-web/render/StoryRender.ts | 25 ++++++++++++++++++- .../preview-api/modules/store/StoryStore.ts | 3 +++ .../modules/store/csf/portable-stories.ts | 5 ++++ .../src/preview-api/modules/store/index.ts | 1 + .../preview-api/modules/store/reporter-api.ts | 14 +++++++++++ code/core/src/types/modules/composedStory.ts | 2 ++ code/package.json | 2 +- code/yarn.lock | 19 ++++++++++---- 15 files changed, 106 insertions(+), 14 deletions(-) create mode 100644 code/core/src/preview-api/modules/store/reporter-api.ts diff --git a/code/addons/interactions/src/components/InteractionsPanel.stories.tsx b/code/addons/interactions/src/components/InteractionsPanel.stories.tsx index b9273b5a04f7..d228b177a814 100644 --- a/code/addons/interactions/src/components/InteractionsPanel.stories.tsx +++ b/code/addons/interactions/src/components/InteractionsPanel.stories.tsx @@ -56,6 +56,17 @@ export default meta; type Story = StoryObj; export const Passing: Story = { + // TODO: Remove after prototyping + beforeEach: async ({ reporting }) => { + reporting.addReport({ + id: 'a11y', + status: 'passed', + version: 1, + result: { + violations: [{ id: 'a11y', impact: 'critical', description: 'A11y violation' }], + }, + }); + }, args: { interactions: getInteractions(CallStates.DONE), }, diff --git a/code/addons/test/src/node/reporter.ts b/code/addons/test/src/node/reporter.ts index 833ab0bf3823..e83d2ee3f549 100644 --- a/code/addons/test/src/node/reporter.ts +++ b/code/addons/test/src/node/reporter.ts @@ -6,6 +6,7 @@ import type { TestingModuleProgressReportPayload, TestingModuleProgressReportProgress, } from 'storybook/internal/core-events'; +import type { Report } from 'storybook/internal/preview-api'; import type { API_StatusUpdate } from '@storybook/types'; @@ -26,6 +27,7 @@ export type TestResultResult = storyId: string; testRunId: string; duration: number; + reports: Report[]; } | { status: 'failed'; @@ -33,6 +35,7 @@ export type TestResultResult = duration: number; testRunId: string; failureMessages: string[]; + reports: Report[]; }; export type TestResult = { @@ -113,16 +116,26 @@ export class StorybookReporter implements Reporter { const status = StatusMap[t.result?.state || t.mode] || 'skipped'; const storyId = (t.meta as any).storyId as string; + const reports = (t.meta as any).reports as Report[]; const duration = t.result?.duration || 0; const testRunId = this.start.toString(); switch (status) { case 'passed': case 'pending': - return [{ status, storyId, duration, testRunId } as TestResultResult]; + return [{ status, storyId, duration, testRunId, reports } as TestResultResult]; case 'failed': const failureMessages = t.result?.errors?.map((e) => e.stack || e.message) || []; - return [{ status, storyId, duration, failureMessages, testRunId } as TestResultResult]; + return [ + { + status, + storyId, + duration, + failureMessages, + testRunId, + reports, + } as TestResultResult, + ]; default: return []; } diff --git a/code/addons/test/src/vitest-plugin/test-utils.ts b/code/addons/test/src/vitest-plugin/test-utils.ts index cdd199d3998b..9ac6fbb487ea 100644 --- a/code/addons/test/src/vitest-plugin/test-utils.ts +++ b/code/addons/test/src/vitest-plugin/test-utils.ts @@ -3,7 +3,7 @@ /* eslint-disable no-underscore-dangle */ import { type RunnerTask, type TaskContext, type TaskMeta, type TestContext } from 'vitest'; -import { composeStory } from 'storybook/internal/preview-api'; +import { type Report, composeStory } from 'storybook/internal/preview-api'; import type { ComponentAnnotations, ComposedStoryFn } from 'storybook/internal/types'; import { setViewport } from './viewports'; @@ -22,10 +22,14 @@ export const testStory = ( context.story = composedStory; - const _task = context.task as RunnerTask & { meta: TaskMeta & { storyId: string } }; + const _task = context.task as RunnerTask & { + meta: TaskMeta & { storyId: string; reports: Report[] }; + }; _task.meta.storyId = composedStory.id; await setViewport(composedStory.parameters, composedStory.globals); await composedStory.run(); + + _task.meta.reports = composedStory.reporting.reports; }; }; diff --git a/code/core/src/core-events/index.ts b/code/core/src/core-events/index.ts index d5d51eeecdf4..e22b5207c18f 100644 --- a/code/core/src/core-events/index.ts +++ b/code/core/src/core-events/index.ts @@ -32,6 +32,7 @@ enum events { STORY_CHANGED = 'storyChanged', STORY_UNCHANGED = 'storyUnchanged', STORY_RENDERED = 'storyRendered', + STORY_COMPLETED = 'storyCompleted', STORY_MISSING = 'storyMissing', STORY_ERRORED = 'storyErrored', STORY_THREW_EXCEPTION = 'storyThrewException', @@ -140,6 +141,7 @@ export const { STORY_PREPARED, STORY_RENDER_PHASE_CHANGED, STORY_RENDERED, + STORY_COMPLETED, STORY_SPECIFIED, STORY_THREW_EXCEPTION, STORY_UNCHANGED, diff --git a/code/core/src/manager/globals/exports.ts b/code/core/src/manager/globals/exports.ts index b9141e853f95..5516b01c574c 100644 --- a/code/core/src/manager/globals/exports.ts +++ b/code/core/src/manager/globals/exports.ts @@ -797,6 +797,7 @@ export default { 'STORIES_EXPAND_ALL', 'STORY_ARGS_UPDATED', 'STORY_CHANGED', + 'STORY_COMPLETED', 'STORY_ERRORED', 'STORY_INDEX_INVALIDATED', 'STORY_MISSING', @@ -861,6 +862,7 @@ export default { 'STORIES_EXPAND_ALL', 'STORY_ARGS_UPDATED', 'STORY_CHANGED', + 'STORY_COMPLETED', 'STORY_ERRORED', 'STORY_INDEX_INVALIDATED', 'STORY_MISSING', @@ -925,6 +927,7 @@ export default { 'STORIES_EXPAND_ALL', 'STORY_ARGS_UPDATED', 'STORY_CHANGED', + 'STORY_COMPLETED', 'STORY_ERRORED', 'STORY_INDEX_INVALIDATED', 'STORY_MISSING', diff --git a/code/core/src/preview-api/index.ts b/code/core/src/preview-api/index.ts index 0a61c7333ab3..d067acad89cc 100644 --- a/code/core/src/preview-api/index.ts +++ b/code/core/src/preview-api/index.ts @@ -60,6 +60,6 @@ export { createPlaywrightTest } from './modules/store/csf/portable-stories'; export type { PropDescriptor } from './store'; /** STORIES API */ -export { StoryStore } from './store'; +export { StoryStore, type Report, ReporterAPI } from './store'; export { Preview, PreviewWeb, PreviewWithSelection, UrlStore, WebView } from './preview-web'; export type { SelectionStore, View } from './preview-web'; diff --git a/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts b/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts index 2bd3a9ca3830..6ce7c9fc11fd 100644 --- a/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts +++ b/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts @@ -4,7 +4,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { Channel } from '@storybook/core/channels'; import type { PreparedStory, Renderer, StoryContext, StoryIndexEntry } from '@storybook/core/types'; -import type { StoryStore } from '../../store'; +import { ReporterAPI, type StoryStore } from '../../store'; import { PREPARE_ABORTED } from './Render'; import { StoryRender } from './StoryRender'; @@ -48,7 +48,9 @@ const buildStory = (overrides: Partial = {}): PreparedStory => const buildStore = (overrides: Partial> = {}): StoryStore => ({ - getStoryContext: () => ({}), + getStoryContext: () => ({ + reporting: new ReporterAPI(), + }), addCleanupCallbacks: vi.fn(), cleanupStory: vi.fn(), ...overrides, diff --git a/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts b/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts index 3441a5c64e97..895674b6b73f 100644 --- a/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts +++ b/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts @@ -14,6 +14,7 @@ import type { import { PLAY_FUNCTION_THREW_EXCEPTION, + STORY_COMPLETED, STORY_RENDERED, STORY_RENDER_PHASE_CHANGED, UNHANDLED_ERRORS_WHILE_PLAYING, @@ -34,6 +35,7 @@ export type RenderPhase = | 'playing' | 'played' | 'completed' + | 'finished' | 'aborted' | 'errored'; @@ -288,7 +290,7 @@ export class StoryRender implements Render = new Set(); + const unhandledErrors: Set = new Set(); const onError = (event: ErrorEvent | PromiseRejectionEvent) => unhandledErrors.add('error' in event ? event.error : event.reason); @@ -349,9 +351,30 @@ export class StoryRender implements Render this.channel.emit(STORY_RENDERED, id) ); + + // The event name 'completed' is unfortunately already reserved by the STORY_RENDERED event + await this.runPhase(abortSignal, 'finished', async () => + this.channel.emit(STORY_COMPLETED, { + storyId: id, + unhandledExceptions: !ignoreUnhandledErrors + ? Array.from(unhandledErrors).map(serializeError) + : [], + status: !ignoreUnhandledErrors && unhandledErrors.size > 0 ? 'error' : 'success', + reporters: context.reporting.reports, + }) + ); } catch (err) { this.phase = 'errored'; this.callbacks.showException(err as Error); + + await this.runPhase(abortSignal, 'finished', async () => + this.channel.emit(STORY_COMPLETED, { + storyId: id, + unhandledExceptions: [serializeError(err)], + status: 'error', + reporters: [], + }) + ); } // If a rerender was enqueued during the render, clear the queue and render again diff --git a/code/core/src/preview-api/modules/store/StoryStore.ts b/code/core/src/preview-api/modules/store/StoryStore.ts index cd7dc40a6ed1..01404a5f1bf5 100644 --- a/code/core/src/preview-api/modules/store/StoryStore.ts +++ b/code/core/src/preview-api/modules/store/StoryStore.ts @@ -45,6 +45,7 @@ import { prepareStory, processCSFFile, } from './csf'; +import { ReporterAPI } from './reporter-api'; export function picky, K extends keyof T>( obj: T, @@ -253,12 +254,14 @@ export class StoryStore { getStoryContext(story: PreparedStory, { forceInitialArgs = false } = {}) { const userGlobals = this.userGlobals.get(); const { initialGlobals } = this.userGlobals; + const reporting = new ReporterAPI(); return prepareContext({ ...story, args: forceInitialArgs ? story.initialArgs : this.args.get(story.id), initialGlobals, globalTypes: this.projectAnnotations.globalTypes, userGlobals, + reporting, globals: { ...userGlobals, ...story.storyGlobals, diff --git a/code/core/src/preview-api/modules/store/csf/portable-stories.ts b/code/core/src/preview-api/modules/store/csf/portable-stories.ts index b9efd7da9792..a2971bcf5dbd 100644 --- a/code/core/src/preview-api/modules/store/csf/portable-stories.ts +++ b/code/core/src/preview-api/modules/store/csf/portable-stories.ts @@ -26,6 +26,7 @@ import { MountMustBeDestructuredError } from '@storybook/core/preview-errors'; import { dedent } from 'ts-dedent'; import { HooksContext } from '../../../addons'; +import { ReporterAPI } from '../reporter-api'; import { composeConfigs } from './composeConfigs'; import { getValuesFromArgTypes } from './getValuesFromArgTypes'; import { normalizeComponentAnnotations } from './normalizeComponentAnnotations'; @@ -146,12 +147,15 @@ export function composeStory { const context: StoryContext = prepareContext({ hooks: new HooksContext(), globals, args: { ...story.initialArgs }, viewMode: 'story', + reporting, loaded: {}, abortSignal: new AbortController().signal, step: (label, play) => story.runStep(label, play, context), @@ -258,6 +262,7 @@ export function composeStory, play: playFunction!, run, + reporting, tags: story.tags, } ); diff --git a/code/core/src/preview-api/modules/store/index.ts b/code/core/src/preview-api/modules/store/index.ts index f6694ad9017b..ea16e35bc908 100644 --- a/code/core/src/preview-api/modules/store/index.ts +++ b/code/core/src/preview-api/modules/store/index.ts @@ -10,3 +10,4 @@ export * from './decorators'; export * from './args'; export * from './autoTitle'; export * from './sortStories'; +export * from './reporter-api'; diff --git a/code/core/src/preview-api/modules/store/reporter-api.ts b/code/core/src/preview-api/modules/store/reporter-api.ts new file mode 100644 index 000000000000..dc2acc34d1d8 --- /dev/null +++ b/code/core/src/preview-api/modules/store/reporter-api.ts @@ -0,0 +1,14 @@ +export interface Report { + id: string; + version: number; + result: unknown; + status: 'failed' | 'passed' | 'warning'; +} + +export class ReporterAPI { + reports: Report[] = []; + + async addReport(report: Report) { + this.reports.push(report); + } +} diff --git a/code/core/src/types/modules/composedStory.ts b/code/core/src/types/modules/composedStory.ts index 7f8d52add055..c5a58b280cd3 100644 --- a/code/core/src/types/modules/composedStory.ts +++ b/code/core/src/types/modules/composedStory.ts @@ -9,6 +9,7 @@ import type { Tag, } from '@storybook/csf'; +import type { ReporterAPI } from '../../preview-api'; import type { AnnotatedStoryFn, Args, @@ -49,6 +50,7 @@ export type ComposedStoryFn< storyName: string; parameters: Parameters; argTypes: StrictArgTypes; + reporting: ReporterAPI; tags: Tag[]; globals: Globals; }; diff --git a/code/package.json b/code/package.json index 9d90dce863a8..0d27b2879a1e 100644 --- a/code/package.json +++ b/code/package.json @@ -123,7 +123,7 @@ "@storybook/codemod": "workspace:*", "@storybook/core": "workspace:*", "@storybook/core-webpack": "workspace:*", - "@storybook/csf": "0.1.11", + "@storybook/csf": "0.1.12--canary.110.bb5bb77.0", "@storybook/csf-plugin": "workspace:*", "@storybook/ember": "workspace:*", "@storybook/eslint-config-storybook": "^4.0.0", diff --git a/code/yarn.lock b/code/yarn.lock index ab06414c85cc..5547b50456ea 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -6426,12 +6426,12 @@ __metadata: languageName: unknown linkType: soft -"@storybook/csf@npm:0.1.11, @storybook/csf@npm:^0.1.11": - version: 0.1.11 - resolution: "@storybook/csf@npm:0.1.11" +"@storybook/csf@npm:0.1.12--canary.110.bb5bb77.0": + version: 0.1.12--canary.110.bb5bb77.0 + resolution: "@storybook/csf@npm:0.1.12--canary.110.bb5bb77.0" dependencies: type-fest: "npm:^2.19.0" - checksum: 10c0/c5329fc13e7d762049b5c91df1bc1c0e510a1a898c401b72b68f1ff64139a85ab64a92f8e681d2fcb226c0a4a55d0f23b569b2bdb517e0f067bd05ea46228356 + checksum: 10c0/c59ef657b9c0b125722baa1136dbb1d6b0e8bbe1c5b3784c1c579e20164be0c96524a556acaad8e7d9bbd8fcbfd2d324e093874657773231a01b7e49a429cfa3 languageName: node linkType: hard @@ -6444,6 +6444,15 @@ __metadata: languageName: node linkType: hard +"@storybook/csf@npm:^0.1.11": + version: 0.1.11 + resolution: "@storybook/csf@npm:0.1.11" + dependencies: + type-fest: "npm:^2.19.0" + checksum: 10c0/c5329fc13e7d762049b5c91df1bc1c0e510a1a898c401b72b68f1ff64139a85ab64a92f8e681d2fcb226c0a4a55d0f23b569b2bdb517e0f067bd05ea46228356 + languageName: node + linkType: hard + "@storybook/docs-mdx@npm:4.0.0-next.1": version: 4.0.0-next.1 resolution: "@storybook/docs-mdx@npm:4.0.0-next.1" @@ -7120,7 +7129,7 @@ __metadata: "@storybook/codemod": "workspace:*" "@storybook/core": "workspace:*" "@storybook/core-webpack": "workspace:*" - "@storybook/csf": "npm:0.1.11" + "@storybook/csf": "npm:0.1.12--canary.110.bb5bb77.0" "@storybook/csf-plugin": "workspace:*" "@storybook/ember": "workspace:*" "@storybook/eslint-config-storybook": "npm:^4.0.0" From d8c2c7eaf9d02a7784632506db8da5dbee82f65f Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 14 Nov 2024 12:11:10 +0100 Subject: [PATCH 2/2] Add tests --- .../preview-web/render/StoryRender.test.ts | 91 ++++++++++++++++++- .../modules/preview-web/render/StoryRender.ts | 2 +- 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts b/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts index 6ce7c9fc11fd..b156836fde2c 100644 --- a/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts +++ b/code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts @@ -1,12 +1,14 @@ // @vitest-environment happy-dom import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { STORY_COMPLETED } from 'storybook/internal/core-events'; + import { Channel } from '@storybook/core/channels'; import type { PreparedStory, Renderer, StoryContext, StoryIndexEntry } from '@storybook/core/types'; import { ReporterAPI, type StoryStore } from '../../store'; import { PREPARE_ABORTED } from './Render'; -import { StoryRender } from './StoryRender'; +import { StoryRender, serializeError } from './StoryRender'; const entry = { type: 'story', @@ -257,6 +259,93 @@ describe('StoryRender', () => { expect(actualMount).toHaveBeenCalled(); }); + it('should handle the "finished" phase correctly when the story finishes successfully', async () => { + // Arrange - setup StoryRender and async gate blocking finished phase + const [finishGate, resolveFinishGate] = createGate(); + const story = buildStory({ + playFunction: vi.fn(async () => { + await finishGate; + }), + }); + const store = buildStore(); + + const channel = new Channel({}); + const emitSpy = vi.spyOn(channel, 'emit'); + + const render = new StoryRender( + channel, + store, + vi.fn() as any, + {} as any, + entry.id, + 'story', + { autoplay: true }, + story + ); + + // Act - render, resolve finish gate, teardown + render.renderToElement({} as any); + await tick(); // go from 'loading' to 'rendering' phase + resolveFinishGate(); + await tick(); // go from 'rendering' to 'finished' phase + render.teardown(); + + // Assert - ensure finished phase is handled correctly + expect(render.phase).toBe('finished'); + expect(emitSpy).toHaveBeenCalledWith(STORY_COMPLETED, { + reporters: [], + status: 'success', + storyId: 'id', + unhandledExceptions: [], + }); + }); + + it('should handle the "finished" phase correctly when the story throws an error', async () => { + // Arrange - setup StoryRender and async gate blocking finished phase + const [finishGate, rejectFinishGate] = createGate(); + const error = new Error('Test error'); + const story = buildStory({ + parameters: {}, + playFunction: vi.fn(async () => { + await finishGate; + throw error; + }), + }); + const store = buildStore(); + + const channel = new Channel({}); + const emitSpy = vi.spyOn(channel, 'emit'); + + const render = new StoryRender( + channel, + store, + vi.fn() as any, + { + showException: vi.fn(), + } as any, + entry.id, + 'story', + { autoplay: true }, + story + ); + + // Act - render, reject finish gate, teardown + render.renderToElement({} as any); + await tick(); // go from 'loading' to 'rendering' phase + rejectFinishGate(); + await tick(); // go from 'rendering' to 'finished' phase + render.teardown(); + + // Assert - ensure finished phase is handled correctly + expect(render.phase).toBe('finished'); + expect(emitSpy).toHaveBeenCalledWith(STORY_COMPLETED, { + reporters: [], + status: 'error', + storyId: 'id', + unhandledExceptions: [serializeError(error)], + }); + }); + describe('teardown', () => { it('throws PREPARE_ABORTED if torndown during prepare', async () => { const [importGate, openImportGate] = createGate(); diff --git a/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts b/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts index 895674b6b73f..4f34c23c378d 100644 --- a/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts +++ b/code/core/src/preview-api/modules/preview-web/render/StoryRender.ts @@ -39,7 +39,7 @@ export type RenderPhase = | 'aborted' | 'errored'; -function serializeError(error: any) { +export function serializeError(error: any) { try { const { name = 'Error', message = String(error), stack } = error; return { name, message, stack };