Merge pull request #424 from Mark-Simulacrum/recursion-not-an-error

Avoid an assertion error on double-adding a group

Author: Mark-Simulacrum

highfive/newpr.py

@@ -343,10 +343,10 @@ def pick_reviewer(self, groups, potential, exclude):
                     reviewers.append(username)
             elif p in groups:
                 # avoid infinite loops
-                assert p not in seen, "group %s refers to itself" % p
-                seen.add(p)
-                # we allow groups in groups, so they need to be queued to be resolved
-                potential.extend(groups[p])
+                if p not in seen:
+                    seen.add(p)
+                    # we allow groups in groups, so they need to be queued to be resolved
+                    potential.extend(groups[p])
 
         if reviewers:
             random.seed()

highfive/tests/test_newpr.py

@@ -1160,10 +1160,10 @@ def test_circular_groups(self):
         handler = HighfiveHandlerMock(
             Payload({}), repo_config=self.fakes['config']['circular_groups']
         ).handler
-        with pytest.raises(AssertionError):
-            handler.choose_reviewer(
-                'rust', 'rust-lang', self.fakes['diff']['normal'], 'fooauthor'
-            )
+        chosen_reviewers = handler.choose_reviewer(
+            'rust', 'rust-lang', self.fakes['diff']['normal'], 'fooauthor'
+        )
+        assert chosen_reviewers is None
 
     def test_nested_groups(self):
         """Test choosing a reviewer from group with nested groups.