From d9cc2c69abf5105b6fa2a2cfda8d6bd3e66e927a Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 8 May 2024 14:34:59 +0200 Subject: [PATCH 1/5] feat(ncu-ci): check for `request-ci` label When running `ncu-ci` without the `--certify-safe` CLI flag, if something was pushed to a PR since the last approving review, check if the last time the `request-ci` label was added, it was done by a Collaborator and after the last push event on the PR. --- lib/ci/run_ci.js | 2 +- lib/pr_checker.js | 22 ++++++++++++++++++++++ lib/pr_data.js | 9 +++++++++ lib/queries/PRLabels.gql | 19 +++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 lib/queries/PRLabels.gql diff --git a/lib/ci/run_ci.js b/lib/ci/run_ci.js index d0572e7e..6edd5833 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() + new PRChecker(cli, this.prData, request, {}).checkCommitsAfterReviewOrLabel() ); } diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 37cdbd51..e6a2610e 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -523,6 +523,28 @@ export default class PRChecker { return true; } + async checkCommitsAfterReviewOrLabel() { + if (this.checkCommitsAfterReview()) return true; + + await Promise.all([this.data.getLabels(), this.data.getCollaborators()]); + + const { + cli, data, pr + } = this; + const { updatedAt } = pr.timelineItems; + const { actor: { login } } = data.labels.findLast( + ({ createdAt, label: { name } }) => name === 'request-ci' && createdAt > updatedAt + ); + 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 53379b8a..9efd99ca 100644 --- a/lib/pr_data.js +++ b/lib/pr_data.js @@ -5,6 +5,7 @@ import { } from './user_status.js'; // lib/queries/*.gql file names +const LABELS_QUERY = 'PRLabels'; const PR_QUERY = 'PR'; const REVIEWS_QUERY = 'Reviews'; const COMMENTS_QUERY = 'PRComments'; @@ -90,6 +91,14 @@ export default class PRData { ]); } + async getLabels() { + const { prid, owner, repo, cli, request, prStr } = this; + const vars = { prid, owner, repo }; + cli.updateSpinner(`Getting labels from ${prStr}`); + this.labels = (await request.gql(LABELS_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/PRLabels.gql b/lib/queries/PRLabels.gql new file mode 100644 index 00000000..72b3b5d7 --- /dev/null +++ b/lib/queries/PRLabels.gql @@ -0,0 +1,19 @@ +query PRLabels($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 + } + } + } + } + } +} From f33bd7e0af87b5381c2b06d3383bcb9ab6056d7b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 8 May 2024 14:50:36 +0200 Subject: [PATCH 2/5] fixup! feat(ncu-ci): check for `request-ci` label fix for when there are no request-ci labels --- lib/pr_checker.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index e6a2610e..82c0c363 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -531,10 +531,14 @@ export default class PRChecker { const { cli, data, pr } = this; + const { updatedAt } = pr.timelineItems; - const { actor: { login } } = data.labels.findLast( + const requestCiLabels = data.labels.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())) { From 941a4aa301f0513c331818ab60b029caea42ce2e Mon Sep 17 00:00:00 2001 From: Mathis Wiehl Date: Mon, 13 May 2024 19:20:54 +0200 Subject: [PATCH 3/5] refactor(ncu-ci): correctly name labeled events --- lib/pr_checker.js | 4 ++-- lib/pr_data.js | 7 ++++--- lib/queries/{PRLabels.gql => PRLabeledEvents.gql} | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) rename lib/queries/{PRLabels.gql => PRLabeledEvents.gql} (81%) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 82c0c363..7d00ca92 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -526,14 +526,14 @@ export default class PRChecker { async checkCommitsAfterReviewOrLabel() { if (this.checkCommitsAfterReview()) return true; - await Promise.all([this.data.getLabels(), this.data.getCollaborators()]); + await Promise.all([this.data.getLabeledEvents(), this.data.getCollaborators()]); const { cli, data, pr } = this; const { updatedAt } = pr.timelineItems; - const requestCiLabels = data.labels.findLast( + const requestCiLabels = data.labeledEvents.findLast( ({ createdAt, label: { name } }) => name === 'request-ci' && createdAt > updatedAt ); if (requestCiLabels == null) return false; diff --git a/lib/pr_data.js b/lib/pr_data.js index 9efd99ca..1ee4adec 100644 --- a/lib/pr_data.js +++ b/lib/pr_data.js @@ -5,7 +5,7 @@ import { } from './user_status.js'; // lib/queries/*.gql file names -const LABELS_QUERY = 'PRLabels'; +const LABELED_EVENTS_QUERY = 'PRLabeledEvents'; const PR_QUERY = 'PR'; const REVIEWS_QUERY = 'Reviews'; const COMMENTS_QUERY = 'PRComments'; @@ -34,6 +34,7 @@ export default class PRData { this.comments = []; this.commits = []; this.reviewers = []; + this.labeledEvents = []; } getThread() { @@ -91,11 +92,11 @@ export default class PRData { ]); } - async getLabels() { + async getLabeledEvents() { const { prid, owner, repo, cli, request, prStr } = this; const vars = { prid, owner, repo }; cli.updateSpinner(`Getting labels from ${prStr}`); - this.labels = (await request.gql(LABELS_QUERY, vars)) + this.labeledEvents = (await request.gql(LABELED_EVENTS_QUERY, vars)) .repository.pullRequest.timelineItems.nodes; } diff --git a/lib/queries/PRLabels.gql b/lib/queries/PRLabeledEvents.gql similarity index 81% rename from lib/queries/PRLabels.gql rename to lib/queries/PRLabeledEvents.gql index 72b3b5d7..f3a0f233 100644 --- a/lib/queries/PRLabels.gql +++ b/lib/queries/PRLabeledEvents.gql @@ -1,4 +1,4 @@ -query PRLabels($prid: Int!, $owner: String!, $repo: String!, $after: String) { +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) { From ea77c5c665499f9c10bfe5f92d47661c10f9c04a Mon Sep 17 00:00:00 2001 From: Mathis Wiehl Date: Mon, 13 May 2024 20:02:05 +0200 Subject: [PATCH 4/5] test(ncu-ci): checkCommitsAfterReviewOrLabel --- 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 | 97 ++++++++++++++++++- 7 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/labeled_events/no-request-ci.json create mode 100644 test/fixtures/labeled_events/old-request-ci-collaborator.json create mode 100644 test/fixtures/labeled_events/recent-request-ci-collaborator.json create mode 100644 test/fixtures/labeled_events/recent-request-ci-non-collaborator.json diff --git a/test/fixtures/data.js b/test/fixtures/data.js index b60fe933..b78333fe 100644 --- a/test/fixtures/data.js +++ b/test/fixtures/data.js @@ -134,3 +134,12 @@ 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 0ae29b6a..658434c3 100644 --- a/test/fixtures/first_timer_pr.json +++ b/test/fixtures/first_timer_pr.json @@ -22,6 +22,7 @@ } ] }, + "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 new file mode 100644 index 00000000..edee9def --- /dev/null +++ b/test/fixtures/labeled_events/no-request-ci.json @@ -0,0 +1,12 @@ +[ + { + "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 new file mode 100644 index 00000000..24a16f78 --- /dev/null +++ b/test/fixtures/labeled_events/old-request-ci-collaborator.json @@ -0,0 +1,12 @@ +[ + { + "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 new file mode 100644 index 00000000..54792093 --- /dev/null +++ b/test/fixtures/labeled_events/recent-request-ci-collaborator.json @@ -0,0 +1,12 @@ +[ + { + "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 new file mode 100644 index 00000000..f799a03c --- /dev/null +++ b/test/fixtures/labeled_events/recent-request-ci-non-collaborator.json @@ -0,0 +1,12 @@ +[ + { + "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 3e10a71c..c919b7d9 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -41,7 +41,8 @@ import { conflictingPR, closedPR, mergedPR, - pullRequests + pullRequests, + labeledEvents } from '../fixtures/data.js'; jobCache.disable(); @@ -2048,6 +2049,100 @@ 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 a79b2b44ac12608e40c486df4177f2f657d04c0c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 14 May 2024 09:41:53 +0200 Subject: [PATCH 5/5] ascii order --- test/unit/pr_checker.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index c919b7d9..e71837f7 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -40,9 +40,9 @@ import { semverMajorPR, conflictingPR, closedPR, + labeledEvents, mergedPR, - pullRequests, - labeledEvents + pullRequests } from '../fixtures/data.js'; jobCache.disable();