From 65dab7de55decd80092a7fa37a4749d8e94e012d Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Fri, 8 May 2026 14:11:16 -0400 Subject: [PATCH 1/7] Refactor calculateFileHashes to return hashes --- .../tasks/prepare/calculateFileHashes.test.ts | 41 ++++++++------- node-src/tasks/prepare/calculateFileHashes.ts | 52 +++++++++---------- node-src/tasks/prepare/index.ts | 44 +++++++++++++++- node-src/types.ts | 23 ++++---- 4 files changed, 102 insertions(+), 58 deletions(-) diff --git a/node-src/tasks/prepare/calculateFileHashes.test.ts b/node-src/tasks/prepare/calculateFileHashes.test.ts index 842cf5403..9420d02eb 100644 --- a/node-src/tasks/prepare/calculateFileHashes.test.ts +++ b/node-src/tasks/prepare/calculateFileHashes.test.ts @@ -1,16 +1,15 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import TestLogger from '../../lib/testLogger'; +import { Deps, FileInfo } from '../../types'; import { calculateFileHashes } from './calculateFileHashes'; vi.mock('../../lib/getFileHashes', () => ({ getFileHashes: (files: string[]) => - Promise.resolve(Object.fromEntries(files.map((f) => [f, 'hash']))), + Promise.resolve(Object.fromEntries(files.map((f) => [f, `hash-${f}`]))), })); -const environment = { CHROMATIC_RETRIES: 2, CHROMATIC_OUTPUT_INTERVAL: 0 }; const log = new TestLogger(); -const http = { fetch: vi.fn() }; afterEach(() => { vi.restoreAllMocks(); @@ -18,30 +17,34 @@ afterEach(() => { }); describe('calculateFileHashes', () => { - it('sets hashes on context.fileInfo', async () => { - const fileInfo = { + it('returns hashes', async () => { + const fileInfo: FileInfo = { + statsPath: '', lengths: [ - { knownAs: 'iframe.html', contentLength: 42 }, - { knownAs: 'index.html', contentLength: 42 }, + { + knownAs: 'iframe.html', + contentLength: 42, + pathname: '', + }, + { + knownAs: 'index.html', + contentLength: 42, + pathname: '', + }, ], paths: ['iframe.html', 'index.html'], total: 84, }; - const ctx = { - env: environment, + const deps = { + env: {} as Deps['env'], log, - http, - sourceDir: '/static/', - options: { fileHashing: true }, - fileInfo, - announcedBuild: { id: '1' }, - } as any; + }; - await calculateFileHashes(ctx, {} as any); + const hashes = await calculateFileHashes(deps, { fileInfo, sourceDir: '/static/' }); - expect(ctx.fileInfo.hashes).toMatchObject({ - 'iframe.html': 'hash', - 'index.html': 'hash', + expect(hashes).toMatchObject({ + 'iframe.html': 'hash-iframe.html', + 'index.html': 'hash-index.html', }); }); }); diff --git a/node-src/tasks/prepare/calculateFileHashes.ts b/node-src/tasks/prepare/calculateFileHashes.ts index 2eb22c12e..e0a32684c 100644 --- a/node-src/tasks/prepare/calculateFileHashes.ts +++ b/node-src/tasks/prepare/calculateFileHashes.ts @@ -1,34 +1,34 @@ -import { getFileHashes } from '../../lib/getFileHashes'; -import { transitionTo } from '../../lib/tasks'; -import { Context, Task } from '../../types'; -import { hashing, invalid } from '../../ui/tasks/prepare'; +import { getFileHashes } from '@cli/getFileHashes'; + +import type { Deps, FileInfo } from '../../types'; + +type CalculateFileHashesDeps = Pick; + +export interface CalculateFileHashesInput { + fileInfo: FileInfo; + sourceDir: string; +} /** * Calculates file hashes for all files to be uploaded. * File hashes are used for deduplication and integrity checking during upload. * Skips calculation if file hashing is disabled or the task is being skipped. * - * @param ctx - The CLI context containing file info and options - * @param task - The current Listr task for UI updates + * @param deps - The CLI dependencies containing logging and environment access + * @param input - The input containing file information for hashing + * + * @returns A mapping of file paths to their calculated hashes. */ -export async function calculateFileHashes(ctx: Context, task: Task) { - if (ctx.skip || !ctx.options.fileHashing) return; - transitionTo(hashing)(ctx, task); - - try { - if (!ctx.fileInfo) { - throw new Error(invalid(ctx).output); - } - - const start = Date.now(); - ctx.fileInfo.hashes = await getFileHashes( - ctx.fileInfo.paths, - ctx.sourceDir, - ctx.env.CHROMATIC_HASH_CONCURRENCY - ); - ctx.log.debug(`Calculated file hashes in ${Date.now() - start}ms`); - } catch (err) { - ctx.log.warn('Failed to calculate file hashes'); - ctx.log.debug(err); - } +export async function calculateFileHashes( + deps: CalculateFileHashesDeps, + input: CalculateFileHashesInput +): Promise> { + const start = Date.now(); + const hashes = await getFileHashes( + input.fileInfo.paths, + input.sourceDir, + deps.env.CHROMATIC_HASH_CONCURRENCY + ); + deps.log.debug(`Calculated file hashes in ${Date.now() - start}ms`); + return hashes; } diff --git a/node-src/tasks/prepare/index.ts b/node-src/tasks/prepare/index.ts index e0df74606..a996098bb 100644 --- a/node-src/tasks/prepare/index.ts +++ b/node-src/tasks/prepare/index.ts @@ -1,11 +1,51 @@ import { createTask, transitionTo } from '../../lib/tasks'; -import { Context } from '../../types'; +import { Context, FileInfo, TaskResult } from '../../types'; import { initial, success, validating } from '../../ui/tasks/prepare'; -import { calculateFileHashes } from './calculateFileHashes'; +import { calculateFileHashes, CalculateFileHashesInput } from './calculateFileHashes'; import { traceChangedFiles } from './traceChangedFiles'; import { validateAndroidArtifact } from './validateAndroidArtifact'; import { validateFiles } from './validateFiles'; +type PrepareDeps = Pick; + +interface PrepareInput { + transitionToHashing: () => void; + invalidFileInfoError: Error; + fileInfo?: FileInfo; + sourceDir: string; +} + +interface PrepareOutput { + hashes?: Record; +} + +// eslint-disable-next-line jsdoc/require-jsdoc +export async function runPrepare( + deps: PrepareDeps, + input: PrepareInput +): Promise> { + let hashes: Record | undefined; + if (deps.options.fileHashing) { + input.transitionToHashing(); + if (input.fileInfo) { + const calculateFileHashesInput: CalculateFileHashesInput = { + fileInfo: input.fileInfo, + sourceDir: input.sourceDir, + }; + try { + hashes = await calculateFileHashes(deps, calculateFileHashesInput); + } catch (err) { + deps.log.warn('Failed to calculate file hashes'); + deps.log.debug(err); + } + } else { + deps.log.warn('Failed to calculate file hashes: no file info available.'); + deps.log.debug(input.invalidFileInfoError); + } + } + return { kind: 'continue', output: { hashes } }; +} + /** * Sets up the Listr task for preparing the built storybook for upload to Chromatic. * diff --git a/node-src/types.ts b/node-src/types.ts index 250a013f0..621ec0182 100644 --- a/node-src/types.ts +++ b/node-src/types.ts @@ -196,6 +196,17 @@ export interface ProjectMetadata { numberOfAppFiles?: number; } +export interface FileInfo { + paths: string[]; + hashes?: Record; + statsPath: string; + lengths: { + knownAs: string; + pathname: string; + contentLength: number; + }[]; + total: number; +} export interface BaselineBuild { id: string; number: number; @@ -427,17 +438,7 @@ export interface Context { buildCommand?: string; buildLogFile?: string; reactNativeBuildLogFile?: string; - fileInfo?: { - paths: string[]; - hashes?: Record; - statsPath: string; - lengths: { - knownAs: string; - pathname: string; - contentLength: number; - }[]; - total: number; - }; + fileInfo?: FileInfo; share?: { shareId: string; shareUrl: string; From 661dea23c38c73100a5e404503dc2405e38cca35 Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Mon, 11 May 2026 09:20:08 -0400 Subject: [PATCH 2/7] Refactor validateFiles to return file info --- node-src/tasks/prepare/index.ts | 91 +++- node-src/tasks/prepare/validateFiles.test.ts | 491 ++++++++++--------- node-src/tasks/prepare/validateFiles.ts | 95 ++-- 3 files changed, 388 insertions(+), 289 deletions(-) diff --git a/node-src/tasks/prepare/index.ts b/node-src/tasks/prepare/index.ts index a996098bb..99125bdab 100644 --- a/node-src/tasks/prepare/index.ts +++ b/node-src/tasks/prepare/index.ts @@ -1,18 +1,31 @@ +import type Listr from 'listr'; + import { createTask, transitionTo } from '../../lib/tasks'; -import { Context, FileInfo, TaskResult } from '../../types'; -import { initial, success, validating } from '../../ui/tasks/prepare'; +import { AnnouncedBuild, Context, FileInfo, TaskResult } from '../../types'; +import { + hashing, + initial, + invalid, + invalidReactNative, + success, + validating, +} from '../../ui/tasks/prepare'; import { calculateFileHashes, CalculateFileHashesInput } from './calculateFileHashes'; import { traceChangedFiles } from './traceChangedFiles'; import { validateAndroidArtifact } from './validateAndroidArtifact'; -import { validateFiles } from './validateFiles'; +import { + isValidReactNativeStorybook, + isValidStorybook, + validateFiles, + ValidateFilesInput, +} from './validateFiles'; -type PrepareDeps = Pick; +type PrepareDeps = Pick; interface PrepareInput { - transitionToHashing: () => void; - invalidFileInfoError: Error; - fileInfo?: FileInfo; sourceDir: string; + validateFilesInput: ValidateFilesInput; + transitionToHashing: () => void; } interface PrepareOutput { @@ -24,28 +37,64 @@ export async function runPrepare( deps: PrepareDeps, input: PrepareInput ): Promise> { + const fileInfo = await validateFiles(deps, input.validateFilesInput); + let hashes: Record | undefined; if (deps.options.fileHashing) { input.transitionToHashing(); - if (input.fileInfo) { - const calculateFileHashesInput: CalculateFileHashesInput = { - fileInfo: input.fileInfo, - sourceDir: input.sourceDir, - }; - try { - hashes = await calculateFileHashes(deps, calculateFileHashesInput); - } catch (err) { - deps.log.warn('Failed to calculate file hashes'); - deps.log.debug(err); - } - } else { - deps.log.warn('Failed to calculate file hashes: no file info available.'); - deps.log.debug(input.invalidFileInfoError); + const calculateFileHashesInput: CalculateFileHashesInput = { + fileInfo, + sourceDir: input.sourceDir, + }; + try { + hashes = await calculateFileHashes(deps, calculateFileHashesInput); + } catch (err) { + deps.log.warn('Failed to calculate file hashes'); + deps.log.debug(err); } } return { kind: 'continue', output: { hashes } }; } +export const extractPrepareInput = ( + ctx: Context, + listrTask: Listr.ListrTaskWrapper +): PrepareInput => { + if (!ctx.announcedBuild) { + throw new Error('Announced build required for prepare task.'); + } + // eslint-disable-next-line unicorn/prevent-abbreviations + const sourceDir = ctx.sourceDir; + const validateFilesInput: ValidateFilesInput = { + browsers: ctx.announcedBuild.browsers, + buildLogFile: ctx.buildLogFile, + getFileInfoErrorBuilder(err: Error): Error { + return new Error(invalid(ctx, err).output); + }, + isReactNativeApp: ctx.isReactNativeApp || false, + sourceDir, + validationErrorBuilder(missingFiles: string[] | undefined): Error { + return ctx.isReactNativeApp + ? new Error(invalidReactNative(ctx, missingFiles).output) + : new Error(invalid(ctx).output); + }, + validator( + fileInfo: FileInfo, + browsers: AnnouncedBuild['browsers'] + ): { valid: boolean; missingFiles: string[] } { + return ctx.isReactNativeApp + ? isValidReactNativeStorybook(fileInfo, browsers) + : isValidStorybook(fileInfo); + }, + }; + + return { + sourceDir, + validateFilesInput, + transitionToHashing: () => transitionTo(hashing)(ctx, listrTask), + }; +}; + /** * Sets up the Listr task for preparing the built storybook for upload to Chromatic. * diff --git a/node-src/tasks/prepare/validateFiles.test.ts b/node-src/tasks/prepare/validateFiles.test.ts index b56719362..e4f7ae241 100644 --- a/node-src/tasks/prepare/validateFiles.test.ts +++ b/node-src/tasks/prepare/validateFiles.test.ts @@ -2,7 +2,12 @@ import { readdirSync, readFileSync, statSync } from 'fs'; import { afterEach, describe, expect, it, vi } from 'vitest'; import TestLogger from '../../lib/testLogger'; -import { validateFiles } from './validateFiles'; +import { + isValidReactNativeStorybook, + isValidStorybook, + validateFiles, + ValidateFilesInput, +} from './validateFiles'; vi.mock('fs'); @@ -10,24 +15,37 @@ const readdirSyncMock = vi.mocked(readdirSync); const readFileSyncMock = vi.mocked(readFileSync); const statSyncMock = vi.mocked(statSync); -const environment = { CHROMATIC_RETRIES: 2, CHROMATIC_OUTPUT_INTERVAL: 0 }; const log = new TestLogger(); -const http = { fetch: vi.fn() }; afterEach(() => { vi.restoreAllMocks(); vi.resetAllMocks(); }); +const makeDeps = () => ({ + log, + options: {} as any, + packageJson: {} as any, +}); + +const makeInput = (overrides: Partial = {}): ValidateFilesInput => ({ + browsers: [], + isReactNativeApp: false, + sourceDir: '/static/', + validator: () => ({ valid: true, missingFiles: [] }), + validationErrorBuilder: () => new Error('invalid build'), + getFileInfoErrorBuilder: (err) => err, + ...overrides, +}); + describe('validateFiles', () => { - it('sets fileInfo on context', async () => { + it('returns fileInfo for the source directory', async () => { readdirSyncMock.mockReturnValue(['iframe.html', 'index.html'] as any); statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - const ctx = { env: environment, log, http, sourceDir: '/static/' } as any; - await validateFiles(ctx); + const fileInfo = await validateFiles(makeDeps(), makeInput()); - expect(ctx.fileInfo).toEqual( + expect(fileInfo).toEqual( expect.objectContaining({ lengths: [ { contentLength: 42, knownAs: 'iframe.html', pathname: 'iframe.html' }, @@ -39,272 +57,289 @@ describe('validateFiles', () => { ); }); - it("throws when index.html doesn't exist", async () => { - readdirSyncMock.mockReturnValue(['iframe.html'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { env: environment, log, http, options: {}, sourceDir: '/static/' } as any; - await expect(validateFiles(ctx)).rejects.toThrow('Invalid Storybook build at /static/'); - }); - - it("throws when iframe.html doesn't exist", async () => { - readdirSyncMock.mockReturnValue(['index.html'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { env: environment, log, http, options: {}, sourceDir: '/static/' } as any; - await expect(validateFiles(ctx)).rejects.toThrow('Invalid Storybook build at /static/'); - }); - it('does not include the .chromatic directory in the file list', async () => { readdirSyncMock.mockImplementation((path) => { - if (path === '.chromatic') { - return ['zip-unpacked.txt'] as any; - } + if (path === '.chromatic') return ['zip-unpacked.txt'] as any; return ['iframe.html', 'index.html', '.chromatic'] as any; }); statSyncMock.mockImplementation((path) => { - if (path === '.chromatic') { - return { isDirectory: () => true, size: 42 } as any; - } + if (path === '.chromatic') return { isDirectory: () => true, size: 42 } as any; return { isDirectory: () => false, size: 42 } as any; }); - const ctx = { env: environment, log, http, sourceDir: '.' } as any; - await validateFiles(ctx); + const fileInfo = await validateFiles(makeDeps(), makeInput({ sourceDir: '.' })); - expect(ctx.fileInfo).toEqual( + expect(fileInfo).toEqual( expect.objectContaining({ - lengths: [ - { contentLength: 42, knownAs: 'iframe.html', pathname: 'iframe.html' }, - { contentLength: 42, knownAs: 'index.html', pathname: 'index.html' }, - ], paths: ['iframe.html', 'index.html'], total: 84, }) ); }); + it('passes the scanned fileInfo and browsers to the validator', async () => { + readdirSyncMock.mockReturnValue(['file'] as any); + statSyncMock.mockReturnValue({ isDirectory: () => false, size: 1 } as any); + + const validator = vi.fn().mockReturnValue({ valid: true, missingFiles: [] }); + await validateFiles(makeDeps(), makeInput({ browsers: ['android', 'ios'], validator })); + + expect(validator).toHaveBeenCalledWith(expect.objectContaining({ paths: ['file'], total: 1 }), [ + 'android', + 'ios', + ]); + }); + + it('throws via validationErrorBuilder with missing files when the validator returns invalid', async () => { + readdirSyncMock.mockReturnValue(['iframe.html'] as any); + statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); + + const validationErrorBuilder = vi.fn((missing) => new Error(`missing: ${missing?.join(',')}`)); + const validator = vi.fn().mockReturnValue({ valid: false, missingFiles: ['index.html'] }); + + await expect( + validateFiles(makeDeps(), makeInput({ validator, validationErrorBuilder })) + ).rejects.toThrow('missing: index.html'); + expect(validationErrorBuilder).toHaveBeenCalledWith(['index.html']); + }); + + it('throws via getFileInfoErrorBuilder when the source directory cannot be scanned', async () => { + const scanError = new Error('ENOENT'); + readdirSyncMock.mockImplementation(() => { + throw scanError; + }); + + const getFileInfoErrorBuilder = vi.fn((err) => new Error(`wrapped: ${err.message}`)); + + await expect(validateFiles(makeDeps(), makeInput({ getFileInfoErrorBuilder }))).rejects.toThrow( + 'wrapped: ENOENT' + ); + expect(getFileInfoErrorBuilder).toHaveBeenCalledWith(scanError); + }); + describe('with buildLogFile', () => { - it('retries using outputDir from build-storybook.log', async () => { - readdirSyncMock.mockReturnValueOnce([]); + it('retries using outputDir from build-storybook.log when initial validation fails', async () => { + readdirSyncMock.mockReturnValueOnce([] as any); readdirSyncMock.mockReturnValueOnce(['iframe.html', 'index.html'] as any); statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); readFileSyncMock.mockReturnValue('info => Output directory: /var/storybook-static'); - const ctx = { - env: environment, - log, - http, + const validator = vi + .fn() + .mockReturnValueOnce({ valid: false, missingFiles: ['iframe.html', 'index.html'] }) + .mockReturnValue({ valid: true, missingFiles: [] }); + const input = makeInput({ sourceDir: '/static/', buildLogFile: 'build-storybook.log', - options: {}, - packageJson: {}, - } as any; - await validateFiles(ctx); + validator, + }); + + const fileInfo = await validateFiles(makeDeps(), input); expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('Unexpected build directory')); - expect(ctx.sourceDir).toBe('/var/storybook-static'); - expect(ctx.fileInfo).toEqual( + expect(input.sourceDir).toBe('/var/storybook-static'); + expect(fileInfo).toEqual( expect.objectContaining({ - lengths: [ - { contentLength: 42, knownAs: 'iframe.html', pathname: 'iframe.html' }, - { contentLength: 42, knownAs: 'index.html', pathname: 'index.html' }, - ], paths: ['iframe.html', 'index.html'], total: 84, }) ); }); - }); - describe('with isReactNativeApp', () => { - it('throws when no React Native browsers are configured', async () => { - readdirSyncMock.mockReturnValue([ - 'storybook.apk', - 'storybook.app/modules.json', - 'manifest.json', - ] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); + it('throws via validationErrorBuilder when retried validation still fails', async () => { + readdirSyncMock.mockReturnValue([] as any); + readFileSyncMock.mockReturnValue('info => Output directory: /var/storybook-static'); - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: [] }, - } as any; - await expect(validateFiles(ctx)).rejects.toThrow( - 'Invalid React Native Storybook build in directory /static' - ); - }); + const validator = vi.fn().mockReturnValue({ valid: false, missingFiles: ['index.html'] }); + const validationErrorBuilder = vi.fn(() => new Error('still invalid')); - describe('Android devices', () => { - it('sets fileInfo on context with valid React Native build', async () => { - readdirSyncMock.mockReturnValue(['storybook.apk', 'manifest.json'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { - env: environment, - log, - http, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: ['android'] }, - } as any; - await validateFiles(ctx); - - expect(ctx.fileInfo).toEqual( - expect.objectContaining({ - lengths: [ - { contentLength: 42, knownAs: 'storybook.apk', pathname: 'storybook.apk' }, - { contentLength: 42, knownAs: 'manifest.json', pathname: 'manifest.json' }, - ], - paths: ['storybook.apk', 'manifest.json'], - total: 84, + await expect( + validateFiles( + makeDeps(), + makeInput({ + buildLogFile: 'build-storybook.log', + validator, + validationErrorBuilder, }) - ); - }); + ) + ).rejects.toThrow('still invalid'); + }); - it("throws when manifest.json doesn't exist", async () => { - readdirSyncMock.mockReturnValue(['storybook.apk'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: ['android'] }, - } as any; - await expect(validateFiles(ctx)).rejects.toThrow( - `Missing files: - → manifest.json - -Invalid React Native Storybook build in directory /static` - ); - }); + it('does not retry when the build log has no output directory', async () => { + readdirSyncMock.mockReturnValue([] as any); + readFileSyncMock.mockReturnValue('no output info in this log'); - it("throws when APK doesn't exist", async () => { - readdirSyncMock.mockReturnValue(['manifest.json'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: ['android'] }, - } as any; - await expect(validateFiles(ctx)).rejects.toThrow( - `→ This build is missing the storybook.apk file required for React Native Storybook for Android. - Please ensure that the file is present in the output directory and named correctly before running the CLI. - -Invalid React Native Storybook build in directory /static` - ); - }); + const validator = vi.fn().mockReturnValue({ valid: false, missingFiles: [] }); + + await expect( + validateFiles( + makeDeps(), + makeInput({ + sourceDir: '/static/', + buildLogFile: 'build-storybook.log', + validator, + validationErrorBuilder: () => new Error('invalid'), + }) + ) + ).rejects.toThrow('invalid'); + expect(readdirSyncMock).toHaveBeenCalledTimes(1); }); - describe('iOS devices', () => { - it('sets fileInfo on context with valid React Native build', async () => { - readdirSyncMock.mockReturnValue(['storybook.app/modules.json', 'manifest.json'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { - env: environment, - log, - http, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: ['ios'] }, - } as any; - await validateFiles(ctx); - - expect(ctx.fileInfo).toEqual( - expect.objectContaining({ - lengths: [ - { - contentLength: 42, - knownAs: 'storybook.app/modules.json', - pathname: 'storybook.app/modules.json', - }, - { contentLength: 42, knownAs: 'manifest.json', pathname: 'manifest.json' }, - ], - paths: ['storybook.app/modules.json', 'manifest.json'], - total: 84, + it('does not retry when the build log output directory matches the source directory', async () => { + readdirSyncMock.mockReturnValue([] as any); + readFileSyncMock.mockReturnValue('info => Output directory: /static/'); + + const validator = vi.fn().mockReturnValue({ valid: false, missingFiles: [] }); + + await expect( + validateFiles( + makeDeps(), + makeInput({ + sourceDir: '/static/', + buildLogFile: 'build-storybook.log', + validator, + validationErrorBuilder: () => new Error('invalid'), }) - ); - }); + ) + ).rejects.toThrow('invalid'); + expect(readdirSyncMock).toHaveBeenCalledTimes(1); + }); - it("throws when manifest.json doesn't exist", async () => { - readdirSyncMock.mockReturnValue(['storybook.app/modules.json'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: ['ios'] }, - } as any; - await expect(validateFiles(ctx)).rejects.toThrow( - `Missing files: - → manifest.json - -Invalid React Native Storybook build in directory /static` - ); + it('swallows build log read errors and falls back to the validation error', async () => { + readdirSyncMock.mockReturnValue([] as any); + readFileSyncMock.mockImplementation(() => { + throw new Error('build log not found'); }); - it("throws when the APP directory doesn't exist", async () => { - readdirSyncMock.mockReturnValue(['manifest.json'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: ['ios'] }, - } as any; - await expect(validateFiles(ctx)).rejects.toThrow( - `→ This build is missing the storybook.app file required for React Native Storybook for iOS. - Please ensure that the file is present in the output directory and named correctly before running the CLI. - -Invalid React Native Storybook build in directory /static` - ); - }); + const validator = vi.fn().mockReturnValue({ valid: false, missingFiles: [] }); + + await expect( + validateFiles( + makeDeps(), + makeInput({ + buildLogFile: 'build-storybook.log', + validator, + validationErrorBuilder: () => new Error('invalid'), + }) + ) + ).rejects.toThrow('invalid'); + expect(log.debug).toHaveBeenCalled(); }); + }); +}); - describe('Android and iOS devices', () => { - it('throws when both native build files are missing', async () => { - readdirSyncMock.mockReturnValue(['manifest.json'] as any); - statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - isReactNativeApp: true, - announcedBuild: { browsers: ['android', 'ios'] }, - } as any; - await expect(validateFiles(ctx)).rejects.toThrow( - `→ This build is missing the storybook.app (iOS) and storybook.apk (Android) files required for React Native Storybook. - Please ensure that the files are present in the output directory and named correctly before running the CLI. - -Invalid React Native Storybook build in directory /static` - ); - }); +describe('isValidStorybook', () => { + it('is valid when iframe.html and index.html are present with non-zero total', () => { + expect(isValidStorybook({ paths: ['iframe.html', 'index.html'], total: 84 })).toEqual({ + valid: true, + missingFiles: [], + }); + }); + + it('reports iframe.html missing when only index.html is present', () => { + expect(isValidStorybook({ paths: ['index.html'], total: 42 })).toEqual({ + valid: false, + missingFiles: ['iframe.html'], + }); + }); + + it('reports index.html missing when only iframe.html is present', () => { + expect(isValidStorybook({ paths: ['iframe.html'], total: 42 })).toEqual({ + valid: false, + missingFiles: ['index.html'], + }); + }); + + it('is invalid when total size is 0 even with required files present', () => { + expect(isValidStorybook({ paths: ['iframe.html', 'index.html'], total: 0 })).toMatchObject({ + valid: false, + }); + }); +}); + +describe('isValidReactNativeStorybook', () => { + it('is invalid when no browsers are configured', () => { + expect( + isValidReactNativeStorybook({ paths: ['storybook.apk', 'manifest.json'], total: 84 }, []) + ).toEqual({ valid: false, missingFiles: [] }); + }); + + it('defaults browsers to an empty list when omitted', () => { + expect( + isValidReactNativeStorybook({ paths: ['storybook.apk', 'manifest.json'], total: 84 }) + ).toEqual({ valid: false, missingFiles: [] }); + }); + + describe('Android', () => { + it('is valid with manifest.json and storybook.apk', () => { + expect( + isValidReactNativeStorybook({ paths: ['storybook.apk', 'manifest.json'], total: 84 }, [ + 'android', + ]) + ).toEqual({ valid: true, missingFiles: [] }); + }); + + it('reports missing storybook.apk', () => { + expect( + isValidReactNativeStorybook({ paths: ['manifest.json'], total: 42 }, ['android']) + ).toEqual({ valid: false, missingFiles: ['storybook.apk'] }); }); + + it('reports missing manifest.json', () => { + expect( + isValidReactNativeStorybook({ paths: ['storybook.apk'], total: 42 }, ['android']) + ).toEqual({ valid: false, missingFiles: ['manifest.json'] }); + }); + }); + + describe('iOS', () => { + it('is valid with manifest.json and a storybook.app/* path', () => { + expect( + isValidReactNativeStorybook( + { paths: ['storybook.app/modules.json', 'manifest.json'], total: 84 }, + ['ios'] + ) + ).toEqual({ valid: true, missingFiles: [] }); + }); + + it('reports missing storybook.app when no path starts with storybook.app/', () => { + expect(isValidReactNativeStorybook({ paths: ['manifest.json'], total: 42 }, ['ios'])).toEqual( + { valid: false, missingFiles: ['storybook.app'] } + ); + }); + + it('reports missing manifest.json', () => { + expect( + isValidReactNativeStorybook({ paths: ['storybook.app/modules.json'], total: 42 }, ['ios']) + ).toEqual({ valid: false, missingFiles: ['manifest.json'] }); + }); + }); + + describe('Android and iOS', () => { + it('is valid with manifest.json, storybook.apk, and a storybook.app/* path', () => { + expect( + isValidReactNativeStorybook( + { + paths: ['storybook.apk', 'storybook.app/modules.json', 'manifest.json'], + total: 126, + }, + ['android', 'ios'] + ) + ).toEqual({ valid: true, missingFiles: [] }); + }); + + it('reports both storybook.apk and storybook.app as missing', () => { + expect( + isValidReactNativeStorybook({ paths: ['manifest.json'], total: 42 }, ['android', 'ios']) + ).toEqual({ valid: false, missingFiles: ['storybook.apk', 'storybook.app'] }); + }); + }); + + it('is invalid when total size is 0 even with required files present', () => { + expect( + isValidReactNativeStorybook({ paths: ['storybook.apk', 'manifest.json'], total: 0 }, [ + 'android', + ]) + ).toMatchObject({ valid: false }); }); }); diff --git a/node-src/tasks/prepare/validateFiles.ts b/node-src/tasks/prepare/validateFiles.ts index ad3a8efbc..ce96c63a2 100644 --- a/node-src/tasks/prepare/validateFiles.ts +++ b/node-src/tasks/prepare/validateFiles.ts @@ -2,9 +2,23 @@ import { readdirSync, readFileSync, statSync } from 'fs'; import path from 'path'; import slash from 'slash'; -import { Context } from '../../types'; +import { AnnouncedBuild, Context, FileInfo } from '../../types'; import deviatingOutputDirectory from '../../ui/messages/warnings/deviatingOutputDirectory'; -import { invalid, invalidReactNative } from '../../ui/tasks/prepare'; + +type ValidateFilesDeps = Pick; + +export interface ValidateFilesInput { + isReactNativeApp: boolean; + sourceDir: string; + buildLogFile?: string; + browsers: AnnouncedBuild['browsers']; + validator: ( + fileInfo: FileInfo, + browsers: AnnouncedBuild['browsers'] + ) => { valid: boolean; missingFiles: string[] }; + validationErrorBuilder: (missingFiles?: string[]) => Error; + getFileInfoErrorBuilder: (err: Error) => Error; +} /** * Represents a file path specification with its content length. @@ -20,30 +34,24 @@ interface PathSpec { * paths will be like iframe.html rather than storybook-static/iframe.html). * Excludes the .chromatic directory which is reserved for internal use. * - * @param ctx - The CLI context for logging * @param rootDirectory - The root directory to scan * @param dirname - The current subdirectory being scanned (relative to rootDirectory) * * @returns Array of path specifications with pathname and content length */ -function getPathSpecsInDirectory(ctx: Context, rootDirectory: string, dirname = '.'): PathSpec[] { +function getPathSpecsInDirectory(rootDirectory: string, dirname = '.'): PathSpec[] { // .chromatic is a special directory reserved for internal use and should not be uploaded if (dirname === '.chromatic') { return []; } - try { - return readdirSync(path.join(rootDirectory, dirname)).flatMap((p: string) => { - const pathname = path.join(dirname, p); - const stats = statSync(path.join(rootDirectory, pathname)); - return stats.isDirectory() - ? getPathSpecsInDirectory(ctx, rootDirectory, pathname) - : [{ pathname, contentLength: stats.size }]; - }); - } catch (err) { - ctx.log.debug(err); - throw new Error(invalid({ ...ctx, sourceDir: rootDirectory }, err).output); - } + return readdirSync(path.join(rootDirectory, dirname)).flatMap((p: string) => { + const pathname = path.join(dirname, p); + const stats = statSync(path.join(rootDirectory, pathname)); + return stats.isDirectory() + ? getPathSpecsInDirectory(rootDirectory, pathname) + : [{ pathname, contentLength: stats.size }]; + }); } /** @@ -68,13 +76,12 @@ function getOutputDirectory(buildLog: string) { * Analyzes a directory to extract file information needed for upload. * Processes all files to calculate total size, collect paths, and locate stats files. * - * @param ctx - The CLI context for logging * @param sourceDirectory - The directory to analyze * * @returns Object containing file lengths, paths, stats path, and total size */ -function getFileInfo(ctx: Context, sourceDirectory: string) { - const lengths = getPathSpecsInDirectory(ctx, sourceDirectory).map((o) => ({ +function getFileInfo(sourceDirectory: string) { + const lengths = getPathSpecsInDirectory(sourceDirectory).map((o) => ({ ...o, knownAs: slash(o.pathname), })); @@ -99,7 +106,7 @@ function getFileInfo(ctx: Context, sourceDirectory: string) { * * @returns True if the directory contains a valid Storybook build */ -const isValidStorybook = ({ paths, total }) => { +export const isValidStorybook = ({ paths, total }) => { const missingFiles = ['iframe.html', 'index.html'].filter((f) => !paths.includes(f)); return { valid: total > 0 && missingFiles.length === 0, missingFiles }; }; @@ -117,7 +124,7 @@ const isValidStorybook = ({ paths, total }) => { * * @returns True if the directory contains a valid React Native Storybook build */ -const isValidReactNativeStorybook = ( +export const isValidReactNativeStorybook = ( { paths, total }, browsers: string[] = [] ): { valid: boolean; missingFiles: string[] } => { @@ -151,36 +158,44 @@ const isValidReactNativeStorybook = ( * If validation fails and a build log is available, attempts to find the * correct output directory from the log and retries validation. * - * @param ctx - The CLI context containing source directory and build log info + * @param deps - Dependencies for logging and file info retrieval + * @param input - Input parameters for validation + * + * @returns File information object if validation succeeds, or throws an error * * @throws {Error} if no valid Storybook build is found */ -export async function validateFiles(ctx: Context) { - const validator = ctx.isReactNativeApp ? isValidReactNativeStorybook : isValidStorybook; - const browsers = ctx.announcedBuild?.browsers; - - ctx.fileInfo = getFileInfo(ctx, ctx.sourceDir); +export async function validateFiles( + deps: ValidateFilesDeps, + input: ValidateFilesInput +): Promise { + let fileInfo: FileInfo; + try { + fileInfo = getFileInfo(input.sourceDir); + } catch (err) { + deps.log.debug(err); + throw input.getFileInfoErrorBuilder(err); + } - if (!validator(ctx.fileInfo, browsers).valid && ctx.buildLogFile) { + if (!input.validator(fileInfo, input.browsers).valid && input.buildLogFile) { try { - const buildLog = readFileSync(ctx.buildLogFile, 'utf8'); + const buildLog = readFileSync(input.buildLogFile, 'utf8'); const outputDirectory = getOutputDirectory(buildLog); - if (outputDirectory && outputDirectory !== ctx.sourceDir) { - ctx.log.warn(deviatingOutputDirectory(ctx, outputDirectory)); - ctx.sourceDir = outputDirectory; - ctx.fileInfo = getFileInfo(ctx, ctx.sourceDir); + if (outputDirectory && outputDirectory !== input.sourceDir) { + deps.log.warn( + deviatingOutputDirectory({ sourceDir: input.sourceDir, ...deps }, outputDirectory) + ); + input.sourceDir = outputDirectory; + fileInfo = getFileInfo(input.sourceDir); } } catch (err) { - ctx.log.debug(err); + deps.log.debug(err); } } - const validatorResult = validator(ctx.fileInfo, browsers); + const validatorResult = input.validator(fileInfo, input.browsers); if (!validatorResult.valid) { - throw new Error( - ctx.isReactNativeApp - ? invalidReactNative(ctx, validatorResult.missingFiles).output - : invalid(ctx).output - ); + throw input.validationErrorBuilder(validatorResult.missingFiles); } + return fileInfo; } From a2f6cd44f25eb7e6e627e56c149a5741dbe1b94e Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Mon, 11 May 2026 10:38:33 -0400 Subject: [PATCH 3/7] Refactor validateAndroidArtifact --- node-src/tasks/prepare/index.ts | 10 ++++ .../prepare/validateAndroidArtifact.test.ts | 50 ++++++------------- .../tasks/prepare/validateAndroidArtifact.ts | 17 +++---- 3 files changed, 32 insertions(+), 45 deletions(-) diff --git a/node-src/tasks/prepare/index.ts b/node-src/tasks/prepare/index.ts index 99125bdab..ad03aed8e 100644 --- a/node-src/tasks/prepare/index.ts +++ b/node-src/tasks/prepare/index.ts @@ -6,6 +6,7 @@ import { hashing, initial, invalid, + invalidAndroidArtifact, invalidReactNative, success, validating, @@ -25,6 +26,7 @@ type PrepareDeps = Pick; interface PrepareInput { sourceDir: string; validateFilesInput: ValidateFilesInput; + invalidAndroidArtifactError: Error; transitionToHashing: () => void; } @@ -39,6 +41,13 @@ export async function runPrepare( ): Promise> { const fileInfo = await validateFiles(deps, input.validateFilesInput); + if (input.validateFilesInput.browsers.includes('android')) { + const isValidAndroidArtifact = await validateAndroidArtifact(input.sourceDir); + if (!isValidAndroidArtifact) { + throw input.invalidAndroidArtifactError; + } + } + let hashes: Record | undefined; if (deps.options.fileHashing) { input.transitionToHashing(); @@ -91,6 +100,7 @@ export const extractPrepareInput = ( return { sourceDir, validateFilesInput, + invalidAndroidArtifactError: new Error(invalidAndroidArtifact(ctx).output), transitionToHashing: () => transitionTo(hashing)(ctx, listrTask), }; }; diff --git a/node-src/tasks/prepare/validateAndroidArtifact.test.ts b/node-src/tasks/prepare/validateAndroidArtifact.test.ts index a19d6b314..8fc9da7f8 100644 --- a/node-src/tasks/prepare/validateAndroidArtifact.test.ts +++ b/node-src/tasks/prepare/validateAndroidArtifact.test.ts @@ -1,31 +1,17 @@ import AdmZip from 'adm-zip'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import TestLogger from '../../lib/testLogger'; import { validateAndroidArtifact } from './validateAndroidArtifact'; vi.mock('adm-zip', () => ({ default: vi.fn() })); const AdmZipMock = vi.mocked(AdmZip); -const environment = { CHROMATIC_RETRIES: 2, CHROMATIC_OUTPUT_INTERVAL: 0 }; -const log = new TestLogger(); -const http = { fetch: vi.fn() }; - afterEach(() => { vi.restoreAllMocks(); vi.resetAllMocks(); }); -const makeContext = (browsers: string[]) => - ({ - env: environment, - log, - http, - sourceDir: '/static/', - announcedBuild: { browsers }, - }) as any; - const mockEntries = (entryNames: string[]) => { const entries = entryNames.map((entryName) => ({ entryName })); AdmZipMock.mockImplementation(function (this: { getEntries: () => typeof entries }) { @@ -34,38 +20,34 @@ const mockEntries = (entryNames: string[]) => { }; describe('validateAndroidArtifact', () => { - it('skips when android is not in browsers', async () => { - const ctx = makeContext(['ios']); - await expect(validateAndroidArtifact(ctx)).resolves.toBeUndefined(); - expect(AdmZipMock).not.toHaveBeenCalled(); - }); - - it('passes when APK has no lib/ entries', async () => { + it('returns true when APK has no lib/ entries', async () => { mockEntries(['AndroidManifest.xml', 'classes.dex']); - await expect(validateAndroidArtifact(makeContext(['android']))).resolves.toBeUndefined(); + await expect(validateAndroidArtifact('/static/')).resolves.toBe(true); }); - it('passes when APK has only lib/x86_64/ entries', async () => { + it('returns true when APK has only lib/x86_64/ entries', async () => { mockEntries(['lib/x86_64/libnative.so']); - await expect(validateAndroidArtifact(makeContext(['android']))).resolves.toBeUndefined(); + await expect(validateAndroidArtifact('/static/')).resolves.toBe(true); }); - it('passes when APK has lib/x86_64/ alongside other ABIs', async () => { + it('returns true when APK has lib/x86_64/ alongside other ABIs', async () => { mockEntries(['lib/x86_64/libnative.so', 'lib/arm64-v8a/libnative.so']); - await expect(validateAndroidArtifact(makeContext(['android']))).resolves.toBeUndefined(); + await expect(validateAndroidArtifact('/static/')).resolves.toBe(true); }); - it('throws when APK has only ARM ABI entries', async () => { + it('returns false when APK has only ARM ABI entries', async () => { mockEntries(['lib/armeabi-v7a/libnative.so']); - await expect(validateAndroidArtifact(makeContext(['android']))).rejects.toThrow( - 'Your storybook.apk contains native libraries but does not include x86_64 support. Chromatic only supports x86_64.' - ); + await expect(validateAndroidArtifact('/static/')).resolves.toBe(false); }); - it('throws when APK has multiple ARM ABIs but no x86_64', async () => { + it('returns false when APK has multiple ARM ABIs but no x86_64', async () => { mockEntries(['lib/armeabi-v7a/libnative.so', 'lib/arm64-v8a/libnative.so']); - await expect(validateAndroidArtifact(makeContext(['android']))).rejects.toThrow( - 'Your storybook.apk contains native libraries but does not include x86_64 support. Chromatic only supports x86_64.' - ); + await expect(validateAndroidArtifact('/static/')).resolves.toBe(false); + }); + + it('reads the APK from storybook.apk under the given source directory', async () => { + mockEntries([]); + await validateAndroidArtifact('/some/source/dir'); + expect(AdmZipMock).toHaveBeenCalledWith('/some/source/dir/storybook.apk'); }); }); diff --git a/node-src/tasks/prepare/validateAndroidArtifact.ts b/node-src/tasks/prepare/validateAndroidArtifact.ts index 5e7b90e63..bf581270c 100644 --- a/node-src/tasks/prepare/validateAndroidArtifact.ts +++ b/node-src/tasks/prepare/validateAndroidArtifact.ts @@ -1,21 +1,18 @@ import AdmZip from 'adm-zip'; import path from 'path'; -import { Context } from '../../types'; -import { invalidAndroidArtifact } from '../../ui/tasks/prepare'; - /** * Validates that the Android APK artifact contains x86_64 native libraries if it contains * any native libraries at all. Chromatic only supports x86_64 Android emulators. * - * @param ctx - The CLI context containing source directory and build info + * @param sourceDirectory - The directory containing the APK artifact + * + * @returns true if the APK contains x86_64 native libraries, false otherwise * * @throws {Error} if the APK contains native libraries without x86_64 support */ -export async function validateAndroidArtifact(ctx: Context) { - if (!ctx.announcedBuild?.browsers?.includes('android')) return; - - const apkPath = path.join(ctx.sourceDir, 'storybook.apk'); +export async function validateAndroidArtifact(sourceDirectory: string): Promise { + const apkPath = path.join(sourceDirectory, 'storybook.apk'); const zip = new AdmZip(apkPath); const entries = zip.getEntries(); @@ -27,7 +24,5 @@ export async function validateAndroidArtifact(ctx: Context) { } } - if (abiDirectories.size > 0 && !abiDirectories.has('x86_64')) { - throw new Error(invalidAndroidArtifact(ctx).output); - } + return !(abiDirectories.size > 0 && !abiDirectories.has('x86_64')); } From ea84c6b7f5cfc1340aa77096bb172ad1f9849b12 Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Mon, 11 May 2026 10:49:43 -0400 Subject: [PATCH 4/7] Refactor traceChangedFiles --- node-src/tasks/prepare/index.ts | 85 ++++++++- .../tasks/prepare/traceChangedFiles.test.ts | 161 +++++++++++------- node-src/tasks/prepare/traceChangedFiles.ts | 105 +++++------- 3 files changed, 225 insertions(+), 126 deletions(-) diff --git a/node-src/tasks/prepare/index.ts b/node-src/tasks/prepare/index.ts index ad03aed8e..bde1961a0 100644 --- a/node-src/tasks/prepare/index.ts +++ b/node-src/tasks/prepare/index.ts @@ -1,18 +1,28 @@ +import { createTask, transitionTo } from '@cli/tasks'; +import * as turbosnap from '@cli/turbosnap'; import type Listr from 'listr'; +import semver from 'semver'; -import { createTask, transitionTo } from '../../lib/tasks'; -import { AnnouncedBuild, Context, FileInfo, TaskResult } from '../../types'; +import { rewriteErrorMessage } from '../../lib/utilities'; +import { AnnouncedBuild, Context, FileInfo, TaskResult, TurboSnap } from '../../types'; +import missingStatsFile from '../../ui/messages/errors/missingStatsFile'; import { + bailed, hashing, initial, invalid, invalidAndroidArtifact, invalidReactNative, success, + traced, + tracing, validating, } from '../../ui/tasks/prepare'; import { calculateFileHashes, CalculateFileHashesInput } from './calculateFileHashes'; -import { traceChangedFiles } from './traceChangedFiles'; +import { + traceChangedFiles, + TraceChangedFilesInput, +} from './traceChangedFiles'; import { validateAndroidArtifact } from './validateAndroidArtifact'; import { isValidReactNativeStorybook, @@ -21,20 +31,32 @@ import { ValidateFilesInput, } from './validateFiles'; + +} from '../../ui/tasks/prepare'; +} from './validateFiles'; + type PrepareDeps = Pick; +type PrepareTraceInput = TraceChangedFilesInput & { + turboSnap: TurboSnap | undefined; + changedFiles: string[] | undefined; + missingStatsError: () => Error; +}; + interface PrepareInput { sourceDir: string; validateFilesInput: ValidateFilesInput; invalidAndroidArtifactError: Error; + traceChangedFilesInput: Omit; transitionToHashing: () => void; } interface PrepareOutput { hashes?: Record; + onlyStoryFiles?: string[]; } -// eslint-disable-next-line jsdoc/require-jsdoc +// eslint-disable-next-line jsdoc/require-jsdoc, complexity export async function runPrepare( deps: PrepareDeps, input: PrepareInput @@ -48,6 +70,29 @@ export async function runPrepare( } } + const traceInput = input.traceChangedFilesInput; + let onlyStoryFiles: string[] | undefined; + if (traceInput.turboSnap && !traceInput.turboSnap.unavailable && traceInput.changedFiles) { + if (!fileInfo.statsPath) { + throw traceInput.missingStatsError(); + } + try { + onlyStoryFiles = await traceChangedFiles(deps, { + ...traceInput, + statsPath: fileInfo.statsPath, + }); + } catch (err) { + if (!deps.options.interactive) { + deps.log.info('Failed to retrieve dependent story files', { + statsPath: fileInfo.statsPath, + changedFiles: traceInput.changedFiles, + err, + }); + } + throw rewriteErrorMessage(err, `Could not retrieve dependent story files.\n${err.message}`); + } + } + let hashes: Record | undefined; if (deps.options.fileHashing) { input.transitionToHashing(); @@ -62,20 +107,20 @@ export async function runPrepare( deps.log.debug(err); } } - return { kind: 'continue', output: { hashes } }; + return { + kind: 'continue', + output: { hashes, onlyStoryFiles }, + }; } export const extractPrepareInput = ( ctx: Context, listrTask: Listr.ListrTaskWrapper ): PrepareInput => { - if (!ctx.announcedBuild) { - throw new Error('Announced build required for prepare task.'); - } // eslint-disable-next-line unicorn/prevent-abbreviations const sourceDir = ctx.sourceDir; const validateFilesInput: ValidateFilesInput = { - browsers: ctx.announcedBuild.browsers, + browsers: ctx.announcedBuild?.browsers || [], buildLogFile: ctx.buildLogFile, getFileInfoErrorBuilder(err: Error): Error { return new Error(invalid(ctx, err).output); @@ -97,10 +142,32 @@ export const extractPrepareInput = ( }, }; + const traceChangedFilesInput: Omit = { + turboSnap: ctx.turboSnap, + changedFiles: ctx.git.changedFiles, + untracedFiles: ctx.untracedFiles, + missingStatsError(): Error { + const nonLegacyStatsSupported = + ctx.storybook?.version && + semver.gte(semver.coerce(ctx.storybook.version) || '0.0.0', '8.0.0'); + if (ctx.turboSnap) ctx.turboSnap.bailReason = { missingStatsFile: true }; + return new Error(missingStatsFile({ legacy: !nonLegacyStatsSupported })); + }, + transitionToTracing: () => transitionTo(tracing)(ctx, listrTask), + transitionToTraced: () => transitionTo(traced)(ctx, listrTask), + transitionToBailed: () => transitionTo(bailed)(ctx, listrTask), + // TECHDEBT: due to some upcoming work around turbosnap, we don't want to refactor `turbosnap.traceChangedFiles` to remove the + // context dependency yet, so we're passing in a closure around the original function to keep context from leaking into task bodies. + // Future refactor work should remove the context dependency from turbosnap and refactor this implementation. + runInnerTrace: (statsPath: FileInfo['statsPath']) => + turbosnap.traceChangedFiles(ctx, statsPath), + }; + return { sourceDir, validateFilesInput, invalidAndroidArtifactError: new Error(invalidAndroidArtifact(ctx).output), + traceChangedFilesInput, transitionToHashing: () => transitionTo(hashing)(ctx, listrTask), }; }; diff --git a/node-src/tasks/prepare/traceChangedFiles.test.ts b/node-src/tasks/prepare/traceChangedFiles.test.ts index 90421091f..cfb5c4c3a 100644 --- a/node-src/tasks/prepare/traceChangedFiles.test.ts +++ b/node-src/tasks/prepare/traceChangedFiles.test.ts @@ -1,62 +1,45 @@ -import { traceChangedFiles as traceChangedFilesDep } from '@cli/turbosnap'; -import { access } from 'fs'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import TestLogger from '../../lib/testLogger'; -import { traceChangedFiles } from './traceChangedFiles'; +import { Deps } from '../../types'; +import { traceChangedFiles, TraceChangedFilesInput } from './traceChangedFiles'; -vi.mock('fs'); -vi.mock('@cli/turbosnap'); -vi.mock('../readStatsFile', () => ({ - readStatsFile: () => - Promise.resolve({ - modules: [ - { - id: '../__mocks__/storybookBaseDir/test.ts', - name: '../__mocks__/storybookBaseDir/test.ts', - }, - ], - }), -})); - -const traceChangedFilesTurbosnap = vi.mocked(traceChangedFilesDep); -const accessMock = vi.mocked(access); - -const environment = { CHROMATIC_RETRIES: 2, CHROMATIC_OUTPUT_INTERVAL: 0 }; const log = new TestLogger(); -const http = { fetch: vi.fn() }; afterEach(() => { vi.restoreAllMocks(); vi.resetAllMocks(); }); -describe('traceChangedFiles', () => { - beforeEach(() => { - accessMock.mockImplementation((_path, callback) => Promise.resolve(callback(null))); - }); +const makeDeps = (options: Partial = {}) => + ({ + log, + options: options as Deps['options'], + }) as Pick; - it('sets onlyStoryFiles on context', async () => { - const deps = { 123: ['./example.stories.js'] }; - traceChangedFilesTurbosnap.mockResolvedValue(deps); +const makeInput = (overrides: Partial = {}): TraceChangedFilesInput => ({ + untracedFiles: undefined, + transitionToTracing: vi.fn(), + transitionToTraced: vi.fn(), + transitionToBailed: vi.fn(), + statsPath: '/static/stats.json', + runInnerTrace: vi.fn().mockResolvedValue(undefined), + ...overrides, +}); - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - fileInfo: { statsPath: '/static/preview-stats.json' }, - git: { changedFiles: ['./example.js'] }, - turboSnap: {}, - } as any; - await traceChangedFiles(ctx, {} as any); +describe('traceChangedFiles', () => { + it('returns onlyStoryFiles from the inner trace result', async () => { + const innerResult = { 123: ['./example.stories.js'] }; + const result = await traceChangedFiles( + makeDeps(), + makeInput({ runInnerTrace: vi.fn().mockResolvedValue(innerResult) }) + ); - expect(ctx.onlyStoryFiles).toStrictEqual(Object.keys(deps)); + expect(result).toStrictEqual(Object.keys(innerResult)); }); - it('escapes special characters on context', async () => { - const deps = { + it('escapes special characters in returned onlyStoryFiles', async () => { + const innerResult = { './$example-new.stories.js': ['./$example-new.stories.js'], './+example-new.stories.js': ['./+example-new.stories.js'], './example-(new).stories.js': ['./example-(new).stories.js'], @@ -65,21 +48,12 @@ describe('traceChangedFiles', () => { '[./example/[account]/[id]/[unit]/language/example.stories.tsx]', ], }; - traceChangedFilesTurbosnap.mockResolvedValue(deps); - - const ctx = { - env: environment, - log, - http, - options: {}, - sourceDir: '/static/', - fileInfo: { statsPath: '/static/preview-stats.json' }, - git: { changedFiles: ['./example.js'] }, - turboSnap: {}, - } as any; - await traceChangedFiles(ctx, {} as any); + const result = await traceChangedFiles( + makeDeps(), + makeInput({ runInnerTrace: vi.fn().mockResolvedValue(innerResult) }) + ); - expect(ctx.onlyStoryFiles).toStrictEqual([ + expect(result).toStrictEqual([ String.raw`./\$example-new.stories.js`, String.raw`./\+example-new.stories.js`, String.raw`./example-\(new\).stories.js`, @@ -87,4 +61,75 @@ describe('traceChangedFiles', () => { String.raw`\[./example/\[account\]/\[id\]/\[unit\]/language/example.stories.tsx\]`, ]); }); + + it('calls transitionToBailed and returns undefined when inner trace returns undefined', async () => { + const transitionToBailed = vi.fn(); + const transitionToTraced = vi.fn(); + const result = await traceChangedFiles( + makeDeps(), + makeInput({ + runInnerTrace: vi.fn().mockResolvedValue(undefined), + transitionToBailed, + transitionToTraced, + }) + ); + expect(result).toBeUndefined(); + expect(transitionToBailed).toHaveBeenCalled(); + expect(transitionToTraced).not.toHaveBeenCalled(); + }); + + it('calls transitionToTracing and transitionToTraced on success', async () => { + const transitionToTracing = vi.fn(); + const transitionToTraced = vi.fn(); + const transitionToBailed = vi.fn(); + await traceChangedFiles( + makeDeps(), + makeInput({ + runInnerTrace: vi.fn().mockResolvedValue({ 1: ['./a.stories.js'] }), + transitionToTracing, + transitionToTraced, + transitionToBailed, + }) + ); + expect(transitionToTracing).toHaveBeenCalled(); + expect(transitionToTraced).toHaveBeenCalled(); + expect(transitionToBailed).not.toHaveBeenCalled(); + }); + + it('logs affected story files when non-interactive and traceChanged is false', async () => { + await traceChangedFiles( + makeDeps({ interactive: false, traceChanged: false } as any), + makeInput({ runInnerTrace: vi.fn().mockResolvedValue({ 1: ['./a.stories.js'] }) }) + ); + expect(log.info).toHaveBeenCalledWith(expect.stringContaining('Found affected story files')); + }); + + it('does not log affected story files when interactive', async () => { + await traceChangedFiles( + makeDeps({ interactive: true } as any), + makeInput({ runInnerTrace: vi.fn().mockResolvedValue({ 1: ['./a.stories.js'] }) }) + ); + expect(log.info).not.toHaveBeenCalled(); + }); + + it('does not log affected story files when traceChanged is true', async () => { + await traceChangedFiles( + makeDeps({ interactive: false, traceChanged: true } as any), + makeInput({ runInnerTrace: vi.fn().mockResolvedValue({ 1: ['./a.stories.js'] }) }) + ); + expect(log.info).not.toHaveBeenCalledWith( + expect.stringContaining('Found affected story files') + ); + }); + + it('logs untraced files when present and non-interactive', async () => { + await traceChangedFiles( + makeDeps({ interactive: false, traceChanged: true } as any), + makeInput({ + runInnerTrace: vi.fn().mockResolvedValue({ 1: ['./a.stories.js'] }), + untracedFiles: ['./untraced.js'], + }) + ); + expect(log.info).toHaveBeenCalledWith(expect.stringContaining('1 untraced files')); + }); }); diff --git a/node-src/tasks/prepare/traceChangedFiles.ts b/node-src/tasks/prepare/traceChangedFiles.ts index 8e76176ef..12ca90dc7 100644 --- a/node-src/tasks/prepare/traceChangedFiles.ts +++ b/node-src/tasks/prepare/traceChangedFiles.ts @@ -1,77 +1,64 @@ -import * as turbosnap from '@cli/turbosnap'; -import semver from 'semver'; - -import { transitionTo } from '../../lib/tasks'; -import { rewriteErrorMessage } from '../../lib/utilities'; -import { Context, Task } from '../../types'; -import missingStatsFile from '../../ui/messages/errors/missingStatsFile'; -import { bailed, traced, tracing } from '../../ui/tasks/prepare'; +import type { Deps, FileInfo } from '../../types'; // These are the special characters that need to be escaped in the filename // because they are used as special characters in picomatch const SPECIAL_CHARS_REGEXP = /([$()*+?[\]^])/g; +type TraceChangedFilesDeps = Pick; + +export interface TraceChangedFilesInput { + untracedFiles: string[] | undefined; + transitionToTracing: () => void; + transitionToTraced: () => void; + transitionToBailed: () => void; + statsPath: FileInfo['statsPath']; + runInnerTrace: ( + statsPath: FileInfo['statsPath'] + ) => Promise | undefined>; +} + /** - * Traces which story files are affected by recent changes using TurboSnap. - * Analyzes changed files to determine which stories need to be tested. + * Runs the TurboSnap inner trace and reports affected story files. * - * @param ctx - The CLI context containing git info and TurboSnap configuration - * @param task - The current Listr task for UI updates + * @param deps - The CLI dependencies containing logging and options + * @param input - The input containing trace callbacks and untraced file metadata * - * @throws {Error} if stats file is missing or tracing fails + * @returns Escaped affected story file paths, or undefined if + * the inner trace bailed (no affected files identified). */ -// TODO: refactor this function -// eslint-disable-next-line complexity -export async function traceChangedFiles(ctx: Context, task: Task) { - if (!ctx.turboSnap || ctx.turboSnap.unavailable) return; - if (!ctx.git.changedFiles) return; - if (!ctx.fileInfo?.statsPath) { - // If we don't know the SB version, we should assume we don't support `--stats-json` - const nonLegacyStatsSupported = - ctx.storybook?.version && - semver.gte(semver.coerce(ctx.storybook.version) || '0.0.0', '8.0.0'); +export async function traceChangedFiles( + deps: TraceChangedFilesDeps, + input: TraceChangedFilesInput +): Promise { + input.transitionToTracing(); - ctx.turboSnap.bailReason = { missingStatsFile: true }; - throw new Error(missingStatsFile({ legacy: !nonLegacyStatsSupported })); + const innerResult = await input.runInnerTrace(input.statsPath); + if (!innerResult) { + input.transitionToBailed(); + return; } - transitionTo(tracing)(ctx, task); + // Escape special characters in the filename so it does not conflict with picomatch + const onlyStoryFiles = Object.keys(innerResult).map((key) => + key.replaceAll(SPECIAL_CHARS_REGEXP, String.raw`\$1`) + ); - const { statsPath } = ctx.fileInfo; - const { changedFiles } = ctx.git; - - try { - const onlyStoryFiles = await turbosnap.traceChangedFiles(ctx); - if (onlyStoryFiles) { - // Escape special characters in the filename so it does not conflict with picomatch - ctx.onlyStoryFiles = Object.keys(onlyStoryFiles).map((key) => - key.replaceAll(SPECIAL_CHARS_REGEXP, String.raw`\$1`) + if (!deps.options.interactive) { + if (!deps.options.traceChanged) { + deps.log.info( + `Found affected story files:\n${Object.entries(innerResult) + .flatMap(([id, files]) => files.map((f) => ` ${f} [${id}]`)) + .join('\n')}` ); - - if (!ctx.options.interactive) { - if (!ctx.options.traceChanged) { - ctx.log.info( - `Found affected story files:\n${Object.entries(onlyStoryFiles) - .flatMap(([id, files]) => files.map((f) => ` ${f} [${id}]`)) - .join('\n')}` - ); - } - if (ctx.untracedFiles && ctx.untracedFiles.length > 0) { - ctx.log.info( - `Encountered ${ctx.untracedFiles.length} untraced files:\n${ctx.untracedFiles - .map((f) => ` ${f}`) - .join('\n')}` - ); - } - } - transitionTo(traced)(ctx, task); - } else { - transitionTo(bailed)(ctx, task); } - } catch (err) { - if (!ctx.options.interactive) { - ctx.log.info('Failed to retrieve dependent story files', { statsPath, changedFiles, err }); + if (input.untracedFiles && input.untracedFiles.length > 0) { + deps.log.info( + `Encountered ${input.untracedFiles.length} untraced files:\n${input.untracedFiles + .map((f) => ` ${f}`) + .join('\n')}` + ); } - throw rewriteErrorMessage(err, `Could not retrieve dependent story files.\n${err.message}`); } + input.transitionToTraced(); + return onlyStoryFiles; } From 36266ea5e96d4e8360fa9b52ac93aec8888e9a04 Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Mon, 11 May 2026 10:49:43 -0400 Subject: [PATCH 5/7] Refactor turbosnap lib to parameterize stats path --- node-src/lib/turbosnap/index.test.ts | 17 ++++++----------- node-src/lib/turbosnap/index.ts | 12 +++++++----- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/node-src/lib/turbosnap/index.test.ts b/node-src/lib/turbosnap/index.test.ts index f5c4e76e8..a58ad9a6d 100644 --- a/node-src/lib/turbosnap/index.test.ts +++ b/node-src/lib/turbosnap/index.test.ts @@ -52,11 +52,10 @@ describe('traceChangedFiles', () => { http, options: {}, sourceDir: '/static/', - fileInfo: { statsPath: '/static/preview-stats.json' }, git: { changedFiles: ['./example.js'] }, turboSnap: {}, } as any; - const onlyStoryFiles = await traceChangedFiles(ctx); + const onlyStoryFiles = await traceChangedFiles(ctx, '/static/preview-stats.json'); expect(onlyStoryFiles).toStrictEqual(deps); expect(findChangedDependencies).not.toHaveBeenCalled(); @@ -74,11 +73,10 @@ describe('traceChangedFiles', () => { http, options: {}, sourceDir: '/static/', - fileInfo: { statsPath: '/static/preview-stats.json' }, git: { changedFiles: ['./example.js', './package.json'], packageMetadataChanges }, turboSnap: {}, } as any; - await traceChangedFiles(ctx); + await traceChangedFiles(ctx, '/static/preview-stats.json'); expect(ctx.turboSnap.bailReason).toEqual({ changedPackageFiles: ['./package.json'] }); expect(findChangedPackageFiles).toHaveBeenCalledWith(ctx, packageMetadataChanges); @@ -96,11 +94,10 @@ describe('traceChangedFiles', () => { http, options: {}, sourceDir: '/static/', - fileInfo: { statsPath: '/static/preview-stats.json' }, git: { changedFiles: ['./example.js', './package.json'], packageMetadataChanges }, turboSnap: {}, } as any; - await traceChangedFiles(ctx); + await traceChangedFiles(ctx, '/static/preview-stats.json'); expect(ctx.git.changedDependencyNames).toEqual(['moment']); }); @@ -120,11 +117,10 @@ describe('traceChangedFiles', () => { http, options: { storybookBaseDir: '/wrong' }, sourceDir: '/static/', - fileInfo: { statsPath: '/static/preview-stats.json' }, git: { changedFiles: ['./example.js'] }, turboSnap: {}, } as any; - await expect(traceChangedFiles(ctx)).rejects.toThrow(); + await expect(traceChangedFiles(ctx, '/static/preview-stats.json')).rejects.toThrow(); expect(ctx.exitCode).toBe(exitCodes.INVALID_OPTIONS); }); @@ -141,11 +137,10 @@ describe('traceChangedFiles', () => { http, options: {}, sourceDir: '/static/', - fileInfo: { statsPath: '/static/preview-stats.json' }, git: { changedFiles: ['./example.js', './package.json'], packageMetadataChanges }, turboSnap: {}, } as any; - const onlyStoryFiles = await traceChangedFiles(ctx); + const onlyStoryFiles = await traceChangedFiles(ctx, '/static/preview-stats.json'); expect(ctx.turboSnap.bailReason).toBeUndefined(); expect(onlyStoryFiles).toStrictEqual(deps); @@ -164,7 +159,7 @@ describe('traceChangedFiles', () => { turboSnap: {}, } as any; - await expect(traceChangedFiles(ctx)).rejects.toThrow(); + await expect(traceChangedFiles(ctx, undefined as any)).rejects.toThrow(); expect(ctx.turboSnap.bailReason).toEqual({ missingStatsFile: true }); }); }); diff --git a/node-src/lib/turbosnap/index.ts b/node-src/lib/turbosnap/index.ts index 707cb5f9e..333bc977a 100644 --- a/node-src/lib/turbosnap/index.ts +++ b/node-src/lib/turbosnap/index.ts @@ -1,7 +1,7 @@ import semver from 'semver'; import { readStatsFile } from '../../tasks/readStatsFile'; -import { Context } from '../../types'; +import { Context, FileInfo } from '../../types'; import missingStatsFile from '../../ui/messages/errors/missingStatsFile'; import bailFile from '../../ui/messages/warnings/bailFile'; import { checkStorybookBaseDirectory } from '../checkStorybookBaseDirectory'; @@ -9,11 +9,14 @@ import { findChangedDependencies } from './findChangedDependencies'; import { findChangedPackageFiles } from './findChangedPackageFiles'; import { getDependentStoryFiles } from './getDependentStoryFiles'; -// eslint-disable-next-line complexity -export const traceChangedFiles = async (ctx: Context) => { +export const traceChangedFiles = async ( + ctx: Omit, + statsPath: FileInfo['statsPath'] + // eslint-disable-next-line complexity +) => { if (!ctx.turboSnap || ctx.turboSnap.unavailable) return; if (!ctx.git.changedFiles) return; - if (!ctx.fileInfo?.statsPath) { + if (!statsPath) { // If we don't know the SB version, we should assume we don't support `--stats-json` const nonLegacyStatsSupported = ctx.storybook?.version && @@ -23,7 +26,6 @@ export const traceChangedFiles = async (ctx: Context) => { throw new Error(missingStatsFile({ legacy: !nonLegacyStatsSupported })); } - const { statsPath } = ctx.fileInfo; const { changedFiles, packageMetadataChanges } = ctx.git; // eslint-disable-next-line @typescript-eslint/no-invalid-void-type From 05eed3488b32e70f785f428db78ebb080403aaa1 Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Tue, 12 May 2026 09:16:57 -0400 Subject: [PATCH 6/7] Prepare task orchestration --- node-src/tasks/prepare/index.ts | 56 +++++++++----------- node-src/tasks/prepare/validateFiles.test.ts | 26 ++++++--- node-src/tasks/prepare/validateFiles.ts | 20 +++---- 3 files changed, 54 insertions(+), 48 deletions(-) diff --git a/node-src/tasks/prepare/index.ts b/node-src/tasks/prepare/index.ts index bde1961a0..0530a239f 100644 --- a/node-src/tasks/prepare/index.ts +++ b/node-src/tasks/prepare/index.ts @@ -18,11 +18,8 @@ import { tracing, validating, } from '../../ui/tasks/prepare'; -import { calculateFileHashes, CalculateFileHashesInput } from './calculateFileHashes'; -import { - traceChangedFiles, - TraceChangedFilesInput, -} from './traceChangedFiles'; +import { calculateFileHashes } from './calculateFileHashes'; +import { traceChangedFiles, TraceChangedFilesInput } from './traceChangedFiles'; import { validateAndroidArtifact } from './validateAndroidArtifact'; import { isValidReactNativeStorybook, @@ -31,10 +28,6 @@ import { ValidateFilesInput, } from './validateFiles'; - -} from '../../ui/tasks/prepare'; -} from './validateFiles'; - type PrepareDeps = Pick; type PrepareTraceInput = TraceChangedFilesInput & { @@ -44,7 +37,6 @@ type PrepareTraceInput = TraceChangedFilesInput & { }; interface PrepareInput { - sourceDir: string; validateFilesInput: ValidateFilesInput; invalidAndroidArtifactError: Error; traceChangedFilesInput: Omit; @@ -52,6 +44,8 @@ interface PrepareInput { } interface PrepareOutput { + fileInfo: FileInfo; + sourceDir: string; hashes?: Record; onlyStoryFiles?: string[]; } @@ -61,10 +55,10 @@ export async function runPrepare( deps: PrepareDeps, input: PrepareInput ): Promise> { - const fileInfo = await validateFiles(deps, input.validateFilesInput); + const { fileInfo, sourceDir } = await validateFiles(deps, input.validateFilesInput); if (input.validateFilesInput.browsers.includes('android')) { - const isValidAndroidArtifact = await validateAndroidArtifact(input.sourceDir); + const isValidAndroidArtifact = await validateAndroidArtifact(sourceDir); if (!isValidAndroidArtifact) { throw input.invalidAndroidArtifactError; } @@ -96,12 +90,9 @@ export async function runPrepare( let hashes: Record | undefined; if (deps.options.fileHashing) { input.transitionToHashing(); - const calculateFileHashesInput: CalculateFileHashesInput = { - fileInfo, - sourceDir: input.sourceDir, - }; + try { - hashes = await calculateFileHashes(deps, calculateFileHashesInput); + hashes = await calculateFileHashes(deps, { fileInfo, sourceDir }); } catch (err) { deps.log.warn('Failed to calculate file hashes'); deps.log.debug(err); @@ -109,7 +100,7 @@ export async function runPrepare( } return { kind: 'continue', - output: { hashes, onlyStoryFiles }, + output: { fileInfo, sourceDir, hashes, onlyStoryFiles }, }; } @@ -117,8 +108,6 @@ export const extractPrepareInput = ( ctx: Context, listrTask: Listr.ListrTaskWrapper ): PrepareInput => { - // eslint-disable-next-line unicorn/prevent-abbreviations - const sourceDir = ctx.sourceDir; const validateFilesInput: ValidateFilesInput = { browsers: ctx.announcedBuild?.browsers || [], buildLogFile: ctx.buildLogFile, @@ -126,7 +115,7 @@ export const extractPrepareInput = ( return new Error(invalid(ctx, err).output); }, isReactNativeApp: ctx.isReactNativeApp || false, - sourceDir, + sourceDir: ctx.sourceDir, validationErrorBuilder(missingFiles: string[] | undefined): Error { return ctx.isReactNativeApp ? new Error(invalidReactNative(ctx, missingFiles).output) @@ -164,7 +153,6 @@ export const extractPrepareInput = ( }; return { - sourceDir, validateFilesInput, invalidAndroidArtifactError: new Error(invalidAndroidArtifact(ctx).output), traceChangedFilesInput, @@ -172,6 +160,15 @@ export const extractPrepareInput = ( }; }; +export const applyPrepareOutput = ( + ctx: Context, + { fileInfo, sourceDir, hashes, onlyStoryFiles }: PrepareOutput +) => { + ctx.fileInfo = { ...fileInfo, hashes }; + ctx.sourceDir = sourceDir; + ctx.onlyStoryFiles = onlyStoryFiles; +}; + /** * Sets up the Listr task for preparing the built storybook for upload to Chromatic. * @@ -186,13 +183,12 @@ export default function main(ctx: Context) { skip: (ctx: Context) => { return !!ctx.skip; }, - steps: [ - transitionTo(validating), - validateFiles, - validateAndroidArtifact, - traceChangedFiles, - calculateFileHashes, - transitionTo(success, true), - ], + transitions: { + pending: validating, + success, + }, + extractInput: extractPrepareInput, + applyOutput: applyPrepareOutput, + run: runPrepare, }); } diff --git a/node-src/tasks/prepare/validateFiles.test.ts b/node-src/tasks/prepare/validateFiles.test.ts index e4f7ae241..415104cc7 100644 --- a/node-src/tasks/prepare/validateFiles.test.ts +++ b/node-src/tasks/prepare/validateFiles.test.ts @@ -39,13 +39,14 @@ const makeInput = (overrides: Partial = {}): ValidateFilesIn }); describe('validateFiles', () => { - it('returns fileInfo for the source directory', async () => { + it('returns fileInfo and sourceDir for the source directory', async () => { readdirSyncMock.mockReturnValue(['iframe.html', 'index.html'] as any); statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); - const fileInfo = await validateFiles(makeDeps(), makeInput()); + const result = await validateFiles(makeDeps(), makeInput({ sourceDir: '/static/' })); - expect(fileInfo).toEqual( + expect(result.sourceDir).toBe('/static/'); + expect(result.fileInfo).toEqual( expect.objectContaining({ lengths: [ { contentLength: 42, knownAs: 'iframe.html', pathname: 'iframe.html' }, @@ -67,7 +68,7 @@ describe('validateFiles', () => { return { isDirectory: () => false, size: 42 } as any; }); - const fileInfo = await validateFiles(makeDeps(), makeInput({ sourceDir: '.' })); + const { fileInfo } = await validateFiles(makeDeps(), makeInput({ sourceDir: '.' })); expect(fileInfo).toEqual( expect.objectContaining({ @@ -118,7 +119,7 @@ describe('validateFiles', () => { }); describe('with buildLogFile', () => { - it('retries using outputDir from build-storybook.log when initial validation fails', async () => { + it('returns the corrected sourceDir from build-storybook.log when initial validation fails', async () => { readdirSyncMock.mockReturnValueOnce([] as any); readdirSyncMock.mockReturnValueOnce(['iframe.html', 'index.html'] as any); statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); @@ -134,11 +135,11 @@ describe('validateFiles', () => { validator, }); - const fileInfo = await validateFiles(makeDeps(), input); + const result = await validateFiles(makeDeps(), input); expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('Unexpected build directory')); - expect(input.sourceDir).toBe('/var/storybook-static'); - expect(fileInfo).toEqual( + expect(result.sourceDir).toBe('/var/storybook-static'); + expect(result.fileInfo).toEqual( expect.objectContaining({ paths: ['iframe.html', 'index.html'], total: 84, @@ -146,6 +147,15 @@ describe('validateFiles', () => { ); }); + it('returns the original sourceDir when no override happens', async () => { + readdirSyncMock.mockReturnValue(['iframe.html', 'index.html'] as any); + statSyncMock.mockReturnValue({ isDirectory: () => false, size: 42 } as any); + + const result = await validateFiles(makeDeps(), makeInput({ sourceDir: '/static/' })); + + expect(result.sourceDir).toBe('/static/'); + }); + it('throws via validationErrorBuilder when retried validation still fails', async () => { readdirSyncMock.mockReturnValue([] as any); readFileSyncMock.mockReturnValue('info => Output directory: /var/storybook-static'); diff --git a/node-src/tasks/prepare/validateFiles.ts b/node-src/tasks/prepare/validateFiles.ts index ce96c63a2..daa510680 100644 --- a/node-src/tasks/prepare/validateFiles.ts +++ b/node-src/tasks/prepare/validateFiles.ts @@ -161,17 +161,19 @@ export const isValidReactNativeStorybook = ( * @param deps - Dependencies for logging and file info retrieval * @param input - Input parameters for validation * - * @returns File information object if validation succeeds, or throws an error + * @returns The validated fileInfo and the (possibly corrected) sourceDir. * * @throws {Error} if no valid Storybook build is found */ export async function validateFiles( deps: ValidateFilesDeps, input: ValidateFilesInput -): Promise { +): Promise<{ fileInfo: FileInfo; sourceDir: string }> { + // eslint-disable-next-line unicorn/prevent-abbreviations + let sourceDir = input.sourceDir; let fileInfo: FileInfo; try { - fileInfo = getFileInfo(input.sourceDir); + fileInfo = getFileInfo(sourceDir); } catch (err) { deps.log.debug(err); throw input.getFileInfoErrorBuilder(err); @@ -181,12 +183,10 @@ export async function validateFiles( try { const buildLog = readFileSync(input.buildLogFile, 'utf8'); const outputDirectory = getOutputDirectory(buildLog); - if (outputDirectory && outputDirectory !== input.sourceDir) { - deps.log.warn( - deviatingOutputDirectory({ sourceDir: input.sourceDir, ...deps }, outputDirectory) - ); - input.sourceDir = outputDirectory; - fileInfo = getFileInfo(input.sourceDir); + if (outputDirectory && outputDirectory !== sourceDir) { + deps.log.warn(deviatingOutputDirectory({ sourceDir, ...deps }, outputDirectory)); + sourceDir = outputDirectory; + fileInfo = getFileInfo(sourceDir); } } catch (err) { deps.log.debug(err); @@ -197,5 +197,5 @@ export async function validateFiles( if (!validatorResult.valid) { throw input.validationErrorBuilder(validatorResult.missingFiles); } - return fileInfo; + return { fileInfo, sourceDir }; } From c1ed781117919c1bfa6df7784f1ee2c97a25cec2 Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Wed, 13 May 2026 09:56:45 -0400 Subject: [PATCH 7/7] Add prepare/index.ts tests --- node-src/tasks/prepare/index.test.ts | 783 +++++++++++++++++++++++++++ 1 file changed, 783 insertions(+) create mode 100644 node-src/tasks/prepare/index.test.ts diff --git a/node-src/tasks/prepare/index.test.ts b/node-src/tasks/prepare/index.test.ts new file mode 100644 index 000000000..ec5abc219 --- /dev/null +++ b/node-src/tasks/prepare/index.test.ts @@ -0,0 +1,783 @@ +/* eslint-disable max-lines */ +import * as turbosnap from '@cli/turbosnap'; +import type Listr from 'listr'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import TestLogger from '../../lib/testLogger'; +import { Context, FileInfo } from '../../types'; +import missingStatsFile from '../../ui/messages/errors/missingStatsFile'; +import { + bailed, + hashing, + invalid, + invalidAndroidArtifact, + invalidReactNative, + traced, + tracing, +} from '../../ui/tasks/prepare'; +import { calculateFileHashes } from './calculateFileHashes'; +import { applyPrepareOutput, extractPrepareInput, runPrepare } from './index'; +import { traceChangedFiles } from './traceChangedFiles'; +import { validateAndroidArtifact } from './validateAndroidArtifact'; +import { isValidReactNativeStorybook, isValidStorybook, validateFiles } from './validateFiles'; + +vi.mock('@cli/turbosnap', () => ({ + traceChangedFiles: vi.fn(), +})); +vi.mock('./validateAndroidArtifact'); +vi.mock('./traceChangedFiles'); +vi.mock('./calculateFileHashes'); +vi.mock('./validateFiles', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, validateFiles: vi.fn() }; +}); +vi.mock('../../ui/tasks/prepare', () => ({ + initial: vi.fn(() => ({ status: 'initial', title: 'initial-title' })), + validating: vi.fn(() => ({ + status: 'pending', + title: 'validating-title', + output: 'validating-output', + })), + invalid: vi.fn((_ctx, err?: Error) => ({ + status: 'error', + title: 'invalid-title', + output: err ? `invalid-output:${err.message}` : 'invalid-output', + })), + invalidAndroidArtifact: vi.fn(() => ({ + status: 'error', + title: 'invalidAndroidArtifact-title', + output: 'invalidAndroidArtifact-output', + })), + invalidReactNative: vi.fn((_ctx, missingFiles?: string[]) => ({ + status: 'error', + title: 'invalidReactNative-title', + output: `invalidReactNative-output:${(missingFiles || []).join(',')}`, + })), + tracing: vi.fn(() => ({ + status: 'pending', + title: 'tracing-title', + output: 'tracing-output', + })), + traced: vi.fn(() => ({ + status: 'pending', + title: 'traced-title', + output: 'traced-output', + })), + bailed: vi.fn(() => ({ + status: 'pending', + title: 'bailed-title', + output: 'bailed-output', + })), + hashing: vi.fn(() => ({ + status: 'pending', + title: 'hashing-title', + output: 'hashing-output', + })), + success: vi.fn(() => ({ status: 'success', title: 'success-title' })), +})); +vi.mock('../../ui/messages/errors/missingStatsFile', () => ({ + default: vi.fn(({ legacy }: { legacy: boolean }) => + legacy ? 'missing-stats:legacy' : 'missing-stats:non-legacy' + ), +})); + +const validateFilesMock = vi.mocked(validateFiles); +const validateAndroidArtifactMock = vi.mocked(validateAndroidArtifact); +const traceChangedFilesMock = vi.mocked(traceChangedFiles); +const calculateFileHashesMock = vi.mocked(calculateFileHashes); +const turbosnapTraceMock = vi.mocked(turbosnap.traceChangedFiles); +const invalidAndroidArtifactMock = vi.mocked(invalidAndroidArtifact); +const invalidReactNativeMock = vi.mocked(invalidReactNative); +const invalidMock = vi.mocked(invalid); +const missingStatsFileMock = vi.mocked(missingStatsFile); +const tracingMock = vi.mocked(tracing); +const tracedMock = vi.mocked(traced); +const bailedMock = vi.mocked(bailed); +const hashingMock = vi.mocked(hashing); + +const log = new TestLogger(); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const fileInfo: FileInfo = { + paths: ['iframe.html', 'index.html'], + statsPath: '/static/preview-stats.json', + lengths: [ + { knownAs: 'iframe.html', pathname: 'iframe.html', contentLength: 42 }, + { knownAs: 'index.html', pathname: 'index.html', contentLength: 42 }, + ], + total: 84, +}; + +const makeDeps = (overrides: { options?: any; env?: any } = {}) => + ({ + log, + env: overrides.env ?? { CHROMATIC_HASH_CONCURRENCY: 1 }, + options: overrides.options ?? {}, + packageJson: {}, + }) as any; + +const makeInput = (overrides: any = {}) => { + const { validateFilesInput, traceChangedFilesInput, ...topLevelOverrides } = overrides; + return { + validateFilesInput: { + browsers: [], + isReactNativeApp: false, + sourceDir: '/static/', + validator: vi.fn(), + validationErrorBuilder: vi.fn(), + getFileInfoErrorBuilder: vi.fn(), + ...validateFilesInput, + }, + invalidAndroidArtifactError: new Error('invalid-android-artifact-error'), + traceChangedFilesInput: { + turboSnap: undefined, + changedFiles: undefined, + untracedFiles: undefined, + missingStatsError: vi.fn(() => new Error('missing-stats-error')), + transitionToTracing: vi.fn(), + transitionToTraced: vi.fn(), + transitionToBailed: vi.fn(), + runInnerTrace: vi.fn(), + ...traceChangedFilesInput, + }, + transitionToHashing: vi.fn(), + ...topLevelOverrides, + }; +}; + +describe('runPrepare', () => { + beforeEach(() => { + validateFilesMock.mockResolvedValue({ fileInfo, sourceDir: '/static/' }); + validateAndroidArtifactMock.mockResolvedValue(true); + traceChangedFilesMock.mockResolvedValue(undefined); + calculateFileHashesMock.mockResolvedValue({}); + }); + + describe('deviating output directory', () => { + it('passes the corrected sourceDir to validateAndroidArtifact', async () => { + validateFilesMock.mockResolvedValue({ fileInfo, sourceDir: '/corrected/' }); + + await runPrepare(makeDeps(), makeInput({ validateFilesInput: { browsers: ['android'] } })); + + expect(validateAndroidArtifactMock).toHaveBeenCalledExactlyOnceWith('/corrected/'); + }); + + it('passes the corrected sourceDir to calculateFileHashes', async () => { + validateFilesMock.mockResolvedValue({ fileInfo, sourceDir: '/corrected/' }); + + await runPrepare(makeDeps({ options: { fileHashing: true } }), makeInput()); + + expect(calculateFileHashesMock).toHaveBeenCalledExactlyOnceWith( + expect.anything(), + expect.objectContaining({ sourceDir: '/corrected/' }) + ); + }); + + it('includes the corrected sourceDir in the output', async () => { + validateFilesMock.mockResolvedValue({ fileInfo, sourceDir: '/corrected/' }); + + const result = await runPrepare(makeDeps(), makeInput()); + + expect(result).toMatchObject({ output: { sourceDir: '/corrected/' } }); + }); + }); + + it('returns continue with composed output from each subtask', async () => { + validateFilesMock.mockResolvedValue({ fileInfo, sourceDir: '/static/' }); + traceChangedFilesMock.mockResolvedValue(['./a.stories.js']); + calculateFileHashesMock.mockResolvedValue({ 'iframe.html': 'hash' }); + + const result = await runPrepare( + makeDeps({ options: { fileHashing: true } }), + makeInput({ + traceChangedFilesInput: { + turboSnap: {}, + changedFiles: ['./example.js'], + }, + }) + ); + + expect(result).toEqual({ + kind: 'continue', + output: { + fileInfo, + sourceDir: '/static/', + hashes: { 'iframe.html': 'hash' }, + onlyStoryFiles: ['./a.stories.js'], + }, + }); + }); + + it('passes statsPath from validated fileInfo to traceChangedFiles', async () => { + const input = makeInput({ + traceChangedFilesInput: { + turboSnap: {}, + changedFiles: ['./example.js'], + }, + }); + + await runPrepare(makeDeps(), input); + + expect(traceChangedFilesMock).toHaveBeenCalledExactlyOnceWith( + expect.anything(), + expect.objectContaining({ statsPath: fileInfo.statsPath }) + ); + }); + + it('passes fileInfo and sourceDir to calculateFileHashes', async () => { + validateFilesMock.mockResolvedValue({ fileInfo, sourceDir: '/build/' }); + + await runPrepare(makeDeps({ options: { fileHashing: true } }), makeInput()); + + expect(calculateFileHashesMock).toHaveBeenCalledExactlyOnceWith(expect.anything(), { + fileInfo, + sourceDir: '/build/', + }); + }); + + describe('android branch', () => { + it('calls validateAndroidArtifact when browsers includes android', async () => { + validateFilesMock.mockResolvedValue({ fileInfo, sourceDir: '/build/' }); + + await runPrepare( + makeDeps(), + makeInput({ validateFilesInput: { browsers: ['android', 'ios'] } }) + ); + + expect(validateAndroidArtifactMock).toHaveBeenCalledExactlyOnceWith('/build/'); + }); + + it('does not call validateAndroidArtifact when browsers omits android', async () => { + await runPrepare( + makeDeps(), + makeInput({ validateFilesInput: { browsers: ['ios', 'chrome'] } }) + ); + + expect(validateAndroidArtifactMock).not.toHaveBeenCalled(); + }); + + it('throws invalidAndroidArtifactError when validateAndroidArtifact returns false', async () => { + validateAndroidArtifactMock.mockResolvedValue(false); + const invalidAndroidArtifactError = new Error('arm-only'); + + await expect( + runPrepare( + makeDeps(), + makeInput({ + invalidAndroidArtifactError, + validateFilesInput: { browsers: ['android'] }, + }) + ) + ).rejects.toBe(invalidAndroidArtifactError); + }); + + it('does not throw when validateAndroidArtifact returns true', async () => { + validateAndroidArtifactMock.mockResolvedValue(true); + + await expect( + runPrepare(makeDeps(), makeInput({ validateFilesInput: { browsers: ['android'] } })) + ).resolves.toBeDefined(); + }); + }); + + describe('turbosnap branch', () => { + it('skips traceChangedFiles when turboSnap is undefined', async () => { + await runPrepare( + makeDeps(), + makeInput({ + traceChangedFilesInput: { turboSnap: undefined, changedFiles: ['./a.js'] }, + }) + ); + + expect(traceChangedFilesMock).not.toHaveBeenCalled(); + }); + + it('skips traceChangedFiles when turboSnap.unavailable is true', async () => { + await runPrepare( + makeDeps(), + makeInput({ + traceChangedFilesInput: { + turboSnap: { unavailable: true }, + changedFiles: ['./a.js'], + }, + }) + ); + + expect(traceChangedFilesMock).not.toHaveBeenCalled(); + }); + + it('skips traceChangedFiles when changedFiles is undefined', async () => { + await runPrepare( + makeDeps(), + makeInput({ + traceChangedFilesInput: { turboSnap: {}, changedFiles: undefined }, + }) + ); + + expect(traceChangedFilesMock).not.toHaveBeenCalled(); + }); + + it('throws missingStatsError when statsPath is empty', async () => { + validateFilesMock.mockResolvedValue({ + fileInfo: { ...fileInfo, statsPath: '' }, + sourceDir: '/static/', + }); + const missingStatsError = vi.fn(() => new Error('missing-stats!')); + + await expect( + runPrepare( + makeDeps(), + makeInput({ + traceChangedFilesInput: { + turboSnap: {}, + changedFiles: ['./a.js'], + missingStatsError, + }, + }) + ) + ).rejects.toThrow('missing-stats!'); + expect(missingStatsError).toHaveBeenCalled(); + expect(traceChangedFilesMock).not.toHaveBeenCalled(); + }); + + it('rewrites error message when inner traceChangedFiles throws', async () => { + traceChangedFilesMock.mockRejectedValue(new Error('inner-trace-failed')); + + await expect( + runPrepare( + makeDeps({ options: { interactive: true } }), + makeInput({ + traceChangedFilesInput: { turboSnap: {}, changedFiles: ['./a.js'] }, + }) + ) + ).rejects.toThrow('Could not retrieve dependent story files.\ninner-trace-failed'); + }); + + it('logs info when traceChangedFiles throws and not interactive', async () => { + traceChangedFilesMock.mockRejectedValue(new Error('inner-trace-failed')); + + await expect( + runPrepare( + makeDeps({ options: { interactive: false } }), + makeInput({ + traceChangedFilesInput: { + turboSnap: {}, + changedFiles: ['./example.js'], + }, + }) + ) + ).rejects.toThrow(); + + expect(log.info).toHaveBeenCalledWith( + 'Failed to retrieve dependent story files', + expect.objectContaining({ + statsPath: fileInfo.statsPath, + changedFiles: ['./example.js'], + }) + ); + }); + + it('does not log info when traceChangedFiles throws and interactive', async () => { + traceChangedFilesMock.mockRejectedValue(new Error('inner-trace-failed')); + + await expect( + runPrepare( + makeDeps({ options: { interactive: true } }), + makeInput({ + traceChangedFilesInput: { + turboSnap: {}, + changedFiles: ['./example.js'], + }, + }) + ) + ).rejects.toThrow(); + + expect(log.info).not.toHaveBeenCalled(); + }); + }); + + describe('file-hashing branch', () => { + it('skips hashing when options.fileHashing is falsy', async () => { + const transitionToHashing = vi.fn(); + + const result = await runPrepare( + makeDeps({ options: {} }), + makeInput({ transitionToHashing }) + ); + + expect(calculateFileHashesMock).not.toHaveBeenCalled(); + expect(transitionToHashing).not.toHaveBeenCalled(); + expect(result).toMatchObject({ output: { hashes: undefined } }); + }); + + it('calls transitionToHashing before calculateFileHashes', async () => { + const order: string[] = []; + const transitionToHashing = vi.fn(() => { + order.push('transition'); + }); + calculateFileHashesMock.mockImplementation(async () => { + order.push('hash'); + return {}; + }); + + await runPrepare( + makeDeps({ options: { fileHashing: true } }), + makeInput({ transitionToHashing }) + ); + + expect(order).toEqual(['transition', 'hash']); + }); + + it('swallows calculateFileHashes errors with warn+debug logs', async () => { + const err = new Error('hash-failed'); + calculateFileHashesMock.mockRejectedValue(err); + + const result = await runPrepare(makeDeps({ options: { fileHashing: true } }), makeInput()); + + expect(result).toMatchObject({ output: { hashes: undefined } }); + expect(log.warn).toHaveBeenCalledWith('Failed to calculate file hashes'); + expect(log.debug).toHaveBeenCalledWith(err); + }); + }); +}); + +const makeListrTask = () => + ({ + title: '', + output: '', + skip: vi.fn(), + report: vi.fn(), + }) as unknown as Listr.ListrTaskWrapper; + +const makeContext = (overrides: Record = {}): Context => + ({ + log, + sourceDir: '/static/', + buildLogFile: undefined, + announcedBuild: undefined, + isReactNativeApp: false, + storybook: undefined, + git: { changedFiles: undefined }, + turboSnap: undefined, + untracedFiles: undefined, + options: {}, + ...overrides, + }) as any; + +describe('extractPrepareInput', () => { + describe('field mapping', () => { + it('passes through sourceDir, buildLogFile, browsers, isReactNativeApp', () => { + const ctx = makeContext({ + sourceDir: '/build/', + buildLogFile: 'build.log', + announcedBuild: { browsers: ['android', 'ios'] }, + isReactNativeApp: true, + }); + + const result = extractPrepareInput(ctx, makeListrTask()); + + expect(result.validateFilesInput).toMatchObject({ + sourceDir: '/build/', + buildLogFile: 'build.log', + browsers: ['android', 'ios'], + isReactNativeApp: true, + }); + }); + + it('defaults browsers to [] when announcedBuild.browsers is undefined', () => { + const ctx = makeContext({ announcedBuild: {} }); + + const result = extractPrepareInput(ctx, makeListrTask()); + + expect(result.validateFilesInput.browsers).toEqual([]); + }); + + it('defaults isReactNativeApp to false when undefined', () => { + const ctx = makeContext({ isReactNativeApp: undefined }); + + const result = extractPrepareInput(ctx, makeListrTask()); + + expect(result.validateFilesInput.isReactNativeApp).toBe(false); + }); + + it('mirrors turboSnap, changedFiles, untracedFiles from ctx', () => { + const ctx = makeContext({ + turboSnap: { foo: 'bar' }, + git: { changedFiles: ['./a.js'] }, + untracedFiles: ['./b.js'], + }); + + const result = extractPrepareInput(ctx, makeListrTask()); + + expect(result.traceChangedFilesInput).toMatchObject({ + turboSnap: { foo: 'bar' }, + changedFiles: ['./a.js'], + untracedFiles: ['./b.js'], + }); + }); + }); + + describe('validator closure', () => { + it('calls isValidStorybook when not React Native', () => { + const ctx = makeContext({ isReactNativeApp: false }); + const result = extractPrepareInput(ctx, makeListrTask()); + + const expected = isValidStorybook({ paths: ['iframe.html'], total: 1 }); + expect( + result.validateFilesInput.validator({ paths: ['iframe.html'], total: 1 } as FileInfo, [ + 'chrome', + ]) + ).toEqual(expected); + }); + + it('calls isValidReactNativeStorybook when React Native', () => { + const ctx = makeContext({ isReactNativeApp: true }); + const result = extractPrepareInput(ctx, makeListrTask()); + + const expected = isValidReactNativeStorybook( + { paths: ['storybook.apk', 'manifest.json'], total: 1 }, + ['android'] + ); + expect( + result.validateFilesInput.validator( + { paths: ['storybook.apk', 'manifest.json'], total: 1 } as FileInfo, + ['android'] + ) + ).toEqual(expected); + }); + }); + + describe('validationErrorBuilder closure', () => { + it('uses invalid() for non-React-Native ctx', () => { + const ctx = makeContext({ isReactNativeApp: false }); + const result = extractPrepareInput(ctx, makeListrTask()); + + const err = result.validateFilesInput.validationErrorBuilder([]); + + expect(err.message).toBe('invalid-output'); + expect(invalidMock).toHaveBeenCalledWith(ctx); + }); + + it('uses invalidReactNative() with missingFiles for React Native ctx', () => { + const ctx = makeContext({ isReactNativeApp: true }); + const result = extractPrepareInput(ctx, makeListrTask()); + + const err = result.validateFilesInput.validationErrorBuilder([ + 'storybook.apk', + 'manifest.json', + ]); + + expect(err.message).toBe('invalidReactNative-output:storybook.apk,manifest.json'); + expect(invalidReactNativeMock).toHaveBeenCalledWith(ctx, ['storybook.apk', 'manifest.json']); + }); + }); + + describe('getFileInfoErrorBuilder closure', () => { + it('wraps cause via invalid(ctx, err)', () => { + const ctx = makeContext(); + const cause = new Error('ENOENT'); + const result = extractPrepareInput(ctx, makeListrTask()); + + const err = result.validateFilesInput.getFileInfoErrorBuilder(cause); + + expect(err.message).toBe('invalid-output:ENOENT'); + expect(invalidMock).toHaveBeenCalledWith(ctx, cause); + }); + }); + + describe('missingStatsError closure', () => { + it('sets turboSnap.bailReason when turboSnap is defined', () => { + const turboSnap: any = {}; + const ctx = makeContext({ turboSnap }); + const result = extractPrepareInput(ctx, makeListrTask()); + + result.traceChangedFilesInput.missingStatsError(); + + expect(turboSnap.bailReason).toEqual({ missingStatsFile: true }); + }); + + it('does not throw when turboSnap is undefined', () => { + const ctx = makeContext({ turboSnap: undefined }); + const result = extractPrepareInput(ctx, makeListrTask()); + + expect(() => result.traceChangedFilesInput.missingStatsError()).not.toThrow(); + }); + + it('builds non-legacy error when storybook.version >= 8.0.0', () => { + const ctx = makeContext({ storybook: { version: '8.1.0' } }); + const result = extractPrepareInput(ctx, makeListrTask()); + + const err = result.traceChangedFilesInput.missingStatsError(); + + expect(err.message).toBe('missing-stats:non-legacy'); + expect(missingStatsFileMock).toHaveBeenCalledWith({ legacy: false }); + }); + + it('builds legacy error when storybook.version < 8.0.0', () => { + const ctx = makeContext({ storybook: { version: '7.6.0' } }); + const result = extractPrepareInput(ctx, makeListrTask()); + + const err = result.traceChangedFilesInput.missingStatsError(); + + expect(err.message).toBe('missing-stats:legacy'); + expect(missingStatsFileMock).toHaveBeenCalledWith({ legacy: true }); + }); + + it('builds legacy error when storybook.version is unknown', () => { + const ctx = makeContext({ storybook: undefined }); + const result = extractPrepareInput(ctx, makeListrTask()); + + const err = result.traceChangedFilesInput.missingStatsError(); + + expect(err.message).toBe('missing-stats:legacy'); + expect(missingStatsFileMock).toHaveBeenCalledWith({ legacy: true }); + }); + }); + + describe('invalidAndroidArtifactError', () => { + it('uses invalidAndroidArtifact(ctx).output as its message', () => { + const ctx = makeContext(); + const result = extractPrepareInput(ctx, makeListrTask()); + + expect(result.invalidAndroidArtifactError.message).toBe('invalidAndroidArtifact-output'); + expect(invalidAndroidArtifactMock).toHaveBeenCalledWith(ctx); + }); + }); + + describe('transition closures', () => { + it('transitionToTracing mutates listrTask using tracing UI state', () => { + const ctx = makeContext(); + const listrTask = makeListrTask(); + const result = extractPrepareInput(ctx, listrTask); + + result.traceChangedFilesInput.transitionToTracing(); + + expect(tracingMock).toHaveBeenCalledWith(ctx); + expect(listrTask.title).toBe('tracing-title'); + expect(listrTask.output).toBe('tracing-output'); + }); + + it('transitionToTraced mutates listrTask using traced UI state', () => { + const ctx = makeContext(); + const listrTask = makeListrTask(); + const result = extractPrepareInput(ctx, listrTask); + + result.traceChangedFilesInput.transitionToTraced(); + + expect(tracedMock).toHaveBeenCalledWith(ctx); + expect(listrTask.title).toBe('traced-title'); + expect(listrTask.output).toBe('traced-output'); + }); + + it('transitionToBailed mutates listrTask using bailed UI state', () => { + const ctx = makeContext(); + const listrTask = makeListrTask(); + const result = extractPrepareInput(ctx, listrTask); + + result.traceChangedFilesInput.transitionToBailed(); + + expect(bailedMock).toHaveBeenCalledWith(ctx); + expect(listrTask.title).toBe('bailed-title'); + expect(listrTask.output).toBe('bailed-output'); + }); + + it('transitionToHashing mutates listrTask using hashing UI state', () => { + const ctx = makeContext(); + const listrTask = makeListrTask(); + const result = extractPrepareInput(ctx, listrTask); + + result.transitionToHashing(); + + expect(hashingMock).toHaveBeenCalledWith(ctx); + expect(listrTask.title).toBe('hashing-title'); + expect(listrTask.output).toBe('hashing-output'); + }); + }); + + describe('runInnerTrace closure', () => { + it('forwards (ctx, statsPath) to turbosnap.traceChangedFiles', () => { + const ctx = makeContext(); + const result = extractPrepareInput(ctx, makeListrTask()); + + result.traceChangedFilesInput.runInnerTrace('/some/stats.json'); + + expect(turbosnapTraceMock).toHaveBeenCalledWith(ctx, '/some/stats.json'); + }); + }); +}); + +describe('applyPrepareOutput', () => { + it('writes the corrected sourceDir to ctx', () => { + const ctx = makeContext({ sourceDir: '/original/' }); + + applyPrepareOutput(ctx, { + fileInfo, + hashes: undefined, + onlyStoryFiles: undefined, + sourceDir: '/corrected/', + }); + + expect(ctx.sourceDir).toBe('/corrected/'); + }); + + it('sets ctx.fileInfo with hashes merged in', () => { + const ctx = makeContext(); + const hashes = { 'iframe.html': 'hash-a' }; + + applyPrepareOutput(ctx, { + fileInfo, + hashes, + onlyStoryFiles: undefined, + sourceDir: '/static/', + }); + + expect(ctx.fileInfo).toEqual({ ...fileInfo, hashes }); + }); + + it('sets ctx.fileInfo with hashes undefined when hashes omitted', () => { + const ctx = makeContext(); + + applyPrepareOutput(ctx, { + fileInfo, + hashes: undefined, + onlyStoryFiles: undefined, + sourceDir: '/static/', + }); + + expect(ctx.fileInfo).toEqual({ ...fileInfo, hashes: undefined }); + expect(ctx.fileInfo).toHaveProperty('hashes', undefined); + }); + + it('sets ctx.onlyStoryFiles from output', () => { + const ctx = makeContext(); + + applyPrepareOutput(ctx, { + fileInfo, + hashes: undefined, + onlyStoryFiles: ['./a.stories.js'], + sourceDir: '/static/', + }); + + expect(ctx.onlyStoryFiles).toEqual(['./a.stories.js']); + }); + + it('sets ctx.onlyStoryFiles to undefined when output omits it', () => { + const ctx = makeContext({ onlyStoryFiles: ['stale'] }); + + applyPrepareOutput(ctx, { + fileInfo, + hashes: undefined, + onlyStoryFiles: undefined, + sourceDir: '/static/', + }); + + expect(ctx.onlyStoryFiles).toBeUndefined(); + }); +});