Skip to content

Commit

Permalink
Report more issues as errors with presubmit --all
Browse files Browse the repository at this point in the history
Change crrev.com/c/3788227 fixed one instance of errors being treated as
warnings when running "git cl presubmit" with the --upload option. This
is undesirable when testing "git cl presubmit" with --all or --files,
because it makes the errors harder to find. This change fixes four
more newly discovered instances of this behavior.

That is, this change makes it so that pylint issues and other serious
problems will be reported as errors when running:

  git cl presubmit --force --all --upload

This will make the pylint errors that this command triggers easier to
find and fix:

  git cl presubmit --force --upload --files mojo\public\tools\bindings\*.py

This change does _not_ turn cpplint warnings into errors, even though
they are errors when running non-upload presubmits. That is because
there are several directories that only run cpplint on upload and these
directories have many errors and there is no short-term path to changing
this.

Bug: 1309977
Change-Id: If49f820fc6894dcd1d9aaaf4d932b04f79922bc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3791744
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
  • Loading branch information
randomascii authored and LUCI CQ committed Aug 2, 2022
1 parent faa1111 commit d895d01
Showing 1 changed file with 6 additions and 3 deletions.
9 changes: 6 additions & 3 deletions presubmit_canned_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def CheckAuthorizedAuthor(input_api, output_api, bot_allowlist=None):
"""For non-googler/chromites committers, verify the author's email address is
in AUTHORS.
"""
if input_api.is_committing:
if input_api.is_committing or input_api.no_diffs:
error_type = output_api.PresubmitError
else:
error_type = output_api.PresubmitPromptWarning
Expand Down Expand Up @@ -263,6 +263,9 @@ def CheckChangeLintsClean(input_api, output_api, source_file_filter=None,
cpplint.ProcessFile(file_name, verbose_level)

if cpplint._cpplint_state.error_count > 0:
# cpplint errors currently cannot be counted as errors during upload
# presubmits because some directories only run cpplint during upload and
# therefore are far from cpplint clean.
if input_api.is_committing:
res_type = output_api.PresubmitError
else:
Expand Down Expand Up @@ -894,7 +897,7 @@ def GetPythonUnitTests(input_api, output_api, unit_tests, python3=False):
DEPRECATED.
"""
# We don't want to hinder users from uploading incomplete patches.
if input_api.is_committing:
if input_api.is_committing or input_api.no_diffs:
message_type = output_api.PresubmitError
else:
message_type = output_api.PresubmitNotifyResult
Expand Down Expand Up @@ -1020,7 +1023,7 @@ def GetPylint(input_api,
'Unsupported pylint version: %s' % version
python2 = (version == '1.5')

if input_api.is_committing:
if input_api.is_committing or input_api.no_diffs:
error_type = output_api.PresubmitError
else:
error_type = output_api.PresubmitPromptWarning
Expand Down

0 comments on commit d895d01

Please sign in to comment.