Randomize results of git cl owners

git cl owners orders owners by score with alphabetization being the tie
breaker. This leads to some owners being suggested far more often than
others.

Adding a tiny amount of randomization to the scoring leads to an even
distribution of equally qualified reviewers. Less qualified reviewers
will still be sorted into distinct buckets - the randomness is too small
to do anything except break ties.

The tests were updated so that they can tolerate the randomness, but
only for breaking ties.

Bug: 1024083
Change-Id: If7d39d1b3bbd980b80b46ab3f62c65215309bdc8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1913642
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

owners.py

@@ -2,7 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""A database of OWNERS files.
+r"""A database of OWNERS files.
 
 OWNERS files indicate who is allowed to approve changes in a specific directory
 (or who is allowed to make changes without needing approval of another OWNER).
@@ -62,6 +62,12 @@ import fnmatch
 import random
 import re
 
+try:
+  # This fallback applies for all versions of Python before 3.3
+  import collections.abc as collections_abc
+except ImportError:
+  import collections as collections_abc
+
 
 # If this is present by itself on a line, this means that everyone can review.
 EVERYONE = '*'
@@ -80,9 +86,9 @@ def _assert_is_collection(obj):
   assert not isinstance(obj, str)
   # Module 'collections' has no 'Iterable' member
   # pylint: disable=no-member
-  if hasattr(collections, 'Iterable') and hasattr(collections, 'Sized'):
-    assert (isinstance(obj, collections.Iterable) and
-            isinstance(obj, collections.Sized))
+  if hasattr(collections_abc, 'Iterable') and hasattr(collections_abc, 'Sized'):
+    assert (isinstance(obj, collections_abc.Iterable) and
+            isinstance(obj, collections_abc.Sized))
 
 
 class SyntaxErrorInOwnersFile(Exception):

owners_finder.py

@@ -9,6 +9,7 @@ from __future__ import print_function
 import os
 import copy
 import owners as owners_module
+import random
 
 
 def first(iterable):
@@ -161,10 +162,12 @@ class OwnersFinder(object):
     self.selected_owners = set()
     self.deselected_owners = set()
 
-    # 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))
+    # Randomize owners' names so that if many reviewers have identical scores
+    # they will be randomly ordered to avoid bias.
+    owners = list(self.owners_to_files.keys())
+    random.shuffle(owners)
+    self.owners_queue = sorted(owners,
+                               key=lambda owner: self.owners_score[owner])
     self.find_mandatory_owners()
 
   def select_owner(self, owner, findMandatoryOwners=True):

tests/owners_finder_test.py

@@ -153,8 +153,12 @@ class OwnersFinderTests(_BaseTestCase):
   def test_reset(self):
     finder = self.defaultFinder()
     for _ in range(2):
-      self.assertEqual(finder.owners_queue,
-                       [brett, darin, john, peter, ken, ben, tom])
+      expected = [brett, darin, john, peter, ken, ben, tom]
+      # darin and john have equal cost, the others have distinct costs.
+      # If the owners_queue has those two swapped then swap them in expected.
+      if finder.owners_queue[1] != expected[1]:
+        expected[1], expected[2] = expected[2], expected[1]
+      self.assertEqual(finder.owners_queue, expected)
       self.assertEqual(finder.unreviewed_files, {
           'base/vlog.h',
           'chrome/browser/defaults.h',
@@ -202,7 +206,13 @@ class OwnersFinderTests(_BaseTestCase):
 
     finder = self.defaultFinder()
     finder.select_owner(brett)
-    self.assertEqual(finder.owners_queue, [darin, john, peter, ken, tom])
+    expected = [darin, john, peter, ken, tom]
+    # darin and john have equal cost, the others have distinct costs.
+    # If the owners_queue has those two swapped then swap them in expected.
+    if finder.owners_queue[0] == john:
+      expected[0], expected[1] = expected[1], expected[0]
+
+    self.assertEqual(finder.owners_queue, expected)
     self.assertEqual(finder.selected_owners, {brett})
     self.assertEqual(finder.deselected_owners, {ben})
     self.assertEqual(finder.reviewed_by,