Skip to content

Commit

Permalink
git-node: add GitHub status as a CI option
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mmarchini committed Nov 1, 2019
1 parent b928703 commit fe21810
Show file tree
Hide file tree
Showing 19 changed files with 623 additions and 3 deletions.
64 changes: 63 additions & 1 deletion lib/pr_checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,20 @@ class PRChecker {
return cis.find(ci => isFullCI(ci) || isLiteCI(ci));
}

checkCI() {
const ciType = this.argv.ciType || 'jenkins';
if (ciType === 'jenkins') {
return this.checkJenkinsCI();
} else if (ciType === 'github-check') {
return this.checkGitHubCI();
}
this.cli.error(`Invalid ciType: ${ciType}`);
return false;
}

// TODO: we might want to check CI status when it's less flaky...
// TODO: not all PR requires CI...labels?
checkCI() {
checkJenkinsCI() {
const { cli, commits, argv } = this;
const { maxCommits } = argv;
const thread = this.data.getThread();
Expand Down Expand Up @@ -251,6 +262,57 @@ class PRChecker {
return status;
}

checkGitHubCI() {
const { cli, commits } = this;

if (!commits) {
cli.error('No commits detected');
return false;
}

// NOTE(mmarchini): we only care about the last commit. Maybe in the future
// we'll want to check all commits for a successful CI.
const { commit } = commits[commits.length - 1];

this.CIStatus = false;
const checkSuites = commit.checkSuites || { nodes: [] };
if (!commit.status && checkSuites.nodes.length === 0) {
cli.error('No CI runs detected');
return false;
}

// GitHub new Check API
for (const { status, conclusion } of checkSuites.nodes) {
if (status !== 'COMPLETED') {
cli.error('CI is still running');
return false;
}

if (!['SUCCESS', 'NEUTRAL'].includes(conclusion)) {
cli.error('Last CI failed');
return false;
}
}

// GitHub old commit status API
if (commit.status) {
const { state } = commit.status;
if (state === 'PENDING') {
cli.error('CI is still running');
return false;
}

if (!['SUCCESS', 'EXPECTED'].includes(state)) {
cli.error('Last CI failed');
return false;
}
}

cli.info('Last CI run was successful');
this.CIStatus = true;
return true;
}

checkAuthor() {
const { cli, commits, pr } = this;

Expand Down
9 changes: 9 additions & 0 deletions lib/queries/PRCommits.gql
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ query Commits($prid: Int!, $owner: String!, $repo: String!, $after: String) {
message
messageHeadline
authoredByCommitter
checkSuites(first: 10) {
nodes {
conclusion,
status
}
}
status {
state
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ class Request {
method: 'POST',
headers: {
Authorization: `Basic ${githubCredentials}`,
'User-Agent': 'node-core-utils'
'User-Agent': 'node-core-utils',
Accept: 'application/vnd.github.antiope-preview+json'
},
body: JSON.stringify({
query: query,
Expand Down
5 changes: 5 additions & 0 deletions lib/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class Session {
upstream: this.upstream,
branch: this.branch,
readme: this.readme,
ciType: this.ciType,
prid: this.prid
};
}
Expand Down Expand Up @@ -81,6 +82,10 @@ class Session {
return this.config.readme;
}

get ciType() {
return this.config.ciType || 'jenkins';
}

get pullName() {
return `${this.owner}/${this.repo}/pulls/${this.prid}`;
}
Expand Down
15 changes: 14 additions & 1 deletion test/fixtures/data.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
'use strict';

const { readJSON, patchPrototype, readFile } = require('./index');
const { basename } = require('path');
const { readdirSync } = require('fs');

const { readJSON, patchPrototype, readFile, path } = require('./index');
const { Collaborator } = require('../../lib/collaborators');
const { Review } = require('../../lib/reviews');

Expand Down Expand Up @@ -82,6 +85,15 @@ const readmeNoCollaborators = readFile('./README/README_no_collaborators.md');
const readmeNoCollaboratorE = readFile('./README/README_no_collaboratorE.md');
const readmeUnordered = readFile('./README/README_unordered.md');

const githubCI = {};

for (const item of readdirSync(path('./github-ci'))) {
if (!item.endsWith('.json')) {
continue;
}
githubCI[basename(item, '.json')] = readJSON(`./github-ci/${item}`);
};

module.exports = {
approved,
requestedChanges,
Expand All @@ -94,6 +106,7 @@ module.exports = {
commentsWithLiteCI,
commentsWithLGTM,
oddCommits,
githubCI,
incorrectGitConfigCommits,
simpleCommits,
singleCommitAfterReview,
Expand Down
24 changes: 24 additions & 0 deletions test/fixtures/github-ci/both-apis-failure.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"status": {
"state": "FAILURE"
},
"checkSuites": {
"nodes": [
{
"status": "COMPLETED",
"conclusion": "FAILURE"
}
]
}
}
}
]

24 changes: 24 additions & 0 deletions test/fixtures/github-ci/both-apis-success.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"status": {
"state": "SUCCESS"
},
"checkSuites": {
"nodes": [
{
"status": "COMPLETED",
"conclusion": "SUCCESS"
}
]
}
}
}
]

20 changes: 20 additions & 0 deletions test/fixtures/github-ci/check-suite-failure.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"checkSuites": {
"nodes": [
{
"status": "COMPLETED",
"conclusion": "FAILURE"
}
]
}
}
}
]
20 changes: 20 additions & 0 deletions test/fixtures/github-ci/check-suite-pending.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"checkSuites": {
"nodes": [
{
"status": "IN_PROGRESS"
}
]
}
}
}
]

20 changes: 20 additions & 0 deletions test/fixtures/github-ci/check-suite-success.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"checkSuites": {
"nodes": [
{
"status": "COMPLETED",
"conclusion": "SUCCESS"
}
]
}
}
}
]
16 changes: 16 additions & 0 deletions test/fixtures/github-ci/commit-status-only-failure.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"status": {
"state": "FAILURE"
}
}
}
]

16 changes: 16 additions & 0 deletions test/fixtures/github-ci/commit-status-only-pending.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"status": {
"state": "PENDING"
}
}
}
]

16 changes: 16 additions & 0 deletions test/fixtures/github-ci/commit-status-only-success.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"status": {
"state": "SUCCESS"
}
}
}
]

13 changes: 13 additions & 0 deletions test/fixtures/github-ci/no-status.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
}
}
}
]

24 changes: 24 additions & 0 deletions test/fixtures/github-ci/status-failure-check-suite-succeed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"status": {
"state": "FAILURE"
},
"checkSuites": {
"nodes": [
{
"status": "COMPLETED",
"conclusion": "SUCCESS"
}
]
}
}
}
]

24 changes: 24 additions & 0 deletions test/fixtures/github-ci/status-succeed-check-suite-failure.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[
{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"messageHeadline": "doc: add api description README",
"author": {
"login": "foo"
},
"status": {
"state": "SUCCESS"
},
"checkSuites": {
"nodes": [
{
"status": "COMPLETED",
"conclusion": "FAILURE"
}
]
}
}
}
]

Loading

0 comments on commit fe21810

Please sign in to comment.