From 6bf8b36560448fe7e029ea5d993604ef15facfa2 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 3 Aug 2022 15:58:16 -0600 Subject: [PATCH 1/5] Discourage postponing release of changed packages When the tool generates a release spec, it will list all packages that have changed since their last release. Editing the release spec to remove a package so that it is no longer included in the release is potentially dangerous, because it means that any package which relies on that package that *is* included in the release could be broken in production. Since it's sometimes necessary to delay the release of a package, this commit changes the release spec validation step such that if it detects that a package that should be listed in the release spec *isn't*, it will throw an error, advising the user of the danger, yet provide a hidden option should the user really want to proceed. --- src/functional.test.ts | 127 +++++++++++-- src/release-specification.test.ts | 173 ++++++++++++++++-- src/release-specification.ts | 95 +++++++--- tests/functional/helpers/environment.ts | 7 + .../helpers/monorepo-environment.ts | 2 +- tests/helpers.ts | 19 ++ tests/setupAfterEnv.ts | 58 ++++++ 7 files changed, 429 insertions(+), 52 deletions(-) diff --git a/src/functional.test.ts b/src/functional.test.ts index 4be3f46..f0fa12d 100644 --- a/src/functional.test.ts +++ b/src/functional.test.ts @@ -32,11 +32,6 @@ describe('create-release-branch (functional)', () => { version: '1.2.3', directoryPath: 'packages/d', }, - e: { - name: '@scope/e', - version: '0.0.3', - directoryPath: 'packages/e', - }, }, workspaces: { '.': ['packages/*'], @@ -68,11 +63,6 @@ describe('create-release-branch (functional)', () => { foo: 'bar', }, }); - await environment.updateJsonFileWithinPackage('e', 'package.json', { - scripts: { - foo: 'bar', - }, - }); await environment.runTool({ releaseSpecification: { @@ -120,13 +110,6 @@ describe('create-release-branch (functional)', () => { version: '1.2.4', scripts: { foo: 'bar' }, }); - expect( - await environment.readJsonFileWithinPackage('e', 'package.json'), - ).toStrictEqual({ - name: '@scope/e', - version: '0.0.3', - scripts: { foo: 'bar' }, - }); }, ); }); @@ -289,5 +272,115 @@ describe('create-release-branch (functional)', () => { }, ); }); + + it('errors before making any changes if the edited release spec omits changed packages', async () => { + await withMonorepoProjectEnvironment( + { + packages: { + $root$: { + name: '@scope/monorepo', + version: '20220101.1.0', + directoryPath: '.', + }, + a: { + name: '@scope/a', + version: '1.0.0', + directoryPath: 'packages/a', + }, + b: { + name: '@scope/b', + version: '1.0.0', + directoryPath: 'packages/b', + }, + c: { + name: '@scope/c', + version: '1.0.0', + directoryPath: 'packages/c', + }, + d: { + name: '@scope/d', + version: '1.0.0', + directoryPath: 'packages/d', + }, + }, + workspaces: { + '.': ['packages/*'], + }, + }, + async (environment) => { + await expect( + environment.runTool({ + releaseSpecification: { + packages: { + a: 'major', + c: 'patch', + }, + }, + }), + ).toThrowExecaError( + ` +Error: Your release spec could not be processed due to the following issues: + +* The following packages, which have changed since their latest release, are missing. + + - @scope/b + - @scope/d + + Consider including them in the release spec so that any packages that rely on them won't break in production. + + If you are ABSOLUTELY SURE that this won't occur, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: + + packages: + "@scope/b": intentionally-skip + "@scope/d": 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. + +<> +<> +`.trim(), + { + replacements: [ + { + from: `${environment.tempDirectoryPath}/RELEASE_SPEC`, + to: '<>', + }, + ], + }, + ); + + expect(await environment.readJsonFile('package.json')).toStrictEqual({ + name: '@scope/monorepo', + version: '20220101.1.0', + private: true, + workspaces: ['packages/*'], + }); + expect( + await environment.readJsonFileWithinPackage('a', 'package.json'), + ).toStrictEqual({ + name: '@scope/a', + version: '1.0.0', + }); + expect( + await environment.readJsonFileWithinPackage('b', 'package.json'), + ).toStrictEqual({ + name: '@scope/b', + version: '1.0.0', + }); + expect( + await environment.readJsonFileWithinPackage('c', 'package.json'), + ).toStrictEqual({ + name: '@scope/c', + version: '1.0.0', + }); + expect( + await environment.readJsonFileWithinPackage('d', 'package.json'), + ).toStrictEqual({ + name: '@scope/d', + version: '1.0.0', + }); + }, + ); + }); }); }); diff --git a/src/release-specification.test.ts b/src/release-specification.test.ts index 79138a2..6cc02bd 100644 --- a/src/release-specification.test.ts +++ b/src/release-specification.test.ts @@ -47,7 +47,7 @@ describe('release-specification', () => { ` # The following is a list of packages in monorepo. # Please indicate the packages for which you want to create a new release -# by updating "null" (which does nothing) to one of the following: +# by updating "null" to one of the following: # # - "major" (if you want to bump the major part of the package's version) # - "minor" (if you want to bump the minor part of the package's version) @@ -110,7 +110,7 @@ packages: ` # The following is a list of packages in monorepo. # Please indicate the packages for which you want to create a new release -# by updating "null" (which does nothing) to one of the following: +# by updating "null" to one of the following: # # - "major" (if you want to bump the major part of the package's version) # - "minor" (if you want to bump the minor part of the package's version) @@ -281,7 +281,7 @@ packages: }); }); - it('removes packages which have "null" as their version specifier', async () => { + it('removes packages from the release spec which have "null" as their version specifier', async () => { await withSandbox(async (sandbox) => { const project = buildMockProject({ workspacePackages: { @@ -320,6 +320,45 @@ packages: }); }); + it('removes packages from the release spec which have "intentionally-skip" as their version specifier', async () => { + await withSandbox(async (sandbox) => { + const project = buildMockProject({ + workspacePackages: { + a: buildMockPackage('a'), + b: buildMockPackage('b'), + c: buildMockPackage('c'), + }, + }); + const releaseSpecificationPath = path.join( + sandbox.directoryPath, + 'release-spec', + ); + await fs.promises.writeFile( + releaseSpecificationPath, + YAML.stringify({ + packages: { + a: 'major', + b: 'intentionally-skip', + c: 'patch', + }, + }), + ); + + const releaseSpecification = await validateReleaseSpecification( + project, + releaseSpecificationPath, + ); + + expect(releaseSpecification).toStrictEqual({ + packages: { + a: 'major', + c: 'patch', + }, + path: releaseSpecificationPath, + }); + }); + }); + it('throws if the release spec cannot be parsed as valid YAML', async () => { await withSandbox(async (sandbox) => { const project = buildMockProject(); @@ -411,8 +450,8 @@ packages: 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', + '\\* Line 2: "foo" is not a package in the project', + '\\* Line 3: "bar" is not a package in the project', ].join('\n'), 'u', ), @@ -448,9 +487,9 @@ packages: 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"', + '\\* 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"', + '\\* 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', @@ -487,9 +526,9 @@ packages: ` 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" +* 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" +* Line 3: "4.5.6" is not a valid version specifier for package "b" ("b" is already at version "4.5.6") `.trim(), ); @@ -524,10 +563,122 @@ Your release spec could not be processed due to the following issues: ` 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" +* 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" +* Line 3: "4.5.5" is not a valid version specifier for package "b" ("b" is at a greater version "4.5.6") +`.trim(), + ); + }); + }); + + it('throws if there are any packages not listed in the release spec which have changed', async () => { + await withSandbox(async (sandbox) => { + const project = buildMockProject({ + workspacePackages: { + a: buildMockPackage('a', { + hasChangesSinceLatestRelease: true, + }), + b: buildMockPackage('b', { + hasChangesSinceLatestRelease: true, + }), + c: buildMockPackage('c', { + hasChangesSinceLatestRelease: true, + }), + }, + }); + const releaseSpecificationPath = path.join( + sandbox.directoryPath, + 'release-spec', + ); + await fs.promises.writeFile( + releaseSpecificationPath, + YAML.stringify({ + packages: { + a: 'major', + }, + }), + ); + + await expect( + validateReleaseSpecification(project, releaseSpecificationPath), + ).rejects.toThrow( + ` +Your release spec could not be processed due to the following issues: + +* The following packages, which have changed since their latest release, are missing. + + - b + - c + + Consider including them in the release spec so that any packages that rely on them won't break in production. + + If you are ABSOLUTELY SURE that this won't occur, 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 + c: 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. + +${releaseSpecificationPath} +`.trim(), + ); + }); + }); + + it('throws if there are any packages listed in the release spec which have changed but their version specifiers are null', async () => { + await withSandbox(async (sandbox) => { + const project = buildMockProject({ + workspacePackages: { + a: buildMockPackage('a', { + hasChangesSinceLatestRelease: true, + }), + b: buildMockPackage('b', { + hasChangesSinceLatestRelease: true, + }), + c: buildMockPackage('c', { + hasChangesSinceLatestRelease: true, + }), + }, + }); + const releaseSpecificationPath = path.join( + sandbox.directoryPath, + 'release-spec', + ); + await fs.promises.writeFile( + releaseSpecificationPath, + YAML.stringify({ + packages: { + a: 'major', + b: null, + c: null, + }, + }), + ); + + await expect( + validateReleaseSpecification(project, releaseSpecificationPath), + ).rejects.toThrow( + ` +Your release spec could not be processed due to the following issues: + +* The following packages, which have changed since their latest release, are missing. + + - b + - c + + Consider including them in the release spec so that any packages that rely on them won't break in production. + + If you are ABSOLUTELY SURE that this won't occur, 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 + c: 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. + +${releaseSpecificationPath} `.trim(), ); }); diff --git a/src/release-specification.ts b/src/release-specification.ts index 976672e..6f6630e 100644 --- a/src/release-specification.ts +++ b/src/release-specification.ts @@ -40,6 +40,9 @@ export interface ReleaseSpecification { path: string; } +const SKIP_PACKAGE_DIRECTIVE = null; +const INTENTIONALLY_SKIP_PACKAGE_DIRECTIVE = 'intentionally-skip'; + /** * Generates a skeleton for a release specification, which describes how a * project should be updated. @@ -68,7 +71,7 @@ export async function generateReleaseSpecificationTemplateForMonorepo({ const instructions = ` # The following is a list of packages in ${rootPackage.validatedManifest.name}. # Please indicate the packages for which you want to create a new release -# by updating "null" (which does nothing) to one of the following: +# by updating "null" to one of the following: # # - "major" (if you want to bump the major part of the package's version) # - "minor" (if you want to bump the minor part of the package's version) @@ -90,7 +93,7 @@ ${afterEditingInstructions} } const packages = changedWorkspacePackages.reduce((obj, pkg) => { - return { ...obj, [pkg.validatedManifest.name]: null }; + return { ...obj, [pkg.validatedManifest.name]: SKIP_PACKAGE_DIRECTIVE }; }, {}); return [instructions, YAML.stringify({ packages })].join('\n\n'); @@ -162,9 +165,6 @@ export async function validateReleaseSpecification( project: Project, releaseSpecificationPath: string, ): Promise { - const workspacePackageNames = Object.values(project.workspacePackages).map( - (pkg) => pkg.validatedManifest.name, - ); const releaseSpecificationContents = await readFile(releaseSpecificationPath); const indexOfFirstUsableLine = releaseSpecificationContents .split('\n') @@ -204,14 +204,51 @@ export async function validateReleaseSpecification( throw new Error(message); } - const errors: { message: string | string[]; lineNumber: number }[] = []; + const errors: { message: string | string[]; lineNumber?: number }[] = []; + + const changedPackageNames = Object.keys(project.workspacePackages).filter( + (packageName) => + project.workspacePackages[packageName].hasChangesSinceLatestRelease, + ); + const missingChangedPackageNames = changedPackageNames.filter( + (packageName) => + !hasProperty(unvalidatedReleaseSpecification.packages, packageName) || + unvalidatedReleaseSpecification.packages[packageName] === null, + ); + + if (missingChangedPackageNames.length > 0) { + errors.push({ + message: [ + 'The following packages, which have changed since their latest release, are missing.', + missingChangedPackageNames + .map((packageName) => ` - ${packageName}`) + .join('\n'), + " Consider including them in the release spec so that any packages that rely on them won't break in production.", + ` If you are ABSOLUTELY SURE that this won't occur, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:`, + YAML.stringify({ + packages: missingChangedPackageNames.reduce((object, packageName) => { + return { + ...object, + [packageName]: INTENTIONALLY_SKIP_PACKAGE_DIRECTIVE, + }; + }, {}), + }) + .trim() + .split('\n') + .map((line) => ` ${line}`) + .join('\n'), + ].join('\n\n'), + }); + } + Object.keys(unvalidatedReleaseSpecification.packages).forEach( (packageName, index) => { - const versionSpecifier = + const versionSpecifierOrDirective = unvalidatedReleaseSpecification.packages[packageName]; const lineNumber = indexOfFirstUsableLine + index + 2; + const pkg = project.workspacePackages[packageName]; - if (!workspacePackageNames.includes(packageName)) { + if (pkg === undefined) { errors.push({ message: `${JSON.stringify( packageName, @@ -221,14 +258,15 @@ export async function validateReleaseSpecification( } if ( - versionSpecifier !== null && - !hasProperty(IncrementableVersionParts, versionSpecifier) && - !isValidSemver(versionSpecifier) + versionSpecifierOrDirective !== SKIP_PACKAGE_DIRECTIVE && + versionSpecifierOrDirective !== INTENTIONALLY_SKIP_PACKAGE_DIRECTIVE && + !hasProperty(IncrementableVersionParts, versionSpecifierOrDirective) && + !isValidSemver(versionSpecifierOrDirective) ) { errors.push({ message: [ `${JSON.stringify( - versionSpecifier, + versionSpecifierOrDirective, )} is not a valid version specifier for package "${packageName}"`, `(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3")`, ], @@ -236,8 +274,8 @@ export async function validateReleaseSpecification( }); } - if (isValidSemver(versionSpecifier)) { - const comparison = new SemVer(versionSpecifier).compare( + if (isValidSemver(versionSpecifierOrDirective)) { + const comparison = new SemVer(versionSpecifierOrDirective).compare( project.workspacePackages[packageName].validatedManifest.version, ); @@ -245,9 +283,9 @@ export async function validateReleaseSpecification( errors.push({ message: [ `${JSON.stringify( - versionSpecifier, + versionSpecifierOrDirective, )} is not a valid version specifier for package "${packageName}"`, - `("${packageName}" is already at version "${versionSpecifier}")`, + `("${packageName}" is already at version "${versionSpecifierOrDirective}")`, ], lineNumber, }); @@ -255,7 +293,7 @@ export async function validateReleaseSpecification( errors.push({ message: [ `${JSON.stringify( - versionSpecifier, + versionSpecifierOrDirective, )} is not a valid version specifier for package "${packageName}"`, `("${packageName}" is at a greater version "${project.workspacePackages[packageName].validatedManifest.version}")`, ], @@ -271,7 +309,12 @@ export async function validateReleaseSpecification( 'Your release spec could not be processed due to the following issues:', errors .flatMap((error) => { - const itemPrefix = '- '; + const itemPrefix = '* '; + + if (error.lineNumber === undefined) { + return `${itemPrefix}${error.message}`; + } + const lineNumberPrefix = `Line ${error.lineNumber}: `; if (Array.isArray(error.message)) { @@ -295,26 +338,32 @@ export async function validateReleaseSpecification( const packages = Object.keys(unvalidatedReleaseSpecification.packages).reduce( (obj, packageName) => { - const versionSpecifier = + const versionSpecifierOrDirective = unvalidatedReleaseSpecification.packages[packageName]; - if (versionSpecifier) { + if ( + versionSpecifierOrDirective !== SKIP_PACKAGE_DIRECTIVE && + versionSpecifierOrDirective !== INTENTIONALLY_SKIP_PACKAGE_DIRECTIVE + ) { if ( Object.values(IncrementableVersionParts).includes( - versionSpecifier as any, + // Typecast: It doesn't matter what type versionSpecifierOrDirective + // is as we are checking for inclusion. + versionSpecifierOrDirective as any, ) ) { return { ...obj, // Typecast: We know what this is as we've checked it above. - [packageName]: versionSpecifier as IncrementableVersionParts, + [packageName]: + versionSpecifierOrDirective as IncrementableVersionParts, }; } return { ...obj, // Typecast: We know that this will safely parse. - [packageName]: semver.parse(versionSpecifier) as SemVer, + [packageName]: semver.parse(versionSpecifierOrDirective) as SemVer, }; } diff --git a/tests/functional/helpers/environment.ts b/tests/functional/helpers/environment.ts index c4a9ac6..30ceb33 100644 --- a/tests/functional/helpers/environment.ts +++ b/tests/functional/helpers/environment.ts @@ -1,3 +1,4 @@ +import path from 'path'; import LocalRepo from './local-repo'; import RemoteRepo from './remote-repo'; import Repo from './repo'; @@ -45,6 +46,8 @@ export default abstract class Environment { protected localRepo: SpecificLocalRepo; + tempDirectoryPath: string; + readJsonFile: SpecificLocalRepo['readJsonFile']; readFile: SpecificLocalRepo['readFile']; @@ -67,6 +70,10 @@ export default abstract class Environment { environmentDirectoryPath: directoryPath, }); this.localRepo = this.buildLocalRepo(options); + this.tempDirectoryPath = path.join( + this.localRepo.getWorkingDirectoryPath(), + 'tmp', + ); this.readJsonFile = this.localRepo.readJsonFile.bind(this.localRepo); this.readFile = this.localRepo.readFile.bind(this.localRepo); this.updateJsonFile = this.localRepo.updateJsonFile.bind(this.localRepo); diff --git a/tests/functional/helpers/monorepo-environment.ts b/tests/functional/helpers/monorepo-environment.ts index fc08246..b849d3c 100644 --- a/tests/functional/helpers/monorepo-environment.ts +++ b/tests/functional/helpers/monorepo-environment.ts @@ -134,7 +134,7 @@ cat "${releaseSpecificationPath}" > "$1" '--project-directory', this.localRepo.getWorkingDirectoryPath(), '--temp-directory', - path.join(this.localRepo.getWorkingDirectoryPath(), 'tmp'), + this.tempDirectoryPath, ], { env }, ); diff --git a/tests/helpers.ts b/tests/helpers.ts index db5bf23..394ee0e 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -4,6 +4,8 @@ import path from 'path'; import util from 'util'; import { nanoid } from 'nanoid'; import rimraf from 'rimraf'; +import type { ExecaError } from 'execa'; +import { hasProperty, isObject } from '@metamask/utils'; /** * A promisified version of `rimraf`. @@ -79,3 +81,20 @@ export async function withSandbox(fn: (sandbox: Sandbox) => any) { export function isErrorWithCode(error: unknown): error is { code: string } { return typeof error === 'object' && error !== null && 'code' in error; } + +/** + * Type guard for determining whether the given value is an error object + * produced by `execa`. + * + * @param error - The possible error object. + * @returns True or false, depending on the result. + */ +export function isExecaError(error: unknown): error is ExecaError { + return ( + isObject(error) && + hasProperty(error, 'message') && + hasProperty(error, 'shortMessage') && + hasProperty(error, 'isCanceled') && + hasProperty(error, 'exitCode') + ); +} diff --git a/tests/setupAfterEnv.ts b/tests/setupAfterEnv.ts index 1fc72ed..dae5bf9 100644 --- a/tests/setupAfterEnv.ts +++ b/tests/setupAfterEnv.ts @@ -1,3 +1,6 @@ +import type { ExecaReturnValue } from 'execa'; +import { isExecaError } from './helpers'; + declare global { // Using `namespace` here is okay because this is how the Jest types are // defined. @@ -5,6 +8,12 @@ declare global { namespace jest { interface Matchers { toResolve(): Promise; + toThrowExecaError( + message: string, + { + replacements, + }: { replacements: { from: string | RegExp; to: string }[] }, + ): Promise; } } } @@ -17,6 +26,8 @@ const UNRESOLVED = Symbol('timedOut'); // Store this in case it gets stubbed later const originalSetTimeout = global.setTimeout; const TIME_TO_WAIT_UNTIL_UNRESOLVED = 100; +const START = '▼▼▼ START ▼▼▼▼▼▼▼▼▼▼▼▼▼▼▼▼▼▼'; +const END = '▲▲▲ END ▲▲▲▲▲▲▲▲▲▲▲▲▲▲▲▲▲▲▲▲'; /** * Produces a sort of dummy promise which can be used in conjunction with a @@ -79,4 +90,51 @@ expect.extend({ pass: true, }; }, + + async toThrowExecaError( + promise: Promise>, + message: string, + { replacements }: { replacements: { from: string | RegExp; to: string }[] }, + ) { + try { + await promise; + return { + message: () => + 'Expected running the tool to fail with the given error message, but it did not.', + pass: false, + }; + } catch (error) { + if (isExecaError(error)) { + const stderr = [ + { + from: /^\s+at.+\)$/msu, + to: '<>', + }, + ...replacements, + ].reduce((string, { from, to }) => { + return string.replace(from, to); + }, error.stderr); + + if (stderr === message) { + return { + message: () => + 'Expected running the tool not to fail with the given error message, but it did', + pass: true, + }; + } + + return { + message: () => + `Expected running the tool to fail with:\n\n${START}\n${message}\n${END}\n\nBut it failed instead with:\n\n${START}\n${stderr}\n${END}`, + pass: false, + }; + } + + return { + message: () => + `Expected running the tool to fail with an error from \`execa\`, but it failed with:\n\n${error}`, + pass: false, + }; + } + }, }); From 6e94e30f40cbe1d5f6106b500ff7d9d4f23834ea Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 4 Oct 2022 14:33:48 -0600 Subject: [PATCH 2/5] Add functional tests for intentionally-skip --- src/functional.test.ts | 192 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 183 insertions(+), 9 deletions(-) diff --git a/src/functional.test.ts b/src/functional.test.ts index f0fa12d..4e2271f 100644 --- a/src/functional.test.ts +++ b/src/functional.test.ts @@ -279,27 +279,27 @@ describe('create-release-branch (functional)', () => { packages: { $root$: { name: '@scope/monorepo', - version: '20220101.1.0', + version: '1.0.0', directoryPath: '.', }, a: { name: '@scope/a', - version: '1.0.0', + version: '0.1.2', directoryPath: 'packages/a', }, b: { name: '@scope/b', - version: '1.0.0', + version: '1.1.4', directoryPath: 'packages/b', }, c: { name: '@scope/c', - version: '1.0.0', + version: '2.0.13', directoryPath: 'packages/c', }, d: { name: '@scope/d', - version: '1.0.0', + version: '1.2.3', directoryPath: 'packages/d', }, }, @@ -351,7 +351,7 @@ The release spec file has been retained for you to edit again and make the neces expect(await environment.readJsonFile('package.json')).toStrictEqual({ name: '@scope/monorepo', - version: '20220101.1.0', + version: '1.0.0', private: true, workspaces: ['packages/*'], }); @@ -359,26 +359,200 @@ The release spec file has been retained for you to edit again and make the neces await environment.readJsonFileWithinPackage('a', 'package.json'), ).toStrictEqual({ name: '@scope/a', - version: '1.0.0', + version: '0.1.2', }); expect( await environment.readJsonFileWithinPackage('b', 'package.json'), ).toStrictEqual({ name: '@scope/b', - version: '1.0.0', + version: '1.1.4', }); expect( await environment.readJsonFileWithinPackage('c', 'package.json'), ).toStrictEqual({ name: '@scope/c', - version: '1.0.0', + version: '2.0.13', }); expect( await environment.readJsonFileWithinPackage('d', 'package.json'), ).toStrictEqual({ name: '@scope/d', + version: '1.2.3', + }); + }, + ); + }); + + it('does not update the versions of any packages that have been tagged with intentionally-skip', async () => { + await withMonorepoProjectEnvironment( + { + packages: { + $root$: { + name: '@scope/monorepo', + version: '1.0.0', + directoryPath: '.', + }, + a: { + name: '@scope/a', + version: '0.1.2', + directoryPath: 'packages/a', + }, + b: { + name: '@scope/b', + version: '1.1.4', + directoryPath: 'packages/b', + }, + c: { + name: '@scope/c', + version: '2.0.13', + directoryPath: 'packages/c', + }, + d: { + name: '@scope/d', + version: '1.2.3', + directoryPath: 'packages/d', + }, + }, + workspaces: { + '.': ['packages/*'], + }, + }, + async (environment) => { + await environment.runTool({ + releaseSpecification: { + packages: { + a: 'major', + b: 'intentionally-skip', + c: 'patch', + d: 'intentionally-skip', + }, + }, + }); + + expect(await environment.readJsonFile('package.json')).toStrictEqual({ + name: '@scope/monorepo', + version: '2.0.0', + private: true, + workspaces: ['packages/*'], + }); + expect( + await environment.readJsonFileWithinPackage('a', 'package.json'), + ).toStrictEqual({ + name: '@scope/a', version: '1.0.0', }); + expect( + await environment.readJsonFileWithinPackage('b', 'package.json'), + ).toStrictEqual({ + name: '@scope/b', + version: '1.1.4', + }); + expect( + await environment.readJsonFileWithinPackage('c', 'package.json'), + ).toStrictEqual({ + name: '@scope/c', + version: '2.0.14', + }); + expect( + await environment.readJsonFileWithinPackage('d', 'package.json'), + ).toStrictEqual({ + name: '@scope/d', + version: '1.2.3', + }); + }, + ); + }); + + it('does not update the changelogs of any packages that have been tagged with --intentionally-skip', async () => { + await withMonorepoProjectEnvironment( + { + packages: { + $root$: { + name: '@scope/monorepo', + version: '1.0.0', + directoryPath: '.', + }, + a: { + name: '@scope/a', + version: '1.0.0', + directoryPath: 'packages/a', + }, + b: { + name: '@scope/b', + version: '1.0.0', + directoryPath: 'packages/b', + }, + }, + workspaces: { + '.': ['packages/*'], + }, + createInitialCommit: false, + }, + async (environment) => { + // Create an initial commit + await environment.writeFileWithinPackage( + 'a', + 'CHANGELOG.md', + buildChangelog(` + ## [Unreleased] + + [Unreleased]: https://github.com/example-org/example-repo + `), + ); + await environment.writeFileWithinPackage( + 'b', + 'CHANGELOG.md', + buildChangelog(` + ## [Unreleased] + + [Unreleased]: https://github.com/example-org/example-repo + `), + ); + await environment.createCommit('Initial commit'); + + // Create another commit that only changes "a" + await environment.writeFileWithinPackage( + 'a', + 'dummy.txt', + 'Some content', + ); + await environment.createCommit('Update "a"'); + + // Run the tool + await environment.runTool({ + releaseSpecification: { + packages: { + a: 'major', + b: 'intentionally-skip', + }, + }, + }); + + // Only "a" should get updated + expect( + await environment.readFileWithinPackage('a', 'CHANGELOG.md'), + ).toStrictEqual( + buildChangelog(` + ## [Unreleased] + + ## [2.0.0] + ### Uncategorized + - Update "a" + - Initial commit + + [Unreleased]: https://github.com/example-org/example-repo/compare/v2.0.0...HEAD + [2.0.0]: https://github.com/example-org/example-repo/releases/tag/v2.0.0 + `), + ); + expect( + await environment.readFileWithinPackage('b', 'CHANGELOG.md'), + ).toStrictEqual( + buildChangelog(` + ## [Unreleased] + + [Unreleased]: https://github.com/example-org/example-repo + `), + ); }, ); }); From 6f4b29f3bf854e4b9a6b8940a89553940f275825 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 4 Oct 2022 14:40:09 -0600 Subject: [PATCH 3/5] Explain what the stack trace (not backtrace) regex does --- src/functional.test.ts | 2 +- tests/setupAfterEnv.ts | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/functional.test.ts b/src/functional.test.ts index 4e2271f..df0e250 100644 --- a/src/functional.test.ts +++ b/src/functional.test.ts @@ -337,7 +337,7 @@ Error: Your release spec could not be processed due to the following issues: 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. <> -<> +<> `.trim(), { replacements: [ diff --git a/tests/setupAfterEnv.ts b/tests/setupAfterEnv.ts index dae5bf9..8837dbd 100644 --- a/tests/setupAfterEnv.ts +++ b/tests/setupAfterEnv.ts @@ -1,6 +1,18 @@ import type { ExecaReturnValue } from 'execa'; import { isExecaError } from './helpers'; +/** + * Matches a line that appears in a stack trace. For example, all of these + * should match: + * + * - " at c (/private/tmp/error.js:10:9)" + * - " at b (/private/tmp/error.js:6:3)" + * - " at a (/private/tmp/error.js:2:3)" + * - " at Object. (/private/tmp/error.js:13:1)" + * - " at Module._compile (node:internal/modules/cjs/loader:1105:14)" + */ +const STACK_TRACE_LINE_REGEX = /^\s+at.+\)$/msu; + declare global { // Using `namespace` here is okay because this is how the Jest types are // defined. @@ -107,8 +119,8 @@ expect.extend({ if (isExecaError(error)) { const stderr = [ { - from: /^\s+at.+\)$/msu, - to: '<>', + from: STACK_TRACE_LINE_REGEX, + to: '<>', }, ...replacements, ].reduce((string, { from, to }) => { From ad4fe11a54d0e1049679a9ca3c93940c44f3455f Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 4 Oct 2022 14:42:13 -0600 Subject: [PATCH 4/5] Fix name of variable --- tests/setupAfterEnv.ts | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/setupAfterEnv.ts b/tests/setupAfterEnv.ts index 8837dbd..afb799a 100644 --- a/tests/setupAfterEnv.ts +++ b/tests/setupAfterEnv.ts @@ -2,16 +2,19 @@ import type { ExecaReturnValue } from 'execa'; import { isExecaError } from './helpers'; /** - * Matches a line that appears in a stack trace. For example, all of these - * should match: + * Matches a series of lines that represent a stack trace (by looking for the + * first instance of "at" preceded by some whitespace and then looking for a + * final ")"). For example, this whole section should match: * - * - " at c (/private/tmp/error.js:10:9)" - * - " at b (/private/tmp/error.js:6:3)" - * - " at a (/private/tmp/error.js:2:3)" - * - " at Object. (/private/tmp/error.js:13:1)" - * - " at Module._compile (node:internal/modules/cjs/loader:1105:14)" + * ``` + * at c (/private/tmp/error.js:10:9) + * at b (/private/tmp/error.js:6:3) + * at a (/private/tmp/error.js:2:3) + * at Object. (/private/tmp/error.js:13:1) + * at Module._compile (node:internal/modules/cjs/loader:1105:14) + * ``` */ -const STACK_TRACE_LINE_REGEX = /^\s+at.+\)$/msu; +const STACK_TRACE_SECTION = /^\s+at.+\)$/msu; declare global { // Using `namespace` here is okay because this is how the Jest types are @@ -119,7 +122,7 @@ expect.extend({ if (isExecaError(error)) { const stderr = [ { - from: STACK_TRACE_LINE_REGEX, + from: STACK_TRACE_SECTION, to: '<>', }, ...replacements, From a908b0e600ae497c3a683cf5f1f4d6192879ce54 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 5 Oct 2022 11:47:47 -0600 Subject: [PATCH 5/5] Fix typo in test name Co-authored-by: Mark Stacey --- src/functional.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/functional.test.ts b/src/functional.test.ts index df0e250..4b7f51c 100644 --- a/src/functional.test.ts +++ b/src/functional.test.ts @@ -463,7 +463,7 @@ The release spec file has been retained for you to edit again and make the neces ); }); - it('does not update the changelogs of any packages that have been tagged with --intentionally-skip', async () => { + it('does not update the changelogs of any packages that have been tagged with intentionally-skip', async () => { await withMonorepoProjectEnvironment( { packages: {