presubmit: upload presubmit result as code findings

The recipe will convert presubmit result to finding as long as it has location data. Change-Id: I8e0ce4cf5f66d6236f10c21a6db87d293b3fe379 Bug: 404837554 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6469944 Commit-Queue: Yiwei Zhang <yiwzhang@google.com> Reviewed-by: Gavin Mak <gavinmak@google.com>
2026-01-11 18:51:29 +00:00 · 2025-04-22 14:14:57 -07:00
parent d49e17e19f
commit 94eb88d04c
4 changed files with 187 additions and 18 deletions
--- a/recipes/README.recipes.md
+++ b/recipes/README.recipes.md
@@ -830,19 +830,22 @@ Raises:
 &mdash; **def [initialize](/recipes/recipe_modules/osx_sdk/api.py#56)(self):**
 ### *recipe_modules* / [presubmit](/recipes/recipe_modules/presubmit)

-[DEPS](/recipes/recipe_modules/presubmit/__init__.py#13): [bot\_update](#recipe_modules-bot_update), [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [git](#recipe_modules-git), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cv][recipe_engine/recipe_modules/cv], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/resultdb][recipe_engine/recipe_modules/resultdb], [recipe\_engine/step][recipe_engine/recipe_modules/step]
+[DEPS](/recipes/recipe_modules/presubmit/__init__.py#13): [bot\_update](#recipe_modules-bot_update), [depot\_tools](#recipe_modules-depot_tools), [gclient](#recipe_modules-gclient), [git](#recipe_modules-git), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cv][recipe_engine/recipe_modules/cv], [recipe\_engine/findings][recipe_engine/recipe_modules/findings], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/resultdb][recipe_engine/recipe_modules/resultdb], [recipe\_engine/step][recipe_engine/recipe_modules/step]


-#### **class [PresubmitApi](/recipes/recipe_modules/presubmit/api.py#16)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
+#### **class [PresubmitApi](/recipes/recipe_modules/presubmit/api.py#17)([RecipeApi][recipe_engine/wkt/RecipeApi]):**

-&mdash; **def [\_\_call\_\_](/recipes/recipe_modules/presubmit/api.py#28)(self, \*args, \*\*kwargs):**
+&mdash; **def [\_\_call\_\_](/recipes/recipe_modules/presubmit/api.py#29)(self, \*args, \*\*kwargs):**

 Returns a presubmit step.

-&mdash; **def [execute](/recipes/recipe_modules/presubmit/api.py#116)(self, bot_update_step, skip_owners=False, run_all=False):**
+&mdash; **def [execute](/recipes/recipe_modules/presubmit/api.py#117)(self, bot_update_step, skip_owners=False, run_all=False):**

 Runs presubmit and sets summary markdown if applicable.

+Also uploads the presubmit results as findings if the results contain
+location data.
+
 Args:
  * bot_update_step: the StepResult from a previously executed bot_update step.
  * skip_owners: a boolean indicating whether Owners checks should be skipped.
@@ -850,7 +853,7 @@ Args:
 Returns:
  a RawResult object, suitable for being returned from RunSteps.

-&mdash; **def [prepare](/recipes/recipe_modules/presubmit/api.py#49)(self, root_solution_revision=None):**
+&mdash; **def [prepare](/recipes/recipe_modules/presubmit/api.py#50)(self, root_solution_revision=None):**

 Sets up a presubmit run.

@@ -867,7 +870,7 @@ Args:
 Returns:
  the StepResult from the bot_update step.

-&emsp; **@property**<br>&mdash; **def [presubmit\_support\_path](/recipes/recipe_modules/presubmit/api.py#24)(self):**
+&emsp; **@property**<br>&mdash; **def [presubmit\_support\_path](/recipes/recipe_modules/presubmit/api.py#25)(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/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/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]
@@ -1205,10 +1208,10 @@ Move things around in a loop!
 &mdash; **def [RunSteps](/recipes/recipe_modules/presubmit/examples/full.py#13)(api):**
 ### *recipes* / [presubmit:tests/execute](/recipes/recipe_modules/presubmit/tests/execute.py)

-[DEPS](/recipes/recipe_modules/presubmit/tests/execute.py#12): [gclient](#recipe_modules-gclient), [presubmit](#recipe_modules-presubmit), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cq][recipe_engine/recipe_modules/cq], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime]
+[DEPS](/recipes/recipe_modules/presubmit/tests/execute.py#13): [gclient](#recipe_modules-gclient), [presubmit](#recipe_modules-presubmit), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/cq][recipe_engine/recipe_modules/cq], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/proto][recipe_engine/recipe_modules/proto], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime]


-&mdash; **def [RunSteps](/recipes/recipe_modules/presubmit/tests/execute.py#25)(api):**
+&mdash; **def [RunSteps](/recipes/recipe_modules/presubmit/tests/execute.py#27)(api):**
 ### *recipes* / [presubmit:tests/prepare](/recipes/recipe_modules/presubmit/tests/prepare.py)

 [DEPS](/recipes/recipe_modules/presubmit/tests/prepare.py#11): [gclient](#recipe_modules-gclient), [presubmit](#recipe_modules-presubmit), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime]
@@ -1279,12 +1282,14 @@ Move things around in a loop!
 [recipe_engine/recipe_modules/cq]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-cq
 [recipe_engine/recipe_modules/cv]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-cv
 [recipe_engine/recipe_modules/file]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-file
+[recipe_engine/recipe_modules/findings]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-findings
 [recipe_engine/recipe_modules/json]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-json
 [recipe_engine/recipe_modules/led]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-led
 [recipe_engine/recipe_modules/milo]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-milo
 [recipe_engine/recipe_modules/path]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-path
 [recipe_engine/recipe_modules/platform]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-platform
 [recipe_engine/recipe_modules/properties]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-properties
+[recipe_engine/recipe_modules/proto]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-proto
 [recipe_engine/recipe_modules/raw_io]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-raw_io
 [recipe_engine/recipe_modules/resultdb]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-resultdb
 [recipe_engine/recipe_modules/runtime]: https://chromium.googlesource.com/infra/luci/recipes-py.git/+/c531c52ec17dfef3e6bc77f0eb7493d48e3679ae/README.recipes.md#recipe_modules-runtime
--- a/recipes/recipe_modules/presubmit/init.py
+++ b/recipes/recipe_modules/presubmit/init.py
@@ -18,6 +18,7 @@ DEPS = [
    'recipe_engine/buildbucket',
    'recipe_engine/context',
    'recipe_engine/cv',
+    'recipe_engine/findings',
    'recipe_engine/json',
    'recipe_engine/path',
    'recipe_engine/properties',
--- a/recipes/recipe_modules/presubmit/api.py
+++ b/recipes/recipe_modules/presubmit/api.py
@@ -6,6 +6,7 @@ from recipe_engine import recipe_api

 from PB.recipe_engine import result as result_pb2
 from PB.go.chromium.org.luci.buildbucket.proto import common as common_pb2
+from PB.go.chromium.org.luci.common.proto.findings import findings as findings_pb

 # 8 minutes seems like a reasonable upper bound on presubmit timings.
 # According to event mon data we have, it seems like anything longer than
@@ -116,6 +117,9 @@ class PresubmitApi(recipe_api.RecipeApi):
  def execute(self, bot_update_step, skip_owners=False, run_all=False):
    """Runs presubmit and sets summary markdown if applicable.

+    Also uploads the presubmit results as findings if the results contain
+    location data.
+
    Args:
      * bot_update_step: the StepResult from a previously executed bot_update step.
      * skip_owners: a boolean indicating whether Owners checks should be skipped.
@@ -210,8 +214,50 @@ class PresubmitApi(recipe_api.RecipeApi):
          ' while running presubmit checks.'
          ' Please [file a bug](https://issues.chromium.org'
          '/issues/new?component=1456211)')
+    if step_json:
+      self._upload_findings_from_result(step_json)
    return raw_result

+  def _upload_findings_from_result(self, result_json):
+    findings = []
+    base_finding = findings_pb.Finding(
+        category='chromium_presubmit',
+        location=findings_pb.Location(
+            gerrit_change_ref=findings_pb.Location.GerritChangeReference(
+                host=self.m.tryserver.gerrit_change.host,
+                project=self.m.tryserver.gerrit_change.project,
+                change=self.m.tryserver.gerrit_change.change,
+                patchset=self.m.tryserver.gerrit_change.patchset,
+            ), ),
+    )
+    for results, level in [
+        (result_json.get('errors',
+                         []), findings_pb.Finding.SEVERITY_LEVEL_ERROR),
+        (result_json.get('warnings',
+                         []), findings_pb.Finding.SEVERITY_LEVEL_WARNING),
+        (result_json.get('notifications',
+                         []), findings_pb.Finding.SEVERITY_LEVEL_INFO)
+    ]:
+      for result in results:
+        message = result.get('message', '')
+        if result.get('long_text', None):
+          message += '\n\n' + result['long_text']
+        for loc in result.get('locations', []):
+          f = findings_pb.Finding()
+          f.CopyFrom(base_finding)
+          f.message = message
+          f.severity_level = level
+          f.location.file_path = loc['file_path'].replace(self.m.path.sep, '/')
+          if loc.get('start_line', None):
+            f.location.range.start_line = loc['start_line']
+            f.location.range.end_line = loc['end_line']
+            f.location.range.start_column = loc.get('start_col', 0)
+            f.location.range.end_column = loc.get('end_col', 0)
+          findings.append(f)
+
+    if findings:
+      self.m.findings.upload_findings(
+          findings, step_name='upload presubmit results as findings')

 def _limitSize(message_list, char_limit=450):
  """Returns a list of strings within a certain character length.
--- a/recipes/recipe_modules/presubmit/tests/execute.py
+++ b/recipes/recipe_modules/presubmit/tests/execute.py
@@ -4,21 +4,23 @@

 import textwrap

+from PB.go.chromium.org.luci.common.proto.findings import findings as findings_pb
 from recipe_engine import post_process
-from recipe_engine import recipe_api
+

 PYTHON_VERSION_COMPATIBILITY = 'PY3'

 DEPS = [
-  'gclient',
-  'presubmit',
-  'recipe_engine/buildbucket',
-  'recipe_engine/context',
-  'recipe_engine/cq',
-  'recipe_engine/json',
-  'recipe_engine/path',
-  'recipe_engine/properties',
-  'recipe_engine/runtime',
+    'gclient',
+    'presubmit',
+    'recipe_engine/buildbucket',
+    'recipe_engine/context',
+    'recipe_engine/cq',
+    'recipe_engine/json',
+    'recipe_engine/path',
+    'recipe_engine/properties',
+    'recipe_engine/proto',
+    'recipe_engine/runtime',
 ]


@@ -249,3 +251,118 @@ def GenTests(api):
      api.post_process(post_process.SummaryMarkdown, bug_msg),
      api.post_process(post_process.DropExpectation),
      status="INFRA_FAILURE")
+
+  def _has_uploaded_findings(check, steps, expected_findings):
+    step_name = 'upload presubmit results as findings'
+    check(step_name in steps)
+    check('findings.json' in steps[step_name].logs)
+    findings = api.proto.decode(steps[step_name].logs['findings.json'],
+                                findings_pb.Findings, 'JSONPB').findings
+    check(len(findings) == len(expected_findings))
+    for (actual, expected) in zip(findings, expected_findings):
+      check(actual == expected)
+
+  gerrit_change_ref = findings_pb.Location.GerritChangeReference(
+      host='chromium-review.googlesource.com',
+      project='infra',
+      change=123456,
+      patchset=7)
+
+  yield api.test(
+      'upload_findings',
+      api.runtime(is_experimental=False),
+      api.buildbucket.try_build(project='infra'),
+      api.step_data(
+          'presubmit',
+          api.json.output(
+              {
+                  'errors': [{
+                      'message':
+                      'bug bug bug',
+                      'long_text':
+                      '',
+                      'items': [],
+                      'locations': [{
+                          'file_path': 'path/to/file1',
+                      }, {
+                          'file_path': 'path/to/file2',
+                      }],
+                      'fatal':
+                      True
+                  }],
+                  'notifications':
+                  [{
+                      'message': 'cc abc@google.com',
+                      'long_text': '',
+                      'items': [],
+                      'locations': [{
+                          'file_path': '/COMMIT_MSG',
+                      }],
+                      'fatal': False
+                  }],
+                  'warnings': [{
+                      'message':
+                      'Change takes 20 min to take effect after landing',
+                      'long_text': '',
+                      'items': [],
+                      'locations': [],
+                      'fatal': False
+                  }, {
+                      'message':
+                      'typo!!',
+                      'long_text':
+                      'replace foo with bar',
+                      'items': [],
+                      'locations': [{
+                          'file_path': 'path/to/file',
+                          'start_line': 1,
+                          'start_col': 2,
+                          'end_line': 1,
+                          'end_col': 5,
+                      }],
+                      'fatal':
+                      False
+                  }]
+              },
+              retcode=1)),
+      api.post_process(
+          _has_uploaded_findings,
+          [
+              findings_pb.Finding(
+                  category='chromium_presubmit',
+                  location=findings_pb.Location(
+                      gerrit_change_ref=gerrit_change_ref,
+                      file_path='path/to/file1'),
+                  message='bug bug bug',
+                  severity_level=findings_pb.Finding.SEVERITY_LEVEL_ERROR),
+              findings_pb.Finding(
+                  category='chromium_presubmit',
+                  location=findings_pb.Location(
+                      gerrit_change_ref=gerrit_change_ref,
+                      file_path='path/to/file2'),
+                  message='bug bug bug',
+                  severity_level=findings_pb.Finding.SEVERITY_LEVEL_ERROR),
+              findings_pb.Finding(
+                  category='chromium_presubmit',
+                  location=findings_pb.Location(
+                      gerrit_change_ref=gerrit_change_ref,
+                      file_path='path/to/file',
+                      range=findings_pb.Location.Range(
+                          start_line=1,
+                          start_column=2,
+                          end_line=1,
+                          end_column=5,
+                      )),
+                  message='typo!!\n\nreplace foo with bar',
+                  severity_level=findings_pb.Finding.SEVERITY_LEVEL_WARNING),
+              findings_pb.Finding(
+                  category='chromium_presubmit',
+                  location=findings_pb.Location(
+                      gerrit_change_ref=gerrit_change_ref,
+                      file_path='/COMMIT_MSG'),
+                  message='cc abc@google.com',
+                  severity_level=findings_pb.Finding.SEVERITY_LEVEL_INFO),
+          ],
+      ),
+      api.post_process(post_process.DropExpectation),
+      status="FAILURE")