From 0da172b0a3f1452f1b6b4d2185bf5779f4641645 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 18:09:30 +0100 Subject: [PATCH 1/5] feat(create-release-branch): group repeated warnings --- packages/create-release-branch/CHANGELOG.md | 1 + .../src/cli/initial-parameters.test.ts | 43 ++---- .../src/cli/initial-parameters.ts | 6 +- .../create-release-branch/src/cli/main.ts | 9 +- .../src/cli/select.test.ts | 8 +- .../create-release-branch/src/cli/select.ts | 3 - .../src/core/cli-output.test.ts | 23 +++ .../src/core/cli-output.ts | 9 ++ .../create-release-branch/src/core/mode.ts | 10 +- .../create-release-branch/src/core/notices.ts | 8 + .../src/core/release-steps.test.ts | 62 +++++++- .../src/core/release-steps.ts | 19 ++- .../src/core/run-release.test.ts | 71 ++++++++- .../src/core/run-release.ts | 20 ++- .../frontends/editor/editor-front-end.test.ts | 12 +- .../editor/release-specification.test.ts | 2 - .../src/frontends/ui/server.test.ts | 5 +- .../src/modes/legacy/legacy-mode.test.ts | 45 ++++-- .../src/modes/legacy/legacy-mode.ts | 8 +- .../src/modes/legacy/release-plan.test.ts | 41 ++++- .../src/modes/legacy/release-plan.ts | 31 ++-- .../src/project/package.test.ts | 106 +++++-------- .../src/project/package.ts | 79 ++++------ .../src/project/project.test.ts | 143 +++++++++++++++--- .../src/project/project.ts | 94 ++++++++---- .../tests/unit/helpers.ts | 5 +- 26 files changed, 571 insertions(+), 292 deletions(-) create mode 100644 packages/create-release-branch/src/core/notices.ts diff --git a/packages/create-release-branch/CHANGELOG.md b/packages/create-release-branch/CHANGELOG.md index 8e00660..00d1156 100644 --- a/packages/create-release-branch/CHANGELOG.md +++ b/packages/create-release-branch/CHANGELOG.md @@ -12,6 +12,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 ([#160](https://github.com/MetaMask/release-tools/pull/160)) +- Group repeated warnings (packages without a tag, changelogs without updates) into one warning with a list of packages ([#](https://github.com/MetaMask/release-tools/pull/)) ### Fixed diff --git a/packages/create-release-branch/src/cli/initial-parameters.test.ts b/packages/create-release-branch/src/cli/initial-parameters.test.ts index c7abcfe..80c070d 100644 --- a/packages/create-release-branch/src/cli/initial-parameters.test.ts +++ b/packages/create-release-branch/src/cli/initial-parameters.test.ts @@ -5,7 +5,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { buildMockProject, buildMockPackage, - createNoopCliOutput, } from '../../tests/unit/helpers.js'; import * as projectModule from '../project/project.js'; import * as envModule from '../utils/env.js'; @@ -28,7 +27,6 @@ describe('initial-parameters', () => { it('returns an object derived from command-line arguments and environment variables that contains data necessary to run the workflow', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -45,13 +43,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/somewhere', - output, }); expect(initialParameters).toStrictEqual({ @@ -71,7 +68,6 @@ describe('initial-parameters', () => { const project = buildMockProject({ rootPackage: buildMockPackage(), }); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -94,17 +90,13 @@ describe('initial-parameters', () => { await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/cwd', - output, }); - expect(readProjectSpy).toHaveBeenCalledWith('/path/to/cwd/project', { - output, - }); + expect(readProjectSpy).toHaveBeenCalledWith('/path/to/cwd/project'); }); it('resolves the given temporary directory relative to the current working directory', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -121,13 +113,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/cwd', - output, }); expect(initialParameters.tempDirectoryPath).toBe('/path/to/cwd/tmp'); @@ -137,7 +128,6 @@ describe('initial-parameters', () => { const project = buildMockProject({ rootPackage: buildMockPackage('@foo/bar'), }); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -154,13 +144,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/cwd', - output, }); expect(initialParameters.tempDirectoryPath).toStrictEqual( @@ -170,7 +159,6 @@ describe('initial-parameters', () => { it('returns initial parameters including reset: true, derived from a command-line argument of "--reset true"', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -187,13 +175,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/somewhere', - output, }); expect(initialParameters.reset).toBe(true); @@ -201,7 +188,6 @@ describe('initial-parameters', () => { it('returns initial parameters including reset: false, derived from a command-line argument of "--reset false"', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -218,13 +204,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/somewhere', - output, }); expect(initialParameters.reset).toBe(false); @@ -232,7 +217,6 @@ describe('initial-parameters', () => { it('returns initial parameters including a releaseType of "backport", derived from a command-line argument of "--backport true"', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -249,13 +233,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/somewhere', - output, }); expect(initialParameters.releaseType).toBe('backport'); @@ -263,7 +246,6 @@ describe('initial-parameters', () => { it('returns initial parameters including a releaseType of "ordinary", derived from a command-line argument of "--backport false"', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -280,13 +262,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/somewhere', - output, }); expect(initialParameters.releaseType).toBe('ordinary'); @@ -294,7 +275,6 @@ describe('initial-parameters', () => { it('returns initial parameters including a frontEndName of "ui", derived from a command-line argument of "--interactive true"', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -311,13 +291,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/somewhere', - output, }); expect(initialParameters.frontEndName).toBe('ui'); @@ -325,7 +304,6 @@ describe('initial-parameters', () => { it('returns initial parameters including a frontEndName of "editor", derived from a command-line argument of "--interactive false"', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(commandLineArgumentsModule, 'readCommandLineArguments')) .calledWith(['arg1', 'arg2']) .thenResolve({ @@ -342,13 +320,12 @@ describe('initial-parameters', () => { EDITOR: undefined, }); vi.when(vi.spyOn(projectModule, 'readProject')) - .calledWith('/path/to/project', { output }) + .calledWith('/path/to/project') .thenResolve(project); const initialParameters = await determineInitialParameters({ argv: ['arg1', 'arg2'], cwd: '/path/to/somewhere', - output, }); expect(initialParameters.frontEndName).toBe('editor'); diff --git a/packages/create-release-branch/src/cli/initial-parameters.ts b/packages/create-release-branch/src/cli/initial-parameters.ts index dee9f98..2e40f94 100644 --- a/packages/create-release-branch/src/cli/initial-parameters.ts +++ b/packages/create-release-branch/src/cli/initial-parameters.ts @@ -1,7 +1,6 @@ import os from 'os'; import path from 'path'; -import type { CliOutput } from '../core/cli-output.js'; import type { FrontEndName } from '../core/front-end.js'; import type { ModeName } from '../core/mode.js'; import type { Formatter, ReleaseType } from '../core/types.js'; @@ -27,22 +26,19 @@ type InitialParameters = { * @param args - The arguments to this function. * @param args.argv - The arguments to this executable. * @param args.cwd - The directory in which this executable was run. - * @param args.output - Where to write user-facing messages. * @returns The initial parameters. */ export async function determineInitialParameters({ argv, cwd, - output, }: { argv: string[]; cwd: string; - output: CliOutput; }): Promise { const args = await readCommandLineArguments(argv); const projectDirectoryPath = path.resolve(cwd, args.projectDirectory); - const project = await readProject(projectDirectoryPath, { output }); + const project = await readProject(projectDirectoryPath); const tempDirectoryPath = args.tempDirectory === undefined ? path.join( diff --git a/packages/create-release-branch/src/cli/main.ts b/packages/create-release-branch/src/cli/main.ts index d8097f7..e9c43e9 100644 --- a/packages/create-release-branch/src/cli/main.ts +++ b/packages/create-release-branch/src/cli/main.ts @@ -39,14 +39,9 @@ export async function main({ frontEndName, port, formatter, - } = await determineInitialParameters({ argv, cwd, output }); + } = await determineInitialParameters({ argv, cwd }); - const mode = selectMode(modeName, { - project, - defaultBranch, - formatter, - output, - }); + const mode = selectMode(modeName, { project, defaultBranch, formatter }); const frontEnd = selectFrontEnd(frontEndName, { tempDirectoryPath, reset, diff --git a/packages/create-release-branch/src/cli/select.test.ts b/packages/create-release-branch/src/cli/select.test.ts index 00ec0a3..18c1ac6 100644 --- a/packages/create-release-branch/src/cli/select.test.ts +++ b/packages/create-release-branch/src/cli/select.test.ts @@ -1,9 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; -import { - buildMockProject, - createNoopCliOutput, -} from '../../tests/unit/helpers.js'; +import { buildMockProject } from '../../tests/unit/helpers.js'; import type { FrontEnd } from '../core/front-end.js'; import type { Mode } from '../core/mode.js'; import * as editorFrontEndModule from '../frontends/editor/editor-front-end.js'; @@ -19,7 +16,6 @@ describe('select', () => { describe('selectMode', () => { it('selectMode returns the legacy mode', () => { const project = buildMockProject(); - const output = createNoopCliOutput(); const legacyMode = { name: 'legacy' } as Mode; const createLegacyModeSpy = vi .spyOn(legacyModeModule, 'createLegacyMode') @@ -29,14 +25,12 @@ describe('select', () => { project, defaultBranch: 'main', formatter: 'prettier', - output, }); expect(createLegacyModeSpy).toHaveBeenCalledWith({ project, defaultBranch: 'main', formatter: 'prettier', - output, }); expect(mode).toBe(legacyMode); }); diff --git a/packages/create-release-branch/src/cli/select.ts b/packages/create-release-branch/src/cli/select.ts index 0f0ba10..759100a 100644 --- a/packages/create-release-branch/src/cli/select.ts +++ b/packages/create-release-branch/src/cli/select.ts @@ -1,6 +1,5 @@ import { assertExhaustive } from '@metamask/utils'; -import type { CliOutput } from '../core/cli-output.js'; import type { FrontEnd, FrontEndName } from '../core/front-end.js'; import type { Mode, ModeName } from '../core/mode.js'; import type { Formatter } from '../core/types.js'; @@ -17,7 +16,6 @@ import type { Project } from '../project/project.js'; * @param options.project - Information about the project. * @param options.defaultBranch - The name of the default branch. * @param options.formatter - The formatter for the changelogs. - * @param options.output - Where to write user-facing messages. * @returns The mode. */ export function selectMode( @@ -26,7 +24,6 @@ export function selectMode( project: Project; defaultBranch: string; formatter: Formatter; - output: CliOutput; }, ): Mode { switch (name) { 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 b7de911..30eb54c 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/packages/create-release-branch/src/core/cli-output.test.ts @@ -119,6 +119,29 @@ describe('cli-output', () => { expect(stdout.data()).toStrictEqual([]); }); + it('warnList writes the title and indented items to stderr', () => { + const stdout = new MockWritable(); + const plainStderr = new MockWritable(); + const styledStderr = buildTerminalStream(); + + createCliOutput({ stdout, stderr: plainStderr }).warnList('Some title:', [ + 'a', + 'b', + ]); + createCliOutput({ stdout, stderr: styledStderr }).warnList( + 'Some title:', + ['a'], + ); + + expect(textOf(plainStderr)).toBe( + `${logLine(S_WARN, 'Some title:')}${S_BAR} a\n${S_BAR} b\n`, + ); + expect(textOf(styledStderr)).toContain( + ` ${styleText('dim', 'a', { validateStream: false })}`, + ); + expect(stdout.data()).toStrictEqual([]); + }); + it('writes stderr without colors when only stdout is a color terminal', () => { const stdout = buildTerminalStream(); const stderr = 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 46ccffb..a545aba 100644 --- a/packages/create-release-branch/src/core/cli-output.ts +++ b/packages/create-release-branch/src/core/cli-output.ts @@ -13,6 +13,8 @@ import { stripVTControlCharacters, styleText } from 'node:util'; * @property success - Writes a message about a completed action to standard * out. * @property warn - Writes a warning to standard error. + * @property warnList - Writes a warning to standard error, with each item, such + * as a package name, on its own line below it. * @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. @@ -27,6 +29,7 @@ export type CliOutput = { info(text: string): void; success(text: string): void; warn(text: string): void; + warnList(title: string, items: string[]): void; error(text: string): void; failure(title: string, lines: string[]): void; format: { name(text: string): string; dim(text: string): string }; @@ -206,6 +209,12 @@ export function createCliOutput({ warn(text) { log.warn(trimTrailingNewlines(text), toStderr); }, + warnList(title, items) { + log.warn( + [title, ...items.map((item) => ` ${format.dim(item)}`)].join('\n'), + toStderr, + ); + }, error(text) { log.error(trimTrailingNewlines(text), toStderr); }, diff --git a/packages/create-release-branch/src/core/mode.ts b/packages/create-release-branch/src/core/mode.ts index 0cb251c..b3a2623 100644 --- a/packages/create-release-branch/src/core/mode.ts +++ b/packages/create-release-branch/src/core/mode.ts @@ -1,4 +1,5 @@ import type { Candidate } from './candidates.js'; +import type { Notice } from './notices.js'; import type { ReleaseChoiceError, ReleaseChoices } from './release-rules.js'; /** @@ -32,19 +33,20 @@ export type ModePlanResult = * * @property name - The name of the mode. * @property prepare - Changes the project on the first run, before the - * "Initialize Release" commit. + * "Initialize Release" commit, and returns the notices to print. * @property listCandidates - Lists the packages that can be released, including * the packages that the given choices pull in. * @property plan - Plans the release from release choices that passed the * shared rules. - * @property apply - Writes the planned release to the project. + * @property apply - Writes the planned release to the project, and returns the + * notices to print. * @property getReleaseNotes - Returns the release notes of a package. */ export type Mode = { name: ModeName; - prepare(): Promise; + prepare(): Promise; listCandidates(choices?: ReleaseChoices): Promise; plan(choices: ReleaseChoices): Promise>; - apply(plan: ModePlan, newRootVersion: string): Promise; + apply(plan: ModePlan, newRootVersion: string): Promise; getReleaseNotes(packageName: string): Promise; }; diff --git a/packages/create-release-branch/src/core/notices.ts b/packages/create-release-branch/src/core/notices.ts new file mode 100644 index 0000000..f521d1c --- /dev/null +++ b/packages/create-release-branch/src/core/notices.ts @@ -0,0 +1,8 @@ +/** + * A warning that the core prints once, with the items it applies to, such as + * package names. + * + * @property title - What happened. + * @property items - What it happened to. + */ +export type Notice = { title: string; items: string[] }; 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 d9964e0..e1d1aa2 100644 --- a/packages/create-release-branch/src/core/release-steps.test.ts +++ b/packages/create-release-branch/src/core/release-steps.test.ts @@ -48,14 +48,22 @@ function buildRecordingOutput(calls: string[]): CliOutput { calls.push(`end task ${label}`); return result; }, + warnList(title, items) { + calls.push(`warnList ${title} ${items.join(', ')}`); + }, }; } +const notices = [ + { title: 'Title 1:', items: ['a', 'b'] }, + { title: 'Title 2:', items: ['c'] }, +]; + describe('release-steps', () => { describe('startRelease', () => { it('prepares and commits only on the first run', async () => { const project = buildMockProject(); - const prepare = vi.fn(); + const prepare = vi.fn().mockResolvedValue([]); const mode = buildMockMode({ prepare }); const determineReleaseVersionSpy = vi .spyOn(releaseBranchModule, 'determineReleaseVersion') @@ -103,6 +111,7 @@ describe('release-steps', () => { const mode = buildMockMode({ prepare: vi.fn().mockImplementation(async () => { calls.push('prepare'); + return []; }), }); vi.spyOn( @@ -143,6 +152,38 @@ describe('release-steps', () => { ]); }); + it('prints the notices from prepare after the step', async () => { + const calls: string[] = []; + vi.spyOn(releaseBranchModule, 'determineReleaseVersion').mockReturnValue( + '2.0.0', + ); + vi.spyOn(releaseBranchModule, 'ensureReleaseBranch').mockResolvedValue({ + firstRun: true, + }); + vi.spyOn(repoModule, 'commitAllChanges').mockImplementation(async () => { + calls.push('commitAllChanges'); + }); + + await startRelease({ + project: buildMockProject(), + releaseType: 'ordinary', + mode: buildMockMode({ + prepare: vi.fn().mockResolvedValue(notices), + }), + output: buildRecordingOutput(calls), + }); + + expect(calls).toStrictEqual([ + 'start Creating the release branch', + 'end Creating the release branch', + 'start Updating changelogs', + 'end Updating changelogs', + 'warnList Title 1: a, b', + 'warnList Title 2: c', + 'commitAllChanges', + ]); + }); + it('only creates the release branch after the first run', async () => { const project = buildMockProject(); const calls: string[] = []; @@ -291,6 +332,7 @@ describe('release-steps', () => { const calls: string[] = []; const apply = vi.fn().mockImplementation(async () => { calls.push('apply'); + return []; }); const mode = buildMockMode({ apply }); const plan: ModePlan = { releases: [], data: 'data' }; @@ -309,6 +351,24 @@ describe('release-steps', () => { 'end Applying the release', ]); }); + + it('prints the notices from apply after the step', async () => { + const calls: string[] = []; + + await applyRelease({ + mode: buildMockMode({ apply: vi.fn().mockResolvedValue(notices) }), + plan: { releases: [], data: 'data' }, + newReleaseVersion: '2.0.0', + output: buildRecordingOutput(calls), + }); + + expect(calls).toStrictEqual([ + 'start Applying the release', + 'end Applying the release', + 'warnList Title 1: a, b', + 'warnList Title 2: c', + ]); + }); }); describe('finishRelease', () => { diff --git a/packages/create-release-branch/src/core/release-steps.ts b/packages/create-release-branch/src/core/release-steps.ts index 8bf2ee2..ae76cf7 100644 --- a/packages/create-release-branch/src/core/release-steps.ts +++ b/packages/create-release-branch/src/core/release-steps.ts @@ -21,7 +21,8 @@ import { /** * Creates (or switches to) the release branch. On the first run, prepares the - * project with the mode and commits the result. + * project with the mode, prints the notices of the mode, and commits the + * result. * * @param args - The arguments. * @param args.project - Information about the project. @@ -49,7 +50,13 @@ export async function startRelease({ ); if (firstRun) { - await output.step('Updating changelogs', async () => await mode.prepare()); + const notices = await output.step( + 'Updating changelogs', + async () => await mode.prepare(), + ); + for (const { title, items } of notices) { + output.warnList(title, items); + } // `prepare` may change nothing, and the release branch still needs this // commit. await commitAllChanges( @@ -122,7 +129,8 @@ export async function checkChoices({ } /** - * Writes the planned release to the project with the mode. + * Writes the planned release to the project with the mode, then prints the + * notices of the mode. * * @param args - The arguments. * @param args.mode - The release mode. @@ -141,10 +149,13 @@ export async function applyRelease({ newReleaseVersion: string; output: CliOutput; }): Promise { - await output.step( + const notices = await output.step( 'Applying the release', async () => await mode.apply(plan, newReleaseVersion), ); + for (const { title, items } of notices) { + output.warnList(title, items); + } } /** diff --git a/packages/create-release-branch/src/core/run-release.test.ts b/packages/create-release-branch/src/core/run-release.test.ts index a5a1e3e..009b260 100644 --- a/packages/create-release-branch/src/core/run-release.test.ts +++ b/packages/create-release-branch/src/core/run-release.test.ts @@ -1,7 +1,12 @@ +import { S_BAR, S_WARN } from '@clack/prompts'; import { MockWritable } from 'stdio-mock'; import { describe, expect, it, vi } from 'vitest'; -import { buildMockProject, writtenText } from '../../tests/unit/helpers.js'; +import { + buildMockPackage, + buildMockProject, + writtenText, +} from '../../tests/unit/helpers.js'; import { createCliOutput } from './cli-output.js'; import type { ReleaseContext } from './front-end.js'; import type { Mode } from './mode.js'; @@ -12,15 +17,21 @@ import { runRelease } from './run-release.js'; * * @param args - The arguments. * @param args.isMonorepo - Whether the project has workspaces. + * @param args.packagesUsingRootTag - The packages that use the root tag. * @param args.stdout - The stream that receives standard out. + * @param args.stderr - The stream that receives standard error. * @returns The release context. */ function buildContext({ isMonorepo, + packagesUsingRootTag = [], stdout, + stderr = new MockWritable(), }: { isMonorepo: boolean; + packagesUsingRootTag?: string[]; stdout: MockWritable; + stderr?: MockWritable; }): ReleaseContext { const mode: Mode = { name: 'legacy', @@ -32,10 +43,14 @@ function buildContext({ }; return { - project: buildMockProject({ isMonorepo }), + project: buildMockProject({ + isMonorepo, + rootPackage: buildMockPackage('root', '1389.0.0'), + packagesUsingRootTag, + }), mode, releaseType: 'ordinary', - output: createCliOutput({ stdout, stderr: new MockWritable() }), + output: createCliOutput({ stdout, stderr }), }; } @@ -54,6 +69,56 @@ describe('run-release', () => { expect(run).toHaveBeenCalledWith(context); }); + it.each([ + { + packagesUsingRootTag: ['a', 'b'], + title: + '2 packages have no tag for their current version, so changes are compared against the root tag v1389.0.0:', + }, + { + packagesUsingRootTag: ['a'], + title: + '1 package has no tag for its current version, so changes are compared against the root tag v1389.0.0:', + }, + ])( + 'prints the packages that use the root tag before running the front end ($packagesUsingRootTag)', + async ({ packagesUsingRootTag, title }) => { + const stderr = new MockWritable(); + const context = buildContext({ + isMonorepo: true, + packagesUsingRootTag, + stdout: new MockWritable(), + stderr, + }); + let stderrWhenRunning = ''; + const run = vi.fn().mockImplementation(async () => { + stderrWhenRunning = writtenText(stderr); + }); + + await runRelease(context, { run }); + + expect(stderrWhenRunning).toBe( + [ + `${S_BAR}\n${S_WARN} ${title}\n`, + ...packagesUsingRootTag.map((name) => `${S_BAR} ${name}\n`), + ].join(''), + ); + }, + ); + + it('prints nothing when no package uses the root tag', async () => { + const stderr = new MockWritable(); + const context = buildContext({ + isMonorepo: true, + stdout: new MockWritable(), + stderr, + }); + + await runRelease(context, { run: vi.fn() }); + + expect(stderr.data()).toStrictEqual([]); + }); + it('prints the polyrepo message and stops', async () => { const stdout = new MockWritable(); const context = buildContext({ isMonorepo: false, stdout }); diff --git a/packages/create-release-branch/src/core/run-release.ts b/packages/create-release-branch/src/core/run-release.ts index a331a72..fee5c56 100644 --- a/packages/create-release-branch/src/core/run-release.ts +++ b/packages/create-release-branch/src/core/run-release.ts @@ -11,13 +11,27 @@ export async function runRelease( context: ReleaseContext, frontEnd: FrontEnd, ): Promise { - if (context.project.isMonorepo) { - context.output.info( + const { project, output } = context; + + if (project.isMonorepo) { + output.info( 'Project appears to have workspaces. Following monorepo workflow.\n', ); + + const { packagesUsingRootTag } = project; + if (packagesUsingRootTag.length > 0) { + const rootTag = `v${project.rootPackage.validatedManifest.version.toString()}`; + output.warnList( + packagesUsingRootTag.length === 1 + ? `1 package has no tag for its current version, so changes are compared against the root tag ${rootTag}:` + : `${packagesUsingRootTag.length} packages have no tag for their current version, so changes are compared against the root tag ${rootTag}:`, + packagesUsingRootTag, + ); + } + await frontEnd.run(context); } else { - context.output.info( + output.info( 'Project does not appear to have any workspaces. Following polyrepo workflow.\n', ); } diff --git a/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts b/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts index bcfee18..3825b21 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 @@ -70,7 +70,7 @@ async function followMonorepoWorkflow({ formatter: Formatter; output: CliOutput; }): Promise { - const mode = createLegacyMode({ project, defaultBranch, formatter, output }); + const mode = createLegacyMode({ project, defaultBranch, formatter }); await createEditorFrontEnd({ tempDirectoryPath, reset: firstRemovingExistingReleaseSpecification, @@ -364,12 +364,10 @@ async function setupFollowMonorepoWorkflow({ if (errorUponExecutingReleasePlan) { vi.when(executeReleasePlanSpy) - .calledWith(project, releasePlan, formatter, output) + .calledWith(project, releasePlan, formatter) .thenReject(errorUponExecutingReleasePlan); } else { - vi.when(executeReleasePlanSpy) - .calledWith(project, releasePlan, formatter, output) - .thenResolve(undefined); + executeReleasePlanSpy.mockResolvedValue([]); } vi.when(commitAllChangesSpy) @@ -668,7 +666,6 @@ describe('editor-front-end', () => { project, releasePlan, formatter, - output, ); }); }); @@ -1199,7 +1196,6 @@ describe('editor-front-end', () => { project, releasePlan, formatter, - output, ); }); }); @@ -1438,7 +1434,6 @@ describe('editor-front-end', () => { project, releasePlan, formatter, - output, ); }); }); @@ -1929,7 +1924,6 @@ describe('editor-front-end', () => { project, releasePlan, formatter, - output, ); }); }); 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 c3ee874..28de33e 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 @@ -8,7 +8,6 @@ import { withSandbox } from '../../../tests/helpers.js'; import { buildMockProject, buildMockPackage, - createNoopCliOutput, } from '../../../tests/unit/helpers.js'; import { checkChoices } from '../../core/release-steps.js'; import type { VersionSpecifiers } from '../../core/types.js'; @@ -46,7 +45,6 @@ async function validateReleaseSpecification( project, defaultBranch: 'main', formatter: 'prettier', - output: createNoopCliOutput(), }); const { data } = await validateReleaseSpecificationWithCheck( releaseSpecificationPath, 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 2a1b06a..e65153b 100644 --- a/packages/create-release-branch/src/frontends/ui/server.test.ts +++ b/packages/create-release-branch/src/frontends/ui/server.test.ts @@ -331,7 +331,7 @@ describe('server', () => { releases: [{ name: 'a', newVersion: '1.1.0' }], data: 'x', }; - const apply = vi.fn(); + const apply = vi.fn().mockResolvedValue([]); const closeServer = vi.fn(); const mode = buildMockMode({ plan: vi.fn().mockResolvedValue({ plan }), @@ -380,13 +380,12 @@ describe('server', () => { c: buildMockPackage('c', '1.0.0'), }, }); - const apply = vi.fn(); + const apply = vi.fn().mockResolvedValue([]); const mode = { ...createLegacyMode({ project, defaultBranch: 'main', formatter: 'prettier', - output: createNoopCliOutput(), }), apply, }; diff --git a/packages/create-release-branch/src/modes/legacy/legacy-mode.test.ts b/packages/create-release-branch/src/modes/legacy/legacy-mode.test.ts index d893546..558e6c4 100644 --- a/packages/create-release-branch/src/modes/legacy/legacy-mode.test.ts +++ b/packages/create-release-branch/src/modes/legacy/legacy-mode.test.ts @@ -4,8 +4,8 @@ import { describe, expect, it, vi } from 'vitest'; import { buildMockPackage, buildMockProject, - createNoopCliOutput, } from '../../../tests/unit/helpers.js'; +import type { Notice } from '../../core/notices.js'; import { IncrementableVersionParts } from '../../core/release-rules.js'; import * as projectModule from '../../project/project.js'; import * as fsModule from '../../utils/fs.js'; @@ -21,23 +21,38 @@ vi.mock(import('./release-plan.js'), async (importActual) => ({ executeReleasePlan: vi.fn(), })); +const notices: Notice[] = [ + { title: '1 changelog has no updates to make:', items: ['a'] }, +]; + describe('legacy mode', () => { describe('createLegacyMode', () => { it('updates the changelogs of changed packages when preparing', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); const mode = createLegacyMode({ project, defaultBranch: 'main', formatter: 'prettier', - output, }); await mode.prepare(); expect( projectModule.updateChangelogsForChangedPackages, - ).toHaveBeenCalledWith({ project, formatter: 'prettier', output }); + ).toHaveBeenCalledWith({ project, formatter: 'prettier' }); + }); + + it('prepare returns the grouped changelog notices', async () => { + vi.mocked( + projectModule.updateChangelogsForChangedPackages, + ).mockResolvedValue(notices); + const mode = createLegacyMode({ + project: buildMockProject(), + defaultBranch: 'main', + formatter: 'prettier', + }); + + expect(await mode.prepare()).toStrictEqual(notices); }); it('lists the legacy candidates for the given choices', async () => { @@ -55,7 +70,6 @@ describe('legacy mode', () => { project, defaultBranch: 'main', formatter: 'prettier', - output: createNoopCliOutput(), }); expect(await mode.listCandidates({ a: 'major' })).toStrictEqual([ @@ -78,7 +92,6 @@ describe('legacy mode', () => { project, defaultBranch: 'main', formatter: 'prettier', - output: createNoopCliOutput(), }); const result = await mode.plan({ @@ -104,7 +117,6 @@ describe('legacy mode', () => { it('apply restores skipped changelogs before executing the plan', async () => { const project = buildMockProject(); - const output = createNoopCliOutput(); const releasePlan: ReleasePlan = { newVersion: '2.0.0', packages: [] }; const releaseSpecificationPackages = { a: IncrementableVersionParts.major, @@ -122,13 +134,13 @@ describe('legacy mode', () => { vi.mocked(releasePlanModule.executeReleasePlan).mockImplementation( async () => { calls.push('executeReleasePlan'); + return []; }, ); const mode = createLegacyMode({ project, defaultBranch: 'main', formatter: 'oxfmt', - output, }); await mode.apply( @@ -157,10 +169,24 @@ describe('legacy mode', () => { project, releasePlan, 'oxfmt', - output, ); }); + it('apply returns the grouped changelog notices', async () => { + vi.mocked(releasePlanModule.executeReleasePlan).mockResolvedValue( + notices, + ); + const mode = createLegacyMode({ + project: buildMockProject(), + defaultBranch: 'main', + formatter: 'prettier', + }); + + expect( + await mode.apply({ releases: [], data: {} }, '2.0.0'), + ).toStrictEqual(notices); + }); + it('getReleaseNotes returns the changelog', async () => { const project = buildMockProject({ workspacePackages: { @@ -176,7 +202,6 @@ describe('legacy mode', () => { project, defaultBranch: 'main', formatter: 'prettier', - output: createNoopCliOutput(), }); expect(await mode.getReleaseNotes('a')).toBe('# Changelog'); diff --git a/packages/create-release-branch/src/modes/legacy/legacy-mode.ts b/packages/create-release-branch/src/modes/legacy/legacy-mode.ts index f2b90d4..4f09b07 100644 --- a/packages/create-release-branch/src/modes/legacy/legacy-mode.ts +++ b/packages/create-release-branch/src/modes/legacy/legacy-mode.ts @@ -1,4 +1,3 @@ -import type { CliOutput } from '../../core/cli-output.js'; import type { Mode } from '../../core/mode.js'; import { normalizeReleaseChoices } from '../../core/release-rules.js'; import type { Formatter, VersionSpecifiers } from '../../core/types.js'; @@ -29,25 +28,22 @@ import { * @param args.project - Information about the project. * @param args.defaultBranch - The name of the default branch in the repository. * @param args.formatter - The formatter to use for formatting the changelog. - * @param args.output - Where to write user-facing messages. * @returns The legacy mode. */ export function createLegacyMode({ project, defaultBranch, formatter, - output, }: { project: Project; defaultBranch: string; formatter: Formatter; - output: CliOutput; }): Mode { return { name: 'legacy', async prepare() { - await updateChangelogsForChangedPackages({ project, formatter, output }); + return await updateChangelogsForChangedPackages({ project, formatter }); }, async listCandidates(choices) { @@ -79,7 +75,7 @@ export function createLegacyMode({ releaseSpecificationPackages, newReleaseVersion: newRootVersion, }); - await executeReleasePlan(project, releasePlan, formatter, output); + return await executeReleasePlan(project, releasePlan, formatter); }, async getReleaseNotes(packageName) { diff --git a/packages/create-release-branch/src/modes/legacy/release-plan.test.ts b/packages/create-release-branch/src/modes/legacy/release-plan.test.ts index 5337475..7eccbe4 100644 --- a/packages/create-release-branch/src/modes/legacy/release-plan.test.ts +++ b/packages/create-release-branch/src/modes/legacy/release-plan.test.ts @@ -4,7 +4,6 @@ import { describe, expect, it, vi } from 'vitest'; import { buildMockProject, buildMockPackage, - createNoopCliOutput, } from '../../../tests/unit/helpers.js'; import { IncrementableVersionParts } from '../../core/release-rules.js'; import * as packageUtils from '../../project/package.js'; @@ -183,23 +182,55 @@ describe('release-plan-utils', () => { }, ], }; - const output = createNoopCliOutput(); const updatePackageSpy = vi.spyOn(packageUtils, 'updatePackage'); - await executeReleasePlan(project, releasePlan, 'prettier', output); + await executeReleasePlan(project, releasePlan, 'prettier'); expect(updatePackageSpy).toHaveBeenNthCalledWith(1, { project, packageReleasePlan: releasePlan.packages[0], formatter: 'prettier', - output, }); expect(updatePackageSpy).toHaveBeenNthCalledWith(2, { project, packageReleasePlan: releasePlan.packages[1], formatter: 'prettier', - output, }); }); + + it('returns the packages without a changelog as a notice', async () => { + const releasePlan = { + newVersion: '2.0.0', + packages: ['root', 'a', 'b'].map((name) => ({ + package: buildMockPackage(name), + newVersion: '2.0.0', + })), + }; + const results = { + root: 'no-changelog', + a: 'updated', + b: 'no-changelog', + } as const; + vi.spyOn(packageUtils, 'updatePackage').mockImplementation( + async ({ packageReleasePlan }) => + results[ + packageReleasePlan.package.validatedManifest + .name as keyof typeof results + ], + ); + + const notices = await executeReleasePlan( + buildMockProject(), + releasePlan, + 'prettier', + ); + + expect(notices).toStrictEqual([ + { + title: '2 packages have no changelog, so it was not updated:', + items: ['root', 'b'], + }, + ]); + }); }); }); diff --git a/packages/create-release-branch/src/modes/legacy/release-plan.ts b/packages/create-release-branch/src/modes/legacy/release-plan.ts index 0ccee75..ea06e6e 100644 --- a/packages/create-release-branch/src/modes/legacy/release-plan.ts +++ b/packages/create-release-branch/src/modes/legacy/release-plan.ts @@ -1,10 +1,10 @@ import { SemVer } from 'semver'; -import type { CliOutput } from '../../core/cli-output.js'; +import type { Notice } from '../../core/notices.js'; import type { Formatter, VersionSpecifiers } from '../../core/types.js'; import { updatePackage } from '../../project/package.js'; import type { PackageReleasePlan } from '../../project/package.js'; -import { Project } from '../../project/project.js'; +import { buildChangelogNotices, Project } from '../../project/project.js'; import { debug } from '../../utils/misc-utils.js'; /** @@ -109,25 +109,26 @@ export async function planRelease({ * @param releasePlan - Compiled instructions on how exactly to update the * project in order to prepare a new release. * @param formatter - The formatter to use for formatting the changelog. - * @param output - Where to write user-facing messages. + * @returns The packages without a changelog as notices. */ export async function executeReleasePlan( project: Project, releasePlan: ReleasePlan, formatter: Formatter, - output: CliOutput, -): Promise { - await Promise.all( +): Promise { + const results = await Promise.all( releasePlan.packages.map(async (workspaceReleasePlan) => { - debug( - `Updating package ${workspaceReleasePlan.package.validatedManifest.name}...`, - ); - await updatePackage({ - project, - packageReleasePlan: workspaceReleasePlan, - formatter, - output, - }); + const { name } = workspaceReleasePlan.package.validatedManifest; + debug(`Updating package ${name}...`); + return { + name, + result: await updatePackage({ + project, + packageReleasePlan: workspaceReleasePlan, + formatter, + }), + }; }), ); + return buildChangelogNotices(results); } diff --git a/packages/create-release-branch/src/project/package.test.ts b/packages/create-release-branch/src/project/package.test.ts index 3fe9904..461a150 100644 --- a/packages/create-release-branch/src/project/package.test.ts +++ b/packages/create-release-branch/src/project/package.test.ts @@ -3,7 +3,6 @@ import fs from 'fs'; import path from 'path'; import type { Plugin } from 'prettier'; import { SemVer } from 'semver'; -import { MockWritable } from 'stdio-mock'; import { describe, expect, it, vi } from 'vitest'; import { @@ -15,11 +14,7 @@ import { buildMockPackage, buildMockProject, buildMockManifest, - createNoopCliOutput, - createNoopWriteStream, - writtenText, } from '../../tests/unit/helpers.js'; -import { createCliOutput } from '../core/cli-output.js'; import * as repoModule from '../core/repo.js'; import * as fsModule from '../utils/fs.js'; import * as packageManifestModule from './package-manifest.js'; @@ -188,7 +183,6 @@ describe('package', () => { describe('readMonorepoWorkspacePackage', () => { it('returns information about the file structure of the package located at the given directory', async () => { - const output = createNoopCliOutput(); vi.spyOn(packageManifestModule, 'readPackageManifest').mockResolvedValue({ unvalidated: {}, validated: buildMockManifest(), @@ -200,7 +194,6 @@ describe('package', () => { projectTagNames: [], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); expect(pkg).toMatchObject({ @@ -213,7 +206,6 @@ describe('package', () => { it('returns information about the manifest (in both unvalidated and validated forms)', async () => { const unvalidatedManifest = {}; const validatedManifest = buildMockManifest(); - const output = createNoopCliOutput(); vi.when(vi.spyOn(packageManifestModule, 'readPackageManifest')) .calledWith('/path/to/package/package.json') .thenResolve({ @@ -227,7 +219,6 @@ describe('package', () => { projectTagNames: [], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); expect(pkg).toMatchObject({ @@ -237,7 +228,6 @@ describe('package', () => { }); it("flags the package as having been changed since its latest release if a tag matching the package name + version exists and changes have been made to the package's directory since the tag", async () => { - const output = createNoopCliOutput(); vi.spyOn(packageManifestModule, 'readPackageManifest').mockResolvedValue({ unvalidated: {}, validated: buildMockManifest({ @@ -259,7 +249,6 @@ describe('package', () => { projectTagNames: ['@scope/workspace-package@1.0.0'], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); expect(pkg).toMatchObject({ @@ -268,7 +257,6 @@ describe('package', () => { }); it("does not flag the package as having been changed since its latest release if a tag matching the package name + version exists, but changes have not been made to the package's directory since the tag", async () => { - const output = createNoopCliOutput(); vi.spyOn(packageManifestModule, 'readPackageManifest').mockResolvedValue({ unvalidated: {}, validated: buildMockManifest({ @@ -290,7 +278,6 @@ describe('package', () => { projectTagNames: ['@scope/workspace-package@1.0.0'], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); expect(pkg).toMatchObject({ @@ -299,7 +286,6 @@ describe('package', () => { }); it("flags the package as having been changed since its latest release if a tag matching 'v' + the root package version exists instead of the package name + version, and changes have been made to the package's directory since the tag", async () => { - const output = createNoopCliOutput(); vi.spyOn(packageManifestModule, 'readPackageManifest').mockResolvedValue({ unvalidated: {}, validated: buildMockManifest({ @@ -317,7 +303,6 @@ describe('package', () => { projectTagNames: ['v5.0.0'], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); expect(pkg).toMatchObject({ @@ -326,7 +311,6 @@ describe('package', () => { }); it("does not flag the package as having been changed since its latest release if a tag matching 'v' + the root package version exists instead of the package name + version, but changes have not been made to the package's directory since the tag", async () => { - const output = createNoopCliOutput(); vi.spyOn(packageManifestModule, 'readPackageManifest').mockResolvedValue({ unvalidated: {}, validated: buildMockManifest({ @@ -343,7 +327,6 @@ describe('package', () => { projectTagNames: ['v5.0.0'], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); expect(pkg).toMatchObject({ @@ -352,7 +335,6 @@ describe('package', () => { }); it('flags the package as having been changed since its latest release if the project has no tags', async () => { - const output = createNoopCliOutput(); vi.spyOn(packageManifestModule, 'readPackageManifest').mockResolvedValue({ unvalidated: {}, validated: buildMockManifest({ @@ -366,7 +348,6 @@ describe('package', () => { projectTagNames: [], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); expect(pkg).toMatchObject({ @@ -374,38 +355,39 @@ describe('package', () => { }); }); - it("prints a warning if a tag matching 'v' + the root package version exists instead of the package name + version", async () => { - const stderr = new MockWritable(); - const output = createCliOutput({ - stdout: createNoopWriteStream(), - stderr, - }); - vi.when(vi.spyOn(packageManifestModule, 'readPackageManifest')) - .calledWith('/path/to/package/package.json') - .thenResolve({ - unvalidated: {}, - validated: buildMockManifest({ - name: '@scope/workspace-package', - version: new SemVer('1.0.0'), - }), + it.each([ + { tagNames: ['v5.0.0'], usesRootTag: true }, + { + tagNames: ['v5.0.0', '@scope/workspace-package@1.0.0'], + usesRootTag: false, + }, + { tagNames: [], usesRootTag: false }, + ])( + 'flags whether the package uses the root tag when the tags are $tagNames', + async ({ tagNames, usesRootTag }) => { + vi.when(vi.spyOn(packageManifestModule, 'readPackageManifest')) + .calledWith('/path/to/package/package.json') + .thenResolve({ + unvalidated: {}, + validated: buildMockManifest({ + name: '@scope/workspace-package', + version: new SemVer('1.0.0'), + }), + }); + + const pkg = await readMonorepoWorkspacePackage({ + packageDirectoryPath: '/path/to/package', + projectDirectoryPath: '/path/to/project', + projectTagNames: tagNames, + rootPackageName: 'root-package', + rootPackageVersion: new SemVer('5.0.0'), }); - await readMonorepoWorkspacePackage({ - packageDirectoryPath: '/path/to/package', - projectDirectoryPath: '/path/to/project', - projectTagNames: ['v5.0.0'], - rootPackageName: 'root-package', - rootPackageVersion: new SemVer('5.0.0'), - output, - }); - - expect(writtenText(stderr)).toContain( - 'WARNING: Could not determine changes for workspace package @scope/workspace-package version 1.0.0 based on Git tag "@scope/workspace-package@1.0.0"; using tag for root package root-package version 5.0.0, "v5.0.0", instead.', - ); - }); + expect(pkg.usesRootTag).toBe(usesRootTag); + }, + ); it("throws if the project has tags, but neither a tag matching the package name + version nor 'v' + the root package version exists", async () => { - const output = createNoopCliOutput(); vi.when(vi.spyOn(packageManifestModule, 'readPackageManifest')) .calledWith('/path/to/package/package.json') .thenResolve({ @@ -422,7 +404,6 @@ describe('package', () => { projectTagNames: ['some-tag'], rootPackageName: 'root-package', rootPackageVersion: new SemVer('5.0.0'), - output, }); await expect(promise).rejects.toThrow( @@ -455,7 +436,6 @@ describe('package', () => { project, packageReleasePlan, formatter: 'prettier', - output: createNoopCliOutput(), }); const newManifest = JSON.parse( @@ -503,7 +483,6 @@ describe('package', () => { project, packageReleasePlan, formatter: 'prettier', - output: createNoopCliOutput(), }); const newChangelogContent = await fs.promises.readFile( @@ -571,7 +550,6 @@ describe('package', () => { project, packageReleasePlan, formatter: 'oxfmt', - output: createNoopCliOutput(), }); const newChangelogContent = await fs.promises.readFile( @@ -625,13 +603,12 @@ describe('package', () => { project, packageReleasePlan, formatter: 'prettier', - output: createNoopCliOutput(), }), ).rejects.toThrow('oops'); }); }); - it('does not throw but merely prints a warning if the package does not have a changelog', async () => { + it('returns no-changelog when the package does not have a changelog', async () => { await withSandbox(async (sandbox) => { const project = buildMockProject(); const changelogPath = path.join(sandbox.directoryPath, 'CHANGELOG.md'); @@ -649,10 +626,9 @@ describe('package', () => { project, packageReleasePlan, formatter: 'prettier', - output: createNoopCliOutput(), }); - expect(result).toBeUndefined(); + expect(result).toBe('no-changelog'); }); }); }); @@ -660,7 +636,6 @@ describe('package', () => { describe('updatePackageChangelog', () => { it('updates the changelog of the package if requested to do so and if the package has one', async () => { await withSandbox(async (sandbox) => { - const output = createNoopCliOutput(); const project = buildMockProject({ repositoryUrl: 'https://repo.url', }); @@ -685,11 +660,10 @@ describe('package', () => { .thenResolve('new changelog'); await fs.promises.writeFile(changelogPath, 'existing changelog'); - await updatePackageChangelog({ + const result = await updatePackageChangelog({ project, package: pkg, formatter: 'prettier', - output, }); const newChangelogContent = await fs.promises.readFile( @@ -697,12 +671,12 @@ describe('package', () => { 'utf8', ); expect(newChangelogContent).toBe('new changelog'); + expect(result).toBe('updated'); }); }); - it('does not update the changelog if updateChangelog returns nothing', async () => { + it('returns no-updates when there is nothing to add', async () => { await withSandbox(async (sandbox) => { - const output = createNoopCliOutput(); const project = buildMockProject({ repositoryUrl: 'https://repo.url', }); @@ -727,11 +701,10 @@ describe('package', () => { .thenResolve(undefined); await fs.promises.writeFile(changelogPath, 'existing changelog'); - await updatePackageChangelog({ + const result = await updatePackageChangelog({ project, package: pkg, formatter: 'prettier', - output, }); const newChangelogContent = await fs.promises.readFile( @@ -739,12 +712,12 @@ describe('package', () => { 'utf8', ); expect(newChangelogContent).toBe('existing changelog'); + expect(result).toBe('no-updates'); }); }); - it('does not throw but merely prints a warning if the package does not have a changelog', async () => { + it('returns no-changelog when the changelog is missing', async () => { await withSandbox(async (sandbox) => { - const output = createNoopCliOutput(); const project = buildMockProject({ repositoryUrl: 'https://repo.url', }); @@ -760,16 +733,14 @@ describe('package', () => { project, package: pkg, formatter: 'prettier', - output, }); - expect(result).toBeUndefined(); + expect(result).toBe('no-changelog'); }); }); it("throws if reading the package's changelog fails in an unexpected way", async () => { await withSandbox(async (sandbox) => { - const output = createNoopCliOutput(); const project = buildMockProject({ repositoryUrl: 'https://repo.url', }); @@ -787,7 +758,6 @@ describe('package', () => { project, package: pkg, formatter: 'prettier', - output, }), ).rejects.toThrow('oops'); }); diff --git a/packages/create-release-branch/src/project/package.ts b/packages/create-release-branch/src/project/package.ts index aa225ec..6145d0a 100644 --- a/packages/create-release-branch/src/project/package.ts +++ b/packages/create-release-branch/src/project/package.ts @@ -8,7 +8,6 @@ import { assertExhaustive } from '@metamask/utils'; import path from 'path'; import { format } from 'util'; -import type { CliOutput } from '../core/cli-output.js'; import { hasChangesInDirectorySinceGitTag } from '../core/repo.js'; import type { Formatter } from '../core/types.js'; import { readFile, writeFile, writeJsonFile } from '../utils/fs.js'; @@ -56,6 +55,12 @@ export type PackageReleasePlan = { newVersion: string; }; +/** + * What happened to the changelog of a package: it was updated, the package + * has no changelog, or there was nothing to add to it. + */ +export type ChangelogUpdateResult = 'updated' | 'no-changelog' | 'no-updates'; + /** * Generates the possible Git tag name for the root package of a monorepo. The * only tag name in use at this time is "v" + the package version. @@ -159,8 +164,9 @@ export async function readMonorepoRootPackage({ * monorepo to which this package belongs. * @param args.projectDirectoryPath - The path to the project directory. * @param args.projectTagNames - The tag names across the whole project. - * @param args.output - Where to write user-facing messages. - * @returns Information about the package. + * @returns Information about the package, and whether the package has no tag + * for its current version, so its changes are found from the tag of the root + * package. */ export async function readMonorepoWorkspacePackage({ packageDirectoryPath, @@ -168,15 +174,13 @@ export async function readMonorepoWorkspacePackage({ rootPackageVersion, projectDirectoryPath, projectTagNames, - output, }: { packageDirectoryPath: string; rootPackageName: string; rootPackageVersion: SemVer; projectDirectoryPath: string; projectTagNames: string[]; - output: CliOutput; -}): Promise { +}): Promise { const manifestPath = path.join(packageDirectoryPath, MANIFEST_FILE_NAME); const changelogPath = path.join(packageDirectoryPath, CHANGELOG_FILE_NAME); const { unvalidated: unvalidatedManifest, validated: validatedManifest } = @@ -215,22 +219,9 @@ export async function readMonorepoWorkspacePackage({ ); } - if ( + const usesRootTag = matchingTagNameForWorkspacePackageLatestRelease === undefined && - matchingTagNameForRootPackageLatestRelease !== undefined - ) { - output.warn( - format( - 'WARNING: Could not determine changes for workspace package %s version %s based on Git tag %s; using tag for root package %s version %s, %s, instead.\n', - validatedManifest.name, - validatedManifest.version, - `"${expectedTagNameForWorkspacePackageLatestRelease}"`, - rootPackageName, - rootPackageVersion, - `"${expectedTagNameForRootPackageLatestRelease}"`, - ), - ); - } + matchingTagNameForRootPackageLatestRelease !== undefined; const hasChangesSinceLatestRelease = matchingTagNameForLatestRelease === undefined @@ -248,6 +239,7 @@ export async function readMonorepoWorkspacePackage({ unvalidatedManifest, changelogPath, hasChangesSinceLatestRelease, + usesRootTag, }; } @@ -284,32 +276,26 @@ export function getFormatter( * @param args.package - A particular package in the project. * @param args.version - The release version to migrate unreleased changes to. * @param args.formatter - The formatter to use for formatting the changelog. - * @param args.output - Where to write user-facing messages. - * @returns The result of writing to the changelog. + * @returns What happened to the changelog. */ export async function migrateUnreleasedChangelogChangesToRelease({ project: { repositoryUrl }, package: pkg, version, formatter, - output, }: { project: Pick; package: Package; version: string; formatter: Formatter; - output: CliOutput; -}): Promise { +}): Promise { let changelogContent; try { changelogContent = await readFile(pkg.changelogPath); } catch (error) { if (isErrorWithCode(error) && error.code === 'ENOENT') { - output.warn( - `${pkg.validatedManifest.name} does not seem to have a changelog. Skipping.\n`, - ); - return; + return 'no-changelog'; } throw error; @@ -325,6 +311,7 @@ export async function migrateUnreleasedChangelogChangesToRelease({ changelog.addRelease({ version }); changelog.migrateUnreleasedChangesToRelease(version); await writeFile(pkg.changelogPath, await changelog.toString()); + return 'updated'; } /** @@ -337,30 +324,24 @@ export async function migrateUnreleasedChangelogChangesToRelease({ * @param args.project.repositoryUrl - The URL of the project's repository. * @param args.package - A particular package in the project. * @param args.formatter - The formatter to use for formatting the changelog. - * @param args.output - Where to write user-facing messages. - * @returns The result of writing to the changelog. + * @returns What happened to the changelog. */ export async function updatePackageChangelog({ project: { repositoryUrl }, package: pkg, formatter, - output, }: { project: Pick; package: Package; formatter: Formatter; - output: CliOutput; -}): Promise { +}): Promise { let changelogContent; try { changelogContent = await readFile(pkg.changelogPath); } catch (error) { if (isErrorWithCode(error) && error.code === 'ENOENT') { - output.warn( - `${pkg.validatedManifest.name} does not seem to have a changelog. Skipping.\n`, - ); - return; + return 'no-changelog'; } throw error; @@ -378,13 +359,12 @@ export async function updatePackageChangelog({ formatter: getFormatter(formatter), }); - if (newChangelogContent) { - await writeFile(pkg.changelogPath, newChangelogContent); - } else { - output.warn( - `Changelog for ${pkg.validatedManifest.name} was not updated as there were no updates to make.`, - ); + if (!newChangelogContent) { + return 'no-updates'; } + + await writeFile(pkg.changelogPath, newChangelogContent); + return 'updated'; } /** @@ -398,19 +378,17 @@ export async function updatePackageChangelog({ * @param args.packageReleasePlan - The release plan for a particular package in the * project. * @param args.formatter - The formatter to use for formatting the changelog. - * @param args.output - Where to write user-facing messages. + * @returns What happened to the changelog. */ export async function updatePackage({ project, packageReleasePlan, formatter, - output, }: { project: Pick; packageReleasePlan: PackageReleasePlan; formatter: Formatter; - output: CliOutput; -}): Promise { +}): Promise { const { package: pkg, newVersion } = packageReleasePlan; await writeJsonFile(pkg.manifestPath, { @@ -418,11 +396,10 @@ export async function updatePackage({ version: newVersion, }); - await migrateUnreleasedChangelogChangesToRelease({ + return await migrateUnreleasedChangelogChangesToRelease({ project, package: pkg, formatter, - output, version: newVersion, }); } diff --git a/packages/create-release-branch/src/project/project.test.ts b/packages/create-release-branch/src/project/project.test.ts index 5a653a1..aeba88d 100644 --- a/packages/create-release-branch/src/project/project.test.ts +++ b/packages/create-release-branch/src/project/project.test.ts @@ -8,7 +8,6 @@ import { withProtectedProcessEnv, withSandbox } from '../../tests/helpers.js'; import { buildMockPackage, buildMockProject, - createNoopCliOutput, } from '../../tests/unit/helpers.js'; import { IncrementableVersionParts } from '../core/release-rules.js'; import * as repoModule from '../core/repo.js'; @@ -16,6 +15,7 @@ import * as fs from '../utils/fs.js'; import * as miscUtils from '../utils/misc-utils.js'; import * as packageModule from './package.js'; import { + buildChangelogNotices, getValidRepositoryUrl, readProject, restoreChangelogsForSkippedPackages, @@ -66,7 +66,6 @@ describe('project', () => { }), }; const projectTagNames = ['tag1', 'tag2', 'tag3']; - const output = createNoopCliOutput(); vi.when(vi.spyOn(repoModule, 'getTagNames')) .calledWith(projectDirectoryPath) .thenResolve(projectTagNames); @@ -99,9 +98,8 @@ describe('project', () => { rootPackageVersion, projectDirectoryPath, projectTagNames, - output, }) - .thenResolve(workspacePackages.a); + .thenResolve({ ...workspacePackages.a, usesRootTag: false }); vi.when(readMonorepoWorkspacePackageMock) .calledWith({ packageDirectoryPath: path.join( @@ -114,9 +112,8 @@ describe('project', () => { rootPackageVersion, projectDirectoryPath, projectTagNames, - output, }) - .thenResolve(workspacePackages.b); + .thenResolve({ ...workspacePackages.b, usesRootTag: false }); await mkdir(path.join(projectDirectoryPath, 'packages')); await mkdir(path.join(projectDirectoryPath, 'packages', 'a')); await mkdir(path.join(projectDirectoryPath, 'packages', 'subpackages')); @@ -124,13 +121,12 @@ describe('project', () => { path.join(projectDirectoryPath, 'packages', 'subpackages', 'b'), ); - expect( - await readProject(projectDirectoryPath, { output }), - ).toStrictEqual({ + expect(await readProject(projectDirectoryPath)).toStrictEqual({ directoryPath: projectDirectoryPath, repositoryUrl: projectRepositoryUrl, rootPackage, workspacePackages, + packagesUsingRootTag: [], isMonorepo: true, releaseVersion: { ordinaryNumber: 4, @@ -139,6 +135,59 @@ describe('project', () => { }); }); }); + + it('lists the packages that use the root tag', async () => { + await withSandbox(async (sandbox) => { + const projectDirectoryPath = sandbox.directoryPath; + const rootPackage = buildMockPackage('root', '1.0.0', { + directoryPath: projectDirectoryPath, + validatedManifest: { workspaces: ['packages/*'] }, + unvalidatedManifest: { repository: 'https://github.com/org/repo' }, + }); + vi.spyOn(repoModule, 'getTagNames').mockResolvedValue(['v1.0.0']); + vi.spyOn(packageModule, 'readMonorepoRootPackage').mockResolvedValue( + rootPackage, + ); + vi.spyOn(actionUtils, 'getWorkspaceLocations').mockResolvedValue([ + 'packages/c', + 'packages/a', + 'packages/b', + ]); + const readMonorepoWorkspacePackageMock = vi.spyOn( + packageModule, + 'readMonorepoWorkspacePackage', + ); + for (const [name, usesRootTag] of [ + ['c', true], + ['a', false], + ['b', true], + ] as const) { + vi.when(readMonorepoWorkspacePackageMock) + .calledWith({ + packageDirectoryPath: path.join( + projectDirectoryPath, + 'packages', + name, + ), + rootPackageName: 'root', + rootPackageVersion: rootPackage.validatedManifest.version, + projectDirectoryPath, + projectTagNames: ['v1.0.0'], + }) + .thenResolve({ ...buildMockPackage(name), usesRootTag }); + } + + const project = await readProject(projectDirectoryPath); + + expect(project.packagesUsingRootTag).toStrictEqual(['c', 'b']); + expect(Object.keys(project.workspacePackages)).toStrictEqual([ + 'c', + 'a', + 'b', + ]); + expect(project.workspacePackages.c).not.toHaveProperty('usesRootTag'); + }); + }); }); describe('getValidRepositoryUrl', () => { @@ -317,7 +366,6 @@ describe('project', () => { describe('updateChangelogsForChangedPackages', () => { it('should update changelog files of all the packages that has changes since latest release', async () => { - const output = createNoopCliOutput(); const project = buildMockProject({ rootPackage: buildMockPackage('monorepo'), workspacePackages: { @@ -330,34 +378,30 @@ describe('project', () => { }, }); - const updatePackageChangelogSpy = vi.spyOn( - packageModule, - 'updatePackageChangelog', - ); + const updatePackageChangelogSpy = vi + .spyOn(packageModule, 'updatePackageChangelog') + .mockResolvedValue('updated'); - await updateChangelogsForChangedPackages({ + const notices = await updateChangelogsForChangedPackages({ project, formatter: 'prettier', - output, }); expect(updatePackageChangelogSpy).toHaveBeenCalledWith({ project, package: project.workspacePackages.a, formatter: 'prettier', - output, }); expect(updatePackageChangelogSpy).toHaveBeenCalledWith({ project, package: project.workspacePackages.b, formatter: 'prettier', - output, }); + expect(notices).toStrictEqual([]); }); it('should not update changelog files of all the packages that has not changed since latest release', async () => { - const output = createNoopCliOutput(); const project = buildMockProject({ rootPackage: buildMockPackage('monorepo'), workspacePackages: { @@ -375,14 +419,67 @@ describe('project', () => { await updateChangelogsForChangedPackages({ project, formatter: 'prettier', - output, }); - expect(updatePackageChangelogSpy).not.toHaveBeenCalledWith({ + expect(updatePackageChangelogSpy).not.toHaveBeenCalled(); + }); + + it('returns the packages without a changelog or without updates as notices', async () => { + const project = buildMockProject({ + workspacePackages: Object.fromEntries( + ['a', 'b', 'c', 'd'].map((name) => [ + name, + buildMockPackage(name, { hasChangesSinceLatestRelease: true }), + ]), + ), + }); + const results = { + a: 'no-updates', + b: 'no-changelog', + c: 'no-updates', + d: 'updated', + } as const; + vi.spyOn(packageModule, 'updatePackageChangelog').mockImplementation( + async ({ package: pkg }) => + results[pkg.validatedManifest.name as keyof typeof results], + ); + + const notices = await updateChangelogsForChangedPackages({ project, - package: project.workspacePackages.a, - output, + formatter: 'prettier', }); + + expect(notices).toStrictEqual([ + { + title: '1 package has no changelog, so it was not updated:', + items: ['b'], + }, + { title: '2 changelogs have no updates to make:', items: ['a', 'c'] }, + ]); + }); + }); + + describe('buildChangelogNotices', () => { + it('uses plural titles for more than one package', () => { + expect( + buildChangelogNotices([ + { name: 'a', result: 'no-changelog' }, + { name: 'b', result: 'no-changelog' }, + { name: 'c', result: 'no-updates' }, + ]), + ).toStrictEqual([ + { + title: '2 packages have no changelog, so it was not updated:', + items: ['a', 'b'], + }, + { title: '1 changelog has no updates to make:', items: ['c'] }, + ]); + }); + + it('returns no notices when every changelog was updated', () => { + expect( + buildChangelogNotices([{ name: 'a', result: 'updated' }]), + ).toStrictEqual([]); }); }); }); diff --git a/packages/create-release-branch/src/project/project.ts b/packages/create-release-branch/src/project/project.ts index e624e3c..9e4e3a2 100644 --- a/packages/create-release-branch/src/project/project.ts +++ b/packages/create-release-branch/src/project/project.ts @@ -2,7 +2,7 @@ import { getWorkspaceLocations } from '@metamask/action-utils'; import { isPlainObject } from '@metamask/utils'; import { resolve } from 'path'; -import type { CliOutput } from '../core/cli-output.js'; +import type { Notice } from '../core/notices.js'; import { getTagNames, restoreFiles } from '../core/repo.js'; import type { Formatter, VersionSpecifiers } from '../core/types.js'; import { fileExists } from '../utils/fs.js'; @@ -16,6 +16,7 @@ import { UnvalidatedPackageManifest, } from './package-manifest.js'; import { + ChangelogUpdateResult, Package, readMonorepoRootPackage, readMonorepoWorkspacePackage, @@ -50,12 +51,16 @@ type ReleaseVersion = { * project is a monorepo). * @property workspacePackages - Information about packages that are referenced * via workspaces (assuming that the project is a monorepo). + * @property packagesUsingRootTag - The names of the workspace packages that + * have no tag for their current version, so their changes are found from the + * tag of the root package, in workspace order. */ export type Project = { directoryPath: string; repositoryUrl: string; rootPackage: Package; workspacePackages: Record; + packagesUsingRootTag: string[]; isMonorepo: boolean; releaseVersion: ReleaseVersion; }; @@ -81,8 +86,6 @@ function examineReleaseVersion(packageVersion: SemVer): ReleaseVersion { * packages within workspaces specified via the root `package.json`. * * @param projectDirectoryPath - The path to the project. - * @param args - Additional arguments. - * @param args.output - Where to write user-facing messages. * @returns An object that represents information about the project. * @throws if the project does not contain a root `package.json` (polyrepo and * monorepo) or if any of the workspaces specified in the root `package.json` do @@ -90,7 +93,6 @@ function examineReleaseVersion(packageVersion: SemVer): ReleaseVersion { */ export async function readProject( projectDirectoryPath: string, - { output }: { output: CliOutput }, ): Promise { const tagNames = await getTagNames(projectDirectoryPath); const rootPackage = await readMonorepoRootPackage({ @@ -112,22 +114,25 @@ export async function readProject( true, ); - const workspacePackages = ( - await Promise.all( - workspaceDirectories.map(async (directory) => { - return await readMonorepoWorkspacePackage({ - packageDirectoryPath: resolve(projectDirectoryPath, directory), - rootPackageName: rootPackage.validatedManifest.name, - rootPackageVersion: rootPackage.validatedManifest.version, - projectDirectoryPath, - projectTagNames: tagNames, - output, - }); - }), - ) - ).reduce>((obj, pkg) => { + const workspacePackageReads = await Promise.all( + workspaceDirectories.map(async (directory) => { + return await readMonorepoWorkspacePackage({ + packageDirectoryPath: resolve(projectDirectoryPath, directory), + rootPackageName: rootPackage.validatedManifest.name, + rootPackageVersion: rootPackage.validatedManifest.version, + projectDirectoryPath, + projectTagNames: tagNames, + }); + }), + ); + const workspacePackages = workspacePackageReads.reduce< + Record + >((obj, { usesRootTag: _usesRootTag, ...pkg }) => { return { ...obj, [pkg.validatedManifest.name]: pkg }; }, {}); + const packagesUsingRootTag = workspacePackageReads + .filter(({ usesRootTag }) => usesRootTag) + .map(({ validatedManifest }) => validatedManifest.name); const isMonorepo = Object.keys(workspacePackages).length > 0; @@ -136,6 +141,7 @@ export async function readProject( repositoryUrl, rootPackage, workspacePackages, + packagesUsingRootTag, isMonorepo, releaseVersion, }; @@ -202,36 +208,68 @@ export async function getValidRepositoryUrl( * * @param args - The arguments. * @param args.project - The project. - * @param args.output - Where to write user-facing messages. * @param args.formatter - The formatter to use for formatting the changelog. - * @returns The result of writing to the changelog. + * @returns The packages without a changelog, and the packages with nothing to + * add to their changelog, as notices. */ export async function updateChangelogsForChangedPackages({ project, formatter, - output, }: { project: Pick< Project, 'directoryPath' | 'repositoryUrl' | 'workspacePackages' >; formatter: Formatter; - output: CliOutput; -}): Promise { - await Promise.all( +}): Promise { + const results = await Promise.all( Object.values(project.workspacePackages) .filter( ({ hasChangesSinceLatestRelease }) => hasChangesSinceLatestRelease, ) - .map(async (pkg) => - updatePackageChangelog({ + .map(async (pkg) => ({ + name: pkg.validatedManifest.name, + result: await updatePackageChangelog({ project, formatter, package: pkg, - output, }), - ), + })), ); + return buildChangelogNotices(results); +} + +/** + * Groups the packages without a changelog, and the packages with nothing to + * add to their changelog, into one notice each. + * + * @param results - What happened to the changelog of each package. + * @returns The notices, without the groups that have no packages. + */ +export function buildChangelogNotices( + results: { name: string; result: ChangelogUpdateResult }[], +): Notice[] { + const namesWith = (result: ChangelogUpdateResult): string[] => + results.filter((entry) => entry.result === result).map(({ name }) => name); + const withoutChangelog = namesWith('no-changelog'); + const withoutUpdates = namesWith('no-updates'); + + return [ + { + title: + withoutChangelog.length === 1 + ? '1 package has no changelog, so it was not updated:' + : `${withoutChangelog.length} packages have no changelog, so it was not updated:`, + items: withoutChangelog, + }, + { + title: + withoutUpdates.length === 1 + ? '1 changelog has no updates to make:' + : `${withoutUpdates.length} changelogs have no updates to make:`, + items: withoutUpdates, + }, + ].filter(({ items }) => items.length > 0); } /** diff --git a/packages/create-release-branch/tests/unit/helpers.ts b/packages/create-release-branch/tests/unit/helpers.ts index 02d7355..2fad518 100644 --- a/packages/create-release-branch/tests/unit/helpers.ts +++ b/packages/create-release-branch/tests/unit/helpers.ts @@ -66,6 +66,7 @@ export function buildMockProject(overrides: Partial = {}): Project { repositoryUrl: 'https://repo.url', rootPackage: buildMockPackage('root'), workspacePackages: {}, + packagesUsingRootTag: [], isMonorepo: false, releaseVersion: { ordinaryNumber: 1, @@ -206,10 +207,10 @@ export function buildMockMode( ): Mode { return { name: 'legacy', - prepare: vi.fn(), + prepare: vi.fn().mockResolvedValue([]), listCandidates: vi.fn().mockResolvedValue([]), plan: vi.fn(), - apply: vi.fn(), + apply: vi.fn().mockResolvedValue([]), getReleaseNotes: vi.fn(), ...overrides, }; From 7765479aea9125c6307c409df74f340b268067c9 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 18:13:44 +0100 Subject: [PATCH 2/5] fix(create-release-branch): address review of the grouped warnings --- .../src/core/release-steps.ts | 21 +++++++++++++------ .../src/modes/legacy/release-plan.test.ts | 2 +- .../src/project/project.test.ts | 4 ++-- .../src/project/project.ts | 4 ++-- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/create-release-branch/src/core/release-steps.ts b/packages/create-release-branch/src/core/release-steps.ts index ae76cf7..512f962 100644 --- a/packages/create-release-branch/src/core/release-steps.ts +++ b/packages/create-release-branch/src/core/release-steps.ts @@ -1,6 +1,7 @@ import type { Project } from '../project/project.js'; import type { CliOutput } from './cli-output.js'; import type { Mode, ModePlan } from './mode.js'; +import type { Notice } from './notices.js'; import { determineReleaseVersion, ensureReleaseBranch, @@ -19,6 +20,18 @@ import { updateYarnLockfile, } from './yarn-commands.js'; +/** + * Prints each notice as one warning with its items. + * + * @param output - Where to write user-facing messages. + * @param notices - The notices. + */ +function printNotices(output: CliOutput, notices: Notice[]): void { + for (const { title, items } of notices) { + output.warnList(title, items); + } +} + /** * Creates (or switches to) the release branch. On the first run, prepares the * project with the mode, prints the notices of the mode, and commits the @@ -54,9 +67,7 @@ export async function startRelease({ 'Updating changelogs', async () => await mode.prepare(), ); - for (const { title, items } of notices) { - output.warnList(title, items); - } + printNotices(output, notices); // `prepare` may change nothing, and the release branch still needs this // commit. await commitAllChanges( @@ -153,9 +164,7 @@ export async function applyRelease({ 'Applying the release', async () => await mode.apply(plan, newReleaseVersion), ); - for (const { title, items } of notices) { - output.warnList(title, items); - } + printNotices(output, notices); } /** diff --git a/packages/create-release-branch/src/modes/legacy/release-plan.test.ts b/packages/create-release-branch/src/modes/legacy/release-plan.test.ts index 7eccbe4..efa9f96 100644 --- a/packages/create-release-branch/src/modes/legacy/release-plan.test.ts +++ b/packages/create-release-branch/src/modes/legacy/release-plan.test.ts @@ -227,7 +227,7 @@ describe('release-plan-utils', () => { expect(notices).toStrictEqual([ { - title: '2 packages have no changelog, so it was not updated:', + title: '2 packages have no changelog to update:', items: ['root', 'b'], }, ]); diff --git a/packages/create-release-branch/src/project/project.test.ts b/packages/create-release-branch/src/project/project.test.ts index aeba88d..ce77357 100644 --- a/packages/create-release-branch/src/project/project.test.ts +++ b/packages/create-release-branch/src/project/project.test.ts @@ -451,7 +451,7 @@ describe('project', () => { expect(notices).toStrictEqual([ { - title: '1 package has no changelog, so it was not updated:', + title: '1 package has no changelog to update:', items: ['b'], }, { title: '2 changelogs have no updates to make:', items: ['a', 'c'] }, @@ -469,7 +469,7 @@ describe('project', () => { ]), ).toStrictEqual([ { - title: '2 packages have no changelog, so it was not updated:', + title: '2 packages have no changelog to update:', items: ['a', 'b'], }, { title: '1 changelog has no updates to make:', items: ['c'] }, diff --git a/packages/create-release-branch/src/project/project.ts b/packages/create-release-branch/src/project/project.ts index 9e4e3a2..4f98a69 100644 --- a/packages/create-release-branch/src/project/project.ts +++ b/packages/create-release-branch/src/project/project.ts @@ -258,8 +258,8 @@ export function buildChangelogNotices( { title: withoutChangelog.length === 1 - ? '1 package has no changelog, so it was not updated:' - : `${withoutChangelog.length} packages have no changelog, so it was not updated:`, + ? '1 package has no changelog to update:' + : `${withoutChangelog.length} packages have no changelog to update:`, items: withoutChangelog, }, { From 211d5e99d6b1735a747886eb2befed22934f5dce Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 18:14:37 +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 00d1156..d4cb848 100644 --- a/packages/create-release-branch/CHANGELOG.md +++ b/packages/create-release-branch/CHANGELOG.md @@ -12,7 +12,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 ([#160](https://github.com/MetaMask/release-tools/pull/160)) -- Group repeated warnings (packages without a tag, changelogs without updates) into one warning with a list of packages ([#](https://github.com/MetaMask/release-tools/pull/)) +- Group repeated warnings (packages without a tag, changelogs without updates) into one warning with a list of packages ([#162](https://github.com/MetaMask/release-tools/pull/162)) ### Fixed From 991c6b385fa74ed9f97ba58b1d534c1332a6f814 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 5 Nov 2026 18:42:11 +0100 Subject: [PATCH 4/5] fix(create-release-branch): address the final review --- .../src/core/cli-output.test.ts | 13 +++++++ .../src/core/cli-output.ts | 7 +++- .../src/modes/legacy/release-plan.test.ts | 37 ++++++++++++++++--- .../src/modes/legacy/release-plan.ts | 6 ++- 4 files changed, 55 insertions(+), 8 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 30eb54c..a9fd77e 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/packages/create-release-branch/src/core/cli-output.test.ts @@ -142,6 +142,19 @@ describe('cli-output', () => { expect(stdout.data()).toStrictEqual([]); }); + it('warnList indents every line of a multi-line item', () => { + const stdout = new MockWritable(); + const stderr = new MockWritable(); + + createCliOutput({ stdout, stderr }).warnList('Some title:', [ + 'Line 1\nLine 2', + ]); + + expect(textOf(stderr)).toBe( + `${logLine(S_WARN, 'Some title:')}${S_BAR} Line 1\n${S_BAR} Line 2\n`, + ); + }); + it('writes stderr without colors when only stdout is a color terminal', () => { const stdout = buildTerminalStream(); const stderr = 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 a545aba..b30ad6b 100644 --- a/packages/create-release-branch/src/core/cli-output.ts +++ b/packages/create-release-branch/src/core/cli-output.ts @@ -211,7 +211,12 @@ export function createCliOutput({ }, warnList(title, items) { log.warn( - [title, ...items.map((item) => ` ${format.dim(item)}`)].join('\n'), + [ + title, + ...items + .flatMap((item) => item.split('\n')) + .map((line) => ` ${format.dim(line)}`), + ].join('\n'), toStderr, ); }, diff --git a/packages/create-release-branch/src/modes/legacy/release-plan.test.ts b/packages/create-release-branch/src/modes/legacy/release-plan.test.ts index efa9f96..bb2433c 100644 --- a/packages/create-release-branch/src/modes/legacy/release-plan.test.ts +++ b/packages/create-release-branch/src/modes/legacy/release-plan.test.ts @@ -201,15 +201,15 @@ describe('release-plan-utils', () => { it('returns the packages without a changelog as a notice', async () => { const releasePlan = { newVersion: '2.0.0', - packages: ['root', 'a', 'b'].map((name) => ({ + packages: ['a', 'b', 'c'].map((name) => ({ package: buildMockPackage(name), newVersion: '2.0.0', })), }; const results = { - root: 'no-changelog', - a: 'updated', - b: 'no-changelog', + a: 'no-changelog', + b: 'updated', + c: 'no-changelog', } as const; vi.spyOn(packageUtils, 'updatePackage').mockImplementation( async ({ packageReleasePlan }) => @@ -228,7 +228,34 @@ describe('release-plan-utils', () => { expect(notices).toStrictEqual([ { title: '2 packages have no changelog to update:', - items: ['root', 'b'], + items: ['a', 'c'], + }, + ]); + }); + + it('does not list the root package as having no changelog', async () => { + const project = buildMockProject({ + rootPackage: buildMockPackage('monorepo'), + }); + const releasePlan = { + newVersion: '2.0.0', + packages: [project.rootPackage, buildMockPackage('a')].map((pkg) => ({ + package: pkg, + newVersion: '2.0.0', + })), + }; + vi.spyOn(packageUtils, 'updatePackage').mockResolvedValue('no-changelog'); + + const notices = await executeReleasePlan( + project, + releasePlan, + 'prettier', + ); + + expect(notices).toStrictEqual([ + { + title: '1 package has no changelog to update:', + items: ['a'], }, ]); }); diff --git a/packages/create-release-branch/src/modes/legacy/release-plan.ts b/packages/create-release-branch/src/modes/legacy/release-plan.ts index ea06e6e..864c316 100644 --- a/packages/create-release-branch/src/modes/legacy/release-plan.ts +++ b/packages/create-release-branch/src/modes/legacy/release-plan.ts @@ -109,7 +109,8 @@ export async function planRelease({ * @param releasePlan - Compiled instructions on how exactly to update the * project in order to prepare a new release. * @param formatter - The formatter to use for formatting the changelog. - * @returns The packages without a changelog as notices. + * @returns The packages without a changelog as notices, leaving out the root + * package. */ export async function executeReleasePlan( project: Project, @@ -121,6 +122,7 @@ export async function executeReleasePlan( const { name } = workspaceReleasePlan.package.validatedManifest; debug(`Updating package ${name}...`); return { + isRoot: workspaceReleasePlan.package === project.rootPackage, name, result: await updatePackage({ project, @@ -130,5 +132,5 @@ export async function executeReleasePlan( }; }), ); - return buildChangelogNotices(results); + return buildChangelogNotices(results.filter(({ isRoot }) => !isRoot)); } From 6c174740a4af68732db231798665aa64c306d11f Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Tue, 10 Nov 2026 15:41:29 +0100 Subject: [PATCH 5/5] test(create-release-branch): use the shared helper in the warning list tests --- packages/create-release-branch/src/core/cli-output.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 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 a9fd77e..574c8a2 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/packages/create-release-branch/src/core/cli-output.test.ts @@ -133,10 +133,10 @@ describe('cli-output', () => { ['a'], ); - expect(textOf(plainStderr)).toBe( + expect(writtenText(plainStderr)).toBe( `${logLine(S_WARN, 'Some title:')}${S_BAR} a\n${S_BAR} b\n`, ); - expect(textOf(styledStderr)).toContain( + expect(writtenText(styledStderr)).toContain( ` ${styleText('dim', 'a', { validateStream: false })}`, ); expect(stdout.data()).toStrictEqual([]); @@ -150,7 +150,7 @@ describe('cli-output', () => { 'Line 1\nLine 2', ]); - expect(textOf(stderr)).toBe( + expect(writtenText(stderr)).toBe( `${logLine(S_WARN, 'Some title:')}${S_BAR} Line 1\n${S_BAR} Line 2\n`, ); });