diff --git a/presubmit_support.py b/presubmit_support.py index 6cbdfed14..a95d8557f 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -222,41 +222,16 @@ class InputApi(object): if depot_path: return depot_path - @staticmethod - def FilterTextFiles(affected_files, include_deletes=True): - """Filters out all except text files and optionally also filters out - deleted files. - - Args: - affected_files: List of AffectedFiles objects. - include_deletes: If false, deleted files will be filtered out. - - Returns: - Filtered list of AffectedFiles objects. - """ - output_files = [] - for af in affected_files: - if include_deletes or af.Action() != 'D': - path = af.AbsoluteLocalPath() - mime_type = gcl.GetSVNFileProperty(path, 'svn:mime-type') - if not mime_type or mime_type.startswith('text/'): - output_files.append(af) - return output_files - def AffectedFiles(self, include_dirs=False, include_deletes=True): """Same as input_api.change.AffectedFiles() except only lists files (and optionally directories) in the same directory as the current presubmit script, or subdirectories thereof. """ - output_files = [] dir_with_slash = normpath("%s/" % self.PresubmitLocalPath()) if len(dir_with_slash) == 1: dir_with_slash = '' - for af in self.change.AffectedFiles(include_dirs, include_deletes): - af_path = normpath(af.LocalPath()) - if af_path.startswith(dir_with_slash): - output_files.append(af) - return output_files + return filter(lambda x: normpath(x.LocalPath()).startswith(dir_with_slash), + self.change.AffectedFiles(include_dirs, include_deletes)) def LocalPaths(self, include_dirs=False): """Returns local paths of input_api.AffectedFiles().""" @@ -270,17 +245,18 @@ class InputApi(object): """Returns server paths of input_api.AffectedFiles().""" return [af.ServerPath() for af in self.AffectedFiles(include_dirs)] - @deprecated - def AffectedTextFiles(self, include_deletes=True): + def AffectedTextFiles(self, include_deletes=None): """Same as input_api.change.AffectedTextFiles() except only lists files in the same directory as the current presubmit script, or subdirectories thereof. - - Warning: This function retrieves the svn property on each file so it can be - slow for large change lists. """ - return InputApi.FilterTextFiles(self.AffectedFiles(include_dirs=False), - include_deletes) + if include_deletes is not None: + warnings.warn("AffectedTextFiles(include_deletes=%s)" + " is deprecated and ignored" % str(include_deletes), + category=DeprecationWarning, + stacklevel=2) + return filter(lambda x: x.IsTextFile(), + self.AffectedFiles(include_dirs=False, include_deletes=False)) def RightHandSideLines(self): """An iterator over all text lines in "new" version of changed files. @@ -360,7 +336,9 @@ class AffectedFile(object): return self._properties.get(property_name, None) def IsTextFile(self): - """Returns True if the file is a text file and not a binary file.""" + """Returns True if the file is a text file and not a binary file. + + Deleted files are not text file.""" raise NotImplementedError() # Implement when needed def NewContents(self): @@ -529,18 +507,15 @@ class GclChange(object): else: return filter(lambda x: x.Action() != 'D', affected) - @deprecated - def AffectedTextFiles(self, include_deletes=True): - """Return a list of the text files in a change. - - It's common to want to iterate over only the text files. - - Args: - include_deletes: Controls whether to return files with "delete" actions, - which commonly aren't relevant to presubmit scripts. - """ - return InputApi.FilterTextFiles(self.AffectedFiles(include_dirs=False), - include_deletes) + def AffectedTextFiles(self, include_deletes=None): + """Return a list of the existing text files in a change.""" + if include_deletes is not None: + warnings.warn("AffectedTextFiles(include_deletes=%s)" + " is deprecated and ignored" % str(include_deletes), + category=DeprecationWarning, + stacklevel=2) + return filter(lambda x: x.IsTextFile(), + self.AffectedFiles(include_dirs=False, include_deletes=False)) def LocalPaths(self, include_dirs=False): """Convenience function.""" diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index f7515db3a..fc088a6ff 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -9,7 +9,6 @@ import os import StringIO import sys import unittest -import warnings # Local imports import __init__ @@ -20,15 +19,11 @@ import presubmit_canned_checks mox = __init__.mox -class PresubmitTestsBase(unittest.TestCase): +class PresubmitTestsBase(mox.MoxTestBase): """Setups and tear downs the mocks but doesn't test anything as-is.""" def setUp(self): - if hasattr(warnings, 'catch_warnings'): - self._warnings_stack = warnings.catch_warnings() - else: - self._warnings_stack = None - warnings.simplefilter("ignore", DeprecationWarning) - self.mox = mox.Mox() + super(PresubmitTestsBase, self).setUp() + self.mox.StubOutWithMock(presubmit, 'warnings') self.original_IsFile = os.path.isfile def MockIsFile(f): dir = os.path.dirname(f) @@ -107,7 +102,6 @@ def CheckChangeOnUpload(input_api, output_api): sys.stdout = self._sys_stdout os.path.exists = self._os_path_exists os.path.isdir = self._os_path_isdir - self._warnings_stack = None @staticmethod def MakeBasicChange(name, description): @@ -201,8 +195,8 @@ class PresubmitUnittest(PresubmitTestsBase): self.failUnless(len(change.AffectedFiles(include_dirs=True, include_deletes=False)) == 4) - affected_text_files = change.AffectedTextFiles(include_deletes=True) - self.failUnless(len(affected_text_files) == 3) + affected_text_files = change.AffectedTextFiles() + self.failUnless(len(affected_text_files) == 2) self.failIf(filter(lambda x: x.LocalPath() == 'binary.dll', affected_text_files)) @@ -470,7 +464,7 @@ class InputApiUnittest(PresubmitTestsBase): def testMembersChanged(self): members = [ 'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles', - 'DepotToLocalPath', 'FilterTextFiles', 'LocalPaths', 'LocalToDepotPath', + 'DepotToLocalPath', 'LocalPaths', 'LocalToDepotPath', 'PresubmitLocalPath', 'RightHandSideLines', 'ServerPaths', 'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', 'marshal', 'os_path', 'pickle', 'platform', @@ -498,30 +492,6 @@ class InputApiUnittest(PresubmitTestsBase): self.failUnless(api.PresubmitLocalPath() == 'foo/path') self.failUnless(api.change == 42) - def testFilterTextFiles(self): - class MockAffectedFile(object): - def __init__(self, path, action): - self.path = path - self.action = action - def Action(self): - return self.action - def LocalPath(self): - return self.path - def AbsoluteLocalPath(self): - return self.path - - list = [MockAffectedFile('foo/blat.txt', 'M'), - MockAffectedFile('foo/binary.blob', 'M'), - MockAffectedFile('blat/flop.txt', 'D')] - - output = presubmit.InputApi.FilterTextFiles(list, include_deletes=True) - self.failUnless(len(output) == 2) - self.failUnless(list[0] in output and list[2] in output) - - output = presubmit.InputApi.FilterTextFiles(list, include_deletes=False) - self.failUnless(len(output) == 1) - self.failUnless(list[0] in output) - def testInputApiPresubmitScriptFiltering(self): description_lines = ('Hello there', 'this is a change', @@ -608,6 +578,16 @@ class InputApiUnittest(PresubmitTestsBase): self.failUnless(absolute_paths[1] == presubmit.normpath('c:/temp/isdir/blat.cc')) + def testDeprecated(self): + presubmit.warnings.warn(mox.IgnoreArg(), category=mox.IgnoreArg(), + stacklevel=2) + self.mox.ReplayAll() + change = presubmit.GclChange(gcl.ChangeInfo(name='mychange', + description='Bleh\n')) + api = presubmit.InputApi(change, 'foo/PRESUBMIT.py') + api.AffectedTextFiles(include_deletes=False) + self.mox.VerifyAll() + class OuputApiUnittest(PresubmitTestsBase): """Tests presubmit.OutputApi.""" @@ -711,6 +691,21 @@ class AffectedFileUnittest(PresubmitTestsBase): self.failUnless(affected_file.IsDirectory()) self.mox.VerifyAll() + def testIsTextFile(self): + list = [presubmit.SvnAffectedFile('foo/blat.txt', 'M'), + presubmit.SvnAffectedFile('foo/binary.blob', 'M'), + presubmit.SvnAffectedFile('blat/flop.txt', 'D')] + os.path.exists(os.path.join('foo', 'blat.txt')).AndReturn(True) + os.path.isdir(os.path.join('foo', 'blat.txt')).AndReturn(False) + os.path.exists(os.path.join('foo', 'binary.blob')).AndReturn(True) + os.path.isdir(os.path.join('foo', 'binary.blob')).AndReturn(False) + self.mox.ReplayAll() + + output = filter(lambda x: x.IsTextFile(), list) + self.failUnless(len(output) == 1) + self.failUnless(list[0] == output[0]) + self.mox.VerifyAll() + class CannedChecksUnittest(PresubmitTestsBase): """Tests presubmit_canned_checks.py."""