Handle failures in the bot_update script.

Currently, if the bot_update script fails, then recipe code that makes assumptions about the returned output JSON cause exceptions to be raised. This fixes a couple instances in the code so that the bot_update failure should surface instead of an uncaught exception. Change-Id: I031e613841b03e5eb4f6d9ca7adb5085088a3077 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2405708 Auto-Submit: Garrett Beaty <gbeaty@chromium.org> Reviewed-by: Josip Sokcevic <sokcevic@google.com> Commit-Queue: Josip Sokcevic <sokcevic@google.com>
5 years ago · 71198cd4a2
parent 970bf5ad2c
commit 71198cd4a2
5 changed files with 118 additions and 5 deletions
--- a/recipes/README.recipes.md
+++ b/recipes/README.recipes.md
@ -58,7 +58,7 @@ Recipe module to ensure a checkout is consistent on a bot.

 Wrapper for easy calling of bot_update.

-&mdash; **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#518)(self, bot_update_step):**
+&mdash; **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#519)(self, bot_update_step):**

 Deapplies a patch, taking care of DEPS and solution revisions properly.
    
@ -89,7 +89,7 @@ Args:
  step_test_data: a null function that returns test bot_update.py output.
    Use test_api.output_json to generate test data.

-&mdash; **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#495)(self, project_name, gclient_config=None):**
+&mdash; **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#496)(self, project_name, gclient_config=None):**

 Returns all property names used for storing the checked-out revision of
 a given project.
@ -107,7 +107,7 @@ Returns (list of str): All properties that'll hold the checked-out revision

 &emsp; **@property**<br>&mdash; **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#44)(self):**

-&mdash; **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#445)(self, bot_update_json, name):**
+&mdash; **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#446)(self, bot_update_json, name):**

 Set a fixed revision for a single dependency using project revision
 properties.
--- a/recipes/recipe_modules/bot_update/api.py
+++ b/recipes/recipe_modules/bot_update/api.py
@ -320,7 +320,7 @@ class BotUpdateApi(recipe_api.RecipeApi):
          step_text = result['step_text']
          step_result.presentation.step_text = step_text

-        if add_blamelists:
+        if add_blamelists and 'manifest' in result:
          blamelist_pins = []
          for name in revisions:
            m = result['manifest'][name]
@ -333,7 +333,8 @@ class BotUpdateApi(recipe_api.RecipeApi):
          self.m.milo.show_blamelist_for(blamelist_pins)

        # Set output commit of the build.
-        if set_output_commit:
+        if (set_output_commit and
+            'got_revision' in self._last_returned_properties):
          # As of April 2019, got_revision describes the output commit,
          # the same commit that Build.output.gitiles_commit describes.
          # In particular, users tend to set got_revision to make Milo display
--- a/recipes/recipe_modules/bot_update/examples/full.expected/add_blamelists_bot_update_failure.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/add_blamelists_bot_update_failure.json
@ -0,0 +1,49 @@
+[
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py",
+      "--spec-path",
+      "cache_dir = '[CACHE]/git'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]",
+      "--revision_mapping_file",
+      "{\"got_angle_revision\": \"src/third_party/angle\", \"got_cr_revision\": \"src\", \"got_revision\": \"src\", \"got_v8_revision\": \"src/v8\"}",
+      "--git-cache-dir",
+      "[CACHE]/git",
+      "--cleanup-dir",
+      "[CLEANUP]/bot_update",
+      "--output_json",
+      "/path/to/tmp/json",
+      "--revision",
+      "src@2d72510e447ab60a9728aeea2362d8be2cbd7789",
+      "--revision",
+      "src/v8@HEAD",
+      "--refs",
+      "refs/heads/master",
+      "--disable-syntax-validation"
+    ],
+    "env": {
+      "GIT_HTTP_LOW_SPEED_LIMIT": "102400",
+      "GIT_HTTP_LOW_SPEED_TIME": "300"
+    },
+    "env_suffixes": {
+      "DEPOT_TOOLS_UPDATE": [
+        "0"
+      ],
+      "PATH": [
+        "RECIPE_REPO[depot_tools]"
+      ]
+    },
+    "infra_step": true,
+    "name": "bot_update (without patch)",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@json.output@{@@@",
+      "@@@STEP_LOG_LINE@json.output@  \"did_run\": true@@@",
+      "@@@STEP_LOG_LINE@json.output@}@@@",
+      "@@@STEP_LOG_END@json.output@@@"
+    ]
+  },
+  {
+    "name": "$result"
+  }
+]
--- a/recipes/recipe_modules/bot_update/examples/full.expected/bot_update_failure.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/bot_update_failure.json
@ -0,0 +1,47 @@
+[
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py",
+      "--spec-path",
+      "cache_dir = '[CACHE]/git'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]",
+      "--revision_mapping_file",
+      "{\"got_angle_revision\": \"src/third_party/angle\", \"got_cr_revision\": \"src\", \"got_revision\": \"src\", \"got_v8_revision\": \"src/v8\"}",
+      "--git-cache-dir",
+      "[CACHE]/git",
+      "--cleanup-dir",
+      "[CLEANUP]/bot_update",
+      "--output_json",
+      "/path/to/tmp/json",
+      "--revision",
+      "src@2d72510e447ab60a9728aeea2362d8be2cbd7789",
+      "--refs",
+      "refs/heads/master",
+      "--disable-syntax-validation"
+    ],
+    "env": {
+      "GIT_HTTP_LOW_SPEED_LIMIT": "102400",
+      "GIT_HTTP_LOW_SPEED_TIME": "300"
+    },
+    "env_suffixes": {
+      "DEPOT_TOOLS_UPDATE": [
+        "0"
+      ],
+      "PATH": [
+        "RECIPE_REPO[depot_tools]"
+      ]
+    },
+    "infra_step": true,
+    "name": "bot_update (without patch)",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@json.output@{@@@",
+      "@@@STEP_LOG_LINE@json.output@  \"did_run\": true@@@",
+      "@@@STEP_LOG_LINE@json.output@}@@@",
+      "@@@STEP_LOG_END@json.output@@@"
+    ]
+  },
+  {
+    "name": "$result"
+  }
+]
--- a/recipes/recipe_modules/bot_update/examples/full.py
+++ b/recipes/recipe_modules/bot_update/examples/full.py
@ -119,6 +119,12 @@ def GenTests(api):
      api.test('unrecognized_commit_repo') +
      ci_build(git_repo='https://unrecognized/repo')
  )
+  yield (
+      api.test('bot_update_failure') +
+      ci_build() +
+      api.properties(bot_update_output={'did_run': True})
+  )
+
  yield (
      api.test('basic_luci') +
      ci_build() +
@ -246,6 +252,16 @@ def GenTests(api):
      )
  )

+  yield (
+      api.test('add_blamelists_bot_update_failure') +
+      ci_build() +
+      api.properties(
+          add_blamelists=True,
+          bot_update_output={'did_run': True},
+          revisions={'src/v8': 'HEAD'},
+      )
+  )
+
  yield (
      api.test('no_cp_checkout_a_specific_commit') +
      ci_build(revision='a' * 40) +