diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 771edcc9..c4b0532a 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -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(); @@ -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; diff --git a/lib/queries/PRCommits.gql b/lib/queries/PRCommits.gql index 4eced126..f5a0647f 100644 --- a/lib/queries/PRCommits.gql +++ b/lib/queries/PRCommits.gql @@ -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 + } } } } diff --git a/lib/request.js b/lib/request.js index 6a3fa255..3aff3e99 100644 --- a/lib/request.js +++ b/lib/request.js @@ -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, diff --git a/lib/session.js b/lib/session.js index 0666ac0c..81087fa3 100644 --- a/lib/session.js +++ b/lib/session.js @@ -55,6 +55,7 @@ class Session { readme: this.readme, waitTimeSingleApproval: this.waitTimeSingleApproval, waitTimeMultiApproval: this.waitTimeMultiApproval, + ciType: this.ciType, prid: this.prid }; } @@ -91,6 +92,10 @@ class Session { return this.config.waitTimeMultiApproval; } + get ciType() { + return this.config.ciType || 'jenkins'; + } + get pullName() { return `${this.owner}/${this.repo}/pulls/${this.prid}`; } diff --git a/test/fixtures/data.js b/test/fixtures/data.js index 9c85dfb4..d8ee3f85 100644 --- a/test/fixtures/data.js +++ b/test/fixtures/data.js @@ -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'); @@ -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, @@ -101,6 +113,7 @@ module.exports = { commentsWithLiteCI, commentsWithLGTM, oddCommits, + githubCI, incorrectGitConfigCommits, simpleCommits, singleCommitAfterReview, diff --git a/test/fixtures/github-ci/both-apis-failure.json b/test/fixtures/github-ci/both-apis-failure.json new file mode 100644 index 00000000..3866f56d --- /dev/null +++ b/test/fixtures/github-ci/both-apis-failure.json @@ -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" + } + ] + } + } + } +] + diff --git a/test/fixtures/github-ci/both-apis-success.json b/test/fixtures/github-ci/both-apis-success.json new file mode 100644 index 00000000..86fb337c --- /dev/null +++ b/test/fixtures/github-ci/both-apis-success.json @@ -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" + } + ] + } + } + } +] + diff --git a/test/fixtures/github-ci/check-suite-failure.json b/test/fixtures/github-ci/check-suite-failure.json new file mode 100644 index 00000000..9f34c110 --- /dev/null +++ b/test/fixtures/github-ci/check-suite-failure.json @@ -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" + } + ] + } + } + } +] diff --git a/test/fixtures/github-ci/check-suite-pending.json b/test/fixtures/github-ci/check-suite-pending.json new file mode 100644 index 00000000..de284a9a --- /dev/null +++ b/test/fixtures/github-ci/check-suite-pending.json @@ -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" + } + ] + } + } + } +] + diff --git a/test/fixtures/github-ci/check-suite-success.json b/test/fixtures/github-ci/check-suite-success.json new file mode 100644 index 00000000..ed7f3d6b --- /dev/null +++ b/test/fixtures/github-ci/check-suite-success.json @@ -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" + } + ] + } + } + } +] diff --git a/test/fixtures/github-ci/commit-status-only-failure.json b/test/fixtures/github-ci/commit-status-only-failure.json new file mode 100644 index 00000000..077b754d --- /dev/null +++ b/test/fixtures/github-ci/commit-status-only-failure.json @@ -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" + } + } + } +] + diff --git a/test/fixtures/github-ci/commit-status-only-pending.json b/test/fixtures/github-ci/commit-status-only-pending.json new file mode 100644 index 00000000..15c4ba64 --- /dev/null +++ b/test/fixtures/github-ci/commit-status-only-pending.json @@ -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" + } + } + } +] + diff --git a/test/fixtures/github-ci/commit-status-only-success.json b/test/fixtures/github-ci/commit-status-only-success.json new file mode 100644 index 00000000..53d332c0 --- /dev/null +++ b/test/fixtures/github-ci/commit-status-only-success.json @@ -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" + } + } + } +] + diff --git a/test/fixtures/github-ci/no-status.json b/test/fixtures/github-ci/no-status.json new file mode 100644 index 00000000..8ef841d3 --- /dev/null +++ b/test/fixtures/github-ci/no-status.json @@ -0,0 +1,13 @@ +[ + { + "commit": { + "committedDate": "2017-10-26T12:10:20Z", + "oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d", + "messageHeadline": "doc: add api description README", + "author": { + "login": "foo" + } + } + } +] + diff --git a/test/fixtures/github-ci/status-failure-check-suite-succeed.json b/test/fixtures/github-ci/status-failure-check-suite-succeed.json new file mode 100644 index 00000000..98b03e69 --- /dev/null +++ b/test/fixtures/github-ci/status-failure-check-suite-succeed.json @@ -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" + } + ] + } + } + } +] + diff --git a/test/fixtures/github-ci/status-succeed-check-suite-failure.json b/test/fixtures/github-ci/status-succeed-check-suite-failure.json new file mode 100644 index 00000000..cef60ce6 --- /dev/null +++ b/test/fixtures/github-ci/status-succeed-check-suite-failure.json @@ -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" + } + ] + } + } + } +] + diff --git a/test/fixtures/github-ci/two-commits-first-ci.json b/test/fixtures/github-ci/two-commits-first-ci.json new file mode 100644 index 00000000..84fc6fd3 --- /dev/null +++ b/test/fixtures/github-ci/two-commits-first-ci.json @@ -0,0 +1,25 @@ +[ + { + "commit": { + "committedDate": "2017-10-25T11:27:02Z", + "oid": "6c0945cbeea2cbbc97d13a3d9e3fe68bd145b985", + "messageHeadline": "fixup: adjust spelling", + "author": { + "login": "bar" + }, + "status": { + "state": "SUCCESS" + } + } + },{ + "commit": { + "committedDate": "2017-10-26T12:10:20Z", + "oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d", + "messageHeadline": "doc: add api description README", + "author": { + "login": "foo" + } + } + } +] + diff --git a/test/fixtures/github-ci/two-commits-last-ci.json b/test/fixtures/github-ci/two-commits-last-ci.json new file mode 100644 index 00000000..8f0b6c9f --- /dev/null +++ b/test/fixtures/github-ci/two-commits-last-ci.json @@ -0,0 +1,25 @@ +[ + { + "commit": { + "committedDate": "2017-10-25T11:27:02Z", + "oid": "6c0945cbeea2cbbc97d13a3d9e3fe68bd145b985", + "messageHeadline": "fixup: adjust spelling", + "author": { + "login": "bar" + } + } + },{ + "commit": { + "committedDate": "2017-10-26T12:10:20Z", + "oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d", + "messageHeadline": "doc: add api description README", + "author": { + "login": "foo" + }, + "status": { + "state": "SUCCESS" + } + } + } +] + diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index a94194dd..dca6e5a9 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -19,6 +19,7 @@ const { singleGreenReviewer, requestedChangesReviewers, approvingReviews, + githubCI, requestingChangesReviews, noReviewers, commentsWithCI, @@ -841,6 +842,268 @@ describe('PRChecker', () => { }); }); + describe('checkGitHubCI', () => { + const baseData = { + pr: firstTimerPR, + reviewers: allGreenReviewers, + comments: commentsWithLGTM, + reviews: approvingReviews, + collaborators, + authorIsNew: () => true, + getThread() { + return PRData.prototype.getThread.call(this); + } + }; + const testArgv = Object.assign({}, argv, { ciType: 'github-check' }); + + it('should error if no CI runs detected', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['No CI runs detected'] + ] + }; + + const commits = githubCI['no-status']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if both statuses failed', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['Last CI failed'] + ] + }; + + const commits = githubCI['both-apis-failure']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should succeed if both statuses succeeded', () => { + const cli = new TestCLI(); + + const expectedLogs = { + info: [ + ['Last CI run was successful'] + ] + }; + + const commits = githubCI['both-apis-success']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if Check suite failed', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['Last CI failed'] + ] + }; + + const commits = githubCI['check-suite-failure']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if Check suite pending', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['CI is still running'] + ] + }; + + const commits = githubCI['check-suite-pending']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should succeed if Check suite succeeded', () => { + const cli = new TestCLI(); + + const expectedLogs = { + info: [ + ['Last CI run was successful'] + ] + }; + + const commits = githubCI['check-suite-success']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if commit status failed', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['Last CI failed'] + ] + }; + + const commits = githubCI['commit-status-only-failure']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if commit status pending', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['CI is still running'] + ] + }; + + const commits = githubCI['commit-status-only-pending']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should succeed if commit status succeeded', () => { + const cli = new TestCLI(); + + const expectedLogs = { + info: [ + ['Last CI run was successful'] + ] + }; + + const commits = githubCI['commit-status-only-success']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if Check succeeded but commit status failed ', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['Last CI failed'] + ] + }; + + const commits = githubCI['status-failure-check-suite-succeed']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if commit status succeeded but Check failed ', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['Last CI failed'] + ] + }; + + const commits = githubCI['status-succeed-check-suite-failure']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should error if last commit doesnt have CI', () => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['No CI runs detected'] + ] + }; + + const commits = githubCI['two-commits-first-ci']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + + it('should succeed with two commits if last one has CI', () => { + const cli = new TestCLI(); + + const expectedLogs = { + info: [ + ['Last CI run was successful'] + ] + }; + + const commits = githubCI['two-commits-last-ci']; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, testArgv); + + const status = checker.checkCI(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); + }); + describe('checkAuthor', () => { it('should check odd commits for first timers', () => { const cli = new TestCLI();