Refactor into modules

git_reviewers/reviewers.py

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import argparse
-import os
 import subprocess
 import sys
 
@@ -11,39 +10,50 @@
 UBER = False
 
 
-def get_changed_files():
-    git_diff_files_command = ['git', 'diff-files']
-    process = subprocess.run(git_diff_files_command, stdout=subprocess.PIPE)
-    git_diff_files = process.stdout.decode("utf-8")
-    files = git_diff_files.split("\n")
-    files = [x.split("\t")[-1].strip() for x in files]
-    files = [x for x in files if x]
-    return files
-
-
-def extract_username(shortlog):
-    shortlog = shortlog.strip()
-    email = shortlog[shortlog.rfind("<")+1:]
-    email = email[:email.find(">")]
-    if UBER:
-        if email[-9:] == '@uber.com':
-            return email[:-9]
-        else:
-            return None
-    return email
-
-
-def get_reviewers(path, changed_files):
-    path = os.path.join(os.getcwd(), path)
-    reviewers = set()
-    for changed in changed_files:
-        git_shortlog_command = ['git', 'shortlog', '-sne', changed]
-        process = subprocess.run(git_shortlog_command, stdout=subprocess.PIPE)
-        git_shortlog = process.stdout.decode("utf-8").split("\n")
-        users = [extract_username(shortlog) for shortlog in git_shortlog]
-        users = [username for username in users if username]
-        reviewers = reviewers.union(users)
-    return reviewers
+class FindReviewers():
+    def get_reviewers(self):
+        raise NotImplementedError()
+
+    def run_command(self, command):
+        process = subprocess.run(command, stdout=subprocess.PIPE)
+        data = process.stdout.decode("utf-8")
+        return data
+
+    def extract_username_from_email(self, email):
+        if UBER:
+            if email[-9:] == '@uber.com':
+                return email[:-9]
+            else:
+                return None
+        return email
+
+
+class FindLogReviewers(FindReviewers):
+    def extract_username_from_shortlog(self, shortlog):
+        shortlog = shortlog.strip()
+        email = shortlog[shortlog.rfind("<")+1:]
+        email = email[:email.find(">")]
+        username = self.extract_username_from_email(email)
+        return username
+
+    def get_changed_files(self):
+        git_diff_files_command = ['git', 'diff-files']
+        git_diff_files = self.run_command(git_diff_files_command)
+        files = git_diff_files.split("\n")
+        files = [x.split("\t")[-1].strip() for x in files]
+        files = [x for x in files if x]
+        return files
+
+    def get_reviewers(self, changed_files):
+        reviewers = set()
+        for changed in changed_files:
+            git_shortlog_command = ['git', 'shortlog', '-sne', changed]
+            process = subprocess.run(git_shortlog_command, stdout=subprocess.PIPE)
+            git_shortlog = process.stdout.decode("utf-8").split("\n")
+            users = [self.extract_username_from_shortlog(shortlog) for shortlog in git_shortlog]
+            users = [username for username in users if username]
+            reviewers = reviewers.union(users)
+        return reviewers
 
 
 def show_reviewers(reviewers):
@@ -68,8 +78,9 @@ def main():
     args = parser.parse_args()
     UBER = args.uber
 
-    changed_files = get_changed_files()
-    reviewers = get_reviewers(args.path, changed_files)
+    finder = FindLogReviewers()
+    changed_files = finder.get_changed_files()
+    reviewers = finder.get_reviewers(changed_files)
     show_reviewers(reviewers)
 
 

git_reviewers/tests/test.py

@@ -8,7 +8,20 @@
 BASE_DIRECTORY = os.path.normpath(os.path.join(directory, '..', '..'))
 
 
-class TestGetChangedFiles(unittest.TestCase):
+class TestFindReviewers(unittest.TestCase):
+    def setUp(self):
+        self.finder = reviewers.FindReviewers()
+
+    def test_get_reviewers(self):
+        with self.assertRaises(NotImplementedError):
+            self.finder.get_reviewers()
+
+
+class TestFindLogReviewers(unittest.TestCase):
+    def setUp(self):
+        reviewers.UBER = False
+        self.finder = reviewers.FindLogReviewers()
+
     @patch('subprocess.run')
     def test_gets_diff_files(self, mock_run):
         process = MagicMock()
@@ -19,44 +32,37 @@ def test_gets_diff_files(self, mock_run):
         output += b'git_reviewers/reviewers.py'
         process.stdout = output
         mock_run.return_value = process
-        diff_files = reviewers.get_changed_files()
+        diff_files = self.finder.get_changed_files()
         expected = ['README.rst', 'git_reviewers/reviewers.py']
         self.assertEqual(diff_files, expected)
 
-
-class TestExtractUsername(unittest.TestCase):
-    def setUp(self):
-        reviewers.UBER = False
-
     def test_gets_emails(self):
         shortlog = '     3\tAlbert Wang <a@example.com>\n'
-        email = reviewers.extract_username(shortlog)
+        email = self.finder.extract_username_from_shortlog(shortlog)
         self.assertEqual(email, 'a@example.com')
 
     def test_excludes_non_uber_emails(self):
         reviewers.UBER = True
         shortlog = '     3\tAlbert Wang <a@example.com>\n'
-        email = reviewers.extract_username(shortlog)
+        email = self.finder.extract_username_from_shortlog(shortlog)
         self.assertEqual(email, None)
 
     def test_gets_uber_emails(self):
         reviewers.UBER = True
         shortlog = '     3\tAlbert Wang <albertyw@uber.com>\n'
-        email = reviewers.extract_username(shortlog)
+        email = self.finder.extract_username_from_shortlog(shortlog)
         self.assertEqual(email, 'albertyw')
 
-
-class TestGetReviewers(unittest.TestCase):
     @patch('subprocess.run')
     def test_gets_reviewers(self, mock_run):
-        path = ''
+        reviewers.UBER = True
         changed_files = ['README.rst']
         process = MagicMock()
         git_shortlog = b'     3\tAlbert Wang <albertyw@uber.com>\n'
         git_shortlog += b'3\tAlbert Wang <a@example.com>\n'
         process.stdout = git_shortlog
         mock_run.return_value = process
-        users = reviewers.get_reviewers(path, changed_files)
+        users = self.finder.get_reviewers(changed_files)
         self.assertEqual(users, set(['albertyw']))