diff --git a/packages/create-release-branch/CHANGELOG.md b/packages/create-release-branch/CHANGELOG.md index d4cb848..e5e5dce 100644 --- a/packages/create-release-branch/CHANGELOG.md +++ b/packages/create-release-branch/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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)) - Show errors without a stack trace (set `DEBUG=create-release-branch:*` to see it), with their causes, and list release spec problems by line and package ([#160](https://github.com/MetaMask/release-tools/pull/160)) - Group repeated warnings (packages without a tag, changelogs without updates) into one warning with a list of packages ([#162](https://github.com/MetaMask/release-tools/pull/162)) +- Start the CLI output with a header, end each step with its result, and end the run with a summary of the released packages ([#163](https://github.com/MetaMask/release-tools/pull/163)) ### Fixed diff --git a/packages/create-release-branch/src/cli/main.test.ts b/packages/create-release-branch/src/cli/main.test.ts index 2576d55..32d0402 100644 --- a/packages/create-release-branch/src/cli/main.test.ts +++ b/packages/create-release-branch/src/cli/main.test.ts @@ -64,7 +64,7 @@ describe('main', () => { output: expect.any(Object) as CliOutput, }); expect(stdout.data().join('')).toContain( - 'Project appears to have workspaces. Following monorepo workflow.', + 'create-release-branch root · monorepo · legacy mode', ); }); 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 585bbf6..6f13df2 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/packages/create-release-branch/src/core/cli-output.test.ts @@ -1,6 +1,7 @@ import { S_BAR, S_BAR_END, + S_BAR_START, S_ERROR, S_INFO, S_STEP_SUBMIT, @@ -494,6 +495,114 @@ describe('cli-output', () => { ); }); + it('header writes the intro', () => { + const plainStdout = new MockWritable(); + const styledStdout = buildTerminalStream(); + const stderr = new MockWritable(); + + createCliOutput({ stdout: plainStdout, stderr }).header( + 'Some title', + 'some details', + ); + createCliOutput({ stdout: styledStdout, stderr }).header( + 'Some title', + 'some details', + ); + + expect(textOf(plainStdout)).toBe( + `${S_BAR_START} Some title some details\n`, + ); + expect(textOf(styledStdout)).toContain( + `${styleText(['inverse', 'cyan'], ' Some title ', { validateStream: false })} ${styleText('dim', 'some details', { validateStream: false })}`, + ); + expect(stderr.data()).toStrictEqual([]); + }); + + it('done writes a completed step', () => { + const stdout = new MockWritable(); + const stderr = new MockWritable(); + + createCliOutput({ stdout, stderr }).done('Did something'); + + expect(textOf(stdout)).toBe(logLine(S_STEP_SUBMIT, 'Did something')); + expect(stderr.data()).toStrictEqual([]); + }); + + it('summary writes the note and the outro', () => { + const plainStdout = new MockWritable(); + const styledStdout = buildTerminalStream(); + const stderr = new MockWritable(); + + createCliOutput({ stdout: plainStdout, stderr }).summary( + 'Some title', + ['Line 1', '', 'Line 2'], + 'All done', + ); + createCliOutput({ stdout: styledStdout, stderr }).summary( + 'Some title', + ['Line 1'], + 'All done', + ); + + const text = textOf(plainStdout); + expect(text).toContain(`${S_STEP_SUBMIT} Some title `); + expect(text).toContain(`${S_BAR} Line 1 `); + expect(text).toContain(`${S_BAR} Line 2 `); + expect(text.endsWith(`${S_BAR}\n${S_BAR_END} All done\n\n`)).toBe(true); + expect(textOf(styledStdout)).toContain( + styleText('green', 'All done', { validateStream: false }), + ); + expect(stderr.data()).toStrictEqual([]); + }); + + it('step ends with the result text', 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 }); + const options = { result: (value: number): string => `Did ${value}` }; + + for (const output of [animated, plain]) { + expect(await output.step('Doing', async () => 42, options)).toBe(42); + await expect( + output.step( + 'Failing', + async () => { + throw new Error('oops'); + }, + options, + ), + ).rejects.toThrow('oops'); + } + + expect(indicators.spinner.stop).toHaveBeenCalledExactlyOnceWith('Did 42'); + expect(indicators.spinner.error).toHaveBeenCalledExactlyOnceWith( + 'Failing', + ); + expect(textOf(stdout)).toBe(logLine(S_STEP_SUBMIT, 'Did 42')); + expect(textOf(plainStderr)).toBe(logLine(S_ERROR, 'Failing')); + }); + + it('task ends with the result text', async () => { + const animated = createCliOutput({ + stdout: new MockWritable(), + stderr: buildTerminalStream(), + }); + const stdout = new MockWritable(); + const plain = createCliOutput({ stdout, stderr: new MockWritable() }); + const options = { result: (value: number): string => `Ran ${value}` }; + + for (const output of [animated, plain]) { + expect(await output.task('Running', async () => 42, options)).toBe(42); + } + + expect(indicators.taskLog.success).toHaveBeenCalledExactlyOnceWith( + 'Ran 42', + ); + expect(textOf(stdout)).toBe(logLine(S_STEP_SUBMIT, 'Ran 42')); + }); + it('formats names and dim text for stderr only when stderr is styled', () => { const styled = createCliOutput({ stdout: new MockWritable(), diff --git a/packages/create-release-branch/src/core/cli-output.ts b/packages/create-release-branch/src/core/cli-output.ts index a64c2d7..2db3bab 100644 --- a/packages/create-release-branch/src/core/cli-output.ts +++ b/packages/create-release-branch/src/core/cli-output.ts @@ -1,4 +1,12 @@ -import { cancel, log, spinner, taskLog } from '@clack/prompts'; +import { + cancel, + intro, + log, + note, + outro, + spinner, + taskLog, +} from '@clack/prompts'; import { Writable } from 'node:stream'; import { stripVTControlCharacters, styleText } from 'node:util'; @@ -8,9 +16,15 @@ import { stripVTControlCharacters, styleText } from 'node:util'; * Each message is laid out by clack, which adds its own spacing, so any * trailing newlines in the text are dropped. * + * @property header - Writes the first line of the run, with a title and + * details, to standard out. * @property info - Writes an informational message to standard out. * @property success - Writes a message about a completed action to standard * out. + * @property done - Writes a completed step that has no spinner to standard + * out. + * @property summary - Writes a box with a title and lines, then the last line + * of the run, to standard out. * @property warn - Writes a warning to standard error. * @property warnList - Writes a warning to standard error, with each item, such * as a package name, on its own line below it. @@ -19,26 +33,45 @@ import { stripVTControlCharacters, styleText } from 'node:util'; * details, and ends the output, to standard error. * @property format - Styles parts of the text for standard error: `name` for * names, such as package names, and `dim` for secondary text. - * @property step - Runs a long-running action and returns its result. + * @property step - Runs a long-running action and returns its result. The + * step ends with the text from `options.result`, or else with the label. * @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. + * the action runs, or if it fails. The task ends like a step. */ export type CliOutput = { + header(title: string, details: string): void; info(text: string): void; success(text: string): void; + done(text: string): void; + summary(title: string, lines: string[], outro: string): void; warn(text: string): void; warnList(title: string, items: string[]): void; error(text: string): void; failure(title: string, lines: string[]): void; format: { name(text: string): string; dim(text: string): string }; - step(label: string, run: () => Promise): Promise; + step( + label: string, + run: () => Promise, + options?: StepOptions, + ): Promise; task( label: string, run: (log: (line: string) => void) => Promise, + options?: StepOptions, ): Promise; }; +/** + * Options for a step or a task. + * + * @property result - Builds the text that the step ends with when it succeeds, + * from the result of the action. + */ +export type StepOptions = { + result?: (value: Result) => string; +}; + /** * A stream that the output can write to, such as `process.stdout`. */ @@ -48,6 +81,11 @@ export type OutputStream = Writable & { hasColors?: () => boolean; }; +/** + * A style, or a list of styles, that `styleText` accepts. + */ +type Style = Parameters[0]; + /** * A key that `readline` reports in a `keypress` event. */ @@ -159,7 +197,8 @@ function trimTrailingNewlines(text: string): string { /** * Creates a `CliOutput` that writes to the given streams. * - * Messages, and lines for completed steps and tasks, go to standard out. + * The header, messages, lines for completed steps and tasks, and the summary + * 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 @@ -194,6 +233,8 @@ export function createCliOutput({ }): CliOutput { const toStdout = { output: styled.stdout ? stdout : withoutColors(stdout) }; const toStderr = { output: styled.stderr ? stderr : withoutColors(stderr) }; + const styleStdout = (style: Style, text: string): string => + styled.stdout ? styleText(style, text, { stream: stdout }) : text; const format = { name: (text: string): string => styled.stderr ? styleText('cyan', text, { stream: stderr }) : text, @@ -202,12 +243,27 @@ export function createCliOutput({ }; return { + header(title, details) { + intro( + styled.stdout + ? `${styleStdout(['inverse', 'cyan'], ` ${title} `)} ${styleStdout('dim', details)}` + : `${title} ${details}`, + toStdout, + ); + }, info(text) { log.info(trimTrailingNewlines(text), toStdout); }, success(text) { log.success(trimTrailingNewlines(text), toStdout); }, + done(text) { + log.step(text, toStdout); + }, + summary(title, lines, outroText) { + note(lines.join('\n'), title, toStdout); + outro(styleStdout('green', outroText), toStdout); + }, warn(text) { log.warn(trimTrailingNewlines(text), toStderr); }, @@ -234,11 +290,11 @@ export function createCliOutput({ cancel('The release did not finish.', toStderr); }, format, - async step(label, run) { + async step(label, run, options) { if (!animate) { try { const result = await run(); - log.step(label, toStdout); + log.step(options?.result?.(result) ?? label, toStdout); return result; } catch (error) { log.error(label, toStderr); @@ -252,7 +308,7 @@ export function createCliOutput({ indicator.start(label); try { const result = await run(); - indicator.stop(label); + indicator.stop(options?.result?.(result) ?? label); return result; } catch (error) { indicator.error(label); @@ -260,14 +316,14 @@ export function createCliOutput({ } }); }, - async task(label, run) { + async task(label, run, options) { if (!animate) { const lines: string[] = []; try { const result = await run((line) => { lines.push(line); }); - log.step(label, toStdout); + log.step(options?.result?.(result) ?? label, toStdout); return result; } catch (error) { log.error(label, toStderr); @@ -290,7 +346,7 @@ export function createCliOutput({ const result = await run((line) => { taskOutput.message(line); }); - taskOutput.success(label); + taskOutput.success(options?.result?.(result) ?? label); return result; } catch (error) { taskOutput.error(label, { showLog: true }); 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 e1d1aa2..72625bd 100644 --- a/packages/create-release-branch/src/core/release-steps.test.ts +++ b/packages/create-release-branch/src/core/release-steps.test.ts @@ -1,3 +1,6 @@ +import { S_STEP_SUBMIT } from '@clack/prompts'; +import { stripVTControlCharacters } from 'node:util'; +import { MockWritable } from 'stdio-mock'; import { describe, expect, it, vi } from 'vitest'; import { @@ -7,6 +10,7 @@ import { createNoopCliOutput, } from '../../tests/unit/helpers.js'; import type { Candidate } from './candidates.js'; +import { createCliOutput } from './cli-output.js'; import type { CliOutput } from './cli-output.js'; import type { ModePlan } from './mode.js'; import * as releaseBranchModule from './release-branch.js'; @@ -51,15 +55,64 @@ function buildRecordingOutput(calls: string[]): CliOutput { warnList(title, items) { calls.push(`warnList ${title} ${items.join(', ')}`); }, + done(text) { + calls.push(`done ${text}`); + }, }; } +/** + * Builds an output that writes plain text to a mock standard out. + * + * @returns The output, and the lines of the completed steps that it wrote. + */ +function buildOutputWithStepLines(): { + output: CliOutput; + stdout: MockWritable; + stepLines: () => string[]; +} { + const stdout = new MockWritable(); + const output = createCliOutput({ stdout, stderr: new MockWritable() }); + const stepLines = (): string[] => + stripVTControlCharacters(stdout.data().join('')) + .split('\n') + .filter((line) => line.startsWith(`${S_STEP_SUBMIT} `)) + .map((line) => line.slice(`${S_STEP_SUBMIT} `.length)); + return { output, stdout, stepLines }; +} + const notices = [ { title: 'Title 1:', items: ['a', 'b'] }, { title: 'Title 2:', items: ['c'] }, ]; describe('release-steps', () => { + it('prints the commit lines', async () => { + const { output, stepLines } = buildOutputWithStepLines(); + vi.spyOn(releaseBranchModule, 'determineReleaseVersion').mockReturnValue( + '2.0.0', + ); + vi.spyOn(releaseBranchModule, 'ensureReleaseBranch').mockResolvedValue({ + firstRun: true, + }); + + await startRelease({ + project: buildMockProject(), + releaseType: 'ordinary', + mode: buildMockMode(), + output, + }); + await finishRelease({ + project: buildMockProject(), + newReleaseVersion: '2.0.0', + releases: [], + output, + }); + + expect(stepLines()).toContain('Committed "Initialize Release 2.0.0"'); + expect(stepLines()).toContain('Committed "Update Release 2.0.0"'); + }); + describe('startRelease', () => { it('prepares and commits only on the first run', async () => { const project = buildMockProject(); @@ -149,9 +202,55 @@ describe('release-steps', () => { 'prepare', 'end Updating changelogs', 'commitAllChanges /path/to/project Initialize Release 2.0.0', + 'done Committed "Initialize Release 2.0.0"', ]); }); + it.each([ + { firstRun: true, result: 'Created branch release/2.0.0' }, + { firstRun: false, result: 'Switched to branch release/2.0.0' }, + ])( + 'ends the "Creating the release branch" step with "$result"', + async ({ firstRun, result }) => { + const { output, stepLines } = buildOutputWithStepLines(); + vi.spyOn( + releaseBranchModule, + 'determineReleaseVersion', + ).mockReturnValue('2.0.0'); + vi.spyOn(releaseBranchModule, 'ensureReleaseBranch').mockResolvedValue({ + firstRun, + }); + + await startRelease({ + project: buildMockProject(), + releaseType: 'ordinary', + mode: buildMockMode(), + output, + }); + + expect(stepLines()[0]).toBe(result); + }, + ); + + it('ends the "Updating changelogs" step with "Updated changelogs"', async () => { + const { output, stepLines } = buildOutputWithStepLines(); + vi.spyOn(releaseBranchModule, 'determineReleaseVersion').mockReturnValue( + '2.0.0', + ); + vi.spyOn(releaseBranchModule, 'ensureReleaseBranch').mockResolvedValue({ + firstRun: true, + }); + + await startRelease({ + project: buildMockProject(), + releaseType: 'ordinary', + mode: buildMockMode(), + output, + }); + + expect(stepLines()[1]).toBe('Updated changelogs'); + }); + it('prints the notices from prepare after the step', async () => { const calls: string[] = []; vi.spyOn(releaseBranchModule, 'determineReleaseVersion').mockReturnValue( @@ -181,6 +280,7 @@ describe('release-steps', () => { 'warnList Title 1: a, b', 'warnList Title 2: c', 'commitAllChanges', + 'done Committed "Initialize Release 2.0.0"', ]); }); @@ -369,6 +469,28 @@ describe('release-steps', () => { 'warnList Title 2: c', ]); }); + + it.each([ + { names: ['a', 'b'], result: 'Bumped 2 packages' }, + { names: ['a'], result: 'Bumped 1 package' }, + ])( + 'ends the "Applying the release" step with "$result"', + async ({ names, result }) => { + const { output, stepLines } = buildOutputWithStepLines(); + + await applyRelease({ + mode: buildMockMode(), + plan: { + releases: names.map((name) => ({ name, newVersion: '1.0.0' })), + data: 'data', + }, + newReleaseVersion: '2.0.0', + output, + }); + + expect(stepLines()).toStrictEqual([result]); + }, + ); }); describe('finishRelease', () => { @@ -403,6 +525,7 @@ describe('release-steps', () => { await finishRelease({ project, newReleaseVersion: '2.0.0', + releases: [], output: buildRecordingOutput(calls), }); @@ -420,7 +543,77 @@ describe('release-steps', () => { 'log dedupe output', 'end task Deduplicating dependencies', 'commitAllChanges /path/to/project Update Release 2.0.0', + 'done Committed "Update Release 2.0.0"', ]); }); + + it.each([ + { + index: 0, + label: 'Fixing Yarn constraints', + result: 'Fixed Yarn constraints', + }, + { + index: 1, + label: 'Installing dependencies', + result: 'Installed dependencies', + }, + { + index: 2, + label: 'Deduplicating dependencies', + result: 'Deduplicated dependencies', + }, + ])('ends the "$label" task with "$result"', async ({ index, result }) => { + const { output, stepLines } = buildOutputWithStepLines(); + + await finishRelease({ + project: buildMockProject(), + newReleaseVersion: '2.0.0', + releases: [], + output, + }); + + expect(stepLines()[index]).toBe(result); + }); + + it('prints the summary with every released package', async () => { + const { output, stdout } = buildOutputWithStepLines(); + + await finishRelease({ + project: buildMockProject(), + newReleaseVersion: '2.0.0', + releases: [ + { name: '@metamask/a', newVersion: '1.0.0' }, + { name: '@metamask/longer-name', newVersion: '12.0.0' }, + ], + output, + }); + + const text = stripVTControlCharacters(stdout.data().join('')); + const summary = text.slice(text.indexOf('2 packages released')); + expect(text.indexOf('Next: review the changelogs')).toBeLessThan( + text.indexOf('2 packages released'), + ); + expect(text).toContain( + 'Next: review the changelogs, then push the branch and open the release PR.', + ); + expect(summary).toContain('Branch release/2.0.0'); + expect(summary).toContain('@metamask/a 1.0.0'); + expect(summary).toContain('@metamask/longer-name 12.0.0'); + expect(summary).toContain('Release 2.0.0 is ready on release/2.0.0'); + }); + + it('says "1 package released" for one package', async () => { + const { output, stdout } = buildOutputWithStepLines(); + + await finishRelease({ + project: buildMockProject(), + newReleaseVersion: '2.0.0', + releases: [{ name: 'a', newVersion: '1.0.0' }], + output, + }); + + expect(stdout.data().join('')).toContain('1 package released'); + }); }); }); diff --git a/packages/create-release-branch/src/core/release-steps.ts b/packages/create-release-branch/src/core/release-steps.ts index 512f962..0872022 100644 --- a/packages/create-release-branch/src/core/release-steps.ts +++ b/packages/create-release-branch/src/core/release-steps.ts @@ -32,6 +32,16 @@ function printNotices(output: CliOutput, notices: Notice[]): void { } } +/** + * Describes a number of packages, such as "1 package" or "2 packages". + * + * @param count - The number of packages. + * @returns The description. + */ +function describePackageCount(count: number): string { + return count === 1 ? '1 package' : `${count} packages`; +} + /** * Creates (or switches to) the release branch. On the first run, prepares the * project with the mode, prints the notices of the mode, and commits the @@ -57,24 +67,32 @@ export async function startRelease({ output: CliOutput; }): Promise<{ newReleaseVersion: string }> { const newReleaseVersion = determineReleaseVersion(project, releaseType); + const releaseBranchName = `release/${newReleaseVersion}`; const { firstRun } = await output.step( 'Creating the release branch', async () => await ensureReleaseBranch(project, newReleaseVersion), + { + result: (value) => + value.firstRun + ? `Created branch ${releaseBranchName}` + : `Switched to branch ${releaseBranchName}`, + }, ); if (firstRun) { const notices = await output.step( 'Updating changelogs', async () => await mode.prepare(), + { result: () => 'Updated changelogs' }, ); printNotices(output, notices); + const commitMessage = `Initialize Release ${newReleaseVersion}`; // `prepare` may change nothing, and the release branch still needs this // commit. - await commitAllChanges( - project.directoryPath, - `Initialize Release ${newReleaseVersion}`, - { allowEmpty: true }, - ); + await commitAllChanges(project.directoryPath, commitMessage, { + allowEmpty: true, + }); + output.done(`Committed "${commitMessage}"`); } return { newReleaseVersion }; @@ -163,44 +181,68 @@ export async function applyRelease({ const notices = await output.step( 'Applying the release', async () => await mode.apply(plan, newReleaseVersion), + { result: () => `Bumped ${describePackageCount(plan.releases.length)}` }, ); printNotices(output, notices); } /** - * Fixes constraints, updates the lockfile, deduplicates dependencies, and then - * commits all changes for the release. + * Fixes constraints, updates the lockfile, deduplicates dependencies, commits + * all changes for the release, and then prints a summary of the release. * * @param args - The arguments. * @param args.project - Information about the project. * @param args.newReleaseVersion - The version of the new release. + * @param args.releases - The new version of every released package, from the + * plan. * @param args.output - Where to write user-facing messages. */ export async function finishRelease({ project, newReleaseVersion, + releases, output, }: { project: Project; newReleaseVersion: string; + releases: ModePlan['releases']; output: CliOutput; }): Promise { await output.task( 'Fixing Yarn constraints', async (log) => await fixConstraints(project.directoryPath, log), + { result: () => 'Fixed Yarn constraints' }, ); await output.task( 'Installing dependencies', async (log) => await updateYarnLockfile(project.directoryPath, log), + { result: () => 'Installed dependencies' }, ); await output.task( 'Deduplicating dependencies', async (log) => await deduplicateDependencies(project.directoryPath, log), + { result: () => 'Deduplicated dependencies' }, ); + const commitMessage = `Update Release ${newReleaseVersion}`; // Not run in a step: a spinner would draw over a GPG pinentry in the // terminal. - await commitAllChanges( - project.directoryPath, - `Update Release ${newReleaseVersion}`, + await commitAllChanges(project.directoryPath, commitMessage); + output.done(`Committed "${commitMessage}"`); + + const releaseBranchName = `release/${newReleaseVersion}`; + const nameWidth = Math.max(...releases.map(({ name }) => name.length)); + output.info( + 'Next: review the changelogs, then push the branch and open the release PR.', + ); + output.summary( + `${describePackageCount(releases.length)} released`, + [ + `Branch ${releaseBranchName}`, + '', + ...releases.map( + ({ name, newVersion }) => `${name.padEnd(nameWidth)} ${newVersion}`, + ), + ], + `Release ${newReleaseVersion} is ready on ${releaseBranchName}`, ); } 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 9442d93..3ffcf47 100644 --- a/packages/create-release-branch/src/core/run-release.test.ts +++ b/packages/create-release-branch/src/core/run-release.test.ts @@ -1,4 +1,4 @@ -import { S_BAR, S_WARN } from '@clack/prompts'; +import { S_BAR, S_BAR_START, S_WARN } from '@clack/prompts'; import { MockWritable } from 'stdio-mock'; import { describe, expect, it, vi } from 'vitest'; @@ -56,18 +56,28 @@ function buildContext({ describe('run-release', () => { describe('runRelease', () => { it('runs the front end for a monorepo', async () => { - const stdout = new MockWritable(); - const context = buildContext({ isMonorepo: true, stdout }); + const context = buildContext({ + isMonorepo: true, + stdout: new MockWritable(), + }); const run = vi.fn(); await runRelease(context, { run }); - expect(stdout.data().join('')).toContain( - 'Project appears to have workspaces. Following monorepo workflow.', - ); expect(run).toHaveBeenCalledWith(context); }); + it('prints the header with the project name, the shape, and the mode', async () => { + const stdout = new MockWritable(); + const context = buildContext({ isMonorepo: true, stdout }); + + await runRelease(context, { run: vi.fn() }); + + expect(stdout.data().join('')).toBe( + `${S_BAR_START} create-release-branch root · monorepo · legacy mode\n`, + ); + }); + it.each([ { packagesUsingRootTag: ['a', 'b'], diff --git a/packages/create-release-branch/src/core/run-release.ts b/packages/create-release-branch/src/core/run-release.ts index fee5c56..bad5af7 100644 --- a/packages/create-release-branch/src/core/run-release.ts +++ b/packages/create-release-branch/src/core/run-release.ts @@ -11,11 +11,12 @@ export async function runRelease( context: ReleaseContext, frontEnd: FrontEnd, ): Promise { - const { project, output } = context; + const { project, mode, output } = context; if (project.isMonorepo) { - output.info( - 'Project appears to have workspaces. Following monorepo workflow.\n', + output.header( + 'create-release-branch', + `${project.rootPackage.validatedManifest.name} · monorepo · ${mode.name} mode`, ); const { packagesUsingRootTag } = project; @@ -32,7 +33,7 @@ export async function runRelease( await frontEnd.run(context); } else { output.info( - 'Project does not appear to have any workspaces. Following polyrepo workflow.\n', + 'Project does not appear to have any workspaces. Following polyrepo workflow.', ); } } diff --git a/packages/create-release-branch/src/core/user-facing-error.test.ts b/packages/create-release-branch/src/core/user-facing-error.test.ts index cb0e185..12bbed5 100644 --- a/packages/create-release-branch/src/core/user-facing-error.test.ts +++ b/packages/create-release-branch/src/core/user-facing-error.test.ts @@ -1,7 +1,11 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import type { CliOutput } from './cli-output.js'; -import { describeError, UserFacingError } from './user-facing-error.js'; +import { + describeError, + UserFacingError, + writeError, +} from './user-facing-error.js'; const format: CliOutput['format'] = { name: (text) => `<${text}>`, @@ -80,4 +84,14 @@ describe('user-facing-error', () => { }); }); }); + + describe('writeError', () => { + it('writes the title and the lines of the description as one error', () => { + const output = { error: vi.fn(), format }; + + writeError(output, new TestError('Some message'), { debug: false }); + + expect(output.error).toHaveBeenCalledExactlyOnceWith('Test title\n'); + }); + }); }); diff --git a/packages/create-release-branch/src/core/user-facing-error.ts b/packages/create-release-branch/src/core/user-facing-error.ts index 4892260..b6ab6bd 100644 --- a/packages/create-release-branch/src/core/user-facing-error.ts +++ b/packages/create-release-branch/src/core/user-facing-error.ts @@ -63,3 +63,21 @@ export function describeError( return { title, lines }; } + +/** + * Describes an error to the user, then writes it as one error message: the + * title, then the lines of details. + * + * @param output - Where to write the error. + * @param error - The error. + * @param options - The options. + * @param options.debug - Whether to include the stack trace. + */ +export function writeError( + output: Pick, + error: unknown, + options: { debug: boolean }, +): void { + const { title, lines } = describeError(error, output.format, options); + output.error([title, ...lines].join('\n')); +} 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 827934f..a9f3df3 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 @@ -1,3 +1,4 @@ +import { S_BAR, S_STEP_SUBMIT } from '@clack/prompts'; import fs from 'fs'; import path from 'path'; import { MockWritable } from 'stdio-mock'; @@ -633,6 +634,52 @@ describe('editor-front-end', () => { }); }); + it('prints the waiting and closed lines around the editor', async () => { + await withSandbox(async (sandbox) => { + const { + project, + stdout, + output, + waitForUserToEditReleaseSpecificationSpy, + formatter, + } = await setupFollowMonorepoWorkflow({ + sandbox, + doesReleaseSpecFileExist: false, + isEditorAvailable: true, + }); + let stdoutWhileEditing = ''; + waitForUserToEditReleaseSpecificationSpy.mockImplementation( + async () => { + stdoutWhileEditing = stdout.data().join(''); + }, + ); + + await followMonorepoWorkflow({ + project, + tempDirectoryPath: sandbox.directoryPath, + firstRemovingExistingReleaseSpecification: false, + releaseType: 'ordinary', + defaultBranch: 'main', + formatter, + output, + }); + + const text = stdout.data().join(''); + expect( + stdoutWhileEditing.endsWith( + 'Waiting for your editor to close RELEASE_SPEC.yml\n', + ), + ).toBe(true); + expect( + text + .slice(stdoutWhileEditing.length) + .startsWith( + `${S_BAR}\n${S_STEP_SUBMIT} Closed RELEASE_SPEC.yml\n`, + ), + ).toBe(true); + }); + }); + it('attempts to execute the release spec if it was successfully edited', async () => { await withSandbox(async (sandbox) => { const { diff --git a/packages/create-release-branch/src/frontends/editor/editor-front-end.ts b/packages/create-release-branch/src/frontends/editor/editor-front-end.ts index e3a78ca..128e95d 100644 --- a/packages/create-release-branch/src/frontends/editor/editor-front-end.ts +++ b/packages/create-release-branch/src/frontends/editor/editor-front-end.ts @@ -101,7 +101,7 @@ async function followEditorWorkflow({ if (!reset && (await fileExists(releaseSpecificationPath))) { output.info( - 'Release spec already exists. Picking back up from previous run.\n', + 'Release spec already exists. Picking back up from previous run.', ); } else { const editor = await determineEditor(); @@ -117,14 +117,15 @@ async function followEditorWorkflow({ if (!editor) { output.info( - `${[ + [ 'A template has been generated that specifies this release. Please open the following file in your editor of choice, then re-run this tool:', `${releaseSpecificationPath}`, - ].join('\n\n')}\n`, + ].join('\n\n'), ); return; } + output.info('Waiting for your editor to close RELEASE_SPEC.yml'); try { await waitForUserToEditReleaseSpecification( releaseSpecificationPath, @@ -134,6 +135,7 @@ async function followEditorWorkflow({ await removeFile(releaseSpecificationPath); throw error; } + output.done('Closed RELEASE_SPEC.yml'); } const plan = await validateReleaseSpecification( @@ -143,5 +145,10 @@ async function followEditorWorkflow({ await applyRelease({ mode, plan, newReleaseVersion, output }); await removeFile(releaseSpecificationPath); - await finishRelease({ project, newReleaseVersion, output }); + await finishRelease({ + project, + newReleaseVersion, + releases: plan.releases, + output, + }); } 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 9f166b9..2efead9 100644 --- a/packages/create-release-branch/src/frontends/ui/server.test.ts +++ b/packages/create-release-branch/src/frontends/ui/server.test.ts @@ -1,3 +1,4 @@ +import { S_BAR, S_ERROR, S_INFO, S_WARN } from '@clack/prompts'; import { ChildProcess } from 'child_process'; import { once } from 'events'; import net from 'net'; @@ -233,7 +234,11 @@ describe('server', () => { it('returns 400 on error', async () => { const stderr = new MockWritable(); const mode = buildMockMode({ - listCandidates: vi.fn().mockRejectedValue(new Error('boom')), + listCandidates: vi + .fn() + .mockRejectedValue( + new Error('boom', { cause: new Error('some cause') }), + ), }); const context = buildContext(mode, { output: createCliOutput({ stdout: new MockWritable(), stderr }), @@ -249,7 +254,9 @@ describe('server', () => { }, ); - expect(stderr.data().join('')).toContain('Release error: Error: boom'); + expect(stderr.data().join('')).toBe( + `${S_BAR}\n${S_ERROR} boom\n${S_BAR} Caused by: some cause\n`, + ); }); }); @@ -432,7 +439,7 @@ describe('server', () => { }, ); - expect(stderr.data().join('')).toContain('Release error: Error: boom'); + expect(stderr.data().join('')).toBe(`${S_BAR}\n${S_ERROR} boom\n`); expect(closeServer).not.toHaveBeenCalled(); }); }); @@ -477,9 +484,7 @@ describe('server', () => { }, ); - expect(stderr.data().join('')).toContain( - 'Changelog error: Error: boom', - ); + expect(stderr.data().join('')).toBe(`${S_BAR}\n${S_ERROR} boom\n`); }); }); }); @@ -504,9 +509,7 @@ describe('server', () => { const running = createUIFrontEnd({ port }).run(context); await vi.waitFor(() => { - expect(stdout.data().join('')).toContain( - `UI server running at http://localhost:${port}`, - ); + expect(openBrowser).toHaveBeenCalled(); }); const response = await postChoices( `http://localhost:${port}/api/release`, @@ -516,12 +519,41 @@ describe('server', () => { expect(await response.json()).toStrictEqual({ status: 'success' }); expect(openBrowser).toHaveBeenCalledWith(`http://localhost:${port}`); - const text = stdout.data().join(''); - expect(text).toContain('Attempting to open UI in browser...'); - expect(text).toContain('Release process completed. Server shutdown.'); + expect(stdout.data().join('')).toContain( + 'Release 2.0.0 is ready on release/2.0.0', + ); }); - it('asks to open the URL manually if the browser does not open', async () => { + it('prints the UI address', async () => { + const port = await findFreePort(); + const stdout = new MockWritable(); + const mode = buildMockMode({ + plan: vi.fn().mockResolvedValue({ plan: { releases: [], data: 'x' } }), + }); + const context = buildContext(mode, { + output: createCliOutput({ stdout, stderr: new MockWritable() }), + }); + vi.spyOn(releaseBranchModule, 'determineReleaseVersion').mockReturnValue( + '2.0.0', + ); + vi.spyOn(releaseBranchModule, 'ensureReleaseBranch').mockResolvedValue({ + firstRun: false, + }); + vi.mocked(openBrowser).mockResolvedValue(new ChildProcess()); + + const running = createUIFrontEnd({ port }).run(context); + await vi.waitFor(() => { + expect(openBrowser).toHaveBeenCalled(); + }); + await postChoices(`http://localhost:${port}/api/release`, {}); + await running; + + expect(stdout.data().join('')).toContain( + `${S_BAR}\n${S_INFO} Opening the release UI at http://localhost:${port}\n`, + ); + }); + + it('warns when the browser cannot be opened', async () => { const port = await findFreePort(); const stderr = new MockWritable(); const mode = buildMockMode({ @@ -540,17 +572,14 @@ describe('server', () => { const running = createUIFrontEnd({ port }).run(context); await vi.waitFor(() => { - expect(stderr.data()).toHaveLength(5); + expect(stderr.data()).toHaveLength(1); }); await postChoices(`http://localhost:${port}/api/release`, {}); await running; - const text = stderr.data().join(''); - expect(text).toContain( - 'Error automatically opening browser: Error: no browser', + expect(stderr.data().join('')).toBe( + `${S_BAR}\n${S_WARN} Could not open your browser (Error: no browser). Open http://localhost:${port} yourself.\n`, ); - expect(text).toContain('Please open the following URL manually:'); - expect(text).toContain(`http://localhost:${port}`); }); it('rejects if the server cannot start', async () => { @@ -570,15 +599,16 @@ describe('server', () => { try { await expect(createUIFrontEnd({ port }).run(context)).rejects.toThrow( - 'EADDRINUSE', + expect.objectContaining({ + message: `Could not start the release UI on port ${port}`, + cause: expect.objectContaining({ code: 'EADDRINUSE' }) as Error, + }), ); } finally { blocker.close(); } - expect(stderr.data().join('')).toMatch( - /Failed to start server: .*EADDRINUSE/u, - ); + expect(stderr.data()).toStrictEqual([]); }); }); }); diff --git a/packages/create-release-branch/src/frontends/ui/server.ts b/packages/create-release-branch/src/frontends/ui/server.ts index 1dabd99..62ac5b8 100644 --- a/packages/create-release-branch/src/frontends/ui/server.ts +++ b/packages/create-release-branch/src/frontends/ui/server.ts @@ -10,7 +10,9 @@ import { finishRelease, startRelease, } from '../../core/release-steps.js'; +import { writeError } from '../../core/user-facing-error.js'; import { getCurrentDirectoryPath } from '../../dirname.js'; +import { debug, wrapError } from '../../utils/misc-utils.js'; const UI_BUILD_DIR = join(getCurrentDirectoryPath(), 'ui'); @@ -55,31 +57,25 @@ async function startUI({ closeServer: () => { // eslint-disable-next-line @typescript-eslint/no-use-before-define server.close(); - output.success('Release process completed. Server shutdown.\n'); }, }); const server = app.listen(port, () => { const url = `http://localhost:${port}`; - output.info(`\nAttempting to open UI in browser...`); - openBrowser(url) - .then(() => { - output.info(`\nUI server running at ${url}\n`); - }) - .catch((error: unknown) => { - output.error(`\n---------------------------------------------------\n`); - output.error(`Error automatically opening browser: ${String(error)}\n`); - output.error(`Please open the following URL manually:\n`); - output.error(`${url}\n`); - output.error(`---------------------------------------------------\n\n`); - }); + output.info(`Opening the release UI at ${url}`); + openBrowser(url).catch((error: unknown) => { + output.warn( + `Could not open your browser (${String(error)}). Open ${url} yourself.`, + ); + }); }); return new Promise((resolve, reject) => { server.on('error', (error: Error) => { - output.error(`Failed to start server: ${error.message}\n`); - reject(error); + reject( + wrapError(`Could not start the release UI on port ${port}`, error), + ); }); server.on('close', () => { @@ -145,7 +141,7 @@ export function createApp({ res.send(changelogContent); } catch (error) { - output.error(`Changelog error: ${String(error)}\n`); + writeError(output, error, { debug: debug.enabled }); res.status(500).send('Internal Server Error'); } }); @@ -172,7 +168,7 @@ export function createApp({ res.json({ status: 'success' }); } catch (error) { - output.error(`Release error: ${String(error)}\n`); + writeError(output, error, { debug: debug.enabled }); res.status(400).send('Invalid request'); } }, @@ -215,13 +211,18 @@ export function createApp({ newReleaseVersion, output, }); - await finishRelease({ project, newReleaseVersion, output }); + await finishRelease({ + project, + newReleaseVersion, + releases: result.plan.releases, + output, + }); res.json({ status: 'success' }); closeServer(); } catch (error) { - output.error(`Release error: ${String(error)}\n`); + writeError(output, error, { debug: debug.enabled }); res.status(400).send('Invalid request'); } },