Skip to content

Commit edbae73

Browse files
committed
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 <panva.ip@gmail.com>
1 parent ca3e4cd commit edbae73

5 files changed

Lines changed: 133 additions & 22 deletions

File tree

‎lib/pr_data.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,30 @@ export default class PRData {
6060
this.getThreadData(),
6161
this.getCommits()
6262
]).then(() => {
63+
this.mergeHeadCommitChecks();
6364
this.cli.stopSpinner(`Done loading data for ${prStr}`);
6465
});
6566
this.analyzeReviewers();
6667
}
6768

69+
mergeHeadCommitChecks() {
70+
const headCommit = this.pr.headCommit?.nodes?.[0]?.commit;
71+
delete this.pr.headCommit;
72+
73+
const lastCommit = this.commits.at(-1)?.commit;
74+
if (headCommit === undefined && lastCommit === undefined) {
75+
return;
76+
}
77+
78+
if (headCommit?.oid !== lastCommit?.oid) {
79+
throw new Error(
80+
'Unable to match pull request head commit while loading data');
81+
}
82+
83+
lastCommit.checkSuites = headCommit.checkSuites;
84+
lastCommit.status = headCommit.status;
85+
}
86+
6887
analyzeReviewers() {
6988
this.reviewers = new ReviewAnalyzer(this).getReviewers();
7089
}

‎lib/queries/PR.gql‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,35 @@ query PR($prid: Int!, $owner: String!, $repo: String!) {
2323
path
2424
}
2525
},
26+
# CI data is only consumed for the pull request's head commit. Querying
27+
# it for every commit multiplies the GraphQL rate-limit cost.
28+
headCommit: commits(last: 1) {
29+
nodes {
30+
commit {
31+
oid
32+
checkSuites(first: 100) {
33+
nodes {
34+
app {
35+
slug
36+
}
37+
conclusion,
38+
status,
39+
checkRuns(first: 40) {
40+
nodes {
41+
name
42+
status
43+
conclusion
44+
detailsUrl
45+
}
46+
}
47+
}
48+
}
49+
status {
50+
state
51+
}
52+
}
53+
}
54+
},
2655
title,
2756
baseRefName,
2857
headRefName,

‎lib/queries/PRCommits.gql‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,6 @@ query Commits($prid: Int!, $owner: String!, $repo: String!, $after: String) {
2525
message
2626
messageHeadline
2727
authoredByCommitter
28-
checkSuites(first: 100) {
29-
nodes {
30-
app {
31-
slug
32-
}
33-
conclusion,
34-
status,
35-
checkRuns(first: 40) {
36-
nodes {
37-
name
38-
status
39-
conclusion
40-
detailsUrl
41-
}
42-
}
43-
}
44-
}
45-
status {
46-
state
47-
}
4828
}
4929
}
5030
}

