Commit 76fe1a70 authored by Bruce Dawson's avatar Bruce Dawson Committed by LUCI CQ

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/+/3668724Reviewed-by: 's avatarGavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
parent 8d2d507a
...@@ -1738,7 +1738,7 @@ def DoPresubmitChecks(change, ...@@ -1738,7 +1738,7 @@ def DoPresubmitChecks(change,
results += executer.ExecPresubmitScript(presubmit_script, filename) results += executer.ExecPresubmitScript(presubmit_script, filename)
else: else:
skipped_count += 1 skipped_count += 1
results += thread_pool.RunAsync() results += thread_pool.RunAsync()
messages = {} messages = {}
...@@ -1754,11 +1754,15 @@ def DoPresubmitChecks(change, ...@@ -1754,11 +1754,15 @@ def DoPresubmitChecks(change,
else: else:
messages.setdefault('Messages', []).append(result) messages.setdefault('Messages', []).append(result)
for name, items in messages.items(): # Print the different message types in a consistent order. ERRORS go last
sys.stdout.write('** Presubmit %s **\n' % name) # so that they will be most visible in the local-presubmit output.
for item in items: for name in ['Messages', 'Warnings', 'ERRORS']:
item.handle() if name in messages:
sys.stdout.write('\n') 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 total_time = time_time() - start_time
if total_time > 1.0: if total_time > 1.0:
...@@ -1774,6 +1778,8 @@ def DoPresubmitChecks(change, ...@@ -1774,6 +1778,8 @@ def DoPresubmitChecks(change,
'Are you sure you wish to continue? (y/N): ') 'Are you sure you wish to continue? (y/N): ')
else: else:
sys.stdout.write('\n') sys.stdout.write('\n')
else:
sys.stdout.write('There were %s presubmit errors.\n' % python_version)
if json_output: if json_output:
# Write the presubmit results to json output # Write the presubmit results to json output
......
...@@ -904,9 +904,11 @@ def CheckChangeOnCommit(input_api, output_api): ...@@ -904,9 +904,11 @@ def CheckChangeOnCommit(input_api, output_api):
RUNNING_PY_CHECKS_TEXT + 'Warning, no PRESUBMIT.py found.\n' RUNNING_PY_CHECKS_TEXT + 'Warning, no PRESUBMIT.py found.\n'
'Running default presubmit script.\n' 'Running default presubmit script.\n'
'** Presubmit ERRORS **\n!!\n\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' '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' '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) self.assertEqual(sys.stdout.getvalue(), text)
def ExampleChange(self, extra_lines=None): def ExampleChange(self, extra_lines=None):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment