From 35ef5ada1db47906a6c6974acd82f7bf18438678 Mon Sep 17 00:00:00 2001 From: Aleksey Khoroshilov Date: Fri, 3 Jun 2022 18:29:25 +0000 Subject: [PATCH] Run presubmit checks without GerritAccessor if GERRIT_HOST is not set. When a repository doesn't work with Gerrit, it doesn't make sense to create GerritAccessor and pass it to presubmit checks. gerrit.host is stored into git config when codereview.settings file contains GERRIT_HOST item. Change-Id: I9740950caf85a1da9a6e4ae12e3612625cf0cec5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3687235 Reviewed-by: Josip Sokcevic Auto-Submit: Aleksey Khoroshilov Commit-Queue: Josip Sokcevic --- git_cl.py | 13 +++++++++--- tests/git_cl_test.py | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/git_cl.py b/git_cl.py index c22c68221..35767c19d 100755 --- a/git_cl.py +++ b/git_cl.py @@ -805,6 +805,12 @@ class Settings(object): return False return None + def GetIsGerrit(self): + """Return True if gerrit.host is set.""" + if self.is_gerrit is None: + self.is_gerrit = bool(self._GetConfig('gerrit.host', False)) + return self.is_gerrit + def GetGerritSkipEnsureAuthenticated(self): """Return True if EnsureAuthenticated should not be done for Gerrit uploads.""" @@ -1323,9 +1329,10 @@ class Changelist(object): remote, remote_branch = self.GetRemoteBranch() target_ref = GetTargetRef(remote, remote_branch, None) - args.extend(['--gerrit_url', self.GetCodereviewServer()]) - args.extend(['--gerrit_project', self.GetGerritProject()]) - args.extend(['--gerrit_branch', target_ref]) + if settings.GetIsGerrit(): + args.extend(['--gerrit_url', self.GetCodereviewServer()]) + args.extend(['--gerrit_project', self.GetGerritProject()]) + args.extend(['--gerrit_branch', target_ref]) author = self.GetAuthor() issue = self.GetIssue() diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 6a67c11a0..e08a8507d 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -3061,6 +3061,7 @@ class ChangelistTest(unittest.TestCase): return_value=('origin', 'refs/remotes/origin/main')).start() mock.patch('git_cl.PRESUBMIT_SUPPORT', 'PRESUBMIT_SUPPORT').start() mock.patch('git_cl.Settings.GetRoot', return_value='root').start() + mock.patch('git_cl.Settings.GetIsGerrit', return_value=True).start() mock.patch('git_cl.time_time').start() mock.patch('metrics.collector').start() mock.patch('subprocess2.Popen').start() @@ -3231,6 +3232,53 @@ class ChangelistTest(unittest.TestCase): '--description_file', '/tmp/fake-temp1', ]) + def testRunHook_NoGerrit(self): + mock.patch('git_cl.Settings.GetIsGerrit', return_value=False).start() + + expected_results = { + 'more_cc': ['cc@example.com', 'more@example.com'], + 'errors': [], + 'notifications': [], + 'warnings': [], + } + gclient_utils.FileRead.return_value = json.dumps(expected_results) + git_cl.time_time.side_effect = [100, 200, 300, 400] + mockProcess = mock.Mock() + mockProcess.wait.return_value = 0 + subprocess2.Popen.return_value = mockProcess + + git_cl.Changelist.GetAuthor.return_value = None + git_cl.Changelist.GetIssue.return_value = None + git_cl.Changelist.GetPatchset.return_value = None + + cl = git_cl.Changelist() + results = cl.RunHook( + committing=False, + may_prompt=False, + verbose=0, + parallel=False, + upstream='upstream', + description='description', + all_files=False, + resultdb=False) + + self.assertEqual(expected_results, results) + subprocess2.Popen.assert_any_call([ + 'vpython3', 'PRESUBMIT_SUPPORT', + '--root', 'root', + '--upstream', 'upstream', + '--upload', + '--json_output', '/tmp/fake-temp2', + '--description_file', '/tmp/fake-temp1', + ]) + gclient_utils.FileWrite.assert_any_call( + '/tmp/fake-temp1', 'description') + metrics.collector.add_repeated('sub_commands', { + 'command': 'presubmit', + 'execution_time': 100, + 'exit_code': 0, + }) + @mock.patch('sys.exit', side_effect=SystemExitMock) def testRunHook_Failure(self, _mock): git_cl.time_time.side_effect = [100, 200]