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 <yiwzhang@google.com>
Commit-Queue: Garrett Beaty <gbeaty@google.com>

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.
 
-&emsp; **@property**<br>&mdash; **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#39)(self):**
+&emsp; **@property**<br>&mdash; **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#120)(self):**
 
-&mdash; **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#527)(self, bot_update_json, name):**
+&mdash; **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]
 
 
-&mdash; **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#27)(api):**
+&emsp; **@recipe_api.ignore_warnings('^depot_tools/BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES$')**<br>&mdash; **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:
 
 
 &mdash; **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]
+
+
+&emsp; **@recipe_api.ignore_warnings('^depot_tools/BOT_UPDATE_CUSTOM_RESULT_ATTRIBUTES$')**<br>&mdash; **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

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 }
+}

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

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)

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

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),
+  )

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: