Add `git cl upload --retry-failed`

The expected behavior is that for CLs that were already uploaded
before, where some tryjobs were run and failed, git cl upload
--retry-failed will be kind of like git cl upload --cq-dry-run
except it will only trigger tryjobs that failed.

Bug: 985887
Change-Id: I6371bca3ba501b1ea2cd7160e2f933530d7e633f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1828322
Auto-Submit: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

git_cl.py

@@ -313,7 +313,8 @@ def _git_set_branch_config_value(key, value, branch=None, **kwargs):
 
 
 def _get_properties_from_options(options):
-  properties = dict(x.split('=', 1) for x in options.properties)
+  prop_list = getattr(options, 'properties', [])
+  properties = dict(x.split('=', 1) for x in prop_list)
   for key, val in properties.iteritems():
     try:
       properties[key] = json.loads(val)
@@ -456,9 +457,9 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset):
   print('To see results in browser, run:  git cl web')
 
   gerrit_changes = [changelist.GetGerritChange(patchset)]
-  shared_properties = {'category': options.category}
+  shared_properties = {'category': getattr(options, 'category', 'git_cl_try')}
   shared_properties.update(changelist.GetLegacyProperties(patchset))
-  if options.clobber:
+  if getattr(options, 'clobber', False):
     shared_properties['clobber'] = True
   shared_properties.update(_get_properties_from_options(options) or {})
 
