From e18bde709dd3c599ea2cf39146951c260d521003 Mon Sep 17 00:00:00 2001 From: bcoe Date: Fri, 29 Jan 2021 14:37:42 -0800 Subject: [PATCH 1/9] feat(cli)!: refactor factory/CLI to be more testable refactor!: helpers in factory class renamed. refactor!: more options pulled to top level (some types changed in process) refactor!: GitHub release now uses "run" rather than "createRelease" to execute --- .../{release-pr-factory.js => factory.js} | 4 +- src/bin/release-please.ts | 86 ++++---- src/factory.ts | 72 +++++++ src/github-release.ts | 66 ++++-- src/github.ts | 190 ++++++++---------- src/index.ts | 26 ++- src/release-pr-factory.ts | 41 ---- src/release-pr.ts | 49 +---- test/cli.ts | 53 +++++ test/{release-pr-factory.ts => factory.ts} | 13 +- test/github-release.ts | 14 +- 11 files changed, 335 insertions(+), 279 deletions(-) rename __snapshots__/{release-pr-factory.js => factory.js} (88%) create mode 100644 src/factory.ts delete mode 100644 src/release-pr-factory.ts create mode 100644 test/cli.ts rename test/{release-pr-factory.ts => factory.ts} (96%) diff --git a/__snapshots__/release-pr-factory.js b/__snapshots__/factory.js similarity index 88% rename from __snapshots__/release-pr-factory.js rename to __snapshots__/factory.js index 5188a755a..d58e37db9 100644 --- a/__snapshots__/release-pr-factory.js +++ b/__snapshots__/factory.js @@ -1,4 +1,4 @@ -exports['ReleasePRFactory buildStatic returns an instance of a statically loaded releaser 1'] = ` +exports['factory ReleasePR returns instance of dynamically loaded releaser 1'] = ` [ [ "CHANGELOG.md", @@ -17,7 +17,7 @@ exports['ReleasePRFactory buildStatic returns an instance of a statically loaded ] ` -exports['ReleasePRFactory build returns instance of dynamically loaded releaser 1'] = ` +exports['factory buildStatic returns an instance of a statically loaded releaser 1'] = ` [ [ "CHANGELOG.md", diff --git a/src/bin/release-please.ts b/src/bin/release-please.ts index 898e923af..115669bad 100644 --- a/src/bin/release-please.ts +++ b/src/bin/release-please.ts @@ -16,9 +16,9 @@ import chalk = require('chalk'); import {coerceOption} from '../util/coerce-option'; -import {GitHubRelease, GitHubReleaseOptions} from '../github-release'; +import {GitHubReleaseOptions} from '../github-release'; import {ReleasePROptions} from '../release-pr'; -import {ReleasePRFactory} from '../release-pr-factory'; +import {factory} from '../factory'; import {getReleaserNames} from '../releasers'; import * as yargs from 'yargs'; @@ -41,34 +41,23 @@ interface YargsOptionsBuilder { option(opt: string, options: YargsOptions): YargsOptionsBuilder; } -const argv = yargs +export const parser = yargs .command( 'release-pr', 'create or update a PR representing the next release', (yargs: YargsOptionsBuilder) => { yargs - .option('package-name', { - describe: 'name of package release is being minted for', - demand: true, - }) .option('version-file', { describe: 'path to version file to update, e.g., version.rb', }) .option('last-package-version', { describe: 'last version # that package was released as', }) - .option('repo-url', { - describe: 'GitHub URL to generate release for', - demand: true, - }) .option('fork', { describe: 'should the PR be created from a fork', type: 'boolean', default: false, }) - .option('label', { - describe: 'label(s) to add to generated PR', - }) .option('snapshot', { describe: 'is it a snapshot (or pre-release) being generated?', type: 'boolean', @@ -78,10 +67,6 @@ const argv = yargs describe: 'default branch to open release PR against', type: 'string', }) - .option('path', { - describe: 'release from path other than root directory', - type: 'string', - }) .option('monorepo-tags', { describe: 'include library name in tags and release branches', type: 'boolean', @@ -89,8 +74,7 @@ const argv = yargs }); }, (argv: ReleasePROptions) => { - const rp = ReleasePRFactory.build(argv.releaseType, argv); - rp.run().catch(handleError); + factory.runCommand('release-pr', argv).catch(handleError); } ) .command( @@ -98,35 +82,15 @@ const argv = yargs 'create a GitHub release from a release PR', (yargs: YargsOptionsBuilder) => { yargs - .option('package-name', { - describe: 'name of package release is being minted for', - }) - .option('repo-url', { - describe: 'GitHub URL to generate release for', - demand: true, - }) .option('changelog-path', { default: 'CHANGELOG.md', describe: 'where can the CHANGELOG be found in the project?', }) - .option('label', { - default: 'autorelease: pending', - describe: 'label to remove from release PR', - }) .option('release-type', { describe: 'what type of repo is a release being created for?', choices: getReleaserNames(), default: 'node', }) - .option('path', { - describe: 'release from path other than root directory', - type: 'string', - }) - .option('monorepo-tags', { - describe: 'include library name in tags and release branches', - type: 'boolean', - default: false, - }) .option('draft', { describe: 'mark release as a draft. no tag is created but tag_name and ' + @@ -137,8 +101,7 @@ const argv = yargs }); }, (argv: GitHubReleaseOptions) => { - const gr = new GitHubRelease(argv); - gr.createRelease().catch(handleError); + factory.runCommand('github-release', argv).catch(handleError); } ) .middleware(_argv => { @@ -147,7 +110,6 @@ const argv = yargs // rather than being passed directly to the bin. if (argv.token) argv.token = coerceOption(argv.token); if (argv.apiUrl) argv.apiUrl = coerceOption(argv.apiUrl); - if (argv.proxyKey) argv.proxyKey = coerceOption(argv.proxyKey); }) .option('token', {describe: 'GitHub token with repo write permissions'}) .option('release-as', { @@ -170,10 +132,6 @@ const argv = yargs default: 'https://api.github.com', type: 'string', }) - .option('proxy-key', { - describe: 'key used by some GitHub proxies', - type: 'string', - }) .option('debug', { describe: 'print verbose errors (use only for local debugging).', default: false, @@ -183,9 +141,35 @@ const argv = yargs describe: '', type: 'string', }) + .option('label', { + default: 'autorelease: pending', + describe: 'label to remove from release PR', + }) + .option('repo-url', { + describe: 'GitHub URL to generate release for', + demand: true, + }) + .option('path', { + describe: 'release from path other than root directory', + type: 'string', + }) + .option('package-name', { + describe: 'name of package release is being minted for', + }) + .option('monorepo-tags', { + describe: 'include library name in tags and release branches', + type: 'boolean', + default: false, + }) .demandCommand(1) - .strict(true) - .parse(); + .strict(true); + +// Only run parser if executed with node bin, this allows +// for the parser to be easily tested: +let argv: yargs.Arguments; +if (require.main === module) { + argv = parser.parse(); +} // The errors returned by octokit currently contain the // request object, this contains information we don't want to @@ -194,7 +178,7 @@ const argv = yargs // the request object, don't do this in CI/CD). function handleError(err: ErrorObject) { let status = ''; - const command = argv._.length === 0 ? '' : argv._[0]; + const command = argv?._?.length ? argv._[0] : ''; if (err.status) { status = '' + err.status; } @@ -203,7 +187,7 @@ function handleError(err: ErrorObject) { `command ${command} failed${status ? ` with status ${status}` : ''}` ) ); - if (argv.debug) { + if (argv?.debug) { console.error('---------'); console.error(err.stack); } diff --git a/src/factory.ts b/src/factory.ts new file mode 100644 index 000000000..faadfd9b5 --- /dev/null +++ b/src/factory.ts @@ -0,0 +1,72 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Factory shared by GitHub Action and CLI for creating Release PRs +// and GitHub Releases: + +import {ReleasePR, ReleasePROptions} from './release-pr'; +import {RubyReleasePROptions} from './releasers/ruby'; +import { + GitHubReleaseOptions, + GitHubRelease, + ReleaseResponse, +} from './github-release'; +import {getReleasers} from './releasers'; + +export function runCommand( + command: string, + options: GitHubReleaseOptions | ReleasePROptions +): Promise { + if (isGitHubRelease(command, options)) { + return factory.run(githubRelease(options)); + } else { + return factory.run(releasePR(options)); + } +} + +function isGitHubRelease( + command: string, + options: GitHubReleaseOptions | ReleasePROptions +): options is GitHubReleaseOptions { + return command === 'github-release' && typeof options === 'object'; +} + +function run(runnable: ReleasePR | GitHubRelease) { + return runnable.run(); +} + +export function githubRelease(options: GitHubReleaseOptions): GitHubRelease { + return new GitHubRelease(options); +} + +export function releasePR(options: ReleasePROptions): ReleasePR { + const releaseOptions: ReleasePROptions | RubyReleasePROptions = options; + return new (factory.releasePRClass(options.releaseType))(releaseOptions); +} +export function releasePRClass(releaseType: string): typeof ReleasePR { + const releasers = getReleasers(); + const releaser = releasers[releaseType]; + if (!releaser) { + throw Error('unknown release type'); + } + return releaser; +} + +export const factory = { + githubRelease, + releasePR, + releasePRClass, + run, + runCommand, +}; diff --git a/src/github-release.ts b/src/github-release.ts index 3859c4123..3fbdeaf73 100644 --- a/src/github-release.ts +++ b/src/github-release.ts @@ -12,8 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {checkpoint, CheckpointType} from './util/checkpoint'; import {ReleasePRFactory} from './release-pr-factory'; +import chalk = require('chalk'); +import {SharedOptions, DEFAULT_LABELS} from './'; +import {checkpoint, CheckpointType} from './util/checkpoint'; +import {packageBranchPrefix} from './util/package-branch-prefix'; +import * as factory from './factory'; import {GitHub, OctokitAPIs} from './github'; import {parse} from 'semver'; import {ReleasePR} from './release-pr'; @@ -22,7 +26,7 @@ import {ReleasePR} from './release-pr'; const parseGithubRepoUrl = require('parse-github-repo-url'); const GITHUB_RELEASE_LABEL = 'autorelease: tagged'; -interface ReleaseResponse { +export interface ReleaseResponse { major: number; minor: number; patch: number; @@ -35,16 +39,7 @@ interface ReleaseResponse { draft: boolean; } -export interface GitHubReleaseOptions { - label: string; - repoUrl: string; - path?: string; - packageName?: string; - monorepoTags?: boolean; - token?: string; - apiUrl: string; - proxyKey?: string; - octokitAPIs?: OctokitAPIs; +export interface GitHubReleaseOptions extends SharedOptions { releaseType?: string; changelogPath?: string; draft?: boolean; @@ -61,15 +56,15 @@ export class GitHubRelease { packageName?: string; monorepoTags?: boolean; token?: string; - proxyKey?: string; releaseType?: string; draft: boolean; defaultBranch?: string; constructor(options: GitHubReleaseOptions) { this.apiUrl = options.apiUrl; - this.proxyKey = options.proxyKey; - this.labels = options.label.split(','); + this.labels = options.label + ? options.label.split(',') + : DEFAULT_LABELS.split(','); this.repoUrl = options.repoUrl; this.monorepoTags = options.monorepoTags; this.token = options.token; @@ -84,11 +79,17 @@ export class GitHubRelease { this.gh = this.gitHubInstance(options.octokitAPIs); } +<<<<<<< HEAD async createRelease(): Promise { +======= + async run(): Promise { + // Attempt to lookup the package name from a well known location, such + // as package.json, if none is provided: +>>>>>>> 5c4ff32 (feat(cli)!: refactor factory/CLI to be more testable) if (!this.packageName && this.releaseType) { - this.packageName = await ReleasePRFactory.class( - this.releaseType - ).lookupPackageName(this.gh, this.path); + this.packageName = await factory + .releasePRClass(this.releaseType) + .lookupPackageName(this.gh, this.path); } if (!this.packageName) { throw Error( @@ -120,6 +121,34 @@ export class GitHubRelease { checkpoint('Unable to build candidate', CheckpointType.Failure); return undefined; } +<<<<<<< HEAD +======= + const version = `v${gitHubReleasePR.version}`; + + checkpoint( + `found release branch ${chalk.green(version)} at ${chalk.green( + gitHubReleasePR.sha + )}`, + CheckpointType.Success + ); + + const changelogContents = ( + await this.gh.getFileContents(this.addPath(this.changelogPath)) + ).parsedContent; + const latestReleaseNotes = GitHubRelease.extractLatestReleaseNotes( + changelogContents, + version + ); + checkpoint( + `found release notes: \n---\n${chalk.grey(latestReleaseNotes)}\n---\n`, + CheckpointType.Success + ); + // Go uses '/' for a tag separator, rather than '-': + let tagSeparator = '-'; + if (this.releaseType) { + tagSeparator = factory.releasePRClass(this.releaseType).tagSeparator(); + } +>>>>>>> 5c4ff32 (feat(cli)!: refactor factory/CLI to be more testable) const release = await this.gh.createRelease( candidate.name, @@ -177,7 +206,6 @@ export class GitHubRelease { owner, repo, apiUrl: this.apiUrl, - proxyKey: this.proxyKey, octokitAPIs, }); } diff --git a/src/github.ts b/src/github.ts index a069d7502..f64b426bf 100644 --- a/src/github.ts +++ b/src/github.ts @@ -89,7 +89,6 @@ interface GitHubOptions { owner: string; repo: string; apiUrl?: string; - proxyKey?: string; octokitAPIs?: OctokitAPIs; } @@ -137,7 +136,6 @@ export class GitHub { owner: string; repo: string; apiUrl: string; - proxyKey?: string; constructor(options: GitHubOptions) { this.defaultBranch = options.defaultBranch; @@ -145,8 +143,6 @@ export class GitHub { this.owner = options.owner; this.repo = options.repo; this.apiUrl = options.apiUrl || 'https://api.github.com'; - this.proxyKey = options.proxyKey; - if (options.octokitAPIs === undefined) { this.octokit = new Octokit({ baseUrl: options.apiUrl, @@ -158,8 +154,7 @@ export class GitHub { 'user-agent': `release-please/${ require('../../package.json').version }`, - // some proxies do not require the token prefix. - Authorization: `${this.proxyKey ? '' : 'token '}${this.token}`, + Authorization: ` token ${this.token}`, }, }; this.request = request.defaults(defaults); @@ -180,11 +175,9 @@ export class GitHub { let opts = Object.assign({}, _opts); if (!probotMode) { opts = Object.assign(opts, { - url: `${this.apiUrl}/graphql${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, + url: `${this.apiUrl}/graphql`, headers: { - authorization: `${this.proxyKey ? '' : 'token '}${this.token}`, + authorization: `token ${this.token}`, 'content-type': 'application/vnd.github.v3+json', }, }); @@ -198,7 +191,7 @@ export class GitHub { } else { return Object.assign(opts, { headers: { - Authorization: `${this.proxyKey ? '' : 'token '}${this.token}`, + Authorization: `token ${this.token}`, }, }); } @@ -442,9 +435,7 @@ export class GitHub { async getTagSha(name: string): Promise { const refResponse = (await this.request( - `GET /repos/:owner/:repo/git/refs/tags/:name${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, + 'GET /repos/:owner/:repo/git/refs/tags/:name', { owner: this.owner, repo: this.repo, @@ -528,9 +519,7 @@ export class GitHub { for await (const response of this.octokit.paginate.iterator( this.decoratePaginateOpts({ method: 'GET', - url: `/repos/${this.owner}/${this.repo}/tags?per_page=100${ - this.proxyKey ? `&key=${this.proxyKey}` : '' - }`, + url: `/repos/${this.owner}/${this.repo}/tags?per_page=100`, }) )) { response.data.forEach((data: ReposListTagsResponseItems) => { @@ -661,32 +650,47 @@ export class GitHub { branchPrefix = branchPrefix?.endsWith('-') ? branchPrefix.replace(/-$/, '') : branchPrefix; - return await this.findMergedPullRequest( - baseBranch, - (mergedPullRequest: MergedGitHubPR) => { - // If labels specified, ensure the pull request has all the specified labels - if ( - labels.length > 0 && - !this.hasAllLabels(labels, mergedPullRequest.labels) - ) { - return false; - } - - const branchName = BranchName.parse(mergedPullRequest.headRefName); - if (!branchName) { - return false; - } - - // If branchPrefix is specified, ensure it is found in the branch name. - // If branchPrefix is not specified, component should also be undefined. - if (branchName.getComponent() !== branchPrefix) { - return false; - } - - // In this implementation we expect to have a release version - const version = branchName.getVersion(); - if (!version) { - return false; + const baseLabel = await this.getBaseLabel(); + const pullsResponse = (await this.request( + `GET /repos/:owner/:repo/pulls?state=closed&per_page=${perPage}&sort=${sort}&direction=desc`, + { + owner: this.owner, + repo: this.repo, + } + )) as {data: PullsListResponseItems}; + for (const pull of pullsResponse.data) { + if ( + labels.length === 0 || + this.hasAllLabels( + labels, + pull.labels.map(l => { + return l.name + ''; + }) + ) + ) { + // it's expected that a release PR will have a + // HEAD matching the format repo:release-v1.0.0. + if (!pull.head) continue; + + // Verify that this PR was based against our base branch of interest. + if (!pull.base || pull.base.label !== baseLabel) continue; + + // The input should look something like: + // user:release-[optional-package-name]-v1.2.3 + // We want the package name and any semver on the end. + const match = pull.head.label.match(VERSION_FROM_BRANCH_RE); + if (!match || !pull.merged_at) continue; + + // The input here should look something like: + // [optional-package-name-]v1.2.3[-beta-or-whatever] + // Because the package name can contain things like "-v1", + // it's easiest/safest to just pull this out by string search. + const version = match[2]; + if (!version) continue; + if (branchPrefix && match[1] !== branchPrefix) { + continue; + } else if (!branchPrefix && match[1]) { + continue; } // What's left by now should just be the version string. @@ -722,9 +726,7 @@ export class GitHub { const openReleasePRs: PullsListResponseItems = []; const pullsResponse = (await this.request( - `GET /repos/:owner/:repo/pulls?state=open&per_page=${perPage}${ - this.proxyKey ? `&key=${this.proxyKey}` : '' - }`, + `GET /repos/:owner/:repo/pulls?state=open&per_page=${perPage}`, { owner: this.owner, repo: this.repo, @@ -757,9 +759,7 @@ export class GitHub { CheckpointType.Success ); return this.request( - `POST /repos/:owner/:repo/issues/:issue_number/labels${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, + 'POST /repos/:owner/:repo/issues/:issue_number/labels', { owner: this.owner, repo: this.repo, @@ -779,7 +779,7 @@ export class GitHub { method: 'GET', url: `/repos/${this.owner}/${this.repo}/issues?labels=${labels.join( ',' - )}${this.proxyKey ? `&key=${this.proxyKey}` : ''}`, + )}`, per_page: 100, }) )) { @@ -853,19 +853,14 @@ export class GitHub { )}`, CheckpointType.Success ); - await this.request( - `PATCH /repos/:owner/:repo/pulls/:pull_number${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, - { - pull_number: openReleasePR.number, - owner: this.owner, - repo: this.repo, - title: options.title, - body: options.body, - state: 'open', - } - ); + await this.request('PATCH /repos/:owner/:repo/pulls/:pull_number', { + pull_number: openReleasePR.number, + owner: this.owner, + repo: this.repo, + title: options.title, + body: options.body, + state: 'open', + }); return openReleasePR.number; } else { return prNumber; @@ -931,7 +926,7 @@ export class GitHub { repo: this.repo, owner: this.owner, headers: { - Authorization: `${this.proxyKey ? '' : 'token '}${this.token}`, + Authorization: `token ${this.token}`, }, }); this.defaultBranch = (data as {default_branch: string}).default_branch; @@ -939,17 +934,12 @@ export class GitHub { } async closePR(prNumber: number) { - await this.request( - `PATCH /repos/:owner/:repo/pulls/:pull_number${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, - { - owner: this.owner, - repo: this.repo, - pull_number: prNumber, - state: 'closed', - } - ); + await this.request('PATCH /repos/:owner/:repo/pulls/:pull_number', { + owner: this.owner, + repo: this.repo, + pull_number: prNumber, + state: 'closed', + }); } // Takes a potentially unqualified branch name, and turns it @@ -977,9 +967,7 @@ export class GitHub { }; if (ref) options.ref = ref; const resp = await this.request( - `GET /repos/:owner/:repo/contents/:path${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, + 'GET /repos/:owner/:repo/contents/:path', options ); return { @@ -999,9 +987,7 @@ export class GitHub { branch, }; const repoTree: OctokitResponse = await this.request( - `GET /repos/:owner/:repo/git/trees/:branch${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, + 'GET /repos/:owner/:repo/git/trees/:branch', options ); @@ -1010,16 +996,11 @@ export class GitHub { throw new Error(`Could not find requested path: ${path}`); } - const resp = await this.request( - `GET /repos/:owner/:repo/git/blobs/:sha${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, - { - owner: this.owner, - repo: this.repo, - sha: blobDescriptor.sha, - } - ); + const resp = await this.request('GET /repos/:owner/:repo/git/blobs/:sha', { + owner: this.owner, + repo: this.repo, + sha: blobDescriptor.sha, + }); return { parsedContent: Buffer.from(resp.data.content, 'base64').toString('utf8'), @@ -1058,20 +1039,15 @@ export class GitHub { ): Promise { checkpoint(`creating release ${tagName}`, CheckpointType.Success); return ( - await this.request( - `POST /repos/:owner/:repo/releases${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, - { - owner: this.owner, - repo: this.repo, - tag_name: tagName, - target_commitish: sha, - body: releaseNotes, - name: `${packageName} ${tagName}`, - draft: draft, - } - ) + await this.request('POST /repos/:owner/:repo/releases', { + owner: this.owner, + repo: this.repo, + tag_name: tagName, + target_commitish: sha, + body: releaseNotes, + name: `${packageName} ${tagName}`, + draft: draft, + }) ).data; } @@ -1085,9 +1061,7 @@ export class GitHub { CheckpointType.Success ); await this.request( - `DELETE /repos/:owner/:repo/issues/:issue_number/labels/:name${ - this.proxyKey ? `?key=${this.proxyKey}` : '' - }`, + 'DELETE /repos/:owner/:repo/issues/:issue_number/labels/:name', { owner: this.owner, repo: this.repo, diff --git a/src/index.ts b/src/index.ts index 2501b423b..4b41db3e6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,13 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. -export { - BuildOptions, - ReleaseCandidate, - ReleasePR, - ReleasePROptions, -} from './release-pr'; -export {ReleasePRFactory} from './release-pr-factory'; +import {OctokitAPIs} from './github'; + +export {ReleaseCandidate, ReleasePR, ReleasePROptions} from './release-pr'; + +// Configuration options shared by Release PRs and +// GitHub releases: +export interface SharedOptions { + label?: string; + repoUrl: string; + path?: string; + packageName?: string; + monorepoTags?: boolean; + token?: string; + apiUrl: string; + octokitAPIs?: OctokitAPIs; +} + +export const DEFAULT_LABELS = 'autorelease: pending'; +export {factory} from './factory'; export {getReleaserNames, getReleasers} from './releasers'; export {GitHubRelease, GitHubReleaseOptions} from './github-release'; export {JavaYoshi} from './releasers/java-yoshi'; diff --git a/src/release-pr-factory.ts b/src/release-pr-factory.ts deleted file mode 100644 index 57fdc0f6d..000000000 --- a/src/release-pr-factory.ts +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2019 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import {BuildOptions, ReleasePR, ReleasePROptions} from './release-pr'; -import {RubyReleasePROptions} from './releasers/ruby'; -import {getReleasers} from './releasers'; - -export class ReleasePRFactory { - static build(releaseType: string, options: BuildOptions): ReleasePR { - const releaseOptions: ReleasePROptions | RubyReleasePROptions = { - ...options, - ...{releaseType}, - }; - return new (ReleasePRFactory.class(releaseType))(releaseOptions); - } - // Return a ReleasePR class, based on the release type, e.g., node, python: - static class(releaseType: string): typeof ReleasePR { - const releasers = getReleasers(); - const releaser = releasers[releaseType]; - if (!releaser) { - throw Error('unknown release type'); - } - return releaser; - } - // TODO(bcoe): this function is deprecated and should be removed in the - // next major; - static buildStatic(releaseType: string, options: BuildOptions) { - return this.build(releaseType, options); - } -} diff --git a/src/release-pr.ts b/src/release-pr.ts index 9b7429af9..4f729631f 100644 --- a/src/release-pr.ts +++ b/src/release-pr.ts @@ -14,6 +14,7 @@ // See: https://github.com/octokit/rest.js/issues/1624 // https://github.com/octokit/types.ts/issues/25. +import {SharedOptions, DEFAULT_LABELS} from './'; import {Octokit} from '@octokit/rest'; import {PromiseValue} from 'type-fest'; type PullsListResponseItems = PromiseValue< @@ -33,33 +34,15 @@ import {extractReleaseNotes} from './util/release-notes'; // eslint-disable-next-line @typescript-eslint/no-var-requires const parseGithubRepoUrl = require('parse-github-repo-url'); -export interface BuildOptions { - // Whether to treat breaking changes as minor semver bumps pre-1.0.0 +export interface ReleasePROptions extends SharedOptions { bumpMinorPreMajor?: boolean; // The target release branch defaultBranch?: string; // Whether to open the pull request from a forked repository fork?: boolean; - // Comma-separated list of additional labels to add - label?: string; - // GitHub API Token - token?: string; - // URL of the GitHub repository - repoUrl: string; - // The name of the package to release - packageName: string; - // When releasing multiple libraries from one repository, whether or not - // to include a prefix on tags and branch names - monorepoTags?: boolean; - // Limit file search to this path - path?: string; - // Override the version to release + // When releasing multiple libraries from one repository, include a prefix + // on tags and branch names: releaseAs?: string; - // GitHub API endpoint - apiUrl: string; - // Legacy GitHub proxy authentication token - proxyKey?: string; - // Whether or not to force a snapshot release snapshot?: boolean; // Override the last released version lastPackageVersion?: string; @@ -67,11 +50,6 @@ export interface BuildOptions { octokitAPIs?: OctokitAPIs; // Path to version.rb file versionFile?: string; - // Optionally provide GitHub instance - github?: GitHub; -} - -export interface ReleasePROptions extends BuildOptions { releaseType: string; changelogSections?: []; } @@ -96,17 +74,6 @@ export interface OpenPROptions { includePackageName: boolean; } -export interface CandidateRelease { - sha: string; - tag: string; - notes: string; - name: string; - version: string; - pullNumber: number; -} - -const DEFAULT_LABELS = 'autorelease: pending'; - export class ReleasePR { static releaserName = 'base'; @@ -122,7 +89,6 @@ export class ReleasePR { packageName: string; monorepoTags: boolean; releaseAs?: string; - proxyKey?: string; snapshot?: boolean; lastPackageVersion?: string; changelogSections?: []; @@ -140,11 +106,13 @@ export class ReleasePR { this.repoUrl = options.repoUrl; this.token = options.token; this.path = options.path; - this.packageName = options.packageName; + // If no packageName has been provided, use the name + // of the repository: + const [, repo] = parseGithubRepoUrl(this.repoUrl); + this.packageName = options.packageName || repo; this.monorepoTags = options.monorepoTags || false; this.releaseAs = options.releaseAs; this.apiUrl = options.apiUrl; - this.proxyKey = options.proxyKey; this.snapshot = options.snapshot; // drop a `v` prefix if provided: this.lastPackageVersion = options.lastPackageVersion @@ -299,7 +267,6 @@ export class ReleasePR { owner, repo, apiUrl: this.apiUrl, - proxyKey: this.proxyKey, octokitAPIs, }); } diff --git a/test/cli.ts b/test/cli.ts new file mode 100644 index 000000000..2b3d805ac --- /dev/null +++ b/test/cli.ts @@ -0,0 +1,53 @@ +// Copyright 2021 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as assert from 'assert'; +import {factory} from '../src/factory'; +import {GitHubRelease} from '../src/github-release'; +import {ReleasePR} from '../src/release-pr'; +import {describe, it, afterEach} from 'mocha'; +import * as sinon from 'sinon'; + +import {parser} from '../src/bin/release-please'; + +const sandbox = sinon.createSandbox(); + +describe('CLI', () => { + afterEach(() => { + sandbox.restore(); + }); + describe('release-pr', () => { + it('instantiates release PR based on command line arguments', () => { + let classToRun: ReleasePR; + // This logic is used to capture the class that would + // be executed, allowing us to validate that the appropriate + // properties were assigned from the argument parser: + sandbox.replace( + factory, + 'run', + (runnable): Promise => { + classToRun = runnable as ReleasePR; + return Promise.resolve(undefined); + } + ); + parser.parse( + 'release-pr --repo-url=googleapis/release-please-cli --package-name=cli-package' + ); + assert.strictEqual(classToRun!.repoUrl, 'googleapis/release-please-cli'); + assert.strictEqual(classToRun!.packageName, 'cli-package'); + // Defaults to Node.js release type: + assert.strictEqual(classToRun!.releaseType, 'node'); + }); + }); +}); diff --git a/test/release-pr-factory.ts b/test/factory.ts similarity index 96% rename from test/release-pr-factory.ts rename to test/factory.ts index 95ef5cb4c..b6e96d420 100644 --- a/test/release-pr-factory.ts +++ b/test/factory.ts @@ -15,7 +15,7 @@ import {describe, it, afterEach} from 'mocha'; import * as assert from 'assert'; import * as nock from 'nock'; -import {ReleasePRFactory} from '../src/release-pr-factory'; +import * as factory from '../src/factory'; import {readFileSync} from 'fs'; import {resolve} from 'path'; import * as snapshot from 'snap-shot-it'; @@ -26,12 +26,12 @@ import * as sinon from 'sinon'; const sandbox = sinon.createSandbox(); const fixturesPath = './test/releasers/fixtures/simple'; -describe('ReleasePRFactory', () => { +describe('factory', () => { afterEach(() => { sandbox.restore(); }); - describe('build', () => { + describe('ReleasePR', () => { it('returns instance of dynamically loaded releaser', async () => { // We stub the entire suggester API, asserting only that the // the appropriate changes are proposed: @@ -108,11 +108,12 @@ describe('ReleasePRFactory', () => { } ) .reply(200); - const releasePR = ReleasePRFactory.build('simple', { + const releasePR = factory.releasePR({ repoUrl: 'googleapis/simple-test-repo', // not actually used by this type of repo. packageName: 'simple-test-repo', apiUrl: 'https://api.github.com', + releaseType: 'simple', }); await releasePR.run(); req.done(); @@ -203,11 +204,13 @@ describe('ReleasePRFactory', () => { } ) .reply(200); - const releasePR = ReleasePRFactory.buildStatic('simple', { + const releasePR = factory.releasePR({ + label: '', repoUrl: 'googleapis/simple-test-repo', // not actually used by this type of repo. packageName: 'simple-test-repo', apiUrl: 'https://api.github.com', + releaseType: 'simple', }); await releasePR.run(); req.done(); diff --git a/test/github-release.ts b/test/github-release.ts index ea6881e99..3f3f880ef 100644 --- a/test/github-release.ts +++ b/test/github-release.ts @@ -35,10 +35,14 @@ function buildFileContent(content: string): GitHubFileContents { } describe('GitHubRelease', () => { +<<<<<<< HEAD afterEach(() => { sandbox.restore(); }); describe('createRelease', () => { +======= + describe('run', () => { +>>>>>>> 5c4ff32 (feat(cli)!: refactor factory/CLI to be more testable) it('creates and labels release on GitHub', async () => { const release = new GitHubRelease({ label: 'autorelease: pending', @@ -86,7 +90,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); strictEqual(created!.tag_name, 'v1.0.3'); strictEqual(created!.major, 1); strictEqual(created!.minor, 0); @@ -142,7 +146,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); strictEqual(created!.tag_name, 'v1.0.3'); strictEqual(created!.major, 1); strictEqual(created!.minor, 0); @@ -260,7 +264,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); strictEqual(created!.tag_name, 'foo/v1.0.3'); }); @@ -394,7 +398,7 @@ describe('GitHubRelease', () => { }); let failed = true; try { - await release.createRelease(); + await release.run(); failed = false; } catch (error) { expect(error.message).to.equal( @@ -442,7 +446,7 @@ describe('GitHubRelease', () => { let failed = true; try { - await release.createRelease(); + await release.run(); failed = false; } catch (error) { expect(error.message).to.equal( From aaf38382f80ca1f98c60532d1740de7409aac109 Mon Sep 17 00:00:00 2001 From: bcoe Date: Fri, 29 Jan 2021 14:52:20 -0800 Subject: [PATCH 2/9] test: add smoke test for GitHub release --- test/cli.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/cli.ts b/test/cli.ts index 2b3d805ac..6f8be123b 100644 --- a/test/cli.ts +++ b/test/cli.ts @@ -50,4 +50,28 @@ describe('CLI', () => { assert.strictEqual(classToRun!.releaseType, 'node'); }); }); + describe('github-release', () => { + it('instantiates a GitHub released based on command line arguments', () => { + let classToRun: GitHubRelease; + // This logic is used to capture the class that would + // be executed, allowing us to validate that the appropriate + // properties were assigned from the argument parser: + sandbox.replace( + factory, + 'run', + (runnable): Promise => { + classToRun = runnable as GitHubRelease; + return Promise.resolve(undefined); + } + ); + parser.parse( + 'github-release --repo-url=googleapis/release-please-cli --package-name=cli-package' + ); + assert.strictEqual(classToRun!.repoUrl, 'googleapis/release-please-cli'); + assert.strictEqual(classToRun!.packageName, 'cli-package'); + // Defaults to Node.js release type: + assert.strictEqual(classToRun!.releaseType, 'node'); + assert.strictEqual(classToRun!.changelogPath, 'CHANGELOG.md'); + }); + }); }); From 5aea3fa999a3a585ab50a585a227f0ae6f33ac8b Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 30 Jan 2021 15:39:35 -0800 Subject: [PATCH 3/9] chore: address code review --- src/bin/release-please.ts | 5 ----- src/release-pr.ts | 5 +---- test/cli.ts | 16 +++++++++------- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/bin/release-please.ts b/src/bin/release-please.ts index 115669bad..867877669 100644 --- a/src/bin/release-please.ts +++ b/src/bin/release-please.ts @@ -86,11 +86,6 @@ export const parser = yargs default: 'CHANGELOG.md', describe: 'where can the CHANGELOG be found in the project?', }) - .option('release-type', { - describe: 'what type of repo is a release being created for?', - choices: getReleaserNames(), - default: 'node', - }) .option('draft', { describe: 'mark release as a draft. no tag is created but tag_name and ' + diff --git a/src/release-pr.ts b/src/release-pr.ts index 4f729631f..c91139ff7 100644 --- a/src/release-pr.ts +++ b/src/release-pr.ts @@ -106,10 +106,7 @@ export class ReleasePR { this.repoUrl = options.repoUrl; this.token = options.token; this.path = options.path; - // If no packageName has been provided, use the name - // of the repository: - const [, repo] = parseGithubRepoUrl(this.repoUrl); - this.packageName = options.packageName || repo; + this.packageName = options.packageName || ''; this.monorepoTags = options.monorepoTags || false; this.releaseAs = options.releaseAs; this.apiUrl = options.apiUrl; diff --git a/test/cli.ts b/test/cli.ts index 6f8be123b..cca32f670 100644 --- a/test/cli.ts +++ b/test/cli.ts @@ -44,10 +44,11 @@ describe('CLI', () => { parser.parse( 'release-pr --repo-url=googleapis/release-please-cli --package-name=cli-package' ); - assert.strictEqual(classToRun!.repoUrl, 'googleapis/release-please-cli'); - assert.strictEqual(classToRun!.packageName, 'cli-package'); + assert.ok(classToRun! instanceof ReleasePR); + assert.strictEqual(classToRun.repoUrl, 'googleapis/release-please-cli'); + assert.strictEqual(classToRun.packageName, 'cli-package'); // Defaults to Node.js release type: - assert.strictEqual(classToRun!.releaseType, 'node'); + assert.strictEqual(classToRun.releaseType, 'node'); }); }); describe('github-release', () => { @@ -67,11 +68,12 @@ describe('CLI', () => { parser.parse( 'github-release --repo-url=googleapis/release-please-cli --package-name=cli-package' ); - assert.strictEqual(classToRun!.repoUrl, 'googleapis/release-please-cli'); - assert.strictEqual(classToRun!.packageName, 'cli-package'); + assert.ok(classToRun! instanceof GitHubRelease); + assert.strictEqual(classToRun.repoUrl, 'googleapis/release-please-cli'); + assert.strictEqual(classToRun.packageName, 'cli-package'); // Defaults to Node.js release type: - assert.strictEqual(classToRun!.releaseType, 'node'); - assert.strictEqual(classToRun!.changelogPath, 'CHANGELOG.md'); + assert.strictEqual(classToRun.releaseType, 'node'); + assert.strictEqual(classToRun.changelogPath, 'CHANGELOG.md'); }); }); }); From b89738284a1fb1c0df3317f17ee3499c33aa2cbc Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Mon, 1 Feb 2021 09:53:37 -0800 Subject: [PATCH 4/9] Update src/release-pr.ts Co-authored-by: Joel Dodge --- src/release-pr.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/release-pr.ts b/src/release-pr.ts index c91139ff7..7e4865407 100644 --- a/src/release-pr.ts +++ b/src/release-pr.ts @@ -106,7 +106,9 @@ export class ReleasePR { this.repoUrl = options.repoUrl; this.token = options.token; this.path = options.path; - this.packageName = options.packageName || ''; + this.packageName = options.packageName || await factory + .releasePRClass(this.releaseType) + .lookupPackageName(this.gh, this.path); this.monorepoTags = options.monorepoTags || false; this.releaseAs = options.releaseAs; this.apiUrl = options.apiUrl; From 9c192d657abaf6be8cfe1944f1757241ec4a3731 Mon Sep 17 00:00:00 2001 From: bcoe Date: Mon, 1 Feb 2021 10:00:55 -0800 Subject: [PATCH 5/9] chore: revert packageName lookup for now --- src/release-pr.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/release-pr.ts b/src/release-pr.ts index 7e4865407..c91139ff7 100644 --- a/src/release-pr.ts +++ b/src/release-pr.ts @@ -106,9 +106,7 @@ export class ReleasePR { this.repoUrl = options.repoUrl; this.token = options.token; this.path = options.path; - this.packageName = options.packageName || await factory - .releasePRClass(this.releaseType) - .lookupPackageName(this.gh, this.path); + this.packageName = options.packageName || ''; this.monorepoTags = options.monorepoTags || false; this.releaseAs = options.releaseAs; this.apiUrl = options.apiUrl; From 601c16dfa2aa72f1e2558d804aec430defed3ddd Mon Sep 17 00:00:00 2001 From: bcoe Date: Wed, 3 Feb 2021 11:27:28 -0800 Subject: [PATCH 6/9] chore: merge changes --- src/github-release.ts | 40 +++--------------------- src/github.ts | 71 +++++++++++++++++------------------------- src/release-pr.ts | 11 +++++++ test/github-release.ts | 20 +++++------- 4 files changed, 52 insertions(+), 90 deletions(-) diff --git a/src/github-release.ts b/src/github-release.ts index 3fbdeaf73..c468b67ae 100644 --- a/src/github-release.ts +++ b/src/github-release.ts @@ -12,11 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {ReleasePRFactory} from './release-pr-factory'; -import chalk = require('chalk'); import {SharedOptions, DEFAULT_LABELS} from './'; import {checkpoint, CheckpointType} from './util/checkpoint'; -import {packageBranchPrefix} from './util/package-branch-prefix'; import * as factory from './factory'; import {GitHub, OctokitAPIs} from './github'; import {parse} from 'semver'; @@ -79,13 +76,9 @@ export class GitHubRelease { this.gh = this.gitHubInstance(options.octokitAPIs); } -<<<<<<< HEAD - async createRelease(): Promise { -======= async run(): Promise { // Attempt to lookup the package name from a well known location, such // as package.json, if none is provided: ->>>>>>> 5c4ff32 (feat(cli)!: refactor factory/CLI to be more testable) if (!this.packageName && this.releaseType) { this.packageName = await factory .releasePRClass(this.releaseType) @@ -108,7 +101,10 @@ export class GitHubRelease { monorepoTags: this.monorepoTags, }; const releasePR = this.releaseType - ? ReleasePRFactory.build(this.releaseType, releaseOptions) + ? factory.releasePR({ + ...releaseOptions, + ...{releaseType: this.releaseType}, + }) : new ReleasePR({ ...releaseOptions, ...{releaseType: 'unknown'}, @@ -121,34 +117,6 @@ export class GitHubRelease { checkpoint('Unable to build candidate', CheckpointType.Failure); return undefined; } -<<<<<<< HEAD -======= - const version = `v${gitHubReleasePR.version}`; - - checkpoint( - `found release branch ${chalk.green(version)} at ${chalk.green( - gitHubReleasePR.sha - )}`, - CheckpointType.Success - ); - - const changelogContents = ( - await this.gh.getFileContents(this.addPath(this.changelogPath)) - ).parsedContent; - const latestReleaseNotes = GitHubRelease.extractLatestReleaseNotes( - changelogContents, - version - ); - checkpoint( - `found release notes: \n---\n${chalk.grey(latestReleaseNotes)}\n---\n`, - CheckpointType.Success - ); - // Go uses '/' for a tag separator, rather than '-': - let tagSeparator = '-'; - if (this.releaseType) { - tagSeparator = factory.releasePRClass(this.releaseType).tagSeparator(); - } ->>>>>>> 5c4ff32 (feat(cli)!: refactor factory/CLI to be more testable) const release = await this.gh.createRelease( candidate.name, diff --git a/src/github.ts b/src/github.ts index f64b426bf..59ff2e709 100644 --- a/src/github.ts +++ b/src/github.ts @@ -143,6 +143,7 @@ export class GitHub { this.owner = options.owner; this.repo = options.repo; this.apiUrl = options.apiUrl || 'https://api.github.com'; + if (options.octokitAPIs === undefined) { this.octokit = new Octokit({ baseUrl: options.apiUrl, @@ -154,7 +155,8 @@ export class GitHub { 'user-agent': `release-please/${ require('../../package.json').version }`, - Authorization: ` token ${this.token}`, + // some proxies do not require the token prefix. + Authorization: `token ${this.token}`, }, }; this.request = request.defaults(defaults); @@ -650,47 +652,32 @@ export class GitHub { branchPrefix = branchPrefix?.endsWith('-') ? branchPrefix.replace(/-$/, '') : branchPrefix; - const baseLabel = await this.getBaseLabel(); - const pullsResponse = (await this.request( - `GET /repos/:owner/:repo/pulls?state=closed&per_page=${perPage}&sort=${sort}&direction=desc`, - { - owner: this.owner, - repo: this.repo, - } - )) as {data: PullsListResponseItems}; - for (const pull of pullsResponse.data) { - if ( - labels.length === 0 || - this.hasAllLabels( - labels, - pull.labels.map(l => { - return l.name + ''; - }) - ) - ) { - // it's expected that a release PR will have a - // HEAD matching the format repo:release-v1.0.0. - if (!pull.head) continue; - - // Verify that this PR was based against our base branch of interest. - if (!pull.base || pull.base.label !== baseLabel) continue; - - // The input should look something like: - // user:release-[optional-package-name]-v1.2.3 - // We want the package name and any semver on the end. - const match = pull.head.label.match(VERSION_FROM_BRANCH_RE); - if (!match || !pull.merged_at) continue; - - // The input here should look something like: - // [optional-package-name-]v1.2.3[-beta-or-whatever] - // Because the package name can contain things like "-v1", - // it's easiest/safest to just pull this out by string search. - const version = match[2]; - if (!version) continue; - if (branchPrefix && match[1] !== branchPrefix) { - continue; - } else if (!branchPrefix && match[1]) { - continue; + return await this.findMergedPullRequest( + baseBranch, + (mergedPullRequest: MergedGitHubPR) => { + // If labels specified, ensure the pull request has all the specified labels + if ( + labels.length > 0 && + !this.hasAllLabels(labels, mergedPullRequest.labels) + ) { + return false; + } + + const branchName = BranchName.parse(mergedPullRequest.headRefName); + if (!branchName) { + return false; + } + + // If branchPrefix is specified, ensure it is found in the branch name. + // If branchPrefix is not specified, component should also be undefined. + if (branchName.getComponent() !== branchPrefix) { + return false; + } + + // In this implementation we expect to have a release version + const version = branchName.getVersion(); + if (!version) { + return false; } // What's left by now should just be the version string. diff --git a/src/release-pr.ts b/src/release-pr.ts index c91139ff7..db104c9b2 100644 --- a/src/release-pr.ts +++ b/src/release-pr.ts @@ -52,6 +52,8 @@ export interface ReleasePROptions extends SharedOptions { versionFile?: string; releaseType: string; changelogSections?: []; + // Optionally provide GitHub instance + github?: GitHub; } export interface ReleaseCandidate { @@ -59,6 +61,15 @@ export interface ReleaseCandidate { previousTag?: string; } +export interface CandidateRelease { + sha: string; + tag: string; + notes: string; + name: string; + version: string; + pullNumber: number; +} + interface GetCommitsOptions { sha?: string; perPage?: number; diff --git a/test/github-release.ts b/test/github-release.ts index 3f3f880ef..18dc85e29 100644 --- a/test/github-release.ts +++ b/test/github-release.ts @@ -35,14 +35,10 @@ function buildFileContent(content: string): GitHubFileContents { } describe('GitHubRelease', () => { -<<<<<<< HEAD afterEach(() => { sandbox.restore(); }); describe('createRelease', () => { -======= - describe('run', () => { ->>>>>>> 5c4ff32 (feat(cli)!: refactor factory/CLI to be more testable) it('creates and labels release on GitHub', async () => { const release = new GitHubRelease({ label: 'autorelease: pending', @@ -205,7 +201,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); expect(created).to.not.be.undefined; strictEqual(created!.tag_name, 'bigquery/v1.0.3'); strictEqual(created!.major, 1); @@ -324,7 +320,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); expect(created).to.not.be.undefined; strictEqual(created!.tag_name, 'foo-v1.0.3'); }); @@ -385,7 +381,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); strictEqual(created!.tag_name, 'v1.0.3'); }); @@ -503,7 +499,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); expect(created).to.not.be.undefined; strictEqual(created!.tag_name, 'v1.0.3'); strictEqual(created!.major, 1); @@ -560,7 +556,7 @@ describe('GitHubRelease', () => { .withArgs(['autorelease: pending'], 1) .resolves(); - const created = await release.createRelease(); + const created = await release.run(); expect(created).to.not.be.undefined; strictEqual(created!.tag_name, 'v1.0.3'); strictEqual(created!.major, 1); @@ -581,7 +577,7 @@ describe('GitHubRelease', () => { sandbox.stub(release.gh, 'findMergedPullRequests').resolves([]); - const created = await release.createRelease(); + const created = await release.run(); expect(created).to.be.undefined; }); @@ -607,7 +603,7 @@ describe('GitHubRelease', () => { }, ]); - const created = await release.createRelease(); + const created = await release.run(); expect(created).to.be.undefined; }); @@ -638,7 +634,7 @@ describe('GitHubRelease', () => { .onSecondCall() .resolves([]); - const created = await release.createRelease(); + const created = await release.run(); expect(created).to.be.undefined; }); }); From 04c379ae5bba2b650231e8e8494b1fa73fc39eec Mon Sep 17 00:00:00 2001 From: bcoe Date: Wed, 3 Feb 2021 14:04:30 -0800 Subject: [PATCH 7/9] meta: address code review --- src/factory.ts | 4 ++-- src/github-release.ts | 2 +- test/factory.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/factory.ts b/src/factory.ts index faadfd9b5..4c74cea56 100644 --- a/src/factory.ts +++ b/src/factory.ts @@ -46,11 +46,11 @@ function run(runnable: ReleasePR | GitHubRelease) { return runnable.run(); } -export function githubRelease(options: GitHubReleaseOptions): GitHubRelease { +function githubRelease(options: GitHubReleaseOptions): GitHubRelease { return new GitHubRelease(options); } -export function releasePR(options: ReleasePROptions): ReleasePR { +function releasePR(options: ReleasePROptions): ReleasePR { const releaseOptions: ReleasePROptions | RubyReleasePROptions = options; return new (factory.releasePRClass(options.releaseType))(releaseOptions); } diff --git a/src/github-release.ts b/src/github-release.ts index c468b67ae..f1b1899cf 100644 --- a/src/github-release.ts +++ b/src/github-release.ts @@ -14,7 +14,7 @@ import {SharedOptions, DEFAULT_LABELS} from './'; import {checkpoint, CheckpointType} from './util/checkpoint'; -import * as factory from './factory'; +import {factory} from './factory'; import {GitHub, OctokitAPIs} from './github'; import {parse} from 'semver'; import {ReleasePR} from './release-pr'; diff --git a/test/factory.ts b/test/factory.ts index b6e96d420..ee5e75517 100644 --- a/test/factory.ts +++ b/test/factory.ts @@ -15,7 +15,7 @@ import {describe, it, afterEach} from 'mocha'; import * as assert from 'assert'; import * as nock from 'nock'; -import * as factory from '../src/factory'; +import {factory} from '../src/factory'; import {readFileSync} from 'fs'; import {resolve} from 'path'; import * as snapshot from 'snap-shot-it'; From 4664f39f72f3d6732a67c3fe672234fbde589421 Mon Sep 17 00:00:00 2001 From: bcoe Date: Wed, 3 Feb 2021 14:10:24 -0800 Subject: [PATCH 8/9] chore: drop export --- src/factory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/factory.ts b/src/factory.ts index 4c74cea56..490e3d6f1 100644 --- a/src/factory.ts +++ b/src/factory.ts @@ -24,7 +24,7 @@ import { } from './github-release'; import {getReleasers} from './releasers'; -export function runCommand( +function runCommand( command: string, options: GitHubReleaseOptions | ReleasePROptions ): Promise { From 197d8e5d6c34974bff64fef8171741640cc367cb Mon Sep 17 00:00:00 2001 From: bcoe Date: Wed, 3 Feb 2021 14:29:19 -0800 Subject: [PATCH 9/9] chore: address code review --- src/factory.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/factory.ts b/src/factory.ts index 490e3d6f1..4fdc54816 100644 --- a/src/factory.ts +++ b/src/factory.ts @@ -54,6 +54,7 @@ function releasePR(options: ReleasePROptions): ReleasePR { const releaseOptions: ReleasePROptions | RubyReleasePROptions = options; return new (factory.releasePRClass(options.releaseType))(releaseOptions); } + export function releasePRClass(releaseType: string): typeof ReleasePR { const releasers = getReleasers(); const releaser = releasers[releaseType];