diff --git a/packages/create-release-branch/CHANGELOG.md b/packages/create-release-branch/CHANGELOG.md index 65a8554..ad4e2d3 100644 --- a/packages/create-release-branch/CHANGELOG.md +++ b/packages/create-release-branch/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Show colors and progress spinners in the CLI output ([#154](https://github.com/MetaMask/release-tools/pull/154)) +- Show the CLI output in one block, with a spinner for long steps and the output of a failing Yarn step ([#159](https://github.com/MetaMask/release-tools/pull/159)) ### Fixed diff --git a/packages/create-release-branch/package.json b/packages/create-release-branch/package.json index 39e7ec3..7dfd2ad 100644 --- a/packages/create-release-branch/package.json +++ b/packages/create-release-branch/package.json @@ -44,15 +44,14 @@ "test:watch": "vitest --watch" }, "dependencies": { + "@clack/prompts": "^1.8.1", "@metamask/action-utils": "^2.0.0", "@metamask/auto-changelog": "^7.1.0", "@metamask/utils": "^11.12.0", - "chalk": "^5.6.2", "debug": "^4.3.4", "execa": "^8.0.1", "express": "^4.21.2", "open": "^10.1.0", - "ora": "^9.4.1", "pony-cause": "^2.1.9", "semver": "^7.8.5", "shell-quote": "^1.8.4", diff --git a/packages/create-release-branch/src/cli/main.test.ts b/packages/create-release-branch/src/cli/main.test.ts index 16a7374..2576d55 100644 --- a/packages/create-release-branch/src/cli/main.test.ts +++ b/packages/create-release-branch/src/cli/main.test.ts @@ -63,8 +63,8 @@ describe('main', () => { releaseType: 'backport', output: expect.any(Object) as CliOutput, }); - expect(stdout.data()).toContain( - 'Project appears to have workspaces. Following monorepo workflow.\n', + expect(stdout.data().join('')).toContain( + 'Project appears to have workspaces. Following monorepo workflow.', ); }); diff --git a/packages/create-release-branch/src/cli/main.ts b/packages/create-release-branch/src/cli/main.ts index 677cacd..d8097f7 100644 --- a/packages/create-release-branch/src/cli/main.ts +++ b/packages/create-release-branch/src/cli/main.ts @@ -1,6 +1,5 @@ -import type { WriteStream } from 'fs'; - import { createCliOutput } from '../core/cli-output.js'; +import type { OutputStream } from '../core/cli-output.js'; import { runRelease } from '../core/run-release.js'; import { determineInitialParameters } from './initial-parameters.js'; import { selectFrontEnd, selectMode } from './select.js'; @@ -8,8 +7,8 @@ import { selectFrontEnd, selectMode } from './select.js'; /** * The main function for this tool. Designed to not access `process.argv`, * `process.cwd()`, `process.stdout`, or `process.stderr` directly so as to be - * more easily testable. The output still reads `NO_COLOR` and the color - * support that chalk detects for the process streams. + * more easily testable. The output still reads `NO_COLOR`, `CI`, and `TERM`, + * and clack picks the colors of its symbols from `process.stdout`. * * @param args - The arguments. * @param args.argv - The name of this executable and its arguments (as obtained @@ -26,8 +25,8 @@ export async function main({ }: { argv: string[]; cwd: string; - stdout: Pick; - stderr: NodeJS.WritableStream; + stdout: OutputStream; + stderr: OutputStream; }): Promise { const output = createCliOutput({ stdout, stderr }); const { diff --git a/packages/create-release-branch/src/core/cli-output.test.ts b/packages/create-release-branch/src/core/cli-output.test.ts index 716bc82..fcc1af9 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/packages/create-release-branch/src/core/cli-output.test.ts @@ -1,63 +1,105 @@ -import chalk, { Chalk, chalkStderr } from 'chalk'; -import ora from 'ora'; +import { + S_BAR, + S_ERROR, + S_INFO, + S_STEP_SUBMIT, + S_SUCCESS, + S_WARN, + spinner, + taskLog, +} from '@clack/prompts'; +import { stripVTControlCharacters } from 'node:util'; import { MockWritable } from 'stdio-mock'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { CliOutput } from './cli-output.js'; import { createCliOutput } from './cli-output.js'; -const spinner = vi.hoisted(() => ({ - start: vi.fn(), - succeed: vi.fn(), - fail: vi.fn(), +const indicators = vi.hoisted(() => ({ + spinner: { start: vi.fn(), stop: vi.fn(), error: vi.fn() }, + taskLog: { message: vi.fn(), success: vi.fn(), error: vi.fn() }, })); -vi.mock('ora', () => ({ default: vi.fn(() => spinner) })); - -const colors = new Chalk({ level: 1 }); +vi.mock('@clack/prompts', async (importOriginal) => ({ + ...(await importOriginal()), + spinner: vi.fn(() => indicators.spinner), + taskLog: vi.fn(() => indicators.taskLog), +})); /** - * Builds a mock stream that reports being a terminal with a width. + * Builds a mock stream that reports being a color terminal with a width. * * @returns The mock stream. */ function buildTerminalStream(): MockWritable & { isTTY: boolean; columns: number; + hasColors: () => boolean; } { - return Object.assign(new MockWritable(), { isTTY: true, columns: 80 }); + return Object.assign(new MockWritable(), { + isTTY: true, + columns: 80, + hasColors: () => true, + }); +} + +/** + * Builds the text that clack writes for one log message: a gutter line, then + * the symbol and the message. + * + * @param symbol - The symbol in front of the message. + * @param message - The message. + * @returns The text, without colors. + */ +function logLine(symbol: string, message: string): string { + return `${S_BAR}\n${symbol} ${message}\n`; +} + +/** + * Gets everything that was written to a stream, as one string. + * + * @param stream - The stream. + * @returns The text. + */ +function textOf(stream: MockWritable): string { + return stream.data().join(''); } /** - * Sends one message through each method of the output, and runs one step. + * Sends one message through each method of the output, runs one step, and + * runs one task that fails after printing a line. * * @param output - The output. */ async function writeEveryKindOfMessage(output: CliOutput): Promise { output.info('Some info\n'); - output.success('\nSome success\n'); + output.success('Some success\n'); output.warn('Some warning\n'); output.error('Some error\n'); await output.step('Doing something', async () => undefined); + await expect( + output.task('Running something', async (log) => { + log('Some line'); + throw new Error('oops'); + }), + ).rejects.toThrow('oops'); } describe('cli-output', () => { - const originalLevels = { stdout: chalk.level, stderr: chalkStderr.level }; - beforeEach(() => { + // Make clack color its symbols, whatever the terminal running the tests. + vi.stubEnv('FORCE_COLOR', '1'); vi.stubEnv('NO_COLOR', undefined); - chalk.level = 1; - chalkStderr.level = 1; + vi.stubEnv('CI', undefined); + vi.stubEnv('TERM', 'xterm-256color'); }); afterEach(() => { vi.unstubAllEnvs(); - chalk.level = originalLevels.stdout; - chalkStderr.level = originalLevels.stderr; }); describe('createCliOutput', () => { - it('writes info and success to stdout as given', () => { + it('writes info and success through clack to stdout', () => { const stdout = new MockWritable(); const stderr = new MockWritable(); const output = createCliOutput({ stdout, stderr }); @@ -65,51 +107,44 @@ describe('cli-output', () => { output.info('Some info\n'); output.success('Some success'); - expect(stdout.data()).toStrictEqual(['Some info\n', 'Some success']); + expect(textOf(stdout)).toBe( + `${logLine(S_INFO, 'Some info')}${logLine(S_SUCCESS, 'Some success')}`, + ); expect(stderr.data()).toStrictEqual([]); }); - it('writes warn and error to stderr as given', () => { + it('writes warnings and errors to stderr', () => { const stdout = new MockWritable(); const stderr = new MockWritable(); const output = createCliOutput({ stdout, stderr }); - output.warn('Some warning'); - output.error('Some error\n'); + output.warn('Some warning\n\n'); + output.error('Some error'); - expect(stderr.data()).toStrictEqual(['Some warning', 'Some error\n']); + expect(textOf(stderr)).toBe( + `${logLine(S_WARN, 'Some warning')}${logLine(S_ERROR, 'Some error')}`, + ); expect(stdout.data()).toStrictEqual([]); }); - it('step runs the callback, returns its result, and writes nothing', async () => { - const stdout = new MockWritable(); + it('writes stderr without colors when only stdout is a color terminal', () => { + const stdout = buildTerminalStream(); const stderr = new MockWritable(); const output = createCliOutput({ stdout, stderr }); - const result = await output.step('Doing something', async () => 42); - - expect(result).toBe(42); - expect(stdout.data()).toStrictEqual([]); - expect(stderr.data()).toStrictEqual([]); - }); - - it('step rethrows the callback error', async () => { - const stdout = new MockWritable(); - const stderr = new MockWritable(); - const output = createCliOutput({ stdout, stderr }); + output.success('Some success'); + output.error('Some error'); - await expect( - output.step('Doing something', async () => { - throw new Error('oops'); - }), - ).rejects.toThrow('oops'); - expect(stdout.data()).toStrictEqual([]); - expect(stderr.data()).toStrictEqual([]); + expect(textOf(stdout)).not.toBe(stripVTControlCharacters(textOf(stdout))); + expect(stripVTControlCharacters(textOf(stdout))).toBe( + logLine(S_SUCCESS, 'Some success'), + ); + expect(textOf(stderr)).toBe(logLine(S_ERROR, 'Some error')); }); it.each([ { - condition: 'not a TTY', + condition: 'the streams are not terminals', buildStream: (): MockWritable => new MockWritable(), arrange: (): void => undefined, }, @@ -117,220 +152,289 @@ describe('cli-output', () => { condition: 'NO_COLOR is set', buildStream: buildTerminalStream, arrange: (): void => { + vi.stubEnv('FORCE_COLOR', undefined); vi.stubEnv('NO_COLOR', '1'); }, }, { - condition: 'chalk detects no color support', + condition: 'the terminals have no colors', + buildStream: (): MockWritable => + Object.assign(buildTerminalStream(), { hasColors: () => false }), + arrange: (): void => undefined, + }, + { + condition: 'the terminals cannot tell whether they have colors', + buildStream: (): MockWritable => + Object.assign(new MockWritable(), { isTTY: true, columns: 80 }), + arrange: (): void => undefined, + }, + { + condition: 'the tool runs in GitHub Actions', + buildStream: buildTerminalStream, + arrange: (): void => { + vi.stubEnv('CI', 'true'); + vi.stubEnv('GITHUB_ACTIONS', 'true'); + }, + }, + { + condition: 'CI is 1', buildStream: buildTerminalStream, arrange: (): void => { - chalk.level = 0; - chalkStderr.level = 0; + vi.stubEnv('CI', '1'); }, }, ])( - 'writes the same bytes as plain output when $condition', + 'writes no ANSI codes when no stream is styled because $condition', async ({ buildStream, arrange }) => { arrange(); - const plainStdout = new MockWritable(); - const plainStderr = new MockWritable(); const stdout = buildStream(); const stderr = buildStream(); - await writeEveryKindOfMessage( - createCliOutput({ - stdout: plainStdout, - stderr: plainStderr, - styled: { stdout: false, stderr: false }, - }), - ); await writeEveryKindOfMessage(createCliOutput({ stdout, stderr })); - expect(stdout.data()).toStrictEqual(plainStdout.data()); - expect(stderr.data()).toStrictEqual(plainStderr.data()); - expect(ora).not.toHaveBeenCalled(); + expect(textOf(stdout)).toBe( + [ + logLine(S_INFO, 'Some info'), + logLine(S_SUCCESS, 'Some success'), + logLine(S_STEP_SUBMIT, 'Doing something'), + ].join(''), + ); + expect(textOf(stderr)).toBe( + [ + logLine(S_WARN, 'Some warning'), + logLine(S_ERROR, 'Some error'), + logLine(S_ERROR, 'Running something'), + logLine(S_BAR, 'Some line'), + ].join(''), + ); + expect(spinner).not.toHaveBeenCalled(); + expect(taskLog).not.toHaveBeenCalled(); }, ); - it('styles only stdout when stderr is not a terminal', async () => { - const stdout = buildTerminalStream(); - const stderr = new MockWritable(); - const output = createCliOutput({ stdout, stderr }); + it('shows a spinner for a step when animating, and static lines otherwise', async () => { + const stderr = buildTerminalStream(); + const animated = createCliOutput({ stdout: new MockWritable(), stderr }); + const stdout = new MockWritable(); + const plainStderr = new MockWritable(); + const plain = createCliOutput({ stdout, stderr: plainStderr }); - output.success('Some success\n'); - output.warn('Some warning\n'); - output.error('Some error\n'); - const result = await output.step('Doing something', async () => 42); + for (const output of [animated, plain]) { + expect(await output.step('Doing something', async () => 42)).toBe(42); + await expect( + output.step('Doing something else', async () => { + throw new Error('oops'); + }), + ).rejects.toThrow('oops'); + } - expect(result).toBe(42); - expect(stdout.data()).toStrictEqual([ - `${colors.green('Some success')}\n`, + expect(vi.mocked(spinner).mock.calls).toStrictEqual([ + [{ output: stderr, errorMessage: 'Doing something' }], + [{ output: stderr, errorMessage: 'Doing something else' }], ]); - expect(stderr.data()).toStrictEqual(['Some warning\n', 'Some error\n']); - expect(ora).not.toHaveBeenCalled(); + expect(indicators.spinner.start.mock.calls).toStrictEqual([ + ['Doing something'], + ['Doing something else'], + ]); + expect(indicators.spinner.stop).toHaveBeenCalledExactlyOnceWith( + 'Doing something', + ); + expect(indicators.spinner.error).toHaveBeenCalledExactlyOnceWith( + 'Doing something else', + ); + expect(stderr.data()).toStrictEqual([]); + expect(textOf(stdout)).toBe(logLine(S_STEP_SUBMIT, 'Doing something')); + expect(textOf(plainStderr)).toBe( + logLine(S_ERROR, 'Doing something else'), + ); }); - it('styles only stderr when stdout is not a terminal', async () => { + it.each([ + { + condition: 'stderr reports no width', + buildStderr: (): MockWritable => + Object.assign(buildTerminalStream(), { columns: 0 }), + arrange: (): void => undefined, + }, + { + condition: 'TERM is dumb', + buildStderr: buildTerminalStream, + arrange: (): void => { + vi.stubEnv('TERM', 'dumb'); + }, + }, + ])('shows no spinner when $condition', async ({ buildStderr, arrange }) => { + arrange(); const stdout = new MockWritable(); - const stderr = buildTerminalStream(); - const output = createCliOutput({ stdout, stderr }); + const output = createCliOutput({ stdout, stderr: buildStderr() }); - output.success('Some success\n'); - output.error('Some error\n'); await output.step('Doing something', async () => undefined); - expect(stdout.data()).toStrictEqual(['Some success\n']); - expect(stderr.data()).toStrictEqual([`${colors.red('Some error')}\n`]); - expect(ora).toHaveBeenCalledTimes(1); + expect(spinner).not.toHaveBeenCalled(); + expect(textOf(stdout)).toBe(logLine(S_STEP_SUBMIT, 'Doing something')); }); - it('colors success, warn, and error when styled', () => { - const stdout = new MockWritable(); - const stderr = new MockWritable(); - const output = createCliOutput({ - stdout, - stderr, - styled: { stdout: true, stderr: true }, - }); + it.each(['', '0', 'false'])( + 'shows a spinner when CI is "%s"', + async (value) => { + vi.stubEnv('CI', value); + const output = createCliOutput({ + stdout: new MockWritable(), + stderr: buildTerminalStream(), + }); - output.info('Some info'); - output.success('Some success'); - output.warn('Some warning'); - output.error('Some error'); + await output.step('Doing something', async () => undefined); - expect(stdout.data()).toStrictEqual([ - 'Some info', - colors.green('Some success'), - ]); - expect(stderr.data()).toStrictEqual([ - `${colors.yellow('Some warning')}\n`, - `${colors.red('Some error')}\n`, - ]); - }); + expect(spinner).toHaveBeenCalledTimes(1); + }, + ); - it('ends warn and error with a newline only when stderr is styled', () => { - const plainStderr = new MockWritable(); - const styledStderr = new MockWritable(); - const plainOutput = createCliOutput({ - stdout: new MockWritable(), - stderr: plainStderr, - styled: { stdout: false, stderr: false }, - }); - const styledOutput = createCliOutput({ - stdout: new MockWritable(), - stderr: styledStderr, - styled: { stdout: false, stderr: true }, - }); + it('streams task output and clears it when the task succeeds', async () => { + const stdout = new MockWritable(); + const stderr = buildTerminalStream(); + const output = createCliOutput({ stdout, stderr }); - for (const output of [plainOutput, styledOutput]) { - output.warn('Some warning'); - output.error('Some error'); - output.warn('Some other warning\n'); - } + const result = await output.task('Running something', async (log) => { + log('Line 1'); + log('Line 2'); + return 42; + }); - expect(plainStderr.data()).toStrictEqual([ - 'Some warning', - 'Some error', - 'Some other warning\n', - ]); - expect(styledStderr.data()).toStrictEqual([ - `${colors.yellow('Some warning')}\n`, - `${colors.red('Some error')}\n`, - `${colors.yellow('Some other warning')}\n`, + expect(result).toBe(42); + expect(taskLog).toHaveBeenCalledExactlyOnceWith({ + title: 'Running something', + limit: 10, + retainLog: true, + output: stderr, + }); + expect(indicators.taskLog.message.mock.calls).toStrictEqual([ + ['Line 1'], + ['Line 2'], ]); + expect(indicators.taskLog.success).toHaveBeenCalledExactlyOnceWith( + 'Running something', + ); + expect(indicators.taskLog.error).not.toHaveBeenCalled(); + expect(stdout.data()).toStrictEqual([]); }); - it('keeps surrounding newlines outside the color codes', () => { - const stdout = new MockWritable(); - const stderr = new MockWritable(); + it('keeps the task output when the task fails', async () => { const output = createCliOutput({ - stdout, - stderr, - styled: { stdout: true, stderr: true }, + stdout: new MockWritable(), + stderr: buildTerminalStream(), }); - output.success('Some success\n'); - output.error('\nSome error\n\n'); - output.warn('\n'); + await expect( + output.task('Running something', async (log) => { + log('Line 1'); + throw new Error('oops'); + }), + ).rejects.toThrow('oops'); - expect(stdout.data()).toStrictEqual([ - `${colors.green('Some success')}\n`, - ]); - expect(stderr.data()).toStrictEqual([ - `\n${colors.red('Some error')}\n\n`, - '\n', - ]); + expect(indicators.taskLog.message).toHaveBeenCalledExactlyOnceWith( + 'Line 1', + ); + expect(indicators.taskLog.error).toHaveBeenCalledExactlyOnceWith( + 'Running something', + { showLog: true }, + ); + expect(indicators.taskLog.success).not.toHaveBeenCalled(); }); - it('step shows a spinner and marks success', async () => { - const stdout = new MockWritable(); + it('shows only the last lines while a task runs, and every line when it fails', async () => { + const actual = + await vi.importActual( + '@clack/prompts', + ); + vi.mocked(taskLog).mockImplementation(actual.taskLog); const stderr = buildTerminalStream(); - const output = createCliOutput({ - stdout, - stderr, - styled: { stdout: true, stderr: true }, - }); + const output = createCliOutput({ stdout: new MockWritable(), stderr }); + const lines = Array.from( + { length: 15 }, + (_, index) => `Line ${index + 1}`, + ); + let writesBeforeFailure = 0; - const result = await output.step('Doing something', async () => 42); + await expect( + output.task('Running something', async (log) => { + lines.forEach(log); + writesBeforeFailure = stderr.data().length; + throw new Error('oops'); + }), + ).rejects.toThrow('oops'); - expect(result).toBe(42); - expect(ora).toHaveBeenCalledWith({ - text: 'Doing something', - stream: stderr, - discardStdin: false, - isEnabled: true, - }); - expect(spinner.start).toHaveBeenCalledTimes(1); - expect(spinner.succeed).toHaveBeenCalledTimes(1); - expect(spinner.fail).not.toHaveBeenCalled(); + const lastRedraw = stripVTControlCharacters( + stderr + .data() + .slice(writesBeforeFailure - 1, writesBeforeFailure) + .join(''), + ); + expect(lastRedraw).not.toContain('Line 5\n'); + expect(lastRedraw).toContain('Line 6\n'); + const failure = stripVTControlCharacters( + stderr.data().slice(writesBeforeFailure).join(''), + ); + expect(failure).toContain(`${S_ERROR} Running something`); + for (const line of lines) { + expect(failure).toContain(`${S_BAR} ${line}\n`); + } }); - it('step marks failure and rethrows', async () => { + it('prints the task output only when it fails in plain output', async () => { const stdout = new MockWritable(); - const stderr = buildTerminalStream(); - const output = createCliOutput({ - stdout, - stderr, - styled: { stdout: true, stderr: true }, - }); + const stderr = new MockWritable(); + const output = createCliOutput({ stdout, stderr }); + const result = await output.task('Running something', async (log) => { + log('Hidden line'); + return 42; + }); await expect( - output.step('Doing something', async () => { + output.task('Running something else', async (log) => { + log('Line 1'); + log('Line 2'); throw new Error('oops'); }), ).rejects.toThrow('oops'); - expect(spinner.start).toHaveBeenCalledTimes(1); - expect(spinner.fail).toHaveBeenCalledTimes(1); - expect(spinner.succeed).not.toHaveBeenCalled(); + + expect(result).toBe(42); + expect(textOf(stdout)).toBe(logLine(S_STEP_SUBMIT, 'Running something')); + expect(textOf(stderr)).toBe( + [ + logLine(S_ERROR, 'Running something else'), + `${S_BAR}\n${S_BAR} Line 1\n${S_BAR} Line 2\n`, + ].join(''), + ); + expect(taskLog).not.toHaveBeenCalled(); }); it.each([ + { keyName: 'Ctrl+C', input: '\u0003', key: { ctrl: true, name: 'c' } }, { - description: '0 columns', - buildStderr: (): MockWritable => - Object.assign(new MockWritable(), { columns: 0 }), + keyName: 'Escape', + input: '\u001b', + key: { ctrl: false, name: 'escape' }, }, - { - description: 'no columns', - buildStderr: (): MockWritable => new MockWritable(), - }, - ])( - 'step does not show a spinner when stderr reports $description', - async ({ buildStderr }) => { - const stdout = new MockWritable(); - const stderr = buildStderr(); - const output = createCliOutput({ - stdout, - stderr, - styled: { stdout: true, stderr: true }, - }); - - const result = await output.step('Doing something', async () => 42); + ])('exits with 130 on $keyName during a step', async ({ input, key }) => { + const exit = vi + .spyOn(process, 'exit') + .mockImplementation(() => undefined as never); + const output = createCliOutput({ + stdout: new MockWritable(), + stderr: buildTerminalStream(), + }); + const press = (): void => { + process.stdin.emit('keypress', input, key); + }; + + await output.step('Doing something', async () => { + process.stdin.emit('keypress', 'c', { ctrl: false, name: 'c' }); + expect(exit).not.toHaveBeenCalled(); + press(); + }); + press(); - expect(result).toBe(42); - expect(ora).not.toHaveBeenCalled(); - expect(stderr.data()).toStrictEqual([]); - }, - ); + expect(exit).toHaveBeenCalledExactlyOnceWith(130); + }); }); }); diff --git a/packages/create-release-branch/src/core/cli-output.ts b/packages/create-release-branch/src/core/cli-output.ts index cede385..fb2aeaa 100644 --- a/packages/create-release-branch/src/core/cli-output.ts +++ b/packages/create-release-branch/src/core/cli-output.ts @@ -1,15 +1,12 @@ -import chalk, { Chalk, chalkStderr } from 'chalk'; -import type { ChalkInstance } from 'chalk'; -import ora from 'ora'; - -import type { WriteStreamLike } from '../utils/fs.js'; +import { log, spinner, taskLog } from '@clack/prompts'; +import { Writable } from 'node:stream'; +import { stripVTControlCharacters, styleText } from 'node:util'; /** * The single place through which the tool writes user-facing messages. * - * Each method writes the text exactly as given, so callers include any - * trailing newline themselves. The one exception: when standard error is - * styled, warnings and errors always end with a newline. + * Each message is laid out by clack, which adds its own spacing, so any + * trailing newlines in the text are dropped. * * @property info - Writes an informational message to standard out. * @property success - Writes a message about a completed action to standard @@ -17,6 +14,9 @@ import type { WriteStreamLike } from '../utils/fs.js'; * @property warn - Writes a warning to standard error. * @property error - Writes an error to standard error. * @property step - Runs a long-running action and returns its result. + * @property task - Runs a long-running action that prints lines of output, + * such as a command, and returns its result. The lines are shown only while + * the action runs, or if it fails. */ export type CliOutput = { info(text: string): void; @@ -24,122 +24,247 @@ export type CliOutput = { warn(text: string): void; error(text: string): void; step(label: string, run: () => Promise): Promise; + task( + label: string, + run: (log: (line: string) => void) => Promise, + ): Promise; +}; + +/** + * A stream that the output can write to, such as `process.stdout`. + */ +export type OutputStream = Writable & { + isTTY?: boolean; + columns?: number; + hasColors?: () => boolean; }; /** - * Applies a color to the text, leaving any leading and trailing whitespace - * outside of the color codes. + * A key that `readline` reports in a `keypress` event. + */ +type Key = { ctrl?: boolean; name?: string }; + +/** + * The exit code that shells use for a process stopped with Ctrl+C. + */ +const INTERRUPTED_EXIT_CODE = 130; + +/** + * The number of lines of output that a running task shows. + */ +const TASK_LOG_LIMIT = 10; + +let isExitingOnInterrupt = false; + +/** + * Runs an action and exits with code 130 if the user presses Ctrl+C or + * Escape meanwhile. * - * @param text - The text. - * @param color - The color to apply. - * @returns The colored text. + * A clack spinner reads standard in in raw mode, so Ctrl+C arrives as a + * keypress. On Ctrl+C or Escape, clack exits with code 0. The spinner also + * listens to SIGINT, and then only stops itself. The listeners here are added + * before any spinner starts, so they run first: Ctrl+C and Escape become a + * SIGINT, and a SIGINT exits with code 130. The SIGINT listener is added once and stays, so a + * later SIGINT also exits with code 130, the code a shell reports by default. + * + * @param run - The action. + * @returns The result of the action. */ -function colorize(text: string, color: ChalkInstance): string { - const content = text.trim(); - const start = text.indexOf(content); - return `${text.slice(0, start)}${color(content)}${text.slice(start + content.length)}`; +async function exitOnInterrupt( + run: () => Promise, +): Promise { + if (!isExitingOnInterrupt) { + isExitingOnInterrupt = true; + process.on('SIGINT', () => { + process.exit(INTERRUPTED_EXIT_CODE); + }); + } + + const onKeypress = (_input: unknown, key: Key | undefined): void => { + if ((key?.ctrl && key.name === 'c') || key?.name === 'escape') { + process.emit('SIGINT', 'SIGINT'); + } + }; + process.stdin.on('keypress', onKeypress); + + try { + return await run(); + } finally { + process.stdin.off('keypress', onKeypress); + } } /** - * Adds a newline to the end of the text if it does not already end with one. + * Determines whether the tool runs in CI. Unlike clack's `isCI`, this accepts + * any value of `CI` other than empty, `0`, or `false`. * - * @param text - The text. - * @returns The text, ending with a newline. + * @returns Whether `CI` is set to a value that means true. */ -function endWithNewline(text: string): string { - return text.endsWith('\n') ? text : `${text}\n`; +function isRunningInCI(): boolean { + const { CI } = process.env; + return CI !== undefined && !['', '0', 'false'].includes(CI); } /** - * Determines whether a stream is a terminal that shows colors. + * Determines whether a stream is a terminal that shows colors, outside of CI. * * @param stream - The stream. - * @param colorLevel - The color level that chalk detected for the stream. * @returns Whether to style what is written to the stream. */ -function isColorTerminal( - stream: { isTTY?: boolean }, - colorLevel: number, -): boolean { +function isColorTerminal(stream: OutputStream): boolean { return ( Boolean(stream.isTTY) && - colorLevel > 0 && - process.env.NO_COLOR === undefined + (stream.hasColors?.() ?? false) && + process.env.NO_COLOR === undefined && + !isRunningInCI() ); } +/** + * Wraps a stream so that what is written to it has no ANSI codes. clack + * decides on colors from `process.stdout`, even for text sent to another + * stream. + * + * @param stream - The stream. + * @returns The wrapped stream. + */ +function withoutColors(stream: Writable): Writable { + return new Writable({ + write(chunk: Buffer | string, _encoding, callback): void { + stream.write(stripVTControlCharacters(chunk.toString())); + callback(); + }, + }); +} + +/** + * Removes the newlines at the end of the text. + * + * @param text - The text. + * @returns The text, without trailing newlines. + */ +function trimTrailingNewlines(text: string): string { + return text.replace(/\n+$/u, ''); +} + /** * Creates a `CliOutput` that writes to the given streams. * - * When standard out is styled, success messages are colored. When standard - * error is styled, warnings and errors are colored and end with a newline, - * and, if standard error reports a width, each step shows a spinner on it. - * Otherwise, the text is written as given and steps write nothing. + * Messages, and lines for completed steps and tasks, go to standard out. + * Warnings, errors, spinners, and task output go to standard error. A stream + * that is not styled gets no ANSI codes. When animating, a step shows a + * spinner and a task streams its output until it ends. Otherwise, each ends + * with one line, and a task prints its output only if it fails. * * @param args - The arguments. * @param args.stdout - A stream that can be used to write to standard out. * @param args.stderr - A stream that can be used to write to standard error. * @param args.styled - Whether to style each stream. By default, a stream is - * styled when it is a terminal, chalk detects color support for it, and - * `NO_COLOR` is not set. + * styled when it is a terminal that has colors, `NO_COLOR` is not set, and the + * tool does not run in CI. + * @param args.animate - Whether to show spinners and live task output on + * standard error. By default, this is the case when standard error is styled + * and reports a width, and when `TERM` is not `dumb`. * @returns The output. */ export function createCliOutput({ stdout, stderr, styled = { - stdout: isColorTerminal(stdout, chalk.level), - stderr: isColorTerminal(stderr, chalkStderr.level), + stdout: isColorTerminal(stdout), + stderr: isColorTerminal(stderr), }, + animate = styled.stderr && + Boolean(stderr.columns) && + process.env.TERM !== 'dumb', }: { - stdout: WriteStreamLike & { isTTY?: boolean }; - stderr: NodeJS.WritableStream & { isTTY?: boolean; columns?: number }; + stdout: OutputStream; + stderr: OutputStream; styled?: { stdout: boolean; stderr: boolean }; + animate?: boolean; }): CliOutput { - const stdoutColors = new Chalk({ level: styled.stdout ? 1 : 0 }); - const stderrColors = new Chalk({ level: styled.stderr ? 1 : 0 }); - // A running spinner clears the current line on its next frame, which would - // erase a message that does not end with a newline. - const toStderr = (text: string): string => - styled.stderr ? endWithNewline(text) : text; + const toStdout = { output: styled.stdout ? stdout : withoutColors(stdout) }; + const toStderr = { output: styled.stderr ? stderr : withoutColors(stderr) }; return { info(text) { - stdout.write(text); + log.info(trimTrailingNewlines(text), toStdout); }, success(text) { - stdout.write(colorize(text, stdoutColors.green)); + log.success(trimTrailingNewlines(text), toStdout); }, warn(text) { - stderr.write(colorize(toStderr(text), stderrColors.yellow)); + log.warn(trimTrailingNewlines(text), toStderr); }, error(text) { - stderr.write(colorize(toStderr(text), stderrColors.red)); + log.error(trimTrailingNewlines(text), toStderr); }, async step(label, run) { - // ora needs the width of the terminal to draw the spinner. - if (!styled.stderr || !stderr.columns) { - return await run(); + if (!animate) { + try { + const result = await run(); + log.step(label, toStdout); + return result; + } catch (error) { + log.error(label, toStderr); + throw error; + } } - // Leave stdin alone so that a prompt during the step still receives - // keystrokes. Standard error is already known to be a color terminal - // here, so ora's own check (which is off in CI) is skipped. - const spinner = ora({ - text: label, - stream: stderr, - discardStdin: false, - isEnabled: true, + // clack shows `errorMessage` if the process exits during the step. + const indicator = spinner({ ...toStderr, errorMessage: label }); + return await exitOnInterrupt(async () => { + indicator.start(label); + try { + const result = await run(); + indicator.stop(label); + return result; + } catch (error) { + indicator.error(label); + throw error; + } }); - spinner.start(); - try { - const result = await run(); - spinner.succeed(); - return result; - } catch (error) { - spinner.fail(); - throw error; + }, + async task(label, run) { + if (!animate) { + const lines: string[] = []; + try { + const result = await run((line) => { + lines.push(line); + }); + log.step(label, toStdout); + return result; + } catch (error) { + log.error(label, toStderr); + log.message( + lines.map((line) => styleText('dim', line, { stream: stderr })), + toStderr, + ); + throw error; + } } + + // clack redraws every shown line on each message, which breaks once the + // lines are taller than the terminal. `retainLog` keeps the full log + // for a failure. + const taskOutput = taskLog({ + title: label, + limit: TASK_LOG_LIMIT, + retainLog: true, + ...toStderr, + }); + return await exitOnInterrupt(async () => { + try { + const result = await run((line) => { + taskOutput.message(line); + }); + taskOutput.success(label); + return result; + } catch (error) { + taskOutput.error(label, { showLog: true }); + throw error; + } + }); }, }; } diff --git a/packages/create-release-branch/src/core/release-steps.test.ts b/packages/create-release-branch/src/core/release-steps.test.ts index 76fa238..a088604 100644 --- a/packages/create-release-branch/src/core/release-steps.test.ts +++ b/packages/create-release-branch/src/core/release-steps.test.ts @@ -24,9 +24,11 @@ vi.mock('./repo'); vi.mock('./yarn-commands'); /** - * Builds an output that records when each step starts and ends. + * Builds an output that records when each step and task starts and ends, and + * the lines that each task logs. * - * @param calls - The list to which the start and end of each step are added. + * @param calls - The list to which the start and end of each step and task, + * and the lines, are added. * @returns The output. */ function buildRecordingOutput(calls: string[]): CliOutput { @@ -38,6 +40,14 @@ function buildRecordingOutput(calls: string[]): CliOutput { calls.push(`end ${label}`); return result; }, + async task(label, run) { + calls.push(`start task ${label}`); + const result = await run((line) => { + calls.push(`log ${line}`); + }); + calls.push(`end task ${label}`); + return result; + }, }; } @@ -303,24 +313,27 @@ describe('release-steps', () => { }); describe('finishRelease', () => { - it('runs each Yarn command in a step, then commits outside any step', async () => { + it('runs each Yarn command in a task, then commits outside any step', async () => { const project = buildMockProject(); const calls: string[] = []; vi.spyOn(yarnCommandsModule, 'fixConstraints').mockImplementation( - async (directoryPath) => { + async (directoryPath, onLine) => { calls.push(`fixConstraints ${directoryPath}`); + onLine('constraints output'); }, ); vi.spyOn(yarnCommandsModule, 'updateYarnLockfile').mockImplementation( - async (directoryPath) => { + async (directoryPath, onLine) => { calls.push(`updateYarnLockfile ${directoryPath}`); + onLine('install output'); }, ); vi.spyOn( yarnCommandsModule, 'deduplicateDependencies', - ).mockImplementation(async (directoryPath) => { + ).mockImplementation(async (directoryPath, onLine) => { calls.push(`deduplicateDependencies ${directoryPath}`); + onLine('dedupe output'); }); vi.spyOn(repoModule, 'commitAllChanges').mockImplementation( async (directoryPath, commitMessage) => { @@ -335,15 +348,18 @@ describe('release-steps', () => { }); expect(calls).toStrictEqual([ - 'start Fixing Yarn constraints', + 'start task Fixing Yarn constraints', 'fixConstraints /path/to/project', - 'end Fixing Yarn constraints', - 'start Installing dependencies', + 'log constraints output', + 'end task Fixing Yarn constraints', + 'start task Installing dependencies', 'updateYarnLockfile /path/to/project', - 'end Installing dependencies', - 'start Deduplicating dependencies', + 'log install output', + 'end task Installing dependencies', + 'start task Deduplicating dependencies', 'deduplicateDependencies /path/to/project', - 'end Deduplicating dependencies', + 'log dedupe output', + 'end task Deduplicating dependencies', 'commitAllChanges /path/to/project Update Release 2.0.0', ]); }); diff --git a/packages/create-release-branch/src/core/release-steps.ts b/packages/create-release-branch/src/core/release-steps.ts index 7af9cb9..8bf2ee2 100644 --- a/packages/create-release-branch/src/core/release-steps.ts +++ b/packages/create-release-branch/src/core/release-steps.ts @@ -165,17 +165,17 @@ export async function finishRelease({ newReleaseVersion: string; output: CliOutput; }): Promise { - await output.step( + await output.task( 'Fixing Yarn constraints', - async () => await fixConstraints(project.directoryPath), + async (log) => await fixConstraints(project.directoryPath, log), ); - await output.step( + await output.task( 'Installing dependencies', - async () => await updateYarnLockfile(project.directoryPath), + async (log) => await updateYarnLockfile(project.directoryPath, log), ); - await output.step( + await output.task( 'Deduplicating dependencies', - async () => await deduplicateDependencies(project.directoryPath), + async (log) => await deduplicateDependencies(project.directoryPath, log), ); // Not run in a step: a spinner would draw over a GPG pinentry in the // terminal. diff --git a/packages/create-release-branch/src/core/run-release.test.ts b/packages/create-release-branch/src/core/run-release.test.ts index 12b50f9..fcc9432 100644 --- a/packages/create-release-branch/src/core/run-release.test.ts +++ b/packages/create-release-branch/src/core/run-release.test.ts @@ -48,9 +48,9 @@ describe('run-release', () => { await runRelease(context, { run }); - expect(stdout.data()).toStrictEqual([ - 'Project appears to have workspaces. Following monorepo workflow.\n', - ]); + expect(stdout.data().join('')).toContain( + 'Project appears to have workspaces. Following monorepo workflow.', + ); expect(run).toHaveBeenCalledWith(context); }); @@ -61,9 +61,9 @@ describe('run-release', () => { await runRelease(context, { run }); - expect(stdout.data()).toStrictEqual([ - 'Project does not appear to have any workspaces. Following polyrepo workflow.\n', - ]); + expect(stdout.data().join('')).toContain( + 'Project does not appear to have any workspaces. Following polyrepo workflow.', + ); expect(run).not.toHaveBeenCalled(); }); }); diff --git a/packages/create-release-branch/src/core/yarn-commands.test.ts b/packages/create-release-branch/src/core/yarn-commands.test.ts index c35bb48..7ca1509 100644 --- a/packages/create-release-branch/src/core/yarn-commands.test.ts +++ b/packages/create-release-branch/src/core/yarn-commands.test.ts @@ -13,18 +13,17 @@ describe('yarn-commands', () => { describe('fixConstraints', () => { it('runs "yarn constraints --fix" with the correct parameters', async () => { const repositoryDirectoryPath = '/path/to/repo'; - vi.when(vi.spyOn(miscUtils, 'getStdoutFromCommand')) - .calledWith('yarn', ['--version']) - .thenResolve('2.0.0'); + const onLine = vi.fn(); - await fixConstraints(repositoryDirectoryPath); + await fixConstraints(repositoryDirectoryPath, onLine); - expect(miscUtils.runCommand).toHaveBeenCalledWith( + expect(miscUtils.runCommandWithOutput).toHaveBeenCalledWith( 'yarn', ['constraints', '--fix'], { cwd: repositoryDirectoryPath, }, + onLine, ); }); }); @@ -32,15 +31,17 @@ describe('yarn-commands', () => { describe('updateYarnLockfile', () => { it('runs "yarn install --no--immutable" with the correct parameters', async () => { const repositoryDirectoryPath = '/path/to/repo'; + const onLine = vi.fn(); - await updateYarnLockfile('/path/to/repo'); + await updateYarnLockfile('/path/to/repo', onLine); - expect(miscUtils.runCommand).toHaveBeenCalledWith( + expect(miscUtils.runCommandWithOutput).toHaveBeenCalledWith( 'yarn', ['install', '--no-immutable'], { cwd: repositoryDirectoryPath, }, + onLine, ); }); }); @@ -48,12 +49,18 @@ describe('yarn-commands', () => { describe('deduplicateDependencies', () => { it('runs "yarn dedupe" with the correct parameters', async () => { const repositoryDirectoryPath = '/path/to/repo'; + const onLine = vi.fn(); - await deduplicateDependencies('/path/to/repo'); + await deduplicateDependencies('/path/to/repo', onLine); - expect(miscUtils.runCommand).toHaveBeenCalledWith('yarn', ['dedupe'], { - cwd: repositoryDirectoryPath, - }); + expect(miscUtils.runCommandWithOutput).toHaveBeenCalledWith( + 'yarn', + ['dedupe'], + { + cwd: repositoryDirectoryPath, + }, + onLine, + ); }); }); }); diff --git a/packages/create-release-branch/src/core/yarn-commands.ts b/packages/create-release-branch/src/core/yarn-commands.ts index 4064f19..c4114d1 100644 --- a/packages/create-release-branch/src/core/yarn-commands.ts +++ b/packages/create-release-branch/src/core/yarn-commands.ts @@ -1,18 +1,22 @@ -import { debug, runCommand } from '../utils/misc-utils.js'; +import { debug, runCommandWithOutput } from '../utils/misc-utils.js'; /** * Runs `yarn constraints --fix` to autofix all unmet constraints. * * @param repositoryDirectoryPath - The path to the repository directory. - * @returns The standard output of the command. + * @param onLine - Receives each line of output of the command. * @throws An execa error object if the command fails in some way. */ export async function fixConstraints( repositoryDirectoryPath: string, + onLine: (line: string) => void, ): Promise { - await runCommand('yarn', ['constraints', '--fix'], { - cwd: repositoryDirectoryPath, - }); + await runCommandWithOutput( + 'yarn', + ['constraints', '--fix'], + { cwd: repositoryDirectoryPath }, + onLine, + ); debug('Yarn constraints fixed successfully.'); } @@ -20,30 +24,38 @@ export async function fixConstraints( * Runs `yarn install --no-immutable` to update the Yarn lockfile. * * @param repositoryDirectoryPath - The path to the repository directory. - * @returns The standard output of the command. + * @param onLine - Receives each line of output of the command. * @throws An execa error object if the command fails in some way. */ export async function updateYarnLockfile( repositoryDirectoryPath: string, + onLine: (line: string) => void, ): Promise { debug('Installing dependencies...'); - await runCommand('yarn', ['install', '--no-immutable'], { - cwd: repositoryDirectoryPath, - }); + await runCommandWithOutput( + 'yarn', + ['install', '--no-immutable'], + { cwd: repositoryDirectoryPath }, + onLine, + ); } /** * Runs `yarn dedupe` to deduplicate dependencies in the project. * * @param repositoryDirectoryPath - The path to the repository directory. - * @returns The standard output of the command. + * @param onLine - Receives each line of output of the command. * @throws An execa error object if the command fails in some way. */ export async function deduplicateDependencies( repositoryDirectoryPath: string, + onLine: (line: string) => void, ): Promise { debug('Deduplicating dependencies...'); - await runCommand('yarn', ['dedupe'], { - cwd: repositoryDirectoryPath, - }); + await runCommandWithOutput( + 'yarn', + ['dedupe'], + { cwd: repositoryDirectoryPath }, + onLine, + ); } diff --git a/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts b/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts index 4f80f86..16ec854 100644 --- a/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts +++ b/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts @@ -587,16 +587,21 @@ describe('editor-front-end', () => { ); expect(fixConstraintsSpy).toHaveBeenCalledTimes(1); - expect(fixConstraintsSpy).toHaveBeenCalledWith(projectDirectoryPath); + expect(fixConstraintsSpy).toHaveBeenCalledWith( + projectDirectoryPath, + expect.any(Function), + ); expect(updateYarnLockfileSpy).toHaveBeenCalledTimes(1); expect(updateYarnLockfileSpy).toHaveBeenCalledWith( projectDirectoryPath, + expect.any(Function), ); expect(deduplicateDependenciesSpy).toHaveBeenCalledTimes(1); expect(deduplicateDependenciesSpy).toHaveBeenCalledWith( projectDirectoryPath, + expect.any(Function), ); // Second call of followMonorepoWorkflow @@ -998,8 +1003,8 @@ describe('editor-front-end', () => { output, }); - expect(stdout.data()[0]).toMatch( - /^A template has been generated that specifies this release/u, + expect(stdout.data().join('')).toContain( + 'A template has been generated that specifies this release', ); }); }); @@ -1761,8 +1766,8 @@ describe('editor-front-end', () => { output, }); - expect(stdout.data()[0]).toMatch( - /^A template has been generated that specifies this release/u, + expect(stdout.data().join('')).toContain( + 'A template has been generated that specifies this release', ); }); }); @@ -2285,8 +2290,8 @@ describe('editor-front-end', () => { output, }); - expect(stdout.data()[0]).toMatch( - /^A template has been generated that specifies this release/u, + expect(stdout.data().join('')).toContain( + 'A template has been generated that specifies this release', ); }); }); diff --git a/packages/create-release-branch/src/frontends/ui/server.test.ts b/packages/create-release-branch/src/frontends/ui/server.test.ts index 7d16ff2..33f191e 100644 --- a/packages/create-release-branch/src/frontends/ui/server.test.ts +++ b/packages/create-release-branch/src/frontends/ui/server.test.ts @@ -249,7 +249,7 @@ describe('server', () => { }, ); - expect(stderr.data()).toStrictEqual(['Release error: Error: boom\n']); + expect(stderr.data().join('')).toContain('Release error: Error: boom'); }); }); @@ -320,12 +320,15 @@ describe('server', () => { expect(apply).toHaveBeenCalledWith(plan, '2.0.0'); expect(yarnCommandsModule.fixConstraints).toHaveBeenCalledWith( '/path/to/project', + expect.any(Function), ); expect(yarnCommandsModule.updateYarnLockfile).toHaveBeenCalledWith( '/path/to/project', + expect.any(Function), ); expect(yarnCommandsModule.deduplicateDependencies).toHaveBeenCalledWith( '/path/to/project', + expect.any(Function), ); expect(repoModule.commitAllChanges).toHaveBeenCalledWith( '/path/to/project', @@ -396,7 +399,7 @@ describe('server', () => { }, ); - expect(stderr.data()).toStrictEqual(['Release error: Error: boom\n']); + expect(stderr.data().join('')).toContain('Release error: Error: boom'); expect(closeServer).not.toHaveBeenCalled(); }); }); @@ -441,7 +444,9 @@ describe('server', () => { }, ); - expect(stderr.data()).toStrictEqual(['Changelog error: Error: boom\n']); + expect(stderr.data().join('')).toContain( + 'Changelog error: Error: boom', + ); }); }); }); @@ -466,8 +471,8 @@ describe('server', () => { const running = createUIFrontEnd({ port }).run(context); await vi.waitFor(() => { - expect(stdout.data()).toContain( - `\nUI server running at http://localhost:${port}\n`, + expect(stdout.data().join('')).toContain( + `UI server running at http://localhost:${port}`, ); }); const response = await postChoices( @@ -478,11 +483,9 @@ describe('server', () => { expect(await response.json()).toStrictEqual({ status: 'success' }); expect(openBrowser).toHaveBeenCalledWith(`http://localhost:${port}`); - expect(stdout.data()).toStrictEqual([ - `\nAttempting to open UI in browser...`, - `\nUI server running at http://localhost:${port}\n`, - 'Release process completed. Server shutdown.\n', - ]); + const text = stdout.data().join(''); + expect(text).toContain('Attempting to open UI in browser...'); + expect(text).toContain('Release process completed. Server shutdown.'); }); it('asks to open the URL manually if the browser does not open', async () => { @@ -509,13 +512,12 @@ describe('server', () => { await postChoices(`http://localhost:${port}/api/release`, {}); await running; - expect(stderr.data()).toStrictEqual([ - `\n---------------------------------------------------\n`, - `Error automatically opening browser: Error: no browser\n`, - `Please open the following URL manually:\n`, - `http://localhost:${port}\n`, - `---------------------------------------------------\n\n`, - ]); + const text = stderr.data().join(''); + expect(text).toContain( + 'Error automatically opening browser: Error: no browser', + ); + expect(text).toContain('Please open the following URL manually:'); + expect(text).toContain(`http://localhost:${port}`); }); it('rejects if the server cannot start', async () => { @@ -541,9 +543,9 @@ describe('server', () => { blocker.close(); } - expect(stderr.data()).toStrictEqual([ - expect.stringMatching(/^Failed to start server: .*EADDRINUSE.*\n$/u), - ]); + expect(stderr.data().join('')).toMatch( + /Failed to start server: .*EADDRINUSE/u, + ); }); }); }); diff --git a/packages/create-release-branch/src/project/package.test.ts b/packages/create-release-branch/src/project/package.test.ts index fb893d8..642526b 100644 --- a/packages/create-release-branch/src/project/package.test.ts +++ b/packages/create-release-branch/src/project/package.test.ts @@ -398,9 +398,9 @@ describe('package', () => { output, }); - expect(stderr.data()).toStrictEqual([ - 'WARNING: Could not determine changes for workspace package @scope/workspace-package version 1.0.0 based on Git tag "@scope/workspace-package@1.0.0"; using tag for root package root-package version 5.0.0, "v5.0.0", instead.\n', - ]); + expect(stderr.data().join('')).toContain( + 'WARNING: Could not determine changes for workspace package @scope/workspace-package version 1.0.0 based on Git tag "@scope/workspace-package@1.0.0"; using tag for root package root-package version 5.0.0, "v5.0.0", instead.', + ); }); it("throws if the project has tags, but neither a tag matching the package name + version nor 'v' + the root package version exists", async () => { diff --git a/packages/create-release-branch/src/utils/fs.ts b/packages/create-release-branch/src/utils/fs.ts index 793beda..bb48130 100644 --- a/packages/create-release-branch/src/utils/fs.ts +++ b/packages/create-release-branch/src/utils/fs.ts @@ -6,12 +6,6 @@ import fs from 'fs'; import { wrapError, isErrorWithCode } from './misc-utils.js'; -/** - * Represents a writeable stream, such as that represented by `process.stdout` - * or `process.stderr`, or a fake one provided in tests. - */ -export type WriteStreamLike = Pick; - /** * Reads the file at the given path, assuming its content is encoded as UTF-8. * diff --git a/packages/create-release-branch/src/utils/misc-utils.test.ts b/packages/create-release-branch/src/utils/misc-utils.test.ts index 6379194..e3d9f03 100644 --- a/packages/create-release-branch/src/utils/misc-utils.test.ts +++ b/packages/create-release-branch/src/utils/misc-utils.test.ts @@ -1,6 +1,6 @@ import * as execaModule from 'execa'; import type { ExecaReturnValue } from 'execa'; -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import * as whichModule from 'which'; import { @@ -10,6 +10,7 @@ import { wrapError, resolveExecutable, runCommand, + runCommandWithOutput, getStdoutFromCommand, getLinesFromCommand, convertToHttpsGitHubRepositoryUrl, @@ -185,6 +186,64 @@ describe('misc-utils', () => { }); }); + describe('runCommandWithOutput', () => { + beforeEach(async () => { + const { execa } = await vi.importActual('execa'); + vi.spyOn(execaModule, 'execa').mockImplementation(execa); + }); + + it('passes each output line to onLine', async () => { + const lines: string[] = []; + const args = [ + '-e', + "process.stdout.write('Line 1\\nLine 2\\n'); setTimeout(() => process.stderr.write('Line 3'), 100);", + ]; + + await runCommandWithOutput(process.execPath, args, {}, (line) => { + lines.push(line); + }); + + expect(execaModule.execa).toHaveBeenCalledWith(process.execPath, args, { + all: true, + buffer: false, + }); + expect(lines).toStrictEqual(['Line 1', 'Line 2', 'Line 3']); + }); + + it('rejects when the command fails', async () => { + const lines: string[] = []; + + await expect( + runCommandWithOutput( + process.execPath, + ['-e', "console.log('Some output'); process.exit(1);"], + {}, + (line) => { + lines.push(line); + }, + ), + ).rejects.toThrow('Command failed with exit code 1'); + expect(lines).toStrictEqual(['Some output']); + }); + + it('rejects without the command output in the message', async () => { + const lines: string[] = []; + + const error = await runCommandWithOutput( + process.execPath, + ['-e', "console.log('Some' + ' output'); process.exit(1);"], + {}, + (line) => { + lines.push(line); + }, + ).catch((caughtError: unknown) => caughtError); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).not.toContain('Some output'); + expect(lines).toStrictEqual(['Some output']); + }); + }); + describe('getStdoutFromCommand', () => { it('executes the given command and returns a version of the standard out from the command with whitespace trimmed', async () => { const execaSpy = vi diff --git a/packages/create-release-branch/src/utils/misc-utils.ts b/packages/create-release-branch/src/utils/misc-utils.ts index 75a00c6..ab5c373 100644 --- a/packages/create-release-branch/src/utils/misc-utils.ts +++ b/packages/create-release-branch/src/utils/misc-utils.ts @@ -1,6 +1,7 @@ -import { isObject } from '@metamask/utils'; +import { assert, isObject } from '@metamask/utils'; import createDebug from 'debug'; import { execa, Options } from 'execa'; +import { createInterface } from 'node:readline'; import { ErrorWithCause } from 'pony-cause'; import which from 'which'; @@ -152,6 +153,33 @@ export async function runCommand( await execa(command, args, options); } +/** + * Runs a command, passing each line of its standard out and standard error to + * a callback as the command prints it. + * + * @param command - The command to execute. + * @param args - The positional arguments to the command. + * @param options - The options to `execa`. + * @param onLine - Receives each line of output. + * @throws An `execa` error object if the command fails in some way. + * @see `execa`. + */ +export async function runCommandWithOutput( + command: string, + args: readonly string[], + options: Options, + onLine: (line: string) => void, +): Promise { + const subprocess = execa(command, args, { + ...options, + all: true, + buffer: false, + }); + assert(subprocess.all, 'Expected `all` to be set by the `all` option.'); + createInterface({ input: subprocess.all }).on('line', onLine); + await subprocess; +} + /** * Runs a command, retrieving the standard output with leading and trailing * whitespace removed. diff --git a/yarn.lock b/yarn.lock index 1c2a0ff..0c5a699 100644 --- a/yarn.lock +++ b/yarn.lock @@ -440,7 +440,7 @@ __metadata: languageName: node linkType: hard -"@clack/prompts@npm:^1.7.0": +"@clack/prompts@npm:^1.7.0, @clack/prompts@npm:^1.8.1": version: 1.8.1 resolution: "@clack/prompts@npm:1.8.1" dependencies: @@ -1069,6 +1069,7 @@ __metadata: version: 0.0.0-use.local resolution: "@metamask/create-release-branch@workspace:packages/create-release-branch" dependencies: + "@clack/prompts": "npm:^1.8.1" "@metamask/action-utils": "npm:^2.0.0" "@metamask/auto-changelog": "npm:^7.1.0" "@metamask/utils": "npm:^11.12.0" @@ -1085,14 +1086,12 @@ __metadata: "@typescript/native": "npm:typescript@^7.0.2" "@vitejs/plugin-react": "npm:^6.1.2" "@vitest/coverage-istanbul": "npm:^5.0.1" - chalk: "npm:^5.6.2" debug: "npm:^4.3.4" deepmerge: "npm:^4.2.2" execa: "npm:^8.0.1" express: "npm:^4.21.2" nanoid: "npm:^3.3.4" open: "npm:^10.1.0" - ora: "npm:^9.4.1" oxfmt: "npm:^0.45.0" pony-cause: "npm:^2.1.9" prettier: "npm:^3.3.3"