Skip to content

Commit

Permalink
fix: allow configuring multiple, separate linked-versions plugins (#1961
Browse files Browse the repository at this point in the history
)

* test: failing test for configuring multiple, separate linked-versions plugins

* fix: allow configuring multiple, separate linked-versions plugins

* test: failing test for having separate branch names

* fix: grouped pull requests should have different branch names

* chore: remove debugging logs

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
chingor13 and gcf-owl-bot[bot] committed May 26, 2023
1 parent a0ccd92 commit a3fac52
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 13 deletions.
58 changes: 58 additions & 0 deletions __snapshots__/linked-versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,64 @@ exports['LinkedVersions plugin can skip grouping pull requests 3'] = `
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
`

exports['LinkedVersions plugin should allow multiple groups of linked versions 1'] = `
:robot: I have created a release *beep* *boop*
---
<details><summary>pkg1: 1.0.1</summary>
## [1.0.1](https://github.com/fake-owner/fake-repo/compare/pkg1-v1.0.0...pkg1-v1.0.1) (1983-10-10)
### Bug Fixes
* some bugfix ([aaaaaa](https://github.com/fake-owner/fake-repo/commit/aaaaaa))
</details>
<details><summary>pkg4: 1.0.1</summary>
## [1.0.1](https://github.com/fake-owner/fake-repo/compare/pkg4-v1.0.0...pkg4-v1.0.1) (1983-10-10)
### Miscellaneous Chores
* **pkg4:** Synchronize second group name versions
</details>
---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
`

exports['LinkedVersions plugin should allow multiple groups of linked versions 2'] = `
:robot: I have created a release *beep* *boop*
---
<details><summary>pkg2: 0.2.4</summary>
## [0.2.4](https://github.com/fake-owner/fake-repo/compare/pkg2-v0.2.3...pkg2-v0.2.4) (1983-10-10)
### Bug Fixes
* some bugfix ([bbbbbb](https://github.com/fake-owner/fake-repo/commit/bbbbbb))
</details>
<details><summary>pkg3: 0.2.4</summary>
## [0.2.4](https://github.com/fake-owner/fake-repo/compare/pkg3-v0.2.3...pkg3-v0.2.4) (1983-10-10)
### Miscellaneous Chores
* **pkg3:** Synchronize group name versions
</details>
---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
`

