diff --git a/CHANGELOG.prerelease.md b/CHANGELOG.prerelease.md index 5834c70763be..817d5dd0bbf9 100644 --- a/CHANGELOG.prerelease.md +++ b/CHANGELOG.prerelease.md @@ -1,3 +1,12 @@ +## 8.4.0-beta.4 + +- Addon Test: Improve Error Handling - [#29476](https://github.com/storybookjs/storybook/pull/29476), thanks @valentinpalkovic! +- Addon Test: Improve postinstall script - [#29479](https://github.com/storybookjs/storybook/pull/29479), thanks @yannbf! +- Addon Test: Throttle Vitest progress updates more heavily - [#29482](https://github.com/storybookjs/storybook/pull/29482), thanks @ghengeveld! +- CLI: Refactor NPMProxy error parsing logic - [#29459](https://github.com/storybookjs/storybook/pull/29459), thanks @yannbf! +- Core: Track test provider state in sessionStorage - [#29450](https://github.com/storybookjs/storybook/pull/29450), thanks @ghengeveld! +- Dependencies: Upgrade VTA to v3.2.0 to resolve peerDep conflict - [#29461](https://github.com/storybookjs/storybook/pull/29461), thanks @ghengeveld! + ## 8.4.0-beta.3 - Addon Test: Only register testing module in Vite projects - [#29472](https://github.com/storybookjs/storybook/pull/29472), thanks @yannbf! diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index 37be0f132bce..0b558a3d4a20 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -80,7 +80,8 @@ const RelativeTime = ({ timestamp, testCount }: { timestamp: Date; testCount: nu }; addons.register(ADDON_ID, (api) => { - if (globalThis.STORYBOOK_BUILDER?.includes('vite')) { + const storybookBuilder = (globalThis as any).STORYBOOK_BUILDER || ''; + if (storybookBuilder.includes('vite')) { const openAddonPanel = () => { api.setSelectedPanel(PANEL_ID); api.togglePanel(true); @@ -94,10 +95,10 @@ addons.register(ADDON_ID, (api) => { name: 'Component tests', title: ({ crashed, failed }) => crashed || failed ? 'Component tests failed' : 'Component tests', - description: ({ failed, running, watching, progress, crashed, details }) => { + description: ({ failed, running, watching, progress, crashed, error }) => { const [isModalOpen, setIsModalOpen] = useState(false); - const errorMessage = details?.error?.message; + const errorMessage = error?.message; let message: string | React.ReactNode = 'Not run'; @@ -106,7 +107,7 @@ addons.register(ADDON_ID, (api) => { ? `Testing... ${progress.numPassedTests}/${progress.numTotalTests}` : 'Starting...'; } else if (failed && !errorMessage) { - message = 'Component tests failed'; + message = ''; } else if (crashed || (failed && errorMessage)) { message = ( <> @@ -116,7 +117,7 @@ addons.register(ADDON_ID, (api) => { setIsModalOpen(true); }} > - {details?.error?.name || 'View full error'} + {error?.name || 'View full error'} ); @@ -177,7 +178,6 @@ addons.register(ADDON_ID, (api) => { ), } as Addon_TestProviderType<{ testResults: TestResult[]; - error?: { message: string; name: string }; }>); } diff --git a/code/addons/test/src/node/boot-test-runner.ts b/code/addons/test/src/node/boot-test-runner.ts index 6e7fe511947d..032be588b107 100644 --- a/code/addons/test/src/node/boot-test-runner.ts +++ b/code/addons/test/src/node/boot-test-runner.ts @@ -97,11 +97,13 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a killChild(); log(result.message); log(result.error); + // eslint-disable-next-line local-rules/no-uncategorized-errors + const error = new Error(`${result.message}\n${result.error}`); // Reject if the child process reports an error before it's ready if (!ready) { - reject(new Error(`${result.message}\n${result.error}`)); + reject(error); } else { - reportFatalError(result.error); + reportFatalError(error); } } else { channel.emit(result.type, ...result.args); diff --git a/code/addons/test/src/node/reporter.ts b/code/addons/test/src/node/reporter.ts index cc55ae75a5fb..833ab0bf3823 100644 --- a/code/addons/test/src/node/reporter.ts +++ b/code/addons/test/src/node/reporter.ts @@ -65,7 +65,7 @@ export class StorybookReporter implements Reporter { sendReport: (payload: TestingModuleProgressReportPayload) => void; constructor(private testManager: TestManager) { - this.sendReport = throttle((payload) => this.testManager.sendProgressReport(payload), 200); + this.sendReport = throttle((payload) => this.testManager.sendProgressReport(payload), 1000); } onInit(ctx: Vitest) { @@ -190,51 +190,55 @@ export class StorybookReporter implements Reporter { async onFinished() { const unhandledErrors = this.ctx.state.getUnhandledErrors(); - if (unhandledErrors?.length) { - this.testManager.reportFatalError( - `Vitest caught ${unhandledErrors.length} unhandled error${unhandledErrors?.length > 1 ? 's' : ''} during the test run.`, - unhandledErrors[0] - ); - } else { - const isCancelled = this.ctx.isCancelling; - const report = this.getProgressReport(Date.now()); + const isCancelled = this.ctx.isCancelling; + const report = this.getProgressReport(Date.now()); - const testSuiteFailures = report.details.testResults.filter( - (t) => t.status === 'failed' && t.results.length === 0 - ); + const testSuiteFailures = report.details.testResults.filter( + (t) => t.status === 'failed' && t.results.length === 0 + ); + + const reducedTestSuiteFailures = new Set(); - const reducedTestSuiteFailures = new Set(); + testSuiteFailures.forEach((t) => { + reducedTestSuiteFailures.add(t.message); + }); - testSuiteFailures.forEach((t) => { - reducedTestSuiteFailures.add(t.message); + if (isCancelled) { + this.sendReport({ + providerId: TEST_PROVIDER_ID, + status: 'cancelled', + ...report, }); + } else if (reducedTestSuiteFailures.size > 0 || unhandledErrors.length > 0) { + const error = + reducedTestSuiteFailures.size > 0 + ? { + name: `${reducedTestSuiteFailures.size} component ${reducedTestSuiteFailures.size === 1 ? 'test' : 'tests'} failed`, + message: Array.from(reducedTestSuiteFailures).reduce( + (acc, curr) => `${acc}\n${curr}`, + '' + ), + } + : { + name: `${unhandledErrors.length} unhandled error${unhandledErrors?.length > 1 ? 's' : ''}`, + message: unhandledErrors + .map((e, index) => `[${index}]: ${(e as any).stack || (e as any).message}`) + .join('\n----------\n'), + }; - if (isCancelled) { - this.sendReport({ - providerId: TEST_PROVIDER_ID, - status: 'cancelled', - ...report, - }); - } else if (reducedTestSuiteFailures.size > 0) { - const message = Array.from(reducedTestSuiteFailures).reduce( - (acc, curr) => `${acc}\n${curr}`, - '' - ); - this.sendReport({ - providerId: TEST_PROVIDER_ID, - status: 'failed', - error: { - name: `${reducedTestSuiteFailures.size} component ${reducedTestSuiteFailures.size === 1 ? 'test' : 'tests'} failed`, - message: message, - }, - }); - } else { - this.sendReport({ - providerId: TEST_PROVIDER_ID, - status: 'success', - ...report, - }); - } + this.sendReport({ + providerId: TEST_PROVIDER_ID, + status: 'failed', + details: report.details, + progress: report.progress, + error, + }); + } else { + this.sendReport({ + providerId: TEST_PROVIDER_ID, + status: 'success', + ...report, + }); } this.clearVitestState(); diff --git a/code/addons/test/src/postinstall.ts b/code/addons/test/src/postinstall.ts index e179b42fa1d2..78aaf0986df8 100644 --- a/code/addons/test/src/postinstall.ts +++ b/code/addons/test/src/postinstall.ts @@ -53,43 +53,6 @@ export default async function postInstall(options: PostinstallOptions) { // if Vitest is installed, we use the same version to keep consistency across Vitest packages const vitestVersionToInstall = vitestVersionSpecifier ?? 'latest'; - const addonInteractionsName = '@storybook/addon-interactions'; - const interactionsAddon = info.addons.find((addon: string | { name: string }) => { - // account for addons as objects, as well as addons with PnP paths - const addonName = typeof addon === 'string' ? addon : addon.name; - return addonName.includes(addonInteractionsName); - }); - - if (!!interactionsAddon) { - let shouldUninstall = options.yes; - if (!options.yes) { - logger.plain(dedent` - We have detected that you're using ${addonInteractionsName}. - The Storybook test addon is a replacement for the interactions addon, so you must uninstall and unregister it in order to use the test addon correctly. This can be done automatically. - - More info: ${picocolors.yellow('https://storybook.js.org/docs/writing-tests/test-addon')} - `); - - const response = await prompts({ - type: 'confirm', - name: 'shouldUninstall', - message: `Would you like me to remove and unregister ${addonInteractionsName}?`, - initial: true, - }); - - shouldUninstall = response.shouldUninstall; - } - - if (shouldUninstall) { - await execa( - packageManager.getRemoteRunCommand(), - ['storybook', 'remove', addonInteractionsName, '--package-manager', options.packageManager], - { - shell: true, - } - ); - } - } const annotationsImport = [ '@storybook/nextjs', '@storybook/experimental-nextjs-vite', @@ -122,9 +85,9 @@ export default async function postInstall(options: PostinstallOptions) { `); } - if (coercedVitestVersion && !satisfies(coercedVitestVersion, '>=2.0.0')) { - reasons.push(` - • The addon requires Vitest 2.0.0 or later. You are currently using ${vitestVersionSpecifier}. + if (coercedVitestVersion && !satisfies(coercedVitestVersion, '>=2.1.0')) { + reasons.push(dedent` + • The addon requires Vitest 2.1.0 or later. You are currently using ${picocolors.bold(vitestVersionSpecifier)}. Please update your ${picocolors.bold(colors.pink('vitest'))} dependency and try again. `); } @@ -145,7 +108,7 @@ export default async function postInstall(options: PostinstallOptions) { ); reasons.push( dedent` - To roll back the installation, remove ${picocolors.bold(colors.pink(ADDON_NAME))} from the "addons" array + You can fix these issues and rerun the command to reinstall. If you wish to roll back the installation, remove ${picocolors.bold(colors.pink(ADDON_NAME))} from the "addons" array in your main Storybook config file and remove the dependency from your package.json file. ` ); @@ -180,6 +143,57 @@ export default async function postInstall(options: PostinstallOptions) { return; } + const addonInteractionsName = '@storybook/addon-interactions'; + const interactionsAddon = info.addons.find((addon: string | { name: string }) => { + // account for addons as objects, as well as addons with PnP paths + const addonName = typeof addon === 'string' ? addon : addon.name; + return addonName.includes(addonInteractionsName); + }); + + if (!!interactionsAddon) { + let shouldUninstall = options.yes; + if (!options.yes) { + printInfo( + '⚠️ Attention', + dedent` + We have detected that you're using ${addonInteractionsName}. + The Storybook test addon is a replacement for the interactions addon, so you must uninstall and unregister it in order to use the test addon correctly. This can be done automatically. + + More info: ${picocolors.cyan('https://storybook.js.org/docs/writing-tests/test-addon')} + ` + ); + + const response = await prompts({ + type: 'confirm', + name: 'shouldUninstall', + message: `Would you like me to remove and unregister ${addonInteractionsName}? Press N to abort the entire installation.`, + initial: true, + }); + + shouldUninstall = response.shouldUninstall; + } + + if (shouldUninstall) { + await execa( + packageManager.getRemoteRunCommand(), + [ + 'storybook', + 'remove', + addonInteractionsName, + '--package-manager', + options.packageManager, + '--config-dir', + options.configDir, + ], + { + shell: true, + } + ); + } else { + return; + } + } + const vitestInfo = getVitestPluginInfo(info.frameworkPackageName); if (info.frameworkPackageName === '@storybook/nextjs') { @@ -317,6 +331,8 @@ export default async function postInstall(options: PostinstallOptions) { // If there's an existing config, we create a workspace file so we can run Storybook tests alongside. const extname = path.extname(rootConfig); const browserWorkspaceFile = path.resolve(dirname(rootConfig), `vitest.workspace${extname}`); + // to be set in vitest config + const vitestSetupFilePath = path.relative(path.dirname(browserWorkspaceFile), vitestSetupFile); logger.line(1); logger.plain(`${step} Creating a Vitest project workspace file:`); @@ -335,7 +351,7 @@ export default async function postInstall(options: PostinstallOptions) { extends: '${viteConfigFile ? relative(dirname(browserWorkspaceFile), viteConfigFile) : ''}', plugins: [ // See options at: https://storybook.js.org/docs/writing-tests/vitest-plugin#storybooktest - storybookTest(),${vitestInfo.frameworkPluginDocs + vitestInfo.frameworkPluginCall} + storybookTest({ configDir: '${options.configDir}' }),${vitestInfo.frameworkPluginDocs + vitestInfo.frameworkPluginCall} ], test: { name: 'storybook', @@ -347,7 +363,7 @@ export default async function postInstall(options: PostinstallOptions) { }, // Make sure to adjust this pattern to match your stories files. include: ['**/*.stories.?(m)[jt]s?(x)'], - setupFiles: ['./.storybook/vitest.setup.ts'], + setupFiles: ['${vitestSetupFilePath}'], }, }, ]); @@ -356,6 +372,8 @@ export default async function postInstall(options: PostinstallOptions) { } else { // If there's no existing Vitest/Vite config, we create a new Vitest config file. const newVitestConfigFile = path.resolve('vitest.config.ts'); + // to be set in vitest config + const vitestSetupFilePath = path.relative(path.dirname(newVitestConfigFile), vitestSetupFile); logger.line(1); logger.plain(`${step} Creating a Vitest project config file:`); @@ -371,7 +389,7 @@ export default async function postInstall(options: PostinstallOptions) { export default defineConfig({ plugins: [ // See options at: https://storybook.js.org/docs/writing-tests/vitest-plugin#storybooktest - storybookTest(),${vitestInfo.frameworkPluginDocs + vitestInfo.frameworkPluginCall} + storybookTest({ configDir: '${options.configDir}' }),${vitestInfo.frameworkPluginDocs + vitestInfo.frameworkPluginCall} ], test: { name: 'storybook', @@ -383,7 +401,7 @@ export default async function postInstall(options: PostinstallOptions) { }, // Make sure to adjust this pattern to match your stories files. include: ['**/*.stories.?(m)[jt]s?(x)'], - setupFiles: ['./.storybook/vitest.setup.ts'], + setupFiles: ['${vitestSetupFilePath}'], }, }); ` diff --git a/code/addons/test/src/preset.ts b/code/addons/test/src/preset.ts index 1d4203a3d5a5..685a56e2baf4 100644 --- a/code/addons/test/src/preset.ts +++ b/code/addons/test/src/preset.ts @@ -59,7 +59,6 @@ export const experimental_serverChannel = async (channel: Channel, options: Opti Information on how to upgrade here: ${picocolors.yellow('https://storybook.js.org/docs/get-started/frameworks/nextjs#with-vite')}\n `); } - return channel; } diff --git a/code/addons/test/typings.d.ts b/code/addons/test/typings.d.ts index ec1d9d7d3808..d5de3255b176 100644 --- a/code/addons/test/typings.d.ts +++ b/code/addons/test/typings.d.ts @@ -5,3 +5,5 @@ interface ImportMetaEnv { interface ImportMeta { readonly env: ImportMetaEnv; } + +declare var STORYBOOK_BUILDER: string | undefined; diff --git a/code/core/src/common/js-package-manager/NPMProxy.test.ts b/code/core/src/common/js-package-manager/NPMProxy.test.ts index 4a783017d917..11f34a986bf1 100644 --- a/code/core/src/common/js-package-manager/NPMProxy.test.ts +++ b/code/core/src/common/js-package-manager/NPMProxy.test.ts @@ -428,7 +428,7 @@ describe('NPM Proxy', () => { describe('parseErrors', () => { it('should parse npm errors', () => { - const NPM_RESOLVE_ERROR_SAMPLE = ` + const NPM_LEGACY_RESOLVE_ERROR_SAMPLE = ` npm ERR! npm ERR! code ERESOLVE npm ERR! ERESOLVE unable to resolve dependency tree @@ -439,6 +439,17 @@ describe('NPM Proxy', () => { npm ERR! react@"30" from the root project `; + const NPM_RESOLVE_ERROR_SAMPLE = ` + npm error + npm error code ERESOLVE + npm error ERESOLVE unable to resolve dependency tree + npm error + npm error While resolving: before-storybook@1.0.0 + npm error Found: react@undefined + npm error node_modules/react + npm error react@"30" from the root project + `; + const NPM_TIMEOUT_ERROR_SAMPLE = ` npm notice npm notice New major version of npm available! 8.5.0 -> 9.6.7 @@ -451,6 +462,9 @@ describe('NPM Proxy', () => { npm ERR! network This is a problem related to network connectivity. `; + expect(npmProxy.parseErrorFromLogs(NPM_LEGACY_RESOLVE_ERROR_SAMPLE)).toEqual( + 'NPM error ERESOLVE - Dependency resolution error.' + ); expect(npmProxy.parseErrorFromLogs(NPM_RESOLVE_ERROR_SAMPLE)).toEqual( 'NPM error ERESOLVE - Dependency resolution error.' ); diff --git a/code/core/src/common/js-package-manager/NPMProxy.ts b/code/core/src/common/js-package-manager/NPMProxy.ts index 90378de122cf..ea673f9df301 100644 --- a/code/core/src/common/js-package-manager/NPMProxy.ts +++ b/code/core/src/common/js-package-manager/NPMProxy.ts @@ -28,8 +28,8 @@ type NpmDependencies = { export type NpmListOutput = { dependencies: NpmDependencies; }; +const NPM_ERROR_REGEX = /npm (ERR!|error) (code|errno) (\w+)/i; -const NPM_ERROR_REGEX = /npm ERR! code (\w+)/; const NPM_ERROR_CODES = { E401: 'Authentication failed or is required.', E403: 'Access to the resource is forbidden.', @@ -320,7 +320,7 @@ export class NPMProxy extends JsPackageManager { const match = logs.match(NPM_ERROR_REGEX); if (match) { - const errorCode = match[1] as keyof typeof NPM_ERROR_CODES; + const errorCode = match[3] as keyof typeof NPM_ERROR_CODES; if (errorCode) { finalMessage = `${finalMessage} ${errorCode}`; } diff --git a/code/core/src/core-events/data/testing-module.ts b/code/core/src/core-events/data/testing-module.ts index fae5446f6e3a..d9e9640935a1 100644 --- a/code/core/src/core-events/data/testing-module.ts +++ b/code/core/src/core-events/data/testing-module.ts @@ -37,6 +37,8 @@ export type TestingModuleProgressReportPayload = | { providerId: TestProviderId; status: 'failed'; + progress?: TestingModuleProgressReportProgress; + details?: { [key: string]: any }; error: { name: string; message: string; diff --git a/code/core/src/manager/components/sidebar/SidebarBottom.tsx b/code/core/src/manager/components/sidebar/SidebarBottom.tsx index 48bf60a5f095..b25defd9c4bd 100644 --- a/code/core/src/manager/components/sidebar/SidebarBottom.tsx +++ b/code/core/src/manager/components/sidebar/SidebarBottom.tsx @@ -30,6 +30,8 @@ const SIDEBAR_BOTTOM_SPACER_ID = 'sidebar-bottom-spacer'; // This ID is used by some integrators to target the (fixed position) sidebar bottom element so it should remain stable. const SIDEBAR_BOTTOM_WRAPPER_ID = 'sidebar-bottom-wrapper'; +const STORAGE_KEY = '@storybook/manager/test-providers'; + const initialTestProviderState: TestProviderState = { details: {} as { [key: string]: any }, cancellable: false, @@ -97,13 +99,17 @@ export const SidebarBottomBase = ({ api, notifications = [], status = {} }: Side const wrapperRef = useRef(null); const [warningsActive, setWarningsActive] = useState(false); const [errorsActive, setErrorsActive] = useState(false); - const [testProviders, setTestProviders] = useState(() => - Object.fromEntries( + const [testProviders, setTestProviders] = useState(() => { + let sessionState: TestProviders = {}; + try { + sessionState = JSON.parse(sessionStorage.getItem(STORAGE_KEY) || '{}'); + } catch (_) {} + return Object.fromEntries( Object.entries(api.getElements(Addon_TypesEnum.experimental_TEST_PROVIDER)).map( - ([id, config]) => [id, { ...config, ...initialTestProviderState }] + ([id, config]) => [id, { ...config, ...initialTestProviderState, ...sessionState[id] }] ) - ) - ); + ); + }); const warnings = Object.values(status).filter((statusByAddonId) => Object.values(statusByAddonId).some((value) => value?.status === 'warn') @@ -116,25 +122,28 @@ export const SidebarBottomBase = ({ api, notifications = [], status = {} }: Side const updateTestProvider = useCallback( (id: TestProviderId, update: Partial) => - setTestProviders((state) => ({ ...state, [id]: { ...state[id], ...update } })), + setTestProviders((state) => { + const newValue = { ...state, [id]: { ...state[id], ...update } }; + sessionStorage.setItem(STORAGE_KEY, JSON.stringify(newValue)); + return newValue; + }), [] ); const clearState = useCallback( - ({ providerId: id }: { providerId: TestProviderId }) => { - const startingState: Partial = { + ({ providerId }: { providerId: TestProviderId }) => { + updateTestProvider(providerId, { cancelling: false, running: true, failed: false, crashed: false, progress: undefined, - }; - setTestProviders((state) => ({ ...state, [id]: { ...state[id], ...startingState } })); - api.experimental_updateStatus(id, (state = {}) => + }); + api.experimental_updateStatus(providerId, (state = {}) => Object.fromEntries(Object.keys(state).map((key) => [key, null])) ); }, - [api] + [api, updateTestProvider] ); const onRunTests = useCallback( @@ -184,11 +193,11 @@ export const SidebarBottomBase = ({ api, notifications = [], status = {} }: Side updateTestProvider(providerId, { details, running: false, crashed: true, watching: false }); }; - const onProgressReport = ({ providerId, ...details }: TestingModuleProgressReportPayload) => { - if (details.status === 'failed') { - updateTestProvider(providerId, { details, running: false, failed: true }); + const onProgressReport = ({ providerId, ...result }: TestingModuleProgressReportPayload) => { + if (result.status === 'failed') { + updateTestProvider(providerId, { ...result, running: false, failed: true }); } else { - const update = { ...details, running: details.status === 'pending' }; + const update = { ...result, running: result.status === 'pending' }; updateTestProvider(providerId, update); const { mapStatusUpdate, ...state } = testProviders[providerId]; diff --git a/code/core/src/types/modules/addons.ts b/code/core/src/types/modules/addons.ts index 61473d42958c..2c995322b09d 100644 --- a/code/core/src/types/modules/addons.ts +++ b/code/core/src/types/modules/addons.ts @@ -474,7 +474,9 @@ export interface Addon_TestProviderType< name: string; title: (state: Addon_TestProviderState
) => ReactNode; description: (state: Addon_TestProviderState
) => ReactNode; - mapStatusUpdate?: (state: Addon_TestProviderState
) => API_StatusUpdate; + mapStatusUpdate?: ( + state: Addon_TestProviderState
+ ) => API_StatusUpdate | ((state: API_StatusState) => API_StatusUpdate); runnable?: boolean; watchable?: boolean; } @@ -489,6 +491,10 @@ export type Addon_TestProviderState