Bug 171960 - Import FileAPI WPT tests
Summary: Import FileAPI WPT tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ben Kelly
URL:
Keywords:
Depends on:
Blocks: 172099 171781
  Show dependency treegraph
 
Reported: 2017-05-10 18:46 PDT by Ben Kelly
Modified: 2017-05-17 09:04 PDT (History)
5 users (show)

See Also:


Attachments
Patch (186.28 KB, patch)
2017-05-10 18:55 PDT, Ben Kelly
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (771.60 KB, application/zip)
2017-05-10 20:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (936.90 KB, application/zip)
2017-05-10 20:12 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-elcapitan (1.53 MB, application/zip)
2017-05-10 20:14 PDT, Build Bot
no flags Details
Patch (8.25 KB, patch)
2017-05-10 20:26 PDT, Ben Kelly
no flags Details | Formatted Diff | Diff
Patch (160.99 KB, patch)
2017-05-13 18:09 PDT, Ben Kelly
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (774.12 KB, application/zip)
2017-05-13 19:18 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (907.62 KB, application/zip)
2017-05-13 19:23 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-elcapitan (1.58 MB, application/zip)
2017-05-13 19:30 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (772.96 KB, application/zip)
2017-05-13 19:48 PDT, Build Bot
no flags Details
Patch (161.70 KB, patch)
2017-05-14 20:56 PDT, Ben Kelly
no flags Details | Formatted Diff | Diff
Patch (161.50 KB, patch)
2017-05-16 17:28 PDT, Ben Kelly
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (1.12 MB, application/zip)
2017-05-17 02:56 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Kelly 2017-05-10 18:46:21 PDT
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.
Comment 1 Ben Kelly 2017-05-10 18:55:07 PDT
Created attachment 309675 [details]
Patch
Comment 2 Build Bot 2017-05-10 20:03:29 PDT
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
Comment 3 Build Bot 2017-05-10 20:03:31 PDT
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 4 Build Bot 2017-05-10 20:12:44 PDT
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
Comment 5 Build Bot 2017-05-10 20:12:45 PDT
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 6 Build Bot 2017-05-10 20:14:33 PDT
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
Comment 7 Build Bot 2017-05-10 20:14:34 PDT
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
Comment 8 Chris Dumez 2017-05-10 20:15:28 PDT
Are you using the importer script in Tools/Scripts? Your patch seems to contain manual tests.
Comment 9 Ben Kelly 2017-05-10 20:21:05 PDT
(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.
Comment 10 Ben Kelly 2017-05-10 20:26:29 PDT
Created attachment 309684 [details]
Patch
Comment 11 Ben Kelly 2017-05-10 20:27:22 PDT
Comment on attachment 309684 [details]
Patch

Wrong bug number.  This was supposed to go to bug 171781.
Comment 12 Chris Dumez 2017-05-10 20:35:31 PDT
(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.
Comment 13 Ben Kelly 2017-05-10 20:38:34 PDT
(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!
Comment 14 Chris Dumez 2017-05-10 20:39:56 PDT
(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!
Comment 15 youenn fablet 2017-05-10 20:45:12 PDT
(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.
Comment 16 youenn fablet 2017-05-10 20:47:13 PDT
(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 !
Comment 17 Ben Kelly 2017-05-13 18:09:31 PDT
Created attachment 310043 [details]
Patch
Comment 18 Ben Kelly 2017-05-13 18:12:35 PDT
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?
Comment 19 Chris Dumez 2017-05-13 18:45:37 PDT
(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 ]
Comment 20 Ben Kelly 2017-05-13 18:48:13 PDT
(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.
Comment 21 Chris Dumez 2017-05-13 18:56:23 PDT
(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).
Comment 22 Ben Kelly 2017-05-13 19:04:06 PDT
(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 23 Build Bot 2017-05-13 19:18:50 PDT
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
Comment 24 Build Bot 2017-05-13 19:18:51 PDT
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 25 Build Bot 2017-05-13 19:23:50 PDT
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
Comment 26 Build Bot 2017-05-13 19:23:51 PDT
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 27 Build Bot 2017-05-13 19:30:57 PDT
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
Comment 28 Build Bot 2017-05-13 19:30:58 PDT
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 29 Build Bot 2017-05-13 19:48:03 PDT
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
Comment 30 Build Bot 2017-05-13 19:48:05 PDT
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
Comment 31 Ben Kelly 2017-05-14 20:56:21 PDT
Created attachment 310105 [details]
Patch
Comment 32 Ben Kelly 2017-05-14 20:57:13 PDT
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 33 youenn fablet 2017-05-14 21:58:27 PDT
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.
Comment 34 Ben Kelly 2017-05-15 06:12:14 PDT
(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.
Comment 35 youenn fablet 2017-05-15 09:07:41 PDT
(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.
Comment 36 Chris Dumez 2017-05-15 09:29:35 PDT
The patch looks good besides Youenn's comment that should be addressed. r=me with Youenn's fix included.
Comment 37 Ben Kelly 2017-05-15 10:02:26 PDT
(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.
Comment 38 youenn fablet 2017-05-15 10:05:31 PDT
(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.
Comment 39 youenn fablet 2017-05-15 22:39:14 PDT
(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.
Comment 40 Ben Kelly 2017-05-16 07:09:19 PDT
(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.
Comment 41 Ben Kelly 2017-05-16 17:28:11 PDT
Created attachment 310328 [details]
Patch
Comment 42 Build Bot 2017-05-17 02:56:11 PDT
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
Comment 43 Build Bot 2017-05-17 02:56:12 PDT
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
Comment 44 Ben Kelly 2017-05-17 06:12:40 PDT
(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.
Comment 45 youenn fablet 2017-05-17 08:33:09 PDT
(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 46 WebKit Commit Bot 2017-05-17 09:04:39 PDT
Comment on attachment 310328 [details]
Patch

Clearing flags on attachment: 310328

Committed r216975: <http://trac.webkit.org/changeset/216975>
Comment 47 WebKit Commit Bot 2017-05-17 09:04:41 PDT
All reviewed patches have been landed.  Closing bug.