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/+/2090375Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

git_cl.py

@@ -1405,22 +1405,24 @@ class Changelist(object):
 
   def _GetCommonPresubmitArgs(self, verbose, upstream):
     args = [
-        '--author', self.GetAuthor(),
         '--root', settings.GetRoot(),
         '--upstream', upstream,
     ]
 
     args.extend(['--verbose'] * verbose)
 
+    author = self.GetAuthor()
+    gerrit_url = self.GetCodereviewServer()
     issue = self.GetIssue()
     patchset = self.GetPatchset()
-    gerrit_url = self.GetCodereviewServer()
+    if author:
+      args.extend(['--author', author])
+    if gerrit_url:
+      args.extend(['--gerrit_url', gerrit_url])
     if issue:
       args.extend(['--issue', str(issue)])
     if patchset:
       args.extend(['--patchset', str(patchset)])
-    if gerrit_url:
-      args.extend(['--gerrit_url', gerrit_url])
 
     return args
 

tests/git_cl_test.py

@@ -2842,13 +2842,13 @@ class ChangelistTest(unittest.TestCase):
     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 @@ class ChangelistTest(unittest.TestCase):
       '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 @@ class ChangelistTest(unittest.TestCase):
 
     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',
     ])