From 3410d917f3a0c9e38577b86b0dab6e07b20d7e61 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Tue, 9 Jun 2009 20:56:16 +0000 Subject: [PATCH] Add InputApi.AffectedSourceFile() and custom filtering support. TEST=none BUG=none Review URL: http://codereview.chromium.org/119365 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@17975 0039d316-1c4b-4281-b951-d872f2087c98 --- presubmit_canned_checks.py | 38 +++++---- presubmit_support.py | 66 +++++++++++++++- tests/presubmit_unittest.py | 148 +++++++++++++++++++++++++----------- 3 files changed, 190 insertions(+), 62 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 14c5a1b37..f31b1687a 100755 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -6,6 +6,8 @@ """Generic presubmit checks that can be reused by other presubmit checks.""" +### Description checks + def CheckChangeHasTestField(input_api, output_api): """Requires that the changelist have a TEST= field.""" if input_api.change.TEST: @@ -51,40 +53,36 @@ def CheckDoNotSubmitInDescription(input_api, output_api): return [] +### Content checks + def CheckDoNotSubmitInFiles(input_api, output_api): """Checks that the user didn't add 'DO NOT ' + 'SUBMIT' to any files.""" keyword = 'DO NOT ' + 'SUBMIT' - for f, line_num, line in input_api.RightHandSideLines(): + # We want to check every text files, not just source files. + for f, line_num, line in input_api.RightHandSideLines(lambda x: x): if keyword in line: text = 'Found ' + keyword + ' in %s, line %s' % (f.LocalPath(), line_num) return [output_api.PresubmitError(text)] return [] -def CheckDoNotSubmit(input_api, output_api): - return ( - CheckDoNotSubmitInDescription(input_api, output_api) + - CheckDoNotSubmitInFiles(input_api, output_api) - ) - - -def CheckChangeHasNoCR(input_api, output_api): +def CheckChangeHasNoCR(input_api, output_api, source_file_filter=None): """Checks that there are no \r, \r\n (CR or CRLF) characters in any of the - text files to be submitted. + source files to be submitted. """ outputs = [] - for f in input_api.AffectedTextFiles(): + for f in input_api.AffectedSourceFiles(source_file_filter): if '\r' in input_api.ReadFile(f, 'rb'): outputs.append(output_api.PresubmitPromptWarning( "Found a CR character in %s" % f.LocalPath())) return outputs -def CheckChangeHasNoTabs(input_api, output_api): +def CheckChangeHasNoTabs(input_api, output_api, source_file_filter=None): """Checks that there are no tab characters in any of the text files to be submitted. """ - for f, line_num, line in input_api.RightHandSideLines(): + for f, line_num, line in input_api.RightHandSideLines(source_file_filter): if '\t' in line: return [output_api.PresubmitPromptWarning( "Found a tab character in %s, line %s" % @@ -92,12 +90,12 @@ def CheckChangeHasNoTabs(input_api, output_api): return [] -def CheckLongLines(input_api, output_api, maxlen=80): +def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None): """Checks that there aren't any lines longer than maxlen characters in any of the text files to be submitted. """ bad = [] - for f, line_num, line in input_api.RightHandSideLines(): + for f, line_num, line in input_api.RightHandSideLines(source_file_filter): # Allow lines with http://, https:// and #define/#pragma/#include/#if/#endif # to exceed the maxlen rule. if (len(line) > maxlen and @@ -121,6 +119,15 @@ def CheckLongLines(input_api, output_api, maxlen=80): return [] +### Other checks + +def CheckDoNotSubmit(input_api, output_api): + return ( + CheckDoNotSubmitInDescription(input_api, output_api) + + CheckDoNotSubmitInFiles(input_api, output_api) + ) + + def CheckTreeIsOpen(input_api, output_api, url, closed): """Checks that an url's content doesn't match a regexp that would mean that the tree is closed.""" @@ -145,6 +152,7 @@ def _RunPythonUnitTests_LoadTests(input_api, module_name): module = getattr(module, part) return input_api.unittest.TestLoader().loadTestsFromModule(module)._tests + def RunPythonUnitTests(input_api, output_api, unit_tests): """Imports the unit_tests modules and run them.""" # We don't want to hinder users from uploading incomplete patches. diff --git a/presubmit_support.py b/presubmit_support.py index 0dbe30abf..ccbca7405 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -137,6 +137,37 @@ class InputApi(object): know stuff about the change they're looking at. """ + # File extensions that are considered source files from a style guide + # perspective. Don't modify this list from a presubmit script! + DEFAULT_WHITE_LIST = ( + # C++ and friends + r".*\.c", r".*\.cc", r".*\.cpp", r".*\.h", r".*\.m", r".*\.mm", + r".*\.inl", r".*\.asm", r".*\.hxx", r".*\.hpp", + # Scripts + r".*\.js", r".*\.py", r".*\.json", r".*\.sh", r".*\.rb", + # No extension at all + r"(^|.*[\\\/])[^.]+$", + # Other + r".*\.java", r".*\.mk", r".*\.am", + ) + + # Path regexp that should be excluded from being considered containing source + # files. Don't modify this list from a presubmit script! + DEFAULT_BLACK_LIST = ( + r".*\bexperimental[\\\/].*", + r".*\bthird_party[\\\/].*", + # Output directories (just in case) + r".*\bDebug[\\\/].*", + r".*\bRelease[\\\/].*", + r".*\bxcodebuild[\\\/].*", + r".*\bsconsbuild[\\\/].*", + # All caps files like README and LICENCE. + r".*\b[A-Z0-9_]+", + # SCM (can happen in dual SCM configuration) + r".*\b\.git[\\\/].*", + r".*\b\.svn[\\\/].*", + ) + def __init__(self, change, presubmit_path, is_committing): """Builds an InputApi object. @@ -250,7 +281,35 @@ class InputApi(object): return filter(lambda x: x.IsTextFile(), self.AffectedFiles(include_dirs=False, include_deletes=False)) - def RightHandSideLines(self): + def FilterSourceFile(self, affected_file, white_list=None, black_list=None): + """Filters out files that aren't considered "source file". + + If white_list or black_list is None, InputApi.DEFAULT_WHITE_LIST + and InputApi.DEFAULT_BLACK_LIST is used respectively. + + The lists will be compiled as regular expression and + AffectedFile.LocalPath() needs to pass both list. + + Note: Copy-paste this function to suit your needs or use a lambda function. + """ + def Find(affected_file, list): + for item in list: + if self.re.match(item, affected_file.LocalPath()): + return True + return False + return (Find(affected_file, white_list or self.DEFAULT_WHITE_LIST) and + not Find(affected_file, black_list or self.DEFAULT_BLACK_LIST)) + + def AffectedSourceFiles(self, source_file): + """Filter the list of AffectedTextFiles by the function source_file. + + If source_file is None, InputApi.FilterSourceFile() is used. + """ + if not source_file: + source_file = self.FilterSourceFile + return filter(source_file, self.AffectedTextFiles()) + + def RightHandSideLines(self, source_file_filter=None): """An iterator over all text lines in "new" version of changed files. Only lists lines from new or modified text files in the change that are @@ -267,9 +326,8 @@ class InputApi(object): Note: The cariage return (LF or CR) is stripped off. """ - return InputApi._RightHandSideLinesImpl( - filter(lambda x: x.IsTextFile(), - self.AffectedFiles(include_deletes=False))) + files = self.AffectedSourceFiles(source_file_filter) + return InputApi._RightHandSideLinesImpl(files) def ReadFile(self, file, mode='r'): """Reads an arbitrary file. diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index cc5825f42..f0ea1d587 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -60,17 +60,19 @@ def CheckChangeOnUpload(input_api, output_api): def setUp(self): mox.MoxTestBase.setUp(self) self.mox.StubOutWithMock(presubmit, 'warnings') - # Stub out 'os' but keep os.path.dirname/join/normpath and os.sep. + # Stub out 'os' but keep os.path.dirname/join/normpath/splitext and os.sep. os_sep = presubmit.os.sep os_path_join = presubmit.os.path.join os_path_dirname = presubmit.os.path.dirname os_path_normpath = presubmit.os.path.normpath + os_path_splitext = presubmit.os.path.splitext self.mox.StubOutWithMock(presubmit, 'os') self.mox.StubOutWithMock(presubmit.os, 'path') presubmit.os.sep = os_sep presubmit.os.path.join = os_path_join presubmit.os.path.dirname = os_path_dirname presubmit.os.path.normpath = os_path_normpath + presubmit.os.path.splitext = os_path_splitext self.mox.StubOutWithMock(presubmit, 'sys') # Special mocks. def MockAbsPath(f): @@ -523,9 +525,12 @@ class InputApiUnittest(PresubmitTestsBase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles', - 'DepotToLocalPath', 'LocalPaths', 'LocalToDepotPath', - 'PresubmitLocalPath', 'RightHandSideLines', 'ReadFile', 'ServerPaths', + 'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedSourceFiles', + 'AffectedTextFiles', + 'DEFAULT_BLACK_LIST', 'DEFAULT_WHITE_LIST', + 'DepotToLocalPath', 'FilterSourceFile', 'LocalPaths', + 'LocalToDepotPath', + 'PresubmitLocalPath', 'ReadFile', 'RightHandSideLines', 'ServerPaths', 'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', 'is_committing', 'marshal', 'os_path', 'pickle', 'platform', 're', 'subprocess', 'tempfile', 'traceback', 'unittest', 'urllib2', @@ -573,24 +578,30 @@ class InputApiUnittest(PresubmitTestsBase): 'BUG=123', ' STORY =http://foo/ \t', 'and some more regular text') + blat = join('foo', 'blat.cc') + readme = join('foo', 'blat', 'READ_ME2') + binary = join('foo', 'blat', 'binary.dll') + weird = join('foo', 'blat', 'weird.xyz') + third_party = join('foo', 'third_party', 'third.cc') + another = join('foo', 'blat', 'another.h') + beingdeleted = join('foo', 'mat', 'beingdeleted.txt') + notfound = join('flop', 'notfound.txt') + flap = join('boo', 'flap.h') files = [ - ['A', join('foo', 'blat.cc')], - ['M', join('foo', 'blat', 'binary.dll')], + ['A', blat], + ['M', readme], + ['M', binary], + ['M', weird], + ['M', another], + ['M', third_party], ['D', 'foo/mat/beingdeleted.txt'], ['M', 'flop/notfound.txt'], ['A', 'boo/flap.h'], ] - - blat = join('foo', 'blat.cc') - binary = join('foo', 'blat', 'binary.dll') - beingdeleted = join('foo', 'mat', 'beingdeleted.txt') - notfound = join('flop', 'notfound.txt') - flap = join('boo', 'flap.h') - presubmit.os.path.exists(blat).AndReturn(True) - presubmit.os.path.isdir(blat).AndReturn(False) - presubmit.os.path.exists(binary).AndReturn(True) - presubmit.os.path.isdir(binary).AndReturn(False) + for i in (blat, readme, binary, weird, another, third_party): + presubmit.os.path.exists(i).AndReturn(True) + presubmit.os.path.isdir(i).AndReturn(False) presubmit.os.path.exists(beingdeleted).AndReturn(False) presubmit.os.path.exists(notfound).AndReturn(False) presubmit.os.path.exists(flap).AndReturn(True) @@ -598,30 +609,80 @@ class InputApiUnittest(PresubmitTestsBase): presubmit.gclient.CaptureSVNInfo(beingdeleted).AndReturn({}) presubmit.gclient.CaptureSVNInfo(notfound).AndReturn({}) presubmit.gcl.GetSVNFileProperty(blat, 'svn:mime-type').AndReturn(None) + presubmit.gcl.GetSVNFileProperty(readme, 'svn:mime-type').AndReturn(None) presubmit.gcl.GetSVNFileProperty(binary, 'svn:mime-type').AndReturn( 'application/octet-stream') + presubmit.gcl.GetSVNFileProperty(weird, 'svn:mime-type').AndReturn(None) + presubmit.gcl.GetSVNFileProperty(another, 'svn:mime-type').AndReturn(None) + presubmit.gcl.GetSVNFileProperty(third_party, 'svn:mime-type' + ).AndReturn(None) presubmit.gcl.ReadFile(blat).AndReturn('whatever\ncookie') + presubmit.gcl.ReadFile(another).AndReturn('whatever\ncookie2') self.mox.ReplayAll() ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) change = presubmit.GclChange(ci) - api = presubmit.InputApi(change, 'foo/PRESUBMIT.py', False) - affected_files = api.AffectedFiles() - self.assertEquals(len(affected_files), 3) - self.assertEquals(affected_files[0].LocalPath(), - presubmit.normpath('foo/blat.cc')) - self.assertEquals(affected_files[1].LocalPath(), - presubmit.normpath('foo/blat/binary.dll')) - self.assertEquals(affected_files[2].LocalPath(), - presubmit.normpath('foo/mat/beingdeleted.txt')) - rhs_lines = [] - for line in api.RightHandSideLines(): - rhs_lines.append(line) - self.assertEquals(len(rhs_lines), 2) - self.assertEqual(rhs_lines[0][0].LocalPath(), - presubmit.normpath('foo/blat.cc')) + input_api = presubmit.InputApi(change, 'foo/PRESUBMIT.py', False) + # Doesn't filter much + got_files = input_api.AffectedFiles() + self.assertEquals(len(got_files), 7) + self.assertEquals(got_files[0].LocalPath(), presubmit.normpath(blat)) + self.assertEquals(got_files[1].LocalPath(), presubmit.normpath(readme)) + self.assertEquals(got_files[2].LocalPath(), presubmit.normpath(binary)) + self.assertEquals(got_files[3].LocalPath(), presubmit.normpath(weird)) + self.assertEquals(got_files[4].LocalPath(), presubmit.normpath(another)) + self.assertEquals(got_files[5].LocalPath(), presubmit.normpath(third_party)) + self.assertEquals(got_files[6].LocalPath(), + presubmit.normpath(beingdeleted)) + # Ignores weird because of whitelist, third_party because of blacklist, + # binary isn't a text file and beingdeleted doesn't exist. The rest is + # outside foo/. + rhs_lines = [x for x in input_api.RightHandSideLines(None)] + self.assertEquals(len(rhs_lines), 4) + self.assertEqual(rhs_lines[0][0].LocalPath(), presubmit.normpath(blat)) + self.assertEqual(rhs_lines[1][0].LocalPath(), presubmit.normpath(blat)) + self.assertEqual(rhs_lines[2][0].LocalPath(), presubmit.normpath(another)) + self.assertEqual(rhs_lines[3][0].LocalPath(), presubmit.normpath(another)) + + def testCustomFilter(self): + def FilterSourceFile(affected_file): + return 'a' in affected_file.LocalPath() + files = [('A', 'eeaee'), ('M', 'eeabee'), ('M', 'eebcee')] + for (action, item) in files: + presubmit.os.path.exists(item).AndReturn(True) + presubmit.os.path.isdir(item).AndReturn(False) + presubmit.gcl.GetSVNFileProperty(item, 'svn:mime-type').AndReturn(None) + self.mox.ReplayAll() + + ci = presubmit.gcl.ChangeInfo('mychange', 0, 0, '', files) + change = presubmit.GclChange(ci) + input_api = presubmit.InputApi(change, './PRESUBMIT.py', False) + got_files = input_api.AffectedSourceFiles(FilterSourceFile) + self.assertEquals(len(got_files), 2) + self.assertEquals(got_files[0].LocalPath(), 'eeaee') + self.assertEquals(got_files[1].LocalPath(), 'eeabee') + + def testLambdaFilter(self): + 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: + presubmit.os.path.exists(item).AndReturn(True) + presubmit.os.path.isdir(item).AndReturn(False) + presubmit.gcl.GetSVNFileProperty(item, 'svn:mime-type').AndReturn(None) + self.mox.ReplayAll() + + ci = presubmit.gcl.ChangeInfo('mychange', 0, 0, '', files) + change = presubmit.GclChange(ci) + input_api = presubmit.InputApi(change, './PRESUBMIT.py', False) + # Sample usage of overiding the default white and black lists. + got_files = input_api.AffectedSourceFiles( + lambda x: input_api.FilterSourceFile(x, white_list, black_list)) + self.assertEquals(len(got_files), 2) + self.assertEquals(got_files[0].LocalPath(), 'eeaee') + self.assertEquals(got_files[1].LocalPath(), 'eecaee') def testGetAbsoluteLocalPath(self): join = presubmit.os.path.join @@ -905,7 +966,7 @@ class CannedChecksUnittest(PresubmitTestsBase): (affected_file, 43, 'yer'), (affected_file, 23, 'ya'), ] - input_api1.RightHandSideLines().AndReturn(output1) + input_api1.RightHandSideLines(mox.IgnoreArg()).AndReturn(output1) input_api2 = self.MockInputApi() input_api2.change = self.MakeBasicChange('foo', 'Foo\n') output2 = [ @@ -913,12 +974,12 @@ class CannedChecksUnittest(PresubmitTestsBase): (affected_file, 43, 'yer'), (affected_file, 23, 'ya'), ] - input_api2.RightHandSideLines().AndReturn(output2) + input_api2.RightHandSideLines(mox.IgnoreArg()).AndReturn(output2) self.mox.ReplayAll() - results1 = check(input_api1, presubmit.OutputApi) + results1 = check(input_api1, presubmit.OutputApi, None) self.assertEquals(results1, []) - results2 = check(input_api2, presubmit.OutputApi) + results2 = check(input_api2, presubmit.OutputApi, None) self.assertEquals(len(results2), 1) self.assertEquals(results2[0].__class__, error_type) @@ -948,31 +1009,32 @@ class CannedChecksUnittest(PresubmitTestsBase): presubmit.OutputApi.PresubmitError) def testCannedCheckDoNotSubmitInFiles(self): - self.TestContent(presubmit_canned_checks.CheckDoNotSubmitInFiles, - 'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT', - presubmit.OutputApi.PresubmitError) + self.TestContent( + lambda x,y,z: presubmit_canned_checks.CheckDoNotSubmitInFiles(x, y), + 'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT', + presubmit.OutputApi.PresubmitError) def testCheckChangeHasNoCR(self): input_api1 = self.MockInputApi() self.mox.StubOutWithMock(input_api1, 'ReadFile') input_api1.change = self.MakeBasicChange('foo', 'Foo\n') affected_file1 = self.mox.CreateMock(presubmit.SvnAffectedFile) - input_api1.AffectedTextFiles().AndReturn([affected_file1]) + input_api1.AffectedSourceFiles(None).AndReturn([affected_file1]) input_api1.ReadFile(affected_file1, 'rb').AndReturn("Hey!\nHo!\n") input_api2 = self.MockInputApi() self.mox.StubOutWithMock(input_api2, 'ReadFile') input_api2.change = self.MakeBasicChange('foo', 'Foo\n') affected_file2 = self.mox.CreateMock(presubmit.SvnAffectedFile) - input_api2.AffectedTextFiles().AndReturn([affected_file2]) + input_api2.AffectedSourceFiles(None).AndReturn([affected_file2]) input_api2.ReadFile(affected_file2, 'rb').AndReturn("Hey!\r\nHo!\r\n") affected_file2.LocalPath().AndReturn('bar.cc') self.mox.ReplayAll() results = presubmit_canned_checks.CheckChangeHasNoCR( - input_api1, presubmit.OutputApi) + input_api1, presubmit.OutputApi, None) self.assertEquals(results, []) results2 = presubmit_canned_checks.CheckChangeHasNoCR( - input_api2, presubmit.OutputApi) + input_api2, presubmit.OutputApi, None) self.assertEquals(len(results2), 1) self.assertEquals(results2[0].__class__, presubmit.OutputApi.PresubmitPromptWarning) @@ -983,7 +1045,7 @@ class CannedChecksUnittest(PresubmitTestsBase): presubmit.OutputApi.PresubmitPromptWarning) def testCannedCheckLongLines(self): - check = lambda x,y: presubmit_canned_checks.CheckLongLines(x, y, 10) + check = lambda x,y,z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) self.TestContent(check, '', 'blah blah blah', presubmit.OutputApi.PresubmitPromptWarning)