Browse Source

Changed naming convention

presubmit:gerrit_host/folder/to/repo:path/to/file/:CheckName
Example
presubmit:chromium-review.googlesource.com/chromium/src/:services/viz/

Bug: 1112667
Change-Id: I095a2c04e73c64d67fa1bb3dbf7e6dfd1d93fe26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2335873
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Saagar Sanghavi <saagarsanghavi@google.com>
Saagar Sanghavi 5 years ago
parent
commit
531d992f00
6 changed files with 36 additions and 15 deletions
  1. 1 0
      git_cl.py
  2. 21 9
      presubmit_support.py
  3. 6 4
      rdb_wrapper.py
  4. 5 0
      tests/git_cl_test.py
  5. 1 0
      tests/presubmit_unittest.py
  6. 2 2
      tests/rdb_wrapper_test.py

+ 1 - 0
git_cl.py

@@ -1290,6 +1290,7 @@ class Changelist(object):
         gclient_utils.FileWrite(description_file, description)
         args.extend(['--json_output', json_output])
         args.extend(['--description_file', description_file])
+        args.extend(['--gerrit_project', self._GetGerritProject()])
 
         start = time_time()
         cmd = ['vpython', PRESUBMIT_SUPPORT] + args

+ 21 - 9
presubmit_support.py

@@ -1497,8 +1497,8 @@ def DoPostUploadExecuter(change,
   return exit_code
 
 class PresubmitExecuter(object):
-  def __init__(self, change, committing, verbose,
-               gerrit_obj, dry_run=None, thread_pool=None, parallel=False):
+  def __init__(self, change, committing, verbose, gerrit_obj, dry_run=None,
+               thread_pool=None, parallel=False, gerrit_project=None):
     """
     Args:
       change: The Change object.
@@ -1516,6 +1516,7 @@ class PresubmitExecuter(object):
     self.more_cc = []
     self.thread_pool = thread_pool
     self.parallel = parallel
+    self.gerrit_project = gerrit_project
 
   def ExecPresubmitScript(self, script_text, presubmit_path):
     """Executes a single presubmit script.
@@ -1555,12 +1556,21 @@ class PresubmitExecuter(object):
 
     context['__args'] = (input_api, output_api)
 
-    # Get path of presubmit directory relative to repository root
+    # Get path of presubmit directory relative to repository root.
     # Always use forward slashes, so that path is same in *nix and Windows
     root = input_api.change.RepositoryRoot()
     rel_path = os.path.relpath(presubmit_dir, root)
     rel_path = rel_path.replace(os.path.sep, '/')
 
+    # Get the URL of git remote origin and use it to identify host and project
+    host = ''
+    if self.gerrit and self.gerrit.host:
+      host = self.gerrit.host
+    project = self.gerrit_project or ''
+
+    # Prefix for test names
+    prefix = 'presubmit:%s/%s:%s/' % (host, project, rel_path)
+
     # Perform all the desired presubmit checks.
     results = []
 
@@ -1575,7 +1585,7 @@ class PresubmitExecuter(object):
             continue
           logging.debug('Running %s in %s', function_name, presubmit_path)
           results.extend(
-              self._run_check_function(function_name, context, rel_path))
+              self._run_check_function(function_name, context, prefix))
           logging.debug('Running %s done.', function_name)
           self.more_cc.extend(output_api.more_cc)
 
@@ -1587,7 +1597,7 @@ class PresubmitExecuter(object):
         if function_name in context:
             logging.debug('Running %s in %s', function_name, presubmit_path)
             results.extend(
-                self._run_check_function(function_name, context, rel_path))
+                self._run_check_function(function_name, context, prefix))
             logging.debug('Running %s done.', function_name)
             self.more_cc.extend(output_api.more_cc)
 
@@ -1636,7 +1646,8 @@ def DoPresubmitChecks(change,
                       gerrit_obj,
                       dry_run=None,
                       parallel=False,
-                      json_output=None):
+                      json_output=None,
+                      gerrit_project=None):
   """Runs all presubmit checks that apply to the files in the change.
 
   This finds all PRESUBMIT.py files in directories enclosing the files in the
@@ -1679,7 +1690,7 @@ def DoPresubmitChecks(change,
     results = []
     thread_pool = ThreadPool()
     executer = PresubmitExecuter(change, committing, verbose, gerrit_obj,
-                                 dry_run, thread_pool, parallel)
+                                 dry_run, thread_pool, parallel, gerrit_project)
     if default_presubmit:
       if verbose:
         sys.stdout.write('Running default presubmit script.\n')
@@ -1923,7 +1934,7 @@ def main(argv=None):
                       help='List of files to be marked as modified when '
                       'executing presubmit or post-upload hooks. fnmatch '
                       'wildcards can also be used.')
-
+  parser.add_argument('--gerrit_project', help=argparse.SUPPRESS)
   options = parser.parse_args(argv)
 
   log_level = logging.ERROR
@@ -1956,7 +1967,8 @@ def main(argv=None):
           gerrit_obj,
           options.dry_run,
           options.parallel,
-          options.json_output)
+          options.json_output,
+          options.gerrit_project)
   except PresubmitFailure as e:
     print(e, file=sys.stderr)
     print('Maybe your depot_tools is out of date?', file=sys.stderr)

+ 6 - 4
rdb_wrapper.py

@@ -21,13 +21,15 @@ class ResultSinkStatus(object):
     self.status = STATUS_PASS
 
 @contextlib.contextmanager
-def setup_rdb(function_name, rel_path):
+def setup_rdb(function_name, prefix):
   """Context Manager function for ResultDB reporting.
 
   Args:
     function_name (str): The name of the function we are about to run.
