Skip to content

Commit

Permalink
fix(monorepos): github-release not created
Browse files Browse the repository at this point in the history
fixes a couple of issues:
1. if node releaser doesn't specify packageName then github-release
wasn't filtering merged PRs correctly.
2. when solving the above, we need to be consistent about the
relationship between packageName and the release branch name.
  • Loading branch information
joeldodge79 committed Dec 16, 2020
1 parent 9ba60ee commit b22ceda
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 30 deletions.
28 changes: 20 additions & 8 deletions __snapshots__/github-release.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,13 @@ exports['GitHubRelease extractLatestReleaseNotes php-yoshi extracts appropriate
`

exports['GitHubRelease createRelease creates and labels release on GitHub 1'] = {
'tag_name': 'v1.0.3',
exports['GitHubRelease createRelease creates releases for submodule in monorepo 1'] = {
'tag_name': 'bigquery/v1.0.3',
'body': '\n* entry',
'name': 'foo v1.0.3'
'name': 'bigquery bigquery/v1.0.3'
}

exports['GitHubRelease createRelease creates and labels release on GitHub 2'] = {
exports['GitHubRelease createRelease creates releases for submodule in monorepo 2'] = {
'labels': [
'autorelease: tagged'
]
Expand All @@ -135,13 +135,25 @@ exports['GitHubRelease createRelease supports submodules in nested folders 2'] =
]
}

exports['GitHubRelease createRelease creates releases for submodule in monorepo 1'] = {
'tag_name': 'bigquery/v1.0.3',
exports['GitHubRelease createRelease creates and labels release on GitHub 1'] = {
'tag_name': 'v1.0.3',
'body': '\n* entry',
'name': 'bigquery bigquery/v1.0.3'
'name': 'foo v1.0.3'
}

exports['GitHubRelease createRelease creates releases for submodule in monorepo 2'] = {
exports['GitHubRelease createRelease creates and labels release on GitHub 2'] = {
'labels': [
'autorelease: tagged'
]
}

exports['GitHubRelease createRelease attempts to guess package name for submodule release 1'] = {
'tag_name': '@google-cloud/foo-v1.0.3',
'body': '\n* entry',
'name': '@google-cloud/foo @google-cloud/foo-v1.0.3'
}

exports['GitHubRelease createRelease attempts to guess package name for submodule release 2'] = {
'labels': [
'autorelease: tagged'
]
Expand Down
16 changes: 8 additions & 8 deletions src/github-release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ export class GitHubRelease {
}

async createRelease(): Promise<ReleaseResponse | undefined> {
// Attempt to lookup the package name from a well known location, such
// as package.json, if none is provided:
if (!this.packageName && this.releaseType) {
this.packageName = await ReleasePRFactory.class(
this.releaseType
).lookupPackageName(this.gh, this.path);
}

// In most configurations, createRelease() should be called close to when
// a release PR is merged, e.g., a GitHub action that kicks off this
// workflow on merge. For tis reason, we can pull a fairly small number of PRs:
Expand Down Expand Up @@ -111,14 +119,6 @@ export class GitHubRelease {
`found release notes: \n---\n${chalk.grey(latestReleaseNotes)}\n---\n`,
CheckpointType.Success
);

// Attempt to lookup the package name from a well known location, such
// as package.json, if none is provided:
if (!this.packageName && this.releaseType) {
this.packageName = await ReleasePRFactory.class(
this.releaseType
).lookupPackageName(this.gh);
}
// Go uses '/' for a tag separator, rather than '-':
let tagSeparator = '-';
if (this.releaseType) {
Expand Down
3 changes: 2 additions & 1 deletion src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import chalk = require('chalk');
import * as semver from 'semver';

import {checkpoint, CheckpointType} from './util/checkpoint';
import {packageBranch} from './util/package-branch';
import {
Commit,
CommitsResponse,
Expand Down Expand Up @@ -584,7 +585,7 @@ export class GitHub {
// it's easiest/safest to just pull this out by string search.
const version = match[2];
if (!version) continue;
if (prefix && match[1] !== prefix) {
if (prefix && match[1] !== packageBranch(prefix)) {
continue;
} else if (!prefix && match[1]) {
continue;
Expand Down
24 changes: 15 additions & 9 deletions src/release-pr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type PullsListResponseItems = PromiseValue<
import * as semver from 'semver';

import {checkpoint, CheckpointType} from './util/checkpoint';
import {packageBranch} from './util/package-branch';
import {ConventionalCommits} from './conventional-commits';
import {GitHub, GitHubTag, OctokitAPIs} from './github';
import {Commit} from './graphql-to-commits';
Expand Down Expand Up @@ -186,8 +187,12 @@ export class ReleasePR {
// A releaser can implement this method to automatically detect
// the release name when creating a GitHub release, for instance by returning
// name in package.json, or setup.py.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
static async lookupPackageName(gh: GitHub): Promise<string | undefined> {
static async lookupPackageName(
// eslint-disable-next-line @typescript-eslint/no-unused-vars
gh: GitHub,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
path?: string
): Promise<string | undefined> {
return Promise.resolve(undefined);
}

Expand Down Expand Up @@ -274,10 +279,7 @@ export class ReleasePR {
const version = options.version;
const includePackageName = options.includePackageName;
// Do not include npm style @org/ prefixes in the branch name:
const branchPrefix = this.packageName.match(/^@[\w-]+\//)
? this.packageName.split('/')[1]
: this.packageName;

const branchPrefix = packageBranch(this.packageName);
const title = includePackageName
? `chore: release ${this.packageName} ${version}`
: `chore: release ${version}`;
Expand Down Expand Up @@ -321,13 +323,17 @@ export class ReleasePR {
return changelogEntry.split('\n').length === 1;
}

addPath(file: string) {
if (this.path === undefined) {
static addPathStatic(file: string, path?: string) {
if (path === undefined) {
return file;
} else {
const path = this.path.replace(/[/\\]$/, '');
path = path.replace(/[/\\]$/, '');
file = file.replace(/^[/\\]/, '');
return `${path}/${file}`;
}
}

addPath(file: string) {
return ReleasePR.addPathStatic(file, this.path);
}
}
7 changes: 5 additions & 2 deletions src/releasers/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,14 @@ export class Node extends ReleasePR {
// A releaser can implement this method to automatically detect
// the release name when creating a GitHub release, for instance by returning
// name in package.json, or setup.py.
static async lookupPackageName(gh: GitHub): Promise<string | undefined> {
static async lookupPackageName(
gh: GitHub,
path?: string
): Promise<string | undefined> {
// Make an effort to populate packageName from the contents of
// the package.json, rather than forcing this to be set:
const contents: GitHubFileContents = await gh.getFileContents(
'package.json'
this.addPathStatic('package.json', path)
);
const pkg = JSON.parse(contents.parsedContent);
if (pkg.name) return pkg.name;
Expand Down
20 changes: 20 additions & 0 deletions src/util/package-branch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2020 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.

// map from a packageName to the prefix used in release branch creation.
export function packageBranch(packageName: string) {
return packageName.match(/^@[\w-]+\//)
? packageName.split('/')[1]
: packageName;
}
70 changes: 68 additions & 2 deletions test/github-release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,72 @@ describe('GitHubRelease', () => {
requests.done();
});

it('attempts to guess package name for submodule release', async () => {
const release = new GitHubRelease({
path: 'src/apis/foo',
label: 'autorelease: pending',
repoUrl: 'googleapis/foo',
apiUrl: 'https://api.github.com',
monorepoTags: true,
releaseType: 'node',
});
const requests = nock('https://api.github.com')
// check for default branch
.get('/repos/googleapis/foo')
.reply(200, repoInfo)
.get(
'/repos/googleapis/foo/pulls?state=closed&per_page=25&sort=merged_at&direction=desc'
)
.reply(200, [
{
labels: [{name: 'autorelease: pending'}],
head: {
label: 'head:release-foo-v1.0.3',
},
base: {
label: 'googleapis:main',
},
number: 1,
merged_at: new Date().toISOString(),
},
])
.get(
'/repos/googleapis/foo/contents/src%2Fapis%2Ffoo%2Fpackage.json?ref=refs/heads/main'
)
.reply(200, {
content: Buffer.from('{"name": "@google-cloud/foo"}', 'utf8'),
})
.get(
'/repos/googleapis/foo/contents/src%2Fapis%2Ffoo%2FCHANGELOG.md?ref=refs/heads/main'
)
.reply(200, {
content: Buffer.from('#Changelog\n\n## v1.0.3\n\n* entry', 'utf8'),
})
.post(
'/repos/googleapis/foo/releases',
(body: {[key: string]: string}) => {
snapshot(body);
return true;
}
)
.reply(200, {tag_name: 'v1.0.3'})
.post(
'/repos/googleapis/foo/issues/1/labels',
(body: {[key: string]: string}) => {
snapshot(body);
return true;
}
)
.reply(200)
.delete(
'/repos/googleapis/foo/issues/1/labels/autorelease%3A%20pending'
)
.reply(200);
const created = await release.createRelease();
strictEqual(created!.tag_name, 'v1.0.3');
requests.done();
});

it('attempts to guess package name for release', async () => {
const release = new GitHubRelease({
label: 'autorelease: pending',
Expand Down Expand Up @@ -260,7 +326,7 @@ describe('GitHubRelease', () => {
return true;
}
)
.reply(200, {tag_name: 'v1.0.2'})
.reply(200, {tag_name: 'v1.0.3'})
.post(
'/repos/googleapis/foo/issues/1/labels',
(body: {[key: string]: string}) => {
Expand All @@ -274,7 +340,7 @@ describe('GitHubRelease', () => {
)
.reply(200);
const created = await release.createRelease();
strictEqual(created!.tag_name, 'v1.0.2');
strictEqual(created!.tag_name, 'v1.0.3');
requests.done();
});
});
Expand Down
51 changes: 51 additions & 0 deletions test/releasers/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

import * as assert from 'assert';
import {describe, it, afterEach} from 'mocha';
import {expect} from 'chai';
import * as nock from 'nock';
import {GitHub} from '../../src/github';
import {Node} from '../../src/releasers/node';
import {readFileSync} from 'fs';
import {resolve} from 'path';
Expand Down Expand Up @@ -238,4 +240,53 @@ describe('Node', () => {
assert.strictEqual(pr, undefined);
});
});

describe('lookupPackageName', () => {
it('finds package name in package.json', async () => {
const github = new GitHub({owner: 'googleapis', repo: 'node-test-repo'});
const packageContent = readFileSync(
resolve(fixturesPath, 'package.json'),
'utf8'
);
const req = nock('https://api.github.com')
.get('/repos/googleapis/node-test-repo')
// eslint-disable-next-line @typescript-eslint/no-var-requires
.reply(200, require('../../../test/fixtures/repo-get-1.json'))
.get(
'/repos/googleapis/node-test-repo/contents/package.json?ref=refs/heads/master'
)
.reply(200, {
content: Buffer.from(packageContent, 'utf8').toString('base64'),
sha: 'abc123',
});
const expectedPackageName = await Node.lookupPackageName(github);
expect(expectedPackageName).to.equal('node-test-repo');
req.done();
});

it('finds package name in submodule package.json', async () => {
const github = new GitHub({owner: 'googleapis', repo: 'node-test-repo'});
const packageContent = readFileSync(
resolve(fixturesPath, 'package.json'),
'utf8'
);
const req = nock('https://api.github.com')
.get('/repos/googleapis/node-test-repo')
// eslint-disable-next-line @typescript-eslint/no-var-requires
.reply(200, require('../../../test/fixtures/repo-get-1.json'))
.get(
'/repos/googleapis/node-test-repo/contents/some-path%2Fpackage.json?ref=refs/heads/master'
)
.reply(200, {
content: Buffer.from(packageContent, 'utf8').toString('base64'),
sha: 'abc123',
});
const expectedPackageName = await Node.lookupPackageName(
github,
'some-path'
);
expect(expectedPackageName).to.equal('node-test-repo');
req.done();
});
});
});
35 changes: 35 additions & 0 deletions test/util/package-branch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2020 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 {packageBranch} from '../../src/util/package-branch';
import {describe, it} from 'mocha';
import {expect} from 'chai';

describe('packageBranch', () => {
const inputs = [
// npm style package names get '@scope/' removed
['@foo/bar', 'bar'],
['@foo-baz/bar', 'bar'],
// currently anything else goes untouched
['foo/bar', 'foo/bar'],
['foobar', 'foobar'],
['', ''],
];
inputs.forEach(input => {
const [packageName, branchPrefix] = input;
it(`maps packageName(${packageName}) to branchPrefix(${branchPrefix})`, async () => {
expect(packageBranch(packageName)).to.equal(branchPrefix);
});
});
});

0 comments on commit b22ceda

Please sign in to comment.