Skip to content

Commit

Permalink
git-node: wait time for PR with single approval
Browse files Browse the repository at this point in the history
One Collaborator approval is enough only if the pull request
has been open for more than 7 days

Refs: nodejs/node#22255
  • Loading branch information
Alicia Lopez committed Dec 3, 2018
1 parent 82c16cf commit a0995b4
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 7 deletions.
28 changes: 22 additions & 6 deletions lib/pr_checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const MINUTE = SECOND * 60;
const HOUR = MINUTE * 60;

const WAIT_TIME = 48;
const WAIT_TIME_SINGLE_APPROVAL = 168;

const {
REVIEW_SOURCES: { FROM_COMMENT, FROM_REVIEW_COMMENT }
Expand Down Expand Up @@ -132,12 +133,16 @@ class PRChecker {
*/
getWait(now) {
const createTime = new Date(this.pr.createdAt);
const timeLeft = WAIT_TIME - Math.ceil(
const hoursFromCreateTime = Math.ceil(
(now.getTime() - createTime.getTime()) / HOUR
);
const timeLeft = WAIT_TIME - hoursFromCreateTime;
const timeLeftSingleApproval =
WAIT_TIME_SINGLE_APPROVAL - hoursFromCreateTime;

return {
timeLeft
timeLeft,
timeLeftSingleApproval
};
}

Expand All @@ -149,12 +154,12 @@ class PRChecker {
const {
pr, cli, reviewers, CIStatus
} = this;
const { approved } = reviewers;
const labels = pr.labels.nodes;

const fast =
labels.some((l) => ['fast-track'].includes(l.name));
if (fast) {
const { approved } = reviewers;
if (approved.length > 1 && CIStatus) {
cli.info('This PR is being fast-tracked');
return true;
Expand All @@ -171,14 +176,25 @@ class PRChecker {
return false;
}

const dateStr = new Date(pr.createdAt).toDateString();
cli.info(`This PR was created on ${dateStr}`);

const wait = this.getWait(now);
if (wait.timeLeft > 0) {
const dateStr = new Date(pr.createdAt).toDateString();
cli.info(`This PR was created on ${dateStr}`);
if (approved.length > 1 && wait.timeLeft > 0) {
cli.warn(`Wait at least ${wait.timeLeft} more hours before landing`);
return false;
} else if (approved.length === 1 && wait.timeLeftSingleApproval > 0) {
const waitMsg = wait.timeLeft > 0
? ` and ${wait.timeLeft} more hours`
: '';
cli.warn('Wait at one of the following:');
cli.warn(`* another approval${waitMsg}`);
cli.warn(`* ${wait.timeLeftSingleApproval} more hours` +
' with existing single approval');
return false;
}

cli.info('No more waiting required');
return true;
}

Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ const allGreenReviewers = {
approved,
requestedChanges: []
};
const singleGreenReviewer = {
approved: [approved[0]],
requestedChanges: []
};
const requestedChangesReviewers = {
requestedChanges,
approved: []
Expand Down Expand Up @@ -76,6 +80,7 @@ module.exports = {
approved,
requestedChanges,
allGreenReviewers,
singleGreenReviewer,
requestedChangesReviewers,
approvingReviews,
requestingChangesReviews,
Expand Down
77 changes: 76 additions & 1 deletion test/unit/pr_checker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const PRChecker = require('../../lib/pr_checker');

const {
allGreenReviewers,
singleGreenReviewer,
requestedChangesReviewers,
approvingReviews,
requestingChangesReviews,
Expand Down Expand Up @@ -169,7 +170,9 @@ describe('PRChecker', () => {
const cli = new TestCLI();

const expectedLogs = {
warn: [['Wait at least 22 more hours before landing']],
warn: [
['Wait at least 22 more hours before landing']
],
info: [['This PR was created on Tue Oct 31 2017']]
};

Expand Down Expand Up @@ -197,6 +200,78 @@ describe('PRChecker', () => {
cli.assertCalledWith(expectedLogs);
});

it('should warn about PR with single approval (<48h)', () => {
const cli = new TestCLI();

const expectedLogs = {
warn: [
['Wait at one of the following:'],
['* another approval and 22 more hours'],
['* 142 more hours with existing single approval']
],
info: [['This PR was created on Tue Oct 31 2017']]
};

const now = new Date('2017-11-01T14:25:41.682Z');
const youngPR = Object.assign({}, firstTimerPR, {
createdAt: '2017-10-31T13:00:41.682Z'
});

const data = {
pr: youngPR,
reviewers: singleGreenReviewer,
comments: commentsWithLGTM,
reviews: approvingReviews,
commits: simpleCommits,
collaborators,
authorIsNew: () => true,
getThread() {
return PRData.prototype.getThread.call(this);
}
};
const checker = new PRChecker(cli, data, argv);

const status = checker.checkPRWait(now);
assert(!status);
cli.assertCalledWith(expectedLogs);
});

it('should warn about PR with single approval (>48h)', () => {
const cli = new TestCLI();

const expectedLogs = {
warn: [
['Wait at one of the following:'],
['* another approval'],
['* 94 more hours with existing single approval']
],
info: [['This PR was created on Tue Oct 31 2017']]
};

const now = new Date('2017-11-03T14:25:41.682Z');
const youngPR = Object.assign({}, firstTimerPR, {
createdAt: '2017-10-31T13:00:41.682Z'
});

const data = {
pr: youngPR,
reviewers: singleGreenReviewer,
comments: commentsWithLGTM,
reviews: approvingReviews,
commits: simpleCommits,
collaborators,
authorIsNew: () => true,
getThread() {
return PRData.prototype.getThread.call(this);
}
};
const checker = new PRChecker(cli, data, argv);

const status = checker.checkPRWait(now);
assert(!status);
cli.assertCalledWith(expectedLogs);
});

it('should log as expected if PR can be fast-tracked', () => {
const cli = new TestCLI();

Expand Down

0 comments on commit a0995b4

Please sign in to comment.