Skip to content

Commit aecac72

Browse files
authored
git-node: add GitHub status as a CI option (#369)
Some project in the org don't use Jenkins, which means PRChecker will never succeed for pull requests on those projects. These projects usually have Travis, AppVeyor or other CI systems in place, and those systems will publish the status to GitHub, which can be retrieved via API. This commit adds GitHub status as an optional way to validate if a PR satisfies the CI requirement. We need to check for the CI status in two fields returned by our GraphQL query: commit.status for services using the old GitHub integration, and commits.checkSuites for services using the new GitHub integration via GitHub Apps.
1 parent 66037a6 commit aecac72

19 files changed

+623
-3
lines changed

lib/pr_checker.js

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,20 @@ class PRChecker {
201201
return cis.find(ci => isFullCI(ci) || isLiteCI(ci));
202202
}
203203

204+
checkCI() {
205+
const ciType = this.argv.ciType || 'jenkins';
206+
if (ciType === 'jenkins') {
207+
return this.checkJenkinsCI();
208+
} else if (ciType === 'github-check') {
209+
return this.checkGitHubCI();
210+
}
211+
this.cli.error(`Invalid ciType: ${ciType}`);
212+
return false;
213+
}
214+
204215
// TODO: we might want to check CI status when it's less flaky...
205216
// TODO: not all PR requires CI...labels?
206-
checkCI() {
217+
checkJenkinsCI() {
207218
const { cli, commits, argv } = this;
208219
const { maxCommits } = argv;
209220
const thread = this.data.getThread();
@@ -265,6 +276,57 @@ class PRChecker {
265276
return status;
266277
}
267278

279+
checkGitHubCI() {
280+
const { cli, commits } = this;
281+
282+
if (!commits) {
283+
cli.error('No commits detected');
284+
return false;
285+
}
286+
287+
// NOTE(mmarchini): we only care about the last commit. Maybe in the future
288+
// we'll want to check all commits for a successful CI.
289+
const { commit } = commits[commits.length - 1];
290+
291+
this.CIStatus = false;
292+
const checkSuites = commit.checkSuites || { nodes: [] };
293+
if (!commit.status && checkSuites.nodes.length === 0) {
294+
cli.error('No CI runs detected');
295+
return false;
296+
}
297+
298+
// GitHub new Check API
299+
for (const { status, conclusion } of checkSuites.nodes) {
300+
if (status !== 'COMPLETED') {
301+
cli.error('CI is still running');
302+
return false;
303+
}
304+
305+
if (!['SUCCESS', 'NEUTRAL'].includes(conclusion)) {
306+
cli.error('Last CI failed');
307+
return false;
308+
}
309+
}
310+
311+
// GitHub old commit status API
312+
if (commit.status) {
313+
const { state } = commit.status;
314+
if (state === 'PENDING') {
315+
cli.error('CI is still running');
316+
return false;
317+
}
318+
319+
if (!['SUCCESS', 'EXPECTED'].includes(state)) {
320+
cli.error('Last CI failed');
321+
return false;
322+
}
323+
}
324+
325+
cli.info('Last CI run was successful');
326+
this.CIStatus = true;
327+
return true;
328+
}
329+
268330
checkAuthor() {
269331
const { cli, commits, pr } = this;
270332

lib/queries/PRCommits.gql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ query Commits($prid: Int!, $owner: String!, $repo: String!, $after: String) {
2525
message
2626
messageHeadline
2727
authoredByCommitter
28+
checkSuites(first: 10) {
29+
nodes {
30+
conclusion,
31+
status
32+
}
33+
}
34+
status {
35+
state
36+
}
2837
}
2938
}
3039
}

lib/request.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ class Request {
6868
method: 'POST',
6969
headers: {
7070
Authorization: `Basic ${githubCredentials}`,
71-
'User-Agent': 'node-core-utils'
71+
'User-Agent': 'node-core-utils',
72+
Accept: 'application/vnd.github.antiope-preview+json'
7273
},
7374
body: JSON.stringify({
7475
query: query,

lib/session.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class Session {
5555
readme: this.readme,
5656
waitTimeSingleApproval: this.waitTimeSingleApproval,
5757
waitTimeMultiApproval: this.waitTimeMultiApproval,
58+
ciType: this.ciType,
5859
prid: this.prid
5960
};
6061
}
@@ -91,6 +92,10 @@ class Session {
9192
return this.config.waitTimeMultiApproval;
9293
}
9394

95+
get ciType() {
96+
return this.config.ciType || 'jenkins';
97+
}
98+
9499
get pullName() {
95100
return `${this.owner}/${this.repo}/pulls/${this.prid}`;
96101
}

test/fixtures/data.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict';
22

3-
const { readJSON, patchPrototype, readFile } = require('./index');
3+
const { basename } = require('path');
4+
const { readdirSync } = require('fs');
5+
6+
const { readJSON, patchPrototype, readFile, path } = require('./index');
47
const { Collaborator } = require('../../lib/collaborators');
58
const { Review } = require('../../lib/reviews');
69

@@ -88,6 +91,15 @@ const readmeNoCollaborators = readFile('./README/README_no_collaborators.md');
8891
const readmeNoCollaboratorE = readFile('./README/README_no_collaboratorE.md');
8992
const readmeUnordered = readFile('./README/README_unordered.md');
9093

94+
const githubCI = {};
95+
96+
for (const item of readdirSync(path('./github-ci'))) {
97+
if (!item.endsWith('.json')) {
98+
continue;
99+
}
100+
githubCI[basename(item, '.json')] = readJSON(`./github-ci/${item}`);
101+
};
102+
91103
module.exports = {
92104
approved,
93105
requestedChanges,
@@ -101,6 +113,7 @@ module.exports = {
101113
commentsWithLiteCI,
102114
commentsWithLGTM,
103115
oddCommits,
116+
githubCI,
104117
incorrectGitConfigCommits,
105118
simpleCommits,
106119
singleCommitAfterReview,
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
[
2+
{
3+
"commit": {
4+
"committedDate": "2017-10-26T12:10:20Z",
5+
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
6+
"messageHeadline": "doc: add api description README",
7+
"author": {
8+
"login": "foo"
9+
},
10+
"status": {
11+
"state": "FAILURE"
12+
},
13+
"checkSuites": {
14+
"nodes": [
15+
{
16+
"status": "COMPLETED",
17+
"conclusion": "FAILURE"
18+
}
19+
]
20+
}
21+
}
22+
}
23+
]
24+
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
[
2+
{
3+
"commit": {
4+
"committedDate": "2017-10-26T12:10:20Z",
5+
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
6+
"messageHeadline": "doc: add api description README",
7+
"author": {
8+
"login": "foo"
9+
},
10+
"status": {
11+
"state": "SUCCESS"
12+
},
13+
"checkSuites": {
14+
"nodes": [
15+
{
16+
"status": "COMPLETED",
17+
"conclusion": "SUCCESS"
18+
}
19+
]
20+
}
21+
}
22+
}
23+
]
24+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
[
2+
{
3+
"commit": {
4+
"committedDate": "2017-10-26T12:10:20Z",
5+
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
6+
"messageHeadline": "doc: add api description README",
7+
"author": {
8+
"login": "foo"
9+
},
10+
"checkSuites": {
11+
"nodes": [
12+
{
13+
"status": "COMPLETED",
14+
"conclusion": "FAILURE"
15+
}
16+
]
17+
}
18+
}
19+
}
20+
]
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
[
2+
{
3+
"commit": {
4+
"committedDate": "2017-10-26T12:10:20Z",
5+
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
6+
"messageHeadline": "doc: add api description README",
7+
"author": {
8+
"login": "foo"
9+
},
10+
"checkSuites": {
11+
"nodes": [
12+
{
13+
"status": "IN_PROGRESS"
14+
}
15+
]
16+
}
17+
}
18+
}
19+
]
20+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
[
2+
{
3+
"commit": {
4+
"committedDate": "2017-10-26T12:10:20Z",
5+
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
6+
"messageHeadline": "doc: add api description README",
7+
"author": {
8+
"login": "foo"
9+
},
10+
"checkSuites": {
11+
"nodes": [
12+
{
13+
"status": "COMPLETED",
14+
"conclusion": "SUCCESS"
15+
}
16+
]
17+
}
18+
}
19+
}
20+
]

0 commit comments

Comments
 (0)