Skip to content

Commit

Permalink
feat: add owlbot template process check for Node
Browse files Browse the repository at this point in the history
  • Loading branch information
sofisl committed Apr 25, 2024
1 parent a1b9003 commit 94ec63b
Show file tree
Hide file tree
Showing 11 changed files with 309 additions and 18 deletions.
11 changes: 9 additions & 2 deletions packages/auto-approve/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ processes:
- "PythonSampleAppDependency"
- "JavaSampleAppDependency"
- "NodeGeneratorDependency"
- "OwlBotTemplateChangesNode"
```

These seven processes represent different workflows for what auto-approve will approve and merge in a given repository. To see their logic in full, see the corresponding file in /src/process-checks.
These processes represent different workflows for what auto-approve will approve and merge in a given repository. To see their logic in full, see the corresponding file in /src/process-checks.

Below is what each process checks for:

Expand Down Expand Up @@ -187,7 +188,13 @@ Below is what each process checks for:
- /package.json$/, /\.bzl$/, or /pnpm-lock\.yaml$/
- Increase the non-major package version of a dependency
- Change the dependency that was there previously, and that is on the title of the PR

* OwlBotTemplateChangesNode:
- Checks that the author is 'gcf-owl-bot[bot]'
- Checks that the title of the PR does NOT include BREAKING, feat, fix, !
- Checks that the title of the PR starts with chore, build, test, refactor
- Checks that the body of the PR does not contain 'PiperOrigin-RevId'
- Checks that the PR is the first of owlbot PRs in the repo (so that they are not merged out of order)
- Checks that there are no other commit authors other than owlbot on the repo


This change in configuration permits the following:
Expand Down
2 changes: 1 addition & 1 deletion packages/auto-approve/__snapshots__/check-config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ exports['check for config whether YAML file has valid schema should fail if ther
`

exports['check for config whether YAML file has valid schema V2 should fail if YAML has any other properties than the ones specified 1'] = `
[{"wrongProperty":{"allowedValues":["UpdateDiscoveryArtifacts","RegenerateReadme","DiscoveryDocUpdate","PythonDependency","PythonSampleDependency","NodeDependency","NodeRelease","JavaApiaryCodegen","JavaDependency","OwlBotTemplateChanges","OwlBotAPIChanges","PHPApiaryCodegen","PythonSampleAppDependency","JavaSampleAppDependency","DockerDependency","GoDependency","GoApiaryCodegen","NodeGeneratorDependency"]},"message":"must be equal to one of the allowed values"}]
[{"wrongProperty":{"allowedValues":["UpdateDiscoveryArtifacts","RegenerateReadme","DiscoveryDocUpdate","PythonDependency","PythonSampleDependency","NodeDependency","NodeRelease","JavaApiaryCodegen","JavaDependency","OwlBotTemplateChanges","OwlBotTemplateChangesNode","OwlBotAPIChanges","PHPApiaryCodegen","PythonSampleAppDependency","JavaSampleAppDependency","DockerDependency","GoDependency","GoApiaryCodegen","NodeGeneratorDependency"]},"message":"must be equal to one of the allowed values"}]
`

