When regularly importing tests from W3C to WebKit, the process is usually: 1. Remove all files, except -expected.txt files from LayoutTests/imported/w3c/web-platform-tests 2. Import test using test importer 3. Remove -expected.txt files that no longer have a corresponding file. It would be nice for test importer to automate step 1 and 3.
Created attachment 268182 [details] Patch
Comment on attachment 268182 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268182&action=review r-'ing this patch because it will delete files in .svn directories and .gitattributes files. > Tools/Scripts/webkitpy/w3c/test_importer.py:235 > + def _should_not_keep_when_importing(self, filesystem, dirname, filename): Would it make sense to keep all hidden files? At the very least we should also keep .gitattributes files and .svn directories. I also noticed in my checkout of /LayoutTests/imported/w3c/web-platform-tests I have files .tools.url and .resources.url. I take it these are generated and OK to delete. > Tools/Scripts/webkitpy/w3c/test_importer.py:237 > + return False It tends to be more difficult to reason about negations and concepts like "not keeping a file" than "keeping a file". I suggest either changing the meaning of this function such that it returns whether to "keep a file when importing" or adding inline comments on this line (line 237), line 239, and line 240 to better describe the implications of the return value. Such comments could be the form "Keep", "Keep", "Do not keep", respectively. > Tools/Scripts/webkitpy/w3c/test_importer_unittest.py:199 > + '/mock-checkout/WebKitBuild/w3c-tests/csswg-tests/test.html': '1', Can we add tests for .gitignore, .gitattributes, and .svn directories?
Thanks for the review. > r-'ing this patch because it will delete files in .svn directories and > .gitattributes files. Right, I will fix that. > > Tools/Scripts/webkitpy/w3c/test_importer.py:235 > > + def _should_not_keep_when_importing(self, filesystem, dirname, filename): > > Would it make sense to keep all hidden files? At the very least we should > also keep .gitattributes files and .svn directories. > > I also noticed in my checkout of > /LayoutTests/imported/w3c/web-platform-tests I have files .tools.url and > .resources.url. I take it these are generated and OK to delete. It is ok to delete them right. But it does not hurt keeping them either. I will change the routine to keep all hidden files. > > Tools/Scripts/webkitpy/w3c/test_importer.py:237 > > + return False > > It tends to be more difficult to reason about negations and concepts like > "not keeping a file" than "keeping a file". I suggest either changing the > meaning of this function such that it returns whether to "keep a file when > importing" or adding inline comments on this line (line 237), line 239, and > line 240 to better describe the implications of the return value. Such > comments could be the form "Keep", "Keep", "Do not keep", respectively. Right. I will change the routine name to _should_remove_before_importing. Then it is clearer. > > > Tools/Scripts/webkitpy/w3c/test_importer_unittest.py:199 > > + '/mock-checkout/WebKitBuild/w3c-tests/csswg-tests/test.html': '1', > > Can we add tests for .gitignore, .gitattributes, and .svn directories? Will add it.
Created attachment 268272 [details] Keeping hidden files
Comment on attachment 268272 [details] Keeping hidden files Clearing flags on attachment: 268272 Committed r197163: <http://trac.webkit.org/changeset/197163>
All reviewed patches have been landed. Closing bug.