Fix the way the weighted list is built

TailorDev · Jan 31, 2017 · a71f404 · a71f404
1 parent 7d37ebc
commit a71f404
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 11 deletions.
diff --git a/tasks/findReviewers.js b/tasks/findReviewers.js
@@ -71,6 +71,27 @@ const getCollaborators = async (github, repository) => github.repos
   .catch([])
 ;
 
+const getPotentialReviewers = (authorsFromHistory, collaborators) => {
+  const potentialReviewers = authorsFromHistory;
+  const authorLogins = Object.keys(authorsFromHistory);
+
+  if (authorLogins.length > 0) {
+    const collaboratorLogins = Object.keys(collaborators);
+
+    authorLogins.forEach((login) => {
+      if (!collaboratorLogins.includes(login)) {
+        delete potentialReviewers[login];
+      }
+    });
+  }
+
+  if (Object.keys(potentialReviewers).length === 0) {
+    return collaborators;
+  }
+
+  return potentialReviewers;
+};
+
 const logPotentialReviewers = (logger, collaborators, authorsFromHistory, reviewers) => {
   logger.info([
     "message='potential reviewers'",
@@ -149,17 +170,7 @@ exports.configure = config => async (repositoryId, number, author, logger) => {
     .then(createWeightedMap)
   ;
 
-  let reviewers = [];
-  if (Object.keys(authorsFromHistory).length > 0) {
-    const allowed = Object.keys(collaborators);
-    reviewers = Object.keys(authorsFromHistory).filter(k => allowed.includes(k));
-  }
-
-  // fallback
-  if (reviewers.length === 0) {
-    reviewers = collaborators;
-  }
-
+  const reviewers = getPotentialReviewers(authorsFromHistory, collaborators);
   logPotentialReviewers(logger, collaborators, authorsFromHistory, reviewers);
 
   // 3 - We're almost there
@@ -189,3 +200,5 @@ exports.configure = config => async (repositoryId, number, author, logger) => {
     })
   ;
 };
+
+exports.getPotentialReviewers = getPotentialReviewers;
diff --git a/test/findReviewers.js b/test/findReviewers.js
@@ -0,0 +1,55 @@
+const { expect } = require('chai');
+const findReviewersTask = require('../tasks/findReviewers');
+
+describe('tasks/findReviewers', () => {
+  describe('getPotentialReviewers()', () => {
+    const getPotentialReviewers = findReviewersTask.getPotentialReviewers;
+
+    it('should compute a weighted list of potential reviwers', () => {
+      const reviewers = getPotentialReviewers(
+        { foo: 40, bar: 2, titi: 1 },
+        { foo: 1, bar: 1, titi: 1 }
+      );
+
+      expect(reviewers).to.contain.all.keys(['foo', 'bar', 'titi']);
+    });
+
+    it('should exclude non-collaborators', () => {
+      const reviewers = getPotentialReviewers(
+        { foo: 40, bar: 2, titi: 1 },
+        { foo: 1 }
+      );
+
+      expect(reviewers).to.contain.all.keys(['foo']);
+      expect(reviewers).not.to.contain.all.keys(['bar', 'titi']);
+    });
+
+    it('should return collaborators if no reviewers from authors', () => {
+      const reviewers = getPotentialReviewers(
+        { foo: 40, bar: 2, titi: 1 },
+        { babar: 1 }
+      );
+
+      expect(reviewers).to.contain.all.keys(['babar']);
+      expect(reviewers).not.to.contain.all.keys(['foo', 'bar', 'titi']);
+    });
+
+    it('should accept an empty set of authors', () => {
+      const reviewers = getPotentialReviewers({}, { babar: 1, 'john-y': 1 });
+
+      expect(reviewers).to.contain.all.keys(['babar', 'john-y']);
+    });
+
+    it('should accept an empty set of collaborators', () => {
+      const reviewers = getPotentialReviewers({}, {});
+
+      expect(reviewers).to.deep.equal({});
+    });
+
+    it('should does not return any reviewer when no collaborators', () => {
+      const reviewers = getPotentialReviewers({ boo: 23, baz: 1 }, {});
+
+      expect(reviewers).to.deep.equal({});
+    });
+  });
+});