Skip to content

Commit

Permalink
Update WIP / Draft check to check for skip prefix in commit messages (#…
Browse files Browse the repository at this point in the history 
…74)

* update wip check to check for skip prefix in commit messages

* address review comments
  • Loading branch information
@jameesjohn
jameesjohn authored Jun 1, 2020
1 parent 65d328c commit 65d7a1b
Show file tree
Hide file tree
Showing 2 changed files with 205 additions and 32 deletions.
63 changes: 43 additions & 20 deletions lib/checkWipDraftPR.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,26 @@ const isWIPPr = ({ title, body }) => {
);
};

/**
* @param {import('probot').Context} context
*
*/
const isSkipCICommit = async (context) => {
/**
* @type {import('probot').Octokit.PullsGetResponse} pullRequest
*/
const pullRequest = context.payload.pull_request;

const commitParams = context.repo({
commit_sha: pullRequest.head.sha
});
const commitResponse = await context.github.git.getCommit(commitParams);

return (
commitResponse.data.message.startsWith('[ci skip]') ||
commitResponse.data.message.startsWith('[skip ci]'));
};

/**
* @param {import('probot').Context} context
*/
Expand All @@ -41,26 +61,29 @@ module.exports.checkWIP = async (context) => {
const prAuthor = pullRequest.user.login;

if (isDraftPr(pullRequest) || isWIPPr(pullRequest)) {
const link = 'here'.link(
'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia' +
'#wip--draft-pull-requests');
// Comment on Pull Request.
const commentBody = (
'Hi @' + prAuthor + ', WIP/Draft PRs are highly discouraged. You can ' +
'learn more about it ' + link + '. You can reopen it when it\'s ' +
'ready to be reviewed and ensure that it is without any WIP phrase ' +
'in title or body. Thanks!');
const commentParams = context.repo({
issue_number: pullRequest.number,
body: commentBody,
});
await context.github.issues.createComment(commentParams);
const hasSkipCIMessage = await isSkipCICommit(context);
if (!hasSkipCIMessage) {
const link = 'here'.link(
'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia' +
'#wip--draft-pull-requests');
// Comment on Pull Request.
const commentBody = (
'Hi @' + prAuthor + ', when creating WIP/Draft PRs, ensure that ' +
'your commit messages are prefixed with **[ci skip]** or ' +
'**[skip ci]** to prevent CI checks from running. ' +
'You can learn more about it ' + link + '.');
const commentParams = context.repo({
issue_number: pullRequest.number,
body: commentBody,
});
await context.github.issues.createComment(commentParams);

// Close Pull Request.
const closePRParams = context.repo({
issue_number: pullRequest.number,
state: 'closed',
});
await context.github.issues.update(closePRParams);
// Close Pull Request.
const closePRParams = context.repo({
issue_number: pullRequest.number,
state: 'closed',
});
await context.github.issues.update(closePRParams);
}
}
};
174 changes: 162 additions & 12 deletions spec/checkWipDraftPRSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,15 @@ describe('Oppiabot\'s', () => {
done();
});

