diff --git a/PRESUBMIT.py b/PRESUBMIT.py index d66f5a80b..99c7c36b5 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -27,17 +27,14 @@ def CommonChecks(input_api, output_api): UNIT_TESTS)) output.extend(WasGitClUploadHookModified(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') - + white_list = [r'.*\.py$', r'.*git-try$'] + black_list = list(input_api.DEFAULT_BLACK_LIST) + [ + r'.*cpplint\.py$', r'.*git_cl_repo.*'] output.extend(input_api.canned_checks.RunPylint( input_api, output_api, - source_file_filter=filter_python_sources)) + white_list=white_list, + black_list=black_list)) return output diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index c7c93be6c..51dfcff37 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -450,17 +450,48 @@ 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 +def RunPylint(input_api, output_api, white_list=None, black_list=None): + """Run pylint on python files. + + The default white_list enforces looking only a *.py files. + """ + white_list = white_list or ['.*\.py$'] + black_list = black_list or input_api.DEFAULT_BLACK_LIST + + # Only trigger if there is at least one python file affected. + src_filter = lambda x: input_api.FilterSourceFile(x, white_list, black_list) + if not input_api.AffectedSourceFiles(src_filter): + return [] + # On certain pylint/python version combination, running pylint throws a lot of # warning messages. + import warnings 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)] + # We cannot use AffectedFiles here because we want to test every python + # file on each single python change. It's because a change in a python file + # can break another unmodified file. + # Use code similar to InputApi.FilterSourceFile() + def Find(filepath, filters): + for item in filters: + if input_api.re.match(item, filepath): + return True + return False + + import os + files = [] + for dirpath, dirnames, filenames in os.walk(input_api.PresubmitLocalPath()): + # Passes dirnames in black list to speed up search. + for item in dirnames[:]: + if Find(input_api.os_path.join(dirpath, item), black_list): + dirnames.remove(item) + for item in filenames: + filepath = input_api.os_path.join(dirpath, item) + if Find(filepath, white_list) and not Find(filepath, black_list): + files.append(filepath) + + # Now that at least one python file was modified and all the python files + # were listed, try to run pylint. try: from pylint import lint if lint.Run(sorted(files)): diff --git a/pylintrc b/pylintrc index 371ff4319..6ed25b7ee 100644 --- a/pylintrc +++ b/pylintrc @@ -44,6 +44,7 @@ load-plugins= # R0901: Too many ancestors (8/7) # R0902: Too many instance attributes (N/7) # R0903: Too few public methods (N/2) +# R0904: Too many public methods (N/20) # R0911: Too many return statements (N/6) # R0912: Too many branches (N/12) # R0913: Too many arguments (N/5) @@ -58,7 +59,7 @@ load-plugins= # W0613: Unused argument '' # W0703: Catch "Exception" # W6501: Specify string format arguments as logging function parameters -disable=C0103,C0111,C0302,I0011,R0401,R0801,R0901,R0902,R0903,R0911,R0912,R0913,R0914,R0915,W0122,W0141,W0142,W0402,W0511,W0603,W0613,W0703,W6501 +disable=C0103,C0111,C0302,I0011,R0401,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,W0122,W0141,W0142,W0402,W0511,W0603,W0613,W0703,W6501 [REPORTS] diff --git a/tests/fake_repos.py b/tests/fake_repos.py index be1b84251..d6332cb39 100755 --- a/tests/fake_repos.py +++ b/tests/fake_repos.py @@ -28,9 +28,13 @@ def addKill(): """Add kill() method to subprocess.Popen for python <2.6""" if getattr(subprocess.Popen, 'kill', None): return + # Unable to import 'module' + # pylint: disable=F0401 if sys.platform == 'win32': def kill_win(process): import win32process + # Access to a protected member _handle of a client class + # pylint: disable=W0212 return win32process.TerminateProcess(process._handle, -1) subprocess.Popen.kill = kill_win else: @@ -169,7 +173,6 @@ def commit_svn(repo): '--no-auth-cache', '--username', 'user1', '--password', 'foo'], cwd=repo) out, err = proc.communicate() - last_line = out.splitlines()[-1] match = re.search(r'(\d+)', out) if not match: raise Exception('Commit failed', out, err, proc.returncode) @@ -267,7 +270,8 @@ class FakeRepos(object): logging.debug('Removing %s' % self.trial_dir()) rmtree(self.trial_dir()) - def _genTree(self, root, tree_dict): + @staticmethod + def _genTree(root, tree_dict): """For a dictionary of file contents, generate a filesystem.""" if not os.path.isdir(root): os.makedirs(root) @@ -292,7 +296,6 @@ class FakeRepos(object): try: check_call(['svnadmin', 'create', root]) except OSError: - self.svn_enabled = False return False write(join(root, 'conf', 'svnserve.conf'), '[general]\n' @@ -548,13 +551,13 @@ hooks = [ def _commit_git(self, repo, tree): repo_root = join(self.git_root, repo) self._genTree(repo_root, tree) - hash = commit_git(repo_root) + commit_hash = commit_git(repo_root) if self.git_hashes[repo][-1]: new_tree = self.git_hashes[repo][-1][1].copy() new_tree.update(tree) else: new_tree = tree.copy() - self.git_hashes[repo].append((hash, new_tree)) + self.git_hashes[repo].append((commit_hash, new_tree)) class FakeReposTestBase(unittest.TestCase): diff --git a/tests/gcl_unittest.py b/tests/gcl_unittest.py index 68534c3e6..c532a4e91 100755 --- a/tests/gcl_unittest.py +++ b/tests/gcl_unittest.py @@ -5,6 +5,9 @@ """Unit tests for gcl.py.""" +# pylint is too confused. +# pylint: disable=E1101,E1103,E1120,W0212,W0403 + # Fixes include path. from super_mox import mox, SuperMoxTestBase diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index f49b2d82b..f86bdcba6 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -5,10 +5,11 @@ """Unit tests for gclient_scm.py.""" +# pylint: disable=E1101,E1103,W0403 + # Import before super_mox to keep valid references. from os import rename from shutil import rmtree -import StringIO from subprocess import Popen, PIPE, STDOUT import tempfile import unittest @@ -62,7 +63,7 @@ class BaseTestCase(GCBaseTestCase, SuperMoxTestBase): class SVNWrapperTestCase(BaseTestCase): class OptionsObject(object): - def __init__(self, verbose=False, revision=None): + def __init__(self, verbose=False, revision=None): self.verbose = verbose self.revision = revision self.manually_grab_svn_rev = True @@ -208,7 +209,6 @@ class SVNWrapperTestCase(BaseTestCase): gclient_scm.os.path.islink(file_path).AndReturn(False) gclient_scm.os.path.isdir(file_path).AndReturn(True) gclient_scm.gclient_utils.RemoveDirectory(file_path) - file_list1 = [] gclient_scm.scm.SVN.RunAndGetFileList(options.verbose, ['update', '--revision', 'BASE'], cwd=self.base_path, @@ -340,10 +340,6 @@ class SVNWrapperTestCase(BaseTestCase): def testUpdateSingleCheckoutSVN14(self): options = self.Options(verbose=True) - file_info = { - 'URL': self.url, - 'Revision': 42, - } # Checks to make sure that we support svn co --depth. gclient_scm.scm.SVN.current_version = None @@ -466,7 +462,7 @@ class GitWrapperTestCase(GCBaseTestCase, StdoutCheck, TestCaseUtils, unittest.TestCase): """This class doesn't use pymox.""" class OptionsObject(object): - def __init__(self, verbose=False, revision=None): + def __init__(self, verbose=False, revision=None): self.verbose = verbose self.revision = revision self.manually_grab_svn_rev = True @@ -523,7 +519,8 @@ from :3 def Options(self, *args, **kwargs): return self.OptionsObject(*args, **kwargs) - def CreateGitRepo(self, git_import, path): + @staticmethod + def CreateGitRepo(git_import, path): """Do it for real.""" try: Popen(['git', 'init', '-q'], stdout=PIPE, stderr=STDOUT, @@ -694,9 +691,9 @@ from :3 options = self.Options() expected_file_list = [] for f in ['a', 'b']: - file_path = join(self.base_path, f) - open(file_path, 'a').writelines('touched\n') - expected_file_list.extend([file_path]) + file_path = join(self.base_path, f) + open(file_path, 'a').writelines('touched\n') + expected_file_list.extend([file_path]) scm = gclient_scm.CreateSCM(url=self.url, root_dir=self.root_dir, relpath=self.relpath) file_list = [] @@ -758,7 +755,7 @@ from :3 scm = gclient_scm.CreateSCM(url=self.url, root_dir=self.root_dir, relpath=self.relpath) file_path = join(self.base_path, 'b') - f = open(file_path, 'w').writelines('conflict\n') + open(file_path, 'w').writelines('conflict\n') exception = ( "error: Your local changes to 'b' would be overwritten by merge. " "Aborting.\n" @@ -773,7 +770,8 @@ from :3 scm = gclient_scm.CreateSCM(url=self.url, root_dir=self.root_dir, relpath=self.relpath) file_path = join(self.base_path, 'b') - f = open(file_path, 'w').writelines('conflict\n') + open(file_path, 'w').writelines('conflict\n') + # pylint: disable=W0212 scm._Run(['commit', '-am', 'test'], options) __builtin__.raw_input = lambda x: 'y' exception = ('Conflict while rebasing this branch.\n' diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index 551581a1c..67ef76aa5 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -10,6 +10,8 @@ Shell out 'gclient' and run basic conformance tests. This test assumes GClientSmokeBase.URL_BASE is valid. """ +# pylint: disable=E1103,W0403 + import logging import os import re @@ -127,7 +129,8 @@ class GClientSmokeBase(FakeReposTestBase): self.assertEquals(len(results), len(items), (stdout, items, len(results))) return results - def svnBlockCleanup(self, out): + @staticmethod + def svnBlockCleanup(out): """Work around svn status difference between svn 1.5 and svn 1.6 I don't know why but on Windows they are reversed. So sorts the items.""" for i in xrange(len(out)): @@ -660,12 +663,13 @@ class GClientSmokeSVN(GClientSmokeBase): if not self.enabled: return self.gclient(['config', self.svn_base + 'trunk/src/']) - self.parseGclient(['sync', '--jobs', '1'], - ['running', 'running', - # This is due to the way svn update is called for a - # single file when File() is used in a DEPS file. - ('running', os.path.join(self.root_dir, 'src', 'file', 'other')), - 'running', 'running', 'running', 'running']) + self.parseGclient( + ['sync', '--jobs', '1'], + ['running', 'running', + # This is due to the way svn update is called for a + # single file when File() is used in a DEPS file. + ('running', os.path.join(self.root_dir, 'src', 'file', 'other')), + 'running', 'running', 'running', 'running']) def testInitialCheckoutFailed(self): # Check that gclient can be executed from an arbitrary sub directory if the @@ -883,9 +887,7 @@ class GClientSmokeGIT(GClientSmokeBase): 'src/repo2/repo_renamed: %(base)srepo_3\n' % { 'base': self.git_base, - 'hash1': self.githash('repo_1', 2)[:7], 'hash2': self.githash('repo_2', 1)[:7], - 'hash3': self.githash('repo_3', 2)[:7], }) self.check((out, '', 0), results) results = self.gclient(['revinfo', '--deps', 'mac', '--actual']) @@ -1012,9 +1014,7 @@ class GClientSmokeBoth(GClientSmokeBase): 'src/third_party/foo: %(svn_base)s/third_party/foo@1\n') % { 'svn_base': self.svn_base + 'trunk', 'git_base': self.git_base, - 'hash1': self.githash('repo_1', 2)[:7], 'hash2': self.githash('repo_2', 1)[:7], - 'hash3': self.githash('repo_3', 2)[:7], } self.check((out, '', 0), results) results = self.gclient(['revinfo', '--deps', 'mac', '--actual']) @@ -1149,10 +1149,10 @@ class GClientSmokeFromCheckout(GClientSmokeBase): return self.gclient(['sync']) # TODO(maruel): This is incorrect, it should run on ./ too. - out = self.parseGclient( + self.parseGclient( ['cleanup', '--deps', 'mac', '--verbose', '--jobs', '1'], [('running', join(self.root_dir, 'foo', 'bar'))]) - out = self.parseGclient( + self.parseGclient( ['diff', '--deps', 'mac', '--verbose', '--jobs', '1'], [('running', join(self.root_dir, 'foo', 'bar'))]) diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index aa759ffb0..4b9ebf2a5 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -3,6 +3,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +# pylint: disable=E1101,W0403 + import StringIO # Fixes include path. @@ -76,7 +78,7 @@ class CheckCallAndFilterTestCase(GclientUtilBase): def __init__(self, test_string): self.stdout = StringIO.StringIO(test_string) def wait(self): - pass + pass def _inner(self, args, test_string): cwd = 'bleh' @@ -112,7 +114,8 @@ class CheckCallAndFilterTestCase(GclientUtilBase): self._inner(args, test_string) self.checkstdout('\n________ running \'boo foo bar\' in \'bleh\'\n' 'ahah\naccb\nallo\naddb\n\n' - '________ running \'boo foo bar\' in \'bleh\'\nahah\naccb\nallo\naddb\n') + '________ running \'boo foo bar\' in \'bleh\'\nahah\naccb\nallo\naddb' + '\n') def testNoLF(self): # Exactly as testCheckCallAndFilterAndHeader without trailing \n diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 0a5ea0f3f..d2d2b39e8 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -5,6 +5,9 @@ """Unit tests for presubmit_support.py and presubmit_canned_checks.py.""" +# pylint is too confused. +# pylint: disable=E1101,E1103,W0212,W0403 + import StringIO # Fixes include path. @@ -166,7 +169,8 @@ class PresubmitUnittest(PresubmitTestsBase): ['D', 'boo/flap.h'], ] blat = presubmit.os.path.join(self.fake_root_dir, 'foo', 'blat.cc') - notfound = presubmit.os.path.join(self.fake_root_dir, 'flop', 'notfound.txt') + notfound = presubmit.os.path.join( + self.fake_root_dir, 'flop', 'notfound.txt') flap = presubmit.os.path.join(self.fake_root_dir, 'boo', 'flap.h') binary = presubmit.os.path.join(self.fake_root_dir, 'binary.dll') isdir = presubmit.os.path.join(self.fake_root_dir, 'isdir') @@ -335,11 +339,11 @@ class PresubmitUnittest(PresubmitTestsBase): self.mox.ReplayAll() output = StringIO.StringIO() - input = StringIO.StringIO('y\n') + input_buf = StringIO.StringIO('y\n') change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks(change, False, True, output, input, - None, False)) + self.failIf(presubmit.DoPresubmitChecks( + change, False, True, output, input_buf, None, False)) self.assertEqual(output.getvalue().count('!!'), 2) self.checkstdout('Running presubmit hooks...\n') @@ -355,7 +359,7 @@ class PresubmitUnittest(PresubmitTestsBase): haspresubmit_path = join(self.fake_root_dir, 'haspresubmit', 'PRESUBMIT.py') inherit_path = presubmit.os.path.join(self.fake_root_dir, self._INHERIT_SETTINGS) - for i in range(2): + for _ in range(2): presubmit.os.path.isfile(inherit_path).AndReturn(False) presubmit.os.path.isfile(presubmit_path).AndReturn(True) presubmit.os.path.isfile(haspresubmit_path).AndReturn(True) @@ -368,17 +372,17 @@ class PresubmitUnittest(PresubmitTestsBase): self.mox.ReplayAll() output = StringIO.StringIO() - input = StringIO.StringIO('n\n') # say no to the warning + input_buf = StringIO.StringIO('n\n') # say no to the warning change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks(change, False, True, output, input, - None, True)) + self.failIf(presubmit.DoPresubmitChecks( + change, False, True, output, input_buf, None, True)) self.assertEqual(output.getvalue().count('??'), 2) output = StringIO.StringIO() - input = StringIO.StringIO('y\n') # say yes to the warning - self.failUnless(presubmit.DoPresubmitChecks(change, False, True, output, - input, None, True)) + input_buf = StringIO.StringIO('y\n') # say yes to the warning + self.failUnless(presubmit.DoPresubmitChecks( + change, False, True, output, input_buf, None, True)) self.assertEquals(output.getvalue().count('??'), 2) self.checkstdout('Running presubmit hooks...\nRunning presubmit hooks...\n') @@ -407,11 +411,11 @@ class PresubmitUnittest(PresubmitTestsBase): self.mox.ReplayAll() output = StringIO.StringIO() - input = StringIO.StringIO() # should be unused + input_buf = StringIO.StringIO() # should be unused change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks(change, False, True, output, input, - None, False)) + self.failIf(presubmit.DoPresubmitChecks( + change, False, True, output, input_buf, None, False)) self.assertEqual(output.getvalue().count('??'), 2) self.assertEqual(output.getvalue().count('XX!!XX'), 2) self.assertEqual(output.getvalue().count('(y/N)'), 0) @@ -443,12 +447,12 @@ def CheckChangeOnCommit(input_api, output_api): self.mox.ReplayAll() output = StringIO.StringIO() - input = StringIO.StringIO('y\n') + input_buf = StringIO.StringIO('y\n') # Always fail. change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks(change, False, True, output, input, - DEFAULT_SCRIPT, False)) + self.failIf(presubmit.DoPresubmitChecks( + change, False, True, output, input_buf, DEFAULT_SCRIPT, False)) text = ('Warning, no presubmit.py found.\n' 'Running default presubmit script.\n' '** Presubmit ERRORS **\n!!\n\n' @@ -516,12 +520,12 @@ def CheckChangeOnCommit(input_api, output_api): self.mox.ReplayAll() output = StringIO.StringIO() - input = StringIO.StringIO('y\n') + input_buf = StringIO.StringIO('y\n') change = presubmit.Change( 'foo', "Blah Blah\n\nSTORY=http://tracker.com/42\nBUG=boo\n", self.fake_root_dir, None, 0, 0) - self.failUnless(presubmit.DoPresubmitChecks(change, False, True, output, - input, DEFAULT_SCRIPT, False)) + self.failUnless(presubmit.DoPresubmitChecks( + change, False, True, output, input_buf, DEFAULT_SCRIPT, False)) self.assertEquals(output.getvalue(), ('Warning, no presubmit.py found.\n' 'Running default presubmit script.\n' @@ -817,7 +821,7 @@ class InputApiUnittest(PresubmitTestsBase): def FilterSourceFile(affected_file): return 'a' in affected_file.LocalPath() files = [('A', 'eeaee'), ('M', 'eeabee'), ('M', 'eebcee')] - for (action, item) in files: + for _, item in files: item = presubmit.os.path.join(self.fake_root_dir, item) presubmit.os.path.exists(item).AndReturn(True) presubmit.os.path.isdir(item).AndReturn(False) @@ -839,7 +843,7 @@ class InputApiUnittest(PresubmitTestsBase): white_list = presubmit.InputApi.DEFAULT_BLACK_LIST + (r".*?a.*?",) black_list = [r".*?b.*?"] files = [('A', 'eeaee'), ('M', 'eeabee'), ('M', 'eebcee'), ('M', 'eecaee')] - for (action, item) in files: + for _, item in files: item = presubmit.os.path.join(self.fake_root_dir, item) presubmit.os.path.exists(item).AndReturn(True) presubmit.os.path.isdir(item).AndReturn(False) @@ -928,18 +932,18 @@ class InputApiUnittest(PresubmitTestsBase): input_api.ReadFile(path, 'x') def testReadFileAffectedFileDenied(self): - file = presubmit.AffectedFile('boo', 'M', 'Unrelated') + fileobj = presubmit.AffectedFile('boo', 'M', 'Unrelated') self.mox.ReplayAll() change = presubmit.Change('foo', 'foo', self.fake_root_dir, [('M', 'AA')], 0, 0) input_api = presubmit.InputApi( change, presubmit.os.path.join(self.fake_root_dir, '/p'), False) - self.assertRaises(IOError, input_api.ReadFile, file, 'x') + self.assertRaises(IOError, input_api.ReadFile, fileobj, 'x') def testReadFileAffectedFileAccepted(self): - file = presubmit.AffectedFile('AA/boo', 'M', self.fake_root_dir) - presubmit.gclient_utils.FileRead(file.AbsoluteLocalPath(), 'x' + fileobj = presubmit.AffectedFile('AA/boo', 'M', self.fake_root_dir) + presubmit.gclient_utils.FileRead(fileobj.AbsoluteLocalPath(), 'x' ).AndReturn(None) self.mox.ReplayAll() @@ -947,7 +951,7 @@ class InputApiUnittest(PresubmitTestsBase): 0, 0) input_api = presubmit.InputApi( change, presubmit.os.path.join(self.fake_root_dir, '/p'), False) - input_api.ReadFile(file, 'x') + input_api.ReadFile(fileobj, 'x') class OuputApiUnittest(PresubmitTestsBase): @@ -989,21 +993,21 @@ class OuputApiUnittest(PresubmitTestsBase): self.failUnless(output.getvalue().count('?see?')) output = StringIO.StringIO() - input = StringIO.StringIO('y') + input_buf = StringIO.StringIO('y') warning = presubmit.OutputApi.PresubmitPromptWarning('???') - self.failUnless(warning._Handle(output, input)) + self.failUnless(warning._Handle(output, input_buf)) self.failUnless(output.getvalue().count('???')) output = StringIO.StringIO() - input = StringIO.StringIO('n') + input_buf = StringIO.StringIO('n') warning = presubmit.OutputApi.PresubmitPromptWarning('???') - self.failIf(warning._Handle(output, input)) + self.failIf(warning._Handle(output, input_buf)) self.failUnless(output.getvalue().count('???')) output = StringIO.StringIO() - input = StringIO.StringIO('\n') + input_buf = StringIO.StringIO('\n') warning = presubmit.OutputApi.PresubmitPromptWarning('???') - self.failIf(warning._Handle(output, input)) + self.failIf(warning._Handle(output, input_buf)) self.failUnless(output.getvalue().count('???')) @@ -1064,7 +1068,7 @@ class AffectedFileUnittest(PresubmitTestsBase): self.failUnless(affected_file.IsDirectory()) def testIsTextFile(self): - list = [presubmit.SvnAffectedFile('foo/blat.txt', 'M'), + files = [presubmit.SvnAffectedFile('foo/blat.txt', 'M'), presubmit.SvnAffectedFile('foo/binary.blob', 'M'), presubmit.SvnAffectedFile('blat/flop.txt', 'D')] blat = presubmit.os.path.join('foo', 'blat.txt') @@ -1078,9 +1082,9 @@ class AffectedFileUnittest(PresubmitTestsBase): ).AndReturn('application/octet-stream') self.mox.ReplayAll() - output = filter(lambda x: x.IsTextFile(), list) + output = filter(lambda x: x.IsTextFile(), files) self.failUnless(len(output) == 1) - self.failUnless(list[0] == output[0]) + self.failUnless(files[0] == output[0]) class GclChangeUnittest(PresubmitTestsBase): @@ -1212,7 +1216,7 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(len(results2), 1) self.assertEquals(results2[0].__class__, error_type) - def SvnPropertyTest(self, check, property, value1, value2, committing, + def SvnPropertyTest(self, check, property_name, value1, value2, committing, error_type, use_source_file): change1 = presubmit.SvnChange('mychange', '', self.fake_root_dir, [], 0, 0) input_api1 = self.MockInputApi(change1, committing) @@ -1225,9 +1229,9 @@ class CannedChecksUnittest(PresubmitTestsBase): else: input_api1.AffectedFiles(include_deleted=False).AndReturn(files1) presubmit.scm.SVN.GetFileProperty(presubmit.normpath('foo/bar.cc'), - property).AndReturn(value1) + property_name).AndReturn(value1) presubmit.scm.SVN.GetFileProperty(presubmit.normpath('foo.cc'), - property).AndReturn(value1) + property_name).AndReturn(value1) change2 = presubmit.SvnChange('mychange', '', self.fake_root_dir, [], 0, 0) input_api2 = self.MockInputApi(change2, committing) files2 = [ @@ -1240,9 +1244,9 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api2.AffectedFiles(include_deleted=False).AndReturn(files2) presubmit.scm.SVN.GetFileProperty(presubmit.normpath('foo/bar.cc'), - property).AndReturn(value2) + property_name).AndReturn(value2) presubmit.scm.SVN.GetFileProperty(presubmit.normpath('foo.cc'), - property).AndReturn(value2) + property_name).AndReturn(value2) self.mox.ReplayAll() results1 = check(input_api1, presubmit.OutputApi, None) @@ -1371,7 +1375,7 @@ class CannedChecksUnittest(PresubmitTestsBase): def testCannedCheckLongLines(self): - check = lambda x,y,z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) + check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) self.ContentTest(check, '', 'blah blah blah', presubmit.OutputApi.PresubmitPromptWarning) @@ -1386,7 +1390,8 @@ class CannedChecksUnittest(PresubmitTestsBase): 'svn:eol-style', 'LF', '', False, presubmit.OutputApi.PresubmitNotifyResult, True) - def _LicenseCheck(self, text, license, committing, expected_result, **kwargs): + def _LicenseCheck(self, text, license_text, committing, expected_result, + **kwargs): change = self.mox.CreateMock(presubmit.SvnChange) change.scm = 'svn' input_api = self.MockInputApi(change, committing) @@ -1398,7 +1403,8 @@ class CannedChecksUnittest(PresubmitTestsBase): self.mox.ReplayAll() result = presubmit_canned_checks.CheckLicense( - input_api, presubmit.OutputApi, license, source_file_filter=42, + input_api, presubmit.OutputApi, license_text, + source_file_filter=42, **kwargs) if expected_result: self.assertEqual(len(result), 1) @@ -1413,11 +1419,11 @@ class CannedChecksUnittest(PresubmitTestsBase): "# All Rights Reserved.\n" "print 'foo'\n" ) - license = ( + license_text = ( r".*? Copyright \(c\) 2037 Nobody." "\n" r".*? All Rights Reserved\." "\n" ) - self._LicenseCheck(text, license, True, None) + self._LicenseCheck(text, license_text, True, None) def testCheckLicenseFailCommit(self): text = ( @@ -1426,11 +1432,11 @@ class CannedChecksUnittest(PresubmitTestsBase): "# All Rights Reserved.\n" "print 'foo'\n" ) - license = ( + license_text = ( r".*? Copyright \(c\) 0007 Nobody." "\n" r".*? All Rights Reserved\." "\n" ) - self._LicenseCheck(text, license, True, + self._LicenseCheck(text, license_text, True, presubmit.OutputApi.PresubmitPromptWarning) def testCheckLicenseFailUpload(self): @@ -1440,20 +1446,20 @@ class CannedChecksUnittest(PresubmitTestsBase): "# All Rights Reserved.\n" "print 'foo'\n" ) - license = ( + license_text = ( r".*? Copyright \(c\) 0007 Nobody." "\n" r".*? All Rights Reserved\." "\n" ) - self._LicenseCheck(text, license, False, + self._LicenseCheck(text, license_text, False, presubmit.OutputApi.PresubmitNotifyResult) def testCheckLicenseEmptySuccess(self): text = '' - license = ( + license_text = ( r".*? Copyright \(c\) 2037 Nobody." "\n" r".*? All Rights Reserved\." "\n" ) - self._LicenseCheck(text, license, True, None, accept_empty_files=True) + self._LicenseCheck(text, license_text, True, None, accept_empty_files=True) def testCannedCheckSvnAccidentalSubmission(self): modified_dir_file = 'foo/' diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 19339b520..9d71d245f 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -5,11 +5,10 @@ """Unit tests for scm.py.""" -from shutil import rmtree -import tempfile +# pylint: disable=E1101,W0403 # Fixes include path. -from super_mox import mox, TestCaseUtils, SuperMoxTestBase +from super_mox import SuperMoxTestBase import scm @@ -171,7 +170,7 @@ class SVNTestCase(BaseSCMTestCase): self.assertEqual(file_info, expected) def testCaptureStatus(self): - text =r""" + text = r""" diff --git a/tests/super_mox.py b/tests/super_mox.py index 2620f8f57..b1989c295 100644 --- a/tests/super_mox.py +++ b/tests/super_mox.py @@ -5,7 +5,6 @@ """Simplify unit tests based on pymox.""" -import __builtin__ import os import random import shutil @@ -41,10 +40,10 @@ class TestCaseUtils(object): ## Some utilities for generating arbitrary arguments. def String(self, max_length): return ''.join([self._RANDOM_CHOICE(self._STRING_LETTERS) - for x in xrange(self._RANDOM_RANDINT(1, max_length))]) + for _ in xrange(self._RANDOM_RANDINT(1, max_length))]) def Strings(self, max_arg_count, max_arg_length): - return [self.String(max_arg_length) for x in xrange(max_arg_count)] + return [self.String(max_arg_length) for _ in xrange(max_arg_count)] def Args(self, max_arg_count=8, max_arg_length=16): return self.Strings(max_arg_count, @@ -75,7 +74,8 @@ class TestCaseUtils(object): if actual_members != expected_members: diff = ([i for i in actual_members if i not in expected_members] + [i for i in expected_members if i not in actual_members]) - print>>sys.stderr, diff + print >> sys.stderr, diff + # pylint: disable=E1101 self.assertEqual(actual_members, expected_members) def setUp(self): @@ -97,6 +97,7 @@ class StdoutCheck(object): def tearDown(self): try: # If sys.stdout was used, self.checkstdout() must be called. + # pylint: disable=E1101 self.assertEquals('', sys.stdout.getvalue()) except AttributeError: pass @@ -105,6 +106,7 @@ class StdoutCheck(object): def checkstdout(self, expected): value = sys.stdout.getvalue() sys.stdout.close() + # pylint: disable=E1101 self.assertEquals(expected, value) @@ -113,7 +115,6 @@ class SuperMoxTestBase(TestCaseUtils, StdoutCheck, mox.MoxTestBase): """Patch a few functions with know side-effects.""" TestCaseUtils.setUp(self) mox.MoxTestBase.setUp(self) - #self.mox.StubOutWithMock(__builtin__, 'open') os_to_mock = ('chdir', 'chown', 'close', 'closerange', 'dup', 'dup2', 'fchdir', 'fchmod', 'fchown', 'fdopen', 'getcwd', 'getpid', 'lseek', 'makedirs', 'mkdir', 'open', 'popen', 'popen2', 'popen3', 'popen4', @@ -144,9 +145,9 @@ class SuperMoxTestBase(TestCaseUtils, StdoutCheck, mox.MoxTestBase): except TypeError: raise TypeError('Couldn\'t mock %s in %s' % (item, parent.__name__)) - def UnMock(self, object, name): + def UnMock(self, obj, name): """Restore an object inside a test.""" for (parent, old_child, child_name) in self.mox.stubs.cache: - if parent == object and child_name == name: + if parent == obj and child_name == name: setattr(parent, child_name, old_child) break diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index 4ee984a85..c57cceea6 100755 --- a/tests/trychange_unittest.py +++ b/tests/trychange_unittest.py @@ -5,8 +5,10 @@ """Unit tests for trychange.py.""" +# pylint: disable=E1103,W0403 + # Fixes include path. -from super_mox import mox, SuperMoxTestBase +from super_mox import SuperMoxTestBase import trychange diff --git a/tests/watchlists_unittest.py b/tests/watchlists_unittest.py index 22afe2feb..edad68b28 100755 --- a/tests/watchlists_unittest.py +++ b/tests/watchlists_unittest.py @@ -5,6 +5,9 @@ """Unit tests for watchlists.py.""" +# pylint is too confused. +# pylint: disable=E1103,E1120,W0212,W0403 + import super_mox import watchlists