From 565adb55a6b560310c938ebacd4a9267958fdac4 Mon Sep 17 00:00:00 2001
From: agable <agable@chromium.org>
Date: Fri, 22 Jul 2016 14:48:07 -0700
Subject: [PATCH] Have presubmit accept various Code-Review label
 configurations

Since different projects can have different configurations for what
the maximum value of the "Code-Review" label is in Gerrit, this teaches
presubmit_support to inspect the maximum configured value and see who
has granted it (the same behavior as the Submit button in Gerrit itself).

R=andybons@chromium.org, martiniss@chromium.org, tandrii@chromium.org
BUG=630738

Review-Url: https://codereview.chromium.org/2178673002
---
 presubmit_support.py        | 11 ++---
 tests/presubmit_unittest.py | 86 +++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/presubmit_support.py b/presubmit_support.py
index 0e85eb0b7a..807da4a05d 100755
--- a/presubmit_support.py
+++ b/presubmit_support.py
@@ -244,13 +244,10 @@ class GerritAccessor(object):
     return self.GetChangeInfo(issue)['owner']['email']
 
   def GetChangeReviewers(self, issue, approving_only=True):
-    # Gerrit has 'approved' sub-section, but it only lists 1 approver.
-    # So, if we look only for approvers, we have to look at all anyway.
-    # Also, assume LGTM means Code-Review label == 2. Other configurations
-    # aren't supported.
-    return [r['email']
-            for r in self.GetChangeInfo(issue)['labels']['Code-Review']['all']
-            if not approving_only or '2' == str(r.get('value', 0))]
+    cr = self.GetChangeInfo(issue)['labels']['Code-Review']
+    max_value = max(int(k) for k in cr['values'].keys())
+    return [r['email'] for r in cr['all']
+            if not approving_only or r.get('value', 0) == max_value]
 
 
 class OutputApi(object):
diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py
index 2863c1446c..1f65eb97cf 100755
--- a/tests/presubmit_unittest.py
+++ b/tests/presubmit_unittest.py
@@ -2725,6 +2725,29 @@ class CannedChecksUnittest(PresubmitTestsBase):
         gerrit_response=response,
         expected_output='')
 
+    # Testing configuration with on -1..+1.
+    response = {
+      "owner": {"email": "john@example.com"},
+      "labels": {"Code-Review": {
+        u'all': [
+          {
+            u'email': u'ben@example.com',
+            u'value': 1
+          },
+        ],
+        u'approved': {u'email': u'ben@example.org'},
+        u'default_value': 0,
+        u'values': {u' 0': u'No score',
+                    u'+1': u'Looks good to me',
+                    u'-1': u"I would prefer that you didn't submit this"}
+      }},
+    }
+    self.AssertOwnersWorks(approvers=set(['ben@example.com']),
+        gerrit_response=response,
+        is_committing=True,
+        expected_output='')
+
+
   def testCannedCheckOwners_Approved(self):
     response = {
       "owner_email": "john@example.com",
@@ -2744,6 +2767,69 @@ class CannedChecksUnittest(PresubmitTestsBase):
         rietveld_response=response,
         expected_output='')
 
+  def testCannedCheckOwners_NotApproved_Gerrit(self):
+    response = {
+      "owner": {"email": "john@example.com"},
+      "labels": {"Code-Review": {
+        u'all': [
+          {
+            u'email': u'john@example.com',  # self +1 :)
+            u'value': 1
+          },
+          {
+            u'email': u'ben@example.com',
+            u'value': 1
+          },
+        ],
+        u'approved': {u'email': u'ben@example.org'},
+        u'default_value': 0,
+        u'values': {u' 0': u'No score',
+                    u'+1': u'Looks good to me, but someone else must approve',
+                    u'+2': u'Looks good to me, approved',
+                    u'-1': u"I would prefer that you didn't submit this",
+                    u'-2': u'Do not submit'}
+      }},
+    }
+    self.AssertOwnersWorks(
+        approvers=set(),
+        reviewers=set(["ben@example.com"]),
+        gerrit_response=response,
+        is_committing=True,
+        expected_output=
+            'Missing LGTM from someone other than john@example.com\n')
+
+    self.AssertOwnersWorks(
+        approvers=set(),
+        reviewers=set(["ben@example.com"]),
+        is_committing=False,
+        gerrit_response=response,
+        expected_output='')
+
+    # Testing configuration with on -1..+1.
+    response = {
+      "owner": {"email": "john@example.com"},
+      "labels": {"Code-Review": {
+        u'all': [
+          {
+            u'email': u'ben@example.com',
+            u'value': 0
+          },
+        ],
+        u'approved': {u'email': u'ben@example.org'},
+        u'default_value': 0,
+        u'values': {u' 0': u'No score',
+                    u'+1': u'Looks good to me',
+                    u'-1': u"I would prefer that you didn't submit this"}
+      }},
+    }
+    self.AssertOwnersWorks(
+        approvers=set(),
+        reviewers=set(["ben@example.com"]),
+        gerrit_response=response,
+        is_committing=True,
+        expected_output=
+            'Missing LGTM from someone other than john@example.com\n')
+
   def testCannedCheckOwners_NotApproved(self):
     response = {
       "owner_email": "john@example.com",