In bug 171781 I'd like to make a change to File which is tested by the WPT FileAPI tests. These tests haven't been imported into webkit yet, though. Lets import the tests first and then I'll make the change in bug 171781 on top of them. That way it will be easier to see what changes.
Created attachment 309675 [details] Patch
Comment on attachment 309675 [details] Patch Attachment 309675 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3715266 New failing tests: imported/w3c/web-platform-tests/FileAPI/url/url_createobjecturl_file_img-manual.html
Created attachment 309679 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 309675 [details] Patch Attachment 309675 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3715294 New failing tests: imported/w3c/web-platform-tests/FileAPI/url/url_createobjecturl_file_img-manual.html
Created attachment 309681 [details] Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 309675 [details] Patch Attachment 309675 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3715275 New failing tests: imported/w3c/web-platform-tests/FileAPI/url/url_createobjecturl_file_img-manual.html
Created attachment 309682 [details] Archive of layout-test-results from ews114 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Are you using the importer script in Tools/Scripts? Your patch seems to contain manual tests.
(In reply to Chris Dumez from comment #8) > Are you using the importer script in Tools/Scripts? Your patch seems to > contain manual tests. Ah, no. I was manually doing it. I'll take a look at the script.
Created attachment 309684 [details] Patch
Comment on attachment 309684 [details] Patch Wrong bug number. This was supposed to go to bug 171781.
(In reply to Ben Kelly from comment #9) > (In reply to Chris Dumez from comment #8) > > Are you using the importer script in Tools/Scripts? Your patch seems to > > contain manual tests. > > Ah, no. I was manually doing it. I'll take a look at the script. Yes, please do. The script does a lot of things for you. Personally, I do: Tools/Scripts/import-w3c-tests FileAPI -l -s /Volumes/Data Note: I have my web-platform-tests checkout under /Volumes/Data.
(In reply to Chris Dumez from comment #12) > Yes, please do. The script does a lot of things for you. I guess it probably would have handled the ".worker.js" WPT magic for me. > Personally, I do: > Tools/Scripts/import-w3c-tests FileAPI -l -s /Volumes/Data > > Note: I have my web-platform-tests checkout under /Volumes/Data. Perfect. I'll try this tomorrow night. Thanks!
(In reply to Ben Kelly from comment #13) > (In reply to Chris Dumez from comment #12) > > Yes, please do. The script does a lot of things for you. > > I guess it probably would have handled the ".worker.js" WPT magic for me. Youenn is more familiar but I believe it does indeed. > > > Personally, I do: > > Tools/Scripts/import-w3c-tests FileAPI -l -s /Volumes/Data > > > > Note: I have my web-platform-tests checkout under /Volumes/Data. > > Perfect. I'll try this tomorrow night. Thanks!
(In reply to Chris Dumez from comment #12) > (In reply to Ben Kelly from comment #9) > > (In reply to Chris Dumez from comment #8) > > > Are you using the importer script in Tools/Scripts? Your patch seems to > > > contain manual tests. > > > > Ah, no. I was manually doing it. I'll take a look at the script. > > Yes, please do. The script does a lot of things for you. > > Personally, I do: > Tools/Scripts/import-w3c-tests FileAPI -l -s /Volumes/Data > > Note: I have my web-platform-tests checkout under /Volumes/Data. I think the recommended way is to do: Tools/Scripts/import-w3c-tests web-platform-tests/FileAPI Now that csswg-test is a thing of the past, we should update the script so that you would only need to type: Tools/Scripts/import-w3c-tests FileAPI If you really want the latests tests, you can use the '-t' option. It will take some time the first time at it will clone the WPT repo in WebKitBuild repository so be patient. The idea is that this programmatically-controlled clone will also be used to export changes from WebKit. The exporter will create the necessary commits/branches/remotes.
(In reply to Ben Kelly from comment #13) > (In reply to Chris Dumez from comment #12) > > Yes, please do. The script does a lot of things for you. > > I guess it probably would have handled the ".worker.js" WPT magic for me. Yes, WebKit test infra requires a .html file to run the test so the importer adds a dummy file, which is never actually served through WPT. > > Personally, I do: > > Tools/Scripts/import-w3c-tests FileAPI -l -s /Volumes/Data > > > > Note: I have my web-platform-tests checkout under /Volumes/Data. > > Perfect. I'll try this tomorrow night. Thanks! Again, Tools/Scripts/import-w3c-tests web-platform-tests/FileAPI !
Created attachment 310043 [details] Patch
Note, when I run this patch locally I get this failure: Regressions: Unexpected image-only failures (1) imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html [ ImageOnlyFailure ] I'm not sure the correct way to handle that here. If the test is flakey, should I disable it in some way?
(In reply to Ben Kelly from comment #18) > Note, when I run this patch locally I get this failure: > > Regressions: Unexpected image-only failures (1) > imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html [ > ImageOnlyFailure ] > > I'm not sure the correct way to handle that here. If the test is flakey, > should I disable it in some way? Yes, you can disable it by adding this line: imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html [ ImageOnlyFailure ] to LayoutTests/TestExpectations Ideally, you'd file a bug for this failure and prepend the bug URL to the line like so: webkit.org/b/12345 imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html [ ImageOnlyFailure ]
(In reply to Chris Dumez from comment #19) > Yes, you can disable it by adding this line: > imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html [ > ImageOnlyFailure ] > > to LayoutTests/TestExpectations Well, the weird thing is I auto-generated the expectations by running the tests after the import. Did the test pass that time, but fail after? Or does that auto-generated expectation feature not work for image failures? I guess I worry about adding a test that sometimes passes and sometimes fails.
(In reply to Ben Kelly from comment #20) > (In reply to Chris Dumez from comment #19) > > Yes, you can disable it by adding this line: > > imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html [ > > ImageOnlyFailure ] > > > > to LayoutTests/TestExpectations > > Well, the weird thing is I auto-generated the expectations by running the > tests after the import. Did the test pass that time, but fail after? Or > does that auto-generated expectation feature not work for image failures? > > I guess I worry about adding a test that sometimes passes and sometimes > fails. Looks like a ref-test so the expected result is an HTML file which comes from web-platform-tests and is not updated/generated by our tools (unlike tests with text expectations).
(In reply to Chris Dumez from comment #21) > Looks like a ref-test so the expected result is an HTML file which comes > from web-platform-tests and is not updated/generated by our tools (unlike > tests with text expectations). Perfect. Thanks for clearing that up. I'll let EWS finish since I already started it in case there are other problems I need to deal with and then upload a patch with a fixed expecation tomorrow.
Comment on attachment 310043 [details] Patch Attachment 310043 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3735835 New failing tests: imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html
Created attachment 310050 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 310043 [details] Patch Attachment 310043 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3735843 New failing tests: imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html
Created attachment 310052 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 310043 [details] Patch Attachment 310043 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3735838 New failing tests: imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html
Created attachment 310053 [details] Archive of layout-test-results from ews113 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 310043 [details] Patch Attachment 310043 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3735874 New failing tests: imported/w3c/web-platform-tests/FileAPI/url/url_xmlhttprequest_img.html
Created attachment 310055 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 310105 [details] Patch
Updated the patch based on comment 19. Unfortunately I couldn't test locally as my tree seems busted after updating this evening. Let's see what EWS says.
Comment on attachment 310105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310105&action=review > LayoutTests/tests-options.json:12 > + "slow" This change is probably not needed anymore.
(In reply to youenn fablet from comment #33) > > LayoutTests/tests-options.json:12 > > + "slow" > > This change is probably not needed anymore. Hmm, can you explain why? AFAICT this was put there by the import script.
(In reply to Ben Kelly from comment #34) > (In reply to youenn fablet from comment #33) > > > LayoutTests/tests-options.json:12 > > > + "slow" > > > > This change is probably not needed anymore. > > Hmm, can you explain why? AFAICT this was put there by the import script. This test is a manual test. Test importer by default does not import such test. In your patch, it does not seem to be added anymore. Was this change done with your last script importer run? If so, there might be a bug in the importer.
The patch looks good besides Youenn's comment that should be addressed. r=me with Youenn's fix included.
(In reply to youenn fablet from comment #35) > This test is a manual test. Test importer by default does not import such > test. > In your patch, it does not seem to be added anymore. > Was this change done with your last script importer run? > If so, there might be a bug in the importer. No, I started from scratch with the importer script. So I'm pretty sure the importer script added this line.
(In reply to Ben Kelly from comment #37) > (In reply to youenn fablet from comment #35) > > This test is a manual test. Test importer by default does not import such > > test. > > In your patch, it does not seem to be added anymore. > > Was this change done with your last script importer run? > > If so, there might be a bug in the importer. > > No, I started from scratch with the importer script. So I'm pretty sure the > importer script added this line. Looking at the importer, it indeed sounds like a bug in the way we compute _slow_tests. I'll fix it.
(In reply to youenn fablet from comment #38) > (In reply to Ben Kelly from comment #37) > > (In reply to youenn fablet from comment #35) > > > This test is a manual test. Test importer by default does not import such > > > test. > > > In your patch, it does not seem to be added anymore. > > > Was this change done with your last script importer run? > > > If so, there might be a bug in the importer. > > > > No, I started from scratch with the importer script. So I'm pretty sure the > > importer script added this line. > > Looking at the importer, it indeed sounds like a bug in the way we compute > _slow_tests. > I'll fix it. Fixed as per http://trac.webkit.org/changeset/216900.
(In reply to youenn fablet from comment #39) > Fixed as per http://trac.webkit.org/changeset/216900. Thanks! Sorry I didn't get to fix this patch last night. I'll fix it up and flag for review tonight.
Created attachment 310328 [details] Patch
Comment on attachment 310328 [details] Patch Attachment 310328 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3760445 New failing tests: http/tests/media/hls/video-duration-accessibility.html http/tests/media/hls/video-controls-live-stream.html
Created attachment 310367 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
(In reply to Build Bot from comment #42) > New failing tests: > http/tests/media/hls/video-duration-accessibility.html > http/tests/media/hls/video-controls-live-stream.html Hmm... Not sure how adding FileAPI WPT tests could cause these media tests to fail.
(In reply to Ben Kelly from comment #44) > (In reply to Build Bot from comment #42) > > New failing tests: > > http/tests/media/hls/video-duration-accessibility.html > > http/tests/media/hls/video-controls-live-stream.html > > Hmm... Not sure how adding FileAPI WPT tests could cause these media tests > to fail. Bots sometimes have such flaky regressions.
Comment on attachment 310328 [details] Patch Clearing flags on attachment: 310328 Committed r216975: <http://trac.webkit.org/changeset/216975>
All reviewed patches have been landed. Closing bug.