diff --git a/git_cl.py b/git_cl.py index 91ef86cf6..52b5147d8 100755 --- a/git_cl.py +++ b/git_cl.py @@ -416,78 +416,74 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset): buckets: A nested dict mapping bucket names to builders to tests. options: Command-line options. """ - assert changelist.GetIssue(), 'CL must be uploaded first' - codereview_url = changelist.GetCodereviewServer() - assert codereview_url, 'CL must be uploaded first' - patchset = patchset or changelist.GetMostRecentPatchset() - assert patchset, 'CL must be uploaded first' - - codereview_host = urlparse.urlparse(codereview_url).hostname - # Cache the buildbucket credentials under the codereview host key, so that - # users can use different credentials for different buckets. - authenticator = auth.get_authenticator_for_host(codereview_host, auth_config) - http = authenticator.authorize(httplib2.Http()) - http.force_exception_to_status_code = True - - buildbucket_put_url = ( - 'https://{hostname}/_ah/api/buildbucket/v1/builds/batch'.format( - hostname=options.buildbucket_host)) - buildset = 'patch/gerrit/{hostname}/{issue}/{patch}'.format( - hostname=codereview_host, - issue=changelist.GetIssue(), - patch=patchset) - - shared_parameters_properties = changelist.GetTryJobProperties(patchset) - shared_parameters_properties['category'] = options.category + _, ret_code = subprocess2.communicate( + ['bb', 'auth-info'], stdout=subprocess2.VOID, stderr=subprocess2.VOID) + if ret_code: + DieWithError('"bb auth-login" must be executed before scheduling try jobs.') + + gerrit_change = changelist.GetGerritChange() + shared_properties = { + 'category': options.category, + } if options.clobber: - shared_parameters_properties['clobber'] = True + shared_properties['clobber'] = True extra_properties = _get_properties_from_options(options) if extra_properties: - shared_parameters_properties.update(extra_properties) + shared_properties.update(extra_properties) - batch_req_body = {'builds': []} + batch_request = {'requests': []} print_text = [] print_text.append('Tried jobs on:') for bucket, builders_and_tests in sorted(buckets.iteritems()): print_text.append('Bucket: %s' % bucket) + project, bucket = bucket.split('.', 1) for builder, tests in sorted(builders_and_tests.iteritems()): print_text.append(' %s: %s' % (builder, tests)) - parameters = { - 'builder_name': builder, - 'changes': [{ - 'author': {'email': changelist.GetIssueOwner()}, - 'revision': options.revision, - }], - 'properties': shared_parameters_properties.copy(), - } + properties = shared_properties.copy() if 'presubmit' in builder.lower(): - parameters['properties']['dry_run'] = 'true' + properties['dry_run'] = 'true' if tests: - parameters['properties']['testfilter'] = tests + properties['testfilter'] = tests tags = [ - 'builder:%s' % builder, - 'buildset:%s' % buildset, - 'user_agent:git_cl_try', + { + 'key': 'builder', + 'value': builder, + }, + { + 'key': 'user_agent', + 'value': 'git_cl_try', + }, ] - batch_req_body['builds'].append( + batch_request['requests'].append( { - 'bucket': bucket, - 'parameters_json': json.dumps(parameters), - 'client_operation_id': str(uuid.uuid4()), - 'tags': tags, + 'scheduleBuild': { + 'builder': { + 'project': options.project or project, + 'bucket': bucket, + 'builder': builder, + }, + 'properties': properties, + 'requestId': str(uuid.uuid4()), + 'gerritChanges': [gerrit_change], + 'tags': tags, + } } ) - _buildbucket_retry( - 'triggering tryjobs', - http, - buildbucket_put_url, - 'PUT', - body=json.dumps(batch_req_body), - headers={'Content-Type': 'application/json'} - ) + (stdout, _), ret_code = subprocess2.communicate( + ['bb', 'batch', '-host', options.buildbucket_host], + stdin=json.dumps(batch_request, sort_keys=True), + stdout=subprocess2.PIPE) + if ret_code: + print_text.append('Failed to schedule builds for some bots:') + if stdout: + responses = json.loads(stdout) + print_text.extend( + ' ' + response['error']['message'] + for response in responses['responses'] if 'error' in response) + print_text.append('To see results here, run: git cl try-results') print_text.append('To see results in browser, run: git cl web') print('\n'.join(print_text)) @@ -1617,10 +1613,6 @@ 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 @@ -1760,10 +1752,6 @@ class _ChangelistCodereviewBase(object): def GetReviewers(self): raise NotImplementedError() - def GetTryJobProperties(self, patchset=None): - raise NotImplementedError() - - class _GerritChangelistImpl(_ChangelistCodereviewBase): def __init__(self, changelist, codereview_host=None): super(_GerritChangelistImpl, self).__init__(changelist) @@ -2882,26 +2870,27 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): if data['status'] in ('ABANDONED', 'MERGED'): return 'CL %s is closed' % self.GetIssue() - def GetTryJobProperties(self, patchset=None): - """Returns dictionary of properties to launch a tryjob.""" - data = self._GetChangeDetail(['ALL_REVISIONS']) + def GetGerritChange(self, patchset=None): + """Returns a buildbucket.v2.GerritChange message for the current issue.""" + host = urlparse.urlparse(self.GetCodereviewServer()).hostname + issue = self.GetIssue() 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: + data = self._GetChangeDetail(['ALL_REVISIONS']) + + assert host and issue and patchset, 'CL must be uploaded first' + + has_patchset = any( + int(revision_data['_number']) == patchset + for revision_data in data['revisions'].itervalues()) + if not has_patchset: raise Exception('Patchset %d is not known in Gerrit change %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(), + 'host': host, + 'change': issue, + 'project': data['project'], + 'patchset': patchset, } def GetIssueOwner(self): diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 82ef8c958..ee9be6d2a 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2381,6 +2381,42 @@ class TestGitCl(TestCase): def test_git_cl_try_buildbucket_with_properties_gerrit(self): self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda _: 7) self.mock(git_cl.uuid, 'uuid4', lambda: 'uuid4') + def mocked_call_with_stdin(*a, **kw): + args = list(a) + if kw.get('stdin'): + args.append(kw['stdin']) + return ([self._mocked_call(*args), ''], 0) + + self.mock(subprocess2, 'communicate', mocked_call_with_stdin) + + bb_request = json.dumps({ + "requests": [{ + "scheduleBuild": { + "requestId": "uuid4", + "builder": { + "project": "luci", + "builder": "win", + "bucket": "chromium.try", + }, + "gerritChanges": [{ + "project": "depot_tools", + "host": "chromium-review.googlesource.com", + "patchset": 7, + "change": 123456, + }], + "properties": { + "category": "git_cl_try", + "json": [{"a": 1}, None], + "key": "val", + }, + "tags": [ + {"value": "win", "key": "builder"}, + {"value": "git_cl_try", "key": "user_agent"}, + ], + }, + }], + }, sort_keys=True) + self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), @@ -2410,46 +2446,12 @@ class TestGitCl(TestCase): }, }, }), + ((['bb', 'auth-info'],), ''), + ((['git', 'config', 'branch.feature.gerritpatchset'],), '7'), + ((['bb', 'batch', '-host', 'cr-buildbucket.appspot.com'], + bb_request,), ''), ] - def _buildbucket_retry(*_, **kw): - # self.maxDiff = 10000 - body = json.loads(kw['body']) - self.assertEqual(len(body['builds']), 1) - build = body['builds'][0] - params = json.loads(build.pop('parameters_json')) - self.assertEqual(params, { - u'builder_name': u'win', - u'changes': [{u'author': {u'email': u'owner@e.mail'}, - u'revision': None}], - u'properties': { - u'category': u'git_cl_try', - u'key': u'val', - u'json': [{u'a': 1}, None], - - u'patch_gerrit_url': - u'https://chromium-review.googlesource.com', - u'patch_issue': 123456, - u'patch_project': u'depot_tools', - u'patch_ref': u'refs/changes/56/123456/7', - u'patch_repository_url': - u'https://chromium.googlesource.com/depot_tools', - u'patch_set': 7, - u'patch_storage': u'gerrit', - } - }) - self.assertEqual(build, { - u'bucket': u'luci.chromium.try', - u'client_operation_id': u'uuid4', - u'tags': [ - u'builder:win', - u'buildset:patch/gerrit/chromium-review.googlesource.com/123456/7', - u'user_agent:git_cl_try', - ], - }) - - self.mock(git_cl, '_buildbucket_retry', _buildbucket_retry) - self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.assertEqual(0, git_cl.main([ 'try', '-B', 'luci.chromium.try', '-b', 'win',