test-webkitpy: split test-finding code into a different module
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Mar 2012 23:31:05 +0000 (23:31 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Mar 2012 23:31:05 +0000 (23:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=82253

Reviewed by Adam Barth.

Per suggestion from abarth, this change splits all the
filesystem-crawling, test-finding code in test-webkitpy out into
a separate module and switches to using a FileSystem object.
This makes things much more testable, so we also add tests :).

We also add a realpath() method to the Filesystem object, since
test-webkitpy needs that in order to be able to resolve symlinks
properly to determine whether a file is under a particular tree
or not.

* Scripts/webkitpy/common/system/filesystem.py:
(FileSystem.realpath):
* Scripts/webkitpy/common/system/filesystem_mock.py:
(MockFileSystem.realpath):
* Scripts/webkitpy/test/main.py:
(Tester.__init__):
(Tester.add_tree):
(Tester.run):
(Tester._run_tests):
(Tester._log_exception):
* Scripts/webkitpy/test/test_finder.py: Added.
(TestDirectoryTree):
(TestDirectoryTree.__init__):
(TestDirectoryTree.find_modules):
(TestDirectoryTree.find_modules.file_filter):
(TestDirectoryTree.to_module):
(TestDirectoryTree.clean):
(TestFinder):
(TestFinder.__init__):
(TestFinder.add_tree):
(TestFinder.additional_paths):
(TestFinder.clean_trees):
(TestFinder.is_module):
(TestFinder.to_module):
(TestFinder.find_names):
(TestFinder._exclude):
* Scripts/webkitpy/test/test_finder_unittest.py: Added.
(TestFinderTest):
(TestFinderTest.setUp):
(TestFinderTest.tearDown):
(TestFinderTest.test_additional_system_paths):
(TestFinderTest.test_is_module):
(TestFinderTest.test_to_module):
(TestFinderTest.test_clean):
(TestFinderTest.test_find_names):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@112168 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/test/main.py
Tools/Scripts/webkitpy/test/test_finder.py [new file with mode: 0644]
Tools/Scripts/webkitpy/test/test_finder_unittest.py [new file with mode: 0644]

index 4bec4a3..b80a66d 100644 (file)
@@ -1,3 +1,56 @@
+2012-03-26  Dirk Pranke  <dpranke@chromium.org>
+
+        test-webkitpy: split test-finding code into a different module
+        https://bugs.webkit.org/show_bug.cgi?id=82253
+
+        Reviewed by Adam Barth.
+
+        Per suggestion from abarth, this change splits all the
+        filesystem-crawling, test-finding code in test-webkitpy out into
+        a separate module and switches to using a FileSystem object.
+        This makes things much more testable, so we also add tests :).
+
+        We also add a realpath() method to the Filesystem object, since
+        test-webkitpy needs that in order to be able to resolve symlinks
+        properly to determine whether a file is under a particular tree
+        or not.
+
+        * Scripts/webkitpy/common/system/filesystem.py:
+        (FileSystem.realpath):
+        * Scripts/webkitpy/common/system/filesystem_mock.py:
+        (MockFileSystem.realpath):
+        * Scripts/webkitpy/test/main.py:
+        (Tester.__init__):
+        (Tester.add_tree):
+        (Tester.run):
+        (Tester._run_tests):
+        (Tester._log_exception):
+        * Scripts/webkitpy/test/test_finder.py: Added.
+        (TestDirectoryTree):
+        (TestDirectoryTree.__init__):
+        (TestDirectoryTree.find_modules):
+        (TestDirectoryTree.find_modules.file_filter):
+        (TestDirectoryTree.to_module):
+        (TestDirectoryTree.clean):
+        (TestFinder):
+        (TestFinder.__init__):
+        (TestFinder.add_tree):
+        (TestFinder.additional_paths):
+        (TestFinder.clean_trees):
+        (TestFinder.is_module):
+        (TestFinder.to_module):
+        (TestFinder.find_names):
+        (TestFinder._exclude):
+        * Scripts/webkitpy/test/test_finder_unittest.py: Added.
+        (TestFinderTest):
+        (TestFinderTest.setUp):
+        (TestFinderTest.tearDown):
+        (TestFinderTest.test_additional_system_paths):
+        (TestFinderTest.test_is_module):
+        (TestFinderTest.test_to_module):
+        (TestFinderTest.test_clean):
+        (TestFinderTest.test_find_names):
+
 2012-03-26  Raphael Kubo da Costa  <rakuco@FreeBSD.org>
 
         [jhbuild] Use $MAKE if it is defined to build jhbuild itself.
index c3a69a5..687a313 100644 (file)
@@ -55,6 +55,9 @@ class FileSystem(object):
     def abspath(self, path):
         return os.path.abspath(path)
 
