From b7f797be55eb8dffdf345d3192e4897dcf35159e Mon Sep 17 00:00:00 2001 From: Josip Sokcevic Date: Thu, 3 Jun 2021 18:53:22 +0000 Subject: [PATCH] Add support for py3 presubmit Depot tools git cl presubmit supports py2 and py3. However, presumbit recipe only runs py2. This patch adds support for running py3 presubmit checks too. Bug: 1214829 Recipe-Nontrivial-Roll: build Change-Id: I089460eaf65d4b6f1238eea13eb087fadb21cad5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2934371 Reviewed-by: Edward Lesmes Commit-Queue: Josip Sokcevic --- recipes/README.recipes.md | 4 +- recipes/recipe_modules/presubmit/api.py | 24 ++++++- .../examples/full.expected/basic.json | 22 ++++++ .../recipe_modules/presubmit/tests/execute.py | 72 +++++++++++++++++++ 4 files changed, 119 insertions(+), 3 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 141040eaf6..c924a6cc83 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -784,7 +784,7 @@ Raises: Returns a presubmit step. -— **def [execute](/recipes/recipe_modules/presubmit/api.py#78)(self, bot_update_step, skip_owners=False):** +— **def [execute](/recipes/recipe_modules/presubmit/api.py#100)(self, bot_update_step, skip_owners=False):** Runs presubmit and sets summary markdown if applicable. @@ -795,7 +795,7 @@ Args: Returns: a RawResult object, suitable for being returned from RunSteps. -— **def [prepare](/recipes/recipe_modules/presubmit/api.py#43)(self):** +— **def [prepare](/recipes/recipe_modules/presubmit/api.py#65)(self):** Sets up a presubmit run. diff --git a/recipes/recipe_modules/presubmit/api.py b/recipes/recipe_modules/presubmit/api.py index a22dacbb56..05c0cb0708 100644 --- a/recipes/recipe_modules/presubmit/api.py +++ b/recipes/recipe_modules/presubmit/api.py @@ -38,7 +38,29 @@ class PresubmitApi(recipe_api.RecipeApi): kwargs['wrapper'] = ('rdb', 'stream', '--') step_data = self.m.python( name, self.presubmit_support_path, presubmit_args, **kwargs) - return step_data.json.output + output = step_data.json.output or {} + if self.m.step.active_result.retcode != 0: + return output + + # Run with vpython3 directly + del (kwargs['venv']) + presubmit_args = list(args) + [ + '--json_output', + self.m.json.output(), + ] + step_data = self.m.step(name + " py3", + ['vpython3', self.presubmit_support_path] + + presubmit_args, **kwargs) + output2 = step_data.json.output or {} + + # combine outputs + for key in output: + if key in output2: + output[key] += output2[key] + del (output2[key]) + for key in output2: + output[key] = output2[key] + return output def prepare(self): """Sets up a presubmit run. diff --git a/recipes/recipe_modules/presubmit/examples/full.expected/basic.json b/recipes/recipe_modules/presubmit/examples/full.expected/basic.json index f7a34181d8..8c0683b19c 100644 --- a/recipes/recipe_modules/presubmit/examples/full.expected/basic.json +++ b/recipes/recipe_modules/presubmit/examples/full.expected/basic.json @@ -21,6 +21,28 @@ "@@@STEP_LOG_END@json.output@@@" ] }, + { + "cmd": [ + "vpython3", + "RECIPE_REPO[depot_tools]/presubmit_support.py", + "--json_output", + "/path/to/tmp/json" + ], + "env_suffixes": { + "DEPOT_TOOLS_UPDATE": [ + "0" + ], + "PATH": [ + "RECIPE_REPO[depot_tools]" + ] + }, + "name": "presubmit py3", + "~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)@@@" + ] + }, { "name": "$result" } diff --git a/recipes/recipe_modules/presubmit/tests/execute.py b/recipes/recipe_modules/presubmit/tests/execute.py index d65a610121..9af77b6430 100644 --- a/recipes/recipe_modules/presubmit/tests/execute.py +++ b/recipes/recipe_modules/presubmit/tests/execute.py @@ -121,6 +121,56 @@ def GenTests(api): Expected "," after item in list ``` + #### To see notifications and warnings, look at the stdout of the presubmit step. + ''').strip()) + api.post_process(post_process.DropExpectation)) + yield ( + api.test('failure py3') + api.runtime(is_experimental=False) + + api.buildbucket.try_build(project='infra') + api.step_data( + 'presubmit py3', + api.json.output( + { + 'errors': [{ + 'message': 'Missing LGTM', + 'long_text': 'Here are some suggested OWNERS: fake@', + 'items': [], + 'fatal': True + }, { + 'message': 'Syntax error in fake.py', + 'long_text': 'Expected "," after item in list', + 'items': [], + 'fatal': True + }], + 'notifications': [{ + 'message': 'If there is a bug associated please add it.', + 'long_text': '', + 'items': [], + 'fatal': False + }], + 'warnings': [{ + 'message': 'Line 100 has more than 80 characters', + 'long_text': '', + 'items': [], + 'fatal': False + }] + }, + retcode=1)) + api.post_process(post_process.StatusFailure) + + api.post_process( + post_process.ResultReason, + textwrap.dedent(u''' + #### There are 2 error(s), 1 warning(s), and 1 notifications(s). Here are the errors: + + **ERROR** + ``` + Missing LGTM + Here are some suggested OWNERS: fake@ + ``` + + **ERROR** + ``` + Syntax error in fake.py + Expected "," after item in list + ``` + #### To see notifications and warnings, look at the stdout of the presubmit step. ''').strip()) + api.post_process(post_process.DropExpectation)) @@ -213,3 +263,25 @@ def GenTests(api): api.post_process(post_process.StatusException) + api.post_process(post_process.ResultReason, bug_msg) + api.post_process(post_process.DropExpectation)) + + yield (api.test('warnings-merged') + api.runtime(is_experimental=False) + + api.buildbucket.try_build(project='infra') + api.step_data( + 'presubmit', + api.json.output({ + 'errors': [], + 'notifications': [], + 'warnings': [{ + 'message': 'warning py2' + }] + }), + ) + api.step_data( + 'presubmit py3', + api.json.output({ + 'errors': [], + 'extra': [], + 'warnings': [{ + 'message': 'warning py3' + }] + }), + ) + api.post_process(post_process.StatusSuccess) + + api.post_process(post_process.DropExpectation))