-    rel_path (str): The relative path from the root of the repository to the
-      directory defining the check being executed.
+    prefix (str): The prefix for the name of the test. The format for this is
+        presubmit:gerrit_host/folder/to/repo:path/to/file/
+      for example,
+        presubmit:chromium-review.googlesource.com/chromium/src/:services/viz/
   """
   sink = None
   if 'LUCI_CONTEXT' in os.environ:
@@ -48,7 +50,7 @@ def setup_rdb(function_name, rel_path):
     elapsed_time = end_time - start_time
     if sink != None:
       tr = {
-          'testId': '{0}/:{1}'.format(rel_path, function_name),
+          'testId': '{0}:{1}'.format(prefix, function_name),
           'status': my_status.status,
           'expected': (my_status.status == STATUS_PASS),
           'duration': '{:.9f}s'.format(elapsed_time)

+ 5 - 0
tests/git_cl_test.py

@@ -2677,6 +2677,8 @@ class ChangelistTest(unittest.TestCase):
     mock.patch('git_cl.time_time').start()
     mock.patch('metrics.collector').start()
     mock.patch('subprocess2.Popen').start()
+    mock.patch('git_cl.Changelist._GetGerritProject',
+        return_value='https://chromium-review.googlesource.com').start()
     self.addCleanup(mock.patch.stopall)
     self.temp_count = 0
 
@@ -2718,6 +2720,7 @@ class ChangelistTest(unittest.TestCase):
         '--all_files',
         '--json_output', '/tmp/fake-temp2',
         '--description_file', '/tmp/fake-temp1',
+        '--gerrit_project', 'https://chromium-review.googlesource.com',
     ])
     gclient_utils.FileWrite.assert_called_once_with(
         '/tmp/fake-temp1', 'description')
@@ -2762,6 +2765,7 @@ class ChangelistTest(unittest.TestCase):
         '--upload',
         '--json_output', '/tmp/fake-temp2',
         '--description_file', '/tmp/fake-temp1',
+        '--gerrit_project', 'https://chromium-review.googlesource.com',
     ])
     gclient_utils.FileWrite.assert_called_once_with(
         '/tmp/fake-temp1', 'description')
@@ -2808,6 +2812,7 @@ class ChangelistTest(unittest.TestCase):
         '--upload',
         '--json_output', '/tmp/fake-temp2',
         '--description_file', '/tmp/fake-temp1',
+        '--gerrit_project', 'https://chromium-review.googlesource.com',
     ])
 
   @mock.patch('sys.exit', side_effect=SystemExitMock)

+ 1 - 0
tests/presubmit_unittest.py

@@ -161,6 +161,7 @@ index fe3de7b..54ae6e1 100755
     class FakeChange(object):
       def __init__(self, obj):
         self._root = obj.fake_root_dir
+        self.issue = 0
       def RepositoryRoot(self):
         return self._root
 

+ 2 - 2
tests/rdb_wrapper_test.py

@@ -38,7 +38,7 @@ class TestSetupRDB(unittest.TestCase):
     mock.patch('time.time', side_effect=[1.0, 2.0, 3.0, 4.0, 5.0]).start()
 
   def test_setup_rdb(self):
-    with rdb_wrapper.setup_rdb("_foobar", './my/folder') as my_status_obj:
+    with rdb_wrapper.setup_rdb("_foobar", './my/folder/') as my_status_obj:
       self.assertEqual(my_status_obj.status, rdb_wrapper.STATUS_PASS)
       my_status_obj.status = rdb_wrapper.STATUS_FAIL
 
@@ -61,7 +61,7 @@ class TestSetupRDB(unittest.TestCase):
 
   def test_setup_rdb_exception(self):
     with self.assertRaises(Exception):
-      with rdb_wrapper.setup_rdb("_foobar", './my/folder'):
+      with rdb_wrapper.setup_rdb("_foobar", './my/folder/'):
         raise Exception("Generic Error")
 
     expectedTr = {