Implement and test git cl try for Gerrit.

git_cl.py

Index: git_cl.py
diff --git a/git_cl.py b/git_cl.py
index 51593f20ec52fc0a99f65b70b859fcd1fa01a807..3d4f42c866bd8e6f3244a57b0885fe38738f2af8 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -427,7 +427,6 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options,
   # _RietveldChangelistImpl does, then caching values in these two variables
   # won't be necessary.
   owner_email = changelist.GetIssueOwner()
-  project = changelist.GetIssueProject()
 
   buildbucket_put_url = (
       'https://{hostname}/_ah/api/buildbucket/v1/builds/batch'.format(
@@ -437,7 +436,14 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options,
       hostname=codereview_host,
       issue=changelist.GetIssue(),
       patch=patchset)
+
+  shared_parameters_properties = changelist.GetTryjobProperties(patchset)
+  shared_parameters_properties['category'] = category
+  if options.clobber:
+    shared_parameters_properties['clobber'] = True
   extra_properties = _get_properties_from_options(options)
+  if extra_properties:
+    shared_parameters_properties.update(extra_properties)
 
   batch_req_body = {'builds': []}
   print_text = []
@@ -455,21 +461,12 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options,
               'author': {'email': owner_email},
               'revision': options.revision,
           }],
-          'properties': {
-              'category': category,
-              'issue': changelist.GetIssue(),
-              'patch_project': project,
-              'patch_storage': 'rietveld',
-              'patchset': patchset,
-              'rietveld': codereview_url,
-          },
+          'properties': shared_parameters_properties.copy(),
       }
       if 'presubmit' in builder.lower():
         parameters['properties']['dry_run'] = 'true'
       if tests:
         parameters['properties']['testfilter'] = tests
-      if extra_properties:
-        parameters['properties'].update(extra_properties)
       if options.clobber:

[Michael Achenbach, 2016/11/04 14:44:58]

Looks like this is double now.

[tandrii(chromium), 2016/11/04 14:48:17]

true.

         parameters['properties']['clobber'] = True
 
@@ -1691,12 +1688,6 @@ class Changelist(object):
     """Get owner from codereview, which may differ from this checkout."""
     return self._codereview_impl.GetIssueOwner()
 
-  def GetIssueProject(self):
-    """Get project from codereview, which may differ from what this
-    checkout's codereview.settings or gerrit project URL say.
-    """
-    return self._codereview_impl.GetIssueProject()
-
   def GetApprovingReviewers(self):
     return self._codereview_impl.GetApprovingReviewers()
 
@@ -1707,6 +1698,10 @@ class Changelist(object):
     """Returns reason (str) if unable trigger tryjobs on this CL or None."""
     return self._codereview_impl.CannotTriggerTryJobReason()
 
+  def GetTryjobProperties(self, patchset=None):
+    """Returns dictionary of properties to launch tryjob."""
+    return self._codereview_impl.GetTryjobProperties(patchset=patchset)
+
   def __getattr__(self, attr):
     # This is because lots of untested code accesses Rietveld-specific stuff
     # directly, and it's hard to fix for sure. So, just let it work, and fix
@@ -1839,7 +1834,7 @@ class _ChangelistCodereviewBase(object):
   def GetIssueOwner(self):
     raise NotImplementedError()
 
-  def GetIssueProject(self):
+  def GetTryjobProperties(self, patchset=None):
     raise NotImplementedError()
 
 
@@ -1920,15 +1915,23 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
       return 'CL %s is private' % self.GetIssue()
     return None
 
+  def GetTryjobProperties(self, patchset=None):
+    """Returns dictionary of properties to launch tryjob."""
+    project = (self.GetIssueProperties() or {}).get('project')
+    return {
+      'issue': self.GetIssue(),
+      'patch_project': project,
+      'patch_storage': 'rietveld',
+      'patchset': patchset or self.GetPatchset(),
+      'rietveld': self.GetCodereviewServer(),
+    }
+
   def GetApprovingReviewers(self):
     return get_approving_reviewers(self.GetIssueProperties())
 
   def GetIssueOwner(self):
     return (self.GetIssueProperties() or {}).get('owner_email')
 
-  def GetIssueProject(self):
-    return (self.GetIssueProperties() or {}).get('project')
-
   def AddComment(self, message):
     return self.RpcServer().add_comment(self.GetIssue(), message)
 
@@ -2868,16 +2871,38 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
                           labels={'Commit-Queue': vote_map[new_state]})
 
   def CannotTriggerTryJobReason(self):
-    # TODO(tandrii): implement for Gerrit.
-    raise NotImplementedError()
+    try:
+      data = self._GetChangeDetail()
+    except GerritIssueNotExists:
+      return 'Gerrit doesn\'t know about your issue %s' % self.GetIssue()
 
-  def GetIssueOwner(self):
-    # TODO(tandrii): implement for Gerrit.
-    raise NotImplementedError()
+    if data['status'] in ('ABANDONED', 'MERGED'):
+      return 'CL %s is closed' % (self.GetIssue())

[Michael Achenbach, 2016/11/04 14:44:58]

nit: no parentheses needed

[tandrii(chromium), 2016/11/04 14:48:17]

Done.

+
+  def GetTryjobProperties(self, patchset=None):
+    """Returns dictionary of properties to launch tryjob."""
+    data = self._GetChangeDetail(['ALL_REVISIONS'])
+    patchset = int(patchset or self.GetPatchset())
+    assert patchset
+    revision_data = None  # Pylint wants it to be defined.
+    for revision_data in data['revisions'].itervalues():
+      if int(revision_data['_number']) == patchset:
+        break
+    else:
+      raise Exception('Patchset %d is not known in Gerrit issue %d' %
+                      (patchset, self.GetIssue()))
+    return {
+      'patch_issue': self.GetIssue(),
+      'patch_set': patchset or self.GetPatchset(),
+      'patch_project': data['project'],
+      'patch_storage': 'gerrit',
+      'patch_ref': revision_data['fetch']['http']['ref'],
+      'patch_repository_url': revision_data['fetch']['http']['url'],
+      'patch_gerrit_url': self.GetCodereviewServer(),
+    }
 
-  def GetIssueProject(self):
-    # TODO(tandrii): implement for Gerrit.
-    raise NotImplementedError()
+  def GetIssueOwner(self):
+    return self._GetChangeDetail(['DETAILED_ACCOUNTS'])['owner']['email']
 
 
 _CODEREVIEW_IMPLEMENTATIONS = {
@@ -4828,12 +4853,6 @@ def CMDtry(parser, args):
   if not cl.GetIssue():
     parser.error('Need to upload first')
 
-  if cl.IsGerrit():
-    parser.error(
-        'Not yet supported for Gerrit (http://crbug.com/599931).\n'
-        'If your project has Commit Queue, dry run is a workaround:\n'
-        '  git cl set-commit --dry-run')
-
   error_message = cl.CannotTriggerTryJobReason()
   if error_message:
     parser.error('Can\'t trigger try jobs: %s' % error_message)