Эх сурвалжийг харах

owners_finder: Use code-owners plugin if available.

Change-Id: I94d7b3cdc17651a6fe5dafa261c58f46f37b8439
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2693919
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Edward Lesmes 4 жил өмнө
parent
commit
5cd75478dd
3 өөрчлөгдсөн 44 нэмэгдсэн , 103 устгасан
  1. 1 10
      git_cl.py
  2. 6 16
      owners_finder.py
  3. 37 77
      tests/owners_finder_test.py

+ 1 - 10
git_cl.py

@@ -4831,21 +4831,12 @@ def CMDowners(parser, args):
     print('\n'.join(owners))
     return 0
 
-  root = settings.GetRoot()
-  owner_files = [f for f in affected_files if 'OWNERS' in os.path.basename(f)]
-  original_owner_files = {
-      f: scm.GIT.GetOldContents(root, f, base_branch).splitlines()
-      for f in owner_files}
-
   return owners_finder.OwnersFinder(
       affected_files,
-      root,
       author,
       [] if options.ignore_current else cl.GetReviewers(),
-      fopen=open,
-      os_path=os.path,
+      cl.owners_client,
       disable_color=options.no_color,
-      override_files=original_owner_files,
       ignore_author=options.ignore_self).run()
 
 

+ 6 - 16
owners_finder.py

@@ -8,7 +8,6 @@ from __future__ import print_function
 
 import os
 import copy
-import owners_client
 
 
 import git_common
@@ -28,11 +27,9 @@ class OwnersFinder(object):
 
   indentation = 0
 
-  def __init__(self, files, local_root, author, reviewers,
-               fopen, os_path,
+  def __init__(self, files, author, reviewers, owners_client,
                email_postfix='@chromium.org',
                disable_color=False,
-               override_files=None,
                ignore_author=False):
     self.email_postfix = email_postfix
 
@@ -42,8 +39,6 @@ class OwnersFinder(object):
       self.COLOR_GREY = ''
       self.COLOR_RESET = ''
 
-    self.os_path = os_path
-
     self.author = author
 
     filtered_files = files
@@ -53,23 +48,18 @@ class OwnersFinder(object):
       reviewers.append(author)
 
     # Eliminate files that existing reviewers can review.
-    self.client = owners_client.DepotToolsClient(
-      root=local_root,
-      branch=git_common.current_branch(),
-      fopen=fopen,
-      os_path=os_path)
-
-    approval_status = self.client.GetFilesApprovalStatus(
+    self.owners_client = owners_client
+    approval_status = self.owners_client.GetFilesApprovalStatus(
       filtered_files, reviewers, [])
     filtered_files = [
       f for f in filtered_files
-      if approval_status[f] != owners_client.OwnersClient.APPROVED]
+      if approval_status[f] != self.owners_client.APPROVED]
 
     # If some files are eliminated.
     if len(filtered_files) != len(files):
       files = filtered_files
 
-    self.files_to_owners = self.client.BatchListOwners(files)
+    self.files_to_owners = self.owners_client.BatchListOwners(files)
 
     self.owners_to_files = {}
     self._map_owners_to_files()
@@ -151,7 +141,7 @@ class OwnersFinder(object):
 
     # Randomize owners' names so that if many reviewers have identical scores
     # they will be randomly ordered to avoid bias.
-    owners = self.client.ScoreOwners(self.files_to_owners.keys())
+    owners = list(self.owners_client.ScoreOwners(self.files_to_owners.keys()))
     if self.author and self.author in owners:
       owners.remove(self.author)
     self.owners_queue = owners

+ 37 - 77
tests/owners_finder_test.py

@@ -33,12 +33,6 @@ tom = 'tom@example.com'
 nonowner = 'nonowner@example.com'
 
 
-def _get_score_owners_darin_variant():
-  return [brett, darin, john, peter, ken, ben, tom]
-
-
-def _get_score_owners_john_variant():
-  return [brett, john, darin, peter, ken, ben, tom]
 
 
 def owners_file(*email_addresses, **kwargs):
@@ -50,45 +44,36 @@ def owners_file(*email_addresses, **kwargs):
   return s + '\n'.join(email_addresses) + '\n'
 
 
-def test_repo():
-  return filesystem_mock.MockFileSystem(files={
-    '/DEPS': '',
-    '/OWNERS': owners_file(ken, peter, tom,
-                           comment='OWNERS_STATUS = build/OWNERS.status'),
-    '/build/OWNERS.status': '%s: bar' % jochen,
-    '/base/vlog.h': '',
-    '/chrome/OWNERS': owners_file(ben, brett),
-    '/chrome/browser/OWNERS': owners_file(brett),
-    '/chrome/browser/defaults.h': '',
-    '/chrome/gpu/OWNERS': owners_file(ken),
-    '/chrome/gpu/gpu_channel.h': '',
-    '/chrome/renderer/OWNERS': owners_file(peter),
-    '/chrome/renderer/gpu/gpu_channel_host.h': '',
-    '/chrome/renderer/safe_browsing/scorer.h': '',
-    '/content/OWNERS': owners_file(john, darin, comment='foo', noparent=True),
-    '/content/content.gyp': '',
-    '/content/bar/foo.cc': '',
-    '/content/baz/OWNERS': owners_file(brett),
-    '/content/baz/froboz.h': '',
-    '/content/baz/ugly.cc': '',
-    '/content/baz/ugly.h': '',
-    '/content/common/OWNERS': owners_file(jochen),
-    '/content/common/common.cc': '',
-    '/content/foo/OWNERS': owners_file(jochen, comment='foo'),
-    '/content/foo/foo.cc': '',
-    '/content/views/OWNERS': owners_file(ben, john,
-                                         owners_client.OwnersClient.EVERYONE,
-                                         noparent=True),
-    '/content/views/pie.h': '',
-  })
+class TestClient(owners_client.OwnersClient):
+  def __init__(self):
+    super(TestClient, self).__init__()
+    self.owners_by_path = {
+      'DEPS': [ken, peter, tom],
+      'base/vlog.h': [ken, peter, tom],
+      'chrome/browser/defaults.h': [brett, ben, ken, peter, tom],
+      'chrome/gpu/gpu_channel.h': [ken, ben, brett, ken, peter, tom],
+      'chrome/renderer/gpu/gpu_channel_host.h': [peter, ben, brett, ken, tom],
+      'chrome/renderer/safe_browsing/scorer.h': [peter, ben, brett, ken, tom],
+      'content/content.gyp': [john, darin],
+      'content/bar/foo.cc': [john, darin],
+      'content/baz/froboz.h': [brett, john, darin],
+      'content/baz/ugly.cc': [brett, john, darin],
+      'content/baz/ugly.h': [brett, john, darin],
+      'content/common/common.cc': [jochen, john, darin],
+      'content/foo/foo.cc': [jochen, john, darin],
+      'content/views/pie.h': [ben, john, self.EVERYONE],
+    }
+
+  def ListOwners(self, path):
+    path = path.replace(os.sep, '/')
+    return self.owners_by_path[path]
 
 
 class OutputInterceptedOwnersFinder(owners_finder.OwnersFinder):
-  def __init__(self, files, local_root, author, reviewers,
-               fopen, os_path, disable_color=False):
+  def __init__(
+      self, files, author, reviewers, client, disable_color=False):
     super(OutputInterceptedOwnersFinder, self).__init__(
-      files, local_root, author, reviewers, fopen, os_path,
-      disable_color=disable_color)
+      files, author, reviewers, client, disable_color=disable_color)
     self.output = []
     self.indentation_stack = []
 
