commit 99df04e8aa2421d6872d2177e92f5444d3467c82
author Edward Lemur <ehmaldonado@google.com> Thu Mar 05 19:39:43 2020 +0000
committer LUCI CQ <infra-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Mar 05 19:39:43 2020 +0000
tree 7d8eedd9781f855f00b3cf9e11c7eaa42faead87
parent 3ccfc90f5032339a302fbcf84b483b62a74ea02e

git-cl: Check if author exists before adding to presubmit command line.

When user.email is not configured in git, git-cl tries to call
presubmit support with --author None, which makes git-cl crash.

Bug: b/150870673
Change-Id: Idc42ba2b970340ed93e1e92f65850fc1a12336d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2090375
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

tests/git_cl_test.py

diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index c817d5a..b7243e2 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -2842,13 +2842,13 @@
     self.assertEqual(expected_results, results)
     subprocess2.Popen.assert_called_once_with([
         'vpython', 'PRESUBMIT_SUPPORT',
-        '--author', 'author',
         '--root', 'root',
         '--upstream', 'upstream',
         '--verbose', '--verbose',
+        '--author', 'author',
+        '--gerrit_url', 'https://chromium-review.googlesource.com',
         '--issue', '123456',
         '--patchset', '7',
-        '--gerrit_url', 'https://chromium-review.googlesource.com',
         '--commit',
         '--may_prompt',
         '--parallel',
@@ -2864,6 +2864,49 @@
       'exit_code': 0,
     })
 
+  def testRunHook_FewerOptions(self):
+    expected_results = {
+        'more_cc': ['more@example.com', 'cc@example.com'],
+        'should_continue': True,
+    }
+    gclient_utils.FileRead.return_value = json.dumps(expected_results)
+    git_cl.time_time.side_effect = [100, 200]
+    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
+    git_cl.Changelist.GetCodereviewServer.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)
+
+    self.assertEqual(expected_results, results)
+    subprocess2.Popen.assert_called_once_with([
+        'vpython', 'PRESUBMIT_SUPPORT',
+        '--root', 'root',
+        '--upstream', 'upstream',
+        '--upload',
+        '--json_output', '/tmp/fake-temp2',
+        '--description_file', '/tmp/fake-temp1',
+    ])
+    gclient_utils.FileWrite.assert_called_once_with(
+        '/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]
@@ -2890,13 +2933,13 @@
 
     subprocess2.Popen.assert_called_once_with([
         'vpython', 'PRESUBMIT_SUPPORT',
-        '--author', 'author',
         '--root', 'root',
         '--upstream', 'upstream',
         '--verbose', '--verbose',
+        '--author', 'author',
+        '--gerrit_url', 'https://chromium-review.googlesource.com',
         '--issue', '123456',
         '--patchset', '7',
-        '--gerrit_url', 'https://chromium-review.googlesource.com',
         '--post_upload',
         '--description_file', '/tmp/fake-temp1',
     ])