@@ -480,7 +481,7 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset):
           'scheduleBuild': {
               'requestId': str(uuid.uuid4()),
               'builder': {
-                  'project': options.project or project,
+                  'project': getattr(options, 'project', None) or project,
                   'bucket': bucket,
                   'builder': builder,
               },
@@ -4458,10 +4459,11 @@ def CMDupload(parser, args):
   parser.add_option('--r-owners', dest='add_owners_to', action='store_const',
                     const='R', help='add a set of OWNERS to R')
   parser.add_option('-c', '--use-commit-queue', action='store_true',
+                    default=False,
                     help='tell the CQ to commit this patchset; '
-                          'implies --send-mail')
+                         'implies --send-mail')
-  parser.add_option('-d', '--cq-dry-run', dest='cq_dry_run',
+  parser.add_option('-d', '--cq-dry-run',
-                    action='store_true',
+                    action='store_true', default=False,
                     help='Send the patchset to do a CQ dry run right after '
                          'upload.')
   parser.add_option('--preserve-tryjobs', action='store_true',
@@ -4478,11 +4480,17 @@ def CMDupload(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('--no-autocc', action='store_true',
                     help='Disables automatic addition of CC emails')
   parser.add_option('--private', action='store_true',
                     help='Set the review private. This implies --no-autocc.')
+  parser.add_option('-R', '--retry-failed', action='store_true',
+                    help='Retry failed tryjobs from old patchset immediately '
+                         'after uploading new patchset. Cannot be used with '
+                         '--use-commit-queue or --cq-dry-run.')
+  parser.add_option('--buildbucket-host', default='cr-buildbucket.appspot.com',
+                    help='Host of buildbucket. The default host is %default.')
+  auth.add_auth_options(parser)
 
   orig_args = args
   _add_codereview_select_options(parser)
@@ -4502,8 +4510,11 @@ def CMDupload(parser, args):
     options.message = gclient_utils.FileRead(options.message_file)
     options.message_file = None
 
-  if options.cq_dry_run and options.use_commit_queue:
+  if ([options.cq_dry_run,
-    parser.error('Only one of --use-commit-queue and --cq-dry-run allowed.')
+       options.use_commit_queue,
+       options.retry_failed].count(True) > 1):
+    parser.error('Only one of --use-commit-queue, --cq-dry-run, or '
+                 '--retry-failed is allowed.')
 
   if options.use_commit_queue:
     options.send_mail = True
@@ -4512,8 +4523,32 @@ def CMDupload(parser, args):
   settings.GetIsGerrit()
 
   cl = Changelist()
+  if options.retry_failed and not cl.GetIssue():
+    print('No previous patchsets, so --retry-failed has no effect.')
+    options.retry_failed = False
+  # cl.GetMostRecentPatchset uses cached information, and can return the last
+  # patchset before upload. Calling it here makes it clear that it's the
+  # last patchset before upload. Note that GetMostRecentPatchset will fail
+  # if no CL has been uploaded yet.
+  if options.retry_failed:
+    patchset = cl.GetMostRecentPatchset()
+
+  ret = cl.CMDUpload(options, args, orig_args)
 
-  return cl.CMDUpload(options, args, orig_args)
+  if options.retry_failed:
+    if ret != 0:
+      print('Upload failed, so --retry-failed has no effect.')
+      return ret
+    auth_config = auth.extract_auth_config_from_options(options)
+    builds = fetch_try_jobs(
+        auth_config, cl, options.buildbucket_host, patchset)
+    buckets = _filter_failed(builds)
+    if len(buckets) == 0:
+      print('No failed tryjobs, so --retry-failed has no effect.')
+      return ret
+    _trigger_try_jobs(auth_config, cl, buckets, options, patchset + 1)
+
+  return ret
 
 
 @subcommand.usage('--description=<description file>')
@@ -4808,7 +4843,7 @@ def CMDtry(parser, args):
       print('There are no failed jobs in the latest set of jobs '
             '(patchset #%d), doing nothing.' % patchset)
       return 0
-    num_builders = sum(len(builders) for builders in buckets.values())
+    num_builders = sum(map(len, buckets.itervalues()))
     if num_builders > 10:
       confirm_or_exit('There are %d builders with failed builds.'
                       % num_builders, action='continue')

tests/git_cl_test.py

@@ -3262,7 +3262,6 @@ class CMDTryTestCase(unittest.TestCase):
     mockCallBuildbucket.assert_called_with(
         mock.ANY, 'cr-buildbucket.appspot.com', 'Batch', expected_request)
 
-
   @mock.patch('git_cl.Changelist._GetChangeDetail')
   def testScheduleOnBuildbucket_WrongBucket(self, mockGetChangeDetail):
     mockGetChangeDetail.return_value = {
@@ -3322,6 +3321,87 @@ class CMDTryTestCase(unittest.TestCase):
             'WARNING Please specify buckets', git_cl.sys.stdout.getvalue())
 
 
+class CMDUploadTestCase(unittest.TestCase):
+
+  def setUp(self):
+    super(CMDUploadTestCase, self).setUp()
+    mock.patch('git_cl.sys.stdout', StringIO.StringIO()).start()
+    mock.patch('git_cl.uuid.uuid4', _constantFn('uuid4')).start()
+    mock.patch('git_cl.Changelist.GetIssue', _constantFn(123456)).start()
+    mock.patch('git_cl.Changelist.GetCodereviewServer',
+               _constantFn('https://chromium-review.googlesource.com')).start()
+    mock.patch('git_cl.Changelist.GetMostRecentPatchset',
+               _constantFn(7)).start()
+    mock.patch('git_cl.auth.get_authenticator_for_host', AuthenticatorMock())
+    self.addCleanup(mock.patch.stopall)
+
+  @mock.patch('git_cl.fetch_try_jobs')
+  @mock.patch('git_cl._trigger_try_jobs')
+  @mock.patch('git_cl.Changelist._GetChangeDetail')
+  @mock.patch('git_cl.Changelist.CMDUpload', _constantFn(0))
+  def testUploadRetryFailed(self, mockGetChangeDetail, mockTriggerTryJobs,
+                            mockFetchTryJobs):
+    # This test mocks out the actual upload part, and just asserts that after
+    # upload, if --retry-failed is added, then the tool will fetch try jobs
+    # from the previous patchset and trigger the right builders on the latest
+    # patchset.
+    mockGetChangeDetail.return_value = {
+        'project': 'depot_tools',
+        'status': 'OPEN',
+        'owner': {'email': 'owner@e.mail'},
+        'current_revision': 'beeeeeef',
+        'revisions': {
+            'deadbeaf': {
+                '_number': 6,
+            },
+            'beeeeeef': {
+                '_number': 7,
+                'fetch': {'http': {
+                    'url': 'https://chromium.googlesource.com/depot_tools',
+                    'ref': 'refs/changes/56/123456/7'
+                }},
+            },
+        },
+    }
+    mockFetchTryJobs.return_value = {
+      '9000': {
+        'id': '9000',
+        'project': 'infra',
+        'bucket': 'luci.infra.try',
+        'created_by': 'user:someone@chromium.org',
+        'created_ts': '147200002222000',
+        'experimental': False,
+        'parameters_json': json.dumps({
+          'builder_name': 'red-bot',
+          'properties': {'category': 'cq'},
+        }),
+        'status': 'COMPLETED',
+        'result': 'FAILURE',
+        'tags': ['user_agent:cq'],
+      },
+      8000: {
+        'id': '8000',
+        'project': 'infra',
+        'bucket': 'luci.infra.try',
+        'created_by': 'user:someone@chromium.org',
+        'created_ts': '147200002222020',
+        'experimental': False,
+        'parameters_json': json.dumps({
+          'builder_name': 'green-bot',
+          'properties': {'category': 'cq'},
+        }),
+        'status': 'COMPLETED',
+        'result': 'SUCCESS',
+        'tags': ['user_agent:cq'],
+      },
+    }
+    self.assertEqual(0, git_cl.main(['upload', '--retry-failed']))
+    mockFetchTryJobs.assert_called_with(
+        mock.ANY, mock.ANY, 'cr-buildbucket.appspot.com', 7)
+    buckets = {'infra/try': {'red-bot': []}}
+    mockTriggerTryJobs.assert_called_once_with(
+        mock.ANY, mock.ANY, buckets, mock.ANY, 8)
+
 if __name__ == '__main__':
   logging.basicConfig(
       level=logging.DEBUG if '-v' in sys.argv else logging.ERROR)