@@ -123,24 +108,10 @@ class _BaseTestCase(unittest.TestCase):
     'content/views/pie.h'
   ]
 
-  def setUp(self):
-    self.repo = test_repo()
-    self.root = '/'
-    self.fopen = self.repo.open_for_reading
-    mock.patch('owners_client.DepotToolsClient._GetOriginalOwnersFiles',
-      return_value={}).start()
-    self.addCleanup(mock.patch.stopall)
-
   def ownersFinder(self, files, author=nonowner, reviewers=None):
     reviewers = reviewers or []
-    finder = OutputInterceptedOwnersFinder(files,
-                                           self.root,
-                                           author,
-                                           reviewers,
-                                           fopen=self.fopen,
-                                           os_path=self.repo,
-                                           disable_color=True)
-    return finder
+    return OutputInterceptedOwnersFinder(
+        files, author, reviewers, TestClient(), disable_color=True)
 
   def defaultFinder(self):
     return self.ownersFinder(self.default_files)
@@ -177,14 +148,9 @@ class OwnersFinderTests(_BaseTestCase):
     finder = self.ownersFinder(files, reviewers=[brett])
     self.assertEqual(finder.unreviewed_files, {'content/bar/foo.cc'})
 
-  def test_reset(self):
-    mock.patch('owners_client.DepotToolsClient.ScoreOwners',
-      side_effect=[
-        _get_score_owners_darin_variant(),
-        _get_score_owners_darin_variant(),
-        _get_score_owners_darin_variant()
-      ]).start()
-
+  @mock.patch('owners_client.OwnersClient.ScoreOwners')
+  def test_reset(self, mockScoreOwners):
+    mockScoreOwners.return_value = [brett, darin, john, peter, ken, ben, tom]
     finder = self.defaultFinder()
     for _ in range(2):
       expected = [brett, darin, john, peter, ken, ben, tom]
@@ -209,14 +175,9 @@ class OwnersFinderTests(_BaseTestCase):
       finder.reset()
       finder.resetText()
 
-  def test_select(self):
-    mock.patch('owners_client.DepotToolsClient.ScoreOwners',
-      side_effect=[
-        _get_score_owners_darin_variant(),
-        _get_score_owners_john_variant(),
-        _get_score_owners_darin_variant()
-      ]).start()
-
+  @mock.patch('owners_client.OwnersClient.ScoreOwners')
+  def test_select(self, mockScoreOwners):
+    mockScoreOwners.return_value = [brett, darin, john, peter, ken, ben, tom]
     finder = self.defaultFinder()
     finder.select_owner(john)
     self.assertEqual(finder.owners_queue, [brett, peter, ken, ben, tom])
@@ -257,10 +218,9 @@ class OwnersFinderTests(_BaseTestCase):
     self.assertEqual(finder.output,
                      ['Selected: ' + brett, 'Deselected: ' + ben])
 
-  def test_deselect(self):
-    mock.patch('owners_client.DepotToolsClient.ScoreOwners',
-      return_value=_get_score_owners_darin_variant()).start()
-
+  @mock.patch('owners_client.OwnersClient.ScoreOwners')
+  def test_deselect(self, mockScoreOwners):
+    mockScoreOwners.return_value = [brett, darin, john, peter, ken, ben, tom]
     finder = self.defaultFinder()
     finder.deselect_owner(john)
     self.assertEqual(finder.owners_queue, [brett, peter, ken, ben, tom])