From 288e358f3f720c0c1cd1c61a681d7d5bdfe7c5d7 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Mon, 4 Nov 2019 21:14:09 +0000 Subject: [PATCH] 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 Reviewed-by: Anthony Polito --- owners_finder.py | 7 ++++--- tests/owners_finder_test.py | 18 ++++++------------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/owners_finder.py b/owners_finder.py index 1d4e542085..61869347ea 100644 --- a/owners_finder.py +++ b/owners_finder.py @@ -161,9 +161,10 @@ class OwnersFinder(object): self.selected_owners = set() self.deselected_owners = set() - # Initialize owners queue, sort it by the score - self.owners_queue = list(sorted(self.owners_to_files.keys(), - key=lambda owner: self.owners_score[owner])) + # Initialize owners queue, sort it by the score and name + self.owners_queue = sorted( + self.owners_to_files.keys(), + key=lambda owner: (self.owners_score[owner], owner)) self.find_mandatory_owners() def select_owner(self, owner, findMandatoryOwners=True): diff --git a/tests/owners_finder_test.py b/tests/owners_finder_test.py index 60a8b495a4..a520d9a2dc 100755 --- a/tests/owners_finder_test.py +++ b/tests/owners_finder_test.py @@ -134,10 +134,6 @@ class OwnersFinderTests(_BaseTestCase): def test_constructor(self): 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): files = [ 'chrome/browser/defaults.h', # owned by brett @@ -156,11 +152,9 @@ class OwnersFinderTests(_BaseTestCase): def test_reset(self): finder = self.defaultFinder() - i = 0 - while i < 2: - i += 1 - self.assertCountEqual(finder.owners_queue, - [brett, john, darin, peter, ken, ben, tom]) + for _ in range(2): + self.assertEqual(finder.owners_queue, + [brett, darin, john, peter, ken, ben, tom]) self.assertEqual(finder.unreviewed_files, { 'base/vlog.h', 'chrome/browser/defaults.h', @@ -196,7 +190,7 @@ class OwnersFinderTests(_BaseTestCase): finder = self.defaultFinder() 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.deselected_owners, {john}) self.assertEqual(finder.reviewed_by, {'content/bar/foo.cc': darin, @@ -208,7 +202,7 @@ class OwnersFinderTests(_BaseTestCase): finder = self.defaultFinder() 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.deselected_owners, {ben}) self.assertEqual(finder.reviewed_by, @@ -224,7 +218,7 @@ class OwnersFinderTests(_BaseTestCase): def test_deselect(self): finder = self.defaultFinder() 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.deselected_owners, {john}) self.assertEqual(finder.reviewed_by, {'content/bar/foo.cc': darin,