diff --git a/src/functional.test.ts b/src/functional.test.ts index 7e6d85f..56ebd54 100644 --- a/src/functional.test.ts +++ b/src/functional.test.ts @@ -77,13 +77,9 @@ describe('create-release-branch (functional)', () => { }, }); - expect(await environment.readJsonFile('package.json')).toStrictEqual({ + expect(await environment.readJsonFile('package.json')).toMatchObject({ name: '@scope/monorepo', version: '2.0.0', - private: true, - workspaces: ['packages/*'], - scripts: { foo: 'bar' }, - packageManager: 'yarn@3.2.1', }); expect( await environment.readJsonFileWithinPackage('a', 'package.json'), @@ -190,13 +186,9 @@ describe('create-release-branch (functional)', () => { }, }); - expect(await environment.readJsonFile('package.json')).toStrictEqual({ + expect(await environment.readJsonFile('package.json')).toMatchObject({ name: '@scope/monorepo', version: '1.1.0', - private: true, - workspaces: ['packages/*'], - scripts: { foo: 'bar' }, - packageManager: 'yarn@3.2.1', }); expect( await environment.readJsonFileWithinPackage('a', 'package.json'), @@ -945,12 +937,9 @@ __metadata: }, }); - expect(await environment.readJsonFile('package.json')).toStrictEqual({ + expect(await environment.readJsonFile('package.json')).toMatchObject({ name: '@scope/monorepo', version: '2.0.0', - private: true, - workspaces: ['packages/*'], - packageManager: 'yarn@3.2.1', }); expect( await environment.readJsonFileWithinPackage('a', 'package.json'), diff --git a/src/misc-utils.test.ts b/src/misc-utils.test.ts index be297b8..cccf8a6 100644 --- a/src/misc-utils.test.ts +++ b/src/misc-utils.test.ts @@ -9,6 +9,7 @@ import { runCommand, getStdoutFromCommand, getLinesFromCommand, + convertToHttpsGitHubRepositoryUrl, } from './misc-utils.js'; jest.mock('which'); @@ -211,4 +212,49 @@ describe('misc-utils', () => { expect(lines).toStrictEqual([' line 1', 'line 2', ' line 3 ']); }); }); + + describe('convertToHttpsRepositoryUrl', () => { + it('returns the URL of the "origin" remote of the given repo if it looks like a HTTPS public GitHub repo URL', () => { + expect( + convertToHttpsGitHubRepositoryUrl( + 'https://github.com/example-org/example-repo', + ), + ).toBe('https://github.com/example-org/example-repo'); + }); + + it('lops ".git" off from the HTTPS public GitHub repo URL', () => { + expect( + convertToHttpsGitHubRepositoryUrl( + 'https://github.com/example-org/example-repo.git', + ), + ).toBe('https://github.com/example-org/example-repo'); + }); + + it('converts an SSH GitHub repo URL into an HTTPS URL (without the trailing ".git")', () => { + expect( + convertToHttpsGitHubRepositoryUrl( + 'git@github.com:example-org/example-repo.git', + ), + ).toBe('https://github.com/example-org/example-repo'); + }); + + it.each([ + 'foo', + 'http://github.com/example-org', + 'https://github.com/example-org', + 'http://github.com/example-org/example-repo', + 'https://github.comzzzz/example-org/example-repo', + 'https://gitbar.foo/example-org/example-repo', + 'git@github.com:example-org', + 'git@gitbar.foo:example-org/example-repo.git', + 'git@github.com:example-org/example-repo.foo', + ])( + 'throws if the URL is in an invalid format such as "%s"', + (repositoryUrl) => { + expect(() => convertToHttpsGitHubRepositoryUrl(repositoryUrl)).toThrow( + `Unrecognized repository URL: ${repositoryUrl}`, + ); + }, + ); + }); }); diff --git a/src/misc-utils.ts b/src/misc-utils.ts index 4cd32c2..0c1e777 100644 --- a/src/misc-utils.ts +++ b/src/misc-utils.ts @@ -15,6 +15,20 @@ export { isObject }; */ export const debug = createDebug('create-release-branch:impl'); +/** + * Matches URLs in the formats: + * + * - "https://github.com/OrganizationName/RepoName" + * - "https://github.com/OrganizationName/RepoName.git" + */ +const HTTPS_GITHUB_URL_REGEX = + /^https:\/\/github\.com\/(.+?)\/(.+?)(?:\.git)?$/u; + +/** + * Matches a URL in the format "git@github.com/OrganizationName/RepoName.git". + */ +const SSH_GITHUB_URL_REGEX = /^git@github\.com:(.+?)\/(.+?)\.git$/u; + /** * Type guard for determining whether the given value is an instance of Error. * For errors generated via `fs.promises`, `error instanceof Error` won't work, @@ -172,3 +186,28 @@ export async function getLinesFromCommand( const { stdout } = await execa(command, args, options); return stdout.split('\n').filter((value) => value !== ''); } + +/** + * Converts the given GitHub repository URL to its HTTPS version. + * + * A "GitHub repository URL" looks like one of: + * + * - https://github.com/OrganizationName/RepositoryName + * - git@github.com:OrganizationName/RepositoryName.git + * + * If the URL does not match either of these patterns, an error is thrown. + * + * @param url - The URL to convert. + * @returns The HTTPS URL of the repository, e.g. + * `https://github.com/OrganizationName/RepositoryName`. + */ +export function convertToHttpsGitHubRepositoryUrl(url: string): string { + const match = + url.match(HTTPS_GITHUB_URL_REGEX) ?? url.match(SSH_GITHUB_URL_REGEX); + + if (match) { + return `https://github.com/${match[1]}/${match[2]}`; + } + + throw new Error(`Unrecognized repository URL: ${url}`); +} diff --git a/src/project.test.ts b/src/project.test.ts index 517a8ea..8fc81f7 100644 --- a/src/project.test.ts +++ b/src/project.test.ts @@ -3,13 +3,15 @@ import path from 'path'; import { when } from 'jest-when'; import { SemVer } from 'semver'; import * as actionUtils from '@metamask/action-utils'; -import { withSandbox } from '../tests/helpers.js'; +import { withProtectedProcessEnv, withSandbox } from '../tests/helpers.js'; import { buildMockPackage, buildMockProject, createNoopWriteStream, } from '../tests/unit/helpers.js'; +import * as miscUtils from './misc-utils.js'; import { + getValidRepositoryUrl, readProject, restoreChangelogsForSkippedPackages, updateChangelogsForChangedPackages, @@ -42,6 +44,11 @@ describe('project', () => { validatedManifest: { workspaces: ['packages/a', 'packages/subpackages/*'], }, + unvalidatedManifest: { + repository: { + url: projectRepositoryUrl, + }, + }, }, ); const workspacePackages = { @@ -59,9 +66,6 @@ describe('project', () => { }; const projectTagNames = ['tag1', 'tag2', 'tag3']; const stderr = createNoopWriteStream(); - when(jest.spyOn(repoModule, 'getRepositoryHttpsUrl')) - .calledWith(projectDirectoryPath) - .mockResolvedValue(projectRepositoryUrl); when(jest.spyOn(repoModule, 'getTagNames')) .calledWith(projectDirectoryPath) .mockResolvedValue(projectTagNames); @@ -126,6 +130,71 @@ describe('project', () => { }); }); }); + + describe('getValidRepositoryUrl', () => { + describe('if the `npm_package_repository_url` environment variable is set', () => { + it('returns the HTTPS version of this URL', async () => { + await withProtectedProcessEnv(async () => { + process.env.npm_package_repository_url = + 'git@github.com:example-org/example-repo.git'; + const packageManifest = {}; + const repositoryDirectoryPath = '/path/to/project'; + + expect( + await getValidRepositoryUrl( + packageManifest, + repositoryDirectoryPath, + ), + ).toBe('https://github.com/example-org/example-repo'); + }); + }); + }); + + describe('if package.json has a string "repository" field', () => { + it('returns the HTTPS version of this URL', async () => { + const packageManifest = { + repository: 'git@github.com:example-org/example-repo.git', + }; + const repositoryDirectoryPath = '/path/to/project'; + + expect( + await getValidRepositoryUrl(packageManifest, repositoryDirectoryPath), + ).toBe('https://github.com/example-org/example-repo'); + }); + }); + + describe('if package.json has an object "repository" field with a string "url" field', () => { + it('returns the HTTPS version of this URL', async () => { + const packageManifest = { + repository: { + url: 'git@github.com:example-org/example-repo.git', + }, + }; + const repositoryDirectoryPath = '/path/to/project'; + + expect( + await getValidRepositoryUrl(packageManifest, repositoryDirectoryPath), + ).toBe('https://github.com/example-org/example-repo'); + }); + }); + + describe('if package.json does not have a "repository" field', () => { + it('returns the HTTPS version of this URL', async () => { + const packageManifest = {}; + const repositoryDirectoryPath = '/path/to/project'; + when(jest.spyOn(miscUtils, 'getStdoutFromCommand')) + .calledWith('git', ['config', '--get', 'remote.origin.url'], { + cwd: repositoryDirectoryPath, + }) + .mockResolvedValue('git@github.com:example-org/example-repo.git'); + + expect( + await getValidRepositoryUrl(packageManifest, repositoryDirectoryPath), + ).toBe('https://github.com/example-org/example-repo'); + }); + }); + }); + describe('restoreChangelogsForSkippedPackages', () => { it('should reset changelog for packages with changes not included in release', async () => { const project = buildMockProject({ diff --git a/src/project.ts b/src/project.ts index 0617256..4d6ef2c 100644 --- a/src/project.ts +++ b/src/project.ts @@ -1,6 +1,7 @@ import { WriteStream } from 'fs'; import { resolve } from 'path'; import { getWorkspaceLocations } from '@metamask/action-utils'; +import { isPlainObject } from '@metamask/utils'; import { WriteStreamLike, fileExists } from './fs.js'; import { Package, @@ -8,10 +9,17 @@ import { readMonorepoWorkspacePackage, updatePackageChangelog, } from './package.js'; -import { getRepositoryHttpsUrl, getTagNames, restoreFiles } from './repo.js'; +import { getTagNames, restoreFiles } from './repo.js'; import { SemVer } from './semver.js'; -import { PackageManifestFieldNames } from './package-manifest.js'; +import { + PackageManifestFieldNames, + UnvalidatedPackageManifest, +} from './package-manifest.js'; import { ReleaseSpecification } from './release-specification.js'; +import { + convertToHttpsGitHubRepositoryUrl, + getStdoutFromCommand, +} from './misc-utils.js'; /** * The release version of the root package of a monorepo extracted from its @@ -83,13 +91,16 @@ export async function readProject( projectDirectoryPath: string, { stderr }: { stderr: WriteStreamLike }, ): Promise { - const repositoryUrl = await getRepositoryHttpsUrl(projectDirectoryPath); const tagNames = await getTagNames(projectDirectoryPath); const rootPackage = await readMonorepoRootPackage({ packageDirectoryPath: projectDirectoryPath, projectDirectoryPath, projectTagNames: tagNames, }); + const repositoryUrl = await getValidRepositoryUrl( + rootPackage.unvalidatedManifest, + projectDirectoryPath, + ); const releaseVersion = examineReleaseVersion( rootPackage.validatedManifest.version, ); @@ -132,6 +143,62 @@ export async function readProject( }; } +/** + * Returns the https-prefixed GitHub repository URL for the project, so that we + * know how to construct links to releases inside of the changelog. + * + * This URL is obtained by looking at the following sources, in this order: + * + * - `process.env.npm_package_repository_url` (to support MetaMask projects + * which still use NPM or Yarn 1.x) + * - The `repository` field inside of `package.json` (to support most MetaMask + * projects) + * - The URL of the "origin" remote (to support newer projects which for some + * reason do not have a "repository" field) + * + * Note that there is a function in `@metamask/auto-changelog` that offers a + * similar capability, and in fact, this function should match it as closely as + * possible so that when changelogs are updated they also pass + * `@metamask/auto-changelog`'s validation step. + * + * @param packageManifest - The manifest for the package that represents a + * project. + * @param repositoryDirectoryPath - The path to the repository for the project. + * @returns The HTTPS URL of the repository, e.g. + * `https://github.com/OrganizationName/RepositoryName`. + * @throws If the URL obtained via `package.json` or the "origin" remote is in + * in invalid format. + */ +export async function getValidRepositoryUrl( + packageManifest: UnvalidatedPackageManifest, + repositoryDirectoryPath: string, +): Promise { + // Set automatically by NPM or Yarn 1.x + const npmPackageRepositoryUrl = process.env.npm_package_repository_url; + + if (npmPackageRepositoryUrl) { + return convertToHttpsGitHubRepositoryUrl(npmPackageRepositoryUrl); + } + + if (typeof packageManifest.repository === 'string') { + return convertToHttpsGitHubRepositoryUrl(packageManifest.repository); + } + + if ( + isPlainObject(packageManifest.repository) && + typeof packageManifest.repository.url === 'string' + ) { + return convertToHttpsGitHubRepositoryUrl(packageManifest.repository.url); + } + + const gitConfigUrl = await getStdoutFromCommand( + 'git', + ['config', '--get', 'remote.origin.url'], + { cwd: repositoryDirectoryPath }, + ); + return convertToHttpsGitHubRepositoryUrl(gitConfigUrl); +} + /** * Updates the changelog files of all packages that have changes since latest release to include those changes. * diff --git a/src/repo.test.ts b/src/repo.test.ts index 6754646..c9c81a2 100644 --- a/src/repo.test.ts +++ b/src/repo.test.ts @@ -1,6 +1,5 @@ import { when } from 'jest-when'; import { - getRepositoryHttpsUrl, commitAllChanges, getTagNames, hasChangesInDirectorySinceGitTag, @@ -13,71 +12,6 @@ import * as miscUtils from './misc-utils.js'; jest.mock('./misc-utils'); describe('repo', () => { - describe('getRepositoryHttpsUrl', () => { - it('returns the URL of the "origin" remote of the given repo if it looks like a HTTPS public GitHub repo URL', async () => { - const repositoryDirectoryPath = '/path/to/project'; - when(jest.spyOn(miscUtils, 'getStdoutFromCommand')) - .calledWith('git', ['config', '--get', 'remote.origin.url'], { - cwd: repositoryDirectoryPath, - }) - .mockResolvedValue('https://github.com/foo'); - - expect(await getRepositoryHttpsUrl(repositoryDirectoryPath)).toBe( - 'https://github.com/foo', - ); - }); - - it('converts an SSH GitHub repo URL into an HTTPS URL', async () => { - const repositoryDirectoryPath = '/path/to/project'; - when(jest.spyOn(miscUtils, 'getStdoutFromCommand')) - .calledWith('git', ['config', '--get', 'remote.origin.url'], { - cwd: repositoryDirectoryPath, - }) - .mockResolvedValue('git@github.com:Foo/Bar.git'); - - expect(await getRepositoryHttpsUrl(repositoryDirectoryPath)).toBe( - 'https://github.com/Foo/Bar', - ); - }); - - it('throws if the URL of the "origin" remote is in an invalid format', async () => { - const repositoryDirectoryPath = '/path/to/project'; - when(jest.spyOn(miscUtils, 'getStdoutFromCommand')) - .calledWith('git', ['config', '--get', 'remote.origin.url'], { - cwd: repositoryDirectoryPath, - }) - .mockResolvedValueOnce('foo') - .mockResolvedValueOnce('http://github.com/Foo/Bar') - .mockResolvedValueOnce('https://gitbar.foo/Foo/Bar') - .mockResolvedValueOnce('git@gitbar.foo:Foo/Bar.git') - .mockResolvedValueOnce('git@github.com:Foo/Bar.foo'); - - await expect( - getRepositoryHttpsUrl(repositoryDirectoryPath), - ).rejects.toThrow('Unrecognized URL for git remote "origin": foo'); - await expect( - getRepositoryHttpsUrl(repositoryDirectoryPath), - ).rejects.toThrow( - 'Unrecognized URL for git remote "origin": http://github.com/Foo/Bar', - ); - await expect( - getRepositoryHttpsUrl(repositoryDirectoryPath), - ).rejects.toThrow( - 'Unrecognized URL for git remote "origin": https://gitbar.foo/Foo/Bar', - ); - await expect( - getRepositoryHttpsUrl(repositoryDirectoryPath), - ).rejects.toThrow( - 'Unrecognized URL for git remote "origin": git@gitbar.foo:Foo/Bar.git', - ); - await expect( - getRepositoryHttpsUrl(repositoryDirectoryPath), - ).rejects.toThrow( - 'Unrecognized URL for git remote "origin": git@github.com:Foo/Bar.foo', - ); - }); - }); - describe('commitAllChanges', () => { it('stages all files, and creates a new commit', async () => { const getStdoutFromCommandSpy = jest.spyOn( diff --git a/src/repo.ts b/src/repo.ts index cc5a47a..ec4074f 100644 --- a/src/repo.ts +++ b/src/repo.ts @@ -30,6 +30,8 @@ export async function runGitCommandWithin( * Runs a Git command within the given repository, obtaining the immediate * output. * + * @deprecated Simply use {@link getStdoutFromCommand} instead of this. It is + * just as easy to test! * @param repositoryDirectoryPath - The path to the repository directory. * @param commandName - The name of the Git command (e.g., "commit"). * @param commandArgs - The arguments to the command. @@ -123,52 +125,6 @@ async function getFilesChangedSince( ); } -/** - * Gets the HTTPS URL of the primary remote with which the given repository has - * been configured. Assumes that the git config `remote.origin.url` string - * matches one of: - * - * - https://github.com/OrganizationName/RepositoryName - * - git@github.com:OrganizationName/RepositoryName.git - * - * If the URL of the "origin" remote matches neither pattern, an error is - * thrown. - * - * @param repositoryDirectoryPath - The path to the repository directory. - * @returns The HTTPS URL of the repository, e.g. - * `https://github.com/OrganizationName/RepositoryName`. - */ -export async function getRepositoryHttpsUrl( - repositoryDirectoryPath: string, -): Promise { - const httpsPrefix = 'https://github.com'; - const sshPrefixRegex = /^git@github\.com:/u; - const sshPostfixRegex = /\.git$/u; - const gitConfigUrl = await getStdoutFromGitCommandWithin( - repositoryDirectoryPath, - 'config', - ['--get', 'remote.origin.url'], - ); - - if (gitConfigUrl.startsWith(httpsPrefix)) { - return gitConfigUrl; - } - - // Extracts "OrganizationName/RepositoryName" from - // "git@github.com:OrganizationName/RepositoryName.git" and returns the - // corresponding HTTPS URL. - if ( - gitConfigUrl.match(sshPrefixRegex) && - gitConfigUrl.match(sshPostfixRegex) - ) { - return `${httpsPrefix}/${gitConfigUrl - .replace(sshPrefixRegex, '') - .replace(sshPostfixRegex, '')}`; - } - - throw new Error(`Unrecognized URL for git remote "origin": ${gitConfigUrl}`); -} - /** * Commits all changes in a git repository with a specified commit message. * diff --git a/tests/functional/helpers/local-monorepo.ts b/tests/functional/helpers/local-monorepo.ts index f4cbd81..bd0338f 100644 --- a/tests/functional/helpers/local-monorepo.ts +++ b/tests/functional/helpers/local-monorepo.ts @@ -159,7 +159,7 @@ export default class LocalMonorepo< protected async afterCreate() { await super.afterCreate(); - await this.writeJsonFile('package.json', { + await this.updateJsonFile('package.json', { private: true, packageManager: 'yarn@3.2.1', }); diff --git a/tests/functional/helpers/local-repo.ts b/tests/functional/helpers/local-repo.ts index 42d70bb..cef7843 100644 --- a/tests/functional/helpers/local-repo.ts +++ b/tests/functional/helpers/local-repo.ts @@ -74,17 +74,20 @@ export default abstract class LocalRepo extends Repo { 'remote', 'add', 'origin', - 'https://github.com/example-org/example-repo', + 'https://github.com/Example-Org/example-repo.git', ]); await this.runCommand('git', [ 'config', `url.${this.#remoteRepoDirectoryPath}.insteadOf`, - 'https://github.com/example-org/example-repo', + 'https://github.com/Example-Org/example-repo.git', ]); await this.writeJsonFile('package.json', { name: this.getPackageName(), version: this.getPackageVersion(), + repository: { + url: 'https://github.com/example-org/example-repo.git', + }, }); await this.writeFile( diff --git a/tests/functional/helpers/with.ts b/tests/functional/helpers/with.ts index 5365917..f8cbe6a 100644 --- a/tests/functional/helpers/with.ts +++ b/tests/functional/helpers/with.ts @@ -1,37 +1,8 @@ -import { withSandbox } from '../../helpers.js'; +import { withProtectedProcessEnv, withSandbox } from '../../helpers.js'; import MonorepoEnvironment, { MonorepoEnvironmentOptions, } from './monorepo-environment.js'; -/** - * Runs the given function and ensures that even if `process.env` is changed - * during the function, it is restored afterward. - * - * @param callback - The function to call that presumably will change - * `process.env`. - * @returns Whatever the callback returns. - */ -export async function withProtectedProcessEnv(callback: () => Promise) { - const originalEnv = { ...process.env }; - - try { - return await callback(); - } finally { - const originalKeys = Object.keys(originalEnv); - const currentKeys = Object.keys(process.env); - - originalKeys.forEach((key) => { - process.env[key] = originalEnv[key]; - }); - - currentKeys - .filter((key) => !originalKeys.includes(key)) - .forEach((key) => { - delete process.env[key]; - }); - } -} - /** * Builds a monorepo project in a temporary directory, then calls the given * function with information about that project. diff --git a/tests/helpers.ts b/tests/helpers.ts index 55b764b..4debd2c 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -135,3 +135,32 @@ export function buildChangelog(variantContent: string): string { return `${invariantContent}\n${normalizeMultilineString(variantContent)}`; } + +/** + * Runs the given function and ensures that even if `process.env` is changed + * during the function, it is restored afterward. + * + * @param callback - The function to call that presumably will change + * `process.env`. + * @returns Whatever the callback returns. + */ +export async function withProtectedProcessEnv(callback: () => Promise) { + const originalEnv = { ...process.env }; + + try { + return await callback(); + } finally { + const originalKeys = Object.keys(originalEnv); + const currentKeys = Object.keys(process.env); + + originalKeys.forEach((key) => { + process.env[key] = originalEnv[key]; + }); + + currentKeys + .filter((key) => !originalKeys.includes(key)) + .forEach((key) => { + delete process.env[key]; + }); + } +} diff --git a/tests/unit/helpers.ts b/tests/unit/helpers.ts index 0527b15..bace880 100644 --- a/tests/unit/helpers.ts +++ b/tests/unit/helpers.ts @@ -7,7 +7,10 @@ import { PackageManifestDependenciesFieldNames, PackageManifestFieldNames, } from '../../src/package-manifest.js'; -import type { ValidatedPackageManifest } from '../../src/package-manifest.js'; +import type { + UnvalidatedPackageManifest, + ValidatedPackageManifest, +} from '../../src/package-manifest.js'; import type { Project } from '../../src/project.js'; /** @@ -38,6 +41,7 @@ type MockPackageOverrides = Omit< Partial, PackageManifestFieldNames.Name | PackageManifestFieldNames.Version >; + unvalidatedManifest?: UnvalidatedPackageManifest; }; /** @@ -105,6 +109,7 @@ export function buildMockPackage( const { validatedManifest = {}, + unvalidatedManifest = {}, directoryPath = `/path/to/packages/${name}`, manifestPath = path.join(directoryPath, 'package.json'), changelogPath = path.join(directoryPath, 'CHANGELOG.md'), @@ -113,7 +118,7 @@ export function buildMockPackage( return { directoryPath, - unvalidatedManifest: {}, + unvalidatedManifest, validatedManifest: buildMockManifest({ ...validatedManifest, [PackageManifestFieldNames.Name]: name,