Do not count multiple reviews from the same user (#2)

* Do not count multiple reviews from the same user * `black check_labels.py` * Manually reformat to make pylint happy
RIOT-OS · Jun 5, 2024 · 153cc61 · 153cc61
1 parent ea0ec58
commit 153cc61
Showing 1 changed file with 59 additions and 19 deletions.
diff --git a/check_labels.py b/check_labels.py
@@ -175,14 +175,14 @@ def parse_condition(condition):
 
 def check_review_approvals(pull, condition, missing_approvals_label):
     condition[1] = int(condition[1])
-    approvals = 0
+    approving_reviewers = set()
     for review in pull.get_reviews():
         if review.state == "APPROVED":
-            approvals += 1
-            if approvals > condition[1]:
+            approving_reviewers.add(review.user.login)
+            if len(approving_reviewers) > condition[1]:
                 print(
-                    f"PR#{pull} has {approvals}/{condition[1] + 1} approvals,"
-                    f" removing label '{missing_approvals_label}'"
+                    f"PR#{pull} has {len(approving_reviewers)}/{condition[1] + 1}"
+                    f" approving reviewers, removing label '{missing_approvals_label}'"
                 )
                 if missing_approvals_label:
                     try:
@@ -197,8 +197,8 @@ def check_review_approvals(pull, condition, missing_approvals_label):
 
     if missing_approvals_label and condition[1] > 0:
         print(
-            f"PR#{pull} has only {approvals}/{condition[1] + 1} approvals,"
-            f" setting label '{missing_approvals_label}'"
+            f"PR#{pull} has only {len(approving_reviewers)}/{condition[1] + 1}"
+            f" approving reviewers, setting label '{missing_approvals_label}'"
         )
         pull.add_to_labels(missing_approvals_label)
 
@@ -364,9 +364,15 @@ def name(self):
 
     # pylint: disable=too-few-public-methods
     class MockReview:
-        def __init__(self, state):
+        def __init__(self, state, reviewer):
             self._state = state
 
+            class MockUser:
+                def __init__(self, login):
+                    self.login = login
+
+            self.user = MockUser(reviewer)
+
         @property
         def state(self):
             return self._state
@@ -403,43 +409,77 @@ def get_reviews(self):
             (
                 ["review.approvals", 2],
                 None,
-                ["APPROVED", "APPROVED", "APPROVED"],
+                [("APPROVED", "a"), ("APPROVED", "b"), ("APPROVED", "c")],
                 "",
                 True,
             ),
-            (["review.approvals", 2], None, ["APPROVED", "APPROVED"], "", False),
-            (["review.approvals", 1], None, ["APPROVED", "APPROVED"], "", True),
-            (["review.approvals", 1], None, ["COMMENT", "APPROVED"], "", False),
-            (["review.approvals", 1], None, ["COMMENT"], "", False),
-            (["review.approvals", 1], None, ["APPROVED"], "DON'T MERGE", False),
-            (["review.approvals", 1], ["FOOBAR"], ["APPROVED"], "DON'T MERGE", False),
+            (
+                ["review.approvals", 2],
+                None,
+                [("APPROVED", "a"), ("APPROVED", "b")],
+                "",
+                False,
+            ),
             (
                 ["review.approvals", 1],
                 None,
-                ["APPROVED", "APPROVED"],
+                [("APPROVED", "a"), ("APPROVED", "b")],
+                "",
+                True,
+            ),
+            (
+                ["review.approvals", 1],
+                None,
+                [("COMMENT", "a"), ("APPROVED", "b")],
+                "",
+                False,
+            ),
+            (["review.approvals", 1], None, [("COMMENT", "a")], "", False),
+            (["review.approvals", 1], None, [("APPROVED", "a")], "DON'T MERGE", False),
+            (
+                ["review.approvals", 1],
+                ["FOOBAR"],
+                [("APPROVED", "a")],
+                "DON'T MERGE",
+                False,
+            ),
+            (
+                ["review.approvals", 1],
+                None,
+                [("APPROVED", "a"), ("APPROVED", "b")],
                 "DON'T MERGE",
                 True,
             ),
             (
                 ["review.approvals", 1],
                 ["FOOBAR"],
-                ["APPROVED", "APPROVED"],
+                [("APPROVED", "a"), ("APPROVED", "b")],
                 "DON'T MERGE",
                 True,
             ),
             (
                 ["review.approvals", 1],
                 ["DON'T MERGE", "FOOBAR"],
-                ["APPROVED", "APPROVED"],
+                [("APPROVED", "a"), ("APPROVED", "b")],
                 "DON'T MERGE",
                 True,
             ),
+            (
+                ["review.approvals", 2],
+                None,
+                [("APPROVED", "a"), ("APPROVED", "a")],
+                "DON'T MERGE",
+                False,
+            ),
         ],
     )
     def test_check_review_approvals(
         value, labels, reviews, missing_approvals_label, exp
     ):
-        pull = MockPull(labels=labels, reviews=[MockReview(state) for state in reviews])
+        pull = MockPull(
+            labels=labels,
+            reviews=[MockReview(state, reviewer) for (state, reviewer) in reviews],
+        )
         assert check_review_approvals(pull, value, missing_approvals_label) == exp
         if not exp and missing_approvals_label:
             assert missing_approvals_label in pull.labels