Following on bug 161307, W3C test importer should generate the list of resource files.
Created attachment 287918 [details] Patch
Comment on attachment 287918 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287918&action=review > Tools/Scripts/webkitpy/w3c/test_importer.py:473 > + if not self._already_identified_as_resource_file(resource_file_path): > + files.append(resource_file_path) > + should_update_json_file = True This would not catch the case when a resource file becomes a test. > Tools/Scripts/webkitpy/w3c/test_importer.py:484 > + for directory in self._test_resource_files["directories"]: > + if path.find(directory) != -1: > + return True > + return False This should just be: return any([path.find(directory) != -1 for directory in self._test_resource_files["directories"]])
Created attachment 287936 [details] Patch
Thanks for the review. > View in context: > https://bugs.webkit.org/attachment.cgi?id=287918&action=review > > > Tools/Scripts/webkitpy/w3c/test_importer.py:473 > > + if not self._already_identified_as_resource_file(resource_file_path): > > + files.append(resource_file_path) > > + should_update_json_file = True > > This would not catch the case when a resource file becomes a test. I added a FIXME for it. In the meantime, the plan is that on each full resync, the list of resource files will be fully regenerated. I also added generation of a warning for tests imported in "resources" folder. > > Tools/Scripts/webkitpy/w3c/test_importer.py:484 > > + for directory in self._test_resource_files["directories"]: > > + if path.find(directory) != -1: > > + return True > > + return False > > This should just be: > return any([path.find(directory) != -1 for directory in > self._test_resource_files["directories"]]) Done.
Comment on attachment 287936 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287936&action=review > Tools/Scripts/webkitpy/w3c/test_importer.py:475 > + resource_file_path = full_path.replace(self.source_directory, '')[1:] Presumably you're stripping the trailing / with [1:] but this is not robust nor is semantically descriptive. Use .rstrip('/') instead. An even better approach is to use self.filesystem.relpath.
Created attachment 287944 [details] Patch for landing
(In reply to comment #5) > Comment on attachment 287936 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=287936&action=review > > > Tools/Scripts/webkitpy/w3c/test_importer.py:475 > > + resource_file_path = full_path.replace(self.source_directory, '')[1:] > > Presumably you're stripping the trailing / with [1:] but this is not robust > nor is semantically descriptive. > Use .rstrip('/') instead. An even better approach is to use > self.filesystem.relpath. Done, thanks.
Comment on attachment 287944 [details] Patch for landing Clearing flags on attachment: 287944 Committed r205447: <http://trac.webkit.org/changeset/205447>
All reviewed patches have been landed. Closing bug.
It broke a python test: [1432/1462] webkitpy.w3c.test_importer_unittest.TestImporterTest.test_clean_directory_option erred: Traceback (most recent call last): File "/home/slave/webkitgtk/gtk-linux-64-release-tests/build/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py", line 204, in test_clean_directory_option fs = self.import_downloaded_tests(['--no-fetch', '--import-all', '-d', 'w3c', '--clean-dest-dir'], FAKE_FILES) File "/home/slave/webkitgtk/gtk-linux-64-release-tests/build/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py", line 132, in import_downloaded_tests importer = TestImporter(host, None, options) File "/home/slave/webkitgtk/gtk-linux-64-release-tests/build/Tools/Scripts/webkitpy/w3c/test_importer.py", line 184, in __init__ self._test_resource_files["files"] = [] TypeError: 'NoneType' object does not support item assignment
This should be covered by bug 161604, let me know if you still see broken tests.