Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: return path along with created release #1114

Merged
merged 3 commits into from
Nov 24, 2021
Merged

feat: return path along with created release #1114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c9ce89
feat: return path along with created release
chingor13 Nov 24, 2021
26a7d96
fix tests
chingor13 Nov 24, 2021
60aa7b7
test: add tests for returned release path
chingor13 Nov 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions src/manifest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export interface CandidateReleasePullRequest {
export interface CandidateRelease extends Release {
pullRequest: PullRequest;
draft?: boolean;
path: string;
}

interface ReleaserConfigJson {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 [];
Expand Down Expand Up @@ -554,7 +559,7 @@ export class Manifest {
}
logger.info(`found ${openPullRequests.length} open release pull requests.`);

const promises: Promise<number | undefined>[] = [];
const promises: Promise<PullRequest | undefined>[] = [];
for (const pullRequest of candidatePullRequests) {
promises.push(
this.createOrUpdatePullRequest(pullRequest, openPullRequests)
Expand All @@ -566,7 +571,7 @@ export class Manifest {
private async createOrUpdatePullRequest(
pullRequest: ReleasePullRequest,
openPullRequests: PullRequest[]
): Promise<number | undefined> {
): Promise<PullRequest | undefined> {
// look for existing, open pull rquest
const existing = openPullRequests.find(
openPullRequest =>
Expand All @@ -589,7 +594,7 @@ export class Manifest {
signoffUser: this.signoffUser,
}
);
return updatedPullRequest.number;
return updatedPullRequest;
} else {
const newPullRequest = await this.github.createReleasePullRequest(
pullRequest,
Expand All @@ -599,7 +604,7 @@ export class Manifest {
signoffUser: this.signoffUser,
}
);
return newPullRequest.number;
return newPullRequest;
}
}

Expand Down Expand Up @@ -669,6 +674,7 @@ export class Manifest {
if (release) {
releases.push({
...release,
path,
pullRequest,
draft: config.draft ?? this.draft,
});
Expand All @@ -686,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<number, CandidateRelease[]> = {};
const pullRequestsByNumber: Record<number, PullRequest> = {};
for (const release of await this.buildReleases()) {
Expand All @@ -698,7 +704,7 @@ export class Manifest {
}
}

const promises: Promise<GitHubRelease[]>[] = [];
const promises: Promise<CreatedRelease[]>[] = [];
for (const pullNumber in releasesByPullRequest) {
promises.push(
this.createReleasesForPullRequest(
Expand All @@ -714,9 +720,9 @@ export class Manifest {
private async createReleasesForPullRequest(
releases: CandidateRelease[],
pullRequest: PullRequest
): Promise<GitHubRelease[]> {
): Promise<CreatedRelease[]> {
// create the release
const promises: Promise<GitHubRelease>[] = [];
const promises: Promise<CreatedRelease>[] = [];
for (const release of releases) {
promises.push(this.createRelease(release));
}
Expand All @@ -733,7 +739,7 @@ export class Manifest {

private async createRelease(
release: CandidateRelease
): Promise<GitHubRelease> {
): Promise<CreatedRelease> {
const githubRelease = await this.github.createRelease(release, {
draft: release.draft,
});
Expand All @@ -742,7 +748,10 @@ export class Manifest {
const comment = `:robot: Release is at ${githubRelease.url} :sunflower:`;
await this.git.luolix.topmentOnIssue(comment, release.pullRequest.number);

return githubRelease;
return {
...githubRelease,
path: release.path,
};
}

private async getStrategiesByPath(): Promise<Record<string, Strategy>> {
Expand Down
53 changes: 39 additions & 14 deletions test/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -282,6 +292,7 @@ describe('CLI', () => {
sha: 'abc123',
notes: 'some release notes',
url: 'url-of-release',
path: '.',
},
]);
});
Expand Down Expand Up @@ -435,16 +446,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'
);
Expand All @@ -465,10 +486,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'
);
Expand All @@ -490,10 +507,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`
);
Expand Down Expand Up @@ -548,7 +561,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(
Expand Down Expand Up @@ -904,6 +927,7 @@ describe('CLI', () => {
sha: 'abc123',
notes: 'some release notes',
url: 'url-of-release',
path: '.',
},
]);
});
Expand Down Expand Up @@ -1054,6 +1078,7 @@ describe('CLI', () => {
sha: 'abc123',
notes: 'some release notes',
url: 'url-of-release',
path: '.',
},
]);
});
Expand Down
26 changes: 20 additions & 6 deletions test/manifest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -1531,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 () => {
Expand Down Expand Up @@ -1615,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 () => {
Expand Down Expand Up @@ -1668,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 () => {
Expand Down Expand Up @@ -1914,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,
Expand Down Expand Up @@ -2018,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,
Expand Down Expand Up @@ -2079,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,
Expand Down