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.

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

feat: check Actions and handle doc-only changes #469

Merged
merged 1 commit into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions lib/ci/ci_type_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const CI_TYPE_ENUM = {
};

const CI_PROVIDERS = {
JENKINS: 'jenkins',
Copy link
Member

Choose a reason for hiding this comment

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

what motivated the name change? i'm not sure it'd be clear to me what "nodejs" CI meant 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't test only jenkins anymore, so a new type made more sense. And since the only repo we use ncu and Jenkins is nodejs/node, I thought a provider specific for that repo made sense. Happy to hear other suggestions for names though.

An alternative I thought was accepting a list of providers, so for nodejs/node we would have ['jenkins', 'actions']. But that increases complexity.

Although now I realized that maybe libuv also uses ncu? They definitely generate metadata and have a similar landing process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also FWIW, we stopped using ncu on llnode, which is what motivated me to have more than one provider here. We can probably get rid of this.

GITHUB: 'github-check'
GITHUB: 'github-check',
NODEJS: 'nodejs'
};

const { JOB_CI, FULL_CI } = CI_TYPE_ENUM;
Expand Down
83 changes: 77 additions & 6 deletions lib/pr_checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -214,8 +214,8 @@ class PRChecker {
}

let status = false;
if (ciType === CI_PROVIDERS.JENKINS) {
status = await this.checkJenkinsCI();
if (ciType === CI_PROVIDERS.NODEJS) {
status = await this.checkNodejsCI();
} else if (ciType === CI_PROVIDERS.GITHUB) {
status = this.checkGitHubCI();
}
Expand All @@ -233,7 +233,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)) {
Expand Down Expand Up @@ -292,17 +292,61 @@ class PRChecker {
cli.error(
`${failures.length} failure(s) on the last Jenkins CI run`);
status = false;
// NOTE(mmarchini): not sure why PEDING returns null
mmarchini marked this conversation as resolved.
Show resolved Hide resolved
} else if (result === null) {
cli.error(
'Last Jenkins CI still running');
status = false;
} else {
cli.ok('Last Jenkins CI successful');
}
}

this.CIStatus = status;
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;
}
Expand All @@ -314,7 +358,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;
}

Expand Down Expand Up @@ -350,6 +394,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;

Expand Down
5 changes: 5 additions & 0 deletions lib/queries/PR.gql
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ query PR($prid: Int!, $owner: String!, $repo: String!) {
name
}
},
files(first: 100) {
nodes {
path
}
},
title,
baseRefName,
headRefName,
Expand Down
2 changes: 1 addition & 1 deletion lib/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class Session {
}

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

get pullName() {
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/comments_with_failed_ci.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
{
"publishedAt": "2018-10-22T04:16:36.458Z",
"bodyText": "CI: https://ci.nodejs.org/job/node-test-pull-request/15442/"
}
]
6 changes: 6 additions & 0 deletions test/fixtures/comments_with_pending_ci.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
{
"publishedAt": "2018-10-22T04:16:36.458Z",
"bodyText": "CI: https://ci.nodejs.org/job/node-test-pull-request/32777/"
}
]
6 changes: 6 additions & 0 deletions test/fixtures/comments_with_success_ci.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
{
"publishedAt": "2018-10-22T04:16:36.458Z",
"bodyText": "CI: https://ci.nodejs.org/job/node-test-pull-request/15237/"
}
]
31 changes: 30 additions & 1 deletion test/fixtures/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ const approvingReviews = readJSON('reviews_approved.json');
const requestingChangesReviews = readJSON('reviews_requesting_changes.json');

const commentsWithCI = readJSON('comments_with_ci.json');
const commentsWithFailedCI = readJSON('comments_with_failed_ci.json');
const commentsWithLGTM = readJSON('comments_with_lgtm.json');
const commentsWithPendingCI = readJSON('comments_with_pending_ci.json');
const commentsWithSuccessCI = readJSON('comments_with_success_ci.json');

const oddCommits = readJSON('odd_commits.json');
const incorrectGitConfigCommits = readJSON('incorrect_git_config_commits.json');
Expand Down Expand Up @@ -101,6 +104,27 @@ for (const item of readdirSync(path('./github-ci'))) {
githubCI[basename(item, '.json')] = readJSON(`./github-ci/${item}`);
};

const pullRequests = {};

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

const jenkinsCI = {};

for (const subdir of readdirSync(path('./jenkins'))) {
for (const item of readdirSync(path(`./jenkins/${subdir}`))) {
if (!item.endsWith('.json')) {
continue;
}
jenkinsCI[`${subdir}/${basename(item, '.json')}`] =
readJSON(`./jenkins/${subdir}/${item}`);
}
};

module.exports = {
approved,
requestedChanges,
Expand All @@ -111,8 +135,12 @@ module.exports = {
approvingReviews,
requestingChangesReviews,
commentsWithCI,
commentsWithFailedCI,
commentsWithLGTM,
commentsWithSuccessCI,
commentsWithPendingCI,
oddCommits,
jenkinsCI,
githubCI,
incorrectGitConfigCommits,
simpleCommits,
Expand Down Expand Up @@ -141,5 +169,6 @@ module.exports = {
closedPR,
mergedPR,
selfRefPR,
duplicateRefPR
duplicateRefPR,
pullRequests
};
Empty file.
Loading