Skip to content

Commit

Permalink
fix: pass group-pull-request-title-pattern to strategies to parse rel…
Browse files Browse the repository at this point in the history
…eases (googleapis#1760)

* test: add failing test for linked-versions + group-pull-request-title

* fix: pass configured group-pull-request-title-pattern to strategies to parse releases

Co-authored-by: Benjamin E. Coe <bencoe@google.com>
  • Loading branch information
chingor13 and bcoe authored Dec 1, 2022
1 parent 81edcc1 commit f7601e4
Show file tree
Hide file tree
Showing 7 changed files with 239 additions and 6 deletions.
28 changes: 28 additions & 0 deletions __snapshots__/linked-versions-group-title.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
exports['Plugin compatibility linked-versions and group-pull-request-title-pattern should find release to create 1'] = `
:robot: I have created a release *beep* *boop*
---
<details><summary>primary: 1.1.0</summary>
## [1.1.0](https://github.com/fake-owner/fake-repo/compare/primary-v1.0.0...primary-v1.1.0) (1983-10-10)
### Features
* some feature ([aaaaaa](https://github.com/fake-owner/fake-repo/commit/aaaaaa))
</details>
<details><summary>pkgA: 1.1.0</summary>
## [1.1.0](https://github.com/fake-owner/fake-repo/compare/pkgA-v1.0.0...pkgA-v1.1.0) (1983-10-10)
### Features
* some feature ([aaaaaa](https://github.com/fake-owner/fake-repo/commit/aaaaaa))
</details>
---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
`
4 changes: 3 additions & 1 deletion src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,9 @@ export class Manifest {
this.logger.debug(`type: ${config.releaseType}`);
this.logger.debug(`targetBranch: ${this.targetBranch}`);
const strategy = strategiesByPath[path];
const release = await strategy.buildRelease(pullRequest);
const release = await strategy.buildRelease(pullRequest, {
groupPullRequestTitlePattern: this.groupPullRequestTitlePattern,
});
if (release) {
releases.push({
...release,
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/linked-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,17 @@ export class LinkedVersions extends ManifestPlugin {
},
[[], []] as CandidateReleasePullRequest[][]
);
this.logger.info(
`found ${inScopeCandidates.length} linked-versions candidates`
);

// delegate to the merge plugin and add merged pull request
if (inScopeCandidates.length > 0) {
const merge = new Merge(
this.github,
this.targetBranch,
this.repositoryConfig,
`chore\${branch}: release ${this.groupName} libraries`
`chore\${scope}: release ${this.groupName} libraries`
);
const merged = await merge.run(inScopeCandidates);
outOfScopeCandidates.push(...merged);
Expand Down
10 changes: 7 additions & 3 deletions src/strategies/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {Strategy} from '../strategy';
import {Strategy, BuildReleaseOptions} from '../strategy';
import {GitHub} from '../github';
import {VersioningStrategy} from '../versioning-strategy';
import {Repository} from '../repository';
Expand Down Expand Up @@ -500,7 +500,8 @@ export abstract class BaseStrategy implements Strategy {
* @returns {Release} The candidate release.
*/
async buildRelease(
mergedPullRequest: PullRequest
mergedPullRequest: PullRequest,
options?: BuildReleaseOptions
): Promise<Release | undefined> {
if (this.skipGitHubRelease) {
this.logger.info('Release skipped from strategy config');
Expand All @@ -510,6 +511,9 @@ export abstract class BaseStrategy implements Strategy {
this.logger.error('Pull request should have been merged');
return;
}
const mergedTitlePattern =
options?.groupPullRequestTitlePattern ??
MANIFEST_PULL_REQUEST_TITLE_PATTERN;

const pullRequestTitle =
PullRequestTitle.parse(
Expand All @@ -519,7 +523,7 @@ export abstract class BaseStrategy implements Strategy {
) ||
PullRequestTitle.parse(
mergedPullRequest.title,
MANIFEST_PULL_REQUEST_TITLE_PATTERN,
mergedTitlePattern,
this.logger
);
if (!pullRequestTitle) {
Expand Down
9 changes: 8 additions & 1 deletion src/strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import {VersioningStrategy} from './versioning-strategy';
import {ChangelogNotes} from './changelog-notes';
import {Version} from './version';

export interface BuildReleaseOptions {
groupPullRequestTitlePattern?: string;
}

/**
* A strategy is responsible for determining which files are
* necessary to update in a release pull request.
Expand Down Expand Up @@ -51,7 +55,10 @@ export interface Strategy {
* @param {PullRequest} mergedPullRequest The merged release pull request.
* @returns {Release} The candidate release.
*/
buildRelease(mergedPullRequest: PullRequest): Promise<Release | undefined>;
buildRelease(
mergedPullRequest: PullRequest,
options?: BuildReleaseOptions
): Promise<Release | undefined>;

/**
* Return the component for this strategy. This may be a computed field.
Expand Down
13 changes: 13 additions & 0 deletions test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,19 @@ export function mockTags(
return sandbox.stub(github, 'tagIterator').returns(fakeGenerator());
}

export function mockPullRequests(
sandbox: sinon.SinonSandbox,
github: GitHub,
pullRequests: PullRequest[]
): sinon.SinonStub {
async function* fakeGenerator() {
for (const pullRequest of pullRequests) {
yield pullRequest;
}
}
return sandbox.stub(github, 'pullRequestIterator').returns(fakeGenerator());
}

export function mockReleaseData(count: number): ReleaseData[] {
const releaseData: ReleaseData[] = [];
const version = Version.parse('1.2.3');
Expand Down
176 changes: 176 additions & 0 deletions test/plugins/compatibility/linked-versions-group-title.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
// Copyright 2022 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 {describe, it, afterEach, beforeEach} from 'mocha';
import * as sinon from 'sinon';
import {GitHub} from '../../../src/github';
import {Manifest} from '../../../src/manifest';
import {Update} from '../../../src/update';
import {
buildGitHubFileContent,
mockReleases,
mockCommits,
safeSnapshot,
stubFilesFromFixtures,
mockPullRequests,
} from '../../helpers';
import {Version} from '../../../src/version';
import {CargoToml} from '../../../src/updaters/rust/cargo-toml';
import {parseCargoManifest} from '../../../src/updaters/rust/common';
import {expect} from 'chai';

const sandbox = sinon.createSandbox();
const fixturesPath = './test/fixtures/plugins/cargo-workspace';

export function buildMockPackageUpdate(
path: string,
fixtureName: string
): Update {
const cachedFileContents = buildGitHubFileContent(fixturesPath, fixtureName);
const manifest = parseCargoManifest(cachedFileContents.parsedContent);
return {
path,
createIfMissing: false,
cachedFileContents,
updater: new CargoToml({
version: Version.parse(manifest.package?.version || 'FIXME'),
}),
};
}

describe('Plugin compatibility', () => {
let github: GitHub;
beforeEach(async () => {
github = await GitHub.create({
owner: 'fake-owner',
repo: 'fake-repo',
defaultBranch: 'main',
});
});
afterEach(() => {
sandbox.restore();
});
describe('linked-versions and group-pull-request-title-pattern', () => {
it('should find release to create', async () => {
// Scenario:
// - package b depends on a
// - package a receives a new feature
// - package b version bumps its dependency on a
// - package a and b should both use a minor version bump
mockReleases(sandbox, github, [
{
id: 123456,
sha: 'abc123',
tagName: 'primary-v1.0.0',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/primary-v1.0.0',
},
{
id: 654321,
sha: 'abc123',
tagName: 'pkgA-v1.0.0',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkgA-v1.0.0',
},
]);
mockCommits(sandbox, github, [
{
sha: 'aaaaaa',
message: 'feat: some feature',
files: ['packages/nodeA/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',
},
},
]);
stubFilesFromFixtures({
sandbox,
github,
fixturePath: fixturesPath,
files: [],
flatten: false,
targetBranch: 'main',
inlineFiles: [
['package.json', '{"name": "primary", "version": "1.0.0"}'],
[
'packages/nodeA/package.json',
'{"name": "pkgA", "version": "1.0.0"}',
],
],
});
const manifest = new Manifest(
github,
'main',
{
'.': {
releaseType: 'node',
component: 'primary',
},
'packages/nodeA': {
releaseType: 'node',
component: 'pkgA',
},
},
{
'.': Version.parse('1.0.0'),
'packages/nodeA': Version.parse('1.0.0'),
},
{
plugins: [
{
type: 'linked-versions',
groupName: 'my group',
components: ['primary', 'pkgA'],
},
],
groupPullRequestTitlePattern: 'chore: Release${component} ${version}',
}
);
const pullRequests = await manifest.buildPullRequests();
expect(pullRequests).lengthOf(1);
const pullRequest = pullRequests[0];
safeSnapshot(pullRequest.body.toString());
expect(pullRequest.title.toString()).to.equal(
'chore: Release primary 1.1.0'
);

console.log('-----------------------------------');

mockPullRequests(sandbox, github, [
{
headBranchName: pullRequest.headRefName,
baseBranchName: 'main',
number: 1234,
title: pullRequest.title.toString(),
body: pullRequest.body.toString(),
labels: pullRequest.labels,
files: pullRequest.updates.map(update => update.path),
sha: 'cccccc',
},
]);
const releases = await manifest.buildReleases();
expect(releases).lengthOf(2);
});
});
});

0 comments on commit f7601e4

Please sign in to comment.