Ensure reviewer and commit author aren't the same

Check all approvers against all commit authors, and ensure a commit
author hasn't approved their own MR, unless more approvers are present.

marge/job.py

@@ -73,6 +73,7 @@ def add_trailers(self, merge_request):
         # add Reviewed-by
         reviewers = (
             _get_reviewer_names_and_emails(
+                merge_request.fetch_commits(),
                 merge_request.fetch_approvals(),
                 self._api,
             ) if self._options.add_reviewers else None
@@ -291,11 +292,14 @@ def update_from_target_branch_and_push(
                 repo.remove_branch(source_branch)
 
 
-def _get_reviewer_names_and_emails(approvals, api):
+def _get_reviewer_names_and_emails(commits, approvals, api):
     """Return a list ['A. Prover <a.prover@example.com', ...]` for `merge_request.`"""
-
     uids = approvals.approver_ids
-    return ['{0.name} <{0.email}>'.format(User.fetch_by_id(uid, api)) for uid in uids]
+    users = [User.fetch_by_id(uid, api) for uid in uids]
+    self_reviewed = {commit['author_email'] for commit in commits} & {user.email for user in users}
+    if self_reviewed and len(users) <= 1:
+        raise CannotMerge('Commits require at least one independent reviewer.')
+    return ['{0.name} <{0.email}>'.format(user) for user in users]
 
 
 JOB_OPTIONS = [

marge/merge_request.py

@@ -147,3 +147,6 @@ def fetch_approvals(self):
         approvals = Approvals(self.api, info)
         approvals.refetch_info()
         return approvals
+
+    def fetch_commits(self):
+        return self._api.call(GET('/projects/{0.project_id}/merge_requests/{0.iid}/commits'.format(self)))

tests/test_approvals.py

@@ -1,11 +1,13 @@
 from unittest.mock import call, Mock, patch
 
+import pytest
+
 from marge.gitlab import Api, GET, POST, Version
 from marge.approvals import Approvals
 from marge.merge_request import MergeRequest
 import marge.user
 # testing this here is more convenient
-from marge.job import _get_reviewer_names_and_emails
+from marge.job import CannotMerge, _get_reviewer_names_and_emails
 
 INFO = {
     "id": 5,
@@ -114,7 +116,27 @@ def test_reapprove(self):
     @patch('marge.user.User.fetch_by_id')
     def test_get_reviewer_names_and_emails(self, user_fetch_by_id):
         user_fetch_by_id.side_effect = lambda id, _: marge.user.User(self.api, USERS[id])
-        assert _get_reviewer_names_and_emails(approvals=self.approvals, api=self.api) == [
+        assert _get_reviewer_names_and_emails(commits=[], approvals=self.approvals, api=self.api) == [
             'Administrator <root@localhost>',
             'Roger Ebert <ebert@example.com>'
         ]
+
+    @patch('marge.user.User.fetch_by_id')
+    def test_approvals_fails_when_same_author(self, user_fetch_by_id):
+        info = dict(INFO, approved_by=list(INFO['approved_by']))
+        del info['approved_by'][1]
+        approvals = Approvals(self.api, info)
+        user_fetch_by_id.side_effect = lambda id, _: marge.user.User(self.api, USERS[id])
+        commits = [{'author_email': 'root@localhost'}]
+        with pytest.raises(CannotMerge):
+            _get_reviewer_names_and_emails(commits=commits, approvals=approvals, api=self.api)
+
+    @patch('marge.user.User.fetch_by_id')
+    def test_approvals_succeeds_with_independent_author(self, user_fetch_by_id):
+        user_fetch_by_id.side_effect = lambda id, _: marge.user.User(self.api, USERS[id])
+        print(INFO['approved_by'])
+        commits = [{'author_email': 'root@localhost'}]
+        assert _get_reviewer_names_and_emails(commits=commits, approvals=self.approvals, api=self.api) == [
+            'Administrator <root@localhost>',
+            'Roger Ebert <ebert@example.com>',
+        ]