From 5ce52650e1a0d15e3543995b43f0379d6db285fb Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Sat, 8 Aug 2020 19:15:00 -0700 Subject: [PATCH] feat: check Actions and handle doc-only changes Doc-only changes don't need a full Jenkins CI, instead we can check if the last Actions run was successful. Therefore this commit also adds check for Action runs. Jenkins CI messages were improved as well. Fix: https://github.com/nodejs/node-core-utils/issues/324 Fix: https://github.com/nodejs/node/issues/32335 Fix: https://github.com/nodejs/node/issues/29770 --- lib/ci/ci_type_parser.js | 3 +- lib/pr_checker.js | 81 ++++++++++++++++++++++++++++++++++-- lib/queries/PR.gql | 5 +++ lib/session.js | 2 +- test/unit/pr_checker.test.js | 35 +++++++++++++--- 5 files changed, 114 insertions(+), 12 deletions(-) diff --git a/lib/ci/ci_type_parser.js b/lib/ci/ci_type_parser.js index dd87a09d..0e151df7 100644 --- a/lib/ci/ci_type_parser.js +++ b/lib/ci/ci_type_parser.js @@ -26,7 +26,8 @@ const CI_TYPE_ENUM = { const CI_PROVIDERS = { JENKINS: 'jenkins', - GITHUB: 'github-check' + GITHUB: 'github-check', + NODEJS: 'nodejs' }; const { JOB_CI, FULL_CI } = CI_TYPE_ENUM; diff --git a/lib/pr_checker.js b/lib/pr_checker.js index bb50efe1..3bb0e2ce 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -204,7 +204,7 @@ class PRChecker { } async checkCI() { - const ciType = this.argv.ciType || CI_PROVIDERS.JENKINS; + const ciType = this.argv.ciType || CI_PROVIDERS.NODEJS; const providers = Object.values(CI_PROVIDERS); if (!providers.includes(ciType)) { @@ -218,6 +218,8 @@ class PRChecker { status = await this.checkJenkinsCI(); } else if (ciType === CI_PROVIDERS.GITHUB) { status = this.checkGitHubCI(); + } else if (ciType === CI_PROVIDERS.NODEJS) { + status &= await this.checkNodejsCI(); } return status; @@ -233,7 +235,7 @@ class PRChecker { let status = true; if (!ciMap.size) { - cli.error('No CI runs detected'); + cli.error('No Jenkins CI runs detected'); this.CIStatus = false; return false; } else if (!this.hasFullCI(ciMap)) { @@ -292,6 +294,13 @@ class PRChecker { cli.error( `${failures.length} failure(s) on the last Jenkins CI run`); status = false; + // NOTE(mmarchini): not sure why PEDING returns null + } else if (result === null) { + cli.error( + 'Last Jenkins CI still running'); + status = false; + } else { + cli.ok('Last Jenkins CI successful'); } } @@ -299,10 +308,47 @@ class PRChecker { return status; } + checkActionsCI() { + const { cli, commits } = this; + + if (!commits || commits.length === 0) { + 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 GitHub CI runs detected'); + return false; + } + + // GitHub new Check API + for (const { status, conclusion } of checkSuites.nodes) { + if (status !== 'COMPLETED') { + cli.error('GitHub CI is still running'); + return false; + } + + if (!['SUCCESS', 'NEUTRAL'].includes(conclusion)) { + cli.error('Last GitHub CI failed'); + return false; + } + } + + cli.ok('Last GitHub Actions successful'); + this.CIStatus = true; + return true; + } + checkGitHubCI() { const { cli, commits } = this; - if (!commits) { + if (!commits || commits.length === 0) { cli.error('No commits detected'); return false; } @@ -314,7 +360,7 @@ class PRChecker { this.CIStatus = false; const checkSuites = commit.checkSuites || { nodes: [] }; if (!commit.status && checkSuites.nodes.length === 0) { - cli.error('No CI runs detected'); + cli.error('No GitHub CI runs detected'); return false; } @@ -350,6 +396,33 @@ class PRChecker { return true; } + isOnlyDocChanges() { + const { cli, pr } = this; + + // NOTE(mmarchini): if files not present, fallback + // to old behavior. This should only be the case on old tests + // TODO(mmarchini): add files to all fixtures on old tests + if (!pr.files) { + return false; + } + + for (const { path } of pr.files.nodes) { + if (!path.endsWith('.md')) { + return false; + } + } + cli.info('Doc-only changes'); + return true; + } + + async checkNodejsCI() { + let status = this.checkActionsCI(); + if (!this.isOnlyDocChanges()) { + status &= await this.checkJenkinsCI(); + } + return status; + } + checkAuthor() { const { cli, commits, pr } = this; diff --git a/lib/queries/PR.gql b/lib/queries/PR.gql index 81b81911..27cc1396 100644 --- a/lib/queries/PR.gql +++ b/lib/queries/PR.gql @@ -18,6 +18,11 @@ query PR($prid: Int!, $owner: String!, $repo: String!) { name } }, + files(first: 100) { + nodes { + path + } + }, title, baseRefName, headRefName, diff --git a/lib/session.js b/lib/session.js index 4db0e3d9..9ba61a5b 100644 --- a/lib/session.js +++ b/lib/session.js @@ -99,7 +99,7 @@ class Session { } get ciType() { - return this.config.ciType || 'jenkins'; + return this.config.ciType || 'nodejs'; } get pullName() { diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 8fca0f21..1b0cf33f 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -619,7 +619,8 @@ describe('PRChecker', () => { const expectedLogs = { error: [ - ['No CI runs detected'] + ['No GitHub CI runs detected'], + ['No Jenkins CI runs detected'] ] }; @@ -646,6 +647,12 @@ describe('PRChecker', () => { const cli = new TestCLI(); const expectedLogs = { + ok: [ + ['Last Jenkins CI successful'] + ], + error: [ + ['No commits detected'] + ], info: [ [ 'Last Full PR CI on 2017-10-25T04:16:36.458Z: ' + @@ -693,7 +700,7 @@ describe('PRChecker', () => { const checker = new PRChecker(cli, data, {}, argv); const status = await checker.checkCI(); - assert(status); + assert(!status); cli.assertCalledWith(expectedLogs, { ignore: ['startSpinner', 'updateSpinner', 'stopSpinner'] }); @@ -712,6 +719,12 @@ describe('PRChecker', () => { ], info: [ ['Last Full PR CI on 2017-10-24T11:19:25Z: https://ci.nodejs.org/job/node-test-pull-request/10984/'] + ], + ok: [ + ['Last Jenkins CI successful'] + ], + error: [ + ['No GitHub CI runs detected'] ] }; @@ -754,7 +767,12 @@ describe('PRChecker', () => { 'https://ci.nodejs.org/job/node-test-pull-request/12984/' ] ], - error: [] + ok: [ + ['Last Jenkins CI successful'] + ], + error: [ + ['No GitHub CI runs detected'] + ] }; const checker = new PRChecker(cli, { @@ -792,7 +810,12 @@ describe('PRChecker', () => { 'https://ci.nodejs.org/job/node-test-pull-request/12984/' ] ], - error: [] + ok: [ + ['Last Jenkins CI successful'] + ], + error: [ + ['No GitHub CI runs detected'] + ] }; const checker = new PRChecker(cli, { @@ -835,7 +858,7 @@ describe('PRChecker', () => { const expectedLogs = { error: [ - ['No CI runs detected'] + ['No GitHub CI runs detected'] ] }; @@ -1044,7 +1067,7 @@ describe('PRChecker', () => { const expectedLogs = { error: [ - ['No CI runs detected'] + ['No GitHub CI runs detected'] ] };