From 6c9ce89be76679ea5c3bdb52551aa5fc88d2cf55 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Wed, 24 Nov 2021 10:24:28 -0800 Subject: [PATCH 1/3] feat: return path along with created release fix!: return created pull request rather than just the pull request number --- src/manifest.ts | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/manifest.ts b/src/manifest.ts index 8e8a29368..a17ca2270 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -72,6 +72,7 @@ export interface CandidateReleasePullRequest { export interface CandidateRelease extends Release { pullRequest: PullRequest; draft?: boolean; + path: string; } interface ReleaserConfigJson { @@ -140,6 +141,10 @@ const DEFAULT_RELEASE_LABELS = ['autorelease: tagged']; export const MANIFEST_PULL_REQUEST_TITLE_PATTERN = 'chore: release ${branch}'; +interface CreatedRelease extends GitHubRelease { + path: string; +} + export class Manifest { private repository: Repository; private github: GitHub; @@ -520,7 +525,7 @@ export class Manifest { * * @returns {number[]} Pull request numbers of release pull requests */ - async createPullRequests(): Promise<(number | undefined)[]> { + async createPullRequests(): Promise<(PullRequest | undefined)[]> { const candidatePullRequests = await this.buildPullRequests(); if (candidatePullRequests.length === 0) { return []; @@ -554,7 +559,7 @@ export class Manifest { } logger.info(`found ${openPullRequests.length} open release pull requests.`); - const promises: Promise[] = []; + const promises: Promise[] = []; for (const pullRequest of candidatePullRequests) { promises.push( this.createOrUpdatePullRequest(pullRequest, openPullRequests) @@ -566,7 +571,7 @@ export class Manifest { private async createOrUpdatePullRequest( pullRequest: ReleasePullRequest, openPullRequests: PullRequest[] - ): Promise { + ): Promise { // look for existing, open pull rquest const existing = openPullRequests.find( openPullRequest => @@ -589,7 +594,7 @@ export class Manifest { signoffUser: this.signoffUser, } ); - return updatedPullRequest.number; + return updatedPullRequest; } else { const newPullRequest = await this.github.createReleasePullRequest( pullRequest, @@ -599,7 +604,7 @@ export class Manifest { signoffUser: this.signoffUser, } ); - return newPullRequest.number; + return newPullRequest; } } @@ -669,6 +674,7 @@ export class Manifest { if (release) { releases.push({ ...release, + path, pullRequest, draft: config.draft ?? this.draft, }); @@ -714,9 +720,9 @@ export class Manifest { private async createReleasesForPullRequest( releases: CandidateRelease[], pullRequest: PullRequest - ): Promise { + ): Promise { // create the release - const promises: Promise[] = []; + const promises: Promise[] = []; for (const release of releases) { promises.push(this.createRelease(release)); } @@ -733,7 +739,7 @@ export class Manifest { private async createRelease( release: CandidateRelease - ): Promise { + ): Promise { const githubRelease = await this.github.createRelease(release, { draft: release.draft, }); @@ -742,7 +748,10 @@ export class Manifest { const comment = `:robot: Release is at ${githubRelease.url} :sunflower:`; await this.github.commentOnIssue(comment, release.pullRequest.number); - return githubRelease; + return { + ...githubRelease, + path: release.path, + }; } private async getStrategiesByPath(): Promise> { From 26a7d96127aa26a7d264423c1ff7f67cf0563536 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Wed, 24 Nov 2021 10:34:23 -0800 Subject: [PATCH 2/3] fix tests --- test/cli.ts | 50 ++++++++++++++++++++++++++++++++++-------------- test/manifest.ts | 14 ++++++++------ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/test/cli.ts b/test/cli.ts index 328d0d3eb..e18fc9a7d 100644 --- a/test/cli.ts +++ b/test/cli.ts @@ -98,7 +98,17 @@ describe('CLI', () => { .resolves(fakeManifest); createPullRequestsStub = sandbox .stub(fakeManifest, 'createPullRequests') - .resolves([123]); + .resolves([ + { + title: 'fake title', + body: 'fake body', + headBranchName: 'head-branch-name', + baseBranchName: 'base-branch-name', + number: 123, + files: [], + labels: [], + }, + ]); }); it('instantiates a basic Manifest', async () => { await await parser.parseAsync( @@ -435,16 +445,26 @@ describe('CLI', () => { describe('release-pr', () => { describe('with manifest options', () => { let fromManifestStub: sinon.SinonStub; + let createPullRequestsStub: sinon.SinonStub; beforeEach(() => { fromManifestStub = sandbox .stub(Manifest, 'fromManifest') .resolves(fakeManifest); + createPullRequestsStub = sandbox + .stub(fakeManifest, 'createPullRequests') + .resolves([ + { + title: 'fake title', + body: 'fake body', + headBranchName: 'head-branch-name', + baseBranchName: 'base-branch-name', + number: 123, + files: [], + labels: [], + }, + ]); }); it('instantiates a basic Manifest', async () => { - const createPullRequestsStub = sandbox - .stub(fakeManifest, 'createPullRequests') - .resolves([123]); - await parser.parseAsync( 'release-pr --repo-url=googleapis/release-please-cli' ); @@ -465,10 +485,6 @@ describe('CLI', () => { sinon.assert.calledOnce(createPullRequestsStub); }); it('instantiates Manifest with custom config/manifest', async () => { - const createPullRequestsStub = sandbox - .stub(fakeManifest, 'createPullRequests') - .resolves([123]); - await parser.parseAsync( 'release-pr --repo-url=googleapis/release-please-cli --config-file=foo.json --manifest-file=.bar.json' ); @@ -490,10 +506,6 @@ describe('CLI', () => { }); for (const flag of ['--target-branch', '--default-branch']) { it(`handles ${flag}`, async () => { - const createPullRequestsStub = sandbox - .stub(fakeManifest, 'createPullRequests') - .resolves([123]); - await parser.parseAsync( `release-pr --repo-url=googleapis/release-please-cli ${flag}=1.x` ); @@ -548,7 +560,17 @@ describe('CLI', () => { .resolves(fakeManifest); createPullRequestsStub = sandbox .stub(fakeManifest, 'createPullRequests') - .resolves([123]); + .resolves([ + { + title: 'fake title', + body: 'fake body', + headBranchName: 'head-branch-name', + baseBranchName: 'base-branch-name', + number: 123, + files: [], + labels: [], + }, + ]); }); it('instantiates a basic Manifest', async () => { await parser.parseAsync( diff --git a/test/manifest.ts b/test/manifest.ts index 514b1710c..29dfa6134 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -1023,8 +1023,8 @@ describe('Manifest', () => { } ); sandbox.stub(manifest, 'buildPullRequests').resolves([]); - const pullRequestNumbers = await manifest.createPullRequests(); - expect(pullRequestNumbers).to.be.empty; + const pullRequests = await manifest.createPullRequests(); + expect(pullRequests).to.be.empty; }); it('handles a single pull request', async function () { @@ -1085,8 +1085,8 @@ describe('Manifest', () => { draft: false, }, ]); - const pullRequestNumbers = await manifest.createPullRequests(); - expect(pullRequestNumbers).lengthOf(1); + const pullRequests = await manifest.createPullRequests(); + expect(pullRequests).lengthOf(1); }); it('handles a multiple pull requests', async () => { @@ -1207,8 +1207,10 @@ describe('Manifest', () => { draft: false, }, ]); - const pullRequestNumbers = await manifest.createPullRequests(); - expect(pullRequestNumbers).to.eql([123, 124]); + const pullRequests = await manifest.createPullRequests(); + expect(pullRequests.map(pullRequest => pullRequest!.number)).to.eql([ + 123, 124, + ]); }); it('handles signoff users', async function () { From 60aa7b7bdfd3fbc96bfdbf67d1bd737a9c2438df Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Wed, 24 Nov 2021 10:42:53 -0800 Subject: [PATCH 3/3] test: add tests for returned release path --- src/manifest.ts | 4 ++-- test/cli.ts | 3 +++ test/manifest.ts | 12 ++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/manifest.ts b/src/manifest.ts index a17ca2270..04de4ed56 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -692,7 +692,7 @@ export class Manifest { * * @returns {GitHubRelease[]} List of created GitHub releases */ - async createReleases(): Promise<(GitHubRelease | undefined)[]> { + async createReleases(): Promise<(CreatedRelease | undefined)[]> { const releasesByPullRequest: Record = {}; const pullRequestsByNumber: Record = {}; for (const release of await this.buildReleases()) { @@ -704,7 +704,7 @@ export class Manifest { } } - const promises: Promise[] = []; + const promises: Promise[] = []; for (const pullNumber in releasesByPullRequest) { promises.push( this.createReleasesForPullRequest( diff --git a/test/cli.ts b/test/cli.ts index e18fc9a7d..2f96bebf7 100644 --- a/test/cli.ts +++ b/test/cli.ts @@ -292,6 +292,7 @@ describe('CLI', () => { sha: 'abc123', notes: 'some release notes', url: 'url-of-release', + path: '.', }, ]); }); @@ -926,6 +927,7 @@ describe('CLI', () => { sha: 'abc123', notes: 'some release notes', url: 'url-of-release', + path: '.', }, ]); }); @@ -1076,6 +1078,7 @@ describe('CLI', () => { sha: 'abc123', notes: 'some release notes', url: 'url-of-release', + path: '.', }, ]); }); diff --git a/test/manifest.ts b/test/manifest.ts index 29dfa6134..9fe7da8e9 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -1533,6 +1533,7 @@ describe('Manifest', () => { expect(releases[0].notes) .to.be.a('string') .and.satisfy((msg: string) => msg.startsWith('### Bug Fixes')); + expect(releases[0].path).to.eql('.'); }); it('should handle a multiple manifest release', async () => { @@ -1617,21 +1618,25 @@ describe('Manifest', () => { expect(releases[0].notes) .to.be.a('string') .and.satisfy((msg: string) => msg.startsWith('### Features')); + expect(releases[0].path).to.eql('packages/bot-config-utils'); expect(releases[1].tag.toString()).to.eql('label-utils-v1.1.0'); expect(releases[1].sha).to.eql('abc123'); expect(releases[1].notes) .to.be.a('string') .and.satisfy((msg: string) => msg.startsWith('### Features')); + expect(releases[1].path).to.eql('packages/label-utils'); expect(releases[2].tag.toString()).to.eql('object-selector-v1.1.0'); expect(releases[2].sha).to.eql('abc123'); expect(releases[2].notes) .to.be.a('string') .and.satisfy((msg: string) => msg.startsWith('### Features')); + expect(releases[2].path).to.eql('packages/object-selector'); expect(releases[3].tag.toString()).to.eql('datastore-lock-v2.1.0'); expect(releases[3].sha).to.eql('abc123'); expect(releases[3].notes) .to.be.a('string') .and.satisfy((msg: string) => msg.startsWith('### Features')); + expect(releases[3].path).to.eql('packages/datastore-lock'); }); it('should handle a single standalone release', async () => { @@ -1670,6 +1675,7 @@ describe('Manifest', () => { expect(releases[0].notes) .to.be.a('string') .and.satisfy((msg: string) => msg.startsWith('### [3.2.7]')); + expect(releases[0].path).to.eql('.'); }); it('should allow skipping releases', async () => { @@ -1916,6 +1922,7 @@ describe('Manifest', () => { expect(releases[0]!.tagName).to.eql('release-brancher-v1.3.1'); expect(releases[0]!.sha).to.eql('abc123'); expect(releases[0]!.notes).to.eql('some release notes'); + expect(releases[0]!.path).to.eql('.'); sinon.assert.calledOnce(commentStub); sinon.assert.calledOnceWithExactly( addLabelsStub, @@ -2020,15 +2027,19 @@ describe('Manifest', () => { expect(releases[0]!.tagName).to.eql('bot-config-utils-v3.2.0'); expect(releases[0]!.sha).to.eql('abc123'); expect(releases[0]!.notes).to.be.string; + expect(releases[0]!.path).to.eql('packages/bot-config-utils'); expect(releases[1]!.tagName).to.eql('label-utils-v1.1.0'); expect(releases[1]!.sha).to.eql('abc123'); expect(releases[1]!.notes).to.be.string; + expect(releases[1]!.path).to.eql('packages/label-utils'); expect(releases[2]!.tagName).to.eql('object-selector-v1.1.0'); expect(releases[2]!.sha).to.eql('abc123'); expect(releases[2]!.notes).to.be.string; + expect(releases[2]!.path).to.eql('packages/object-selector'); expect(releases[3]!.tagName).to.eql('datastore-lock-v2.1.0'); expect(releases[3]!.sha).to.eql('abc123'); expect(releases[3]!.notes).to.be.string; + expect(releases[3]!.path).to.eql('packages/datastore-lock'); sinon.assert.callCount(commentStub, 4); sinon.assert.calledOnceWithExactly( addLabelsStub, @@ -2081,6 +2092,7 @@ describe('Manifest', () => { expect(releases[0]!.tagName).to.eql('v3.2.7'); expect(releases[0]!.sha).to.eql('abc123'); expect(releases[0]!.notes).to.be.string; + expect(releases[0]!.path).to.eql('.'); sinon.assert.calledOnce(commentStub); sinon.assert.calledOnceWithExactly( addLabelsStub,