Commit 93248c5c authored by Aaron Gable's avatar Aaron Gable Committed by Commit Bot

owners: handle CLs which only have partial OWNERS

If every file in a change has OWNERS, then the code would find
them fine. Similarly, if no files has OWNERS, then this code would
return an empty set just fine.

But if some files had OWNERS while others didn't, it would crash
when it tried to find an OWNER for file 'foo' while all the possible
OWNERS only provided coverage for file 'bar'.

This code purges the list of possible OWNERS as they become useless
for providing additional coverage, and returns whatever set we have
accumulated so far when the set of possible OWNERS becomes empty.

R=iannucci@chromium.org

Bug: 715062
Change-Id: I408601bd89379381db1cc7df56beed97ab3c27e6
Reviewed-on: https://chromium-review.googlesource.com/506239
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: 's avatarRobbie Iannucci <iannucci@chromium.org>
parent 8b5d2302
......@@ -418,19 +418,28 @@ class Database(object):
dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files)
all_possible_owners = self.all_possible_owners(dirs_remaining, author)
suggested_owners = set()
if len(all_possible_owners) == 0:
return suggested_owners
while dirs_remaining:
while dirs_remaining and all_possible_owners:
owner = self.lowest_cost_owner(all_possible_owners, dirs_remaining)
suggested_owners.add(owner)
dirs_to_remove = set(el[0] for el in all_possible_owners[owner])
dirs_remaining -= dirs_to_remove
# Now that we've used `owner` and covered all their dirs, remove them
# from consideration.
del all_possible_owners[owner]
for o, dirs in all_possible_owners.items():
new_dirs = [(d, dist) for (d, dist) in dirs if d not in dirs_to_remove]
if not new_dirs:
del all_possible_owners[o]
else:
all_possible_owners[o] = new_dirs
return suggested_owners
def all_possible_owners(self, dirs, author):
"""Returns a list of (potential owner, distance-from-dir) tuples; a
distance of 1 is the lowest/closest possible distance (which makes the
subsequent math easier)."""
"""Returns a dict of {potential owner: (dir, distance)} mappings.
A distance of 1 is the lowest/closest possible distance (which makes the
subsequent math easier).
"""
all_possible_owners = {}
for current_dir in dirs:
dirname = current_dir
......
......@@ -467,6 +467,13 @@ class ReviewersForTest(_BaseTestCase):
self.assert_reviewers_for(['chrome/gpu/gpu_channel.h'],
[[ben], [brett]], author=ken)
def test_reviewers_for__ignores_unowned_files(self):
# Clear the root OWNERS file.
self.files['/OWNERS'] = ''
self.assert_reviewers_for(['base/vlog.h', 'chrome/browser/deafults/h'],
[[brett]])
def test_reviewers_file_includes__absolute(self):
self.assert_reviewers_for(['content/qux/foo.cc'],
[[peter], [brett], [john], [darin]])
......
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