‎test/unit/graphql_queries.test.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { readFileSync } from 'node:fs';
2+
import { describe, it } from 'node:test';
3+
import assert from 'node:assert';
4+
5+
const prQuery = readFileSync(
6+
new URL('../../lib/queries/PR.gql', import.meta.url), 'utf8');
7+
const commitsQuery = readFileSync(
8+
new URL('../../lib/queries/PRCommits.gql', import.meta.url), 'utf8');
9+
10+
describe('GraphQL queries', () => {
11+
it('requests CI details only for the pull request head commit', () => {
12+
const headCommitStart = prQuery.indexOf(
13+
'headCommit: commits(last: 1)');
14+
const headCommitEnd = prQuery.indexOf('\n title,', headCommitStart);
15+
const headCommitQuery = prQuery.slice(headCommitStart, headCommitEnd);
16+
17+
assert.notStrictEqual(headCommitStart, -1);
18+
assert.notStrictEqual(headCommitEnd, -1);
19+
assert.match(headCommitQuery, /checkSuites\(first: 100\)/);
20+
assert.match(headCommitQuery, /checkRuns\(first: 40\)/);
21+
assert.match(headCommitQuery, /status \{\s+state\s+\}/);
22+
assert.doesNotMatch(commitsQuery, /checkSuites/);
23+
assert.doesNotMatch(commitsQuery, /checkRuns/);
24+
assert.doesNotMatch(commitsQuery, /\sstatus\s*\{/);
25+
});
26+
});

‎test/unit/pr_data.test.js‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,24 @@ function toRaw(obj) {
2323
const rawPR = toRaw({
2424
repository: { pullRequest: firstTimerPR }
2525
});
26+
const headCommitChecks = {
27+
oid: oddCommits.at(-1).commit.oid,
28+
checkSuites: {
29+
nodes: [{
30+
app: { slug: 'github-actions' },
31+
status: 'COMPLETED',
32+
conclusion: 'SUCCESS',
33+
checkRuns: { nodes: [] }
34+
}]
35+
},
36+
status: { state: 'SUCCESS' }
37+
};
38+
rawPR.repository.pullRequest.headCommit = {
39+
nodes: [{ commit: toRaw(headCommitChecks) }]
40+
};
41+
42+
const commitsWithHeadChecks = toRaw(oddCommits);
43+
Object.assign(commitsWithHeadChecks.at(-1).commit, headCommitChecks);
2644

2745
describe('PRData', function() {
2846
const request = {
@@ -34,7 +52,8 @@ describe('PRData', function() {
3452
.withArgs('https://raw.githubusercontent.com/nodejs/node/HEAD/README.md')
3553
.returns(Promise.resolve(readme));
3654
request.text.returns(new Error('unknown query'));
37-
request.gql.withArgs('PR').returns(Promise.resolve(rawPR));
55+
request.gql.withArgs('PR').callsFake(
56+
() => Promise.resolve(toRaw(rawPR)));
3857
request.gql.withArgs('Reviews').returns(
3958
Promise.resolve(toRaw(approvingReviews)));
4059
request.gql.withArgs('PRComments').returns(
@@ -53,7 +72,45 @@ describe('PRData', function() {
5372
assert.deepStrictEqual(data.pr, firstTimerPR, 'pr');
5473
assert.deepStrictEqual(data.reviews, approvingReviews, 'reviews');
5574
assert.deepStrictEqual(data.comments, commentsWithLGTM, 'comments');
56-
assert.deepStrictEqual(data.commits, oddCommits, 'commits');
75+
assert.deepStrictEqual(
76+
data.commits, commitsWithHeadChecks, 'commits');
5777
assert.deepStrictEqual(data.reviewers, allGreenReviewers, 'reviewers');
5878
});
79+
80+
describe('mergeHeadCommitChecks', () => {
81+
it('allows an empty commit list', () => {
82+
const data = new PRData(argv, new TestCLI(), request);
83+
data.pr = { headCommit: { nodes: [] } };
84+
data.commits = [];
85+
86+
data.mergeHeadCommitChecks();
87+
88+
assert.deepStrictEqual(data.pr, {});
89+
assert.deepStrictEqual(data.commits, []);
90+
});
91+
92+
it('rejects missing head commit data', () => {
93+
const data = new PRData(argv, new TestCLI(), request);
94+
data.pr = {};
95+
data.commits = toRaw(oddCommits);
96+
97+
assert.throws(
98+
() => data.mergeHeadCommitChecks(),
99+
/Unable to match pull request head commit/);
100+
});
101+
102+
it('rejects a mismatched head commit', () => {
103+
const data = new PRData(argv, new TestCLI(), request);
104+
data.pr = {
105+
headCommit: {
106+
nodes: [{ commit: { ...headCommitChecks, oid: 'different' } }]
107+
}
108+
};
109+
data.commits = toRaw(oddCommits);
110+
111+
assert.throws(
112+
() => data.mergeHeadCommitChecks(),
113+
/Unable to match pull request head commit/);
114+
});
115+
});
59116
});

0 commit comments

Comments
 (0)