+    def realpath(self, path):
+        return os.path.realpath(path)
+
     def path_to_module(self, module_name):
         """A wrapper for all calls to __file__ to allow easy unit testing."""
         # FIXME: This is the only use of sys in this file. It's possible this function should move elsewhere.
index 2ff688a..8899668 100644 (file)
@@ -82,6 +82,9 @@ class MockFileSystem(object):
             return self.normpath(path)
         return self.abspath(self.join(self.cwd, path))
 
+    def realpath(self, path):
+        return self.abspath(path)
+
     def basename(self, path):
         return self._split(path)[1]
 
index 4bfa087..b2ebabe 100644 (file)
@@ -30,18 +30,19 @@ import sys
 import traceback
 import unittest
 
-# NOTE: We intentionally do not depend on anything else in webkitpy here to avoid breaking test-webkitpy.
+from webkitpy.common.system.filesystem import FileSystem
+from webkitpy.test.test_finder import TestFinder
 
 _log = logging.getLogger(__name__)
 
 
 class Tester(object):
-    def __init__(self):
+    def __init__(self, filesystem=None):
         self._verbosity = 1
-        self._trees = []
+        self.finder = TestFinder(filesystem or FileSystem())
 
     def add_tree(self, top_directory, starting_subdirectory=None):
-        self._trees.append(TestDirectoryTree(top_directory, starting_subdirectory))
+        self.finder.add_tree(top_directory, starting_subdirectory)
 
     def _parse_args(self):
         parser = optparse.OptionParser(usage='usage: %prog [options] [args...]')
@@ -136,49 +137,10 @@ class Tester(object):
         options, args = self._parse_args()
         self._configure(options)
 
-        for tree in self._trees:
-            tree.clean()
+        self.finder.clean_trees()
 
-        args = args or self._find_modules()
-        return self._run_tests(args)
-
-    def _find_modules(self):
-        suffixes = ['_unittest.py']
-        if not self._options.skip_integrationtests:
-            suffixes.append('_integrationtest.py')
-
-        modules = []
-        for tree in self._trees:
-            modules.extend(tree.find_modules(suffixes))
-        modules.sort()
-
-        for module in modules:
-            _log.debug("Found: %s" % module)
-
-        # FIXME: Figure out how to move this to test-webkitpy in order to to make this file more generic.
-        if not self._options.all:
-            slow_tests = ('webkitpy.common.checkout.scm.scm_unittest',)
-            self._exclude(modules, slow_tests, 'are really, really slow', 31818)
-
-            if sys.platform == 'win32':
-                win32_blacklist = ('webkitpy.common.checkout',
-                                   'webkitpy.common.config',
-                                   'webkitpy.tool')
-                self._exclude(modules, win32_blacklist, 'fail horribly on win32', 54526)
-
-        return modules
-
-    def _exclude(self, modules, module_prefixes, reason, bugid):
-        _log.info('Skipping tests in the following modules or packages because they %s:' % reason)
-        for prefix in module_prefixes:
-            _log.info('    %s' % prefix)
-            modules_to_exclude = filter(lambda m: m.startswith(prefix), modules)
-            for m in modules_to_exclude:
-                if len(modules_to_exclude) > 1:
-                    _log.debug('        %s' % m)
-                modules.remove(m)
-        _log.info('    (https://bugs.webkit.org/show_bug.cgi?id=%d; use --all to include)' % bugid)
-        _log.info('')
+        names = self.finder.find_names(args, self._options.skip_integrationtests, self._options.all)
+        return self._run_tests(names)
 
     def _run_tests(self, args):
         if self._options.coverage:
@@ -191,14 +153,14 @@ class Tester(object):
             cov.start()
 
         # Make sure PYTHONPATH is set up properly.
-        sys.path = [tree.top_directory for tree in self._trees if tree.top_directory not in sys.path] + sys.path
+        sys.path = self.finder.additional_paths(sys.path) + sys.path
 
         _log.debug("Loading the tests...")
 
         loader = unittest.defaultTestLoader
         suites = []
         for name in args:
-            if self._is_module(name):
+            if self.finder.is_module(name):
                 # import modules explicitly before loading their tests because
                 # loadTestsFromName() produces lousy error messages for bad modules.
                 try:
@@ -224,42 +186,8 @@ class Tester(object):
             cov.report(show_missing=False)
         return result.wasSuccessful()
 
-    def _is_module(self, name):
-        relpath = name.replace('.', os.sep) + '.py'
-        return any(os.path.exists(os.path.join(tree.top_directory, relpath)) for tree in self._trees)
-
     def _log_exception(self):
         s = StringIO.StringIO()
         traceback.print_exc(file=s)
         for l in s.buflist:
             _log.error('  ' + l.rstrip())
