From 24025d3e71ac1d38ad22d30c8517743062186744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Hajdan=2C=20Jr?= Date: Tue, 11 Jul 2017 16:38:21 +0200 Subject: [PATCH] tryserver: add support for gerrit footers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 644609 Change-Id: Ied7439d688b89f90b9705968927521b5060c5fb4 Reviewed-on: https://chromium-review.googlesource.com/565564 Commit-Queue: Paweł Hajdan Jr. Reviewed-by: Andrii Shyshkalov --- gerrit_client.py | 7 +- recipes/README.recipes.md | 36 ++++-- recipes/recipe_modules/gerrit/api.py | 36 +++++- .../gerrit/examples/full.expected/basic.json | 114 +++++++++++++++++- .../recipe_modules/gerrit/examples/full.py | 14 ++- recipes/recipe_modules/gerrit/test_api.py | 8 ++ recipes/recipe_modules/git_cl/api.py | 19 +-- .../recipe_modules/git_cl/examples/full.py | 6 +- recipes/recipe_modules/tryserver/__init__.py | 1 + recipes/recipe_modules/tryserver/api.py | 24 ++-- .../full.expected/with_gerrit_patch.json | 66 ++++++---- .../with_gerrit_patch_deprecated.json | 40 ------ .../recipe_modules/tryserver/examples/full.py | 19 +-- 13 files changed, 273 insertions(+), 117 deletions(-) delete mode 100644 recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_deprecated.json diff --git a/gerrit_client.py b/gerrit_client.py index e7aac7291..f2501f1ed 100644 --- a/gerrit_client.py +++ b/gerrit_client.py @@ -67,6 +67,8 @@ def CMDbranch(parser, args): def CMDchanges(parser, args): parser.add_option('-p', '--param', dest='params', action='append', help='repeatable query parameter, format: -p key=value') + parser.add_option('-o', '--o-param', dest='o_params', action='append', + help='gerrit output parameters, e.g. ALL_REVISIONS') parser.add_option('--limit', dest='limit', type=int, help='maximum number of results to return') parser.add_option('--start', dest='start', type=int, @@ -78,8 +80,9 @@ def CMDchanges(parser, args): result = gerrit_util.QueryChanges( urlparse.urlparse(opt.host).netloc, list(tuple(p.split('=', 1)) for p in opt.params), - start=opt.start, # Default: None - limit=opt.limit, # Default: None + start=opt.start, # Default: None + limit=opt.limit, # Default: None + o_params=opt.o_params, # Default: None ) logging.info('Change query returned %d changes.', len(result)) write_result(result, opt) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 525f21a42..0ecf95e68 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -312,6 +312,18 @@ Create a new branch from given project and commit Returns: the ref of the branch created +— **def [get\_change\_description](/recipes/recipe_modules/gerrit/api.py#93)(self, host, change, patchset):** + +Get the description for a given CL and patchset. + +Args: + host: Gerrit host to query. + change: The change number. + patchset: The patchset number. + +Returns: + The description corresponding to given CL and patchset. + — **def [get\_change\_destination\_branch](/recipes/recipe_modules/gerrit/api.py#68)(self, host, change, \*\*kwargs):** Get the upstream branch for a given CL. @@ -323,7 +335,7 @@ Args: Returns: the name of the branch -— **def [get\_changes](/recipes/recipe_modules/gerrit/api.py#93)(self, host, query_params, start=None, limit=None, \*\*kwargs):** +— **def [get\_changes](/recipes/recipe_modules/gerrit/api.py#122)(self, host, query_params, start=None, limit=None, o_params=None, \*\*kwargs):** Query changes for the given host. @@ -334,6 +346,8 @@ Args: https://gerrit-review.googlesource.com/Documentation/user-search.html#search-operators start: How many changes to skip (starting with the most recent). limit: Maximum number of results to return. + o_params: A list of additional output specifiers, as documented here: + https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes Returns: A list of change dicts as documented here: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes @@ -474,13 +488,15 @@ remote_name (str): the remote name to rebase from if not origin — **def [\_\_call\_\_](/recipes/recipe_modules/git_cl/api.py#10)(self, subcmd, args, name=None, \*\*kwargs):** -— **def [get\_description](/recipes/recipe_modules/git_cl/api.py#23)(self, patch=None, codereview=None, \*\*kwargs):** +— **def [get\_description](/recipes/recipe_modules/git_cl/api.py#23)(self, patch_url=None, codereview=None, \*\*kwargs):** -— **def [issue](/recipes/recipe_modules/git_cl/api.py#51)(self, \*\*kwargs):** +DEPRECATED. Consider using gerrit.get_change_description instead. -— **def [set\_description](/recipes/recipe_modules/git_cl/api.py#32)(self, description, patch=None, codereview=None, \*\*kwargs):** +— **def [issue](/recipes/recipe_modules/git_cl/api.py#54)(self, \*\*kwargs):** -— **def [upload](/recipes/recipe_modules/git_cl/api.py#44)(self, message, upload_args=None, \*\*kwargs):** +— **def [set\_description](/recipes/recipe_modules/git_cl/api.py#34)(self, description, patch_url=None, codereview=None, \*\*kwargs):** + +— **def [upload](/recipes/recipe_modules/git_cl/api.py#47)(self, message, upload_args=None, \*\*kwargs):** ### *recipe_modules* / [gitiles](/recipes/recipe_modules/gitiles) [DEPS](/recipes/recipe_modules/gitiles/__init__.py#1): [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/url][recipe_engine/recipe_modules/url] @@ -644,7 +660,7 @@ Returns: given is unknown. ### *recipe_modules* / [tryserver](/recipes/recipe_modules/tryserver) -[DEPS](/recipes/recipe_modules/tryserver/__init__.py#5): [git](#recipe_modules-git), [git\_cl](#recipe_modules-git_cl), [rietveld](#recipe_modules-rietveld), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step] +[DEPS](/recipes/recipe_modules/tryserver/__init__.py#5): [gerrit](#recipe_modules-gerrit), [git](#recipe_modules-git), [git\_cl](#recipe_modules-git_cl), [rietveld](#recipe_modules-rietveld), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step] #### **class [TryserverApi](/recipes/recipe_modules/tryserver/api.py#16)([RecipeApi][recipe_engine/wkt/RecipeApi]):** @@ -682,7 +698,7 @@ TODO(tandrii): remove this doc. Unless you use patch_root=None, in which case old behavior is used which returns paths relative to checkout aka solution[0].name. -— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#285)(self, tag, patch_text=None):** +— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#287)(self, tag, patch_text=None):** Gets a specific tag from a CL description @@ -713,6 +729,8 @@ Args: This is only used if the patch comes from Rietveld. Possible values: None, 'oauth2' (see also api.rietveld.apply_issue.) +— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#291)(self, footer):** + — **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#203)(self):** Mark the tryjob result as a compile failure. @@ -780,9 +798,9 @@ like checkout or compile), and some of these tests have failed. — **def [RunSteps](/recipes/recipe_modules/gclient/tests/patch_project.py#20)(api, patch_project):** ### *recipes* / [gerrit:examples/full](/recipes/recipe_modules/gerrit/examples/full.py) -[DEPS](/recipes/recipe_modules/gerrit/examples/full.py#5): [gerrit](#recipe_modules-gerrit) +[DEPS](/recipes/recipe_modules/gerrit/examples/full.py#5): [gerrit](#recipe_modules-gerrit), [recipe\_engine/step][recipe_engine/recipe_modules/step] -— **def [RunSteps](/recipes/recipe_modules/gerrit/examples/full.py#10)(api):** +— **def [RunSteps](/recipes/recipe_modules/gerrit/examples/full.py#11)(api):** ### *recipes* / [git:examples/full](/recipes/recipe_modules/git/examples/full.py) [DEPS](/recipes/recipe_modules/git/examples/full.py#5): [git](#recipe_modules-git), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step] diff --git a/recipes/recipe_modules/gerrit/api.py b/recipes/recipe_modules/gerrit/api.py index 69621fca2..0b3532137 100644 --- a/recipes/recipe_modules/gerrit/api.py +++ b/recipes/recipe_modules/gerrit/api.py @@ -90,7 +90,37 @@ class GerritApi(recipe_api.RecipeApi): 'Error quering for branch of CL %s' % change) return changes[0]['branch'] - def get_changes(self, host, query_params, start=None, limit=None, **kwargs): + def get_change_description(self, host, change, patchset): + """ + Get the description for a given CL and patchset. + + Args: + host: Gerrit host to query. + change: The change number. + patchset: The patchset number. + + Returns: + The description corresponding to given CL and patchset. + """ + assert int(change), change + assert int(patchset), patchset + cls = self.get_changes( + host, + query_params=[('change', str(change))], + o_params=['ALL_REVISIONS', 'ALL_COMMITS'], + limit=1) + cl = cls[0] if len(cls) == 1 else {'revisions': {}} + for ri in cl['revisions'].itervalues(): + # TODO(tandrii): add support for patchset=='current'. + if str(ri['_number']) == str(patchset): + return ri['commit']['message'] + + raise self.m.step.InfraFailure( + 'Error querying for CL description: host:%r change:%r; patchset:%r' % ( + host, change, patchset)) + + def get_changes(self, host, query_params, start=None, limit=None, + o_params=None, **kwargs): """ Query changes for the given host. @@ -101,6 +131,8 @@ class GerritApi(recipe_api.RecipeApi): https://gerrit-review.googlesource.com/Documentation/user-search.html#search-operators start: How many changes to skip (starting with the most recent). limit: Maximum number of results to return. + o_params: A list of additional output specifiers, as documented here: + https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes Returns: A list of change dicts as documented here: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes @@ -116,6 +148,8 @@ class GerritApi(recipe_api.RecipeApi): args += ['--limit', str(limit)] for k, v in query_params: args += ['-p', '%s=%s' % (k, v)] + for v in (o_params or []): + args += ['-o', v] return self( kwargs.pop('name', 'changes'), diff --git a/recipes/recipe_modules/gerrit/examples/full.expected/basic.json b/recipes/recipe_modules/gerrit/examples/full.expected/basic.json index f2b2b9dfe..6aeacafb3 100644 --- a/recipes/recipe_modules/gerrit/examples/full.expected/basic.json +++ b/recipes/recipe_modules/gerrit/examples/full.expected/basic.json @@ -93,6 +93,62 @@ "@@@STEP_LOG_LINE@json.output@ \"created\": \"2017-01-30 13:11:20.000000000\", @@@", "@@@STEP_LOG_LINE@json.output@ \"has_review_started\": false, @@@", "@@@STEP_LOG_LINE@json.output@ \"project\": \"chromium/src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"commit\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"message\": \"Change commit message\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"status\": \"NEW\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"subject\": \"Change title\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@]@@@", + "@@@STEP_LOG_END@json.output@@@" + ] + }, + { + "cmd": [ + "python", + "-u", + "RECIPE_PACKAGE_REPO[depot_tools]/gerrit_client.py", + "changes", + "--host", + "https://chromium-review.googlesource.com", + "--json_file", + "/path/to/tmp/json", + "--limit", + "1", + "-p", + "change=123", + "-o", + "ALL_REVISIONS", + "-o", + "ALL_COMMITS" + ], + "env": { + "PATH": ":RECIPE_PACKAGE_REPO[depot_tools]" + }, + "infra_step": true, + "name": "gerrit changes (2)", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@json.output@[@@@", + "@@@STEP_LOG_LINE@json.output@ {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"91827\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"branch\": \"master\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"change_id\": \"Ideadbeef\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"created\": \"2017-01-30 13:11:20.000000000\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"has_review_started\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"project\": \"chromium/src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"commit\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"message\": \"Change commit message\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", "@@@STEP_LOG_LINE@json.output@ \"status\": \"NEW\", @@@", "@@@STEP_LOG_LINE@json.output@ \"subject\": \"Change title\"@@@", "@@@STEP_LOG_LINE@json.output@ }@@@", @@ -129,6 +185,14 @@ "@@@STEP_LOG_LINE@json.output@ \"created\": \"2017-01-30 13:11:20.000000000\", @@@", "@@@STEP_LOG_LINE@json.output@ \"has_review_started\": false, @@@", "@@@STEP_LOG_LINE@json.output@ \"project\": \"chromium/src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"commit\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"message\": \"Change commit message\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", "@@@STEP_LOG_LINE@json.output@ \"status\": \"NEW\", @@@", "@@@STEP_LOG_LINE@json.output@ \"subject\": \"Change title\"@@@", "@@@STEP_LOG_LINE@json.output@ }@@@", @@ -162,9 +226,57 @@ "@@@STEP_EXCEPTION@@@" ] }, + { + "cmd": [ + "python", + "-u", + "RECIPE_PACKAGE_REPO[depot_tools]/gerrit_client.py", + "changes", + "--host", + "https://chromium-review.googlesource.com", + "--json_file", + "/path/to/tmp/json", + "--limit", + "1", + "-p", + "change=123", + "-o", + "ALL_REVISIONS", + "-o", + "ALL_COMMITS" + ], + "env": { + "PATH": ":RECIPE_PACKAGE_REPO[depot_tools]" + }, + "infra_step": true, + "name": "gerrit changes (3)", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@json.output@[@@@", + "@@@STEP_LOG_LINE@json.output@ {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"91827\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"branch\": \"master\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"change_id\": \"Ideadbeef\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"created\": \"2017-01-30 13:11:20.000000000\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"has_review_started\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"project\": \"chromium/src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"commit\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"message\": \"Change commit message\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"status\": \"NEW\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"subject\": \"Change title\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@]@@@", + "@@@STEP_LOG_END@json.output@@@" + ] + }, { "name": "$result", - "reason": "Error quering for branch of CL 123", + "reason": "2 out of 2 aggregated steps failed. Failures: Error quering for branch of CL 123, Error querying for CL description: host:'https://chromium-review.googlesource.com' change:123; patchset:3", "recipe_result": null, "status_code": 1 } diff --git a/recipes/recipe_modules/gerrit/examples/full.py b/recipes/recipe_modules/gerrit/examples/full.py index ecab1e89a..5b7d8a473 100644 --- a/recipes/recipe_modules/gerrit/examples/full.py +++ b/recipes/recipe_modules/gerrit/examples/full.py @@ -3,7 +3,8 @@ # found in the LICENSE file. DEPS = [ - 'gerrit' + 'gerrit', + 'recipe_engine/step', ] @@ -32,10 +33,17 @@ def RunSteps(api): limit=1, ) + api.gerrit.get_change_description( + host, change=123, patchset=1) + api.gerrit.get_change_destination_branch(host, change=123) - api.gerrit.get_change_destination_branch( - host, change=123, name='missing_cl') + with api.step.defer_results(): + api.gerrit.get_change_destination_branch( + host, change=123, name='missing_cl') + + api.gerrit.get_change_description( + host, change=123, patchset=3) def GenTests(api): diff --git a/recipes/recipe_modules/gerrit/test_api.py b/recipes/recipe_modules/gerrit/test_api.py index 9b1be1c05..56969f145 100644 --- a/recipes/recipe_modules/gerrit/test_api.py +++ b/recipes/recipe_modules/gerrit/test_api.py @@ -36,6 +36,14 @@ class GerritTestApi(recipe_test_api.RecipeTestApi): 'has_review_started': False, 'branch': 'master', 'subject': 'Change title', + 'revisions': { + '184ebe53805e102605d11f6b143486d15c23a09c': { + '_number': '1', + 'commit': { + 'message': 'Change commit message', + }, + }, + }, }, ]) diff --git a/recipes/recipe_modules/git_cl/api.py b/recipes/recipe_modules/git_cl/api.py index ff6d7684e..47ed09505 100644 --- a/recipes/recipe_modules/git_cl/api.py +++ b/recipes/recipe_modules/git_cl/api.py @@ -20,20 +20,23 @@ class GitClApi(recipe_api.RecipeApi): name, [self.package_repo_resource('git_cl.py'), subcmd] + args, **kwargs) - def get_description(self, patch=None, codereview=None, **kwargs): + def get_description(self, patch_url=None, codereview=None, **kwargs): + """DEPRECATED. Consider using gerrit.get_change_description instead.""" args = ['-d'] - if patch or codereview: - assert patch and codereview, "Both patch and codereview must be provided" + if patch_url or codereview: + assert patch_url and codereview, ( + 'Both patch_url and codereview must be provided') args.append('--%s' % codereview) - args.append(patch) + args.append(patch_url) return self('description', args, stdout=self.m.raw_io.output(), **kwargs) - def set_description(self, description, patch=None, codereview=None, **kwargs): + def set_description(self, description, patch_url=None, codereview=None, **kwargs): args = ['-n', '-'] - if patch or codereview: - assert patch and codereview, "Both patch and codereview must be provided" - args.append(patch) + if patch_url or codereview: + assert patch_url and codereview, ( + 'Both patch_url and codereview must be provided') + args.append(patch_url) args.append('--%s' % codereview) return self( diff --git a/recipes/recipe_modules/git_cl/examples/full.py b/recipes/recipe_modules/git_cl/examples/full.py index 15c872132..d41a0affe 100644 --- a/recipes/recipe_modules/git_cl/examples/full.py +++ b/recipes/recipe_modules/git_cl/examples/full.py @@ -18,9 +18,11 @@ def RunSteps(api): api.git_cl.upload("Do the thing foobar\nNow with emoji: 😄") api.git_cl.issue() result = api.git_cl.get_description( - patch='https://code.review/123', codereview='rietveld', suffix='build') + patch_url='https://code.review/123', + codereview='rietveld', + suffix='build') api.git_cl.set_description( - 'bammmm', patch='https://code.review/123', codereview='rietveld') + 'bammmm', patch_url='https://code.review/123', codereview='rietveld') api.step('echo', ['echo', result.stdout]) api.git_cl.set_config('basic') diff --git a/recipes/recipe_modules/tryserver/__init__.py b/recipes/recipe_modules/tryserver/__init__.py index a5f38b9c7..22cf172b2 100644 --- a/recipes/recipe_modules/tryserver/__init__.py +++ b/recipes/recipe_modules/tryserver/__init__.py @@ -3,6 +3,7 @@ # found in the LICENSE file. DEPS = [ + 'gerrit', 'git', 'git_cl', 'recipe_engine/context', diff --git a/recipes/recipe_modules/tryserver/api.py b/recipes/recipe_modules/tryserver/api.py index 21867887b..89f68b7b7 100644 --- a/recipes/recipe_modules/tryserver/api.py +++ b/recipes/recipe_modules/tryserver/api.py @@ -264,17 +264,19 @@ class TryserverApi(recipe_api.RecipeApi): git-footers documentation for more information. """ if patch_text is None: - codereview = None - if not self.can_apply_issue: #pragma: no cover - raise recipe_api.StepFailure("Cannot get tags from gerrit yet.") - else: - codereview = 'rietveld' - patch = ( - self.m.properties['rietveld'].strip('/') + '/' + + if self.is_gerrit_issue: + patch_text = self.m.gerrit.get_change_description( + self.m.properties['patch_gerrit_url'], + self.m.properties['patch_issue'], + self.m.properties['patch_set']) + elif self.can_apply_issue: + patch_url = ( + self.m.properties['rietveld'].rstrip('/') + '/' + str(self.m.properties['issue'])) - - patch_text = self.m.git_cl.get_description( - patch=patch, codereview=codereview).stdout + patch_text = self.m.git_cl.get_description( + patch_url=patch_url, codereview='rietveld').stdout + else: # pragma: no cover + raise recipe_api.StepFailure('Unknown patch storage.') result = self.m.python( 'parse description', self.package_repo_resource('git_footers.py'), @@ -286,3 +288,5 @@ class TryserverApi(recipe_api.RecipeApi): """Gets a specific tag from a CL description""" return self.get_footers(patch_text).get(tag, []) + def normalize_footer_name(self, footer): + return '-'.join([ word.title() for word in footer.strip().split('-') ]) diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json index 83e52a3ce..cf4e40ad8 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json @@ -1,39 +1,55 @@ [ - { - "cmd": [ - "git", - "diff", - "--cached", - "--name-only" - ], - "cwd": "[START_DIR]", - "infra_step": true, - "name": "git diff to analyze patch", - "stdout": "/path/to/tmp/", - "~followup_annotations": [ - "@@@STEP_LOG_LINE@files@foo.cc@@@", - "@@@STEP_LOG_END@files@@@", - "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", - "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" - ] - }, { "cmd": [ "python", "-u", - "import sys; sys.exit(1)" + "RECIPE_PACKAGE_REPO[depot_tools]/gerrit_client.py", + "changes", + "--host", + "https://chromium-review.googlesource.com", + "--json_file", + "/path/to/tmp/json", + "--limit", + "1", + "-p", + "change=456789", + "-o", + "ALL_REVISIONS", + "-o", + "ALL_COMMITS" ], - "name": "fail", + "env": { + "PATH": ":RECIPE_PACKAGE_REPO[depot_tools]" + }, + "infra_step": true, + "name": "gerrit changes", "~followup_annotations": [ - "step returned non-zero exit code: 1", - "@@@STEP_TEXT@foo@@@", - "@@@STEP_FAILURE@@@", - "@@@SET_BUILD_PROPERTY@failure_hash@\"c2311ad770732eade3d2f48247abd147e40a70e7\"@@@" + "@@@STEP_LOG_LINE@json.output@[@@@", + "@@@STEP_LOG_LINE@json.output@ {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"91827\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"branch\": \"master\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"change_id\": \"Ideadbeef\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"created\": \"2017-01-30 13:11:20.000000000\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"has_review_started\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"project\": \"chromium/src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"184ebe53805e102605d11f6b143486d15c23a09c\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"_number\": \"1\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"commit\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"message\": \"Change commit message\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"status\": \"NEW\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"subject\": \"Change title\"@@@", + "@@@STEP_LOG_LINE@json.output@ }@@@", + "@@@STEP_LOG_LINE@json.output@]@@@", + "@@@STEP_LOG_END@json.output@@@" ] }, { "name": "$result", - "reason": "Step('fail') failed with return_code 1", + "reason": "Error querying for CL description: host:'https://chromium-review.googlesource.com' change:456789; patchset:12", "recipe_result": null, "status_code": 1 } diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_deprecated.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_deprecated.json deleted file mode 100644 index 83e52a3ce..000000000 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_deprecated.json +++ /dev/null @@ -1,40 +0,0 @@ -[ - { - "cmd": [ - "git", - "diff", - "--cached", - "--name-only" - ], - "cwd": "[START_DIR]", - "infra_step": true, - "name": "git diff to analyze patch", - "stdout": "/path/to/tmp/", - "~followup_annotations": [ - "@@@STEP_LOG_LINE@files@foo.cc@@@", - "@@@STEP_LOG_END@files@@@", - "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", - "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" - ] - }, - { - "cmd": [ - "python", - "-u", - "import sys; sys.exit(1)" - ], - "name": "fail", - "~followup_annotations": [ - "step returned non-zero exit code: 1", - "@@@STEP_TEXT@foo@@@", - "@@@STEP_FAILURE@@@", - "@@@SET_BUILD_PROPERTY@failure_hash@\"c2311ad770732eade3d2f48247abd147e40a70e7\"@@@" - ] - }, - { - "name": "$result", - "reason": "Step('fail') failed with return_code 1", - "recipe_result": null, - "status_code": 1 - } -] \ No newline at end of file diff --git a/recipes/recipe_modules/tryserver/examples/full.py b/recipes/recipe_modules/tryserver/examples/full.py index b8bce4557..08a149a4d 100644 --- a/recipes/recipe_modules/tryserver/examples/full.py +++ b/recipes/recipe_modules/tryserver/examples/full.py @@ -29,7 +29,7 @@ def RunSteps(api): return api.tryserver.maybe_apply_issue() - if api.tryserver.can_apply_issue: + if api.tryserver.can_apply_issue or api.tryserver.is_gerrit_issue: api.tryserver.get_footers() api.tryserver.get_files_affected_by_patch( api.properties.get('test_patch_root')) @@ -42,6 +42,8 @@ def RunSteps(api): api.tryserver.set_test_failure_tryjob_result() api.tryserver.set_invalid_test_results_tryjob_result() + api.tryserver.normalize_footer_name('Cr-Commit-Position') + with api.tryserver.set_failure_hash(): api.python.failing_step('fail', 'foo') @@ -74,21 +76,6 @@ def GenTests(api): api.properties.tryserver(test_patch_root='sub/project') + description_step) - yield api.test('with_gerrit_patch_deprecated') + api.properties.tryserver( - patch_project='infra/infra', - gerrit='https://chromium-review.googlesource.com', - patch_storage='gerrit', - repository='https://chromium.googlesource.com/infra/infra', - rietveld=None, - **{ - 'event.change.id': 'infra%2Finfra~master~Ideadbeaf', - 'event.change.number': 338811, - 'event.change.url': - 'https://chromium-review.googlesource.com/#/c/338811', - 'event.patchSet.ref': 'refs/changes/11/338811/3', - } - ) - yield (api.test('with_gerrit_patch') + api.properties.tryserver(gerrit_project='infra/infra'))