Add a way to require approval from owners other than the author.

tests/owners_unittest.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Unit tests for owners.py."""
 
 import os
 import sys
 import unittest
@@ skipping 196 matching lines @@
   def test_per_file_glob_across_dirs_not_allowed(self):
     self.files['/OWNERS'] = 'per-file content/*=john@example.org\n'
     self.assertRaises(owners.SyntaxErrorInOwnersFile,
         self.db().files_not_covered_by, ['DEPS'], [brett])
 
   def assert_syntax_error(self, owners_file_contents):
     db = self.db()
     self.files['/foo/OWNERS'] = owners_file_contents
     self.files['/foo/DEPS'] = ''
     try:
-      db.reviewers_for(['foo/DEPS'])
+      db.reviewers_for(['foo/DEPS'], None)
       self.fail()  # pragma: no cover
     except owners.SyntaxErrorInOwnersFile, e:
       self.assertTrue(str(e).startswith('/foo/OWNERS:1'))
 
   def test_syntax_error__unknown_token(self):
     self.assert_syntax_error('{}\n')
 
   def test_syntax_error__unknown_set(self):
     self.assert_syntax_error('set myfatherisbillgates\n')
 
   def test_syntax_error__bad_email(self):
     self.assert_syntax_error('ben\n')
 
 
 class ReviewersForTest(_BaseTestCase):
-  def assert_reviewers_for(self, files, *potential_suggested_reviewers):
+  def assert_reviewers_for(self, files, potential_suggested_reviewers,
+                           author=None):
     db = self.db()
-    suggested_reviewers = db.reviewers_for(set(files))
+    suggested_reviewers = db.reviewers_for(set(files), author)
     self.assertTrue(suggested_reviewers in
         [set(suggestion) for suggestion in potential_suggested_reviewers])
 
   def test_reviewers_for__basic_functionality(self):
     self.assert_reviewers_for(['chrome/gpu/gpu_channel.h'],
-                              [ken])
+                              [[ken]])
 
   def test_reviewers_for__set_noparent_works(self):
     self.assert_reviewers_for(['content/content.gyp'],
-                              [john],
-                              [darin])
+                              [[john],
+                               [darin]])
 
   def test_reviewers_for__valid_inputs(self):
     db = self.db()
 
     # Check that we're passed in a sequence that isn't a string.
-    self.assertRaises(AssertionError, db.reviewers_for, 'foo')
+    self.assertRaises(AssertionError, db.reviewers_for, 'foo', None)
     if hasattr(owners.collections, 'Iterable'):
       self.assertRaises(AssertionError, db.reviewers_for,
-                        (f for f in ['x', 'y']))
+                        (f for f in ['x', 'y']), None)
 
     # Check that the files are under the root.
     db.root = '/checkout'
-    self.assertRaises(AssertionError, db.reviewers_for, ['/OWNERS'])
+    self.assertRaises(AssertionError, db.reviewers_for, ['/OWNERS'], None)
 
   def test_reviewers_for__wildcard_dir(self):
-    self.assert_reviewers_for(['DEPS'], ['<anyone>'])
-    self.assert_reviewers_for(['DEPS', 'chrome/gpu/gpu_channel.h'], [ken])
+    self.assert_reviewers_for(['DEPS'], [['<anyone>']])
+    self.assert_reviewers_for(['DEPS', 'chrome/gpu/gpu_channel.h'], [[ken]])
 
   def test_reviewers_for__one_owner(self):
     self.assert_reviewers_for([
         'chrome/gpu/gpu_channel.h',
         'content/baz/froboz.h',
         'chrome/renderer/gpu/gpu_channel_host.h'],
-        [brett])
+        [[brett]])
 
   def test_reviewers_for__two_owners(self):
     self.assert_reviewers_for([
         'chrome/gpu/gpu_channel.h',
         'content/content.gyp',
         'content/baz/froboz.h',
         'content/views/pie.h'],
-        [ken, john])
+        [[ken, john]])
 
   def test_reviewers_for__all_files(self):
     self.assert_reviewers_for([
         'chrome/gpu/gpu_channel.h',
         'chrome/renderer/gpu/gpu_channel_host.h',
         'chrome/renderer/safe_browsing/scorer.h',
         'content/content.gyp',
         'content/bar/foo.cc',
         'content/baz/froboz.h',
         'content/views/pie.h'],
-        [peter, ken, john])
+        [[peter, ken, john]])
 
   def test_reviewers_for__per_file_owners_file(self):
     self.files['/content/baz/OWNERS'] = owners_file(lines=[
         'per-file ugly.*=tom@example.com'])
     self.assert_reviewers_for(['content/baz/OWNERS'],
-                              [john],
-                              [darin])
+                              [[john],
+                               [darin]])
 
   def test_reviewers_for__per_file(self):
     self.files['/content/baz/OWNERS'] = owners_file(lines=[
         'per-file ugly.*=tom@example.com'])
     self.assert_reviewers_for(['content/baz/ugly.cc'],
-                              [tom])
+                              [[tom]])
 
   def test_reviewers_for__two_nested_dirs(self):
     # The same owner is listed in two directories (one above the other)
     self.assert_reviewers_for(['chrome/browser/defaults.h'],
-                              [brett])
+                              [[brett]])
 
     # Here, although either ben or brett could review both files,
     # someone closer to the gpu_channel_host.h should also be suggested.
     # This also tests that we can handle two suggested reviewers
     # with overlapping sets of directories properly.
     self.files['/chrome/renderer/gpu/OWNERS'] = owners_file(ken)
     self.assert_reviewers_for(['chrome/OWNERS',
                                'chrome/renderer/gpu/gpu_channel_host.h'],
-                              [ben, ken],
-                              [brett, ken])
+                              [[ben, ken],
+                               [brett, ken]])
+
+  def test_reviewers_for__author_is_known(self):
+    # We should never suggest ken as a reviewer for his own changes.
+    self.assert_reviewers_for(['chrome/gpu/gpu_channel.h'],
+                              [[ben], [brett]], author=ken)
 
 
 class LowestCostOwnersTest(_BaseTestCase):
   # Keep the data in the test_lowest_cost_owner* methods as consistent with
   # test_repo() where possible to minimize confusion.
 
   def check(self, possible_owners, dirs, *possible_lowest_cost_owners):
     suggested_owner = owners.Database.lowest_cost_owner(possible_owners, dirs)
     self.assertTrue(suggested_owner in possible_lowest_cost_owners)
 
@@ skipping 58 matching lines @@
                       ('chrome/browser', 2)],
                 ken: [('chrome/gpu', 1)],
                 peter: [('chrome/renderer', 1)],
                 brett: [('chrome/browser', 1)]},
                ['chrome/gpu', 'chrome/renderer',
                 'chrome/browser'],
                ben)
 
 if __name__ == '__main__':
   unittest.main()