From c61dded672148506e6038db9b56547266c7f7ac6 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 17:52:10 +0100 Subject: [PATCH 1/5] feat(create-release-branch): show actionable errors --- packages/create-release-branch/CHANGELOG.md | 1 + packages/create-release-branch/src/cli.ts | 15 +- .../src/core/cli-output.test.ts | 40 +- .../src/core/cli-output.ts | 19 +- .../src/core/release-rules.test.ts | 141 ++++- .../src/core/release-rules.ts | 99 +-- .../src/core/release-steps.test.ts | 7 +- .../src/core/user-facing-error.test.ts | 72 +++ .../src/core/user-facing-error.ts | 63 ++ .../frontends/editor/editor-front-end.test.ts | 4 +- .../editor/release-specification.test.ts | 568 ++++++++---------- .../frontends/editor/release-specification.ts | 166 +++-- .../src/frontends/ui/server.test.ts | 42 +- .../src/frontends/ui/server.ts | 15 +- 14 files changed, 790 insertions(+), 462 deletions(-) create mode 100644 packages/create-release-branch/src/core/user-facing-error.test.ts create mode 100644 packages/create-release-branch/src/core/user-facing-error.ts diff --git a/packages/create-release-branch/CHANGELOG.md b/packages/create-release-branch/CHANGELOG.md index ad4e2d3..768ca11 100644 --- a/packages/create-release-branch/CHANGELOG.md +++ b/packages/create-release-branch/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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)) +- 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 ([#](https://github.com/MetaMask/release-tools/pull/)) ### Fixed diff --git a/packages/create-release-branch/src/cli.ts b/packages/create-release-branch/src/cli.ts index 41dd9b9..e1dfdb3 100644 --- a/packages/create-release-branch/src/cli.ts +++ b/packages/create-release-branch/src/cli.ts @@ -1,7 +1,7 @@ -import util from 'util'; - import { main } from './cli/main.js'; import { createCliOutput } from './core/cli-output.js'; +import { describeError } from './core/user-facing-error.js'; +import { debug } from './utils/misc-utils.js'; /** * The entrypoint to this tool. @@ -16,8 +16,13 @@ async function cli(): Promise { } cli().catch((error) => { - createCliOutput({ stdout: process.stdout, stderr: process.stderr }).error( - `${util.inspect(error)}\n`, - ); + const output = createCliOutput({ + stdout: process.stdout, + stderr: process.stderr, + }); + const { title, lines } = describeError(error, output.format, { + debug: debug.enabled, + }); + output.failure(title, lines); process.exitCode = 1; }); 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 fcc1af9..db0eaa0 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/packages/create-release-branch/src/core/cli-output.test.ts @@ -1,5 +1,6 @@ import { S_BAR, + S_BAR_END, S_ERROR, S_INFO, S_STEP_SUBMIT, @@ -8,7 +9,7 @@ import { spinner, taskLog, } from '@clack/prompts'; -import { stripVTControlCharacters } from 'node:util'; +import { stripVTControlCharacters, styleText } from 'node:util'; import { MockWritable } from 'stdio-mock'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -408,6 +409,43 @@ describe('cli-output', () => { expect(taskLog).not.toHaveBeenCalled(); }); + it('failure writes the title, the lines, and the cancel line to stderr', () => { + const stdout = new MockWritable(); + const stderr = new MockWritable(); + const output = createCliOutput({ stdout, stderr }); + + output.failure('Something failed', ['Line 1', '', 'Line 2']); + + expect(textOf(stderr)).toBe( + [ + logLine(S_ERROR, 'Something failed'), + `${S_BAR}\n${S_BAR} Line 1\n${S_BAR}\n${S_BAR} Line 2\n`, + `${S_BAR_END} No release was created.\n\n`, + ].join(''), + ); + expect(stdout.data()).toStrictEqual([]); + }); + + it('formats names and dim text for stderr only when stderr is styled', () => { + const styled = createCliOutput({ + stdout: new MockWritable(), + stderr: buildTerminalStream(), + }); + const plain = createCliOutput({ + stdout: buildTerminalStream(), + stderr: new MockWritable(), + }); + + expect(styled.format.name('a')).toBe( + styleText('cyan', 'a', { validateStream: false }), + ); + expect(styled.format.dim('a')).toBe( + styleText('dim', 'a', { validateStream: false }), + ); + expect(plain.format.name('a')).toBe('a'); + expect(plain.format.dim('a')).toBe('a'); + }); + it.each([ { keyName: 'Ctrl+C', input: '\u0003', key: { ctrl: true, name: 'c' } }, { diff --git a/packages/create-release-branch/src/core/cli-output.ts b/packages/create-release-branch/src/core/cli-output.ts index fb2aeaa..337a049 100644 --- a/packages/create-release-branch/src/core/cli-output.ts +++ b/packages/create-release-branch/src/core/cli-output.ts @@ -1,4 +1,4 @@ -import { log, spinner, taskLog } from '@clack/prompts'; +import { cancel, log, spinner, taskLog } from '@clack/prompts'; import { Writable } from 'node:stream'; import { stripVTControlCharacters, styleText } from 'node:util'; @@ -13,6 +13,10 @@ import { stripVTControlCharacters, styleText } from 'node:util'; * out. * @property warn - Writes a warning to standard error. * @property error - Writes an error to standard error. + * @property failure - Writes the error that stopped the run, with lines of + * 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 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 @@ -23,6 +27,8 @@ export type CliOutput = { success(text: string): void; warn(text: 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; task( label: string, @@ -199,6 +205,17 @@ export function createCliOutput({ error(text) { log.error(trimTrailingNewlines(text), toStderr); }, + failure(title, lines) { + log.error(title, toStderr); + log.message(lines, toStderr); + cancel('No release was created.', toStderr); + }, + format: { + name: (text) => + styled.stderr ? styleText('cyan', text, { stream: stderr }) : text, + dim: (text) => + styled.stderr ? styleText('dim', text, { stream: stderr }) : text, + }, async step(label, run) { if (!animate) { try { diff --git a/packages/create-release-branch/src/core/release-rules.test.ts b/packages/create-release-branch/src/core/release-rules.test.ts index afe3090..10fb1d2 100644 --- a/packages/create-release-branch/src/core/release-rules.test.ts +++ b/packages/create-release-branch/src/core/release-rules.test.ts @@ -52,18 +52,10 @@ describe('release-rules', () => { expect(checkReleaseChoices(project, { a: 'major' })).toStrictEqual({ errors: [ { - message: ` -The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. - - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip -`.trim(), + packageName: 'a', + problem: + 'has a major bump, but its direct dependents are not in the release spec: b', + hint: 'Add them to the release spec, or set them to intentionally-skip.', }, ], dependencyFindings: { @@ -125,18 +117,133 @@ The following direct dependents of package 'a', which is being released with a m checkReleaseChoices(project, { a: 'asdf', b: 'minor' }).errors, ).toStrictEqual([ { - message: [ - '"asdf" is not a valid version specifier for package "a"', - '(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3")', - ], + packageName: 'a', entryIndex: 0, + problem: '"asdf" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', }, { - message: '"b" is not a package in the project', + packageName: 'b', entryIndex: 1, + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', }, ]); }); + + it.each([ + { + rule: 'an unknown package', + workspacePackages: { a: buildMockPackage('a', '1.0.0') }, + choices: { b: 'minor' }, + error: { + packageName: 'b', + entryIndex: 0, + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', + }, + }, + { + rule: 'an invalid version specifier', + workspacePackages: { a: buildMockPackage('a', '1.0.0') }, + choices: { a: 'asdf' }, + error: { + packageName: 'a', + entryIndex: 0, + problem: '"asdf" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + }, + { + rule: 'the current version', + workspacePackages: { a: buildMockPackage('a', '1.0.0') }, + choices: { a: '1.0.0' }, + error: { + packageName: 'a', + entryIndex: 0, + problem: 'is already at version 1.0.0', + hint: 'Use a higher version, or major, minor, or patch.', + }, + }, + { + rule: 'a lower version', + workspacePackages: { a: buildMockPackage('a', '1.0.0') }, + choices: { a: '0.9.0' }, + error: { + packageName: 'a', + entryIndex: 0, + problem: '"0.9.0" is lower than the current version 1.0.0', + hint: 'Use a higher version, or major, minor, or patch.', + }, + }, + { + rule: 'missing direct dependents', + workspacePackages: { + a: buildMockPackage('a', '1.0.0'), + b: buildMockPackage('b', { + validatedManifest: { dependencies: { a: '1.0.0' } }, + }), + c: buildMockPackage('c', { + validatedManifest: { dependencies: { a: '1.0.0' } }, + }), + }, + choices: { a: 'major' }, + error: { + packageName: 'a', + problem: + 'has a major bump, but its direct dependents are not in the release spec: b, c', + hint: 'Add them to the release spec, or set them to intentionally-skip.', + }, + }, + { + rule: 'missing peer dependents', + workspacePackages: { + a: buildMockPackage('a', '1.0.0'), + b: buildMockPackage('b', { + validatedManifest: { peerDependencies: { a: '1.0.0' } }, + }), + c: buildMockPackage('c', { + validatedManifest: { peerDependencies: { a: '1.0.0' } }, + }), + }, + choices: { a: 'major' }, + error: { + packageName: 'a', + problem: + 'has a major bump, but its peer dependents are not in the release spec: b, c', + hint: 'Add them so they stay compatible, or set them to intentionally-skip if they can wait.', + }, + }, + { + rule: 'missing changed dependencies', + workspacePackages: { + a: buildMockPackage('a', { + validatedManifest: { + dependencies: { b: '1.0.0' }, + peerDependencies: { c: '1.0.0' }, + }, + }), + b: buildMockPackage('b', { hasChangesSinceLatestRelease: true }), + c: buildMockPackage('c', { hasChangesSinceLatestRelease: true }), + }, + choices: { a: 'minor' }, + error: { + packageName: 'a', + problem: + 'depends on packages with unreleased changes that are not in the release spec: b, c', + hint: 'Add them to the release spec, or set them to intentionally-skip if they can wait.', + }, + }, + ])( + 'reports $rule with its problem and hint', + ({ workspacePackages, choices, error }) => { + const project = buildMockProject({ workspacePackages }); + + expect(checkReleaseChoices(project, choices).errors).toStrictEqual([ + error, + ]); + }, + ); }); describe('normalizeReleaseChoices', () => { diff --git a/packages/create-release-branch/src/core/release-rules.ts b/packages/create-release-branch/src/core/release-rules.ts index 03f61c4..8b1221f 100644 --- a/packages/create-release-branch/src/core/release-rules.ts +++ b/packages/create-release-branch/src/core/release-rules.ts @@ -1,5 +1,4 @@ import { diff } from 'semver'; -import YAML from 'yaml'; import type { Package } from '../project/package.js'; import type { Project } from '../project/project.js'; @@ -48,13 +47,17 @@ export type DependencyFinding = { /** * A problem with the release choices. * - * @property message - The message, as a single string or as separate lines. + * @property packageName - The name of the package that the problem is about. * @property entryIndex - The index of the choice in `Object.entries(choices)`, * for problems that point at a single entry. + * @property problem - What is wrong, written to follow the package name. + * @property hint - How to fix it. */ export type ReleaseChoiceError = { - message: string | string[]; + packageName?: string; entryIndex?: number; + problem: string; + hint?: string; }; /** @@ -208,29 +211,6 @@ function findCandidateDependencies( return []; } -/** - * Builds the YAML example, indented for an error message, that lists the - * given packages with the "intentionally-skip" directive. - * - * @param packageNames - The names of the packages to list. - * @returns The indented YAML example. - */ -function buildIntentionallySkipExample(packageNames: string[]): string { - return YAML.stringify({ - packages: packageNames.reduce( - (object, packageName) => ({ - ...object, - [packageName]: INTENTIONALLY_SKIP_PACKAGE_DIRECTIVE, - }), - {}, - ), - }) - .trim() - .split('\n') - .map((line) => ` ${line}`) - .join('\n'); -} - /** * Checks the release choices against the project: the names of the packages, * the version specifiers, and the dependencies and dependents that must be @@ -270,8 +250,10 @@ export function checkReleaseChoices( if (changedPackage === undefined) { errors.push({ - message: `${JSON.stringify(changedPackageName)} is not a package in the project`, + packageName: changedPackageName, entryIndex, + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', }); } @@ -282,37 +264,32 @@ export function checkReleaseChoices( !isValidSemver(versionSpecifierOrDirective) ) { errors.push({ - message: [ - `${JSON.stringify(versionSpecifierOrDirective)} is not a valid version specifier for package "${changedPackageName}"`, - `(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3")`, - ], + packageName: changedPackageName, entryIndex, + problem: `"${String(versionSpecifierOrDirective)}" is not a valid version specifier`, + hint: 'Use major, minor, patch, or a version like 1.2.3.', }); } if (isValidSemver(versionSpecifierOrDirective)) { + const currentVersion = changedPackage.validatedManifest.version; const comparison = new SemVer(versionSpecifierOrDirective).compare( - changedPackage.validatedManifest.version, + currentVersion, ); if (comparison === 0) { errors.push({ - message: [ - `${JSON.stringify(versionSpecifierOrDirective)} is not a valid version specifier for package "${changedPackageName}"`, - `("${changedPackageName}" is already at version "${versionSpecifierOrDirective}")`, - ], + packageName: changedPackageName, entryIndex, + problem: `is already at version ${versionSpecifierOrDirective}`, + hint: 'Use a higher version, or major, minor, or patch.', }); } else if (comparison < 0) { errors.push({ - message: [ - `${JSON.stringify(versionSpecifierOrDirective)} is not a valid version specifier for package "${changedPackageName}"`, - `("${changedPackageName}" is at a greater version "${String( - project.workspacePackages[changedPackageName].validatedManifest - .version, - )}")`, - ], + packageName: changedPackageName, entryIndex, + problem: `"${versionSpecifierOrDirective}" is lower than the current version ${String(currentVersion)}`, + hint: 'Use a higher version, or major, minor, or patch.', }); } } @@ -328,15 +305,9 @@ export function checkReleaseChoices( if (missingDirectDependentNames.length > 0) { errors.push({ - message: [ - `The following direct dependents of package '${changedPackageName}', which is being released with a major version bump, are missing from the release spec.`, - missingDirectDependentNames - .map((dependent) => ` - ${dependent}`) - .join('\n'), - ` Consider including them in the release spec so that the dependency tree of consuming projects can be kept small.`, - ` If you do not want to do this, then list it with a directive of "intentionally-skip". For example:`, - buildIntentionallySkipExample(missingDirectDependentNames), - ].join('\n\n'), + packageName: changedPackageName, + problem: `has a major bump, but its direct dependents are not in the release spec: ${missingDirectDependentNames.join(', ')}`, + hint: 'Add them to the release spec, or set them to intentionally-skip.', }); } @@ -351,15 +322,9 @@ export function checkReleaseChoices( if (missingPeerDependentNames.length > 0) { errors.push({ - message: [ - `The following dependents of package '${changedPackageName}', which is being released with a major version bump, are missing from the release spec.`, - missingPeerDependentNames - .map((dependent) => ` - ${dependent}`) - .join('\n'), - ` Consider including them in the release spec so that they are compatible with the new '${changedPackageName}' version.`, - ` If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:`, - buildIntentionallySkipExample(missingPeerDependentNames), - ].join('\n\n'), + packageName: changedPackageName, + problem: `has a major bump, but its peer dependents are not in the release spec: ${missingPeerDependentNames.join(', ')}`, + hint: 'Add them so they stay compatible, or set them to intentionally-skip if they can wait.', }); } @@ -372,15 +337,9 @@ export function checkReleaseChoices( if (missingDependencies.length > 0) { errors.push({ - message: [ - `The following packages, which are dependencies or peer dependencies of the package '${changedPackageName}' being released, are missing from the release spec.`, - missingDependencies - .map((dependency) => ` - ${dependency}`) - .join('\n'), - ` These packages may have changes that '${changedPackageName}' relies upon. Consider including them in the release spec.`, - ` If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:`, - buildIntentionallySkipExample(missingDependencies), - ].join('\n\n'), + packageName: changedPackageName, + problem: `depends on packages with unreleased changes that are not in the release spec: ${missingDependencies.join(', ')}`, + hint: 'Add them to the release spec, or set them to intentionally-skip if they can wait.', }); } 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 a088604..9c6706f 100644 --- a/packages/create-release-branch/src/core/release-steps.test.ts +++ b/packages/create-release-branch/src/core/release-steps.test.ts @@ -185,11 +185,10 @@ describe('release-steps', () => { expect(result).toStrictEqual({ errors: [ { - message: [ - '"foo" is not a valid version specifier for package "a"', - '(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3")', - ], + packageName: 'a', entryIndex: 0, + problem: '"foo" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', }, ], dependencyFindings: {}, 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 new file mode 100644 index 0000000..3ea03b9 --- /dev/null +++ b/packages/create-release-branch/src/core/user-facing-error.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest'; + +import type { CliOutput } from './cli-output.js'; +import { describeError, UserFacingError } from './user-facing-error.js'; + +const format: CliOutput['format'] = { + name: (text) => `<${text}>`, + dim: (text) => `(${text})`, +}; + +class TestError extends UserFacingError { + describe( + errorFormat: CliOutput['format'], + ): ReturnType { + return { title: 'Test title', lines: [errorFormat.name('a')] }; + } +} + +describe('user-facing-error', () => { + describe('describeError', () => { + it('uses the description of a user-facing error', () => { + const error = new TestError('Some message', { + cause: new Error('Some cause'), + }); + + expect(describeError(error, format, { debug: true })).toStrictEqual({ + title: 'Test title', + lines: [''], + }); + }); + + it('prints causes and hides the stack trace', () => { + const error = new Error('Could not read file\nmore details', { + cause: new Error('Permission denied', { cause: 'EACCES' }), + }); + + expect(describeError(error, format, { debug: false })).toStrictEqual({ + title: 'Could not read file', + lines: [ + 'more details', + 'Caused by: Permission denied', + 'Caused by: EACCES', + ], + }); + }); + + it('prints the stack trace when debugging is on', () => { + const error = new Error('Something failed', { + cause: new Error('Some cause'), + }); + error.stack = 'Error: Something failed\n at line 1\n at line 2'; + + expect(describeError(error, format, { debug: true })).toStrictEqual({ + title: 'Something failed', + lines: [ + 'Caused by: Some cause', + '', + 'Error: Something failed', + ' at line 1', + ' at line 2', + ], + }); + }); + + it('describes a value that is not an error', () => { + expect(describeError('oops', format, { debug: true })).toStrictEqual({ + title: 'oops', + lines: [], + }); + }); + }); +}); diff --git a/packages/create-release-branch/src/core/user-facing-error.ts b/packages/create-release-branch/src/core/user-facing-error.ts new file mode 100644 index 0000000..dce1bed --- /dev/null +++ b/packages/create-release-branch/src/core/user-facing-error.ts @@ -0,0 +1,63 @@ +import type { CliOutput } from './cli-output.js'; + +/** + * An error that describes itself to the user, with a title and lines of + * details, instead of being printed with its message, causes, and stack. + */ +export abstract class UserFacingError extends Error { + /** + * Describes the error to the user. + * + * @param format - Styles parts of the text. + * @returns The title and the lines of details. + */ + abstract describe(format: CliOutput['format']): { + title: string; + lines: string[]; + }; +} + +/** + * Gets the message of an error, or the text of a value that is not an error. + * + * @param error - The error. + * @returns The message. + */ +function messageOf(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** + * Describes an error to the user. A `UserFacingError` describes itself. Any + * other error is described by its message, then one `Caused by` line for each + * error in its chain of causes, then its stack trace when debugging. + * + * @param error - The error. + * @param format - Styles parts of the text. + * @param options - The options. + * @param options.debug - Whether to include the stack trace. + * @returns The title and the lines of details. + */ +export function describeError( + error: unknown, + format: CliOutput['format'], + { debug }: { debug: boolean }, +): { title: string; lines: string[] } { + if (error instanceof UserFacingError) { + return error.describe(format); + } + + const [title, ...lines] = messageOf(error).split('\n'); + + let cause = error instanceof Error ? error.cause : undefined; + while (cause !== undefined) { + lines.push(`Caused by: ${messageOf(cause)}`); + cause = cause instanceof Error ? cause.cause : undefined; + } + + if (debug && error instanceof Error && error.stack !== undefined) { + lines.push('', ...error.stack.split('\n')); + } + + return { title, lines }; +} 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 16ec854..bd69df9 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 @@ -1093,8 +1093,10 @@ describe('editor-front-end', () => { { errors: [ { - message: '"foo" is not a package in the project', + packageName: 'foo', entryIndex: 0, + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', }, ], dependencyFindings: {}, diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts index 059597a..a23d019 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts @@ -18,6 +18,7 @@ import type { Project } from '../../project/project.js'; import * as miscUtils from '../../utils/misc-utils.js'; import { generateReleaseSpecificationTemplateForMonorepo, + ReleaseSpecificationError, waitForUserToEditReleaseSpecification, validateReleaseSpecification as validateReleaseSpecificationWithCheck, } from './release-specification.js'; @@ -404,9 +405,10 @@ packages: validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( expect.objectContaining({ - message: expect.stringMatching( - /^Your release spec does not appear to be valid YAML\.\n/u, - ) as string, + problems: [ + { problem: 'is not valid YAML', hint: 'Fix the syntax.' }, + ], + specPath: releaseSpecificationPath, cause: expect.objectContaining({ message: expect.stringMatching( /^Missing closing "quote/u, @@ -432,7 +434,14 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - /^Your release spec could not be processed because it needs to be an object/u, + expect.objectContaining({ + problems: [ + { + problem: + 'needs a packages object, where each key is a package name and each value is a version specifier', + }, + ], + }), ); }); }); @@ -452,7 +461,14 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - /^Your release spec could not be processed because it needs to be an object/u, + expect.objectContaining({ + problems: [ + { + problem: + 'needs a packages object, where each key is a package name and each value is a version specifier', + }, + ], + }), ); }); }); @@ -481,14 +497,22 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - new RegExp( - [ - '^Your release spec could not be processed due to the following issues:\n', - '\\* Line 2: "foo" is not a package in the project', - '\\* Line 3: "bar" is not a package in the project', - ].join('\n'), - 'u', - ), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'foo', + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', + }, + { + lineNumber: 3, + packageName: 'bar', + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', + }, + ], + }), ); }); }); @@ -518,16 +542,22 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - new RegExp( - [ - '^Your release spec could not be processed due to the following issues:\n', - '\\* Line 2: "asdflksdaf" is not a valid version specifier for package "a"', - ' \\(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1\\.2\\.3"\\)', - '\\* Line 3: "1.2\\.\\.\\.3\\." is not a valid version specifier for package "b"', - ' \\(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1\\.2\\.3"\\)', - ].join('\n'), - 'u', - ), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'a', + problem: '"asdflksdaf" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + { + lineNumber: 3, + packageName: 'b', + problem: '"1.2...3." is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + ], + }), ); }); }); @@ -557,14 +587,22 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - ` -Your release spec could not be processed due to the following issues: - -* Line 2: "1.2.3" is not a valid version specifier for package "a" - ("a" is already at version "1.2.3") -* Line 3: "4.5.6" is not a valid version specifier for package "b" - ("b" is already at version "4.5.6") -`.trim(), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'a', + problem: 'is already at version 1.2.3', + hint: 'Use a higher version, or major, minor, or patch.', + }, + { + lineNumber: 3, + packageName: 'b', + problem: 'is already at version 4.5.6', + hint: 'Use a higher version, or major, minor, or patch.', + }, + ], + }), ); }); }); @@ -594,14 +632,22 @@ Your release spec could not be processed due to the following issues: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - ` -Your release spec could not be processed due to the following issues: - -* Line 2: "1.2.2" is not a valid version specifier for package "a" - ("a" is at a greater version "1.2.3") -* Line 3: "4.5.5" is not a valid version specifier for package "b" - ("b" is at a greater version "4.5.6") -`.trim(), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'a', + problem: '"1.2.2" is lower than the current version 1.2.3', + hint: 'Use a higher version, or major, minor, or patch.', + }, + { + lineNumber: 3, + packageName: 'b', + problem: '"4.5.5" is lower than the current version 4.5.6', + hint: 'Use a higher version, or major, minor, or patch.', + }, + ], + }), ); }); }); @@ -640,21 +686,13 @@ Your release spec could not be processed due to the following issues: validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: +The release spec has 1 problem: -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -695,21 +733,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -750,21 +780,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -805,21 +827,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that they are compatible with the new 'a' version. +The release spec has 1 problem: - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -861,21 +875,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that they are compatible with the new 'a' version. +The release spec has 1 problem: - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -917,21 +923,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b +The release spec has 1 problem: - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -973,21 +971,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b +The release spec has 1 problem: - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1029,21 +1019,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. +The release spec has 1 problem: - - b - - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1084,21 +1066,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. - - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip +The release spec has 1 problem: -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1139,21 +1113,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: +The release spec has 1 problem: -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. - - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1194,21 +1160,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. - - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1249,21 +1207,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. - - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1305,21 +1255,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. +The release spec has 1 problem: - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1361,21 +1303,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b - - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. +The release spec has 1 problem: - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1417,21 +1351,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b +The release spec has 1 problem: - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. - - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1473,21 +1399,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b +The release spec has 1 problem: - Consider including them in the release spec so that the dependency tree of consuming projects can be kept small. - - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1708,21 +1626,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. +The release spec has 1 problem: - - b - - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1763,21 +1673,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. - - - b - - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip +The release spec has 1 problem: -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1819,21 +1721,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: +The release spec has 1 problem: -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. - - - b - - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1875,21 +1769,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. - - - b - - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -2076,7 +1962,7 @@ ${releaseSpecificationPath} }); }); - it("keeps today's line numbers for a generated template", async () => { + it("keeps today's line numbers in the new layout", async () => { await withSandbox(async (sandbox) => { const project = buildMockProject({ workspacePackages: { @@ -2100,9 +1986,89 @@ ${releaseSpecificationPath} await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - '* Line 29: "foo" is not a valid version specifier for package "a"', + [ + 'Line 29 a', + ' "foo" is not a valid version specifier', + ' Use major, minor, patch, or a version like 1.2.3.', + ].join('\n'), ); }); }); }); + + describe('ReleaseSpecificationError', () => { + const format = { + name: (text: string): string => `<${text}>`, + dim: (text: string): string => `(${text})`, + }; + + it('describes each problem with its line, package, problem, and hint', () => { + const error = new ReleaseSpecificationError({ + problems: [ + { + lineNumber: 37, + packageName: 'a', + problem: '"foo" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + { + packageName: 'b', + problem: 'has a major bump', + hint: 'Add them.', + }, + { problem: 'is not valid YAML', hint: 'Fix the syntax.' }, + ], + specPath: '/path/to/RELEASE_SPEC.yml', + }); + + expect(error.describe(format)).toStrictEqual({ + title: 'The release spec has 3 problems:', + lines: [ + 'Line 37 ', + ' "foo" is not a valid version specifier', + ' (Use major, minor, patch, or a version like 1.2.3.)', + '', + '', + ' has a major bump', + ' (Add them.)', + '', + 'The release spec', + ' is not valid YAML', + ' (Fix the syntax.)', + '', + 'Fix the release spec and run the tool again. It was kept at:', + '(/path/to/RELEASE_SPEC.yml)', + ], + }); + }); + + it('uses the singular for one problem', () => { + const error = new ReleaseSpecificationError({ + problems: [{ packageName: 'a', problem: 'is not a package' }], + specPath: '/path/to/RELEASE_SPEC.yml', + }); + + expect(error.describe(format)).toStrictEqual({ + title: 'The release spec has 1 problem:', + lines: [ + '', + ' is not a package', + '', + 'Fix the release spec and run the tool again. It was kept at:', + '(/path/to/RELEASE_SPEC.yml)', + ], + }); + expect(error.message).toBe( + [ + 'The release spec has 1 problem:', + '', + 'a', + ' is not a package', + '', + 'Fix the release spec and run the tool again. It was kept at:', + '/path/to/RELEASE_SPEC.yml', + ].join('\n'), + ); + }); + }); }); diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.ts b/packages/create-release-branch/src/frontends/editor/release-specification.ts index fc98976..012d5f1 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.ts @@ -1,12 +1,14 @@ import YAML from 'yaml'; import type { Candidate } from '../../core/candidates.js'; +import type { CliOutput } from '../../core/cli-output.js'; import type { ModeName } from '../../core/mode.js'; import { SKIP_PACKAGE_DIRECTIVE } from '../../core/release-rules.js'; import type { ReleaseChoiceError, ReleaseChoices, } from '../../core/release-rules.js'; +import { UserFacingError } from '../../core/user-facing-error.js'; import { readFile } from '../../utils/fs.js'; import { debug, @@ -75,6 +77,93 @@ const TEMPLATE_TEXT_BY_MODE: Record = { }, }; +/** + * The indentation of the problem and the hint below the line and package of a + * release spec problem. + */ +const PROBLEM_INDENT = ' '.repeat(9); + +/** + * A problem in the release spec. + * + * @property lineNumber - The line of the release spec that has the problem. + * @property packageName - The name of the package that has the problem. + * @property problem - What is wrong, written to follow the package name, or + * "The release spec" if there is no package. + * @property hint - How to fix it. + */ +export type ReleaseSpecificationProblem = { + lineNumber?: number; + packageName?: string; + problem: string; + hint?: string; +}; + +/** + * The problems in a release spec that the user has edited. The release spec + * is kept, so the user can fix it and run the tool again. + */ +export class ReleaseSpecificationError extends UserFacingError { + problems: ReleaseSpecificationProblem[]; + + specPath: string; + + /** + * Creates the error. + * + * @param args - The arguments. + * @param args.problems - The problems in the release spec. + * @param args.specPath - The path to the release spec. + * @param options - The options. + * @param options.cause - The error that caused the problems. + */ + constructor( + { + problems, + specPath, + }: { problems: ReleaseSpecificationProblem[]; specPath: string }, + options?: { cause: unknown }, + ) { + super(undefined, options); + this.problems = problems; + this.specPath = specPath; + const { title, lines } = this.describe({ + name: (text) => text, + dim: (text) => text, + }); + this.message = [title, '', ...lines].join('\n'); + } + + describe(format: CliOutput['format']): { title: string; lines: string[] } { + const count = this.problems.length; + const title = `The release spec has ${count} ${count === 1 ? 'problem' : 'problems'}:`; + const problemLines = this.problems.map( + ({ lineNumber, packageName, problem, hint }) => { + const subject = + packageName === undefined + ? 'The release spec' + : format.name(packageName); + return [ + lineNumber === undefined ? subject : `Line ${lineNumber} ${subject}`, + `${PROBLEM_INDENT}${problem}`, + ...(hint === undefined + ? [] + : [`${PROBLEM_INDENT}${format.dim(hint)}`]), + ]; + }, + ); + + return { + title, + lines: [ + ...problemLines.flatMap((lines) => [...lines, '']), + 'Fix the release spec and run the tool again. It was kept at:', + format.dim(this.specPath), + ], + }; + } +} + /** * Generates a skeleton for a release specification, which describes how a * project should be updated. @@ -156,7 +245,7 @@ export async function waitForUserToEditReleaseSpecification( * @param check - Checks the release choices. It returns the plan only when * there are no errors. * @returns The plan that `check` returns. - * @throws If there are any issues with the file. + * @throws A `ReleaseSpecificationError` if there are any issues with the file. */ export async function validateReleaseSpecification( releaseSpecificationPath: string, @@ -173,22 +262,17 @@ export async function validateReleaseSpecification( packages: ReleaseChoices; }; - const afterwordForAllErrorMessages = [ - "The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool.", - releaseSpecificationPath, - ].join('\n\n'); - try { unvalidatedReleaseSpecification = YAML.parse( releaseSpecificationContents, ) as { packages: ReleaseChoices }; } catch (error) { - throw wrapError( - [ - 'Your release spec does not appear to be valid YAML.', - afterwordForAllErrorMessages, - ].join('\n\n'), - error, + throw new ReleaseSpecificationError( + { + problems: [{ problem: 'is not valid YAML', hint: 'Fix the syntax.' }], + specPath: releaseSpecificationPath, + }, + { cause: error }, ); } @@ -196,54 +280,28 @@ export async function validateReleaseSpecification( !isObject(unvalidatedReleaseSpecification) || unvalidatedReleaseSpecification.packages === undefined ) { - const message = [ - `Your release spec could not be processed because it needs to be an object with a \`packages\` property. The value of \`packages\` must itself be an object, where each key is a workspace package in the project and each value is a version specifier ("major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3").`, - `Here is the parsed version of the file you provided:`, - JSON.stringify(unvalidatedReleaseSpecification, null, 2), - afterwordForAllErrorMessages, - ].join('\n\n'); - throw new Error(message); + throw new ReleaseSpecificationError({ + problems: [ + { + problem: + 'needs a packages object, where each key is a package name and each value is a version specifier', + }, + ], + specPath: releaseSpecificationPath, + }); } const result = await check(unvalidatedReleaseSpecification.packages); if (!('plan' in result)) { - const errors = result.errors.map(({ message, entryIndex }) => ({ - message, - lineNumber: + throw new ReleaseSpecificationError({ + problems: result.errors.map(({ entryIndex, ...problem }) => entryIndex === undefined - ? undefined - : indexOfFirstUsableLine + entryIndex + 2, - })); - const message = [ - 'Your release spec could not be processed due to the following issues:', - errors - .flatMap((error) => { - const itemPrefix = '* '; - - if (error.lineNumber === undefined) { - return `${itemPrefix}${String(error.message)}`; - } - - const lineNumberPrefix = `Line ${error.lineNumber}: `; - - if (Array.isArray(error.message)) { - return [ - `${itemPrefix}${lineNumberPrefix}${error.message[0]}`, - ...error.message.slice(1).map((line) => { - const indentedLineLength = - itemPrefix.length + lineNumberPrefix.length + line.length; - return line.padStart(indentedLineLength, ' '); - }), - ]; - } - - return `${itemPrefix}${lineNumberPrefix}${error.message}`; - }) - .join('\n'), - afterwordForAllErrorMessages, - ].join('\n\n'); - throw new Error(message); + ? problem + : { lineNumber: indexOfFirstUsableLine + entryIndex + 2, ...problem }, + ), + specPath: releaseSpecificationPath, + }); } return result.plan; 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 33f191e..a8e065e 100644 --- a/packages/create-release-branch/src/frontends/ui/server.test.ts +++ b/packages/create-release-branch/src/frontends/ui/server.test.ts @@ -274,12 +274,16 @@ describe('server', () => { status: 'error', errors: [ { - message: expect.stringContaining( - "The following direct dependents of package 'a'", - ) as string, + message: [ + 'a: has a major bump, but its direct dependents are not in the release spec: b', + 'Add them to the release spec, or set them to intentionally-skip.', + ], }, { - message: '"nonexistent" is not a package in the project', + message: [ + 'nonexistent: is not a package in the project', + 'Check the name, or remove the line.', + ], lineNumber: 3, }, ], @@ -291,6 +295,36 @@ describe('server', () => { expect(closeServer).not.toHaveBeenCalled(); }); + it('returns the problem and hint as the message', async () => { + const mode = buildMockMode({ + plan: vi.fn().mockResolvedValue({ + errors: [ + { packageName: 'a', problem: 'is wrong', hint: 'Fix it.' }, + { problem: 'is broken' }, + ], + }), + }); + + await withApp( + { + context: buildContext(mode), + newReleaseVersion: '2.0.0', + closeServer: vi.fn(), + }, + async (url) => { + const response = await postChoices(`${url}/api/release`, {}); + + expect(await response.json()).toStrictEqual({ + status: 'error', + errors: [ + { message: ['a: is wrong', 'Fix it.'] }, + { message: 'is broken' }, + ], + }); + }, + ); + }); + it('applies, finishes, and closes the server', async () => { const plan = { releases: [{ name: 'a', newVersion: '1.1.0' }], diff --git a/packages/create-release-branch/src/frontends/ui/server.ts b/packages/create-release-branch/src/frontends/ui/server.ts index 591d440..1dabd99 100644 --- a/packages/create-release-branch/src/frontends/ui/server.ts +++ b/packages/create-release-branch/src/frontends/ui/server.ts @@ -193,10 +193,17 @@ export function createApp({ if (!('plan' in result)) { res.json({ status: 'error', - errors: result.errors.map(({ message, entryIndex }) => - entryIndex === undefined - ? { message } - : { message, lineNumber: entryIndex + 2 }, + errors: result.errors.map( + ({ packageName, entryIndex, problem, hint }) => { + const text = + packageName === undefined + ? problem + : `${packageName}: ${problem}`; + const message = hint === undefined ? text : [text, hint]; + return entryIndex === undefined + ? { message } + : { message, lineNumber: entryIndex + 2 }; + }, ), }); return; From 758366bcfef28511c582ae06c17be94e280c5db5 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 17:56:39 +0100 Subject: [PATCH 2/5] fix(create-release-branch): address review of the error output --- .../src/core/release-rules.test.ts | 12 ++++++++++++ .../create-release-branch/src/core/release-rules.ts | 6 +++++- .../src/core/release-steps.test.ts | 2 +- .../src/core/user-facing-error.test.ts | 11 +++++++++++ .../src/core/user-facing-error.ts | 4 +++- .../frontends/editor/release-specification.test.ts | 12 +++++++++--- .../src/frontends/editor/release-specification.ts | 9 ++++++++- 7 files changed, 49 insertions(+), 7 deletions(-) diff --git a/packages/create-release-branch/src/core/release-rules.test.ts b/packages/create-release-branch/src/core/release-rules.test.ts index 10fb1d2..f83d9ca 100644 --- a/packages/create-release-branch/src/core/release-rules.test.ts +++ b/packages/create-release-branch/src/core/release-rules.test.ts @@ -10,6 +10,7 @@ import { IncrementableVersionParts, normalizeReleaseChoices, } from './release-rules.js'; +import type { ReleaseChoices } from './release-rules.js'; describe('release-rules', () => { describe('checkReleaseChoices', () => { @@ -154,6 +155,17 @@ describe('release-rules', () => { hint: 'Use major, minor, patch, or a version like 1.2.3.', }, }, + { + rule: 'a version specifier that is not a string', + workspacePackages: { a: buildMockPackage('a', '1.0.0') }, + choices: { a: { version: '2.0.0' } } as unknown as ReleaseChoices, + error: { + packageName: 'a', + entryIndex: 0, + problem: '"{"version":"2.0.0"}" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + }, { rule: 'the current version', workspacePackages: { a: buildMockPackage('a', '1.0.0') }, diff --git a/packages/create-release-branch/src/core/release-rules.ts b/packages/create-release-branch/src/core/release-rules.ts index 8b1221f..5c73279 100644 --- a/packages/create-release-branch/src/core/release-rules.ts +++ b/packages/create-release-branch/src/core/release-rules.ts @@ -266,7 +266,11 @@ export function checkReleaseChoices( errors.push({ packageName: changedPackageName, entryIndex, - problem: `"${String(versionSpecifierOrDirective)}" is not a valid version specifier`, + problem: `"${ + typeof versionSpecifierOrDirective === 'string' + ? versionSpecifierOrDirective + : JSON.stringify(versionSpecifierOrDirective) + }" is not a valid version specifier`, hint: 'Use major, minor, patch, or a version like 1.2.3.', }); } 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 9c6706f..d9964e0 100644 --- a/packages/create-release-branch/src/core/release-steps.test.ts +++ b/packages/create-release-branch/src/core/release-steps.test.ts @@ -201,7 +201,7 @@ describe('release-steps', () => { const project = buildMockProject({ workspacePackages: { a: buildMockPackage('a', '1.0.0') }, }); - const planErrors = [{ message: 'Nothing to release' }]; + const planErrors = [{ problem: 'Nothing to release' }]; const plan = vi.fn().mockResolvedValue({ errors: planErrors }); const mode = buildMockMode({ plan }); 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 3ea03b9..cb0e185 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 @@ -62,6 +62,17 @@ describe('user-facing-error', () => { }); }); + it('stops at a cause that is already in the chain', () => { + const error = new Error('Something failed'); + const cause = new Error('Some cause', { cause: error }); + error.cause = cause; + + expect(describeError(error, format, { debug: false })).toStrictEqual({ + title: 'Something failed', + lines: ['Caused by: Some cause'], + }); + }); + it('describes a value that is not an error', () => { expect(describeError('oops', format, { debug: true })).toStrictEqual({ title: 'oops', 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 dce1bed..4892260 100644 --- a/packages/create-release-branch/src/core/user-facing-error.ts +++ b/packages/create-release-branch/src/core/user-facing-error.ts @@ -49,8 +49,10 @@ export function describeError( const [title, ...lines] = messageOf(error).split('\n'); + const seen = new Set([error]); let cause = error instanceof Error ? error.cause : undefined; - while (cause !== undefined) { + while (cause !== undefined && !seen.has(cause)) { + seen.add(cause); lines.push(`Caused by: ${messageOf(cause)}`); cause = cause instanceof Error ? cause.cause : undefined; } diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts index a23d019..775d677 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts @@ -406,7 +406,10 @@ packages: ).rejects.toThrow( expect.objectContaining({ problems: [ - { problem: 'is not valid YAML', hint: 'Fix the syntax.' }, + { + problem: 'is not valid YAML', + hint: 'Fix the syntax: Missing closing "quote at line 1, column 10:', + }, ], specPath: releaseSpecificationPath, cause: expect.objectContaining({ @@ -2016,7 +2019,10 @@ ${releaseSpecificationPath} problem: 'has a major bump', hint: 'Add them.', }, - { problem: 'is not valid YAML', hint: 'Fix the syntax.' }, + { + problem: 'is not valid YAML', + hint: 'Fix the syntax: Missing closing "quote at line 1, column 10:', + }, ], specPath: '/path/to/RELEASE_SPEC.yml', }); @@ -2034,7 +2040,7 @@ ${releaseSpecificationPath} '', 'The release spec', ' is not valid YAML', - ' (Fix the syntax.)', + ' (Fix the syntax: Missing closing "quote at line 1, column 10:)', '', 'Fix the release spec and run the tool again. It was kept at:', '(/path/to/RELEASE_SPEC.yml)', diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.ts b/packages/create-release-branch/src/frontends/editor/release-specification.ts index 012d5f1..876a448 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.ts @@ -267,9 +267,16 @@ export async function validateReleaseSpecification( releaseSpecificationContents, ) as { packages: ReleaseChoices }; } catch (error) { + // Typecast: `YAML.parse` throws a `YAMLParseError`, which is an `Error`. + const [firstLineOfMessage] = (error as Error).message.split('\n'); throw new ReleaseSpecificationError( { - problems: [{ problem: 'is not valid YAML', hint: 'Fix the syntax.' }], + problems: [ + { + problem: 'is not valid YAML', + hint: `Fix the syntax: ${firstLineOfMessage}`, + }, + ], specPath: releaseSpecificationPath, }, { cause: error }, From ac678df92aa5350b8c02a103724608c271f85ac3 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 17:58:28 +0100 Subject: [PATCH 3/5] docs(create-release-branch): link the changelog entry to the pull request --- packages/create-release-branch/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/create-release-branch/CHANGELOG.md b/packages/create-release-branch/CHANGELOG.md index 768ca11..8e00660 100644 --- a/packages/create-release-branch/CHANGELOG.md +++ b/packages/create-release-branch/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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)) -- 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 ([#](https://github.com/MetaMask/release-tools/pull/)) +- 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)) ### Fixed From 164387d749639ebcbedd41864723727f334d62ad Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 18:39:58 +0100 Subject: [PATCH 4/5] fix(create-release-branch): address the final review --- .../src/core/cli-output.test.ts | 36 +++++++++++++++++-- .../src/core/cli-output.ts | 25 ++++++------- .../editor/release-specification.test.ts | 6 ++-- .../frontends/editor/release-specification.ts | 2 +- 4 files changed, 51 insertions(+), 18 deletions(-) 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 db0eaa0..c869eef 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/packages/create-release-branch/src/core/cli-output.test.ts @@ -419,13 +419,45 @@ describe('cli-output', () => { expect(textOf(stderr)).toBe( [ logLine(S_ERROR, 'Something failed'), - `${S_BAR}\n${S_BAR} Line 1\n${S_BAR}\n${S_BAR} Line 2\n`, - `${S_BAR_END} No release was created.\n\n`, + `${S_BAR}\n${S_BAR} Line 1\n${S_BAR}\n${S_BAR} Line 2\n${S_BAR}\n`, + `${S_BAR_END} The release did not finish.\n\n`, ].join(''), ); expect(stdout.data()).toStrictEqual([]); }); + it('failure writes a gutter line before the closing line when there are no lines', () => { + const stdout = new MockWritable(); + const stderr = new MockWritable(); + const output = createCliOutput({ stdout, stderr }); + + output.failure('Something failed', []); + + expect(textOf(stderr)).toBe( + [ + logLine(S_ERROR, 'Something failed'), + `${S_BAR}\n`, + `${S_BAR_END} The release did not finish.\n\n`, + ].join(''), + ); + }); + + it('failure keeps the gutter on every line of a multi-line entry', () => { + const stdout = new MockWritable(); + const stderr = new MockWritable(); + const output = createCliOutput({ stdout, stderr }); + + output.failure('Something failed', ['Line 1\nLine 2']); + + expect(textOf(stderr)).toBe( + [ + logLine(S_ERROR, 'Something failed'), + `${S_BAR}\n${S_BAR} Line 1\n${S_BAR} Line 2\n${S_BAR}\n`, + `${S_BAR_END} The release did not finish.\n\n`, + ].join(''), + ); + }); + 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 337a049..8456a4f 100644 --- a/packages/create-release-branch/src/core/cli-output.ts +++ b/packages/create-release-branch/src/core/cli-output.ts @@ -191,6 +191,12 @@ export function createCliOutput({ }): CliOutput { const toStdout = { output: styled.stdout ? stdout : withoutColors(stdout) }; const toStderr = { output: styled.stderr ? stderr : withoutColors(stderr) }; + const format = { + name: (text: string): string => + styled.stderr ? styleText('cyan', text, { stream: stderr }) : text, + dim: (text: string): string => + styled.stderr ? styleText('dim', text, { stream: stderr }) : text, + }; return { info(text) { @@ -206,16 +212,14 @@ export function createCliOutput({ log.error(trimTrailingNewlines(text), toStderr); }, failure(title, lines) { + const details = lines.flatMap((line) => line.split('\n')); log.error(title, toStderr); - log.message(lines, toStderr); - cancel('No release was created.', toStderr); - }, - format: { - name: (text) => - styled.stderr ? styleText('cyan', text, { stream: stderr }) : text, - dim: (text) => - styled.stderr ? styleText('dim', text, { stream: stderr }) : text, + // clack writes one gutter line before the details, so an empty entry + // after them adds the same gutter line before the closing line. + log.message(details.length > 0 ? [...details, ''] : [], toStderr); + cancel('The release did not finish.', toStderr); }, + format, async step(label, run) { if (!animate) { try { @@ -253,10 +257,7 @@ export function createCliOutput({ return result; } catch (error) { log.error(label, toStderr); - log.message( - lines.map((line) => styleText('dim', line, { stream: stderr })), - toStderr, - ); + log.message(lines.map(format.dim), toStderr); throw error; } } diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts index 775d677..7d5cf14 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts @@ -408,7 +408,7 @@ packages: problems: [ { problem: 'is not valid YAML', - hint: 'Fix the syntax: Missing closing "quote at line 1, column 10:', + hint: 'Fix the syntax: Missing closing "quote at line 1, column 10', }, ], specPath: releaseSpecificationPath, @@ -2021,7 +2021,7 @@ ${releaseSpecificationPath} }, { problem: 'is not valid YAML', - hint: 'Fix the syntax: Missing closing "quote at line 1, column 10:', + hint: 'Fix the syntax: Missing closing "quote at line 1, column 10', }, ], specPath: '/path/to/RELEASE_SPEC.yml', @@ -2040,7 +2040,7 @@ ${releaseSpecificationPath} '', 'The release spec', ' is not valid YAML', - ' (Fix the syntax: Missing closing "quote at line 1, column 10:)', + ' (Fix the syntax: Missing closing "quote at line 1, column 10)', '', 'Fix the release spec and run the tool again. It was kept at:', '(/path/to/RELEASE_SPEC.yml)', diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.ts b/packages/create-release-branch/src/frontends/editor/release-specification.ts index 876a448..ec69015 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.ts @@ -274,7 +274,7 @@ export async function validateReleaseSpecification( problems: [ { problem: 'is not valid YAML', - hint: `Fix the syntax: ${firstLineOfMessage}`, + hint: `Fix the syntax: ${firstLineOfMessage.replace(/:$/u, '')}`, }, ], specPath: releaseSpecificationPath, From ab9bacf5bb3b0834d8069bf56dd8df690c759d4c Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 19:15:30 +0100 Subject: [PATCH 5/5] fix(create-release-branch): report unknown packages with a version, and align problem lines --- .../src/core/release-rules.test.ts | 19 ++++ .../src/core/release-rules.ts | 1 + .../editor/release-specification.test.ts | 100 +++++++++--------- .../frontends/editor/release-specification.ts | 17 ++- 4 files changed, 76 insertions(+), 61 deletions(-) diff --git a/packages/create-release-branch/src/core/release-rules.test.ts b/packages/create-release-branch/src/core/release-rules.test.ts index f83d9ca..3bf8bc2 100644 --- a/packages/create-release-branch/src/core/release-rules.test.ts +++ b/packages/create-release-branch/src/core/release-rules.test.ts @@ -132,6 +132,25 @@ describe('release-rules', () => { ]); }); + it('reports an unknown package with an exact version only as unknown', () => { + const project = buildMockProject({ + workspacePackages: { + a: buildMockPackage('a', '1.0.0'), + }, + }); + + expect(checkReleaseChoices(project, { b: '1.2.3' }).errors).toStrictEqual( + [ + { + packageName: 'b', + entryIndex: 0, + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', + }, + ], + ); + }); + it.each([ { rule: 'an unknown package', diff --git a/packages/create-release-branch/src/core/release-rules.ts b/packages/create-release-branch/src/core/release-rules.ts index 5c73279..539439a 100644 --- a/packages/create-release-branch/src/core/release-rules.ts +++ b/packages/create-release-branch/src/core/release-rules.ts @@ -255,6 +255,7 @@ export function checkReleaseChoices( problem: 'is not a package in the project', hint: 'Check the name, or remove the line.', }); + return; } if ( diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts index 7d5cf14..c3ee874 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts @@ -692,8 +692,8 @@ packages: The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -739,8 +739,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -786,8 +786,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -833,8 +833,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -881,8 +881,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -929,8 +929,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -977,8 +977,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1025,8 +1025,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its peer dependents are not in the release spec: b - Add them so they stay compatible, or set them to intentionally-skip if they can wait. + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1072,8 +1072,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1119,8 +1119,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1166,8 +1166,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1213,8 +1213,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1261,8 +1261,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1309,8 +1309,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1357,8 +1357,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1405,8 +1405,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - has a major bump, but its direct dependents are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip. + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1632,8 +1632,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - depends on packages with unreleased changes that are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip if they can wait. + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1679,8 +1679,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - depends on packages with unreleased changes that are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip if they can wait. + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1727,8 +1727,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - depends on packages with unreleased changes that are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip if they can wait. + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -1775,8 +1775,8 @@ ${releaseSpecificationPath} The release spec has 1 problem: a - depends on packages with unreleased changes that are not in the release spec: b - Add them to the release spec, or set them to intentionally-skip if they can wait. + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} @@ -2009,7 +2009,7 @@ ${releaseSpecificationPath} const error = new ReleaseSpecificationError({ problems: [ { - lineNumber: 37, + lineNumber: 7, packageName: 'a', problem: '"foo" is not a valid version specifier', hint: 'Use major, minor, patch, or a version like 1.2.3.', @@ -2030,17 +2030,17 @@ ${releaseSpecificationPath} expect(error.describe(format)).toStrictEqual({ title: 'The release spec has 3 problems:', lines: [ - 'Line 37 ', - ' "foo" is not a valid version specifier', - ' (Use major, minor, patch, or a version like 1.2.3.)', + 'Line 7 ', + ' "foo" is not a valid version specifier', + ' (Use major, minor, patch, or a version like 1.2.3.)', '', '', - ' has a major bump', - ' (Add them.)', + ' has a major bump', + ' (Add them.)', '', 'The release spec', - ' is not valid YAML', - ' (Fix the syntax: Missing closing "quote at line 1, column 10)', + ' is not valid YAML', + ' (Fix the syntax: Missing closing "quote at line 1, column 10)', '', 'Fix the release spec and run the tool again. It was kept at:', '(/path/to/RELEASE_SPEC.yml)', @@ -2058,7 +2058,7 @@ ${releaseSpecificationPath} title: 'The release spec has 1 problem:', lines: [ '', - ' is not a package', + ' is not a package', '', 'Fix the release spec and run the tool again. It was kept at:', '(/path/to/RELEASE_SPEC.yml)', @@ -2069,7 +2069,7 @@ ${releaseSpecificationPath} 'The release spec has 1 problem:', '', 'a', - ' is not a package', + ' is not a package', '', 'Fix the release spec and run the tool again. It was kept at:', '/path/to/RELEASE_SPEC.yml', diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.ts b/packages/create-release-branch/src/frontends/editor/release-specification.ts index ec69015..6d996cf 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.ts @@ -77,12 +77,6 @@ const TEMPLATE_TEXT_BY_MODE: Record = { }, }; -/** - * The indentation of the problem and the hint below the line and package of a - * release spec problem. - */ -const PROBLEM_INDENT = ' '.repeat(9); - /** * A problem in the release spec. * @@ -143,12 +137,13 @@ export class ReleaseSpecificationError extends UserFacingError { packageName === undefined ? 'The release spec' : format.name(packageName); + const linePrefix = + lineNumber === undefined ? '' : `Line ${lineNumber} `; + const indent = ' '.repeat(Math.max(linePrefix.length, 2)); return [ - lineNumber === undefined ? subject : `Line ${lineNumber} ${subject}`, - `${PROBLEM_INDENT}${problem}`, - ...(hint === undefined - ? [] - : [`${PROBLEM_INDENT}${format.dim(hint)}`]), + `${linePrefix}${subject}`, + `${indent}${problem}`, + ...(hint === undefined ? [] : [`${indent}${format.dim(hint)}`]), ]; }, );