From d94f9a6d20c8607724b344adf214bedbda9d2d03 Mon Sep 17 00:00:00 2001 From: Garrett Beaty Date: Mon, 24 Jan 2022 23:46:17 +0000 Subject: [PATCH] Add a method for enforcing a tryjob. The tryserver.require_is_tryserver method will create an infra-failing step that raises an exception if there is no CL associated with the build. In the case of an LED task, it will only fail so as not to cause the removal of the builder cache in the case of a minor user error. Change-Id: I05d3a86cc727577f067fb3eae49e46398345f672 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413971 Auto-Submit: Garrett Beaty Reviewed-by: Gavin Mak Commit-Queue: Gavin Mak --- recipes/README.recipes.md | 37 +++++++----- recipes/recipe_modules/tryserver/__init__.py | 1 + recipes/recipe_modules/tryserver/api.py | 13 +++++ .../tryserver/tests/require_is_tryserver.py | 58 +++++++++++++++++++ 4 files changed, 96 insertions(+), 13 deletions(-) create mode 100644 recipes/recipe_modules/tryserver/tests/require_is_tryserver.py diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index f371df86f..9eb049133 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -40,6 +40,7 @@ * [tryserver:tests/gerrit_change_fetch_ref_timeout](#recipes-tryserver_tests_gerrit_change_fetch_ref_timeout) (Python3 ✅) * [tryserver:tests/gerrit_change_owner](#recipes-tryserver_tests_gerrit_change_owner) (Python3 ✅) * [tryserver:tests/gerrit_change_target_ref](#recipes-tryserver_tests_gerrit_change_target_ref) (Python3 ✅) + * [tryserver:tests/require_is_tryserver](#recipes-tryserver_tests_require_is_tryserver) (Python3 ✅) * [windows_sdk:examples/full](#recipes-windows_sdk_examples_full) (Python3 ✅) ## Recipe Modules @@ -779,7 +780,7 @@ Returns:   **@property**
— **def [presubmit\_support\_path](/recipes/recipe_modules/presubmit/api.py#24)(self):** ### *recipe_modules* / [tryserver](/recipes/recipe_modules/tryserver) -[DEPS](/recipes/recipe_modules/tryserver/__init__.py#7): [gerrit](#recipe_modules-gerrit), [git](#recipe_modules-git), [git\_cl](#recipe_modules-git_cl), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [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#7): [gerrit](#recipe_modules-gerrit), [git](#recipe_modules-git), [git\_cl](#recipe_modules-git_cl), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/led][recipe_engine/recipe_modules/led], [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] PYTHON_VERSION_COMPATIBILITY: PY2+3 @@ -844,7 +845,7 @@ Returns gerrit change patchset, e.g. 6 for a patch ref of Populated iff gerrit_change is populated Returns None if not populated.. -— **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#212)(self, patch_root, report_files_via_property=None, \*\*kwargs):** +— **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#225)(self, patch_root, report_files_via_property=None, \*\*kwargs):** Returns list of paths to files affected by the patch. @@ -856,11 +857,11 @@ Args: Returned paths will be relative to to patch_root. -— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#352)(self, tag, patch_text=None):** +— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#365)(self, tag, patch_text=None):** Gets a specific tag from a CL description -— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#316)(self, patch_text=None):** +— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#329)(self, patch_text=None):** Retrieves footers from the patch description. @@ -879,20 +880,22 @@ Returns true iff the properties exist to match a Gerrit issue. Returns true iff we have a change to check out. -— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#360)(self, footer):** +— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#373)(self, footer):** -— **def [set\_change](/recipes/recipe_modules/tryserver/api.py#363)(self, change):** +— **def [require\_is\_tryserver](/recipes/recipe_modules/tryserver/api.py#212)(self):** + +— **def [set\_change](/recipes/recipe_modules/tryserver/api.py#376)(self, change):** Set the gerrit change for this module. Args: * change: a self.m.buildbucket.common_pb2.GerritChange. -— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#277)(self):** +— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#290)(self):** Mark the tryjob result as a compile failure. -— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#289)(self):** +— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#302)(self):** Mark the tryjob result as having invalid test results. @@ -900,32 +903,32 @@ 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). -— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#273)(self):** +— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#286)(self):** Mark the tryjob result as failure to apply the patch. -— **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#251)(self, subproject_tag):** +— **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#264)(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. -— **def [set\_test\_expired\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#306)(self):** +— **def [set\_test\_expired\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#319)(self):** Mark the tryjob result as a test expiration. This means a test task expired and was never scheduled, most likely due to lack of capacity. -— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#281)(self):** +— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#294)(self):** Mark the tryjob result as a test failure. This means we started running actual tests (not prerequisite steps like checkout or compile), and some of these tests have failed. -— **def [set\_test\_timeout\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#298)(self):** +— **def [set\_test\_timeout\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#311)(self):** Mark the tryjob result as a test timeout. @@ -1139,6 +1142,13 @@ PYTHON_VERSION_COMPATIBILITY: PY2+3 PYTHON_VERSION_COMPATIBILITY: PY2+3 — **def [RunSteps](/recipes/recipe_modules/tryserver/tests/gerrit_change_target_ref.py#18)(api):** +### *recipes* / [tryserver:tests/require\_is\_tryserver](/recipes/recipe_modules/tryserver/tests/require_is_tryserver.py) + +[DEPS](/recipes/recipe_modules/tryserver/tests/require_is_tryserver.py#12): [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] + +PYTHON_VERSION_COMPATIBILITY: PY2+3 + +— **def [RunSteps](/recipes/recipe_modules/tryserver/tests/require_is_tryserver.py#19)(api):** ### *recipes* / [windows\_sdk:examples/full](/recipes/recipe_modules/windows_sdk/examples/full.py) [DEPS](/recipes/recipe_modules/windows_sdk/examples/full.py#7): [windows\_sdk](#recipe_modules-windows_sdk), [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/step][recipe_engine/recipe_modules/step] @@ -1155,6 +1165,7 @@ PYTHON_VERSION_COMPATIBILITY: PY2+3 [recipe_engine/recipe_modules/cq]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/a476355d81d68d946bb2ab2b9431d648e7ad2167/README.recipes.md#recipe_modules-cq [recipe_engine/recipe_modules/file]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/a476355d81d68d946bb2ab2b9431d648e7ad2167/README.recipes.md#recipe_modules-file [recipe_engine/recipe_modules/json]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/a476355d81d68d946bb2ab2b9431d648e7ad2167/README.recipes.md#recipe_modules-json +[recipe_engine/recipe_modules/led]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/a476355d81d68d946bb2ab2b9431d648e7ad2167/README.recipes.md#recipe_modules-led [recipe_engine/recipe_modules/milo]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/a476355d81d68d946bb2ab2b9431d648e7ad2167/README.recipes.md#recipe_modules-milo [recipe_engine/recipe_modules/path]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/a476355d81d68d946bb2ab2b9431d648e7ad2167/README.recipes.md#recipe_modules-path [recipe_engine/recipe_modules/platform]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/a476355d81d68d946bb2ab2b9431d648e7ad2167/README.recipes.md#recipe_modules-platform diff --git a/recipes/recipe_modules/tryserver/__init__.py b/recipes/recipe_modules/tryserver/__init__.py index 2485bf038..76c65dfb5 100644 --- a/recipes/recipe_modules/tryserver/__init__.py +++ b/recipes/recipe_modules/tryserver/__init__.py @@ -11,6 +11,7 @@ DEPS = [ 'recipe_engine/buildbucket', 'recipe_engine/context', 'recipe_engine/json', + 'recipe_engine/led', 'recipe_engine/path', 'recipe_engine/platform', 'recipe_engine/properties', diff --git a/recipes/recipe_modules/tryserver/api.py b/recipes/recipe_modules/tryserver/api.py index cdce52a0c..099c3ea6d 100644 --- a/recipes/recipe_modules/tryserver/api.py +++ b/recipes/recipe_modules/tryserver/api.py @@ -209,6 +209,19 @@ class TryserverApi(recipe_api.RecipeApi): self.m.properties.get('patch_repo_url') and self.m.properties.get('patch_ref')) + def require_is_tryserver(self): + if self.m.tryserver.is_tryserver: + return + + status = self.m.step.EXCEPTION + step_text = 'This recipe requires a gerrit CL for the source under test' + if self.m.led.launched_by_led: + status = self.m.step.FAILURE + step_text += ( + "\n run 'led edit-cr-cl ' to attach a CL to test" + ) + self.m.step.empty('not a tryjob', status=status, step_text=step_text) + def get_files_affected_by_patch(self, patch_root, report_files_via_property=None, **kwargs): diff --git a/recipes/recipe_modules/tryserver/tests/require_is_tryserver.py b/recipes/recipe_modules/tryserver/tests/require_is_tryserver.py new file mode 100644 index 000000000..cc0afb784 --- /dev/null +++ b/recipes/recipe_modules/tryserver/tests/require_is_tryserver.py @@ -0,0 +1,58 @@ +# Copyright 2022 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 PB.go.chromium.org.luci.swarming.proto.api import swarming as swarming_pb +from PB.recipe_modules.recipe_engine.led import properties as led_properties_pb + +PYTHON_VERSION_COMPATIBILITY = 'PY2+3' + +DEPS = [ + 'tryserver', + 'recipe_engine/buildbucket', + 'recipe_engine/properties', +] + + +def RunSteps(api): + api.tryserver.require_is_tryserver() + + +def GenTests(api): + yield api.test( + 'tryjob', + api.buildbucket.try_build(), + api.post_check(post_process.StatusSuccess), + api.post_process(post_process.DropExpectation), + ) + + yield api.test( + 'not-a-tryjob', + api.post_check(post_process.StatusException), + api.post_check(post_process.StepException, 'not a tryjob'), + api.post_process(post_process.DropExpectation), + ) + + yield api.test( + 'not-a-tryjob-led', + api.properties( + **{ + '$recipe_engine/led': + led_properties_pb.InputProperties( + led_run_id='fake-run-id', + rbe_cas_input=swarming_pb.CASReference( + cas_instance=( + 'projects/example/instances/default_instance'), + digest=swarming_pb.Digest( + hash='examplehash', + size_bytes=71, + ), + ), + ), + }), + api.post_check(post_process.StatusFailure), + api.post_check(post_process.StepFailure, 'not a tryjob'), + api.post_process(post_process.DropExpectation), + )