From 37f499daecbc6f9ce8e403914d2c29e59387e5b4 Mon Sep 17 00:00:00 2001 From: Garrett Beaty Date: Tue, 2 May 2023 16:56:48 +0000 Subject: [PATCH] Default to an empty footers dict instead of None. By default during tests, the _get_footers_step call will return None for the returned footers because it does not set any test data. This does not match the actual behavior of the recipe because git_footers.py returns an empty json object if the commit message has no footers. This change updates it to return an empty dict instead of None and checks for the cached footers value to be None instead of any non-true value so that a commit message without footers doesn't get parsed multiple times. Change-Id: I716a27e964eb92de138228df5cc0876322d82823 Recipe-Manual-Change: build Recipe-Nontrivial-Roll: build_limited Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4492269 Commit-Queue: Garrett Beaty Reviewed-by: Gavin Mak --- recipes/README.recipes.md | 8 ++--- recipes/recipe_modules/tryserver/api.py | 22 +++++++------ .../full.expected/with_gerrit_patch.json | 32 ++----------------- .../with_gerrit_patch_and_target_ref.json | 32 ++----------------- 4 files changed, 20 insertions(+), 74 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index a4f638985..45f89c01e 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -868,7 +868,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\_change\_description](/recipes/recipe_modules/tryserver/api.py#388)(self):** +— **def [get\_change\_description](/recipes/recipe_modules/tryserver/api.py#390)(self):** Gets the CL description. @@ -884,7 +884,7 @@ Args: Returned paths will be relative to to api.path['root']. -— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#377)(self, tag, patch_text=None):** +— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#382)(self, tag, patch_text=None):** Gets a specific tag from a CL description @@ -907,11 +907,11 @@ 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#385)(self, footer):** +— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#387)(self, footer):** — **def [require\_is\_tryserver](/recipes/recipe_modules/tryserver/api.py#221)(self):** -— **def [set\_change](/recipes/recipe_modules/tryserver/api.py#393)(self, change):** +— **def [set\_change](/recipes/recipe_modules/tryserver/api.py#395)(self, change):** Set the gerrit change for this module. diff --git a/recipes/recipe_modules/tryserver/api.py b/recipes/recipe_modules/tryserver/api.py index 2d65a56f2..7df678702 100644 --- a/recipes/recipe_modules/tryserver/api.py +++ b/recipes/recipe_modules/tryserver/api.py @@ -356,7 +356,7 @@ class TryserverApi(recipe_api.RecipeApi): def _get_footers(self, patch_text=None): if patch_text is not None: return self._get_footer_step(patch_text) - if self._change_footers: #pragma: nocover + if self._change_footers is not None: return self._change_footers if self.gerrit_change: self._ensure_gerrit_commit_message() @@ -366,20 +366,22 @@ class TryserverApi(recipe_api.RecipeApi): 'No patch text or associated changelist, cannot get footers') #pragma: nocover def _get_footer_step(self, patch_text): - result = self.m.step('parse description', [ - 'python3', - self.repo_resource('git_footers.py'), '--json', - self.m.json.output() - ], - stdin=self.m.raw_io.input(data=patch_text)) + result = self.m.step( + 'parse description', + [ + 'python3', + self.repo_resource('git_footers.py'), + '--json', + self.m.json.output(), + ], + stdin=self.m.raw_io.input(data=patch_text), + step_test_data=lambda: self.m.json.test_api.output({}), + ) return result.json.output def get_footer(self, tag, patch_text=None): """Gets a specific tag from a CL description""" footers = self._get_footers(patch_text) - if footers is None: - return [] - return footers.get(tag, []) def normalize_footer_name(self, footer): diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json index f617981f1..b47821cd1 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json @@ -138,36 +138,8 @@ "name": "parse description", "stdin": "Change commit message", "~followup_annotations": [ - "@@@STEP_LOG_END@json.output (invalid)@@@", - "@@@STEP_LOG_LINE@json.output (exception)@No JSON object could be decoded@@@", - "@@@STEP_LOG_END@json.output (exception)@@@" - ] - }, - { - "cmd": [ - "python3", - "RECIPE_REPO[depot_tools]/git_footers.py", - "--json", - "/path/to/tmp/json" - ], - "luci_context": { - "realm": { - "name": "chromium:linux" - }, - "resultdb": { - "current_invocation": { - "name": "invocations/build:8945511751514863184", - "update_token": "token" - }, - "hostname": "rdbhost" - } - }, - "name": "parse description (2)", - "stdin": "Change commit message", - "~followup_annotations": [ - "@@@STEP_LOG_END@json.output (invalid)@@@", - "@@@STEP_LOG_LINE@json.output (exception)@No JSON object could be decoded@@@", - "@@@STEP_LOG_END@json.output (exception)@@@" + "@@@STEP_LOG_LINE@json.output@{}@@@", + "@@@STEP_LOG_END@json.output@@@" ] }, { diff --git 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 index 36c2663cb..27d801e63 100644 --- 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 @@ -138,36 +138,8 @@ "name": "parse description", "stdin": "Change commit message", "~followup_annotations": [ - "@@@STEP_LOG_END@json.output (invalid)@@@", - "@@@STEP_LOG_LINE@json.output (exception)@No JSON object could be decoded@@@", - "@@@STEP_LOG_END@json.output (exception)@@@" - ] - }, - { - "cmd": [ - "python3", - "RECIPE_REPO[depot_tools]/git_footers.py", - "--json", - "/path/to/tmp/json" - ], - "luci_context": { - "realm": { - "name": "chromium:linux" - }, - "resultdb": { - "current_invocation": { - "name": "invocations/build:8945511751514863184", - "update_token": "token" - }, - "hostname": "rdbhost" - } - }, - "name": "parse description (2)", - "stdin": "Change commit message", - "~followup_annotations": [ - "@@@STEP_LOG_END@json.output (invalid)@@@", - "@@@STEP_LOG_LINE@json.output (exception)@No JSON object could be decoded@@@", - "@@@STEP_LOG_END@json.output (exception)@@@" + "@@@STEP_LOG_LINE@json.output@{}@@@", + "@@@STEP_LOG_END@json.output@@@" ] }, {