owners_finder: Fix flaky tests.

Sorting owners by score is non-deterministic, so sort by
name too.

Bug: 1009814
Change-Id: I93a9370a5852634b4bbb2768b9ebbb5b866348e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1891665
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
changes/65/1891665/5
Edward Lemur 6 years ago committed by Commit Bot
parent 94d6f48ba1
commit 288e358f3f

@ -161,9 +161,10 @@ class OwnersFinder(object):
self.selected_owners = set() self.selected_owners = set()
self.deselected_owners = set() self.deselected_owners = set()
# Initialize owners queue, sort it by the score # Initialize owners queue, sort it by the score and name
self.owners_queue = list(sorted(self.owners_to_files.keys(), self.owners_queue = sorted(
key=lambda owner: self.owners_score[owner])) self.owners_to_files.keys(),
key=lambda owner: (self.owners_score[owner], owner))
self.find_mandatory_owners() self.find_mandatory_owners()
def select_owner(self, owner, findMandatoryOwners=True): def select_owner(self, owner, findMandatoryOwners=True):

@ -134,10 +134,6 @@ class OwnersFinderTests(_BaseTestCase):
def test_constructor(self): def test_constructor(self):
self.assertNotEqual(self.defaultFinder(), None) self.assertNotEqual(self.defaultFinder(), None)
# A surrogate for the python 3 assertCountEqual() method.
def assertCountEqual(self, a, b):
self.assertEqual(sorted(a), sorted(b))
def test_skip_files_owned_by_reviewers(self): def test_skip_files_owned_by_reviewers(self):
files = [ files = [
'chrome/browser/defaults.h', # owned by brett 'chrome/browser/defaults.h', # owned by brett
@ -156,11 +152,9 @@ class OwnersFinderTests(_BaseTestCase):
def test_reset(self): def test_reset(self):
finder = self.defaultFinder() finder = self.defaultFinder()
i = 0 for _ in range(2):
while i < 2: self.assertEqual(finder.owners_queue,
i += 1 [brett, darin, john, peter, ken, ben, tom])
self.assertCountEqual(finder.owners_queue,
[brett, john, darin, peter, ken, ben, tom])
self.assertEqual(finder.unreviewed_files, { self.assertEqual(finder.unreviewed_files, {
'base/vlog.h', 'base/vlog.h',
'chrome/browser/defaults.h', 'chrome/browser/defaults.h',
@ -196,7 +190,7 @@ class OwnersFinderTests(_BaseTestCase):
finder = self.defaultFinder() finder = self.defaultFinder()
finder.select_owner(darin) finder.select_owner(darin)
self.assertCountEqual(finder.owners_queue, [brett, peter, ken, ben, tom]) self.assertEqual(finder.owners_queue, [brett, peter, ken, ben, tom])
self.assertEqual(finder.selected_owners, {darin}) self.assertEqual(finder.selected_owners, {darin})
self.assertEqual(finder.deselected_owners, {john}) self.assertEqual(finder.deselected_owners, {john})
self.assertEqual(finder.reviewed_by, {'content/bar/foo.cc': darin, self.assertEqual(finder.reviewed_by, {'content/bar/foo.cc': darin,
@ -208,7 +202,7 @@ class OwnersFinderTests(_BaseTestCase):
finder = self.defaultFinder() finder = self.defaultFinder()
finder.select_owner(brett) finder.select_owner(brett)
self.assertCountEqual(finder.owners_queue, [john, darin, peter, ken, tom]) self.assertEqual(finder.owners_queue, [darin, john, peter, ken, tom])
self.assertEqual(finder.selected_owners, {brett}) self.assertEqual(finder.selected_owners, {brett})
self.assertEqual(finder.deselected_owners, {ben}) self.assertEqual(finder.deselected_owners, {ben})
self.assertEqual(finder.reviewed_by, self.assertEqual(finder.reviewed_by,
@ -224,7 +218,7 @@ class OwnersFinderTests(_BaseTestCase):
def test_deselect(self): def test_deselect(self):
finder = self.defaultFinder() finder = self.defaultFinder()
finder.deselect_owner(john) finder.deselect_owner(john)
self.assertCountEqual(finder.owners_queue, [brett, peter, ken, ben, tom]) self.assertEqual(finder.owners_queue, [brett, peter, ken, ben, tom])
self.assertEqual(finder.selected_owners, {darin}) self.assertEqual(finder.selected_owners, {darin})
self.assertEqual(finder.deselected_owners, {john}) self.assertEqual(finder.deselected_owners, {john})
self.assertEqual(finder.reviewed_by, {'content/bar/foo.cc': darin, self.assertEqual(finder.reviewed_by, {'content/bar/foo.cc': darin,

Loading…
Cancel
Save