lib: refactor collaborator getters

Put the readme request code into the helper so it
can be reused by other utilities.

lib/cli.js

@@ -137,4 +137,6 @@ class CLI {
   }
 };
 
+CLI.SPINNER_STATUS = SPINNER_STATUS;
+
 module.exports = CLI;

lib/collaborators.js

@@ -1,5 +1,7 @@
 'use strict';
 
+const fs = require('fs');
+
 const TSC_TITLE = '### TSC (Technical Steering Committee)';
 const TSCE_TITLE = '### TSC Emeriti';
 const CL_TITLE = '### Collaborators';
@@ -41,10 +43,35 @@ Collaborator.TYPES = {
   TSC, COLLABORATOR
 };
 
-function getCollaborators(readme, cli, owner, repo) {
+async function getCollaborators(cli, request, argv) {
+  const { readme, owner, repo } = argv;
+  let readmeText;
+  if (readme) {
+    cli.updateSpinner(`Reading collaborator contacts from ${readme}`);
+    readmeText = fs.readFileSync(readme, 'utf8');
+  } else {
+    cli.updateSpinner(
+      `Getting collaborator contacts from README of ${owner}/${repo}`);
+    const url = `https://raw.githubusercontent.com/${owner}/${repo}/master/README.md`;
+    readmeText = await request.text(url);
+  }
+
+  let collaborators;
+  try {
+    collaborators = parseCollaborators(readmeText, cli);
+  } catch (err) {
+    const readmePath = readme || `${owner}/${repo}/README.md`;
+    cli.stopSpinner(`Failed to get collaborator info from ${readmePath}`,
+      cli.SPINNER_STATUS.FAILED);
+    throw err;
+  }
+  return collaborators;
+}
+
+function parseCollaborators(readme, cli) {
   // This is more or less taken from
   // https://github.com/rvagg/iojs-tools/blob/master/pr-metadata/pr-metadata.js
-  const members = new Map();
+  const collaborators = new Map();
   let m;
 
   const tscIndex = readme.indexOf(TSC_TITLE);
@@ -77,24 +104,26 @@ function getCollaborators(readme, cli, owner, repo) {
   // eslint-disable-next-line no-cond-assign
   while ((m = CONTACT_RE.exec(readme)) && CONTACT_RE.lastIndex < tsceIndex) {
     const login = m[1].toLowerCase();
-    members.set(login, new Collaborator(m[1], m[2], m[3], TSC));
+    const user = new Collaborator(m[1], m[2], m[3], TSC);
+    collaborators.set(login, user);
   }
 
   CONTACT_RE.lastIndex = clIndex;
   // eslint-disable-next-line no-cond-assign
   while ((m = CONTACT_RE.exec(readme)) &&
     CONTACT_RE.lastIndex < cleIndex) {
     const login = m[1].toLowerCase();
-    if (!members.get(login)) {
-      members.set(login, new Collaborator(m[1], m[2], m[3], COLLABORATOR));
+    if (!collaborators.get(login)) {
+      const user = new Collaborator(m[1], m[2], m[3], COLLABORATOR);
+      collaborators.set(login, user);
     }
   }
 
-  if (!members.size) {
+  if (!collaborators.size) {
     throw new Error('Could not find any collaborators');
   }
 
-  return members;
+  return collaborators;
 }
 
 /**

lib/pr_data.js

@@ -2,7 +2,6 @@
 
 const { getCollaborators } = require('./collaborators');
 const { ReviewAnalyzer } = require('./reviews');
-const fs = require('fs');
 
 const {
   FIRST_TIME_CONTRIBUTOR, FIRST_TIMER
@@ -59,25 +58,8 @@ class PRData {
   }
 
   async getCollaborators() {
-    const { owner, repo, cli, request, argv } = this;
-    let readme;
-    if (argv.readme) {
-      cli.updateSpinner(`Reading collaborator contacts from ${argv.readme}`);
-      readme = fs.readFileSync(argv.readme, 'utf8');
-    } else {
-      cli.updateSpinner(
-        `Getting collaborator contacts from README of ${owner}/${repo}`);
-      const url = `https://raw.githubusercontent.com/${owner}/${repo}/master/README.md`;
-      readme = await request.text(url);
-    }
-    try {
-      this.collaborators = getCollaborators(readme, cli, owner, repo);
-    } catch (err) {
-      const readme = argv.readme || `${owner}/${repo}/README.md`;
-      cli.stopSpinner(`Failed to get collaborator info from ${readme}`,
-        cli.SPINNER_STATUS.FAILED);
-      throw err;
-    }
+    const { cli, request, argv } = this;
+    this.collaborators = await getCollaborators(cli, request, argv);
   }
 
   async getPR() {

test/fixtures/test_cli.js

@@ -17,15 +17,25 @@ class TestCLI {
   constructor() {
     this.spinner = {};
     this._calls = newCalls();
+    this.SPINNER_STATUS = CLI.SPINNER_STATUS;
   }
 
   clearCalls() {
     this._calls = newCalls();
   }
 
-  assertCalledWith(calls, msg) {
+  assertCalledWith(calls, options = {}) {
     const expected = Object.assign(newCalls(), calls);
-    assert.deepStrictEqual(this._calls, expected);
+    const actual = {};
+    const ignore = options.ignore || [];
+    for (const func of Object.keys(this._calls)) {
+      if (!ignore.includes(func)) {
+        actual[func] = this._calls[func];
+      } else {
+        actual[func] = [];
+      }
+    }
+    assert.deepStrictEqual(actual, expected);
   }
 }
 

test/unit/collaborators.test.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const assert = require('assert');
+const path = require('path');
 
 const {
   isCollaborator,
@@ -20,11 +21,6 @@ const assertThrowsAsync = require('../fixtures/assert_throws_async');
 
 describe('collaborators', function() {
   const collaborator = collaborators.get('bar');
-  let cli = null;
-
-  beforeEach(() => {
-    cli = new TestCLI();
-  });
 
   describe('Collaborator', () => {
     describe('isActor', () => {
@@ -83,64 +79,101 @@ describe('collaborators', function() {
   });
 
   describe('getCollaborators', () => {
+    let cli = null;
+
+    beforeEach(() => {
+      cli = new TestCLI();
+    });
+
+    function mockRequest(content, argv) {
+      const { owner, repo } = argv;
+      const expectedUrl =
+        `https://raw.githubusercontent.com/${owner}/${repo}/master/README.md`;
+      return {
+        async text(url) {
+          assert.strictEqual(url, expectedUrl);
+          return content;
+        }
+      };
+    }
+
+    it('should use specified readme', async function() {
+      const readmePath = path.resolve(
+        __dirname, '..', 'fixtures', 'README', 'README.md');
+      const argv = { owner: 'nodejs', repo: 'node', readme: readmePath };
+      const request = { async text() { assert.fail('should not call'); } };
+      const parsed = await getCollaborators(cli, request, argv);
+      assert.deepStrictEqual(parsed, collaborators);
+    });
+
     it('should return all collaborators', async function() {
-      const parsed = getCollaborators(readme, cli, 'nodejs', 'node');
+      const argv = { owner: 'nodejs', repo: 'node' };
+      const request = mockRequest(readme, argv);
+      const parsed = await getCollaborators(cli, request, argv);
       assert.deepStrictEqual(parsed, collaborators);
     });
 
-    it(
-      'should throw error if there is no TSC section in the README',
+    it('should throw error if there is no TSC section in the README',
       async() => {
+        const argv = { owner: 'nodejs', repo: 'node' };
+        const request = mockRequest(readmeNoTsc, argv);
         await assertThrowsAsync(
-          async() => getCollaborators(readmeNoTsc, cli, 'nodejs', 'node'),
+          async() => getCollaborators(cli, request, argv),
           Error);
       });
 
     it(
       'should throw error if there is no TSC Emeriti section in the README',
       async() => {
+        const argv = { owner: 'nodejs', repo: 'node' };
+        const request = mockRequest(readmeNoTscE, argv);
         await assertThrowsAsync(
-          async() => getCollaborators(readmeNoTscE, cli, 'nodejs', 'node'),
+          async() => getCollaborators(cli, request, argv),
           /Error: Couldn't find ### TSC Emeriti in the README/);
       });
 
     it('should throw error if there is no Collaborators section in the README',
       async() => {
+        const argv = { owner: 'nodejs', repo: 'node' };
+        const request = mockRequest(readmeNoCollaborators, argv);
         await assertThrowsAsync(
-          async() => getCollaborators(
-            readmeNoCollaborators, cli, 'nodejs', 'node'),
+          async() => getCollaborators(cli, request, argv),
           /Error: Couldn't find ### Collaborators in the README/);
       });
 
     it(
       'should throw error if there is no Collaborator' +
       'Emeriti section in the README',
       async() => {
+        const argv = { owner: 'nodejs', repo: 'node' };
+        const request = mockRequest(readmeNoCollaboratorE, argv);
         await assertThrowsAsync(
-          async() => getCollaborators(
-            readmeNoCollaboratorE, cli, 'nodejs', 'node'),
+          async() => getCollaborators(cli, request, argv),
           /Error: Couldn't find ### Collaborator Emeriti in the README/);
       });
 
     it(
       'should WARN if the TSC and Collaborators' +
       'section are not ordered in the README',
       async() => {
+        const argv = { owner: 'nodejs', repo: 'node' };
+        const request = mockRequest(readmeUnordered, argv);
         await assertThrowsAsync(
-          async() => getCollaborators(
-            readmeUnordered, cli, 'nodejs', 'node'),
+          async() => getCollaborators(cli, request, argv),
           /Error/);
         cli.assertCalledWith({
           warn: [[
             'Contacts in the README is out of order, analysis could go wrong.',
             { newline: true }
           ]]
-        });
+        }, { ignore: ['updateSpinner', 'stopSpinner'] });
       });
 
     it('should throw error if there are no collaborators', async() => {
+      const argv = { owner: 'nodejs', repo: 'node' };
+      const request = mockRequest(readmeUnordered, argv);
       await assertThrowsAsync(
-        async() => getCollaborators(readmeUnordered, cli, 'nodejs', 'node'),
+        async() => getCollaborators(cli, request, argv),
         /Error: Could not find any collaborators/);
     });
   });

test/unit/pr_data.test.js

@@ -2,7 +2,7 @@
 
 const assert = require('assert');
 const sinon = require('sinon');
-const path = require('path');
+
 const {
   approvingReviews,
   allGreenReviewers,
@@ -56,37 +56,3 @@ describe('PRData', function() {
     assert.deepStrictEqual(data.reviewers, allGreenReviewers, 'reviewers');
   });
 });
-
-describe('PRData', function() {
-  const request = {
-    text: sinon.stub(),
-    gql: sinon.stub()
-  };
-  request.text
-    .withArgs('https://raw.githubusercontent.com/nodejs/node/master/README.md')
-    .returns(new Error('Should not call'));
-  request.text.returns(new Error('unknown query'));
-  request.gql.withArgs('PR').returns(Promise.resolve(rawPR));
-  request.gql.withArgs('Reviews').returns(
-    Promise.resolve(toRaw(approvingReviews)));
-  request.gql.withArgs('PRComments').returns(
-    Promise.resolve(toRaw(commentsWithLGTM)));
-  request.gql.withArgs('PRCommits').returns(
-    Promise.resolve(toRaw(oddCommits)));
-  request.gql.returns(new Error('unknown query'));
-
-  it('getAll with specified readme', async() => {
-    const cli = new TestCLI();
-    const readmePath = path.resolve(
-      __dirname, '..', 'fixtures', 'README', 'README.md');
-    const argv2 = Object.assign({ readme: readmePath });
-    const data = new PRData(argv2, cli, request);
-    await data.getAll();
-    assert.deepStrictEqual(data.collaborators, collaborators, 'collaborators');
-    assert.deepStrictEqual(data.pr, firstTimerPR, 'pr');
-    assert.deepStrictEqual(data.reviews, approvingReviews, 'reviews');
-    assert.deepStrictEqual(data.comments, commentsWithLGTM, 'comments');
-    assert.deepStrictEqual(data.commits, oddCommits, 'commits');
-    assert.deepStrictEqual(data.reviewers, allGreenReviewers, 'reviewers');
-  });
-});