Skip to content

Commit

Permalink
pr_summary: warn if we can't fetch author's name and email
Browse files Browse the repository at this point in the history
This displays a warning if we cannot fetch user's name and email from github
api, instead of displaying blank and null output in the author field (cli table).

Refs: nodejs/node#18721
Fixes: nodejs/node-core-utils#180
  • Loading branch information
gerkai committed Feb 17, 2018
1 parent fd05a42 commit 6e633df
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 3 deletions.
12 changes: 10 additions & 2 deletions lib/pr_summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,16 @@ class PRSummary {
cli.table('Title', `${title} (#${prid})`);
const authorHint =
this.data.authorIsNew() ? ', first-time contributor' : '';
cli.table('Author',
`${author.name} <${author.email}> (@${author.login}${authorHint})`);

if (author.name && author.email) {
cli.table('Author',
`${author.name} <${author.email}> (@${author.login}${authorHint})`);
} else {
// Unable to retrive email/name of the PR Author
cli.warn('Could not retrieve the email or name ' +
"of the PR author's from user's GitHub profile!");
}

cli.table('Branch', `${branch}`);
cli.table('Labels', `${labelStr}`);

Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const firstTimerPrivatePR = readJSON('first_timer_pr_with_private_email.json');
const semverMajorPR = readJSON('semver_major_pr.json');
const fixAndRefPR = readJSON('pr_with_fixes_and_refs.json');
const conflictingPR = readJSON('conflicting_pr.json');
const emptyProfilePR = readJSON('empty_profile_pr.json');
const closedPR = readJSON('./closed_pr.json');
const mergedPR = readJSON('./merged_pr.json');
const readme = readFile('./README/README.md');
Expand Down Expand Up @@ -90,6 +91,7 @@ module.exports = {
semverMajorPR,
fixAndRefPR,
conflictingPR,
emptyProfilePR,
readme,
readmeNoTsc,
readmeNoTscE,
Expand Down
22 changes: 22 additions & 0 deletions test/fixtures/empty_profile_pr.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"createdAt": "2017-10-24T11:13:43Z",
"authorAssociation": "COLLABORATOR",
"author": {
"login": "pr_author",
"email": "",
"name": null
},
"url": "https://github.com/nodejs/node/pull/18721",
"bodyHTML": "<p>Fix mdn links</p>",
"bodyText": "Fix mdn links",
"labels": {
"nodes": [
{
"name": "doc"
}
]
},
"title": "doc: fix mdn links",
"baseRefName": "master",
"headRefName": "fix-links"
}
36 changes: 35 additions & 1 deletion test/unit/pr_summary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ const {
oddCommits,
simpleCommits,
firstTimerPR,
semverMajorPR
semverMajorPR,
emptyProfilePR
} = require('../fixtures/data');
const TestCLI = require('../fixtures/test_cli');
const PRSummary = require('../../lib/pr_summary');
Expand Down Expand Up @@ -86,4 +87,37 @@ describe('PRSummary', () => {
summary.display();
cli.assertCalledWith(expectedLogs);
});

it('displays warning if pr author/email is not present', () => {
const cli = new TestCLI();
const prData = {
pr: emptyProfilePR,
commits: simpleCommits,
authorIsNew() {
return false;
}
};

const expectedLogs = {
log: [
[' - doc: some changes'],
[' - Their Github Account email <pr_author@example.com>']
],
table: [
['Title', 'doc: fix mdn links (#16348)'],
['Branch', 'pr_author:fix-links -> nodejs:master'],
['Labels', 'doc'],
['Commits', '1'],
['Committers', '1']
],
warn: [
['Could not retrieve the email or name ' +
"of the PR author's from user's GitHub profile!"]
]
};

const summary = new PRSummary(argv, cli, prData);
summary.display();
cli.assertCalledWith(expectedLogs);
});
});

0 comments on commit 6e633df

Please sign in to comment.