Add a limit to the update_policy_state_for_assignment loop to prevent

infinite loops, e.g. when a team has only a single reviewer.
 - Legacy-Id: 17087
This commit is contained in:
Sasha Romijn 2019-11-21 12:37:02 +00:00
parent 084978e105
commit 6bf7d15b70
2 changed files with 14 additions and 2 deletions

View file

@ -92,10 +92,13 @@ class AbstractReviewerQueuePolicy:
# Loop through the list until finding the first person with skip_next=0,
# who is not the current assignee. Anyone with skip_next>0 encountered before
# has their skip_next decreased.
# has their skip_next decreased. There is a cap on the number of loops, which can
# be hit e.g. if there is only a single reviewer, and the current assignee is excluded
# from being set as NextReviewerInTeam.
current_idx = 0
max_loops = sum([self._reviewer_settings_for(r).skip_next for r in rotation_list]) + len(rotation_list)
if in_order_assignment:
while True:
while current_idx <= max_loops:
current_idx_person = reviewer_at_index(current_idx)
settings = self._reviewer_settings_for(current_idx_person)
if settings.skip_next > 0:

View file

@ -198,6 +198,15 @@ class RotateAlphabeticallyReviewerAndGenericQueuePolicyTest(TestCase):
self.assertEqual(get_skip_next(reviewers[3]), 1)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Leave only a single reviewer remaining, which should not trigger an infinite loop.
# The deletion also causes NextReviewerInTeam to be deleted.
[reviewer.delete() for reviewer in reviewers[1:]]
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=False)
# No NextReviewerInTeam should be created, the only possible next is the excluded assignee.
self.assertFalse(NextReviewerInTeam.objects.filter(team=team))
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
def test_return_reviewer_to_top_rotation(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
list_email="rotationteam@ietf.org",