describe('WIP PRs', () => {
describe('WIP PRs without skip prefix', () => {
beforeEach((done) => {
github.git = {
getCommit: jasmine.createSpy('getCommit').and.resolveTo({
data: {
message: 'commit without skip prefix'
}
})
};
spyOn(checkWipDraftPRModule, 'checkWIP').and.callThrough();
robot.receive(pullRequestEditedPayload);
done();
Expand All @@ -81,6 +88,16 @@ describe('Oppiabot\'s', () => {
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalledTimes(1);
});

it('calls get commit', () => {
expect(github.git.getCommit).toHaveBeenCalled();
expect(github.git.getCommit).toHaveBeenCalledTimes(1);
expect(github.git.getCommit).toHaveBeenCalledWith({
owner: pullRequestEditedPayload.payload.repository.owner.login,
repo: pullRequestEditedPayload.payload.repository.name,
commit_sha: pullRequestEditedPayload.payload.pull_request.head.sha
});
});

it('creates comment for WIP PRs', () => {
expect(github.issues.createComment).toHaveBeenCalled();
expect(github.issues.createComment).toHaveBeenCalledTimes(1);
Expand All @@ -91,10 +108,10 @@ describe('Oppiabot\'s', () => {
'#wip--draft-pull-requests');
const author = pullRequestEditedPayload.payload.pull_request.user.login;
const commentBody = (
'Hi @' + author + ', WIP/Draft PRs are highly discouraged. You can ' +
'learn more about it ' + link + '. You can reopen it when it\'s ' +
'ready to be reviewed and ensure that it is without any WIP phrase ' +
'in title or body. Thanks!');
'Hi @' + author + ', when creating WIP/Draft PRs, ensure that ' +
'your commit messages are prefixed with **[ci skip]** or ' +
'**[skip ci]** to prevent CI checks from running. ' +
'You can learn more about it ' + link + '.');

expect(github.issues.createComment).toHaveBeenCalledWith({
issue_number: pullRequestEditedPayload.payload.pull_request.number,
Expand All @@ -117,16 +134,63 @@ describe('Oppiabot\'s', () => {
});
});

describe('WIP PRs with skip prefix', () => {
beforeEach((done) => {
github.git = {
getCommit: jasmine.createSpy('getCommit').and.resolveTo({
data: {
message: '[ci skip] commit with skip prefix'
}
})
};
spyOn(checkWipDraftPRModule, 'checkWIP').and.callThrough();
robot.receive(pullRequestEditedPayload);
done();
});

it('calls checkWIP function', () =>{
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalled();
});

it('calls checkWIP once', () =>{
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalledTimes(1);
});

it('calls get commit', () => {
expect(github.git.getCommit).toHaveBeenCalled();
expect(github.git.getCommit).toHaveBeenCalledTimes(1);
expect(github.git.getCommit).toHaveBeenCalledWith({
owner: pullRequestEditedPayload.payload.repository.owner.login,
repo: pullRequestEditedPayload.payload.repository.name,
commit_sha: pullRequestEditedPayload.payload.pull_request.head.sha
});
});

it('does not create comment for WIP PRs', () => {
expect(github.issues.createComment).not.toHaveBeenCalled();
});

it('does not close WIP PRs', () => {
expect(github.issues.update).not.toHaveBeenCalled();
});
});

describe('Checks when PR is opened or reopened', () => {
it('should check when PR is opened', async () => {
github.git = {
getCommit: jasmine.createSpy('getCommit').and.resolveTo({
data: {
message: 'changes'
}
})
};
spyOn(checkWipDraftPRModule, 'checkWIP').and.callThrough();

// Trigger pull_request.opened event.
pullRequestEditedPayload.payload.action = 'opened';
await robot.receive(pullRequestEditedPayload);

expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalled();
expect(github.git.getCommit).toHaveBeenCalled();
expect(github.issues.createComment).toHaveBeenCalled();
expect(github.issues.createComment).toHaveBeenCalledTimes(1);
expect(github.issues.update).toHaveBeenCalled();
Expand All @@ -135,12 +199,19 @@ describe('Oppiabot\'s', () => {

it('should check when PR is reopnend', async() => {
spyOn(checkWipDraftPRModule, 'checkWIP').and.callThrough();

github.git = {
getCommit: jasmine.createSpy('getCommit').and.resolveTo({
data: {
message: 'commit without skip prefix'
}
})
};
// Trigger pull_request.opened event.
pullRequestEditedPayload.payload.action = 'reopened';
await robot.receive(pullRequestEditedPayload);

expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalled();
expect(github.git.getCommit).toHaveBeenCalled();
expect(github.issues.createComment).toHaveBeenCalled();
expect(github.issues.createComment).toHaveBeenCalledTimes(1);
expect(github.issues.update).toHaveBeenCalled();
Expand All @@ -150,6 +221,13 @@ describe('Oppiabot\'s', () => {

describe('Draft PRs', () => {
beforeEach((done) => {
github.git = {
getCommit: jasmine.createSpy('getCommit').and.resolveTo({
data: {
message: 'commit without skip prefix'
}
})
};
spyOn(checkWipDraftPRModule, 'checkWIP').and.callThrough();
// Receive a draft payload and remove WIP from title.
pullRequestEditedPayload.payload.pull_request.draft = true;
Expand All @@ -162,6 +240,16 @@ describe('Oppiabot\'s', () => {
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalled();
});

it('calls get commit', () => {
expect(github.git.getCommit).toHaveBeenCalled();
expect(github.git.getCommit).toHaveBeenCalledTimes(1);
expect(github.git.getCommit).toHaveBeenCalledWith({
owner: pullRequestEditedPayload.payload.repository.owner.login,
repo: pullRequestEditedPayload.payload.repository.name,
commit_sha: pullRequestEditedPayload.payload.pull_request.head.sha
});
});

it('calls checkWIP once', () => {
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalledTimes(1);
});
Expand All @@ -181,10 +269,10 @@ describe('Oppiabot\'s', () => {
'#wip--draft-pull-requests');
const author = pullRequestEditedPayload.payload.pull_request.user.login;
const commentBody = (
'Hi @' + author + ', WIP/Draft PRs are highly discouraged. You can ' +
'learn more about it ' + link + '. You can reopen it when it\'s ' +
'ready to be reviewed and ensure that it is without any WIP phrase ' +
'in title or body. Thanks!');
'Hi @' + author + ', when creating WIP/Draft PRs, ensure that ' +
'your commit messages are prefixed with **[ci skip]** or ' +
'**[skip ci]** to prevent CI checks from running. ' +
'You can learn more about it ' + link + '.');

expect(github.issues.createComment).toHaveBeenCalledWith({
issue_number: pullRequestEditedPayload.payload.pull_request.number,
Expand All @@ -207,8 +295,65 @@ describe('Oppiabot\'s', () => {
});
});

describe('Draft PRs with skip prefix', () => {
beforeEach((done) => {
github.git = {
getCommit: jasmine.createSpy('getCommit').and.resolveTo({
data: {
message: '[skip ci] commit with skip prefix'
}
})
};
spyOn(checkWipDraftPRModule, 'checkWIP').and.callThrough();
// Receive a draft payload and remove WIP from title.
pullRequestEditedPayload.payload.pull_request.draft = true;
pullRequestEditedPayload.payload.pull_request.title = 'Testing Draft';
robot.receive(pullRequestEditedPayload);
done();
});

it('calls checkWIP function', () => {
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalled();
});

it('calls checkWIP once', () => {
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalledTimes(1);
});

it('calls get commit', () => {
expect(github.git.getCommit).toHaveBeenCalled();
expect(github.git.getCommit).toHaveBeenCalledTimes(1);
expect(github.git.getCommit).toHaveBeenCalledWith({
owner: pullRequestEditedPayload.payload.repository.owner.login,
repo: pullRequestEditedPayload.payload.repository.name,
commit_sha: pullRequestEditedPayload.payload.pull_request.head.sha
});
});

it('calls with draft payload', () => {
const args = checkWipDraftPRModule.checkWIP.calls.argsFor(0)[0];
expect(args.payload.pull_request.draft).toBe(true);
expect(args.payload.pull_request.title).toBe('Testing Draft');
});

it('does not create comment for draft PRs', () => {
expect(github.issues.createComment).not.toHaveBeenCalled();
});

it('does not close draft PRs', () => {
expect(github.issues.update).not.toHaveBeenCalled();
});
});

describe('Neither Draft nor WIP PRs', () => {
beforeEach((done) => {
github.git = {
getCommit: jasmine.createSpy('getCommit').and.resolveTo({
data: {
message: '[skip ci] commit with skip prefix'
}
})
};
spyOn(checkWipDraftPRModule, 'checkWIP').and.callThrough();
// Receive a neither draft nor WIP payload.
pullRequestEditedPayload.payload.pull_request.draft = false;
Expand All @@ -220,12 +365,17 @@ describe('Oppiabot\'s', () => {
it('calls checkWIP function', () => {
expect(checkWipDraftPRModule.checkWIP).toHaveBeenCalled();
});

it('does not call get commit', () => {
expect(github.git.getCommit).not.toHaveBeenCalled();
});

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

it('does not close the PR', ()=> {
expect(github.issues.update).not.toHaveBeenCalled();
});
});

});

0 comments on commit 65d7a1b

Please sign in to comment.