tryserver: add a 10 minute timeout on fetching CL info from gerrit.

Follow-up to crbug.com/970293. Recipe-Nontrivial-Roll: build Recipe-Nontrivial-Roll: build_limited_scripts_slave Recipe-Nontrivial-Roll: infra Recipe-Nontrivial-Roll: skia Change-Id: I2e0f71bc663e0ab571c0b9c98d57b2784366fb18 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1643679 Commit-Queue: John Budorick <jbudorick@chromium.org> Auto-Submit: John Budorick <jbudorick@chromium.org> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
6 years ago · 075cb05bde
parent d2f31cc65d
commit 075cb05bde
17 changed files with 72 additions and 17 deletions
--- a/recipes/README.recipes.md
+++ b/recipes/README.recipes.md
@ -37,6 +37,7 @@
  * [osx_sdk:examples/full](#recipes-osx_sdk_examples_full)
  * [presubmit:examples/full](#recipes-presubmit_examples_full)
  * [tryserver:examples/full](#recipes-tryserver_examples_full)
+  * [tryserver:tests/gerrit_change_fetch_ref](#recipes-tryserver_tests_gerrit_change_fetch_ref)
  * [windows_sdk:examples/full](#recipes-windows_sdk_examples_full)
 ## Recipe Modules

@ -753,7 +754,7 @@ Return a presubmit step.

 #### **class [TryserverApi](/recipes/recipe_modules/tryserver/api.py#11)([RecipeApi][recipe_engine/wkt/RecipeApi]):**

-&mdash; **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#220)(self, reason):**
+&mdash; **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#221)(self, reason):**

 Records a more detailed reason why build is failing.

@ -765,7 +766,7 @@ Returns current gerrit change, if there is exactly one.

 Returns a self.m.buildbucket.common_pb2.GerritChange or None.

-&emsp; **@property**<br>&mdash; **def [gerrit\_change\_fetch\_ref](/recipes/recipe_modules/tryserver/api.py#97)(self):**
+&emsp; **@property**<br>&mdash; **def [gerrit\_change\_fetch\_ref](/recipes/recipe_modules/tryserver/api.py#98)(self):**

 Returns gerrit patch ref, e.g. "refs/heads/45/12345/6, or None.

@ -777,13 +778,13 @@ Returns canonical URL of the gitiles repo of the current Gerrit CL.

 Populated iff gerrit_change is populated.

-&emsp; **@property**<br>&mdash; **def [gerrit\_change\_target\_ref](/recipes/recipe_modules/tryserver/api.py#106)(self):**
+&emsp; **@property**<br>&mdash; **def [gerrit\_change\_target\_ref](/recipes/recipe_modules/tryserver/api.py#107)(self):**

 Returns gerrit change destination ref, e.g. "refs/heads/master".

 Populated iff gerrit_change is populated.

-&mdash; **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#136)(self, patch_root, \*\*kwargs):**
+&mdash; **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#137)(self, patch_root, \*\*kwargs):**

 Returns list of paths to files affected by the patch.

@ -793,11 +794,11 @@ Argument:

 Returned paths will be relative to to patch_root.

-&mdash; **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#278)(self, tag, patch_text=None):**
+&mdash; **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#279)(self, tag, patch_text=None):**

 Gets a specific tag from a CL description

-&mdash; **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#258)(self, patch_text=None):**
+&mdash; **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#259)(self, patch_text=None):**

 Retrieves footers from the patch description.

@ -806,30 +807,30 @@ git-footers documentation for more information.

 &mdash; **def [initialize](/recipes/recipe_modules/tryserver/api.py#23)(self):**

-&emsp; **@property**<br>&mdash; **def [is\_gerrit\_issue](/recipes/recipe_modules/tryserver/api.py#120)(self):**
+&emsp; **@property**<br>&mdash; **def [is\_gerrit\_issue](/recipes/recipe_modules/tryserver/api.py#121)(self):**

 Returns true iff the properties exist to match a Gerrit issue.

-&emsp; **@property**<br>&mdash; **def [is\_patch\_in\_git](/recipes/recipe_modules/tryserver/api.py#130)(self):**
+&emsp; **@property**<br>&mdash; **def [is\_patch\_in\_git](/recipes/recipe_modules/tryserver/api.py#131)(self):**

-&emsp; **@property**<br>&mdash; **def [is\_tryserver](/recipes/recipe_modules/tryserver/api.py#115)(self):**
+&emsp; **@property**<br>&mdash; **def [is\_tryserver](/recipes/recipe_modules/tryserver/api.py#116)(self):**

 Returns true iff we have a change to check out.

-&mdash; **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#282)(self, footer):**
+&mdash; **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#283)(self, footer):**

-&mdash; **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#199)(self):**
+&mdash; **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#200)(self):**

 Mark the tryjob result as a compile failure.

-&mdash; **def [set\_do\_not\_retry\_build](/recipes/recipe_modules/tryserver/api.py#185)(self):**
+&mdash; **def [set\_do\_not\_retry\_build](/recipes/recipe_modules/tryserver/api.py#186)(self):**

 A flag to indicate the build should not be retried by the CQ.

 This mechanism is used to reduce CQ duration when retrying will likely
 return an identical result.

-&emsp; **@contextlib.contextmanager**<br>&mdash; **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#229)(self):**
+&emsp; **@contextlib.contextmanager**<br>&mdash; **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#230)(self):**

 Context manager that sets a failure_hash build property on StepFailure.

@ -838,7 +839,7 @@ for the same reason. For example, if a patch is bad (breaks something),
 we'd expect it to always break in the same way. Different failures
 for the same patch are usually a sign of flakiness.

-&mdash; **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#211)(self):**
+&mdash; **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#212)(self):**

 Mark the tryjob result as having invalid test results.

@ -846,18 +847,18 @@ This means we run some tests, but the results were not valid
 (e.g. no list of specific test cases that failed, or too many
 tests failing, etc).

-&mdash; **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#195)(self):**
+&mdash; **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#196)(self):**

 Mark the tryjob result as failure to apply the patch.

-&mdash; **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#163)(self, subproject_tag):**
+&mdash; **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#164)(self, subproject_tag):**

 Adds a subproject tag to the build.

 This can be used to distinguish between builds that execute different steps
 depending on what was patched, e.g. blink vs. pure chromium patches.

-&mdash; **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#203)(self):**
+&mdash; **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#204)(self):**

 Mark the tryjob result as a test failure.

@ -991,6 +992,11 @@ Move things around in a loop!
 [DEPS](/recipes/recipe_modules/tryserver/examples/full.py#5): [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/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]

 &mdash; **def [RunSteps](/recipes/recipe_modules/tryserver/examples/full.py#19)(api):**
+### *recipes* / [tryserver:tests/gerrit\_change\_fetch\_ref](/recipes/recipe_modules/tryserver/tests/gerrit_change_fetch_ref.py)
+
+[DEPS](/recipes/recipe_modules/tryserver/tests/gerrit_change_fetch_ref.py#7): [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/properties][recipe_engine/recipe_modules/properties]
+
+&mdash; **def [RunSteps](/recipes/recipe_modules/tryserver/tests/gerrit_change_fetch_ref.py#15)(api):**
 ### *recipes* / [windows\_sdk:examples/full](/recipes/recipe_modules/windows_sdk/examples/full.py)

 [DEPS](/recipes/recipe_modules/windows_sdk/examples/full.py#5): [windows\_sdk](#recipe_modules-windows_sdk), [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/step][recipe_engine/recipe_modules/step]
--- a/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/no_apply_patch_on_gclient.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/no_apply_patch_on_gclient.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_angle.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_angle.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_branch_heads.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_branch_heads.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_feature_branch.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_feature_branch.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_v8_feature_branch.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_v8_feature_branch.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8_head_by_default.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8_head_by_default.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/tryserver/api.py
+++ b/recipes/recipe_modules/tryserver/api.py
@ -81,6 +81,7 @@ class TryserverApi(recipe_api.RecipeApi):
        o_params=['ALL_REVISIONS', 'DOWNLOAD_COMMANDS'],
        limit=1,
        name='fetch current CL info',
+        timeout=600,
        step_test_data=lambda: self.m.json.test_api.output(mock_res))[0]

    self._gerrit_change_target_ref = res['branch']
--- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json
+++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json
+++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json
@ -23,6 +23,7 @@
    },
    "infra_step": true,
    "name": "gerrit fetch current CL info",
+    "timeout": 600,
    "~followup_annotations": [
      "@@@STEP_LOG_LINE@json.output@[@@@",
      "@@@STEP_LOG_LINE@json.output@  {@@@",
--- a/recipes/recipe_modules/tryserver/tests/gerrit_change_fetch_ref.py
+++ b/recipes/recipe_modules/tryserver/tests/gerrit_change_fetch_ref.py
@ -0,0 +1,34 @@
+# Copyright 2019 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
+
+DEPS = [
+  'gerrit',
+  'recipe_engine/buildbucket',
+  'recipe_engine/properties',
+  'tryserver',
+]
+
+
+def RunSteps(api):
+  api.tryserver.gerrit_change_fetch_ref
+
+
+def GenTests(api):
+  yield (
+      api.test('timeout') +
+      api.buildbucket.try_build(
+        'chromium',
+        'linux',
+        git_repo='https://chromium.googlesource.com/chromium/src',
+        change_number=91827,
+        patch_set=1) +
+      api.properties(target_ref='refs/heads/master') +
+      api.tryserver.gerrit_change_target_ref('refs/heads/master') +
+      api.override_step_data('gerrit fetch current CL info',
+                             times_out_after=1200) +
+      api.post_process(post_process.StatusException) +
+      api.post_process(post_process.DropExpectation)
+  )