Commit ca45aff3 authored by Edward Lesmes's avatar Edward Lesmes Committed by LUCI CQ

[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 <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: 's avatarJosip Sokcevic <sokcevic@google.com>
parent 91cf42a0
...@@ -38,7 +38,6 @@ def _owner_combinations(owners, num_owners): ...@@ -38,7 +38,6 @@ def _owner_combinations(owners, num_owners):
return reversed(list(itertools.combinations(reversed(owners), num_owners))) return reversed(list(itertools.combinations(reversed(owners), num_owners)))
class InvalidOwnersConfig(Exception): class InvalidOwnersConfig(Exception):
pass pass
...@@ -124,15 +123,16 @@ class OwnersClient(object): ...@@ -124,15 +123,16 @@ class OwnersClient(object):
# Select the minimum number of owners that can approve all paths. # Select the minimum number of owners that can approve all paths.
# We start at 2 to avoid sending all changes that require multiple reviewers # We start at 2 to avoid sending all changes that require multiple reviewers
# to top-level owners. # to top-level owners.
num_owners = 2 if len(owners) < 2:
while True: return owners
for num_owners in range(2, len(owners)):
# Iterate all combinations of `num_owners` by decreasing score, and select # Iterate all combinations of `num_owners` by decreasing score, and select
# the first one that covers all paths. # the first one that covers all paths.
for selected in _owner_combinations(owners, num_owners): for selected in _owner_combinations(owners, num_owners):
covered = set.union(*(paths_by_owner[o] for o in selected)) covered = set.union(*(paths_by_owner[o] for o in selected))
if len(covered) == len(paths): if len(covered) == len(paths):
return selected return selected
num_owners += 1
class DepotToolsClient(OwnersClient): class DepotToolsClient(OwnersClient):
......
...@@ -166,6 +166,11 @@ class OwnersClientTest(unittest.TestCase): ...@@ -166,6 +166,11 @@ class OwnersClientTest(unittest.TestCase):
(emily, dave)]) (emily, dave)])
def testSuggestOwners(self): 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.client.owners_by_path = {'abcd': [alice, bob, chris, dave]}
self.assertEqual( self.assertEqual(
self.client.SuggestOwners('project', 'branch', ['abcd']), self.client.SuggestOwners('project', 'branch', ['abcd']),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment