From ca45aff39147048011e42a5d459f04b17f47b3e0 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Thu, 3 Dec 2020 23:11:01 +0000 Subject: [PATCH] [owners] Fix SuggestOwners when there is only one owner. If there are less than two owners for the given paths, current implementation will get stuck in infinite loop. Change it to return any possible owner. Change-Id: I75f18e0a00057c58d227dac23dc8572f1fba23f5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2572802 Auto-Submit: Edward Lesmes Commit-Queue: Josip Sokcevic Reviewed-by: Josip Sokcevic --- owners_client.py | 8 ++++---- tests/owners_client_test.py | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/owners_client.py b/owners_client.py index 7bd6ec88d..01b3f181c 100644 --- a/owners_client.py +++ b/owners_client.py @@ -38,7 +38,6 @@ def _owner_combinations(owners, num_owners): return reversed(list(itertools.combinations(reversed(owners), num_owners))) - class InvalidOwnersConfig(Exception): pass @@ -124,15 +123,16 @@ class OwnersClient(object): # Select the minimum number of owners that can approve all paths. # We start at 2 to avoid sending all changes that require multiple reviewers # to top-level owners. - num_owners = 2 - while True: + if len(owners) < 2: + return owners + + for num_owners in range(2, len(owners)): # Iterate all combinations of `num_owners` by decreasing score, and select # the first one that covers all paths. for selected in _owner_combinations(owners, num_owners): covered = set.union(*(paths_by_owner[o] for o in selected)) if len(covered) == len(paths): return selected - num_owners += 1 class DepotToolsClient(OwnersClient): diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py index fc58c90c5..489bb718d 100644 --- a/tests/owners_client_test.py +++ b/tests/owners_client_test.py @@ -166,6 +166,11 @@ class OwnersClientTest(unittest.TestCase): (emily, dave)]) def testSuggestOwners(self): + self.client.owners_by_path = {'a': [alice]} + self.assertEqual( + self.client.SuggestOwners('project', 'branch', ['a']), + [alice]) + self.client.owners_by_path = {'abcd': [alice, bob, chris, dave]} self.assertEqual( self.client.SuggestOwners('project', 'branch', ['abcd']),