From f155639336aff1e8c3822974749ac695ec83b2b5 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 10 May 2023 00:06:15 +0000 Subject: [PATCH] presubmit: remove presubmit step running python2 This is to deactivate `luci.buildbucket.omit_python2` experiment. I'll submit this after https://crrev.com/c/4515085 is submitted. Bug: 1401307, 1441784 Recipe-Nontrivial-Roll: build Change-Id: I0aec58e9b96c961da21f0de0850c4078bb679c33 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4513661 Auto-Submit: Takuto Ikuta Reviewed-by: Josip Sokcevic Commit-Queue: Takuto Ikuta --- recipes/README.recipes.md | 4 +- recipes/recipe_modules/presubmit/api.py | 31 +------- .../examples/full.expected/basic.json | 24 +----- .../recipe_modules/presubmit/tests/execute.py | 76 +------------------ 4 files changed, 6 insertions(+), 129 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index be4764773..0ef218479 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -758,7 +758,7 @@ Raises: Returns a presubmit step. -— **def [execute](/recipes/recipe_modules/presubmit/api.py#130)(self, bot_update_step, skip_owners=False, run_all=False):** +— **def [execute](/recipes/recipe_modules/presubmit/api.py#103)(self, bot_update_step, skip_owners=False, run_all=False):** Runs presubmit and sets summary markdown if applicable. @@ -769,7 +769,7 @@ Args: Returns: a RawResult object, suitable for being returned from RunSteps. -— **def [prepare](/recipes/recipe_modules/presubmit/api.py#78)(self, root_solution_revision=None):** +— **def [prepare](/recipes/recipe_modules/presubmit/api.py#51)(self, root_solution_revision=None):** Sets up a presubmit run. diff --git a/recipes/recipe_modules/presubmit/api.py b/recipes/recipe_modules/presubmit/api.py index c669fa533..ab047f95a 100644 --- a/recipes/recipe_modules/presubmit/api.py +++ b/recipes/recipe_modules/presubmit/api.py @@ -30,43 +30,16 @@ class PresubmitApi(recipe_api.RecipeApi): name = kwargs.pop('name', 'presubmit') with self.m.depot_tools.on_path(): - # Use only vpython3 on bots that don't have vpython2 on the path any - # longer. - # TODO(https://crbug.com/1401307): Switch this to vpython3 premanently - # and remove py3 part below. - experiments = self.m.buildbucket.build.input.experiments - if 'luci.buildbucket.omit_python2' in experiments: - cmd = ['vpython3', self.presubmit_support_path, '--use-python3'] - else: - cmd = ['vpython', self.presubmit_support_path] - - cmd.extend(args) - cmd.extend(['--json_output', self.m.json.output()]) if self.m.resultdb.enabled: kwargs['wrapper'] = ('rdb', 'stream', '--') - step_data = self.m.step(name, cmd, **kwargs) - output = step_data.json.output or {} - if self.m.step.active_result.retcode != 0 or \ - 'luci.buildbucket.omit_python2' in experiments: - return output - presubmit_args = list(args) + [ '--json_output', self.m.json.output(), ] - step_data = self.m.step(name + " py3", + step_data = self.m.step(name, ['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 + return step_data.json.output or {} @property def _relative_root(self): diff --git a/recipes/recipe_modules/presubmit/examples/full.expected/basic.json b/recipes/recipe_modules/presubmit/examples/full.expected/basic.json index 42490d5e6..e19d71e14 100644 --- a/recipes/recipe_modules/presubmit/examples/full.expected/basic.json +++ b/recipes/recipe_modules/presubmit/examples/full.expected/basic.json @@ -1,7 +1,7 @@ [ { "cmd": [ - "vpython", + "vpython3", "RECIPE_REPO[depot_tools]/presubmit_support.py", "--json_output", "/path/to/tmp/json" @@ -20,28 +20,6 @@ "@@@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 4edd0467e..a2fb4fcab 100644 --- a/recipes/recipe_modules/presubmit/tests/execute.py +++ b/recipes/recipe_modules/presubmit/tests/execute.py @@ -38,7 +38,6 @@ def GenTests(api): git_repo='https://chromium.googlesource.com/infra/infra'), api.properties(run_all=True), api.step_data('presubmit', api.json.output({})), - api.step_data('presubmit py3', api.json.output({})), api.post_process(post_process.StatusSuccess), api.post_process(post_process.DropExpectation), ) @@ -71,7 +70,7 @@ def GenTests(api): yield (api.test('vpython3') + api.runtime(is_experimental=False) + api.buildbucket.try_build( - project='infra', experiments=['luci.buildbucket.omit_python2']) + + project='infra') + api.post_process(post_process.StepCommandContains, 'presubmit', ['vpython3']) + api.post_process(post_process.StatusSuccess) + @@ -145,57 +144,6 @@ 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', status="FAILURE") + - 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)) @@ -291,25 +239,3 @@ 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))