exports['LinkedVersions plugin should group pull requests 1'] = `
:robot: I have created a release *beep* *boop*
---
Expand Down
9 changes: 3 additions & 6 deletions src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,12 +746,9 @@ export class Manifest {
// pull requests
if (!this.separatePullRequests) {
this.plugins.push(
new Merge(
this.github,
this.targetBranch,
this.repositoryConfig,
this.groupPullRequestTitlePattern
)
new Merge(this.github, this.targetBranch, this.repositoryConfig, {
pullRequestTitlePattern: this.groupPullRequestTitlePattern,
})
);
}

Expand Down
11 changes: 10 additions & 1 deletion src/plugins/linked-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {Release} from '../release';
import {Version} from '../version';
import {buildStrategy} from '../factory';
import {Merge} from './merge';
import {BranchName} from '../util/branch-name';

interface LinkedVersionsPluginOptions {
merge?: boolean;
Expand Down Expand Up @@ -154,6 +155,7 @@ export class LinkedVersions extends ManifestPlugin {
(collection, candidate) => {
if (!candidate.pullRequest.version) {
this.logger.warn('pull request missing version', candidate);
collection[1].push(candidate);
return collection;
}
if (this.components.has(candidate.config.component || '')) {
Expand All @@ -175,7 +177,14 @@ export class LinkedVersions extends ManifestPlugin {
this.github,
this.targetBranch,
this.repositoryConfig,
`chore\${scope}: release ${this.groupName} libraries`
{
pullRequestTitlePattern: `chore\${scope}: release ${this.groupName} libraries`,
forceMerge: true,
headBranchName: BranchName.ofGroupTargetBranch(
this.groupName,
this.targetBranch
).toString(),
}
);
const merged = await merge.run(inScopeCandidates);
outOfScopeCandidates.push(...merged);
Expand Down
24 changes: 18 additions & 6 deletions src/plugins/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ import {Update} from '../update';
import {mergeUpdates} from '../updaters/composite';
import {GitHub} from '../github';

interface MergeOptions {
pullRequestTitlePattern?: string;
pullRequestHeader?: string;
headBranchName?: string;
forceMerge?: boolean;
}

/**
* This plugin merges multiple pull requests into a single
* release pull request.
Expand All @@ -35,18 +42,21 @@ import {GitHub} from '../github';
export class Merge extends ManifestPlugin {
private pullRequestTitlePattern?: string;
private pullRequestHeader?: string;
private headBranchName?: string;
private forceMerge: boolean;

constructor(
github: GitHub,
targetBranch: string,
repositoryConfig: RepositoryConfig,
pullRequestTitlePattern?: string,
pullRequestHeader?: string
options: MergeOptions = {}
) {
super(github, targetBranch, repositoryConfig);
this.pullRequestTitlePattern =
pullRequestTitlePattern || MANIFEST_PULL_REQUEST_TITLE_PATTERN;
this.pullRequestHeader = pullRequestHeader;
options.pullRequestTitlePattern ?? MANIFEST_PULL_REQUEST_TITLE_PATTERN;
this.pullRequestHeader = options.pullRequestHeader;
this.headBranchName = options.headBranchName;
this.forceMerge = options.forceMerge ?? false;
}

async run(
Expand All @@ -61,7 +71,7 @@ export class Merge extends ManifestPlugin {
Array<Array<CandidateReleasePullRequest>>
>(
(collection, candidate) => {
if (candidate.config.separatePullRequests) {
if (candidate.config.separatePullRequests && !this.forceMerge) {
collection[1].push(candidate);
} else {
collection[0].push(candidate);
Expand Down Expand Up @@ -101,7 +111,9 @@ export class Merge extends ManifestPlugin {
}),
updates,
labels: Array.from(labels),
headRefName: BranchName.ofTargetBranch(this.targetBranch).toString(),
headRefName:
this.headBranchName ??
BranchName.ofTargetBranch(this.targetBranch).toString(),
draft: !candidates.some(candidate => !candidate.pullRequest.draft),
};

Expand Down
33 changes: 33 additions & 0 deletions src/util/branch-name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function getAllResourceNames(): BranchNameType[] {
return [
AutoreleaseBranchName,
ComponentBranchName,
GroupBranchName,
DefaultBranchName,
V12ComponentBranchName,
V12DefaultBranchName,
Expand Down Expand Up @@ -78,6 +79,13 @@ export class BranchName {
`${RELEASE_PLEASE}--branches--${targetBranch}--components--${component}`
);
}
static ofGroupTargetBranch(group: string, targetBranch: string): BranchName {
return new GroupBranchName(
`${RELEASE_PLEASE}--branches--${targetBranch}--groups--${safeBranchName(
group
)}`
);
}
constructor(_branchName: string) {}

static matches(_branchName: string): boolean {
Expand Down Expand Up @@ -211,3 +219,28 @@ class ComponentBranchName extends BranchName {
return `${RELEASE_PLEASE}--branches--${this.targetBranch}--components--${this.component}`;
}
}

const GROUP_PATTERN = `^${RELEASE_PLEASE}--branches--(?<branch>.+)--groups--(?<group>.+)$`;
class GroupBranchName extends BranchName {
static matches(branchName: string): boolean {
return !!branchName.match(GROUP_PATTERN);
}
constructor(branchName: string) {
super(branchName);
const match = branchName.match(GROUP_PATTERN);
if (match?.groups) {
this.targetBranch = match.groups['branch'];
this.component = match.groups['group'];
}
}
toString(): string {
return `${RELEASE_PLEASE}--branches--${this.targetBranch}--groups--${this.component}`;
}
}

function safeBranchName(branchName: string): string {
// convert disallowed characters in branch names, replacing them with '-'.
// replace multiple consecutive '-' with a single '-' to avoid interfering with
// our regexes for parsing the branch names
return branchName.replace(/[^\w\d]/g, '-').replace(/-+/g, '-');
}
81 changes: 81 additions & 0 deletions test/plugins/linked-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ describe('LinkedVersions plugin', () => {
tagName: 'pkg3-v0.2.3',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg3-v0.2.3',
},
{
id: 4,
sha: 'abc123',
tagName: 'pkg4-v1.0.0',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg1-v1.0.0',
},
]);
mockCommits(sandbox, github, [
{
Expand Down Expand Up @@ -265,4 +271,79 @@ describe('LinkedVersions plugin', () => {
safeSnapshot(pullRequest.body.toString());
}
});
it('should allow multiple groups of linked versions', async () => {
const manifest = new Manifest(
github,
'target-branch',
{
'path/a': {
releaseType: 'simple',
component: 'pkg1',
},
'path/b': {
releaseType: 'simple',
component: 'pkg2',
},
'path/c': {
releaseType: 'simple',
component: 'pkg3',
},
'path/d': {
releaseType: 'simple',
component: 'pkg4',
},
},
{
'path/a': Version.parse('1.0.0'),
'path/b': Version.parse('0.2.3'),
'path/c': Version.parse('0.2.3'),
'path/d': Version.parse('1.0.0'),
},
{
separatePullRequests: true,
plugins: [
{
type: 'linked-versions',
groupName: 'group name',
components: ['pkg2', 'pkg3'],
},
{
type: 'linked-versions',
groupName: 'second group name',
components: ['pkg1', 'pkg4'],
},
],
}
);
const pullRequests = await manifest.buildPullRequests();
expect(pullRequests).lengthOf(2);
const groupPullRequest1 = pullRequests[1];
const packageData1 = groupPullRequest1.body.releaseData.find(
data => data.component === 'pkg1'
);
expect(packageData1).to.not.be.undefined;
const packageData4 = groupPullRequest1.body.releaseData.find(
data => data.component === 'pkg4'
);
expect(packageData4).to.not.be.undefined;
safeSnapshot(groupPullRequest1.body.toString());

const groupPullRequest2 = pullRequests[0];
const packageData2 = groupPullRequest2.body.releaseData.find(
data => data.component === 'pkg2'
);
expect(packageData2).to.not.be.undefined;
const packageData3 = groupPullRequest2.body.releaseData.find(
data => data.component === 'pkg3'
);
expect(packageData3).to.not.be.undefined;
expect(packageData2?.version).to.eql(packageData3?.version);
safeSnapshot(groupPullRequest2.body.toString());

expect(groupPullRequest1.headRefName).not.to.eql(
groupPullRequest2.headRefName
);
expect(groupPullRequest1.headRefName).to.not.include(' ');
expect(groupPullRequest2.headRefName).to.not.include(' ');
});
});

0 comments on commit a3fac52

Please sign in to comment.