exports['check for config whether YAML file has valid schema V2 should fail if the property is wrong 1'] = `
Expand Down
5 changes: 5 additions & 0 deletions packages/auto-approve/src/check-pr-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {DockerDependency} from './process-checks/sample-application-repos/docker
import {NodeDependency} from './process-checks/node/dependency';
import {NodeGeneratorDependency} from './process-checks/node/generator-dependency';
import {NodeRelease} from './process-checks/node/release';
import {OwlBotTemplateChangesNode} from './process-checks/node/owlbot-template-changes';
import {JavaDependency} from './process-checks/java/dependency';
import {OwlBotTemplateChanges} from './process-checks/owl-bot-template-changes';
import {OwlBotAPIChanges} from './process-checks/owl-bot-api-changes';
Expand Down Expand Up @@ -68,6 +69,10 @@ const typeMap = [
configValue: 'NodeRelease',
configType: NodeRelease,
},
{
configValue: 'OwlBotTemplateChangesNode',
configType: OwlBotTemplateChangesNode,
},
{
configValue: 'JavaApiaryCodegen',
configType: JavaApiaryCodegen,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import {
checkAuthor,
checkTitleOrBody,
reportIndividualChecks,
getOpenPRsInRepoFromSameAuthor,
} from '../../utils-for-pr-checking';
import {listCommitsOnAPR} from '../../get-pr-info';
import {Octokit} from '@octokit/rest';
import {OwlBotTemplateChanges} from '../owl-bot-template-changes';

Expand All @@ -27,12 +29,14 @@ import {OwlBotTemplateChanges} from '../owl-bot-template-changes';
- has an author that is 'gcf-owl-bot[bot]'
- has a title that does NOT include BREAKING, or !
- has a PR body that does not contain 'PiperOrigin-RevId'
- is the first owlbot template PR in a repo (so they are merged in order)
- has no other commit authors on the PR
*/
export class OwlBotTemplateChangesNode extends OwlBotTemplateChanges {
classRule = {
author: 'gcf-owl-bot[bot]',
// For this particular rule, we want to check a pattern and an antipattern;
// we want it to start with regular commit convention,
// we want it to start with regular commit convention,
// and it should not be breaking or fix or feat
titleRegex: /$(chore|build|tests|refactor)/,
titleRegexExclude: /(fix|feat|breaking|!)/,
Expand Down Expand Up @@ -60,9 +64,51 @@ export class OwlBotTemplateChangesNode extends OwlBotTemplateChanges {
bodyMatches = checkTitleOrBody(incomingPR.body, this.classRule.bodyRegex);
}

const openOwlBotPRs = await getOpenPRsInRepoFromSameAuthor(
incomingPR.repoOwner,
incomingPR.repoName,
incomingPR.author,
this.octokit
);

let otherOwlBotPRs = true;
if (
openOwlBotPRs.length > 0 &&
incomingPR.prNumber === openOwlBotPRs[0].number
) {
otherOwlBotPRs = false;
}

const commitsOnPR = await listCommitsOnAPR(
incomingPR.repoOwner,
incomingPR.repoName,
incomingPR.prNumber,
this.octokit
);

const commitAuthors = commitsOnPR.filter(
x => x.author?.login !== this.classRule.author
);
let otherCommitAuthors = false;
if (commitAuthors.length > 0) {
otherCommitAuthors = true;
}

reportIndividualChecks(
['authorshipMatches', 'titleMatches', 'bodyMatches'],
[authorshipMatches, titleMatches, !bodyMatches],
[
'authorshipMatches',
'titleMatches',
'bodyMatches',
'otherOwlBotPRs',
'otherCommitAuthors',
],
[
authorshipMatches,
titleMatches,
!bodyMatches,
otherOwlBotPRs,
otherCommitAuthors,
],
incomingPR.repoOwner,
incomingPR.repoName,
incomingPR.prNumber
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class OwlBotAPIChanges extends BaseLanguageRule {
);

let otherOwlBotPRs = false;
if (openOwlBotPRs > 1) {
if (openOwlBotPRs.length > 1) {
otherOwlBotPRs = true;
}

Expand Down
6 changes: 4 additions & 2 deletions packages/auto-approve/src/utils-for-pr-checking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import timezone from 'dayjs/plugin/timezone';
import {logger} from 'gcf-utils';
import {Octokit} from '@octokit/rest';
import * as semver from 'semver';
import {components} from '@octokit/openapi-types/types';

dayjs.extend(utc);
dayjs.extend(timezone);
Expand Down Expand Up @@ -655,14 +656,15 @@ export async function getOpenPRsInRepoFromSameAuthor(
repo: string,
author: string,
octokit: Octokit
): Promise<number> {
): Promise<components['schemas']['pull-request-simple'][]> {
const otherPRs = await octokit.paginate(octokit.rest.pulls.list, {
owner,
repo,
state: 'open',
direction: 'asc',
});

const matchingPRs = otherPRs.filter(x => x.user?.login === author);

return matchingPRs.length;
return matchingPRs;
}
1 change: 1 addition & 0 deletions packages/auto-approve/src/valid-pr-schema-v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"JavaApiaryCodegen",
"JavaDependency",
"OwlBotTemplateChanges",
"OwlBotTemplateChangesNode",
"OwlBotAPIChanges",
"PHPApiaryCodegen",
"PythonSampleAppDependency",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1136,7 +1136,7 @@ describe('run additional versioning checks', () => {
describe('getting all open PRs in a repo from an author', async () => {
it('should return the number of PRs from anotherAuthor', async () => {
const prRequest = nock('https://api.github.com')
.get('/repos/testRepoOwner/testRepoName/pulls?state=open')
.get('/repos/testRepoOwner/testRepoName/pulls?state=open&direction=asc')
.reply(200, [{id: 1, user: {login: 'anotherAuthor'}}]);

const prCount = await getOpenPRsInRepoFromSameAuthor(
Expand All @@ -1147,12 +1147,12 @@ describe('run additional versioning checks', () => {
);

prRequest.done();
assert.deepStrictEqual(prCount, 1);
assert.deepStrictEqual(prCount.length, 1);
});

it('should return the number of PRs from someOtherAuthor', async () => {
const prRequest = nock('https://api.github.com')
.get('/repos/testRepoOwner/testRepoName/pulls?state=open')
.get('/repos/testRepoOwner/testRepoName/pulls?state=open&direction=asc')
.reply(200, [{id: 1, user: {login: 'anotherAuthor'}}]);

const prCount2 = await getOpenPRsInRepoFromSameAuthor(
Expand All @@ -1163,7 +1163,7 @@ describe('run additional versioning checks', () => {
);

prRequest.done();
assert.deepStrictEqual(prCount2, 0);
assert.deepStrictEqual(prCount2.length, 0);
});

it('should throw an error if prs are not received', async () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/auto-approve/test/check-pr-V2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function getPRsOnRepo(
response: {id: number; user: {login: string}}[]
) {
return nock('https://api.github.com')
.get(`/repos/${owner}/${repo}/pulls?state=open`)
.get(`/repos/${owner}/${repo}/pulls?state=open&direction=asc`)
.reply(200, response);
}

Expand Down Expand Up @@ -109,6 +109,9 @@ describe('check pr against config', async () => {
[{filename: 'requirements.txt', sha: '1234'}],
200
),
getPRsOnRepo('GoogleCloudPlatform', 'python-docs-samples-1', [
{id: 1, user: {login: 'gcf-owl-bot[bot]'}},
]),
listCommitsOnAPR('GoogleCloudPlatform', 'python-docs-samples-1', [
{author: {login: 'gcf-owl-bot[bot]'}},
]),
Expand All @@ -132,9 +135,6 @@ describe('check pr against config', async () => {
'ewogICAgIm5hbWUiOiAiZGxwIiwKICAgICJuYW1lX3ByZXR0eSI6ICJDbG91ZCBEYXRhIExvc3MgUHJldmVudGlvbiIsCiAgICAicHJvZHVjdF9kb2N1bWVudGF0aW9uIjogImh0dHBzOi8vY2xvdWQuZ29vZ2xlLmNvbS9kbHAvZG9jcy8iLAogICAgImNsaWVudF9kb2N1bWVudGF0aW9uIjogImh0dHBzOi8vY2xvdWQuZ29vZ2xlLmNvbS9weXRob24vZG9jcy9yZWZlcmVuY2UvZGxwL2xhdGVzdCIsCiAgICAiaXNzdWVfdHJhY2tlciI6ICIiLAogICAgInJlbGVhc2VfbGV2ZWwiOiAiZ2EiLAogICAgImxhbmd1YWdlIjogInB5dGhvbiIsCiAgICAibGlicmFyeV90eXBlIjogIkdBUElDX0FVVE8iLAogICAgInJlcG8iOiAiZ29vZ2xlYXBpcy9weXRob24tZGxwIiwKICAgICJkaXN0cmlidXRpb25fbmFtZSI6ICJnb29nbGUtY2xvdWQtZGxwIiwKICAgICJhcGlfaWQiOiAiZGxwLmdvb2dsZWFwaXMuY29tIiwKICAgICJyZXF1aXJlc19iaWxsaW5nIjogdHJ1ZSwKICAgICJkZWZhdWx0X3ZlcnNpb24iOiAidjIiLAogICAgImNvZGVvd25lcl90ZWFtIjogIiIKfQ==',
}
),
getPRsOnRepo('GoogleCloudPlatform', 'python-docs-samples-1', [
{id: 2, user: {login: 'gcf-owl-bot[bot]'}},
]),
];

const pr = require(resolve(
Expand Down

0 comments on commit 94ec63b

Please sign in to comment.