diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 9531deee3..d66f5a80b 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -19,27 +19,41 @@ UNIT_TESTS = [ 'tests.watchlists_unittest', ] -def CheckChangeOnUpload(input_api, output_api): +def CommonChecks(input_api, output_api): output = [] - output.extend(input_api.canned_checks.RunPythonUnitTests(input_api, - output_api, - UNIT_TESTS)) + output.extend(input_api.canned_checks.RunPythonUnitTests( + input_api, + output_api, + UNIT_TESTS)) output.extend(WasGitClUploadHookModified(input_api, output_api)) - output.extend(RunPylint(input_api, output_api)) + + def filter_python_sources(affected_file): + filepath = affected_file.LocalPath() + return ((filepath.endswith('.py') and + filepath != 'cpplint.py' and + not filepath.startswith('tests')) or + filepath == 'git-try') + + output.extend(input_api.canned_checks.RunPylint( + input_api, + output_api, + source_file_filter=filter_python_sources)) return output +def CheckChangeOnUpload(input_api, output_api): + return CommonChecks(input_api, output_api) + + def CheckChangeOnCommit(input_api, output_api): output = [] - output.extend(input_api.canned_checks.RunPythonUnitTests(input_api, - output_api, - UNIT_TESTS)) - output.extend(input_api.canned_checks.CheckDoNotSubmit(input_api, - output_api)) - output.extend(WasGitClUploadHookModified(input_api, output_api)) - output.extend(RunPylint(input_api, output_api)) + output.extend(CommonChecks(input_api, output_api)) + output.extend(input_api.canned_checks.CheckDoNotSubmit( + input_api, + output_api)) return output + def WasGitClUploadHookModified(input_api, output_api): for affected_file in input_api.AffectedSourceFiles(None): if (input_api.os_path.basename(affected_file.LocalPath()) == @@ -48,27 +62,3 @@ def WasGitClUploadHookModified(input_api, output_api): 'Don\'t forget to fix git-cl to download the newest version of ' 'git-cl-upload-hook')] return [] - -def RunPylint(input_api, output_api): - import glob - files = glob.glob('*.py') - # It's a python script - files.append('git-try') - # It uses non-standard pylint exceptions that makes pylint always fail. - files.remove('cpplint.py') - try: - proc = input_api.subprocess.Popen(['pylint'] + sorted(files)) - proc.communicate() - if proc.returncode: - return [output_api.PresubmitError('Fix pylint errors first.')] - return [] - except OSError: - if input_api.platform == 'win32': - return [output_api.PresubmitNotifyResult( - 'Warning: Can\'t run pylint because it is not installed. Please ' - 'install manually\n' - 'Cannot do static analysis of python files.')] - return [output_api.PresubmitError( - 'Please install pylint with "sudo apt-get install python-setuptools; ' - 'sudo easy_install pylint"\n' - 'Cannot do static analysis of python files.')] diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 2dec66b4a..c7c93be6c 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -450,6 +450,36 @@ def RunPythonUnitTests(input_api, output_api, unit_tests): return [] +def RunPylint(input_api, output_api, source_file_filter=None): + """Run pylint on python files.""" + import warnings + # On certain pylint/python version combination, running pylint throws a lot of + # warning messages. + warnings.filterwarnings('ignore', category=DeprecationWarning) + try: + if not source_file_filter: + source_file_filter = lambda f: f.LocalPath().endswith('.py') + files = [f.LocalPath() + for f in input_api.AffectedSourceFiles(source_file_filter)] + try: + from pylint import lint + if lint.Run(sorted(files)): + return [output_api.PresubmitPromptWarning('Fix pylint errors first.')] + return [] + except ImportError: + if input_api.platform == 'win32': + return [output_api.PresubmitNotifyResult( + 'Warning: Can\'t run pylint because it is not installed. Please ' + 'install manually\n' + 'Cannot do static analysis of python files.')] + return [output_api.PresubmitError( + 'Please install pylint with "sudo apt-get install python-setuptools; ' + 'sudo easy_install pylint"\n' + 'Cannot do static analysis of python files.')] + finally: + warnings.filterwarnings('default', category=DeprecationWarning) + + def CheckRietveldTryJobExecution(input_api, output_api, host_url, platforms, owner): if not input_api.is_committing: diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 8e59e8915..0a5ea0f3f 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -1137,6 +1137,7 @@ class CannedChecksUnittest(PresubmitTestsBase): 'CheckDoNotSubmit', 'CheckDoNotSubmitInDescription', 'CheckDoNotSubmitInFiles', 'CheckLongLines', 'CheckTreeIsOpen', 'RunPythonUnitTests', + 'RunPylint', 'CheckBuildbotPendingBuilds', 'CheckRietveldTryJobExecution', ] # If this test fails, you should add the relevant test.