Skip to content

Commit

Permalink
feat: check CI failures before land
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed May 22, 2020
1 parent f58051c commit b3796ec
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 130 deletions.
7 changes: 4 additions & 3 deletions components/metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const fs = require('fs');

module.exports = async function getMetadata(argv, cli) {
const credentials = await auth({
github: true
github: true,
jenkins: true
});
const request = new Request(credentials);

Expand All @@ -38,8 +39,8 @@ module.exports = async function getMetadata(argv, cli) {
cli.write(metadata);
cli.separator();

const checker = new PRChecker(cli, data, argv);
const status = checker.checkAll(argv.checkComments);
const checker = new PRChecker(cli, data, request, argv);
const status = await checker.checkAll(argv.checkComments);
return {
status,
request,
Expand Down
6 changes: 6 additions & 0 deletions lib/ci/ci_type_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ const CI_TYPE_ENUM = {
JOB_CI: 1 << 2
};

const CI_PROVIDERS = {
JENKINS: 'jenkins',
GITHUB: 'github-check'
};

const { JOB_CI, FULL_CI, LITE_CI } = CI_TYPE_ENUM;

const CI_TYPES = new Map([
Expand Down Expand Up @@ -209,6 +214,7 @@ module.exports = {
CITGM, PR, COMMIT, BENCHMARK, LIBUV, V8, NOINTL,
LINTER, LITE_PR, LITE_COMMIT
},
CI_PROVIDERS,
isFullCI,
isLiteCI,
JobParser,
Expand Down
64 changes: 44 additions & 20 deletions lib/pr_checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ const {
const {
JobParser,
CI_TYPES,
CI_PROVIDERS,
isLiteCI,
isFullCI
} = require('./ci/ci_type_parser');
const { PRBuild } = require('./ci/ci_result_parser');

const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork';

Expand All @@ -31,8 +33,9 @@ class PRChecker {
* @param {{}} cli
* @param {PRData} data
*/
constructor(cli, data, argv) {
constructor(cli, data, request, argv) {
this.cli = cli;
this.request = request;
this.data = data;
const {
pr, reviewers, comments, reviews, commits, collaborators
Expand Down Expand Up @@ -66,10 +69,10 @@ class PRChecker {
return this.argv.waitTimeMultiApproval;
}

checkAll(checkComments = false) {
async checkAll(checkComments = false) {
const status = [
this.checkCommitsAfterReview(),
this.checkCI(),
await this.checkCI(),
this.checkReviewsAndWait(new Date(), checkComments),
this.checkMergeableState(),
this.checkPRState(),
Expand Down Expand Up @@ -201,32 +204,42 @@ 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();
async checkCI() {
const ciType = this.argv.ciType || CI_PROVIDERS.JENKINS;
const providers = Object.values(CI_PROVIDERS);

if (!providers.includes(ciType)) {
this.cli.error(
`Invalid ciType ${ciType} - must be one of ${providers.join(', ')}`);
return false;
}
this.cli.error(`Invalid ciType: ${ciType}`);
return false;

let status = false;
if (ciType === CI_PROVIDERS.JENKINS) {
status = await this.checkJenkinsCI();
} else if (ciType === CI_PROVIDERS.GITHUB) {
status = this.checkGitHubCI();
}

return status;
}

// TODO: we might want to check CI status when it's less flaky...
// TODO: not all PR requires CI...labels?
checkJenkinsCI() {
const { cli, commits, argv } = this;
async checkJenkinsCI() {
const { cli, commits, request, argv } = this;
const { maxCommits } = argv;
const thread = this.data.getThread();
const ciMap = new JobParser(thread).parse();

let status = true;
if (!ciMap.size) {
cli.error('No CI runs detected');
this.CIStatus = false;
return false;
} else if (!this.hasFullOrLiteCI(ciMap)) {
status = false;
cli.error('No full CI runs or lite CI runs detected');
cli.error('No full or lite Jenkins CI runs detected');
}

let lastCI;
Expand All @@ -236,7 +249,8 @@ class PRChecker {
if (!lastCI || lastCI.date < ci.date) {
lastCI = {
typeName: name,
date: ci.date
date: ci.date,
jobId: ci.jobid
};
}
}
Expand Down Expand Up @@ -270,6 +284,16 @@ class PRChecker {
cli.warn(infoMsg);
}
}

// Check the last CI run for its results.
const build = new PRBuild(cli, request, lastCI.jobId);
const { result, failures } = await build.getResults();

if (result === 'FAILURE') {
cli.error(
`${failures.length} failure(s) on the last Jenkins CI run`);
status = false;
}
}

this.CIStatus = status;
Expand Down Expand Up @@ -298,12 +322,12 @@ class PRChecker {
// GitHub new Check API
for (const { status, conclusion } of checkSuites.nodes) {
if (status !== 'COMPLETED') {
cli.error('CI is still running');
cli.error('GitHub CI is still running');
return false;
}

if (!['SUCCESS', 'NEUTRAL'].includes(conclusion)) {
cli.error('Last CI failed');
cli.error('Last GitHub CI failed');
return false;
}
}
Expand All @@ -312,17 +336,17 @@ class PRChecker {
if (commit.status) {
const { state } = commit.status;
if (state === 'PENDING') {
cli.error('CI is still running');
cli.error('GitHub CI is still running');
return false;
}

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

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

0 comments on commit b3796ec

Please sign in to comment.