W3C test downloader should be able to import specific files/sub-directories in a...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Oct 2016 10:27:27 +0000 (10:27 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Oct 2016 10:27:27 +0000 (10:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161789

Patch by Youenn Fablet <youenn@apple.com> on 2016-10-10
Reviewed by Ryosuke Niwa.

Removed tests_directory option and corresponding test.
This option was allowing to import a sub-directory from a test repository.
We can readd it if necessary.

Adding support for finer-grained import rules.
Previously skipped directories were fully removed.
Now, directories may be skipped but sub directories of them may be imported.
This currently happens in web-platform-tests repo.

* Scripts/webkitpy/w3c/test_downloader.py:
(TestDownloader._add_test_suite_paths): Removing tests_directory option.
(TestDownloader.copy_tests): Refactoring file copy by generating the list of all directories for which direct files should be imported.
(TestDownloader.copy_tests.should_copy_dir):
(TestDownloader.copy_tests.should_copy_file):
* Scripts/webkitpy/w3c/test_importer_unittest.py:
(TestImporterTest.test_tests_directory): Deleted.
(TestImporterTest.test_skip_test_import): Added.

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

Tools/ChangeLog
Tools/Scripts/webkitpy/w3c/test_downloader.py
Tools/Scripts/webkitpy/w3c/test_importer_unittest.py

index 51c9de1..84de018 100644 (file)
@@ -1,3 +1,28 @@
+2016-10-10  Youenn Fablet  <youenn@apple.com>
+
+        W3C test downloader should be able to import specific files/sub-directories in a skipped directory
+        https://bugs.webkit.org/show_bug.cgi?id=161789
+
+        Reviewed by Ryosuke Niwa.
+
+        Removed tests_directory option and corresponding test.
+        This option was allowing to import a sub-directory from a test repository.
+        We can readd it if necessary.
+
+        Adding support for finer-grained import rules.
+        Previously skipped directories were fully removed.
+        Now, directories may be skipped but sub directories of them may be imported.
+        This currently happens in web-platform-tests repo.
+
+        * Scripts/webkitpy/w3c/test_downloader.py:
+        (TestDownloader._add_test_suite_paths): Removing tests_directory option.
+        (TestDownloader.copy_tests): Refactoring file copy by generating the list of all directories for which direct files should be imported.
+        (TestDownloader.copy_tests.should_copy_dir):
+        (TestDownloader.copy_tests.should_copy_file):
+        * Scripts/webkitpy/w3c/test_importer_unittest.py:
+        (TestImporterTest.test_tests_directory): Deleted.
+        (TestImporterTest.test_skip_test_import): Added.
+
 2016-10-09  Simon Fraser  <simon.fraser@apple.com>
 
         Make validate-committer-lists show inactive reviewers
index 7278648..d102bb9 100644 (file)
@@ -26,7 +26,6 @@
 """
  This script downloads W3C test repositories.
 """
-#FIXME: Implement submodules extraction to prepare for generation of LayoutTests/imported/w3C/resources/WPTModules
 
 import json
 import logging
@@ -104,12 +103,11 @@ class TestDownloader(object):
             elif 'PASS' in line.expectations:
                 self.paths_to_import.append(line.name)
 
-    def _add_test_suite_paths(self, test_paths, directory, webkit_path, tests_directory):
-        complete_directory = self._filesystem.join(directory, tests_directory)
-        for name in self._filesystem.listdir(complete_directory):
-            original_path = self._filesystem.join(webkit_path, tests_directory, name)
+    def _add_test_suite_paths(self, test_paths, directory, webkit_path):
+        for name in self._filesystem.listdir(directory):
+            original_path = self._filesystem.join(webkit_path, name)
             if not name.startswith('.') and not original_path in self.paths_to_skip:
-                test_paths.append((original_path, self._filesystem.join(webkit_path, name)))
+                test_paths.append(original_path)
 
     def _empty_directory(self, directory):
         if self._filesystem.exists(directory):
@@ -123,35 +121,57 @@ class TestDownloader(object):
         copy_paths = []
         if test_paths:
             for path in test_paths:
-                copy_paths.append((path, path))
+                copy_paths.append(path)
             for path in self.paths_to_import:
-                copy_paths.append((path, path))
+                copy_paths.append(path)
         else:
             for test_repository in self.test_repositories:
-                self._add_test_suite_paths(copy_paths, self._filesystem.join(self.repository_directory, test_repository['name']), test_repository['name'],
-                    test_repository['tests_directory'] if ('tests_directory' in test_repository) else '')
+                self._add_test_suite_paths(copy_paths, self._filesystem.join(self.repository_directory, test_repository['name']), test_repository['name'])
             # Handling of tests marked as [ Pass ] in expectations file.
             for path in self.paths_to_import:
-                if not (path, path) in copy_paths:
-                    copy_paths.append((path, path))
-
-        for paths in copy_paths:
-            source_path = self._filesystem.join(self.repository_directory, paths[0])
-            destination_path = self._filesystem.join(destination_directory, paths[1])
-            if not self._filesystem.exists(source_path):
-                _log.error('Cannot copy %s' % source_path)
-            elif self._filesystem.isdir(source_path):
-                self._filesystem.copytree(source_path, destination_path)
-            else:
-                self._filesystem.maybe_make_directory(self._filesystem.dirname(destination_path))
-                self._filesystem.copyfile(source_path, destination_path)
-
-        for path in self.paths_to_skip:
-            destination_path = self._filesystem.join(destination_directory, path)
-            if self._filesystem.isfile(destination_path):
-                self._filesystem.remove(destination_path)
-            elif self._filesystem.isdir(destination_path):
-                self._filesystem.rmtree(destination_path)
+                if not path in copy_paths:
+                    copy_paths.append(path)
+
+        def longest_path(filesystem, paths):
+            longest_matching_path = ""
+            for path in paths:
+                if path.startswith(longest_matching_path):
+                    longest_matching_path = path
+            return longest_matching_path
+
+        def should_copy_dir(filesystem, directory):
+            relative_path = self._filesystem.relpath(directory, self.repository_directory)
+            if relative_path == ".":
+                return True
+
+            potential_copy_paths = [copy_directory for copy_directory in copy_paths if relative_path.startswith(copy_directory)]
+            if (not potential_copy_paths):
+                return False
+
+            potential_skip_paths = [skip_directory for skip_directory in self.paths_to_skip if relative_path.startswith(skip_directory)]
+            if (not potential_skip_paths):
+                return True
+
+            longest_copy_path = longest_path(filesystem, potential_copy_paths)
+            longest_skip_path = longest_path(filesystem, potential_skip_paths)
+            return longest_copy_path.startswith(longest_skip_path)
+
+        # Compute directories for which we should copy direct children
+        directories_to_copy = self._filesystem.dirs_under(self.repository_directory, should_copy_dir)
+
+        def should_copy_file(filesystem, dirname, filename):
+            full_path = filesystem.join(dirname, filename)
+            relative_path = self._filesystem.relpath(full_path, self.repository_directory)
+            if relative_path in copy_paths:
+                return True
+            if relative_path in self.paths_to_skip:
+                return False
+            return dirname in directories_to_copy
+
+        for source_path in self._filesystem.files_under(self.repository_directory, file_filter=should_copy_file):
+            destination_path = self._filesystem.join(destination_directory, self._filesystem.relpath(source_path, self.repository_directory))
+            self._filesystem.maybe_make_directory(self._filesystem.dirname(destination_path))
+            self._filesystem.copyfile(source_path, destination_path)
 
     def _git_submodules_description(self, test_repository):
         directory = self._filesystem.join(self.repository_directory, test_repository['name'])
index 4e234e4..b64f0bd 100644 (file)
@@ -164,27 +164,37 @@ class TestImporterTest(unittest.TestCase):
         # FIXME: Mock-up of git cannot use submodule command, hence the json file is empty, but still it should be created
         #self.assertTrue('https://github.com/w3c/testharness.js/archive/db4d391a69877d4a1eaaf51d1725c99a5b8ed84.tar.gz' in fs.read_text_file('/mock-checkout/LayoutTests/w3c/resources/web-platform-tests-modules.json'))
 
-    def test_tests_directory(self):
+    def test_skip_test_import(self):
         FAKE_FILES = {
             '/mock-checkout/WebKitBuild/w3c-tests/streams-api/reference-implementation/web-platform-tests/test.html': '<!doctype html><script src="/resources/testharness.js"></script><script src="/resources/testharnessreport.js"></script>',
-    '/mock-checkout/LayoutTests/imported/w3c/resources/TestRepositories': '''
+            '/mock-checkout/LayoutTests/imported/w3c/resources/TestRepositories': '''
 [
     {
-        "name": "streams-api",
-        "url": "https://github.com/whatwg/streams.git",
+        "name": "web-platform-tests",
+        "url": "https://github.com/myrepo",
         "revision": "7cc96dd",
-        "tests_directory": "reference-implementation/web-platform-tests",
         "paths_to_skip": [],
         "paths_to_import": [],
         "import_options": []
      }
-]
-'''}
+]''',
+            '/mock-checkout/LayoutTests/imported/w3c/resources/ImportExpectations': '''
+web-platform-tests/dir-to-skip [ Skip ]
+web-platform-tests/dir-to-skip/dir-to-import [ Pass ]
+web-platform-tests/dir-to-skip/file-to-import.html [ Pass ]
+''',
+            '/mock-checkout/WebKitBuild/w3c-tests/web-platform-tests/dir-to-skip/test-to-skip.html': 'to be skipped',
+            '/mock-checkout/WebKitBuild/w3c-tests/web-platform-tests/dir-to-skip/dir-to-import/test-to-import.html': 'to be imported',
+            '/mock-checkout/WebKitBuild/w3c-tests/web-platform-tests/dir-to-skip/dir-to-not-import/test-to-not-import.html': 'to be skipped',
+            '/mock-checkout/WebKitBuild/w3c-tests/web-platform-tests/dir-to-skip/file-to-import.html': 'to be imported',
+        }
 
-        fs = self.import_downloaded_tests(['--no-fetch', '--import-all', '-d', 'w3c'], FAKE_FILES)
+        fs = self.import_downloaded_tests(['--no-fetch', '-d', 'w3c'], FAKE_FILES)
 
-        self.assertFalse(fs.exists('/mock-checkout/LayoutTests/w3c/streams-api/reference-implementation/web-platform-tests/test.html'))
-        self.assertTrue(fs.exists('/mock-checkout/LayoutTests/w3c/streams-api/test.html'))
+        self.assertTrue(fs.exists('/mock-checkout/LayoutTests/w3c/web-platform-tests/dir-to-skip/file-to-import.html'))
+        self.assertTrue(fs.exists('/mock-checkout/LayoutTests/w3c/web-platform-tests/dir-to-skip/dir-to-import/test-to-import.html'))
+        self.assertFalse(fs.exists('/mock-checkout/LayoutTests/w3c/web-platform-tests/dir-to-skip/dir-to-not-import/test-to-not-import.html'))
+        self.assertFalse(fs.exists('/mock-checkout/LayoutTests/w3c/web-platform-tests/dir-to-skip/test-to-skip.html'))
 
     def test_clean_directory_option(self):
         FAKE_FILES = {