From 10a8286824639fe34a8c14e8d4075cdf56c5a69d Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Fri, 27 May 2022 19:25:56 +0000 Subject: [PATCH] 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 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 "", line 1, in 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 "", line 1, in 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 Commit-Queue: Bruce Dawson --- presubmit_support.py | 14 ++++++-------- tests/presubmit_unittest.py | 21 --------------------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/presubmit_support.py b/presubmit_support.py index 5c715ad45..30c003601 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -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: diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 109d104f5..e47766a52 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -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(