diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts index 6019d294ae9..8b3ff2e1991 100644 --- a/packages/cli-kit/src/public/node/system.test.ts +++ b/packages/cli-kit/src/public/node/system.test.ts @@ -1,5 +1,5 @@ import * as system from './system.js' -import {execa} from 'execa' +import {execa, execaCommand} from 'execa' import {describe, expect, test, vi} from 'vitest' import which from 'which' import {Readable} from 'stream' @@ -40,6 +40,233 @@ describe('captureOutput', () => { }) }) +describe('captureOutputWithExitCode', () => { + test('returns stdout, stderr, and exitCode on success', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/command') + vi.mocked(execa).mockResolvedValueOnce({stdout: 'output', stderr: '', exitCode: 0} as any) + + // When + const got = await system.captureOutputWithExitCode('command', [], {cwd: '/currentDirectory'}) + + // Then + expect(got).toEqual({stdout: 'output', stderr: '', exitCode: 0}) + }) + + test('returns non-zero exit code without throwing', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/command') + vi.mocked(execa).mockResolvedValueOnce({stdout: '', stderr: 'error message', exitCode: 1} as any) + + // When + const got = await system.captureOutputWithExitCode('command', [], {cwd: '/currentDirectory'}) + + // Then + expect(got).toEqual({stdout: '', stderr: 'error message', exitCode: 1}) + }) + + test('raises an error if the command to run is found in the current directory', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/currentDirectory/command') + + // When + const got = system.captureOutputWithExitCode('command', [], {cwd: '/currentDirectory'}) + + // Then + await expect(got).rejects.toThrowError('Skipped run of unsecure binary command found in the current directory.') + }) +}) + +describe('captureCommandWithExitCode', () => { + test('returns stdout, stderr, and exitCode on success', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/echo') + vi.mocked(execa).mockResolvedValueOnce({stdout: 'hello', stderr: '', exitCode: 0} as any) + + // When + const got = await system.captureCommandWithExitCode('echo hello') + + // Then + expect(got).toEqual({stdout: 'hello', stderr: '', exitCode: 0}) + expect(execa).toHaveBeenCalledWith('echo', ['hello'], expect.objectContaining({reject: false})) + }) + + test('returns non-zero exit code without throwing', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/exit') + vi.mocked(execa).mockResolvedValueOnce({stdout: '', stderr: 'command failed', exitCode: 1} as any) + + // When + const got = await system.captureCommandWithExitCode('exit 1') + + // Then + expect(got).toEqual({stdout: '', stderr: 'command failed', exitCode: 1}) + }) + + test('handles command with spaces in arguments (quoted strings)', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/ls') + vi.mocked(execa).mockResolvedValueOnce({stdout: 'found', stderr: '', exitCode: 0} as any) + + // When + const got = await system.captureCommandWithExitCode('ls "my folder"') + + // Then + expect(got).toEqual({stdout: 'found', stderr: '', exitCode: 0}) + // The quoted argument should be parsed into a single argument without quotes + expect(execa).toHaveBeenCalledWith('ls', ['my folder'], expect.objectContaining({reject: false})) + }) + + test('handles shopify theme push with quoted theme name', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/shopify') + vi.mocked(execa).mockResolvedValueOnce({stdout: 'success', stderr: '', exitCode: 0} as any) + + // When + const got = await system.captureCommandWithExitCode('shopify theme push --theme "My Theme Name"') + + // Then + expect(got).toEqual({stdout: 'success', stderr: '', exitCode: 0}) + // The quoted theme name should be parsed as a single argument + expect(execa).toHaveBeenCalledWith( + 'shopify', + ['theme', 'push', '--theme', 'My Theme Name'], + expect.objectContaining({reject: false}), + ) + }) + + test('handles single-quoted strings', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/echo') + vi.mocked(execa).mockResolvedValueOnce({stdout: 'hello world', stderr: '', exitCode: 0} as any) + + // When + const got = await system.captureCommandWithExitCode("echo 'hello world'") + + // Then + expect(got).toEqual({stdout: 'hello world', stderr: '', exitCode: 0}) + expect(execa).toHaveBeenCalledWith('echo', ['hello world'], expect.objectContaining({reject: false})) + }) + + test('uses provided cwd option', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/ls') + vi.mocked(execa).mockResolvedValueOnce({stdout: '', stderr: '', exitCode: 0} as any) + + // When + await system.captureCommandWithExitCode('ls', {cwd: '/custom/path'}) + + // Then + expect(execa).toHaveBeenCalledWith('ls', [], expect.objectContaining({cwd: '/custom/path'})) + }) + + test('merges custom env with process.env', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/env') + vi.mocked(execa).mockResolvedValueOnce({stdout: '', stderr: '', exitCode: 0} as any) + + // When + await system.captureCommandWithExitCode('env', {env: {MY_VAR: 'value'}}) + + // Then + expect(execa).toHaveBeenCalledWith( + 'env', + [], + expect.objectContaining({ + env: expect.objectContaining({MY_VAR: 'value'}), + }), + ) + }) + + test('defaults exitCode to 0 when undefined', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/cmd') + vi.mocked(execa).mockResolvedValueOnce({stdout: 'out', stderr: '', exitCode: undefined} as any) + + // When + const got = await system.captureCommandWithExitCode('cmd') + + // Then + expect(got.exitCode).toBe(0) + }) + + test('raises an error if the command to run is found in the current directory', async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/currentDirectory/command') + + // When + const got = system.captureCommandWithExitCode('command', {cwd: '/currentDirectory'}) + + // Then + await expect(got).rejects.toThrowError('Skipped run of unsecure binary command found in the current directory.') + }) +}) + +describe('execCommand', () => { + test('runs command successfully without throwing', async () => { + // Given + vi.mocked(execaCommand).mockResolvedValueOnce({} as any) + + // When/Then + await expect(system.execCommand('echo hello')).resolves.toBeUndefined() + }) + + test('throws ExternalError on command failure', async () => { + // Given + const error = new Error('command not found') + vi.mocked(execaCommand).mockRejectedValueOnce(error) + + // When/Then + await expect(system.execCommand('nonexistent')).rejects.toThrow('command not found') + }) + + test('calls custom error handler when provided', async () => { + // Given + const error = new Error('custom error') + vi.mocked(execaCommand).mockRejectedValueOnce(error) + const customHandler = vi.fn() + + // When + await system.execCommand('failing', {externalErrorHandler: customHandler}) + + // Then + expect(customHandler).toHaveBeenCalledWith(error) + }) + + test('handles command with spaces in arguments', async () => { + // Given + vi.mocked(execaCommand).mockResolvedValueOnce({} as any) + + // When + await system.execCommand('touch "my file.txt"') + + // Then + expect(execaCommand).toHaveBeenCalledWith('touch "my file.txt"', expect.anything()) + }) + + test('uses provided cwd option', async () => { + // Given + vi.mocked(execaCommand).mockResolvedValueOnce({} as any) + + // When + await system.execCommand('pwd', {cwd: '/some/dir'}) + + // Then + expect(execaCommand).toHaveBeenCalledWith('pwd', expect.objectContaining({cwd: '/some/dir'})) + }) + + test('passes stdin option to execaCommand', async () => { + // Given + vi.mocked(execaCommand).mockResolvedValueOnce({} as any) + + // When + await system.execCommand('cat', {stdin: 'inherit'}) + + // Then + expect(execaCommand).toHaveBeenCalledWith('cat', expect.objectContaining({stdin: 'inherit'})) + }) +}) + describe('isStdinPiped', () => { test('returns true when stdin is a FIFO (pipe)', () => { // Given diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index b9064174315..b4750c3b076 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -6,7 +6,7 @@ import {isTruthy} from './context/utilities.js' import {renderWarning} from './ui.js' import {platformAndArch} from './os.js' import {shouldDisplayColors, outputDebug} from '../../public/node/output.js' -import {execa, ExecaChildProcess} from 'execa' +import {execa, execaCommand, ExecaChildProcess} from 'execa' import which from 'which' import {delimiter} from 'pathe' import {fstatSync} from 'fs' @@ -27,6 +27,14 @@ export interface ExecOptions { background?: boolean } +/** + * Options passed directly to execa. + */ +interface BuildExecOptions { + /** Whether to throw on non-zero exit codes (default: true). */ + reject?: boolean +} + /** * Opens a URL in the user's default browser. * @@ -57,6 +65,165 @@ export async function captureOutput(command: string, args: string[], options?: E return result.stdout } +/** + * Result from running a command with captureOutputWithExitCode. + */ +export interface CaptureOutputResult { + /** Standard output. */ + stdout: string + /** Standard error. */ + stderr: string + /** Exit code (0 = success). */ + exitCode: number +} + +/** + * Runs a command asynchronously and returns stdout, stderr, and exit code. + * Unlike captureOutput, this function does NOT throw on non-zero exit codes. + * + * @param command - Command to be executed. + * @param args - Arguments to pass to the command. + * @param options - Optional settings for how to run the command. + * @returns A promise that resolves with stdout, stderr, and exitCode. + * + * @example + * ```typescript + * const result = await captureOutputWithExitCode('ls', ['-la']) + * if (result.exitCode !== 0) \{ + * console.error('Command failed:', result.stderr) + * \} + * ``` + */ +export async function captureOutputWithExitCode( + command: string, + args: string[], + options?: ExecOptions, +): Promise { + const result = await buildExec(command, args, options, {reject: false}) + return { + stdout: result.stdout, + stderr: result.stderr, + exitCode: result.exitCode ?? 0, + } +} + +/** + * Parse a command string into an array of arguments, respecting quoted strings. + * Handles both single and double quotes, preserving spaces within quoted sections. + * + * @param command - The command string to parse (e.g., 'ls -la "my folder"'). + * @returns An array of command parts with quotes removed. + * + * @example + * parseCommand('shopify theme push --theme "My Theme Name"') // ['shopify', 'theme', 'push', '--theme', 'My Theme Name'] + */ +function parseCommand(command: string): string[] { + const result: string[] = [] + let current = '' + let inQuote: string | null = null + + for (const char of command) { + if (inQuote) { + if (char === inQuote) { + // End of quoted section + inQuote = null + } else { + current += char + } + } else if (char === '"' || char === "'") { + // Start of quoted section + inQuote = char + } else if (char === ' ' || char === '\t') { + // Whitespace outside quotes - end current token + if (current) { + result.push(current) + current = '' + } + } else { + current += char + } + } + + // Don't forget the last token + if (current) { + result.push(current) + } + + return result +} + +/** + * Runs a command string asynchronously and returns stdout, stderr, and exit code. + * Parses the command string into command and arguments (handles quoted strings). + * Unlike captureOutput, this function does NOT throw on non-zero exit codes. + * + * @param command - Full command string to be executed (e.g., 'ls -la "my folder"'). + * @param options - Optional settings for how to run the command. + * @returns A promise that resolves with stdout, stderr, and exitCode. + * + * @example + * ```typescript + * const result = await captureCommandWithExitCode('shopify theme push --theme "My Theme"') + * if (result.exitCode !== 0) { + * console.error('Command failed:', result.stderr) + * } + * ``` + */ +export async function captureCommandWithExitCode(command: string, options?: ExecOptions): Promise { + const env = options?.env ?? process.env + if (shouldDisplayColors()) { + env.FORCE_COLOR = '1' + } + const executionCwd = options?.cwd ?? cwd() + const [cmd, ...args] = parseCommand(command) + if (!cmd) { + return {stdout: '', stderr: 'Empty command', exitCode: 1} + } + checkCommandSafety(cmd, {cwd: executionCwd}) + const result = await execa(cmd, args, { + env, + cwd: executionCwd, + reject: false, + }) + return { + stdout: result.stdout, + stderr: result.stderr, + exitCode: result.exitCode ?? 0, + } +} + +/** + * Runs a command string asynchronously (parses command and arguments from the string). + * + * @param command - Full command string to be executed (e.g., 'ls -la "my folder"'). + * @param options - Optional settings for how to run the command. + */ +export async function execCommand(command: string, options?: ExecOptions): Promise { + const env = options?.env ?? process.env + if (shouldDisplayColors()) { + env.FORCE_COLOR = '1' + } + const executionCwd = options?.cwd ?? cwd() + try { + await execaCommand(command, { + env, + cwd: executionCwd, + stdin: options?.stdin, + stdout: options?.stdout === 'inherit' ? 'inherit' : undefined, + stderr: options?.stderr === 'inherit' ? 'inherit' : undefined, + }) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } catch (processError: any) { + if (options?.externalErrorHandler) { + await options.externalErrorHandler(processError) + } else { + const abortError = new ExternalError(processError.message, command, []) + abortError.stack = processError.stack + throw abortError + } + } +} + /** * Runs a command asynchronously. * @@ -115,9 +282,15 @@ export async function exec(command: string, args: string[], options?: ExecOption * @param command - Command to be executed. * @param args - Arguments to pass to the command. * @param options - Optional settings for how to run the command. + * @param execaOptions - Options passed directly to execa. * @returns A promise for a result with stdout and stderr properties. */ -function buildExec(command: string, args: string[], options?: ExecOptions): ExecaChildProcess { +function buildExec( + command: string, + args: string[], + options?: ExecOptions, + execaOptions?: BuildExecOptions, +): ExecaChildProcess { const env = options?.env ?? process.env if (shouldDisplayColors()) { env.FORCE_COLOR = '1' @@ -137,6 +310,7 @@ function buildExec(command: string, args: string[], options?: ExecOptions): Exec windowsHide: false, detached: options?.background, cleanup: !options?.background, + ...execaOptions, }) outputDebug(`Running system process${options?.background ? ' in background' : ''}: · Command: ${command} ${args.join(' ')} diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index e1df719bc35..6ebb772cbfa 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -3063,6 +3063,98 @@ "strict": true, "summary": "Trigger delivery of a sample webhook topic payload to a designated address." }, + "audit": { + "aliases": [ + ], + "args": { + }, + "description": "Run CLI audit tests", + "enableJsonFlag": false, + "flags": { + }, + "hasDynamicHelp": false, + "hidden": true, + "hiddenAliases": [ + ], + "id": "audit", + "pluginAlias": "@shopify/cli", + "pluginName": "@shopify/cli", + "pluginType": "core", + "strict": true + }, + "audit:theme": { + "aliases": [ + ], + "args": { + }, + "description": "Run all theme command audit tests", + "enableJsonFlag": false, + "flags": { + "environment": { + "char": "e", + "description": "The environment to use from shopify.theme.toml (required for store-connected tests).", + "env": "SHOPIFY_FLAG_ENVIRONMENT", + "hasDynamicHelp": false, + "multiple": false, + "name": "environment", + "required": true, + "type": "option" + }, + "no-color": { + "allowNo": false, + "description": "Disable color output.", + "env": "SHOPIFY_FLAG_NO_COLOR", + "hidden": false, + "name": "no-color", + "type": "boolean" + }, + "password": { + "description": "Password from Theme Access app (overrides environment).", + "env": "SHOPIFY_FLAG_PASSWORD", + "hasDynamicHelp": false, + "multiple": false, + "name": "password", + "type": "option" + }, + "path": { + "char": "p", + "default": ".", + "description": "The path to run tests in. Defaults to current directory.", + "env": "SHOPIFY_FLAG_PATH", + "hasDynamicHelp": false, + "multiple": false, + "name": "path", + "type": "option" + }, + "store": { + "char": "s", + "description": "Store URL (overrides environment).", + "env": "SHOPIFY_FLAG_STORE", + "hasDynamicHelp": false, + "multiple": false, + "name": "store", + "type": "option" + }, + "verbose": { + "allowNo": false, + "description": "Increase the verbosity of the output.", + "env": "SHOPIFY_FLAG_VERBOSE", + "hidden": false, + "name": "verbose", + "type": "boolean" + } + }, + "hasDynamicHelp": false, + "hidden": true, + "hiddenAliases": [ + "audit theme" + ], + "id": "audit:theme", + "pluginAlias": "@shopify/cli", + "pluginName": "@shopify/cli", + "pluginType": "core", + "strict": true + }, "auth:login": { "aliases": [ ], diff --git a/packages/cli/src/cli/commands/audit/audit.ts b/packages/cli/src/cli/commands/audit/audit.ts new file mode 100644 index 00000000000..431991fdc72 --- /dev/null +++ b/packages/cli/src/cli/commands/audit/audit.ts @@ -0,0 +1,21 @@ +import Command from '@shopify/cli-kit/node/base-command' +import {renderInfo} from '@shopify/cli-kit/node/ui' + +export default class Audit extends Command { + static description = 'Run CLI audit tests' + static hidden = true + + async run(): Promise { + renderInfo({ + headline: 'Shopify CLI Audit.', + body: [ + 'Available audit commands:', + '', + ' shopify audit theme -e Run all theme command tests', + '', + 'The -e/--environment flag is required to specify the store configuration.', + 'Use --help with any command for more options.', + ], + }) + } +} diff --git a/packages/cli/src/cli/commands/audit/theme/index.ts b/packages/cli/src/cli/commands/audit/theme/index.ts new file mode 100644 index 00000000000..2965a5b51ae --- /dev/null +++ b/packages/cli/src/cli/commands/audit/theme/index.ts @@ -0,0 +1,54 @@ +import {runThemeAudit} from '../../../services/audit/theme/runner.js' +import Command from '@shopify/cli-kit/node/base-command' +import {globalFlags} from '@shopify/cli-kit/node/cli' +import {Flags} from '@oclif/core' +import {resolvePath, cwd} from '@shopify/cli-kit/node/path' + +export default class AuditTheme extends Command { + static description = 'Run all theme command audit tests' + static hidden = true + static hiddenAliases = ['audit theme'] + + static flags = { + ...globalFlags, + path: Flags.string({ + char: 'p', + description: 'The path to run tests in. Defaults to current directory.', + env: 'SHOPIFY_FLAG_PATH', + parse: async (input) => resolvePath(input), + default: async () => cwd(), + }), + environment: Flags.string({ + char: 'e', + description: 'The environment to use from shopify.theme.toml (required for store-connected tests).', + env: 'SHOPIFY_FLAG_ENVIRONMENT', + required: true, + }), + store: Flags.string({ + char: 's', + description: 'Store URL (overrides environment).', + env: 'SHOPIFY_FLAG_STORE', + }), + password: Flags.string({ + description: 'Password from Theme Access app (overrides environment).', + env: 'SHOPIFY_FLAG_PASSWORD', + }), + } + + async run(): Promise { + const {flags} = await this.parse(AuditTheme) + + const results = await runThemeAudit({ + path: flags.path, + environment: flags.environment, + store: flags.store, + password: flags.password, + }) + + // Exit with error code if any tests failed + const failed = results.some((result) => result.status === 'failed') + if (failed) { + process.exitCode = 1 + } + } +} diff --git a/packages/cli/src/cli/services/audit/context.ts b/packages/cli/src/cli/services/audit/context.ts new file mode 100644 index 00000000000..f1bdd8811d2 --- /dev/null +++ b/packages/cli/src/cli/services/audit/context.ts @@ -0,0 +1,12 @@ +import {cwd} from '@shopify/cli-kit/node/path' +import type {AuditContext, ThemeAuditOptions} from './types.js' + +export function createAuditContext(options: ThemeAuditOptions): AuditContext { + return { + workingDirectory: options.path ?? cwd(), + environment: options.environment, + store: options.store, + password: options.password, + data: {}, + } +} diff --git a/packages/cli/src/cli/services/audit/framework.test.ts b/packages/cli/src/cli/services/audit/framework.test.ts new file mode 100644 index 00000000000..ddadcdb38e3 --- /dev/null +++ b/packages/cli/src/cli/services/audit/framework.test.ts @@ -0,0 +1,326 @@ +import {AuditSuite} from './framework.js' +import {describe, expect, test, vi, beforeEach} from 'vitest' +import type {AuditContext} from './types.js' + +vi.mock('@shopify/cli-kit/node/fs') +vi.mock('@shopify/cli-kit/node/system') + +/** + * Creates a minimal AuditContext for testing + */ +function createTestContext(overrides?: Partial): AuditContext { + return { + workingDirectory: '/test/dir', + environment: 'test', + data: {}, + ...overrides, + } +} + +/** + * Concrete test suite for testing AuditSuite behavior + */ +class TestSuite extends AuditSuite { + static description = 'Test suite for framework testing' + + private readonly testDefinitions: {name: string; fn: () => Promise}[] = [] + + addTest(name: string, fn: () => Promise): void { + this.testDefinitions.push({name, fn}) + } + + // Expose protected methods for testing + public exposeAssert(condition: boolean, message: string): void { + this.assert(condition, message) + } + + public exposeAssertEqual(actual: T, expected: T, message: string): void { + this.assertEqual(actual, expected, message) + } + + protected tests(): void { + for (const def of this.testDefinitions) { + this.test(def.name, def.fn) + } + } +} + +describe('AuditSuite', () => { + let suite: TestSuite + let context: AuditContext + + beforeEach(() => { + suite = new TestSuite() + context = createTestContext() + }) + + describe('test registration', () => { + test('registers tests via test() method', async () => { + // Given + suite.addTest('first test', async () => {}) + suite.addTest('second test', async () => {}) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results).toHaveLength(2) + expect(results[0]!.name).toBe('first test') + expect(results[1]!.name).toBe('second test') + }) + + test('runs tests in registration order', async () => { + // Given + const order: string[] = [] + suite.addTest('A', async () => { + order.push('A') + }) + suite.addTest('B', async () => { + order.push('B') + }) + suite.addTest('C', async () => { + order.push('C') + }) + + // When + await suite.runSuite(context) + + // Then + expect(order).toEqual(['A', 'B', 'C']) + }) + + test('returns empty results when no tests registered', async () => { + // Given - no tests added + + // When + const results = await suite.runSuite(context) + + // Then + expect(results).toHaveLength(0) + }) + }) + + describe('assertion tracking', () => { + test('collects assertions from test', async () => { + // Given + suite.addTest('with assertions', async () => { + suite.exposeAssert(true, 'first assertion') + suite.exposeAssert(true, 'second assertion') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.assertions).toHaveLength(2) + expect(results[0]!.assertions[0]!.description).toBe('first assertion') + expect(results[0]!.assertions[1]!.description).toBe('second assertion') + }) + + test('resets assertions between tests', async () => { + // Given + suite.addTest('test1', async () => { + suite.exposeAssert(true, 'assertion from test1') + }) + suite.addTest('test2', async () => { + suite.exposeAssert(true, 'assertion from test2') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.assertions).toHaveLength(1) + expect(results[0]!.assertions[0]!.description).toBe('assertion from test1') + expect(results[1]!.assertions).toHaveLength(1) + expect(results[1]!.assertions[0]!.description).toBe('assertion from test2') + }) + + test('tracks assertion pass/fail status', async () => { + // Given + suite.addTest('mixed assertions', async () => { + suite.exposeAssert(true, 'passing') + suite.exposeAssert(false, 'failing') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.assertions[0]!.passed).toBe(true) + expect(results[0]!.assertions[1]!.passed).toBe(false) + }) + }) + + describe('pass/fail determination', () => { + test('marks test as passed when all assertions pass', async () => { + // Given + suite.addTest('all pass', async () => { + suite.exposeAssert(true, 'pass1') + suite.exposeAssert(true, 'pass2') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.status).toBe('passed') + }) + + test('marks test as failed when any assertion fails', async () => { + // Given + suite.addTest('one fails', async () => { + suite.exposeAssert(true, 'pass') + suite.exposeAssert(false, 'fail') + suite.exposeAssert(true, 'pass again') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.status).toBe('failed') + }) + + test('marks test as passed with no assertions', async () => { + // Given + suite.addTest('no assertions', async () => {}) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.status).toBe('passed') + }) + }) + + describe('error handling', () => { + test('marks test as failed when test throws Error', async () => { + // Given + suite.addTest('throws error', async () => { + throw new Error('Test error') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.status).toBe('failed') + expect(results[0]!.error).toBeInstanceOf(Error) + expect(results[0]!.error!.message).toBe('Test error') + }) + + test('converts non-Error throws to Error', async () => { + // Given + suite.addTest('throws string', async () => { + throw 'string error' + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.status).toBe('failed') + expect(results[0]!.error).toBeInstanceOf(Error) + expect(results[0]!.error!.message).toBe('string error') + }) + + test('continues running other tests after error', async () => { + // Given + suite.addTest('throws', async () => { + throw new Error('boom') + }) + suite.addTest('succeeds', async () => { + suite.exposeAssert(true, 'ok') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results).toHaveLength(2) + expect(results[0]!.status).toBe('failed') + expect(results[1]!.status).toBe('passed') + }) + + test('preserves assertions collected before error', async () => { + // Given + suite.addTest('throws after assertion', async () => { + suite.exposeAssert(true, 'before error') + throw new Error('after assertion') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.assertions).toHaveLength(1) + expect(results[0]!.assertions[0]!.description).toBe('before error') + }) + }) + + describe('duration tracking', () => { + test('records duration for each test', async () => { + // Given + suite.addTest('quick test', async () => {}) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.duration).toBeGreaterThanOrEqual(0) + expect(typeof results[0]!.duration).toBe('number') + }) + }) + + describe('assertEqual', () => { + test('passes when values are equal', async () => { + // Given + suite.addTest('equal values', async () => { + suite.exposeAssertEqual(42, 42, 'numbers match') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.assertions[0]!.passed).toBe(true) + expect(results[0]!.assertions[0]!.expected).toBe('42') + expect(results[0]!.assertions[0]!.actual).toBe('42') + }) + + test('fails when values are not equal', async () => { + // Given + suite.addTest('unequal values', async () => { + suite.exposeAssertEqual('foo', 'bar', 'strings should match') + }) + + // When + const results = await suite.runSuite(context) + + // Then + expect(results[0]!.assertions[0]!.passed).toBe(false) + expect(results[0]!.assertions[0]!.expected).toBe('bar') + expect(results[0]!.assertions[0]!.actual).toBe('foo') + }) + }) + + describe('suite reusability', () => { + test('can run suite multiple times with fresh state', async () => { + // Given + let runCount = 0 + suite.addTest('increments', async () => { + runCount++ + suite.exposeAssert(true, `run ${runCount}`) + }) + + // When + const results1 = await suite.runSuite(context) + const results2 = await suite.runSuite(context) + + // Then + expect(results1[0]!.assertions[0]!.description).toBe('run 1') + expect(results2[0]!.assertions[0]!.description).toBe('run 2') + }) + }) +}) diff --git a/packages/cli/src/cli/services/audit/framework.ts b/packages/cli/src/cli/services/audit/framework.ts new file mode 100644 index 00000000000..af56b0aee11 --- /dev/null +++ b/packages/cli/src/cli/services/audit/framework.ts @@ -0,0 +1,373 @@ +import {fileExists, readFile} from '@shopify/cli-kit/node/fs' +import {joinPath, relativePath} from '@shopify/cli-kit/node/path' +import {execCommand, captureCommandWithExitCode} from '@shopify/cli-kit/node/system' +import type {AuditContext, TestResult, AssertionResult} from './types.js' + +/** + * Result from running a CLI command + */ +interface CommandResult { + /** The full command that was run */ + command: string + /** Exit code (0 = success) */ + exitCode: number + /** Standard output */ + stdout: string + /** Standard error */ + stderr: string + /** Combined output (stdout + stderr) */ + output: string + /** Whether the command succeeded (exitCode === 0) */ + success: boolean +} + +/** + * A registered test with its name and function + */ +interface RegisteredTest { + name: string + fn: () => Promise +} + +/** + * Base class for audit test suites. + * + * Write tests using the test() method: + * + * ```typescript + * export default class MyTests extends AuditSuite { + * static description = 'My test suite' + * + * tests() { + * this.test('basic case', async () => { + * const result = await this.run('shopify theme init') + * this.assertSuccess(result) + * }) + * + * this.test('error case', async () => { + * const result = await this.run('shopify theme init --invalid') + * this.assertError(result, /unknown flag/) + * }) + * } + * } + * ``` + */ +export abstract class AuditSuite { + static description = 'Audit test suite' + + protected context!: AuditContext + private assertions: AssertionResult[] = [] + private registeredTests: RegisteredTest[] = [] + + /** + * Run the entire test suite + */ + async runSuite(context: AuditContext): Promise { + this.context = context + this.registeredTests = [] + const results: TestResult[] = [] + + // Call tests() to register tests via this.test() + this.tests() + + // Run all registered tests + for (const registeredTest of this.registeredTests) { + this.assertions = [] + const startTime = Date.now() + + try { + // eslint-disable-next-line no-await-in-loop + await registeredTest.fn() + + results.push({ + name: registeredTest.name, + status: this.hasFailures() ? 'failed' : 'passed', + duration: Date.now() - startTime, + assertions: [...this.assertions], + }) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + results.push({ + name: registeredTest.name, + status: 'failed', + duration: Date.now() - startTime, + assertions: [...this.assertions], + error: error instanceof Error ? error : new Error(String(error)), + }) + } + } + + return results + } + + /** + * Register a test with a name and function. + * + * @param name - The test name + * @param fn - The async test function + */ + protected test(name: string, fn: () => Promise): void { + this.registeredTests.push({name, fn}) + } + + /** + * Override this method to register tests using this.test() + */ + protected tests(): void { + // Subclasses override this to register tests + } + + // ============================================ + // Command execution + // ============================================ + + /** + * Run a CLI command and return the result. + * + * @example + * const result = await this.run('shopify theme init my-theme') + * const result = await this.run('shopify theme push --json') + */ + protected async run( + command: string, + options?: {cwd?: string; env?: {[key: string]: string}}, + ): Promise { + const cwd = options?.cwd ?? this.context.workingDirectory + const result = await captureCommandWithExitCode(command, {cwd, env: options?.env}) + + return { + command, + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + output: result.stdout + result.stderr, + success: result.exitCode === 0, + } + } + + /** + * Run a command without capturing output (for interactive commands). + * Returns only success/failure. + */ + protected async runInteractive( + command: string, + options?: {cwd?: string; env?: {[key: string]: string}}, + ): Promise { + const cwd = options?.cwd ?? this.context.workingDirectory + let exitCode = 0 + + try { + await execCommand(command, {cwd, env: options?.env, stdin: 'inherit'}) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + exitCode = 1 + } + + return { + command, + exitCode, + stdout: '', + stderr: '', + output: '', + success: exitCode === 0, + } + } + + // ============================================ + // Assertions + // ============================================ + + /** + * Assert that a command succeeded (exit code 0) + */ + protected assertSuccess(result: CommandResult, message?: string): void { + this.assertions.push({ + description: message ?? `Command succeeded: ${result.command}`, + passed: result.success, + expected: 'exit code 0', + actual: `exit code ${result.exitCode}`, + }) + } + + /** + * Assert that a command failed with an error matching the pattern + */ + protected assertError(result: CommandResult, pattern?: RegExp | string, message?: string): void { + const failed = !result.success + + if (pattern) { + const regex = typeof pattern === 'string' ? new RegExp(pattern) : pattern + const matches = regex.test(result.output) + let actualValue: string + if (!failed) { + actualValue = 'command succeeded' + } else if (matches) { + actualValue = 'matched' + } else { + actualValue = `output: ${result.output.slice(0, 200)}` + } + this.assertions.push({ + description: message ?? `Command failed with expected error: ${pattern}`, + passed: failed && matches, + expected: `failure with error matching ${pattern}`, + actual: actualValue, + }) + } else { + this.assertions.push({ + description: message ?? `Command failed: ${result.command}`, + passed: failed, + expected: 'non-zero exit code', + actual: `exit code ${result.exitCode}`, + }) + } + } + + /** + * Assert that a file exists and optionally matches content + */ + protected async assertFile(path: string, contentPattern?: RegExp | string, message?: string): Promise { + const fullPath = path.startsWith('/') ? path : joinPath(this.context.workingDirectory, path) + const displayPath = relativePath(this.context.workingDirectory, fullPath) + const exists = await fileExists(fullPath) + + if (!exists) { + this.assertions.push({ + description: message ?? `File exists: ${displayPath}`, + passed: false, + expected: 'file exists', + actual: 'file not found', + }) + return + } + + if (contentPattern) { + const content = await readFile(fullPath) + const regex = typeof contentPattern === 'string' ? new RegExp(contentPattern) : contentPattern + const matches = regex.test(content) + this.assertions.push({ + description: message ?? `File ${displayPath} matches ${contentPattern}`, + passed: matches, + expected: `content matching ${contentPattern}`, + actual: matches ? 'matched' : `content: ${content.slice(0, 200)}...`, + }) + } else { + this.assertions.push({ + description: message ?? `File exists: ${displayPath}`, + passed: true, + expected: 'file exists', + actual: 'file exists', + }) + } + } + + /** + * Assert that a file does not exist + */ + protected async assertNoFile(path: string, message?: string): Promise { + const fullPath = path.startsWith('/') ? path : joinPath(this.context.workingDirectory, path) + const displayPath = relativePath(this.context.workingDirectory, fullPath) + const exists = await fileExists(fullPath) + this.assertions.push({ + description: message ?? `File does not exist: ${displayPath}`, + passed: !exists, + expected: 'file does not exist', + actual: exists ? 'file exists' : 'file does not exist', + }) + } + + /** + * Assert that a directory exists + */ + protected async assertDirectory(path: string, message?: string): Promise { + const fullPath = path.startsWith('/') ? path : joinPath(this.context.workingDirectory, path) + const displayPath = relativePath(this.context.workingDirectory, fullPath) + const exists = await fileExists(fullPath) + this.assertions.push({ + description: message ?? `Directory exists: ${displayPath}`, + passed: exists, + expected: 'directory exists', + actual: exists ? 'directory exists' : 'directory not found', + }) + } + + /** + * Assert that output contains a pattern + */ + protected assertOutput(result: CommandResult, pattern: RegExp | string, message?: string): void { + const regex = typeof pattern === 'string' ? new RegExp(pattern) : pattern + const matches = regex.test(result.output) + this.assertions.push({ + description: message ?? `Output matches ${pattern}`, + passed: matches, + expected: `output matching ${pattern}`, + actual: matches ? 'matched' : `output: ${result.output.slice(0, 200)}`, + }) + } + + /** + * Assert that output contains valid JSON and optionally validate it + */ + protected assertJson( + result: CommandResult, + validator?: (json: T) => boolean, + message?: string, + ): T | undefined { + try { + const json = JSON.parse(result.stdout) as T + if (validator) { + const valid = validator(json) + this.assertions.push({ + description: message ?? 'Output is valid JSON matching validator', + passed: valid, + expected: 'valid JSON matching validator', + actual: valid ? 'matched' : 'validator returned false', + }) + } else { + this.assertions.push({ + description: message ?? 'Output is valid JSON', + passed: true, + expected: 'valid JSON', + actual: 'valid JSON', + }) + } + return json + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + this.assertions.push({ + description: message ?? 'Output is valid JSON', + passed: false, + expected: 'valid JSON', + actual: `invalid JSON: ${result.stdout.slice(0, 100)}`, + }) + return undefined + } + } + + /** + * Assert a boolean condition + */ + protected assert(condition: boolean, message: string): void { + this.assertions.push({ + description: message, + passed: condition, + expected: 'true', + actual: String(condition), + }) + } + + /** + * Assert two values are equal + */ + protected assertEqual(actual: T, expected: T, message: string): void { + this.assertions.push({ + description: message, + passed: actual === expected, + expected: String(expected), + actual: String(actual), + }) + } + + private hasFailures(): boolean { + return this.assertions.some((assertion) => !assertion.passed) + } +} diff --git a/packages/cli/src/cli/services/audit/reporter.ts b/packages/cli/src/cli/services/audit/reporter.ts new file mode 100644 index 00000000000..4a9543e1601 --- /dev/null +++ b/packages/cli/src/cli/services/audit/reporter.ts @@ -0,0 +1,100 @@ +import colors from '@shopify/cli-kit/node/colors' +import {outputInfo} from '@shopify/cli-kit/node/output' +import {relativizePath} from '@shopify/cli-kit/node/path' +import type {TestResult, AssertionResult} from './types.js' + +const log = (message: string) => outputInfo(message) + +// Reporter context for path +let reporterBasePath: string | undefined + +/** + * Initialize the reporter with a base path for truncating file paths in output. + * Call this before running tests to enable path truncation. + */ +export function initReporter(basePath: string): void { + reporterBasePath = basePath +} + +/** + * Truncate absolute paths to be relative to the base path. + * Looks for paths in common patterns like "File exists: /path/to/file" + */ +function truncatePaths(text: string): string { + if (!reporterBasePath) return text + + // Match absolute paths + // relativizePath will convert paths under reporterBasePath to relative paths + // and keep other paths unchanged + const absolutePathPattern = /\/[^\s,)]+/g + + return text.replace(absolutePathPattern, (path) => { + return relativizePath(path, reporterBasePath) + }) +} + +export function reportSuiteStart(suiteName: string, description: string): void { + log('') + log(colors.bold(colors.cyan(`Suite: ${suiteName}`))) + log(colors.dim(description)) +} + +export function reportTestStart(testName: string): void { + log(colors.bold(colors.blue(`Running: ${testName}`))) +} + +export function reportTestResult(result: TestResult): void { + const durationStr = `(${(result.duration / 1000).toFixed(2)}s)` + + if (result.status === 'passed') { + log(colors.bold(colors.green(`PASSED: ${result.name} ${colors.dim(durationStr)}`))) + for (const line of formatAssertions(result.assertions)) { + log(line) + } + } else if (result.status === 'failed') { + log(colors.red(`FAILED: ${result.name} ${colors.dim(durationStr)}`)) + for (const line of formatAssertions(result.assertions)) { + log(line) + } + if (result.error) { + log(colors.red(` Error: ${truncatePaths(result.error.message)}`)) + } + } else { + log(colors.yellow(`SKIPPED: ${result.name}`)) + } +} + +export function reportSummary(results: TestResult[]): void { + const passed = results.filter((result) => result.status === 'passed').length + const failed = results.filter((result) => result.status === 'failed').length + const skipped = results.filter((result) => result.status === 'skipped').length + const total = results.length + const totalDuration = results.reduce((sum, result) => sum + result.duration, 0) + + log('') + log(colors.bold('─'.repeat(40))) + + if (failed > 0) { + log(colors.red(colors.bold(`Audit Complete: ${failed}/${total} tests failed`))) + } else { + log(colors.green(colors.bold(`Audit Complete: ${passed}/${total} tests passed`))) + } + + log(` Passed: ${colors.green(String(passed))}`) + log(` Failed: ${colors.red(String(failed))}`) + if (skipped > 0) { + log(` Skipped: ${colors.yellow(String(skipped))}`) + } + log(` Total time: ${colors.dim(`${(totalDuration / 1000).toFixed(2)}s`)}`) +} + +function formatAssertions(assertions: AssertionResult[]): string[] { + return assertions.map((assertion) => { + if (assertion.passed) { + return colors.green(` [OK] ${assertion.description}`) + } else { + const details = ` (expected: ${assertion.expected}, actual: ${assertion.actual})` + return colors.red(` [FAIL] ${assertion.description}${details}`) + } + }) +} diff --git a/packages/cli/src/cli/services/audit/theme/runner.ts b/packages/cli/src/cli/services/audit/theme/runner.ts new file mode 100644 index 00000000000..a0aa2ca4058 --- /dev/null +++ b/packages/cli/src/cli/services/audit/theme/runner.ts @@ -0,0 +1,47 @@ +import ThemeInitTests from './tests/init.js' +import ThemePushTests from './tests/push.js' +import {createAuditContext} from '../context.js' +import {reportTestStart, reportTestResult, reportSuiteStart, reportSummary, initReporter} from '../reporter.js' +import {AuditSuite} from '../framework.js' +import type {TestResult, ThemeAuditOptions} from '../types.js' + +// Test suites run in order. If a test relies on another, ensure that test runs after it's dependency. +const themeSuites: (new () => AuditSuite)[] = [ThemeInitTests, ThemePushTests] + +/** + * Run all theme audit tests. + * Stops on first failure. + */ +export async function runThemeAudit(options: ThemeAuditOptions): Promise { + const results: TestResult[] = [] + const context = createAuditContext(options) + + // Initialize reporter with working directory + initReporter(context.workingDirectory) + + // Run all test suites in order + for (const SuiteClass of themeSuites) { + const suite = new SuiteClass() + const description = (SuiteClass as unknown as {description: string}).description ?? 'Test suite' + + reportSuiteStart(SuiteClass.name, description) + + // eslint-disable-next-line no-await-in-loop + const suiteResults = await suite.runSuite(context) + + for (const result of suiteResults) { + reportTestStart(result.name) + reportTestResult(result) + results.push(result) + + // Stop on first failure + if (result.status === 'failed') { + reportSummary(results) + return results + } + } + } + + reportSummary(results) + return results +} diff --git a/packages/cli/src/cli/services/audit/theme/tests/init.ts b/packages/cli/src/cli/services/audit/theme/tests/init.ts new file mode 100644 index 00000000000..b04cee63d7a --- /dev/null +++ b/packages/cli/src/cli/services/audit/theme/tests/init.ts @@ -0,0 +1,55 @@ +import {AuditSuite} from '../../framework.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {getRandomName} from '@shopify/cli-kit/common/string' + +/** + * Tests for `shopify theme init` command + */ +export default class ThemeInitTests extends AuditSuite { + static description = 'Tests the theme init command creates a valid theme structure' + + private themeName = '' + private themePath = '' + + tests() { + this.test('init creates theme directory', async () => { + this.themeName = `audit-theme-${getRandomName('creative')}` + this.themePath = joinPath(this.context.workingDirectory, this.themeName) + + const result = await this.runInteractive( + `shopify theme init ${this.themeName} --path ${this.context.workingDirectory}`, + ) + this.assertSuccess(result) + + // Store for tests later in the suite + this.context.themeName = this.themeName + this.context.themePath = this.themePath + }) + + this.test('essential theme files exist', async () => { + const essentialFiles = ['layout/theme.liquid', 'config/settings_schema.json', 'templates/index.json'] + + for (const file of essentialFiles) { + // eslint-disable-next-line no-await-in-loop + await this.assertFile(joinPath(this.themePath, file)) + } + }) + + this.test('theme directories exist', async () => { + const directories = ['sections', 'snippets', 'assets', 'locales'] + + for (const dir of directories) { + // eslint-disable-next-line no-await-in-loop + await this.assertDirectory(joinPath(this.themePath, dir)) + } + }) + + this.test('layout/theme.liquid has valid content', async () => { + await this.assertFile( + joinPath(this.themePath, 'layout/theme.liquid'), + /| { + // Check prerequisite: theme must be initialized + if (!this.context.themePath) { + this.assert(false, 'Theme init did not complete successfully; themePath is missing in context') + return + } + + // Build command + const themeName = this.context.themeName ?? 'audit-theme' + let cmd = `shopify theme push --unpublished --json --path ${this.context.themePath} -t ${themeName}` + + if (this.context.environment) { + cmd += ` -e ${this.context.environment}` + } + if (this.context.store) { + cmd += ` -s ${this.context.store}` + } + if (this.context.password) { + cmd += ` --password ${this.context.password}` + } + + const result = await this.run(cmd) + this.assertSuccess(result) + + // Parse and validate JSON output + const json = this.assertJson(result, (data) => typeof data.theme?.id === 'number') + + if (json?.theme) { + this.assert(typeof json.theme.id === 'number', 'Theme was created with a valid ID') + this.assertEqual(json.theme.role, 'unpublished', 'Theme role is unpublished') + this.assert(json.theme.editor_url.includes('/admin/themes/'), 'Editor URL is provided') + this.assert(json.theme.preview_url.includes('preview_theme_id='), 'Preview URL is provided') + + // Update context for subsequent tests + this.context.themeId = String(json.theme.id) + this.context.data.editorUrl = json.theme.editor_url + this.context.data.previewUrl = json.theme.preview_url + } + }) + } +} diff --git a/packages/cli/src/cli/services/audit/types.ts b/packages/cli/src/cli/services/audit/types.ts new file mode 100644 index 00000000000..589c28826b4 --- /dev/null +++ b/packages/cli/src/cli/services/audit/types.ts @@ -0,0 +1,46 @@ +type TestStatus = 'passed' | 'failed' | 'skipped' + +export interface AssertionResult { + description: string + passed: boolean + expected?: unknown + actual?: unknown +} + +export interface TestResult { + name: string + status: TestStatus + duration: number + assertions: AssertionResult[] + error?: Error +} + +export interface AuditContext { + // Working directory for tests (user's current directory) + workingDirectory: string + // Environment name from shopify.theme.toml (required) + environment: string + // Store URL (from environment or flags) + store?: string + // Password/token for Theme Access app + password?: string + // Theme name created during init + themeName?: string + // Theme path after init + themePath?: string + // Theme ID after push + themeId?: string + // Custom data that tests can share + data: {[key: string]: unknown} +} + +export interface ThemeAuditOptions { + // Working directory (defaults to cwd) + path?: string + // Environment name from shopify.theme.toml (required) + environment: string + // Store URL (overrides environment) + store?: string + // Password/token (overrides environment) + password?: string +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 1fd491d8c90..b7d1316c5b9 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -9,6 +9,8 @@ import KitchenSinkAsync from './cli/commands/kitchen-sink/async.js' import KitchenSinkPrompts from './cli/commands/kitchen-sink/prompts.js' import KitchenSinkStatic from './cli/commands/kitchen-sink/static.js' import KitchenSink from './cli/commands/kitchen-sink/index.js' +import Audit from './cli/commands/audit/audit.js' +import AuditTheme from './cli/commands/audit/theme/index.js' import DocsGenerate from './cli/commands/docs/generate.js' import HelpCommand from './cli/commands/help.js' import List from './cli/commands/notifications/list.js' @@ -142,6 +144,8 @@ export const COMMANDS: any = { 'kitchen-sink:async': KitchenSinkAsync, 'kitchen-sink:prompts': KitchenSinkPrompts, 'kitchen-sink:static': KitchenSinkStatic, + audit: Audit, + 'audit:theme': AuditTheme, 'docs:generate': DocsGenerate, 'notifications:list': List, 'notifications:generate': Generate,