From fd81abb19d7cded72715ffdc16eb80640a782b2f Mon Sep 17 00:00:00 2001 From: Garrett Beaty Date: Fri, 10 May 2024 17:42:14 +0000 Subject: [PATCH] Return a custom result type from bot_update.ensure_checkout. In order to facilitate removing uses of api.path.checkout_dir from downstream repos, this change adds a custom return type for bot_update.ensure_checkout. Now instead of a standard step result, an object of Result will be returned. Result records the relevant paths (directory where the checkout was performed, the repo that was checked out and the repo that was patched, if any). This provides the caller the ability to work in any of these directories without using api.path.checkout_dir and without requiring boilerplate to construct the paths. It also includes some attributes that provide details from within the json output to abstract that out. Bug: 329113288, 339472834 Change-Id: I2ec6db635c5b799bdb65d4e9364e7d99aae4159e Recipe-Manual-Change: build Recipe-Manual-Change: build_limited Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5523194 Reviewed-by: Yiwei Zhang Commit-Queue: Garrett Beaty --- recipes/README.recipes.md | 28 +++-- recipes/recipe.warnings | 36 ++++++ recipes/recipe_modules/bot_update/__init__.py | 4 + recipes/recipe_modules/bot_update/api.py | 113 ++++++++++++++++-- .../bot_update/examples/full.py | 6 + .../ensure_checkout_return_custom_result.py | 76 ++++++++++++ recipes/recipe_modules/presubmit/api.py | 3 +- 7 files changed, 244 insertions(+), 22 deletions(-) create mode 100644 recipes/recipe.warnings create mode 100644 recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index ac1828b82..694c931b0 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -21,6 +21,7 @@ * [bot_update:tests/do_not_retry_patch_failures_in_cq](#recipes-bot_update_tests_do_not_retry_patch_failures_in_cq) * [bot_update:tests/download_topics](#recipes-bot_update_tests_download_topics) * [bot_update:tests/ensure_checkout](#recipes-bot_update_tests_ensure_checkout) + * [bot_update:tests/ensure_checkout_return_custom_result](#recipes-bot_update_tests_ensure_checkout_return_custom_result) * [depot_tools:examples/full](#recipes-depot_tools_examples_full) * [gclient:examples/full](#recipes-gclient_examples_full) * [gclient:tests/diff_deps](#recipes-gclient_tests_diff_deps) @@ -51,23 +52,23 @@ ### *recipe_modules* / [bot\_update](/recipes/recipe_modules/bot_update) -[DEPS](/recipes/recipe_modules/bot_update/__init__.py#3): [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [gitiles](#recipe_modules-gitiles), [gsutil](#recipe_modules-gsutil), [tryserver](#recipe_modules-tryserver), [recipe\_engine/archive][recipe_engine/recipe_modules/archive], [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/commit\_position][recipe_engine/recipe_modules/commit_position], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cv][recipe_engine/recipe_modules/cv], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/led][recipe_engine/recipe_modules/led], [recipe\_engine/milo][recipe_engine/recipe_modules/milo], [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/runtime][recipe_engine/recipe_modules/runtime], [recipe\_engine/step][recipe_engine/recipe_modules/step] +[DEPS](/recipes/recipe_modules/bot_update/__init__.py#3): [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [gitiles](#recipe_modules-gitiles), [gsutil](#recipe_modules-gsutil), [tryserver](#recipe_modules-tryserver), [recipe\_engine/archive][recipe_engine/recipe_modules/archive], [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/commit\_position][recipe_engine/recipe_modules/commit_position], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cv][recipe_engine/recipe_modules/cv], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/led][recipe_engine/recipe_modules/led], [recipe\_engine/milo][recipe_engine/recipe_modules/milo], [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/runtime][recipe_engine/recipe_modules/runtime], [recipe\_engine/step][recipe_engine/recipe_modules/step], [recipe\_engine/warning][recipe_engine/recipe_modules/warning] Recipe module to ensure a checkout is consistent on a bot. -#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#12)([RecipeApi][recipe_engine/wkt/RecipeApi]):** +#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#93)([RecipeApi][recipe_engine/wkt/RecipeApi]):** -— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#20)(self, name, cmd, \*\*kwargs):** +— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#101)(self, name, cmd, \*\*kwargs):** Wrapper for easy calling of bot_update. -— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#599)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#693)(self, bot_update_result):** Deapplies a patch, taking care of DEPS and solution revisions properly. -— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#128)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, \*\*kwargs):** +— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#209)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, \*\*kwargs):** Args: * gclient_config: The gclient configuration to use when running bot_update. @@ -103,7 +104,7 @@ Args: change's commit message to get this revision override requested by the author. -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#576)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#670)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -117,9 +118,9 @@ Args: Returns (list of str): All properties that'll hold the checked-out revision of the given project. An empty list if no such properties exist. -  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#39)(self):** +  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#120)(self):** -— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#527)(self, bot_update_json, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#621)(self, bot_update_json, name):** Sets a fixed revision for a single dependency using project revision properties. @@ -1031,10 +1032,10 @@ Raises: ### *recipes* / [bot\_update:examples/full](/recipes/recipe_modules/bot_update/examples/full.py) -[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#9): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [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/runtime][recipe_engine/recipe_modules/runtime] +[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#10): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [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/runtime][recipe_engine/recipe_modules/runtime] -— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#27)(api):** +  **@recipe_api.ignore_warnings('^depot_tools/BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES$')**
— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#32)(api):** ### *recipes* / [bot\_update:tests/do\_not\_retry\_patch\_failures\_in\_cq](/recipes/recipe_modules/bot_update/tests/do_not_retry_patch_failures_in_cq.py) [DEPS](/recipes/recipe_modules/bot_update/tests/do_not_retry_patch_failures_in_cq.py#9): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/cv][recipe_engine/recipe_modules/cv], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/step][recipe_engine/recipe_modules/step] @@ -1053,6 +1054,12 @@ Raises: — **def [RunSteps](/recipes/recipe_modules/bot_update/tests/ensure_checkout.py#16)(api):** +### *recipes* / [bot\_update:tests/ensure\_checkout\_return\_custom\_result](/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py) + +[DEPS](/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py#10): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [recipe\_engine/assertions][recipe_engine/recipe_modules/assertions], [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] + + +  **@recipe_api.ignore_warnings('^depot_tools/BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES$')**
— **def [RunSteps](/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py#29)(api, expected_checkout_dir, expected_source_root_name, expected_patch_root_name):** ### *recipes* / [depot\_tools:examples/full](/recipes/recipe_modules/depot_tools/examples/full.py) [DEPS](/recipes/recipe_modules/depot_tools/examples/full.py#7): [depot\_tools](#recipe_modules-depot_tools), [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime], [recipe\_engine/step][recipe_engine/recipe_modules/step] @@ -1234,4 +1241,5 @@ Move things around in a loop! [recipe_engine/recipe_modules/time]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/61d032078b9e8f1a9a512bbaed668224deedca9d/README.recipes.md#recipe_modules-time [recipe_engine/recipe_modules/url]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/61d032078b9e8f1a9a512bbaed668224deedca9d/README.recipes.md#recipe_modules-url [recipe_engine/recipe_modules/version]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/61d032078b9e8f1a9a512bbaed668224deedca9d/README.recipes.md#recipe_modules-version +[recipe_engine/recipe_modules/warning]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/61d032078b9e8f1a9a512bbaed668224deedca9d/README.recipes.md#recipe_modules-warning [recipe_engine/wkt/RecipeApi]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/61d032078b9e8f1a9a512bbaed668224deedca9d/recipe_engine/recipe_api.py#433 diff --git a/recipes/recipe.warnings b/recipes/recipe.warnings new file mode 100644 index 000000000..33080dd4c --- /dev/null +++ b/recipes/recipe.warnings @@ -0,0 +1,36 @@ +google_issue_default { + host: "crbug.com" +} +warning { + name: "BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES" + description: "The custom result type returned from" + description: "`bot_update.ensure_checkout` provides attributes that give easy" + description: "access to the information that is often accessed via the" + description: "existing step result's `presentation` or `json.output` values." + description: "These uses should be updated as follows:" + description: "" + description: "1) `result.presentation.properties` should be replaced with" + description: "`result.properties`." + description: "" + description: "2) `result.json.output['properties']`," + description: "`result.json.output['manifest']` and" + description: "`result.json.output['fixed_revisions']` should be replaced" + description: "with `result.properties`, `result.manifest` and" + description: "`result.fixed_revisions`, respectively." + description: "" + description: "3) Getting the source root and the patch root should no longer" + description: "be done via `result.json.output['root']` and" + description: "`result.json.output['patch_root']`. `result.source_root` and" + description: "`result.patch_root` should be used instead. These objects have" + description: "`path` and `name` attributes. The `path` attribute gives a" + description: "`Path` for the location of the repo. The `name` attribute is" + description: "the gclient \"name\" of the repo, i.e. the path relative to the" + description: "directory where the checkout was performed. This removes the" + description: "need to do path manipulation in the common cases. If no patch" + description: "was applied, `result.patch_root` will be `None`." + description: "" + description: "4) `result.json.output['did_run']` does not need to be" + description: "accessed, it is always guaranteed to be True if a result was" + description: "returned." + google_issue { id: 339472834 } +} \ No newline at end of file diff --git a/recipes/recipe_modules/bot_update/__init__.py b/recipes/recipe_modules/bot_update/__init__.py index 4b3fa114b..74efdbc11 100644 --- a/recipes/recipe_modules/bot_update/__init__.py +++ b/recipes/recipe_modules/bot_update/__init__.py @@ -20,6 +20,7 @@ DEPS = [ 'recipe_engine/raw_io', 'recipe_engine/runtime', 'recipe_engine/step', + 'recipe_engine/warning', 'tryserver', ] @@ -36,3 +37,6 @@ PROPERTIES = { default={}, ), } + +# Forward these types so that they can be used without importing api +from .api import RelativeRoot, Result \ No newline at end of file diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 5e34318f3..5961cebc9 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -4,11 +4,92 @@ """Recipe module to ensure a checkout is consistent on a bot.""" +import dataclasses +import typing + from recipe_engine import recipe_api +from recipe_engine.config_types import Path +from recipe_engine.engine_types import StepPresentation from PB.go.chromium.org.luci.buildbucket.proto import common as common_pb2 +@dataclasses.dataclass(kw_only=True, frozen=True) +class RelativeRoot: + """A root that is relative to the checkout root. + + Attributes: + name: The name of the root/the path to the root relative to the checkout + directory. + path: The absolute path to the root. + """ + name: str + path: Path + + @classmethod + def create(cls, checkout_dir: Path, name: str) -> typing.Self: + return cls(name=name, path=checkout_dir / name) + + +@dataclasses.dataclass(kw_only=True, frozen=True) +class Json: + output: dict[str, typing.Any] + + +class ManifestRepo(typing.TypedDict): + repository: str + revision: str + + +@dataclasses.dataclass(kw_only=True, frozen=True) +class Result: + """The noteworthy paths for a source checkout. + + Attributes: + checkout_dir: The directory where the checkout was performed. + source_root: The root for the repo identified by the first gclient solution. + patch_root: The root for the repo that was patched, if a patch was applied. + Otherwise, None. + presentation: DEPRECATED. The presentation of the bot_update step. This is + used by some code to get the properties. This is provided for backwards + compatibility, code should access the properties attribute instead. + json: DEPRECATED. The result of json outputs for the bot_update step. This + is provided for backwards compatibility, attributes on this object are + provided for accessing the contents of json.output. + properties: The properties set by the bot_update execution. + manifest: The manifest mapping the checkout_dir-relative path to the + repository and revision that was checked out. + fixed_revisions: The explicitly requested revisions; a mapping from the + checkout_dir-relative path to the requested revision. + """ + # Directories relevant to the checkout + checkout_dir: Path + source_root: RelativeRoot + patch_root: RelativeRoot | None = None + + # Details about the revisions that were checked out + properties: dict[str, str] + manifest: dict[str, ManifestRepo] + fixed_revisions: dict[str, str] + + # TODO: crbug.com/339472834 - Once all downstream users are switched to use + # the above fields, these attributes and the property methods can be removed, + # as well as the Json type + _api: 'BotUpdateApi' + _presentation: StepPresentation + _json: Json + + @property + def presentation(self): + self._api.m.warning.issue('BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES') + return self._presentation + + @property + def json(self): + self._api.m.warning.issue('BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES') + return self._json + + class BotUpdateApi(recipe_api.RecipeApi): def __init__(self, properties, deps_revision_overrides, *args, **kwargs): @@ -495,7 +576,20 @@ class BotUpdateApi(recipe_api.RecipeApi): cwd = self.m.context.cwd or self.m.path.start_dir self.m.path.checkout_dir = cwd / co_root - return step_result + assert result.get('did_run') and result.get('root') + checkout_dir = self.m.context.cwd or self.m.path.start_dir + return Result( + checkout_dir=checkout_dir, + source_root=RelativeRoot.create(checkout_dir, result['root']), + patch_root=(RelativeRoot.create(checkout_dir, result['patch_root']) + if result['patch_root'] is not None else None), + properties=result.get('properties', {}), + manifest=result.get('manifest', {}), + fixed_revisions=result.get('fixed_revisions', {}), + _api=self, + _presentation=step_result.presentation, + _json=Json(output=result), + ) def _destination_ref(self, cfg, path): """Returns the ref branch of a CL for the matching project if available or @@ -533,7 +627,7 @@ class BotUpdateApi(recipe_api.RecipeApi): name: bot_update_json['properties'][rev_properties[0]] } - def _resolve_fixed_revisions(self, bot_update_json): + def _resolve_fixed_revisions(self, bot_update_result): """Sets all fixed revisions from the first sync to their respective got_X_revision values. @@ -564,12 +658,12 @@ class BotUpdateApi(recipe_api.RecipeApi): bot_update.py --revision src@abc When deapplying the patch, v8 will be synced to v8_before. """ - for name in bot_update_json.get('fixed_revisions', {}): + for name in bot_update_result.fixed_revisions: rev_properties = self.get_project_revision_properties(name) - if (rev_properties and - bot_update_json['properties'].get(rev_properties[0])): + if (rev_properties + and bot_update_result.properties.get(rev_properties[0])): self.m.gclient.c.revisions[name] = str( - bot_update_json['properties'][rev_properties[0]]) + bot_update_result.properties[rev_properties[0]]) # TODO(machenbach): Replace usages of this method eventually by direct calls # to the manifest output. @@ -596,10 +690,9 @@ class BotUpdateApi(recipe_api.RecipeApi): if project == project_name ) - def deapply_patch(self, bot_update_step): + def deapply_patch(self, bot_update_result): """Deapplies a patch, taking care of DEPS and solution revisions properly. """ - bot_update_json = bot_update_step.json.output # We only override first solution here to make sure that we correctly revert # changes to DEPS file, which is particularly important for auto-rolls. It # is also imporant that we do not assume that corresponding revision is @@ -608,8 +701,8 @@ class BotUpdateApi(recipe_api.RecipeApi): first_solution_name = self.m.gclient.c.solutions[0].name rev_property = self.get_project_revision_properties(first_solution_name)[0] self.m.gclient.c.revisions[first_solution_name] = str( - bot_update_json['properties'][rev_property]) - self._resolve_fixed_revisions(bot_update_json) + bot_update_result.properties[rev_property]) + self._resolve_fixed_revisions(bot_update_result) self.ensure_checkout( patch=False, no_fetch_tags=True, update_presentation=False) diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index 826956a16..566ba8317 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -3,6 +3,7 @@ # found in the LICENSE file. from recipe_engine import post_process +from recipe_engine import recipe_api PYTHON_VERSION_COMPATIBILITY = 'PY3' @@ -24,6 +25,11 @@ from recipe_engine import engine_types from RECIPE_MODULES.depot_tools import gclient from PB.go.chromium.org.luci.buildbucket.proto.build import Build + +# TODO: crbug.com/339472834 - Once all downstream uses of presentation and +# json.output have been removed, this test can be updated to not reference them +# and the decorator can be removed +@recipe_api.ignore_warnings('^depot_tools/BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES$') def RunSteps(api): api.gclient.use_mirror = True commit = api.buildbucket.build.input.gitiles_commit diff --git a/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py b/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py new file mode 100644 index 000000000..c2b2fe586 --- /dev/null +++ b/recipes/recipe_modules/bot_update/tests/ensure_checkout_return_custom_result.py @@ -0,0 +1,76 @@ +# Copyright 2018 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +from recipe_engine import post_process +from recipe_engine import recipe_api + +PYTHON_VERSION_COMPATIBILITY = 'PY3' + +DEPS = [ + 'bot_update', + 'gclient', + 'recipe_engine/assertions', + 'recipe_engine/buildbucket', + 'recipe_engine/path', + 'recipe_engine/properties', +] + +PROPERTIES = { + 'expected_checkout_dir': recipe_api.Property(), + 'expected_source_root_name': recipe_api.Property(), + 'expected_patch_root_name': recipe_api.Property(default=None), +} + + +# TODO: crbug.com/339472834 - Once all downstream uses of presentation and +# json.output have been removed, this test can be updated to not reference them +# and the decorator can be removed +@recipe_api.ignore_warnings('^depot_tools/BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES$') +def RunSteps(api, expected_checkout_dir, expected_source_root_name, + expected_patch_root_name): + api.gclient.set_config('depot_tools') + result = api.bot_update.ensure_checkout() + + api.assertions.assertEqual(result.checkout_dir, expected_checkout_dir) + + api.assertions.assertEqual(result.source_root.name, expected_source_root_name) + api.assertions.assertEqual(result.source_root.path, + expected_checkout_dir / expected_source_root_name) + + if expected_patch_root_name is not None: + api.assertions.assertEqual(result.patch_root.name, expected_patch_root_name) + api.assertions.assertEqual(result.patch_root.path, + expected_checkout_dir / expected_patch_root_name) + else: + api.assertions.assertIsNone(result.patch_root) + + api.assertions.assertEqual(result.properties, result.presentation.properties) + api.assertions.assertEqual(result.manifest, + result.json.output.get('manifest', {})) + api.assertions.assertEqual(result.fixed_revisions, + result.json.output.get('fixed_revisions', {})) + + +def GenTests(api): + yield api.test( + 'basic', + api.properties( + expected_checkout_dir=api.path.start_dir, + expected_source_root_name='depot_tools', + ), + api.expect_status('SUCCESS'), + api.post_process(post_process.DropExpectation), + ) + + yield api.test( + 'patch', + api.buildbucket.try_build(), + api.properties( + expected_checkout_dir=api.path.start_dir, + expected_source_root_name='depot_tools', + expected_patch_root_name='depot_tools', + ), + api.expect_status('SUCCESS'), + api.post_process(post_process.DropExpectation), + ) diff --git a/recipes/recipe_modules/presubmit/api.py b/recipes/recipe_modules/presubmit/api.py index e6b015352..1eb6807a4 100644 --- a/recipes/recipe_modules/presubmit/api.py +++ b/recipes/recipe_modules/presubmit/api.py @@ -116,8 +116,7 @@ class PresubmitApi(recipe_api.RecipeApi): # as the delimiter. This breaks on windows otherwise. self._relative_root.replace(self.m.path.sep, '/'), self.m.gclient.c) - upstream = bot_update_step.json.output['properties'].get( - got_revision_properties[0]) + upstream = bot_update_step.properties.get(got_revision_properties[0]) presubmit_args = [] if self.m.tryserver.is_tryserver: