Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

git-node: add GitHub status as a CI option #369

Merged
merged 1 commit into from
Mar 9, 2020
Merged

git-node: add GitHub status as a CI option #369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 63 additions & 1 deletion lib/pr_checker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,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 @@ -265,6 +276,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class Session {
readme: this.readme,
waitTimeSingleApproval: this.waitTimeSingleApproval,
waitTimeMultiApproval: this.waitTimeMultiApproval,
ciType: this.ciType,
prid: this.prid
};
}
Expand Down Expand Up @@ -91,6 +92,10 @@ class Session {
return this.config.waitTimeMultiApproval;
}

get ciType() {
return this.config.ciType || 'jenkins';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make github-check the default now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet. If we set github-check as the default, ncu will not warn users that Jenkins jobs didn't ran on a nodejs/node Pull Request, since Travis and GitHub Actions also publish to GitHub Check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we'll need to add extra logic for Travis v.s. Jenkins status updates, then. Until then it always warns unless you do ncu-config set ciType github-check since the bot (or people) no longer posts that comment to the PRs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the bot (or people) no longer posts that comment to the PRs

Which comment? The bot is still commenting when a CI starts.

}

get pullName() {
return `${this.owner}/${this.repo}/pulls/${this.prid}`;
}
Expand Down
15 changes: 14 additions & 1 deletion test/fixtures/data.js
View file
Open in desktop
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 @@ -88,6 +91,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 @@ -101,6 +113,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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