From b845f486119402ca3140f57ac428eb42b727b58c Mon Sep 17 00:00:00 2001 From: cPhost <23620441+cPhost@users.noreply.github.com> Date: Fri, 10 Nov 2017 21:14:03 +0000 Subject: [PATCH 1/3] PRChecker: support new label fast-track --- lib/pr_checker.js | 6 +++++- test/unit/pr_checker.test.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 68b3db3a..67709e3a 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -153,7 +153,7 @@ class PRChecker { const { pr } = this; const { cli } = this; const labels = pr.labels.nodes; - const fast = labels.some((l) => l.name === 'code-and-learn') || + const fast = labels.some((l) => isFast(l.name)) || (labels.length === 1 && labels[0].name === 'doc'); if (fast) { return true; } const wait = this.getWait(now); @@ -165,6 +165,10 @@ class PRChecker { return false; } + function isFast(label) { + return (label === 'code-and-learn' || label === 'fast-track'); + } + return true; } diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 8109d67e..9129ed37 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -232,6 +232,35 @@ describe('PRChecker', () => { assert(status); cli.assertCalledWith(expectedLogs); }); + + it('should skip wait check for fast-track labelled PR', () => { + const cli = new TestCLI(); + + const expectedLogs = {}; + + const now = new Date(); + const youngPR = Object.assign({}, firstTimerPR, { + createdAt: '2017-10-27T14:25:41.682Z', + labels: { + nodes: [ + { name: 'fast-track' } + ] + } + }); + + const checker = new PRChecker(cli, { + pr: youngPR, + reviewers: allGreenReviewers, + comments: commentsWithLGTM, + reviews: approvingReviews, + commits: simpleCommits, + collaborators + }); + + const status = checker.checkPRWait(now); + assert(status); + cli.assertCalledWith(expectedLogs); + }); }); describe('checkCI', () => { From 4f2b7bc3fa6a515db5b405de3b4324c5e4ccdc99 Mon Sep 17 00:00:00 2001 From: cPhost <23620441+cPhost@users.noreply.github.com> Date: Fri, 10 Nov 2017 23:03:19 +0000 Subject: [PATCH 2/3] add warn msg for fast-track Pull Requests. --- lib/pr_checker.js | 14 ++++++++------ test/unit/pr_checker.test.js | 11 +++++++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 67709e3a..1fbcab1f 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -153,9 +153,15 @@ class PRChecker { const { pr } = this; const { cli } = this; const labels = pr.labels.nodes; - const fast = labels.some((l) => isFast(l.name)) || + + const fast = + labels.some((l) => ['code-and-learn', 'fast-track'].includes(l.name)) || (labels.length === 1 && labels[0].name === 'doc'); - if (fast) { return true; } + if (fast) { + cli.info('This PR is being fast-tracked.'); + return true; + } + const wait = this.getWait(now); if (wait.timeLeft > 0) { const dateStr = new Date(pr.createdAt).toDateString(); @@ -165,10 +171,6 @@ class PRChecker { return false; } - function isFast(label) { - return (label === 'code-and-learn' || label === 'fast-track'); - } - return true; } diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 9129ed37..0161b23f 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -204,7 +204,9 @@ describe('PRChecker', () => { it('should skip wait check for Code & Learn PR', () => { const cli = new TestCLI(); - const expectedLogs = {}; + const expectedLogs = { + info: [['This PR is being fast-tracked.']] + }; const now = new Date(); const youngPR = Object.assign({}, firstTimerPR, { @@ -236,9 +238,10 @@ describe('PRChecker', () => { it('should skip wait check for fast-track labelled PR', () => { const cli = new TestCLI(); - const expectedLogs = {}; + const expectedLogs = { + info: [['This PR is being fast-tracked.']] + }; - const now = new Date(); const youngPR = Object.assign({}, firstTimerPR, { createdAt: '2017-10-27T14:25:41.682Z', labels: { @@ -257,7 +260,7 @@ describe('PRChecker', () => { collaborators }); - const status = checker.checkPRWait(now); + const status = checker.checkPRWait(new Date()); assert(status); cli.assertCalledWith(expectedLogs); }); From 41cc6032bfc486a603d60f65fc3dc83065b72791 Mon Sep 17 00:00:00 2001 From: cPhost <23620441+cPhost@users.noreply.github.com> Date: Tue, 21 Nov 2017 21:30:49 +0000 Subject: [PATCH 3/3] fast-track, test: check per current expectation of fast-track PR --- lib/pr_checker.js | 29 ++++++-- test/unit/pr_checker.test.js | 124 +++++++++++++++++++++++++++++------ 2 files changed, 125 insertions(+), 28 deletions(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 1fbcab1f..0a516054 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -53,8 +53,8 @@ class PRChecker { const status = [ this.checkReviews(comments), this.checkCommitsAfterReview(), - this.checkPRWait(new Date()), this.checkCI(), + this.checkPRWait(new Date()), this.checkMergeableState() ]; @@ -150,16 +150,29 @@ class PRChecker { * @param {Date} now */ checkPRWait(now) { - const { pr } = this; - const { cli } = this; + const { + pr, cli, reviewers, CIStatus + } = this; const labels = pr.labels.nodes; const fast = - labels.some((l) => ['code-and-learn', 'fast-track'].includes(l.name)) || - (labels.length === 1 && labels[0].name === 'doc'); + labels.some((l) => ['fast-track'].includes(l.name)); if (fast) { - cli.info('This PR is being fast-tracked.'); - return true; + const { approved } = reviewers; + if (approved.length > 1 && CIStatus) { + cli.info('This PR is being fast-tracked'); + return true; + } else { + const msg = ['This PR is being fast-tracked, but awating ']; + if (approved.length < 2) msg.push('approvals of 2 contributors'); + if (!CIStatus) msg.push('a CI run'); + + let warnMsg = msg.length === 2 + ? msg.join('') : `${msg[0] + msg[1]} and ${msg[2]}`; + cli.warn(warnMsg); + } + + return false; } const wait = this.getWait(now); @@ -188,6 +201,7 @@ class PRChecker { let status = true; if (!ciMap.size) { cli.error('No CI runs detected'); + this.CIStatus = false; return false; } else if (!ciMap.get(FULL)) { status = false; @@ -237,6 +251,7 @@ class PRChecker { } } + this.CIStatus = status; return status; } diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 0161b23f..85156bf2 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -201,49 +201,55 @@ describe('PRChecker', () => { cli.assertCalledWith(expectedLogs); }); - it('should skip wait check for Code & Learn PR', () => { + it('should log as expected if PR can be fast-tracked', () => { const cli = new TestCLI(); const expectedLogs = { - info: [['This PR is being fast-tracked.']] + info: [ + [ 'This PR is being fast-tracked' ] + ] }; - const now = new Date(); - const youngPR = Object.assign({}, firstTimerPR, { - createdAt: '2017-10-27T14:25:41.682Z', + const now = new Date('2017-11-01T14:25:41.682Z'); + const PR = Object.assign({}, firstTimerPR, { + createdAt: '2017-10-31T13:00:41.682Z', labels: { nodes: [ - { - name: 'code-and-learn' - } + { name: 'fast-track' } ] } }); const options = { - pr: youngPR, + pr: PR, reviewers: allGreenReviewers, - comments: commentsWithLGTM, + comments: commentsWithCI, reviews: approvingReviews, - commits: simpleCommits, + commits: [], collaborators }; const checker = new PRChecker(cli, options, argv); + checker.checkCI(); + cli.clearCalls(); const status = checker.checkPRWait(now); assert(status); cli.assertCalledWith(expectedLogs); }); - it('should skip wait check for fast-track labelled PR', () => { + it('should warn about approvals and CI for fast-tracked PR', () => { const cli = new TestCLI(); const expectedLogs = { - info: [['This PR is being fast-tracked.']] + warn: [ + [ 'This PR is being fast-tracked, but awating ' + + 'approvals of 2 contributors and a CI run' ] + ] }; - const youngPR = Object.assign({}, firstTimerPR, { - createdAt: '2017-10-27T14:25:41.682Z', + const now = new Date('2017-11-01T14:25:41.682Z'); + const PR = Object.assign({}, firstTimerPR, { + createdAt: '2017-10-31T13:00:41.682Z', labels: { nodes: [ { name: 'fast-track' } @@ -251,17 +257,93 @@ describe('PRChecker', () => { } }); - const checker = new PRChecker(cli, { - pr: youngPR, + const options = { + pr: PR, + reviewers: requestedChangesReviewers, + comments: [], + reviews: requestingChangesReviews, + commits: simpleCommits, + collaborators + }; + const checker = new PRChecker(cli, options, argv); + + checker.checkCI(); + cli.clearCalls(); + const status = checker.checkPRWait(now); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should warn cannot be fast-tracked because of approvals', () => { + const cli = new TestCLI(); + + const expectedLogs = { + warn: [ + [ 'This PR is being fast-tracked, but awating ' + + 'approvals of 2 contributors' ] + ] + }; + + const now = new Date('2017-11-01T14:25:41.682Z'); + const PR = Object.assign({}, firstTimerPR, { + createdAt: '2017-10-31T13:00:41.682Z', + labels: { + nodes: [ + { name: 'fast-track' } + ] + } + }); + + const options = { + pr: PR, + reviewers: requestedChangesReviewers, + comments: commentsWithCI, + reviews: approvingReviews, + commits: [], + collaborators + }; + const checker = new PRChecker(cli, options, argv); + + checker.checkCI(); + cli.clearCalls(); + const status = checker.checkPRWait(now); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should warn if the PR has no CI and cannot be fast-tracked', () => { + const cli = new TestCLI(); + + const expectedLogs = { + warn: [ + [ 'This PR is being fast-tracked, but awating a CI run' ] + ] + }; + + const now = new Date('2017-11-01T14:25:41.682Z'); + const PR = Object.assign({}, firstTimerPR, { + createdAt: '2017-10-31T13:00:41.682Z', + labels: { + nodes: [ + { name: 'fast-track' } + ] + } + }); + + const options = { + pr: PR, reviewers: allGreenReviewers, - comments: commentsWithLGTM, + comments: [], reviews: approvingReviews, commits: simpleCommits, collaborators - }); + }; + const checker = new PRChecker(cli, options, argv); - const status = checker.checkPRWait(new Date()); - assert(status); + checker.checkCI(); + cli.clearCalls(); + const status = checker.checkPRWait(now); + assert(!status); cli.assertCalledWith(expectedLogs); }); });