-
-
-class TestDirectoryTree(object):
-    def __init__(self, top_directory, starting_subdirectory):
-        self.top_directory = os.path.realpath(top_directory)
-        self.search_directory = self.top_directory
-        self.top_package = ''
-        if starting_subdirectory:
-            self.top_package = starting_subdirectory.replace(os.sep, '.') + '.'
-            self.search_directory = os.path.join(self.top_directory, starting_subdirectory)
-
-    def find_modules(self, suffixes):
-        modules = []
-        for dir_path, _, filenames in os.walk(self.search_directory):
-            dir_path = os.path.join(dir_path, '')
-            package = dir_path.replace(self.top_directory + os.sep, '').replace(os.sep, '.')
-            for f in filenames:
-                if any(f.endswith(suffix) for suffix in suffixes):
-                    modules.append(package + f.replace('.py', ''))
-        return modules
-
-    def clean(self):
-        """Delete all .pyc files in the tree that have no matching .py file."""
-        _log.debug("Cleaning orphaned *.pyc files from: %s" % self.search_directory)
-        for dir_path, dir_names, file_names in os.walk(self.search_directory):
-            for file_name in file_names:
-                if file_name.endswith(".pyc") and file_name[:-1] not in file_names:
-                    file_path = os.path.join(dir_path, file_name)
-                    _log.info("Deleting orphan *.pyc file: %s" % file_path)
-                    os.remove(file_path)
diff --git a/Tools/Scripts/webkitpy/test/test_finder.py b/Tools/Scripts/webkitpy/test/test_finder.py
new file mode 100644 (file)
index 0000000..cae4695
--- /dev/null
@@ -0,0 +1,128 @@
+# Copyright (C) 2012 Google, Inc.
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org)
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1.  Redistributions of source code must retain the above copyright
+#     notice, this list of conditions and the following disclaimer.
+# 2.  Redistributions in binary form must reproduce the above copyright
+#     notice, this list of conditions and the following disclaimer in the
+#     documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR
+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""this module is responsible for finding python tests."""
+
+import logging
+import sys
+
+
+_log = logging.getLogger(__name__)
+
+
+class TestDirectoryTree(object):
+    def __init__(self, filesystem, top_directory, starting_subdirectory):
+        self.filesystem = filesystem
+        self.top_directory = filesystem.realpath(top_directory)
+        self.search_directory = self.top_directory
+        self.top_package = ''
+        if starting_subdirectory:
+            self.top_package = starting_subdirectory.replace(filesystem.sep, '.') + '.'
+            self.search_directory = filesystem.join(self.top_directory, starting_subdirectory)
+
+    def find_modules(self, suffixes):
+
+        def file_filter(filesystem, dirname, basename):
+            return any(basename.endswith(suffix) for suffix in suffixes)
+
+        filenames = self.filesystem.files_under(self.search_directory, file_filter=file_filter)
+        return [self.to_module(filename) for filename in filenames]
+
+    def to_module(self, path):
+        return path.replace(self.top_directory + self.filesystem.sep, '').replace(self.filesystem.sep, '.')[:-3]
+
+    def clean(self):
+        """Delete all .pyc files in the tree that have no matching .py file."""
+        _log.debug("Cleaning orphaned *.pyc files from: %s" % self.search_directory)
+        filenames = self.filesystem.files_under(self.search_directory)
+        for filename in filenames:
+            if filename.endswith(".pyc") and filename[:-1] not in filenames:
+                _log.info("Deleting orphan *.pyc file: %s" % filename)
+                self.filesystem.remove(filename)
+
+
+class TestFinder(object):
+    def __init__(self, filesystem):
+        self.filesystem = filesystem
+        self.trees = []
+
+    def add_tree(self, top_directory, starting_subdirectory=None):
+        self.trees.append(TestDirectoryTree(self.filesystem, top_directory, starting_subdirectory))
+
+    def additional_paths(self, paths):
+        return [tree.top_directory for tree in self.trees if tree.top_directory not in paths]
+
+    def clean_trees(self):
+        for tree in self.trees:
+            tree.clean()
+
+    def is_module(self, name):
+        relpath = name.replace('.', self.filesystem.sep) + '.py'
+        return any(self.filesystem.exists(self.filesystem.join(tree.top_directory, relpath)) for tree in self.trees)
+
+    def to_module(self, path):
+        for tree in self.trees:
+            if path.startswith(tree.top_directory):
+                return tree.to_module(path)
+        return None
+
+    def find_names(self, args, skip_integrationtests, find_all):
+        if args:
+            return args
+
+        suffixes = ['_unittest.py']
+        if not skip_integrationtests:
+            suffixes.append('_integrationtest.py')
+
+        modules = []
+        for tree in self.trees:
+            modules.extend(tree.find_modules(suffixes))
+        modules.sort()
+
+        for module in modules:
+            _log.debug("Found: %s" % module)
+
+        # FIXME: Figure out how to move this to test-webkitpy in order to to make this file more generic.
+        if not find_all:
+            slow_tests = ('webkitpy.common.checkout.scm.scm_unittest',)
+            self._exclude(modules, slow_tests, 'are really, really slow', 31818)
+
+            if sys.platform == 'win32':
+                win32_blacklist = ('webkitpy.common.checkout',
+                                   'webkitpy.common.config',
+                                   'webkitpy.tool')
+                self._exclude(modules, win32_blacklist, 'fail horribly on win32', 54526)
+
+        return modules
+
+    def _exclude(self, modules, module_prefixes, reason, bugid):
+        _log.info('Skipping tests in the following modules or packages because they %s:' % reason)
+        for prefix in module_prefixes:
+            _log.info('    %s' % prefix)
+            modules_to_exclude = filter(lambda m: m.startswith(prefix), modules)
+            for m in modules_to_exclude:
+                if len(modules_to_exclude) > 1:
+                    _log.debug('        %s' % m)
+                modules.remove(m)
+        _log.info('    (https://bugs.webkit.org/show_bug.cgi?id=%d; use --all to include)' % bugid)
+        _log.info('')
diff --git a/Tools/Scripts/webkitpy/test/test_finder_unittest.py b/Tools/Scripts/webkitpy/test/test_finder_unittest.py
new file mode 100644 (file)
index 0000000..2f746da
--- /dev/null
@@ -0,0 +1,88 @@
+# Copyright (C) 2012 Google, Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1.  Redistributions of source code must retain the above copyright
+#     notice, this list of conditions and the following disclaimer.
+# 2.  Redistributions in binary form must reproduce the above copyright
+#     notice, this list of conditions and the following disclaimer in the
+#     documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR
+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import logging
+import unittest
+
+from webkitpy.common.system.filesystem_mock import MockFileSystem
+from webkitpy.test.test_finder import TestFinder
+
+
+class TestFinderTest(unittest.TestCase):
+    def setUp(self):
+        files = {
+          '/foo/bar/baz.py': '',
+          '/foo/bar/baz_unittest.py': '',
+          '/foo2/bar2/baz2.py': '',
+          '/foo2/bar2/baz2.pyc': '',
+          '/foo2/bar2/baz2_integrationtest.py': '',
+          '/foo2/bar2/missing.pyc': '',
+        }
+        self.fs = MockFileSystem(files)
+        self.finder = TestFinder(self.fs)
+        self.finder.add_tree('/foo', 'bar')
+        self.finder.add_tree('/foo2')
+        self.root_logger = logging.getLogger()
+        self.log_level = self.root_logger.level
+        self.root_logger.setLevel(logging.WARNING)
+
+    def tearDown(self):
+        self.root_logger.setLevel(self.log_level)
+
+    def test_additional_system_paths(self):
+        self.assertEquals(self.finder.additional_paths(['/usr']),
+                          ['/foo', '/foo2'])
+
+    def test_is_module(self):
+        self.assertTrue(self.finder.is_module('bar.baz'))
+        self.assertTrue(self.finder.is_module('bar2.baz2'))
+        self.assertTrue(self.finder.is_module('bar2.baz2_integrationtest'))
+
+        # Missing the proper namespace.
+        self.assertFalse(self.finder.is_module('baz'))
+
+    def test_to_module(self):
+        self.assertEquals(self.finder.to_module('/foo/test.py'), 'test')
+        self.assertEquals(self.finder.to_module('/foo/bar/test.py'), 'bar.test')
+        self.assertEquals(self.finder.to_module('/foo/bar/pytest.py'), 'bar.pytest')
+
+    def test_clean(self):
+        self.assertTrue(self.fs.exists('/foo2/bar2/missing.pyc'))
+        self.finder.clean_trees()
+        self.assertFalse(self.fs.exists('/foo2/bar2/missing.pyc'))
+
+    def test_find_names(self):
+        self.assertEquals(self.finder.find_names([], skip_integrationtests=False, find_all=True),
+            ['bar.baz_unittest', 'bar2.baz2_integrationtest'])
+
+        self.assertEquals(self.finder.find_names([], skip_integrationtests=True, find_all=True),
+            ['bar.baz_unittest'])
+        self.assertEquals(self.finder.find_names([], skip_integrationtests=True, find_all=False),
+            ['bar.baz_unittest'])
+
+        # Should return the names given it, even if they don't exist.
+        self.assertEquals(self.finder.find_names(['foobar'], skip_integrationtests=True, find_all=True),
+            ['foobar'])
+
+
+if __name__ == '__main__':
+    unittest.main()