Augment presubmit_support to output a json summary.

Adds functionality to presubmit api to return error,
notification and warning messages caught in
presubmit_support.py in the form of a json.

Recipe-Nontrivial-Roll: build
Recipe-Nontrivial-Roll: build_limited_scripts_slave
Bug:971895
Change-Id: I42a3df3994077342216d002381b6135012b4334c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1666250
Commit-Queue: Debrian Figueroa <debrian@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>

presubmit_support.py

@@ -303,6 +303,14 @@ class _PresubmitResult(object):
     if self.fatal:
       output.fail()
 
+  def json_format(self):
+    return {
+        'message': self._message,
+        'items': self._items,
+        'long_text': self._long_text,
+        'fatal': self.fatal
+    }
+
 
 # Top level object so multiprocessing can pickle
 # Public access through OutputApi object.
@@ -1462,7 +1470,8 @@ def DoPresubmitChecks(change,
                       may_prompt,
                       gerrit_obj,
                       dry_run=None,
-                      parallel=False):
+                      parallel=False,
+                      json_output=None):
   """Runs all presubmit checks that apply to the files in the change.
 
   This finds all PRESUBMIT.py files in directories enclosing the files in the
@@ -1501,6 +1510,7 @@ def DoPresubmitChecks(change,
     os.environ['PYTHONDONTWRITEBYTECODE'] = '1'
 
     output = PresubmitOutput(input_stream, output_stream)
+
     if committing:
       output.write("Running presubmit commit checks ...\n")
     else:
@@ -1541,6 +1551,22 @@ def DoPresubmitChecks(change,
       else:
         notifications.append(result)
 
+    if json_output:
+      # Write the presubmit results to json output
+      presubmit_results = {
+        'errors': [
+            error.json_format() for error in errors
+        ],
+        'notifications': [
+            notification.json_format() for notification in notifications
+        ],
+        'warnings': [
+            warning.json_format() for warning in warnings
+        ]
+      }
+
+      gclient_utils.FileWrite(json_output, json.dumps(presubmit_results))
+
     output.write('\n')
     for name, items in (('Messages', notifications),
                         ('Warnings', warnings),
@@ -1674,6 +1700,8 @@ def main(argv=None):
   parser.add_option('--parallel', action='store_true',
                     help='Run all tests specified by input_api.RunTests in all '
                          'PRESUBMIT files in parallel.')
+  parser.add_option('--json_output',
+                    help='Write presubmit errors to json output.')
 
   options, args = parser.parse_args(argv)
 
@@ -1718,7 +1746,8 @@ def main(argv=None):
           options.may_prompt,
           gerrit_obj,
           options.dry_run,
-          options.parallel)
+          options.parallel,
+          options.json_output)
     return not results.should_continue()
   except PresubmitFailure as e:
     print(e, file=sys.stderr)

recipes/README.recipes.md

@@ -739,7 +739,7 @@ Raises:
     StepFailure or InfraFailure.
 ### *recipe_modules* / [presubmit](/recipes/recipe_modules/presubmit)
 
-[DEPS](/recipes/recipe_modules/presubmit/__init__.py#1): [depot\_tools](#recipe_modules-depot_tools), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/step][recipe_engine/recipe_modules/step]
+[DEPS](/recipes/recipe_modules/presubmit/__init__.py#1): [depot\_tools](#recipe_modules-depot_tools), [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/python][recipe_engine/recipe_modules/python], [recipe\_engine/step][recipe_engine/recipe_modules/step]
 
 #### **class [PresubmitApi](/recipes/recipe_modules/presubmit/api.py#7)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
 
@@ -984,9 +984,9 @@ Move things around in a loop!
 — **def [RunSteps](/recipes/recipe_modules/osx_sdk/examples/full.py#13)(api):**
 ### *recipes* / [presubmit:examples/full](/recipes/recipe_modules/presubmit/examples/full.py)
 
-[DEPS](/recipes/recipe_modules/presubmit/examples/full.py#5): [presubmit](#recipe_modules-presubmit)
+[DEPS](/recipes/recipe_modules/presubmit/examples/full.py#5): [presubmit](#recipe_modules-presubmit), [recipe\_engine/json][recipe_engine/recipe_modules/json]
 
-— **def [RunSteps](/recipes/recipe_modules/presubmit/examples/full.py#10)(api):**
+— **def [RunSteps](/recipes/recipe_modules/presubmit/examples/full.py#11)(api):**
 ### *recipes* / [tryserver:examples/full](/recipes/recipe_modules/tryserver/examples/full.py)
 
 [DEPS](/recipes/recipe_modules/tryserver/examples/full.py#5): [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [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]

recipes/recipe_modules/presubmit/__init__.py

@@ -1,5 +1,6 @@
 DEPS = [
   'depot_tools',
+  'recipe_engine/json',
   'recipe_engine/context',
   'recipe_engine/path',
   'recipe_engine/python',

recipes/recipe_modules/presubmit/api.py

@@ -13,7 +13,10 @@ class PresubmitApi(recipe_api.RecipeApi):
     """Return a presubmit step."""
 
     name = kwargs.pop('name', 'presubmit')
-
     with self.m.depot_tools.on_path():
-      return self.m.python(
+      presubmit_args = list(args) + [
-          name, self.presubmit_support_path, list(args), **kwargs)
+          '--json_output', self.m.json.output(),
+      ]
+      step_data = self.m.python(
+          name, self.presubmit_support_path, presubmit_args, **kwargs)
+      return step_data.json.output

recipes/recipe_modules/presubmit/examples/full.expected/basic.json

@@ -3,14 +3,20 @@
     "cmd": [
       "python",
       "-u",
-      "RECIPE_REPO[depot_tools]/presubmit_support.py"
+      "RECIPE_REPO[depot_tools]/presubmit_support.py",
+      "--json_output",
+      "/path/to/tmp/json"
     ],
     "env_prefixes": {
       "PATH": [
         "RECIPE_REPO[depot_tools]"
       ]
     },
-    "name": "presubmit"
+    "name": "presubmit",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@json.output@{}@@@",
+      "@@@STEP_LOG_END@json.output@@@"
+    ]
   },
   {
     "name": "$result"

recipes/recipe_modules/presubmit/examples/full.py

@@ -4,6 +4,7 @@
 
 DEPS = [
   'presubmit',
+  'recipe_engine/json'
 ]
 
 
@@ -12,4 +13,7 @@ def RunSteps(api):
 
 
 def GenTests(api):
-  yield api.test('basic')
+  yield (
+    api.test('basic') +
+    api.step_data('presubmit', api.json.output({}))
+  )

tests/presubmit_unittest.py

@@ -614,13 +614,103 @@ class PresubmitUnittest(PresubmitTestsBase):
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=None,
-        default_presubmit=None, may_prompt=False, gerrit_obj=None)
+        default_presubmit=None, may_prompt=False,
+        gerrit_obj=None, json_output=None)
     self.failUnless(output.should_continue())
     self.assertEqual(output.getvalue().count('!!'), 0)
     self.assertEqual(output.getvalue().count('??'), 0)
     self.assertEqual(output.getvalue().count(
         'Running presubmit upload checks ...\n'), 1)
 
+  def testDoPresubmitChecksJsonOutput(self):
+    fake_error = 'Missing LGTM'
+    fake_error_items = '["!", "!!", "!!!"]'
+    fake_error_long_text = "Error long text..."
+    fake_error2 = 'This failed was found in file fake.py'
+    fake_error2_items = '["!!!", "!!", "!"]'
+    fake_error2_long_text = " Error long text" * 3
+    fake_warning = 'Line 88 is more than 80 characters.'
+    fake_warning_items = '["W", "w"]'
+    fake_warning_long_text = 'Warning long text...'
+    fake_notify = 'This is a dry run'
+    fake_notify_items = '["N"]'
+    fake_notify_long_text = 'Notification long text...'
+    always_fail_presubmit_script = """
+def CheckChangeOnUpload(input_api, output_api):
+  return [
+    output_api.PresubmitError("%s",%s, "%s"),
+    output_api.PresubmitError("%s",%s, "%s"),
+    output_api.PresubmitPromptWarning("%s",%s, "%s"),
+    output_api.PresubmitNotifyResult("%s",%s, "%s")
+  ]
+def CheckChangeOnCommit(input_api, output_api):
+  raise Exception("Test error")
+""" % (fake_error, fake_error_items, fake_error_long_text,
+       fake_error2, fake_error2_items, fake_error2_long_text,
+       fake_warning, fake_warning_items, fake_warning_long_text,
+       fake_notify, fake_notify_items, fake_notify_long_text
+       )
+
+    inherit_path = presubmit.os.path.join(
+        self.fake_root_dir, self._INHERIT_SETTINGS)
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
+    presubmit.os.listdir(self.fake_root_dir).AndReturn([])
+    presubmit.os.listdir(presubmit.os.path.join(
+        self.fake_root_dir, 'haspresubmit')).AndReturn(['PRESUBMIT.py'])
+    presubmit.os.path.isfile(presubmit.os.path.join(
+        self.fake_root_dir, 'haspresubmit', 'PRESUBMIT.py')).AndReturn(False)
+    presubmit.random.randint(0, 4).AndReturn(0)
+
+    change = self.ExampleChange(extra_lines=['ERROR=yes'])
+
+    temp_path = 'temp.json'
+
+    fake_result = {
+        'notifications': [
+          {
+            'message': fake_notify,
+            'items': json.loads(fake_notify_items),
+            'fatal': False,
+            'long_text': fake_notify_long_text
+          }
+        ],
+        'errors': [
+          {
+            'message': fake_error,
+            'items': json.loads(fake_error_items),
+            'fatal': True,
+            'long_text': fake_error_long_text
+          },
+          {
+            'message': fake_error2,
+            'items': json.loads(fake_error2_items),
+            'fatal': True,
+            'long_text': fake_error2_long_text
+          }
+        ],
+        'warnings': [
+          {
+            'message': fake_warning,
+            'items': json.loads(fake_warning_items),
+            'fatal': False,
+            'long_text': fake_warning_long_text
+          }
+        ]
+    }
+
+    fake_result_json = json.dumps(fake_result)
+    presubmit.gclient_utils.FileWrite(temp_path, fake_result_json)
+
+    self.mox.ReplayAll()
+
+    output = presubmit.DoPresubmitChecks(
+        change=change, committing=False, verbose=True,
+        output_stream=None, input_stream=None,
+        default_presubmit=always_fail_presubmit_script,
+        may_prompt=False, gerrit_obj=None, json_output=temp_path)
+
+    self.failIf(output.should_continue())
+
   def testDoPresubmitChecksPromptsAfterWarnings(self):
     presubmit_path = presubmit.os.path.join(self.fake_root_dir, 'PRESUBMIT.py')
     haspresubmit_path = presubmit.os.path.join(
@@ -649,7 +739,8 @@ class PresubmitUnittest(PresubmitTestsBase):
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=input_buf,
-        default_presubmit=None, may_prompt=True, gerrit_obj=None)
+        default_presubmit=None, may_prompt=True,
+        gerrit_obj=None, json_output=None)
     self.failIf(output.should_continue())
     self.assertEqual(output.getvalue().count('??'), 2)
 
@@ -657,7 +748,8 @@ class PresubmitUnittest(PresubmitTestsBase):
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=input_buf,
-        default_presubmit=None, may_prompt=True, gerrit_obj=None)
+        default_presubmit=None, may_prompt=True,
+        gerrit_obj=None, json_output=None)
     self.failUnless(output.should_continue())
     self.assertEquals(output.getvalue().count('??'), 2)
     self.assertEqual(output.getvalue().count(
@@ -688,7 +780,8 @@ class PresubmitUnittest(PresubmitTestsBase):
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=None,
-        default_presubmit=None, may_prompt=False, gerrit_obj=None)
+        default_presubmit=None, may_prompt=False,
+        gerrit_obj=None, json_output=None)
     # A warning is printed, and should_continue is True.
     self.failUnless(output.should_continue())
     self.assertEquals(output.getvalue().count('??'), 2)
@@ -719,7 +812,8 @@ class PresubmitUnittest(PresubmitTestsBase):
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=None,
-        default_presubmit=None, may_prompt=True, gerrit_obj=None)
+        default_presubmit=None, may_prompt=True,
+        gerrit_obj=None, json_output=None)
     self.failIf(output.should_continue())
     self.assertEqual(output.getvalue().count('??'), 0)
     self.assertEqual(output.getvalue().count('!!'), 2)
@@ -753,7 +847,7 @@ def CheckChangeOnCommit(input_api, output_api):
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=input_buf,
         default_presubmit=always_fail_presubmit_script,
-        may_prompt=False, gerrit_obj=None)
+        may_prompt=False, gerrit_obj=None, json_output=None)
     self.failIf(output.should_continue())
     text = (
         'Running presubmit upload checks ...\n'
@@ -895,7 +989,8 @@ def CheckChangeOnCommit(input_api, output_api):
     presubmit.DoPresubmitChecks(mox.IgnoreArg(), False, False,
                                 mox.IgnoreArg(),
                                 mox.IgnoreArg(),
-                                None, False, None, None, None).AndReturn(output)
+                                None, False, None, None, None,
+                                None).AndReturn(output)
     self.mox.ReplayAll()
 
     self.assertEquals(