From 76fe1a701b5ede5bd9e282623ede7fa099cc4a6f Mon Sep 17 00:00:00 2001
From: Bruce Dawson <brucedawson@chromium.org>
Date: Wed, 25 May 2022 20:52:21 +0000
Subject: [PATCH] Print presubmit messages in a consistent order

Presubmit message types were printed in an inconsistent order, generally
determined by which type was encountered first. This unpredictability
can be confusing when comparing runs. This change enforces a consistent
order, in increasing order of severity. ERRORS always go last so that
they will be the most visible when running presubmits locally.

This also adds a "There were Python %d presubmit errors." message to
the end to make it easier to tell when there were presubmit errors.
This is particularly useful when manually running presubmits.

Bug: 1309977
Change-Id: Ib2b73c7625789bad5b21ae12abf238b746cd11e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3668724
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
---
 presubmit_support.py        | 18 ++++++++++++------
 tests/presubmit_unittest.py |  4 +++-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/presubmit_support.py b/presubmit_support.py
index d9ae4cae9..4684ee773 100755
--- a/presubmit_support.py
+++ b/presubmit_support.py
@@ -1738,7 +1738,7 @@ def DoPresubmitChecks(change,
         results += executer.ExecPresubmitScript(presubmit_script, filename)
       else:
         skipped_count += 1
-        
+
     results += thread_pool.RunAsync()
 
     messages = {}
@@ -1754,11 +1754,15 @@ def DoPresubmitChecks(change,
       else:
         messages.setdefault('Messages', []).append(result)
 
-    for name, items in messages.items():
-      sys.stdout.write('** Presubmit %s **\n' % name)
-      for item in items:
-        item.handle()
-        sys.stdout.write('\n')
+    # Print the different message types in a consistent order. ERRORS go last
+    # so that they will be most visible in the local-presubmit output.
+    for name in ['Messages', 'Warnings', 'ERRORS']:
+      if name in messages:
+        items = messages[name]
+        sys.stdout.write('** Presubmit %s **\n' % name)
+        for item in items:
+          item.handle()
+          sys.stdout.write('\n')
 
     total_time = time_time() - start_time
     if total_time > 1.0:
@@ -1774,6 +1778,8 @@ def DoPresubmitChecks(change,
             'Are you sure you wish to continue? (y/N): ')
       else:
         sys.stdout.write('\n')
+    else:
+      sys.stdout.write('There were %s presubmit errors.\n' % python_version)
 
     if json_output:
       # Write the presubmit results to json output
diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py
index 4f313c758..91c9faa7d 100755
--- a/tests/presubmit_unittest.py
+++ b/tests/presubmit_unittest.py
@@ -904,9 +904,11 @@ def CheckChangeOnCommit(input_api, output_api):
           RUNNING_PY_CHECKS_TEXT + 'Warning, no PRESUBMIT.py found.\n'
           'Running default presubmit script.\n'
           '** Presubmit ERRORS **\n!!\n\n'
+          'There were Python %d presubmit errors.\n'
           'Was the presubmit check useful? If not, run "git cl presubmit -v"\n'
           'to figure out which PRESUBMIT.py was run, then run git blame\n'
-          'on the file to figure out who to ask for help.\n')
+          'on the file to figure out who to ask for help.\n' %
+          sys.version_info.major)
       self.assertEqual(sys.stdout.getvalue(), text)
 
   def ExampleChange(self, extra_lines=None):