Don't halt presubmits on exceptions

Some presubmit failures result in uncaught exceptions which historically
have halted presubmit runs. This behavior makes it more difficult to
find all failures, measure presubmit times, or detect other behavior
over time. This is particularly problematic when running "git cl
presubmit --all".

As an example, presubmit --all was broken and would not complete from
when crrev.com/3642633 landed until crrev.com/c/3657600. The minimal
repro was this presubmit command:

    git cl presubmit --files ui\accessibility\ax_mode.h

This command, or presubmit --all, triggered this error, which halts the
presubmits:

Evaluation of CheckChangeOnCommit failed: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js'
Traceback (most recent call last):
  File "presubmit_support.py", line 2038, in <module>
    sys.exit(main())
  File "presubmit_support.py", line 2015, in main
    return DoPresubmitChecks(
  File "presubmit_support.py", line 1738, in DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "presubmit_support.py", line 1602, in ExecPresubmitScript
    self._run_check_function(function_name, context, sink,
  File "presubmit_support.py", line 1640, in _run_check_function
    six.reraise(e_type, e_value, e_tb)
  File "C:\Users\brucedawson\.vpython-root\e726d2\lib\site-packages\six.py", line 686, in reraise
    raise value
  File "presubmit_support.py", line 1630, in _run_check_function
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "PRESUBMIT.py", line 314, in CheckChangeOnCommit
    errs.extend(CheckModesMatch(input_api, output_api))
  File "PRESUBMIT.py", line 283, in CheckModesMatch
    ax_modes_in_js = GetAccessibilityModesFromFile(
  File "PRESUBMIT.py", line 245, in GetAccessibilityModesFromFile
    for line in open(fullpath).readlines():
FileNotFoundError: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js'

With this change the exception is handled and presubmits continue after
printing this message:

Evaluation of CheckChangeOnCommit failed: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js', Traceback (most recent call last):
  File "presubmit_support.py", line 1630, in _run_check_function
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "PRESUBMIT.py", line 314, in CheckChangeOnCommit
    errs.extend(CheckModesMatch(input_api, output_api))
  File "PRESUBMIT.py", line 283, in CheckModesMatch
    ax_modes_in_js = GetAccessibilityModesFromFile(
  File "PRESUBMIT.py", line 245, in GetAccessibilityModesFromFile
    for line in open(fullpath).readlines():
FileNotFoundError: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js'

That is, the crucial information regarding the failure is printed, but
subsequent presubmits are not skipped.

Bug: 1309977
Change-Id: I7cfeda85c830e6c8e567c0df3c50f27af1dbe835
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3653978
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
changes/78/3653978/7
Bruce Dawson 3 years ago committed by LUCI CQ
parent f69f264fba
commit 10a8286824

@ -1638,14 +1638,12 @@ class PresubmitExecuter(object):
result = eval(function_name + '(*__args)', context)
self._check_result_type(result)
except Exception:
if sink:
elapsed_time = time_time() - start_time
sink.report(function_name, rdb_wrapper.STATUS_FAIL, elapsed_time)
# TODO(crbug.com/953884): replace reraise with native py3:
# raise .. from e
e_type, e_value, e_tb = sys.exc_info()
print('Evaluation of %s failed: %s' % (function_name, e_value))
six.reraise(e_type, e_value, e_tb)
_, e_value, _ = sys.exc_info()
result = [
OutputApi.PresubmitError(
'Evaluation of %s failed: %s, %s' %
(function_name, e_value, traceback.format_exc()))
]
elapsed_time = time_time() - start_time
if elapsed_time > 10.0:

@ -540,12 +540,6 @@ class PresubmitUnittest(PresubmitTestsBase):
' else:\n'
' return ()'), fake_presubmit))
self.assertRaises(
presubmit.PresubmitFailure, executer.ExecPresubmitScript,
self.presubmit_text_prefix +
'def CheckChangeOnCommit(input_api, output_api):\n'
' return "foo"', fake_presubmit)
self.assertFalse(
executer.ExecPresubmitScript(
self.presubmit_text_prefix +
@ -561,12 +555,6 @@ class PresubmitUnittest(PresubmitTestsBase):
' input_api, output_api))\n'
' return results\n', fake_presubmit))
self.assertRaises(
presubmit.PresubmitFailure, executer.ExecPresubmitScript,
self.presubmit_text_prefix +
'def CheckChangeOnCommit(input_api, output_api):\n'
' return ["foo"]', fake_presubmit)
def testExecPresubmitScriptWithResultDB(self):
description_lines = ('Hello there', 'this is a change', 'BUG=123')
files = [['A', 'foo\\blat.cc']]
@ -585,15 +573,6 @@ class PresubmitUnittest(PresubmitTestsBase):
sink.report.assert_called_with('CheckChangeOnCommit',
rdb_wrapper.STATUS_PASS, 0)
# STATUS_FAIL on exception
sink.reset_mock()
self.assertRaises(
Exception, executer.ExecPresubmitScript, self.presubmit_text_prefix +
'def CheckChangeOnCommit(input_api, output_api):\n'
' raise Exception("boom")', fake_presubmit)
sink.report.assert_called_with('CheckChangeOnCommit',
rdb_wrapper.STATUS_FAIL, 0)
# STATUS_FAIL on fatal error
sink.reset_mock()
executer.ExecPresubmitScript(

Loading…
Cancel
Save