From edbae732e03aab068fd5fedff8decb07ac9ccbf7 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 9 Aug 2026 21:08:24 +0200 Subject: [PATCH] perf: query CI metadata only for pull request head Landing checks only consume CI data from the final commit. Avoid querying check suites and runs for every commit, which can exhaust the GraphQL rate limit, and merge the head data after verifying its OID. Signed-off-by: Filip Skokan --- lib/pr_data.js | 19 ++++++++++ lib/queries/PR.gql | 29 +++++++++++++++ lib/queries/PRCommits.gql | 20 ---------- test/unit/graphql_queries.test.js | 26 +++++++++++++ test/unit/pr_data.test.js | 61 ++++++++++++++++++++++++++++++- 5 files changed, 133 insertions(+), 22 deletions(-) create mode 100644 test/unit/graphql_queries.test.js diff --git a/lib/pr_data.js b/lib/pr_data.js index 3dda6ebd..bb6d4ee7 100644 --- a/lib/pr_data.js +++ b/lib/pr_data.js @@ -60,11 +60,30 @@ export default class PRData { this.getThreadData(), this.getCommits() ]).then(() => { + this.mergeHeadCommitChecks(); this.cli.stopSpinner(`Done loading data for ${prStr}`); }); this.analyzeReviewers(); } + mergeHeadCommitChecks() { + const headCommit = this.pr.headCommit?.nodes?.[0]?.commit; + delete this.pr.headCommit; + + const lastCommit = this.commits.at(-1)?.commit; + if (headCommit === undefined && lastCommit === undefined) { + return; + } + + if (headCommit?.oid !== lastCommit?.oid) { + throw new Error( + 'Unable to match pull request head commit while loading data'); + } + + lastCommit.checkSuites = headCommit.checkSuites; + lastCommit.status = headCommit.status; + } + analyzeReviewers() { this.reviewers = new ReviewAnalyzer(this).getReviewers(); } diff --git a/lib/queries/PR.gql b/lib/queries/PR.gql index 19c86a3a..824e5c5a 100644 --- a/lib/queries/PR.gql +++ b/lib/queries/PR.gql @@ -23,6 +23,35 @@ query PR($prid: Int!, $owner: String!, $repo: String!) { path } }, + # CI data is only consumed for the pull request's head commit. Querying + # it for every commit multiplies the GraphQL rate-limit cost. + headCommit: commits(last: 1) { + nodes { + commit { + oid + checkSuites(first: 100) { + nodes { + app { + slug + } + conclusion, + status, + checkRuns(first: 40) { + nodes { + name + status + conclusion + detailsUrl + } + } + } + } + status { + state + } + } + } + }, title, baseRefName, headRefName, diff --git a/lib/queries/PRCommits.gql b/lib/queries/PRCommits.gql index 70cf2e4e..b8567e37 100644 --- a/lib/queries/PRCommits.gql +++ b/lib/queries/PRCommits.gql @@ -25,26 +25,6 @@ query Commits($prid: Int!, $owner: String!, $repo: String!, $after: String) { message messageHeadline authoredByCommitter - checkSuites(first: 100) { - nodes { - app { - slug - } - conclusion, - status, - checkRuns(first: 40) { - nodes { - name - status - conclusion - detailsUrl - } - } - } - } - status { - state - } } } } diff --git a/test/unit/graphql_queries.test.js b/test/unit/graphql_queries.test.js new file mode 100644 index 00000000..9b8d750e --- /dev/null +++ b/test/unit/graphql_queries.test.js @@ -0,0 +1,26 @@ +import { readFileSync } from 'node:fs'; +import { describe, it } from 'node:test'; +import assert from 'node:assert'; + +const prQuery = readFileSync( + new URL('../../lib/queries/PR.gql', import.meta.url), 'utf8'); +const commitsQuery = readFileSync( + new URL('../../lib/queries/PRCommits.gql', import.meta.url), 'utf8'); + +describe('GraphQL queries', () => { + it('requests CI details only for the pull request head commit', () => { + const headCommitStart = prQuery.indexOf( + 'headCommit: commits(last: 1)'); + const headCommitEnd = prQuery.indexOf('\n title,', headCommitStart); + const headCommitQuery = prQuery.slice(headCommitStart, headCommitEnd); + + assert.notStrictEqual(headCommitStart, -1); + assert.notStrictEqual(headCommitEnd, -1); + assert.match(headCommitQuery, /checkSuites\(first: 100\)/); + assert.match(headCommitQuery, /checkRuns\(first: 40\)/); + assert.match(headCommitQuery, /status \{\s+state\s+\}/); + assert.doesNotMatch(commitsQuery, /checkSuites/); + assert.doesNotMatch(commitsQuery, /checkRuns/); + assert.doesNotMatch(commitsQuery, /\sstatus\s*\{/); + }); +}); diff --git a/test/unit/pr_data.test.js b/test/unit/pr_data.test.js index 86f7b31d..7123845a 100644 --- a/test/unit/pr_data.test.js +++ b/test/unit/pr_data.test.js @@ -23,6 +23,24 @@ function toRaw(obj) { const rawPR = toRaw({ repository: { pullRequest: firstTimerPR } }); +const headCommitChecks = { + oid: oddCommits.at(-1).commit.oid, + checkSuites: { + nodes: [{ + app: { slug: 'github-actions' }, + status: 'COMPLETED', + conclusion: 'SUCCESS', + checkRuns: { nodes: [] } + }] + }, + status: { state: 'SUCCESS' } +}; +rawPR.repository.pullRequest.headCommit = { + nodes: [{ commit: toRaw(headCommitChecks) }] +}; + +const commitsWithHeadChecks = toRaw(oddCommits); +Object.assign(commitsWithHeadChecks.at(-1).commit, headCommitChecks); describe('PRData', function() { const request = { @@ -34,7 +52,8 @@ describe('PRData', function() { .withArgs('https://raw.githubusercontent.com/nodejs/node/HEAD/README.md') .returns(Promise.resolve(readme)); request.text.returns(new Error('unknown query')); - request.gql.withArgs('PR').returns(Promise.resolve(rawPR)); + request.gql.withArgs('PR').callsFake( + () => Promise.resolve(toRaw(rawPR))); request.gql.withArgs('Reviews').returns( Promise.resolve(toRaw(approvingReviews))); request.gql.withArgs('PRComments').returns( @@ -53,7 +72,45 @@ describe('PRData', function() { assert.deepStrictEqual(data.pr, firstTimerPR, 'pr'); assert.deepStrictEqual(data.reviews, approvingReviews, 'reviews'); assert.deepStrictEqual(data.comments, commentsWithLGTM, 'comments'); - assert.deepStrictEqual(data.commits, oddCommits, 'commits'); + assert.deepStrictEqual( + data.commits, commitsWithHeadChecks, 'commits'); assert.deepStrictEqual(data.reviewers, allGreenReviewers, 'reviewers'); }); + + describe('mergeHeadCommitChecks', () => { + it('allows an empty commit list', () => { + const data = new PRData(argv, new TestCLI(), request); + data.pr = { headCommit: { nodes: [] } }; + data.commits = []; + + data.mergeHeadCommitChecks(); + + assert.deepStrictEqual(data.pr, {}); + assert.deepStrictEqual(data.commits, []); + }); + + it('rejects missing head commit data', () => { + const data = new PRData(argv, new TestCLI(), request); + data.pr = {}; + data.commits = toRaw(oddCommits); + + assert.throws( + () => data.mergeHeadCommitChecks(), + /Unable to match pull request head commit/); + }); + + it('rejects a mismatched head commit', () => { + const data = new PRData(argv, new TestCLI(), request); + data.pr = { + headCommit: { + nodes: [{ commit: { ...headCommitChecks, oid: 'different' } }] + } + }; + data.commits = toRaw(oddCommits); + + assert.throws( + () => data.mergeHeadCommitChecks(), + /Unable to match pull request head commit/); + }); + }); });