2011-01-19 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jan 2011 21:30:09 +0000 (21:30 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jan 2011 21:30:09 +0000 (21:30 +0000)
        Reviewed by Mihai Parparita.

        new-run-webkit-tests: remove use of os.walk, use mock filesystem for better
        unit testing. os.walk() is too much of a hassle to implement on
        top of the in-memory mock filesystem and adding the necessary
        interface to files_under() gives clients a cleaner API anyway
        (for this particular usage model).

        https://bugs.webkit.org/show_bug.cgi?id=52691

        * Scripts/webkitpy/common/system/filesystem.py:
        * Scripts/webkitpy/common/system/filesystem_mock.py:
        * Scripts/webkitpy/layout_tests/port/test_files.py:
        * Scripts/webkitpy/layout_tests/port/test_files_unittest.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76155 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/system/filesystem.py
Tools/Scripts/webkitpy/common/system/filesystem_mock.py
Tools/Scripts/webkitpy/layout_tests/port/test_files.py
Tools/Scripts/webkitpy/layout_tests/port/test_files_unittest.py

index 80133e0b923f1d58586941d437e5ba4ea510d5ff..86c689fc0a0fd2abc8e0ffb122dc93ceb46e4b86 100644 (file)
@@ -1,3 +1,20 @@
+2011-01-19  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Mihai Parparita.
+
+        new-run-webkit-tests: remove use of os.walk, use mock filesystem for better
+        unit testing. os.walk() is too much of a hassle to implement on
+        top of the in-memory mock filesystem and adding the necessary
+        interface to files_under() gives clients a cleaner API anyway
+        (for this particular usage model).
+
+        https://bugs.webkit.org/show_bug.cgi?id=52691
+
+        * Scripts/webkitpy/common/system/filesystem.py:
+        * Scripts/webkitpy/common/system/filesystem_mock.py:
+        * Scripts/webkitpy/layout_tests/port/test_files.py:
+        * Scripts/webkitpy/layout_tests/port/test_files_unittest.py:
+
 2011-01-19  Dirk Pranke  <dpranke@chromium.org>
 
         Reviewed by Tony Chang.
index d45d4c1b1f696f42e1e3ee348f4e221facf8d384..6dcf2072016cc27b21b8c3b0d4355619c181c48e 100644 (file)
@@ -62,11 +62,39 @@ class FileSystem(object):
         """Return whether the path exists in the filesystem."""
         return os.path.exists(path)
 
-    def files_under(self, path):
-        """Return the list of all files under the given path."""
-        return [self.join(path_to_file, filename)
-            for (path_to_file, _, filenames) in os.walk(path)
-            for filename in filenames]
+    def files_under(self, path, dirs_to_skip=[], file_filter=None):
+        """Return the list of all files under the given path in topdown order.
+
+        Args:
+            dirs_to_skip: a list of directories to skip over during the
+                traversal (e.g., .svn, resources, etc.)
+            file_filter: if not None, the filter will be invoked
+                with the filesystem object and the dirname and basename of
+                each file found. The file is included in the result if the
+                callback returns True.
+        """
+        def filter_all(fs, dirpath, basename):
+            return True
+
+        file_filter = file_filter or filter_all
+        files = []
+        if self.isfile(path):
+            if file_filter(self, self.dirname(path), self.basename(path)):
+                files.append(path)
+            return files
+
+        if self.basename(path) in dirs_to_skip:
+            return []
+
+        for (dirpath, dirnames, filenames) in os.walk(path):
+            for d in dirs_to_skip:
+                if d in dirnames:
+                    dirnames.remove(d)
+
+            for filename in filenames:
+                if file_filter(self, dirpath, filename):
+                    files.append(self.join(dirpath, filename))
+        return files
 
     def glob(self, path):
         """Wraps glob.glob()."""
index fd2438db2f585eac3037e8a3d2eec3f5c63c0882..096fdf7bbfc3a8fcf33e9831152a6c5397b812dd 100644 (file)
@@ -71,10 +71,37 @@ class MockFileSystem(object):
     def exists(self, path):
         return self.isfile(path) or self.isdir(path)
 
-    def files_under(self, path):
+    def files_under(self, path, dirs_to_skip=[], file_filter=None):
+        def filter_all(fs, dirpath, basename):
+            return True
+
+        file_filter = file_filter or filter_all
+        files = []
+        if self.isfile(path):
+            if file_filter(self, self.dirname(path), self.basename(path)):
+                files.append(path)
+            return files
+
+        if self.basename(path) in dirs_to_skip:
+            return []
+
         if not path.endswith('/'):
             path += '/'
-        return [file for file in self.files if file.startswith(path)]
+
+        dir_substrings = ['/' + d + '/' for d in dirs_to_skip]
+        for filename in self.files:
+            if not filename.startswith(path):
+                continue
+
+            suffix = filename[len(path) - 1:]
+            if any(dir_substring in suffix for dir_substring in dir_substrings):
+                continue
+
+            dirpath, basename = self._split(filename)
+            if file_filter(self, dirpath, basename):
+                files.append(filename)
+
+        return files
 
     def glob(self, path):
         # FIXME: This only handles a wildcard '*' at the end of the path.
index 2901a9fd333396767191465b95c3b09684e7ab28..41d918fa601d122f8c39ea761a774d6658869995 100644 (file)
@@ -34,8 +34,6 @@ list of test files is constrained to those found under the paths passed in,
 i.e. calling find(["LayoutTests/fast"]) will only return files
 under that directory."""
 
-import glob
-import os
 import time
 
 from webkitpy.common.system import logutils
@@ -61,6 +59,7 @@ def find(port, paths):
     fs = port._filesystem
     gather_start_time = time.time()
     paths_to_walk = set()
+
     # if paths is empty, provide a pre-defined list.
     if paths:
         _log.debug("Gathering tests from: %s relative to %s" % (paths, port.layout_tests_dir()))
@@ -76,30 +75,12 @@ def find(port, paths):
         _log.debug("Gathering tests from: %s" % port.layout_tests_dir())
         paths_to_walk.add(port.layout_tests_dir())
 
-    # Now walk all the paths passed in on the command line and get filenames
+    # FIXME: I'm not sure there's much point in this being a set. A list would
+    # probably be faster.
     test_files = set()
     for path in paths_to_walk:
-        if fs.isfile(path) and _is_test_file(fs, path):
-            test_files.add(fs.normpath(path))
-            continue
-
-        for root, dirs, files in os.walk(path):
-            # Don't walk skipped directories or their sub-directories.
-            if os.path.basename(root) in _skipped_directories:
-                del dirs[:]
-                continue
-            # This copy and for-in is slightly inefficient, but
-            # the extra walk avoidance consistently shaves .5 seconds
-            # off of total walk() time on my MacBook Pro.
-            for directory in dirs[:]:
-                if directory in _skipped_directories:
-                    dirs.remove(directory)
-
-            for filename in files:
-                if _is_test_file(fs, filename):
-                    filename = fs.join(root, filename)
-                    filename = fs.normpath(filename)
-                    test_files.add(filename)
+        files = fs.files_under(path, _skipped_directories, _is_test_file)
+        test_files.update(set(files))
 
     gather_time = time.time() - gather_start_time
     _log.debug("Test gathering took %f seconds" % gather_time)
@@ -123,7 +104,7 @@ def _is_reference_html_file(filename):
     return False
 
 
-def _is_test_file(fs, filename):
+def _is_test_file(fs, dirname, filename):
     """Return true if the filename points to a test file."""
     return (_has_supported_extension(fs, filename) and
             not _is_reference_html_file(filename))
index 56973911935a340839eaff8b479305090db34183..eabdfd9c2b0be008de4b8bc76612f7d75ffe3505 100644 (file)
@@ -66,11 +66,11 @@ class TestFilesTest(unittest.TestCase):
     def test_is_test_file(self):
         port = base.Port()
         fs = port._filesystem
-        self.assertTrue(test_files._is_test_file(fs, 'foo.html'))
-        self.assertTrue(test_files._is_test_file(fs, 'foo.shtml'))
-        self.assertFalse(test_files._is_test_file(fs, 'foo.png'))
-        self.assertFalse(test_files._is_test_file(fs, 'foo-expected.html'))
-        self.assertFalse(test_files._is_test_file(fs, 'foo-expected-mismatch.html'))
+        self.assertTrue(test_files._is_test_file(fs, '', 'foo.html'))
+        self.assertTrue(test_files._is_test_file(fs, '', 'foo.shtml'))
+        self.assertFalse(test_files._is_test_file(fs, '', 'foo.png'))
+        self.assertFalse(test_files._is_test_file(fs, '', 'foo-expected.html'))
+        self.assertFalse(test_files._is_test_file(fs, '', 'foo-expected-mismatch.html'))
 
 
 if __name__ == '__main__':