Skip to content

Commit

Permalink
feat: check commits after the last ci run (nodejs#69)
Browse files Browse the repository at this point in the history
Fixes: https://github.com/joyeecheung/node-core-utils/issues/66

* fixup: format warn message && format code

* fixup: set status false if exist commit after last ci

* fix: define lastCI as oldest one in latesst cis of all types

* test: add more commit cases in commits_after_ci.json

* feat: add bullet in warning message

* style: format code

* fix: log commit's title only
  • Loading branch information
ah-yu authored and priyank-p committed Nov 7, 2017
1 parent e829262 commit 72a2034
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 2 deletions.
25 changes: 24 additions & 1 deletion lib/pr_checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class PRChecker {
// TODO: we might want to check CI status when it's less flaky...
// TODO: not all PR requires CI...labels?
checkCI() {
const { pr, logger, comments, reviews } = this;
const { pr, logger, comments, reviews, commits } = this;
const prNode = {
publishedAt: pr.createdAt,
bodyText: pr.bodyText
Expand All @@ -175,9 +175,32 @@ class PRChecker {
logger.warn('No full CI runs detected');
}

let lastCI;
for (const [type, ci] of ciMap) {
const name = CI_TYPES.get(type).name;
logger.info(`Last ${name} CI on ${ci.date}: ${ci.link}`);
if (!lastCI || lastCI.date > ci.date) {
lastCI = {
typeName: name,
date: ci.date
};
}
}

if (lastCI) {
let didLogHeader = false;
commits.forEach((c) => {
const commit = c.commit;
if (commit.committedDate > lastCI.date) {
if (!didLogHeader) {
status = false;
didLogHeader = true;
logger.warn(`Commits pushed after the last ${lastCI.typeName} ` +
'CI run:');
}
logger.warn(`- ${commit.message.split('\n')[0]}`);
}
});
}

return status;
Expand Down
44 changes: 44 additions & 0 deletions test/fixtures/commits_after_ci.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"commits": [{
"commit": {
"committedDate": "2017-10-25T11:27:02Z",
"oid": "6c0945cbeea2cbbc97d13a3d9e3fe68bd145b985",
"message": "fixup: adjust spelling",
"author": {
"login": "bar"
}
}
},{
"commit": {
"committedDate": "2017-10-26T12:10:20Z",
"oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
"message": "doc: add api description README",
"author": {
"login": "foo"
}
}
},{
"commit": {
"committedDate": "2017-10-28T06:40:50Z",
"oid": "7dsjdw8olmxvfdh820jxnska0o28wjnjdsw9jdazxz",
"message": "feat: add something",
"author": {
"login": "bar"
}
}
},{
"commit": {
"committedDate": "2017-10-23T08:02:30Z",
"oid": "3edy729kzmdh74shd937dfnalxmdj38kjdnmaj87l9",
"message": "style: format code",
"author": {
"login": "Quo"
}
}
}],
"comment": [{
"bodyText": "CI: https://ci.nodejs.org/job/node-test-pull-request/10984/",
"publishedAt": "2017-10-24T11:19:25Z"
}]
}

2 changes: 2 additions & 0 deletions test/fixtures/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const multipleCommitsAfterReview = {
commits: readJSON('multiple_commits_after_review_commits.json'),
reviews: readJSON('multiple_commits_after_review_reviews.json')
};
const commitsAfterCi = readJSON('commits_after_ci.json');

collabArr.forEach((c) => {
Object.setPrototypeOf(c, Collaborator.prototype);
Expand Down Expand Up @@ -70,6 +71,7 @@ module.exports = {
simpleCommits,
singleCommitAfterReview,
multipleCommitsAfterReview,
commitsAfterCi,
collaborators,
firstTimerPR,
semverMajorPR,
Expand Down
35 changes: 34 additions & 1 deletion test/unit/pr_checker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const {
multipleCommitsAfterReview,
oddCommits,
simpleCommits,
commitsAfterCi,
collaborators,
firstTimerPR,
semverMajorPR
Expand Down Expand Up @@ -265,14 +266,46 @@ describe('PRChecker', () => {
reviewers: allGreenReviewers,
comments: commentsWithCI,
reviews: approvingReviews,
commits: simpleCommits,
commits: [],
collaborators
});

const status = checker.checkCI();
assert(status);
assert.deepStrictEqual(logger.logs, expectedLogs);
});

it('should check commits after last ci', () => {
const logger = new TestLogger();
const {commits, comment} = commitsAfterCi;

const expectedLogs = {
warn: [
['Commits pushed after the last Full CI run:'],
['- fixup: adjust spelling'],
['- doc: add api description README'],
['- feat: add something']
],
info: [
['Last Full CI on 2017-10-24T11:19:25Z: https://ci.nodejs.org/job/node-test-pull-request/10984/']
],
error: [],
trace: []
};

const checker = new PRChecker(logger, {
pr: firstTimerPR,
reviewers: allGreenReviewers,
comments: comment,
reviews: approvingReviews,
commits: commits,
collaborators
});

const status = checker.checkCI();
assert(!status);
assert.deepStrictEqual(logger.logs, expectedLogs);
});
});

describe('authorIsNew/checkAuthor', () => {
Expand Down

0 comments on commit 72a2034

Please sign in to comment.