diff --git a/git_cl.py b/git_cl.py index 09496db9b..3878b965d 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1273,7 +1273,7 @@ class Changelist(object): return args def RunHook(self, committing, may_prompt, verbose, parallel, upstream, - description, all_files, resultdb=False, realm=None): + description, all_files, resultdb=False): """Calls sys.exit() if the hook fails; returns a HookResults otherwise.""" args = self._GetCommonPresubmitArgs(verbose, upstream) args.append('--commit' if committing else '--upload') @@ -1292,15 +1292,10 @@ class Changelist(object): args.extend(['--description_file', description_file]) start = time_time() + cmd = ['vpython', PRESUBMIT_SUPPORT] + args - if resultdb and realm: - cmd = ['rdb', 'stream', '-new', '-realm', realm] + cmd - elif resultdb: - # TODO (crbug.com/1113463): store realm somewhere and look it up so - # it is not required to pass the realm flag - print('Note: ResultDB reporting will NOT be performed because --realm' - ' was not specified. To enable ResultDB, please run the command' - ' again with the --realm argument to specify the LUCI realm.') + if resultdb: + cmd = ['rdb', 'stream', '-new'] + cmd p = subprocess2.Popen(cmd) exit_code = p.wait() @@ -1418,8 +1413,7 @@ class Changelist(object): upstream=base_branch, description=change_desc.description, all_files=False, - resultdb=options.resultdb, - realm=options.realm) + resultdb=options.resultdb) self.ExtendCC(hook_results['more_cc']) print_stats(git_diff_args) @@ -1870,7 +1864,7 @@ class Changelist(object): detail = self._GetChangeDetail(['LABELS']) return u'Commit-Queue' in detail.get('labels', {}) - def CMDLand(self, force, bypass_hooks, verbose, parallel, resultdb, realm): + def CMDLand(self, force, bypass_hooks, verbose, parallel): if git_common.is_dirty_git_tree('land'): return 1 @@ -1911,8 +1905,7 @@ class Changelist(object): upstream=upstream, description=description, all_files=False, - resultdb=resultdb, - realm=realm) + resultdb=False) self.SubmitIssue(wait_for_merge=True) print('Issue %s has been submitted.' % self.GetIssueURL()) @@ -3838,7 +3831,6 @@ def CMDpresubmit(parser, args): parser.add_option('--resultdb', action='store_true', help='Run presubmit checks in the ResultSink environment ' 'and send results to the ResultDB database.') - parser.add_option('--realm', help='LUCI realm if reporting to ResultDB') options, args = parser.parse_args(args) if not options.force and git_common.is_dirty_git_tree('presubmit'): @@ -3865,8 +3857,7 @@ def CMDpresubmit(parser, args): upstream=base_branch, description=description, all_files=options.all, - resultdb=options.resultdb, - realm=options.realm) + resultdb=options.resultdb) return 0 @@ -4080,7 +4071,6 @@ def CMDupload(parser, args): parser.add_option('--resultdb', action='store_true', help='Run presubmit checks in the ResultSink environment ' 'and send results to the ResultDB database.') - parser.add_option('--realm', help='LUCI realm if reporting to ResultDB') orig_args = args (options, args) = parser.parse_args(args) @@ -4227,10 +4217,6 @@ def CMDland(parser, args): parser.add_option('--parallel', action='store_true', help='Run all tests specified by input_api.RunTests in all ' 'PRESUBMIT files in parallel.') - parser.add_option('--resultdb', action='store_true', - help='Run presubmit checks in the ResultSink environment ' - 'and send results to the ResultDB database.') - parser.add_option('--realm', help='LUCI realm if reporting to ResultDB') (options, args) = parser.parse_args(args) cl = Changelist() @@ -4239,8 +4225,8 @@ def CMDland(parser, args): DieWithError('You must upload the change first to Gerrit.\n' ' If you would rather have `git cl land` upload ' 'automatically for you, see http://crbug.com/642759') - return cl.CMDLand(options.force, options.bypass_hooks, options.verbose, - options.parallel, options.resultdb, options.realm) + return cl.CMDLand(options.force, options.bypass_hooks, + options.verbose, options.parallel) @subcommand.usage('') diff --git a/presubmit_support.py b/presubmit_support.py index 6c41f20a3..cd2eb7b08 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -1529,7 +1529,6 @@ class PresubmitExecuter(object): Return: A list of result objects, empty if no problems. """ - # Change to the presubmit file's directory to support local imports. main_path = os.getcwd() presubmit_dir = os.path.dirname(presubmit_path) @@ -1548,41 +1547,41 @@ class PresubmitExecuter(object): except Exception as e: raise PresubmitFailure('"%s" had an exception.\n%s' % (presubmit_path, e)) - context['__args'] = (input_api, output_api) + # These function names must change if we make substantial changes to + # the presubmit API that are not backwards compatible. + if self.committing: + function_name = 'CheckChangeOnCommit' + else: + function_name = 'CheckChangeOnUpload' + if function_name in context: + try: + context['__args'] = (input_api, output_api) + logging.debug('Running %s in %s', function_name, presubmit_path) - # 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, '/') + # TODO (crbug.com/1106943): Dive into each of the individual checks + + # 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, '/') - # Perform all the desired presubmit checks. - results = [] - try: - for function_name in context: - if not function_name.startswith('Check'): - continue - if function_name.endswith('Commit') and not self.committing: - continue - if function_name.endswith('Upload') and self.committing: - continue - logging.debug('Running %s in %s', function_name, presubmit_path) with rdb_wrapper.setup_rdb(function_name, rel_path) as my_status: result = eval(function_name + '(*__args)', context) self._check_result_type(result) if any(res.fatal for res in result): my_status.status = rdb_wrapper.STATUS_FAIL - results.extend(result) logging.debug('Running %s done.', function_name) self.more_cc.extend(output_api.more_cc) - - finally: - for f in input_api._named_temporary_files: - os.remove(f) + finally: + for f in input_api._named_temporary_files: + os.remove(f) + else: + result = () # no error since the script doesn't care about current event. # Return the process to the original working directory. os.chdir(main_path) - return results + return result def _check_result_type(self, result): """Helper function which ensures result is a list, and all elements are diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 64196c97a..7bcd64091 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2152,9 +2152,7 @@ class TestGitCl(unittest.TestCase): self.assertEqual(0, cl.CMDLand(force=True, bypass_hooks=True, verbose=True, - parallel=False, - resultdb=False, - realm=None)) + parallel=False)) self.assertIn( 'Issue chromium-review.googlesource.com/123 has been submitted', sys.stdout.getvalue()) @@ -2796,12 +2794,11 @@ class ChangelistTest(unittest.TestCase): upstream='upstream', description='description', all_files=False, - resultdb=True, - realm='chromium:public') + resultdb=True) self.assertEqual(expected_results, results) subprocess2.Popen.assert_called_once_with([ - 'rdb', 'stream', '-new', '-realm', 'chromium:public', + 'rdb', 'stream', '-new', 'vpython', 'PRESUBMIT_SUPPORT', '--root', 'root', '--upstream', 'upstream', @@ -2940,8 +2937,7 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='upstream', description='fetch description', all_files=None, - resultdb=None, - realm=None) + resultdb=None) def testNoIssue(self): git_cl.Changelist.GetIssue.return_value = None @@ -2954,8 +2950,7 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='upstream', description='get description', all_files=None, - resultdb=None, - realm=None) + resultdb=None) def testCustomBranch(self): self.assertEqual(0, git_cl.main(['presubmit', 'custom_branch'])) @@ -2967,13 +2962,12 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='custom_branch', description='fetch description', all_files=None, - resultdb=None, - realm=None) + resultdb=None) def testOptions(self): self.assertEqual( 0, git_cl.main(['presubmit', '-v', '-v', '--all', '--parallel', '-u', - '--resultdb', '--realm', 'chromium:public'])) + '--resultdb'])) git_cl.Changelist.RunHook.assert_called_once_with( committing=False, may_prompt=False, @@ -2982,8 +2976,7 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='upstream', description='fetch description', all_files=True, - resultdb=True, - realm='chromium:public') + resultdb=True) class CMDTryResultsTestCase(CMDTestCaseBase): _DEFAULT_REQUEST = { diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index d04e6e0f9..e21c957a6 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -555,7 +555,7 @@ class PresubmitUnittest(PresubmitTestsBase): executer = presubmit.PresubmitExecuter( self.fake_change, False, None, False) - self.assertEqual([], executer.ExecPresubmitScript( + self.assertEqual((), executer.ExecPresubmitScript( ('def CheckChangeOnUpload(input_api, output_api):\n' ' if len(input_api._named_temporary_files):\n' ' return (output_api.PresubmitError("!!"),)\n'