From f073737078358c08013d1bec88d89aa83de23459 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 26 Mar 2025 21:01:30 +0100 Subject: [PATCH 1/3] Revert "feat(ncu-ci): check for `request-ci` label (#806)" This reverts commit 6cc2b1ab0b436b77baf0d4cd5b22f5a15e692018. --- lib/ci/run_ci.js | 2 +- lib/pr_checker.js | 26 ----- lib/pr_data.js | 10 -- lib/queries/PRLabeledEvents.gql | 19 ---- test/fixtures/data.js | 9 -- test/fixtures/first_timer_pr.json | 1 - .../labeled_events/no-request-ci.json | 12 --- .../old-request-ci-collaborator.json | 12 --- .../recent-request-ci-collaborator.json | 12 --- .../recent-request-ci-non-collaborator.json | 12 --- test/unit/pr_checker.test.js | 95 ------------------- 11 files changed, 1 insertion(+), 209 deletions(-) delete mode 100644 lib/queries/PRLabeledEvents.gql delete mode 100644 test/fixtures/labeled_events/no-request-ci.json delete mode 100644 test/fixtures/labeled_events/old-request-ci-collaborator.json delete mode 100644 test/fixtures/labeled_events/recent-request-ci-collaborator.json delete mode 100644 test/fixtures/labeled_events/recent-request-ci-non-collaborator.json diff --git a/lib/ci/run_ci.js b/lib/ci/run_ci.js index 6edd5833..d0572e7e 100644 --- a/lib/ci/run_ci.js +++ b/lib/ci/run_ci.js @@ -27,7 +27,7 @@ export class RunPRJob { this.certifySafe = certifySafe || Promise.all([this.prData.getReviews(), this.prData.getPR()]).then(() => - new PRChecker(cli, this.prData, request, {}).checkCommitsAfterReviewOrLabel() + new PRChecker(cli, this.prData, request, {}).checkCommitsAfterReview() ); } diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 754a89c2..33be87fc 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -524,32 +524,6 @@ export default class PRChecker { return true; } - async checkCommitsAfterReviewOrLabel() { - if (this.checkCommitsAfterReview()) return true; - - await Promise.all([this.data.getLabeledEvents(), this.data.getCollaborators()]); - - const { - cli, data, pr - } = this; - - const { updatedAt } = pr.timelineItems; - const requestCiLabels = data.labeledEvents.findLast( - ({ createdAt, label: { name } }) => name === 'request-ci' && createdAt > updatedAt - ); - if (requestCiLabels == null) return false; - - const { actor: { login } } = requestCiLabels; - const collaborators = Array.from(data.collaborators.values(), - (c) => c.login.toLowerCase()); - if (collaborators.includes(login.toLowerCase())) { - cli.info('request-ci label was added by a Collaborator after the last push event.'); - return true; - } - - return false; - } - checkCommitsAfterReview() { const { commits, reviews, cli, argv diff --git a/lib/pr_data.js b/lib/pr_data.js index 1ee4adec..53379b8a 100644 --- a/lib/pr_data.js +++ b/lib/pr_data.js @@ -5,7 +5,6 @@ import { } from './user_status.js'; // lib/queries/*.gql file names -const LABELED_EVENTS_QUERY = 'PRLabeledEvents'; const PR_QUERY = 'PR'; const REVIEWS_QUERY = 'Reviews'; const COMMENTS_QUERY = 'PRComments'; @@ -34,7 +33,6 @@ export default class PRData { this.comments = []; this.commits = []; this.reviewers = []; - this.labeledEvents = []; } getThread() { @@ -92,14 +90,6 @@ export default class PRData { ]); } - async getLabeledEvents() { - const { prid, owner, repo, cli, request, prStr } = this; - const vars = { prid, owner, repo }; - cli.updateSpinner(`Getting labels from ${prStr}`); - this.labeledEvents = (await request.gql(LABELED_EVENTS_QUERY, vars)) - .repository.pullRequest.timelineItems.nodes; - } - async getComments() { const { prid, owner, repo, cli, request, prStr } = this; const vars = { prid, owner, repo }; diff --git a/lib/queries/PRLabeledEvents.gql b/lib/queries/PRLabeledEvents.gql deleted file mode 100644 index f3a0f233..00000000 --- a/lib/queries/PRLabeledEvents.gql +++ /dev/null @@ -1,19 +0,0 @@ -query PRLabeledEvents($prid: Int!, $owner: String!, $repo: String!, $after: String) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $prid) { - timelineItems(itemTypes: LABELED_EVENT, after: $after, last: 100) { - nodes { - ... on LabeledEvent { - actor { - login - } - label { - name - } - createdAt - } - } - } - } - } -} diff --git a/test/fixtures/data.js b/test/fixtures/data.js index b78333fe..b60fe933 100644 --- a/test/fixtures/data.js +++ b/test/fixtures/data.js @@ -134,12 +134,3 @@ for (const subdir of readdirSync(path('./jenkins'))) { readJSON(`./jenkins/${subdir}/${item}`); } }; - -export const labeledEvents = {}; - -for (const item of readdirSync(path('./labeled_events'))) { - if (!item.endsWith('.json')) { - continue; - } - labeledEvents[basename(item, '.json')] = readJSON(`./labeled_events/${item}`); -} diff --git a/test/fixtures/first_timer_pr.json b/test/fixtures/first_timer_pr.json index 658434c3..0ae29b6a 100644 --- a/test/fixtures/first_timer_pr.json +++ b/test/fixtures/first_timer_pr.json @@ -22,7 +22,6 @@ } ] }, - "timelineItems": { "updatedAt": "2017-10-24T11:13:43Z" }, "title": "test: awesome changes", "baseRefName": "main", "headRefName": "awesome-changes" diff --git a/test/fixtures/labeled_events/no-request-ci.json b/test/fixtures/labeled_events/no-request-ci.json deleted file mode 100644 index edee9def..00000000 --- a/test/fixtures/labeled_events/no-request-ci.json +++ /dev/null @@ -1,12 +0,0 @@ -[ - { - "actor": { "login": "nodejs-github-bot" }, - "label": { "name": "doc" }, - "createdAt": "2024-05-13T15:57:10Z" - }, - { - "actor": { "login": "nodejs-github-bot" }, - "label": { "name": "test_runner" }, - "createdAt": "2024-05-13T15:57:10Z" - } -] diff --git a/test/fixtures/labeled_events/old-request-ci-collaborator.json b/test/fixtures/labeled_events/old-request-ci-collaborator.json deleted file mode 100644 index 24a16f78..00000000 --- a/test/fixtures/labeled_events/old-request-ci-collaborator.json +++ /dev/null @@ -1,12 +0,0 @@ -[ - { - "actor": { "login": "nodejs-github-bot" }, - "label": { "name": "doc" }, - "createdAt": "2024-05-13T15:57:10Z" - }, - { - "actor": { "login": "foo" }, - "label": { "name": "request-ci" }, - "createdAt": "1999-10-24T11:13:43Z" - } -] diff --git a/test/fixtures/labeled_events/recent-request-ci-collaborator.json b/test/fixtures/labeled_events/recent-request-ci-collaborator.json deleted file mode 100644 index 54792093..00000000 --- a/test/fixtures/labeled_events/recent-request-ci-collaborator.json +++ /dev/null @@ -1,12 +0,0 @@ -[ - { - "actor": { "login": "nodejs-github-bot" }, - "label": { "name": "doc" }, - "createdAt": "2024-05-13T15:57:10Z" - }, - { - "actor": { "login": "foo" }, - "label": { "name": "request-ci" }, - "createdAt": "2024-05-13T15:57:10Z" - } -] diff --git a/test/fixtures/labeled_events/recent-request-ci-non-collaborator.json b/test/fixtures/labeled_events/recent-request-ci-non-collaborator.json deleted file mode 100644 index f799a03c..00000000 --- a/test/fixtures/labeled_events/recent-request-ci-non-collaborator.json +++ /dev/null @@ -1,12 +0,0 @@ -[ - { - "actor": { "login": "nodejs-github-bot" }, - "label": { "name": "doc" }, - "createdAt": "2024-05-13T15:57:10Z" - }, - { - "actor": { "login": "random-person" }, - "label": { "name": "request-ci" }, - "createdAt": "2024-05-13T15:57:10Z" - } -] diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 9b5fa43c..679f37dd 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -41,7 +41,6 @@ import { semverMajorPR, conflictingPR, closedPR, - labeledEvents, mergedPR, pullRequests } from '../fixtures/data.js'; @@ -2050,100 +2049,6 @@ describe('PRChecker', () => { }); }); - describe('checkCommitsAfterReviewOrLabel', () => { - it('should return true if PR passes post review checks', async() => { - const cli = new TestCLI(); - const checker = new PRChecker(cli, { - pr: semverMajorPR, - reviewers: allGreenReviewers, - comments: commentsWithLGTM, - reviews: approvingReviews, - commits: simpleCommits, - collaborators, - authorIsNew: () => true, - getThread: PRData.prototype.getThread - }, {}, argv); - - const status = await checker.checkCommitsAfterReviewOrLabel(); - assert.strictEqual(status, true); - }); - - describe('without approvals', () => { - const data = { - pr: firstTimerPR, - reviewers: noReviewers, - comments: [], - reviews: [], - commits: [], - collaborators: [], - labeledEvents: [], - authorIsNew: () => true, - getThread: PRData.prototype.getThread, - getLabeledEvents: async() => { - data.labeledEvents = []; - }, - getCollaborators: async() => { - data.collaborators = collaborators; - } - }; - - it('should return false if PR has no labels', async() => { - const cli = new TestCLI(); - data.getLabeledEvents = async() => { - data.labeledEvents = []; - }; - const checker = new PRChecker(cli, data, {}, argv); - - const status = await checker.checkCommitsAfterReviewOrLabel(); - assert.strictEqual(status, false); - }); - - it('should return false if PR has no request-ci label', async() => { - const cli = new TestCLI(); - data.getLabeledEvents = async() => { - data.labeledEvents = labeledEvents['no-request-ci']; - }; - const checker = new PRChecker(cli, data, {}, argv); - - const status = await checker.checkCommitsAfterReviewOrLabel(); - assert.strictEqual(status, false); - }); - - it('should return false if PR has request-ci from non-collaborator', async() => { - const cli = new TestCLI(); - data.getLabeledEvents = async() => { - data.labeledEvents = labeledEvents['recent-request-ci-non-collaborator']; - }; - const checker = new PRChecker(cli, data, {}, argv); - - const status = await checker.checkCommitsAfterReviewOrLabel(); - assert.strictEqual(status, false); - }); - - it('should return false if PR has outdated request-ci from a collaborator', async() => { - const cli = new TestCLI(); - data.getLabeledEvents = async() => { - data.labeledEvents = labeledEvents['old-request-ci-collaborator']; - }; - const checker = new PRChecker(cli, data, {}, argv); - - const status = await checker.checkCommitsAfterReviewOrLabel(); - assert.strictEqual(status, false); - }); - - it('should return true if PR has recent request-ci from a collaborator', async() => { - const cli = new TestCLI(); - data.getLabeledEvents = async() => { - data.labeledEvents = labeledEvents['recent-request-ci-collaborator']; - }; - const checker = new PRChecker(cli, data, {}, argv); - - const status = await checker.checkCommitsAfterReviewOrLabel(); - assert.strictEqual(status, true); - }); - }); - }); - describe('checkCommitsAfterReview', () => { const cli = new TestCLI(); From 646e7e2dae01676b3f5f65120c86da4eeb3e8386 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 26 Mar 2025 21:23:19 +0100 Subject: [PATCH 2/3] feat(ncu-ci): pass `COMMIT_SHA_CHECK` via `--certify-safe` (#911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes have been made on Jenkins side, tip of the PR head must be sent to Jenkins in order to start a PR. This commit adapts ncu-ci CLI to pass that information – or get the SHA from the latest review if not passed. --- bin/ncu-ci.js | 6 +++--- lib/ci/run_ci.js | 3 ++- lib/pr_checker.js | 8 ++++++-- test/unit/ci_start.test.js | 7 ++++--- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/bin/ncu-ci.js b/bin/ncu-ci.js index 41db7ccb..3ecf9ee0 100755 --- a/bin/ncu-ci.js +++ b/bin/ncu-ci.js @@ -115,9 +115,9 @@ const args = yargs(hideBin(process.argv)) type: 'number' }) .positional('certify-safe', { - describe: 'If not provided, the command will reject PRs that have ' + - 'been pushed since the last review', - type: 'boolean' + describe: 'SHA of the commit that is expected to be at the tip of the PR head. ' + + 'If not provided, the command will use the SHA of the last approved commit.', + type: 'string' }) .option('owner', { default: '', diff --git a/lib/ci/run_ci.js b/lib/ci/run_ci.js index d0572e7e..0c79b99b 100644 --- a/lib/ci/run_ci.js +++ b/lib/ci/run_ci.js @@ -27,7 +27,7 @@ export class RunPRJob { this.certifySafe = certifySafe || Promise.all([this.prData.getReviews(), this.prData.getPR()]).then(() => - new PRChecker(cli, this.prData, request, {}).checkCommitsAfterReview() + (this.certifySafe = new PRChecker(cli, this.prData, request, {}).getApprovedTipOfHead()) ); } @@ -45,6 +45,7 @@ export class RunPRJob { payload.append('json', JSON.stringify({ parameter: [ { name: 'CERTIFY_SAFE', value: 'on' }, + { name: 'COMMIT_SHA_CHECK', value: this.certifySafe }, { name: 'TARGET_GITHUB_ORG', value: this.owner }, { name: 'TARGET_REPO_NAME', value: this.repo }, { name: 'PR_ID', value: this.prid }, diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 33be87fc..b2e61808 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -524,7 +524,7 @@ export default class PRChecker { return true; } - checkCommitsAfterReview() { + getApprovedTipOfHead() { const { commits, reviews, cli, argv } = this; @@ -577,7 +577,11 @@ export default class PRChecker { return false; } - return true; + return reviews[reviewIndex].commit.oid; + } + + checkCommitsAfterReview() { + return !!this.getApprovedTipOfHead(); } checkMergeableState() { diff --git a/test/unit/ci_start.test.js b/test/unit/ci_start.test.js index 9a89a7f5..c56e5308 100644 --- a/test/unit/ci_start.test.js +++ b/test/unit/ci_start.test.js @@ -26,6 +26,7 @@ describe('Jenkins', () => { const { parameter } = JSON.parse(value); const expectedParameters = { CERTIFY_SAFE: 'on', + COMMIT_SHA_CHECK: 'deadbeef', TARGET_GITHUB_ORG: owner, TARGET_REPO_NAME: repo, PR_ID: prid, @@ -91,7 +92,7 @@ describe('Jenkins', () => { json: sinon.stub().withArgs(CI_CRUMB_URL) .returns(Promise.resolve({ crumb })) }; - const jobRunner = new RunPRJob(cli, request, owner, repo, prid, true); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef'); assert.ok(await jobRunner.start()); }); @@ -124,8 +125,8 @@ describe('Jenkins', () => { }safe`, async() => { const cli = new TestCLI(); - sinon.replace(PRChecker.prototype, 'checkCommitsAfterReview', - sinon.fake.returns(Promise.resolve(certifySafe))); + sinon.replace(PRChecker.prototype, 'getApprovedTipOfHead', + sinon.fake.returns(certifySafe && 'deadbeef')); const request = { gql: sinon.stub().returns({ From cdf482f3eb96396f500844a9acff3e25dbdf739e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 26 Mar 2025 15:36:24 +0100 Subject: [PATCH 3/3] fix(git-node): do not rely on commit date (#911) ...to display commits pushed since last approving review. It would only matter if time on the comitter machine is very different from GitHub's own time. That allows us to skip quering for `timelineItems`. --- lib/pr_checker.js | 38 +++++++------------ lib/queries/PR.gql | 3 -- ...an_three_commits_after_review_commits.json | 6 +-- ...an_three_commits_after_review_reviews.json | 1 + ...multiple_commits_after_review_reviews.json | 1 + test/fixtures/reviews_approved.json | 7 ++++ test/fixtures/semver_major_pr.json | 3 -- .../single_commit_after_review_reviews.json | 3 ++ test/unit/pr_checker.test.js | 25 ------------ 9 files changed, 28 insertions(+), 59 deletions(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index b2e61808..8fff6b32 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -530,6 +530,11 @@ export default class PRChecker { } = this; const { maxCommits } = argv; + if (commits.length === 0) { + cli.warn('No commits found'); + return false; + } + const reviewIndex = reviews.findLastIndex( review => review.authorCanPushToRepository && review.state === 'APPROVED' ); @@ -539,38 +544,21 @@ export default class PRChecker { return false; } - const reviewDate = reviews[reviewIndex].publishedAt; - - const afterCommits = []; - commits.forEach((commit) => { - commit = commit.commit; - if (commit.committedDate > reviewDate) { - afterCommits.push(commit); - } - }); - - const totalCommits = afterCommits.length; - if (totalCommits === 0 && this.pr.timelineItems.updatedAt > reviewDate) { - // Some commits were pushed, but all the commits have a commit date prior - // to the last review. It means that either that a long time elapsed - // between the commit and the push, or that the clock on the dev machine - // is wrong, or the commit date was forged. - cli.warn('Something was pushed to the Pull Request branch since the last approving review.'); - return false; - } + const reviewedCommitIndex = commits + .findLastIndex(({ commit }) => commit.oid === reviews[reviewIndex].commit.oid); - if (totalCommits > 0) { + if (reviewedCommitIndex !== commits.length - 1) { cli.warn('Commits were pushed since the last approving review:'); - const sliceLength = maxCommits === 0 ? totalCommits : -maxCommits; - afterCommits.slice(sliceLength) - .forEach(commit => { + commits.slice(Math.max(reviewedCommitIndex + 1, commits.length - maxCommits)) + .forEach(({ commit }) => { cli.warn(`- ${commit.messageHeadline}`); }); + const totalCommits = commits.length - reviewedCommitIndex - 1; if (totalCommits > maxCommits) { const infoMsg = '...(use `' + - `--max-commits ${totalCommits}` + - '` to see the full list of commits)'; + `--max-commits ${totalCommits}` + + '` to see the full list of commits)'; cli.warn(infoMsg); } diff --git a/lib/queries/PR.gql b/lib/queries/PR.gql index 22bb803b..27cc1396 100644 --- a/lib/queries/PR.gql +++ b/lib/queries/PR.gql @@ -23,9 +23,6 @@ query PR($prid: Int!, $owner: String!, $repo: String!) { path } }, - timelineItems(itemTypes: [HEAD_REF_FORCE_PUSHED_EVENT, PULL_REQUEST_COMMIT]) { - updatedAt - }, title, baseRefName, headRefName, diff --git a/test/fixtures/more_than_three_commits_after_review_commits.json b/test/fixtures/more_than_three_commits_after_review_commits.json index 49949058..cfd620e9 100644 --- a/test/fixtures/more_than_three_commits_after_review_commits.json +++ b/test/fixtures/more_than_three_commits_after_review_commits.json @@ -21,7 +21,7 @@ },{ "commit": { "committedDate": "2017-09-25T11:27:02Z", - "oid": "f230e691459f8b0f448e1eec1b83fbf7f708eba3", + "oid": "f230e691459f8b0f448e1eec1b83fbf7f708eba4", "messageHeadline": "src: add requested feature", "message": "src: new functionality\n works really great", "author": { @@ -31,7 +31,7 @@ },{ "commit": { "committedDate": "2017-10-25T12:42:02Z", - "oid": "9416475a6dc1b27c3e343dcbc07674a439c88db1", + "oid": "9416475a6dc1b27c3e343dcbc07674a439c88db2", "messageHeadline": "nit: edit mistakes", "message": "nit: fix mistakes\n fixed requested errors", "author": { @@ -41,7 +41,7 @@ },{ "commit": { "committedDate": "2017-10-25T12:42:02Z", - "oid": "9416475a6dc1b27c3e343dcbc07674a439c88db1", + "oid": "9416475a6dc1b27c3e343dcbc07674a439c88db3", "messageHeadline": "final: we should be good to go", "message": "final: we should be good to go\n fixed requested errors", "author": { diff --git a/test/fixtures/more_than_three_commits_after_review_reviews.json b/test/fixtures/more_than_three_commits_after_review_reviews.json index 039b51d3..e1b3db5f 100644 --- a/test/fixtures/more_than_three_commits_after_review_reviews.json +++ b/test/fixtures/more_than_three_commits_after_review_reviews.json @@ -4,6 +4,7 @@ "author": { "login": "foo" }, + "commit": {"oid": "f230e691459f8b0f448e1eec1b83fbf7f708eba3"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-89923489", "publishedAt": "2017-07-23T11:19:25Z" diff --git a/test/fixtures/multiple_commits_after_review_reviews.json b/test/fixtures/multiple_commits_after_review_reviews.json index 1307b1eb..6dd2b335 100644 --- a/test/fixtures/multiple_commits_after_review_reviews.json +++ b/test/fixtures/multiple_commits_after_review_reviews.json @@ -4,6 +4,7 @@ "author": { "login": "foo" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-89923489", "publishedAt": "2017-09-23T11:19:25Z" diff --git a/test/fixtures/reviews_approved.json b/test/fixtures/reviews_approved.json index 4024c5ad..12ccf9a6 100644 --- a/test/fixtures/reviews_approved.json +++ b/test/fixtures/reviews_approved.json @@ -4,6 +4,7 @@ "author": { "login": "foo" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480624", "publishedAt": "2017-10-24T11:19:00Z" @@ -14,6 +15,7 @@ "author": { "login": "Baz" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71488392", "publishedAt": "2017-10-24T11:50:52Z" @@ -24,6 +26,7 @@ "author": { "login": "Baz" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-714882992", "publishedAt": "2017-10-24T12:30:52Z" @@ -34,6 +37,7 @@ "author": { "login": "Quux" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71817236", "publishedAt": "2017-10-24T14:49:01Z" @@ -44,6 +48,7 @@ "author": { "login": "Baz" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71488236", "publishedAt": "2017-10-24T14:49:02Z" @@ -54,6 +59,7 @@ "author": { "login": "Quo" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71817236", "publishedAt": "2017-10-24T19:09:52Z" @@ -64,6 +70,7 @@ "author": { "login": "bot" }, + "commit": {"oid": "ffdef335209c77f66d933bd873950747bfe42264"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71839232", "publishedAt": "2017-10-28T19:21:52Z" diff --git a/test/fixtures/semver_major_pr.json b/test/fixtures/semver_major_pr.json index e62a29b5..c5bbe786 100644 --- a/test/fixtures/semver_major_pr.json +++ b/test/fixtures/semver_major_pr.json @@ -16,9 +16,6 @@ } ] }, - "timelineItems": { - "updatedAt": "2017-10-24T11:13:43Z" - }, "title": "lib: awesome changes", "baseRefName": "main", "headRefName": "awesome-changes" diff --git a/test/fixtures/single_commit_after_review_reviews.json b/test/fixtures/single_commit_after_review_reviews.json index c37c880c..2e3f9755 100644 --- a/test/fixtures/single_commit_after_review_reviews.json +++ b/test/fixtures/single_commit_after_review_reviews.json @@ -4,6 +4,7 @@ "author": { "login": "foo" }, + "commit": {"oid": "deadbeef"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480624", "publishedAt": "2017-10-24T11:19:25Z" @@ -13,6 +14,7 @@ "author": { "login": "foo" }, + "commit": {"oid": "6c0945cbeea2cbbc97d13a3d9e3fe68bd145b985"}, "authorCanPushToRepository": true, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480625", "publishedAt": "2017-10-26T11:19:25Z" @@ -22,6 +24,7 @@ "author": { "login": "bar" }, + "commit": {"oid": "6c0945cbeea2cbbc97d13a3d9e3fe68bd145b985"}, "authorCanPushToRepository": false, "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480626", "publishedAt": "2017-10-26T11:19:25Z" diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 679f37dd..bd98c6e4 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -2153,31 +2153,6 @@ describe('PRChecker', () => { cli.assertCalledWith(expectedLogs); }); - it('should return true if PR can be landed', () => { - const expectedLogs = { - warn: [ - ['Something was pushed to the Pull Request branch since the last approving review.'] - ], - info: [], - error: [] - }; - - const checker = new PRChecker(cli, { - pr: { ...semverMajorPR, timelineItems: { updatedAt: new Date().toISOString() } }, - reviewers: allGreenReviewers, - comments: commentsWithLGTM, - reviews: approvingReviews, - commits: simpleCommits, - collaborators, - authorIsNew: () => true, - getThread: PRData.prototype.getThread - }, {}, argv); - - const status = checker.checkCommitsAfterReview(); - assert.deepStrictEqual(status, false); - cli.assertCalledWith(expectedLogs); - }); - it('should skip the check if there are no reviews', () => { const { commits } = multipleCommitsAfterReview; const expectedLogs = {