Skip to content

Commit

Permalink
Prevent PRs from develop, test and release branches (#70)
Browse files Browse the repository at this point in the history
* check branch

* add license text

* use nyc binary

* write test for branch check

* address review comments

* modify comment string

* address review comments
  • Loading branch information
jameesjohn authored May 23, 2020
1 parent b3f29d8 commit 65d328c
Show file tree
Hide file tree
Showing 3 changed files with 271 additions and 0 deletions.
4 changes: 4 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const scheduler = require('./lib/scheduler');
const apiForSheetsModule = require('./lib/apiForSheets');
const checkMergeConflictsModule = require('./lib/checkMergeConflicts');
const checkPullRequestLabelsModule = require('./lib/checkPullRequestLabels');
const checkPullRequestBranchModule = require('./lib/checkPullRequestBranch');
const checkWIPModule = require('./lib/checkWipDraftPR');

const whitelistedAccounts = (
Expand All @@ -28,13 +29,16 @@ module.exports = (oppiabot) => {
if (whitelistedAccounts.includes(context.repo().owner.toLowerCase())) {
await apiForSheetsModule.checkClaStatus(context);
await checkPullRequestLabelsModule.checkChangelogLabel(context);
await checkPullRequestBranchModule.checkBranch(context);
await checkWIPModule.checkWIP(context);
}
});

oppiabot.on('pull_request.reopened', async context => {
if (whitelistedAccounts.includes(context.repo().owner.toLowerCase())) {
await checkPullRequestLabelsModule.checkChangelogLabel(context);
// Prevent user from reopening the PR.
await checkPullRequestBranchModule.checkBranch(context);
await checkWIPModule.checkWIP(context);
}
});
Expand Down
56 changes: 56 additions & 0 deletions lib/checkPullRequestBranch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2020 The Oppia Authors. All Rights Reserved.
//
// 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.

const DISSALLOWED_BRANCH_PREFIXES = ['develop', 'release-', 'test-'];

/**
* @param {import('probot').Context} context
*/
const checkBranch = async (context) => {
/**
* @type {import('probot').Octokit.PullsGetResponse} pullRequest
*/
const pullRequest = context.payload.pull_request;
const branchName = pullRequest.head.ref;
const branchIsInvalid = DISSALLOWED_BRANCH_PREFIXES.some((prefix) => {
return branchName.startsWith(prefix);
});

if (branchIsInvalid) {
const prAuthor = pullRequest.user.login;
// Comment on the pull request.
const wiki = 'wiki'.link(
'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' +
'instructions-for-making-a-code-change');
const commentParams = context.repo({
issue_number: pullRequest.number,
body:
'Hi @' + prAuthor + ', PRs made from develop, release or test ' +
'branches are not allowed. So this PR is being closed. Please make ' +
'your changes in another branch and send in the PR. To learn more ' +
'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!'
});
await context.github.issues.createComment(commentParams);
// Close the pull request.
const closeIssueParams = context.repo({
issue_number: pullRequest.number,
state: 'closed',
});
await context.github.issues.update(closeIssueParams);
}
};

module.exports = {
checkBranch,
};
211 changes: 211 additions & 0 deletions spec/checkPullRequestBranchSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
// Copyright 2020 The Oppia Authors. All Rights Reserved.
//
// 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.

require('dotenv').config();
const { createProbot } = require('probot');
// The plugin refers to the actual app in index.js.
const oppiaBot = require('../index');
const checkWipDraftPRModule = require('../lib/checkWipDraftPR');
const scheduler = require('../lib/scheduler');
const pullRequestPayload = require('../fixtures/pullRequestPayload.json');
const apiForSheetsModule = require('../lib/apiForSheets');
const checkPullRequestLabelsModule = require('../lib/checkPullRequestLabels');
const checkPullRequestBranchModule = require('../lib/checkPullRequestBranch');

describe('Pull Request Branch Check', () => {
/**
* @type {import('probot').Probot} robot
*/
let robot;

/**
* @type {import('probot').Octokit} github
*/
let github;

/**
* @type {import('probot').Application} app
*/
let app;

beforeEach(() => {
spyOn(scheduler, 'createScheduler').and.callFake(() => {});

// Spy on other modules that will be triggered by the payload.
spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => {});
spyOn(
checkPullRequestLabelsModule,
'checkChangelogLabel'
).and.callFake(() => {});
spyOn(checkWipDraftPRModule, 'checkWIP').and.callFake(() => {});

github = {
issues: {
createComment: jasmine.createSpy('createComment').and.returnValue({}),
update: jasmine.createSpy('update').and.resolveTo({}),
},
};

robot = createProbot({
id: 1,
cert: 'test',
githubToken: 'test',
});

app = robot.load(oppiaBot);
spyOn(app, 'auth').and.resolveTo(github);
spyOn(checkPullRequestBranchModule, 'checkBranch').and.callThrough();
});

describe('Invalid branch name', () => {
describe('develop branch check', () => {
beforeEach(async () => {
pullRequestPayload.payload.pull_request.head.ref = 'develop';
await app.receive(pullRequestPayload);
});

it('should call appropriate module', async () => {
expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled();
});

it('should create appropriate comment', () => {
expect(github.issues.createComment).toHaveBeenCalled();
const author = pullRequestPayload.payload.pull_request.user.login;
const wiki = 'wiki'.link(
'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' +
'instructions-for-making-a-code-change');
const commentBody = (
'Hi @' + author + ', PRs made from develop, release or test ' +
'branches are not allowed. So this PR is being closed. Please make ' +
'your changes in another branch and send in the PR. To learn more ' +
'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!');
expect(github.issues.createComment).toHaveBeenCalledWith({
issue_number: pullRequestPayload.payload.pull_request.number,
owner: pullRequestPayload.payload.repository.owner.login,
repo: pullRequestPayload.payload.repository.name,
body: commentBody
});
});

it('should close pull request', () => {
expect(github.issues.update).toHaveBeenCalled();
expect(github.issues.update).toHaveBeenCalledWith({
issue_number: pullRequestPayload.payload.pull_request.number,
owner: pullRequestPayload.payload.repository.owner.login,
repo: pullRequestPayload.payload.repository.name,
state: 'closed',
});
});
});

describe('release branch check', () => {
beforeEach(async () => {
pullRequestPayload.payload.pull_request.head.ref = 'release-2';
await app.receive(pullRequestPayload);
});

it('should call appropriate module', async () => {
expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled();
});

it('should create appropriate comment', () => {
expect(github.issues.createComment).toHaveBeenCalled();
const author = pullRequestPayload.payload.pull_request.user.login;
const wiki = 'wiki'.link(
'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' +
'instructions-for-making-a-code-change');
const commentBody = (
'Hi @' + author + ', PRs made from develop, release or test ' +
'branches are not allowed. So this PR is being closed. Please make ' +
'your changes in another branch and send in the PR. To learn more ' +
'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!');
expect(github.issues.createComment).toHaveBeenCalledWith({
issue_number: pullRequestPayload.payload.pull_request.number,
owner: pullRequestPayload.payload.repository.owner.login,
repo: pullRequestPayload.payload.repository.name,
body: commentBody
});
});

it('should close pull request', () => {
expect(github.issues.update).toHaveBeenCalled();
expect(github.issues.update).toHaveBeenCalledWith({
issue_number: pullRequestPayload.payload.pull_request.number,
owner: pullRequestPayload.payload.repository.owner.login,
repo: pullRequestPayload.payload.repository.name,
state: 'closed',
});
});
});

describe('test branch check', () => {
beforeEach(async () => {
pullRequestPayload.payload.pull_request.head.ref = 'test-2';
await app.receive(pullRequestPayload);
});

it('should call appropriate module', async () => {
expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled();
});

it('should create appropriate comment', () => {
expect(github.issues.createComment).toHaveBeenCalled();
const author = pullRequestPayload.payload.pull_request.user.login;
const wiki = 'wiki'.link(
'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' +
'instructions-for-making-a-code-change');
const commentBody = (
'Hi @' + author + ', PRs made from develop, release or test ' +
'branches are not allowed. So this PR is being closed. Please make ' +
'your changes in another branch and send in the PR. To learn more ' +
'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!');
expect(github.issues.createComment).toHaveBeenCalledWith({
issue_number: pullRequestPayload.payload.pull_request.number,
owner: pullRequestPayload.payload.repository.owner.login,
repo: pullRequestPayload.payload.repository.name,
body: commentBody
});
});

it('should close pull request', () => {
expect(github.issues.update).toHaveBeenCalled();
expect(github.issues.update).toHaveBeenCalledWith({
issue_number: pullRequestPayload.payload.pull_request.number,
owner: pullRequestPayload.payload.repository.owner.login,
repo: pullRequestPayload.payload.repository.name,
state: 'closed',
});
});
});
});

describe('Valid branch name', () => {
beforeEach(async () => {
pullRequestPayload.payload.pull_request.head.ref = 'valid-branch';
await app.receive(pullRequestPayload);
});

it('should call appropriate module', async () => {
expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled();
});

it('should not create comment', () => {
expect(github.issues.createComment).not.toHaveBeenCalled();
});

it('should not close pull request', () => {
expect(github.issues.update).not.toHaveBeenCalled();
});
});
});

0 comments on commit 65d328c

Please sign in to comment.