From 8eb24955ac0516d7a7c1f671ca1737879ccfc011 Mon Sep 17 00:00:00 2001 From: Jonathan Hiles Date: Sun, 30 Jul 2023 23:05:22 +1000 Subject: [PATCH 1/3] test: add failing tests for proposed 'Component' commit footer See googleapis/release-please#1905 --- test/manifest.ts | 102 ++++++++++++++++++++++++++++++++++++++ test/util/commit-split.ts | 57 ++++++++++++++++++++- 2 files changed, 158 insertions(+), 1 deletion(-) diff --git a/test/manifest.ts b/test/manifest.ts index d1ecc6ebb..cfd37fe33 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -1814,6 +1814,108 @@ describe('Manifest', () => { snapshot(dateSafe(pullRequests[0].body.toString())); }); + it('should handle multiple package repository with Component commit footers', async () => { + mockReleases(sandbox, github, [ + { + id: 123456, + sha: 'abc123', + tagName: 'pkg1-v1.0.0', + url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg1-v1.0.0', + }, + { + id: 654321, + sha: 'def234', + tagName: 'pkg2-v0.2.3', + url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg2-v0.2.3', + }, + { + id: 987654, + sha: 'def234', + tagName: 'pkg3-v0.1.2', + url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg3-v0.1.2', + }, + ]); + mockCommits(sandbox, github, [ + { + sha: 'aaaaaa', + message: 'fix: some bugfix', + files: ['path/a/foo'], + }, + { + sha: 'abc123', + message: 'chore: release main', + files: [], + pullRequest: { + headBranchName: 'release-please/branches/main', + baseBranchName: 'main', + number: 123, + title: 'chore: release main', + body: '', + labels: [], + files: [], + sha: 'abc123', + }, + }, + { + sha: 'bbbbbb', + message: 'fix: some bugfix', + files: ['path/b/foo'], + }, + { + sha: 'cccccc', + message: 'fix: some bugfix', + files: ['path/a/foo'], + }, + { + sha: 'dddddd', + message: 'chore(pkg3): transition to beta\n\nComponent: pkg3\nRelease-As: 0.1.2-beta', + files: [], + }, + { + sha: 'def234', + message: 'chore: release main', + files: [], + pullRequest: { + headBranchName: 'release-please/branches/main', + baseBranchName: 'main', + number: 123, + title: 'chore: release main', + body: '', + labels: [], + files: [], + sha: 'def234', + }, + }, + ]); + const manifest = new Manifest( + github, + 'main', + { + 'path/a': { + releaseType: 'simple', + component: 'pkg1', + }, + 'path/b': { + releaseType: 'simple', + component: 'pkg2', + }, + 'path/c': { + releaseType: 'simple', + component: 'pkg3', + }, + }, + { + 'path/a': Version.parse('1.0.0'), + 'path/b': Version.parse('0.2.3'), + 'path/c': Version.parse('0.1.2'), + } + ); + const pullRequests = await manifest.buildPullRequests(); + expect(pullRequests).lengthOf(1); + expect(pullRequests[0].labels).to.eql(['autorelease: pending']); + snapshot(dateSafe(pullRequests[0].body.toString())); + }); + it('should allow creating multiple pull requests', async () => { mockReleases(sandbox, github, [ { diff --git a/test/util/commit-split.ts b/test/util/commit-split.ts index a8ea1ccf9..17f6281aa 100644 --- a/test/util/commit-split.ts +++ b/test/util/commit-split.ts @@ -98,8 +98,35 @@ describe('CommitSplit', () => { expect(splitCommits['pkg3']).to.be.undefined; expect(splitCommits['pkg4']).lengthOf(1); }); + it('should separate commits by Component footer', () => { + const commitSplit = new CommitSplit({includeEmpty: true}); + const splitCommits = commitSplit.split([ + ...commits, + {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, + ]); + expect(splitCommits['pkg1']).lengthOf(4); /* commits: abc123, def234, efg, klm */ + expect(splitCommits['pkg1'][3].sha).to.eql('klm'); + expect(splitCommits['pkg2']).lengthOf(2); /* commits: abc123, klm */ + expect(splitCommits['pkg3']).lengthOf(2); /* commits: def234, efg */ + expect(splitCommits['pkg4']).lengthOf(1); /* commits: nop */ + expect(splitCommits['pkg4'][0].sha).to.eql('nop'); + }); + it('should separate commits by Component footer with limited list of paths', () => { + const commitSplit = new CommitSplit({includeEmpty: true, packagePaths: ['pkg1', 'pkg4']}); + const splitCommits = commitSplit.split([ + ...commits, + {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, + ]); + expect(splitCommits['pkg1']).lengthOf(4); /* commits: abc123, def234, efg, klm */ + expect(splitCommits['pkg1'][3].sha).to.eql('klm'); + expect(splitCommits['pkg2']).to.be.undefined; + expect(splitCommits['pkg3']).to.be.undefined; + expect(splitCommits['pkg4']).lengthOf(1); /* commits: nop */ + expect(splitCommits['pkg4'][0].sha).to.eql('nop'); + }); }); - describe('excluding empty commits', () => { it('should separate commits', () => { const commitSplit = new CommitSplit({ @@ -122,5 +149,33 @@ describe('CommitSplit', () => { expect(splitCommits['pkg3']).to.be.undefined; expect(splitCommits['pkg4']).to.be.undefined; }); + it('should separate commits by Component footer', () => { + const commitSplit = new CommitSplit({includeEmpty: false}); + const splitCommits = commitSplit.split([ + ...commits, + {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, + ]); + expect(splitCommits['pkg1']).lengthOf(3); /* commits: abc123, def234, klm */ + expect(splitCommits['pkg1'][2].sha).to.eql('klm'); + expect(splitCommits['pkg2']).lengthOf(2); /* commits: abc123, klm */ + expect(splitCommits['pkg3']).lengthOf(1); /* commits: def234, efg */ + expect(splitCommits['pkg4']).lengthOf(1); /* commits: nop */ + expect(splitCommits['pkg4'][0].sha).to.eql('nop'); + }); + it('should separate commits by Component footer with limited list of paths', () => { + const commitSplit = new CommitSplit({includeEmpty: false, packagePaths: ['pkg1', 'pkg4']}); + const splitCommits = commitSplit.split([ + ...commits, + {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, + ]); + expect(splitCommits['pkg1']).lengthOf(3); /* commits: abc123, def234, klm */ + expect(splitCommits['pkg1'][2].sha).to.eql('klm'); + expect(splitCommits['pkg2']).to.be.undefined; + expect(splitCommits['pkg3']).to.be.undefined; + expect(splitCommits['pkg4']).lengthOf(1); /* commits: nop */ + expect(splitCommits['pkg4'][0].sha).to.eql('nop'); + }); }); }); From 5e67d02500a30e942fe28b3bcb46f9b2824c9fb8 Mon Sep 17 00:00:00 2001 From: Jonathan Hiles Date: Sun, 30 Jul 2023 23:06:37 +1000 Subject: [PATCH 2/3] mark where 'Component' commit footer logic takes place --- src/util/commit-split.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/commit-split.ts b/src/util/commit-split.ts index c8a2d5648..b190f2743 100644 --- a/src/util/commit-split.ts +++ b/src/util/commit-split.ts @@ -71,7 +71,9 @@ export class CommitSplit { * with a set of tracked package paths, then only consider paths for * configured components. If `includeEmpty` is configured, then a commit * that does not touch any files will be applied to all components' - * commits. + * commits. If a commit contains `Component: ...` footer(s), it will be + * included in those components as well any package paths it touches. + * * @param {Commit[]} commits The commits to split * @returns {Record} Commits indexed by component path */ @@ -106,6 +108,7 @@ export class CommitSplit { if (!splitCommits[pkgName]) splitCommits[pkgName] = []; splitCommits[pkgName].push(commit); } + // todo: handle 'Component: ...' commit footers if (commit.files.length === 0 && this.includeEmpty) { if (this.packagePaths) { for (const pkgName of this.packagePaths) { From 85174f5a53e5ccffe5deebffe34062fe3a0e42f5 Mon Sep 17 00:00:00 2001 From: Jonathan Hiles Date: Mon, 31 Jul 2023 12:58:01 +1000 Subject: [PATCH 3/3] style: apply linting --- test/manifest.ts | 3 ++- test/util/commit-split.ts | 50 +++++++++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/test/manifest.ts b/test/manifest.ts index cfd37fe33..3041dd58c 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -1868,7 +1868,8 @@ describe('Manifest', () => { }, { sha: 'dddddd', - message: 'chore(pkg3): transition to beta\n\nComponent: pkg3\nRelease-As: 0.1.2-beta', + message: + 'chore(pkg3): transition to beta\n\nComponent: pkg3\nRelease-As: 0.1.2-beta', files: [], }, { diff --git a/test/util/commit-split.ts b/test/util/commit-split.ts index 17f6281aa..af96edb54 100644 --- a/test/util/commit-split.ts +++ b/test/util/commit-split.ts @@ -102,10 +102,16 @@ describe('CommitSplit', () => { const commitSplit = new CommitSplit({includeEmpty: true}); const splitCommits = commitSplit.split([ ...commits, - {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + { + sha: 'klm', + message: 'commit klm\n\nComponent: pkg1', + files: ['pkg2/baz.txt'], + }, {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, ]); - expect(splitCommits['pkg1']).lengthOf(4); /* commits: abc123, def234, efg, klm */ + expect(splitCommits['pkg1']).lengthOf( + 4 /* commits: abc123, def234, efg, klm */ + ); expect(splitCommits['pkg1'][3].sha).to.eql('klm'); expect(splitCommits['pkg2']).lengthOf(2); /* commits: abc123, klm */ expect(splitCommits['pkg3']).lengthOf(2); /* commits: def234, efg */ @@ -113,13 +119,22 @@ describe('CommitSplit', () => { expect(splitCommits['pkg4'][0].sha).to.eql('nop'); }); it('should separate commits by Component footer with limited list of paths', () => { - const commitSplit = new CommitSplit({includeEmpty: true, packagePaths: ['pkg1', 'pkg4']}); + const commitSplit = new CommitSplit({ + includeEmpty: true, + packagePaths: ['pkg1', 'pkg4'], + }); const splitCommits = commitSplit.split([ ...commits, - {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + { + sha: 'klm', + message: 'commit klm\n\nComponent: pkg1', + files: ['pkg2/baz.txt'], + }, {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, ]); - expect(splitCommits['pkg1']).lengthOf(4); /* commits: abc123, def234, efg, klm */ + expect(splitCommits['pkg1']).lengthOf( + 4 /* commits: abc123, def234, efg, klm */ + ); expect(splitCommits['pkg1'][3].sha).to.eql('klm'); expect(splitCommits['pkg2']).to.be.undefined; expect(splitCommits['pkg3']).to.be.undefined; @@ -153,10 +168,16 @@ describe('CommitSplit', () => { const commitSplit = new CommitSplit({includeEmpty: false}); const splitCommits = commitSplit.split([ ...commits, - {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + { + sha: 'klm', + message: 'commit klm\n\nComponent: pkg1', + files: ['pkg2/baz.txt'], + }, {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, ]); - expect(splitCommits['pkg1']).lengthOf(3); /* commits: abc123, def234, klm */ + expect(splitCommits['pkg1']).lengthOf( + 3 /* commits: abc123, def234, klm */ + ); expect(splitCommits['pkg1'][2].sha).to.eql('klm'); expect(splitCommits['pkg2']).lengthOf(2); /* commits: abc123, klm */ expect(splitCommits['pkg3']).lengthOf(1); /* commits: def234, efg */ @@ -164,13 +185,22 @@ describe('CommitSplit', () => { expect(splitCommits['pkg4'][0].sha).to.eql('nop'); }); it('should separate commits by Component footer with limited list of paths', () => { - const commitSplit = new CommitSplit({includeEmpty: false, packagePaths: ['pkg1', 'pkg4']}); + const commitSplit = new CommitSplit({ + includeEmpty: false, + packagePaths: ['pkg1', 'pkg4'], + }); const splitCommits = commitSplit.split([ ...commits, - {sha: 'klm', message: 'commit klm\n\nComponent: pkg1', files: ['pkg2/baz.txt']}, + { + sha: 'klm', + message: 'commit klm\n\nComponent: pkg1', + files: ['pkg2/baz.txt'], + }, {sha: 'nop', message: 'commit nop\n\nComponent: pkg4', files: []}, ]); - expect(splitCommits['pkg1']).lengthOf(3); /* commits: abc123, def234, klm */ + expect(splitCommits['pkg1']).lengthOf( + 3 /* commits: abc123, def234, klm */ + ); expect(splitCommits['pkg1'][2].sha).to.eql('klm'); expect(splitCommits['pkg2']).to.be.undefined; expect(splitCommits['pkg3']).to.be.undefined;