From 531d992f00bbd6861b45ebff44c73f66fa172d5f Mon Sep 17 00:00:00 2001 From: Saagar Sanghavi Date: Mon, 10 Aug 2020 20:14:01 +0000 Subject: [PATCH] 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 Reviewed-by: Edward Lesmes Commit-Queue: Saagar Sanghavi --- git_cl.py | 1 + presubmit_support.py | 30 +++++++++++++++++++++--------- rdb_wrapper.py | 10 ++++++---- tests/git_cl_test.py | 5 +++++ tests/presubmit_unittest.py | 1 + tests/rdb_wrapper_test.py | 4 ++-- 6 files changed, 36 insertions(+), 15 deletions(-) diff --git a/git_cl.py b/git_cl.py index a809e68b8..f071fbcc7 100755 --- a/git_cl.py +++ b/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 diff --git a/presubmit_support.py b/presubmit_support.py index e44b739e9..7ef84d167 100755 --- a/presubmit_support.py +++ b/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) diff --git a/rdb_wrapper.py b/rdb_wrapper.py index 302ec5e01..e075c8983 100644 --- a/rdb_wrapper.py +++ b/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) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index ff588e7df..45f34c8c3 100755 --- a/tests/git_cl_test.py +++ b/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) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index d04e6e0f9..fa538995b 100755 --- a/tests/presubmit_unittest.py +++ b/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 diff --git a/tests/rdb_wrapper_test.py b/tests/rdb_wrapper_test.py index 968fafa91..b6aa04f78 100644 --- a/tests/rdb_wrapper_test.py +++ b/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 = {