Bug 163018 - Import html/webappapis web platform tests
Summary: Import html/webappapis web platform tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-06 09:39 PDT by Chris Dumez
Modified: 2016-10-10 03:10 PDT (History)
4 users (show)

See Also:


Attachments
Patch (269.29 KB, patch)
2016-10-06 10:20 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (266.72 KB, patch)
2016-10-06 11:44 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-10-06 09:39:29 PDT
Import html/webappapis web platform tests to extend test coverage.
Comment 1 Chris Dumez 2016-10-06 10:20:27 PDT
Created attachment 290828 [details]
Patch
Comment 2 youenn fablet 2016-10-06 11:24:40 PDT
Comment on attachment 290828 [details]
Patch

r=me.
Would you be able to fix the link issues?
And for the tests being blocked, maybe it is not worth importing them at the moment?

View in context: https://bugs.webkit.org/attachment.cgi?id=290828&action=review

> LayoutTests/imported/w3c/web-platform-tests/html/webappapis/animation-frames/callback-exception.html:5
> +    <script src="../../../../../../resources/testharness.js"></script>

The links should just be src="../../../../../../resources/testharness.js".
Maybe I miss that on your previous patches.
In your set-up, you might need to use --no-links-conversion option I think.

> LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/events/inline-event-handler-ordering-expected.txt:4
> +Harness Error (FAIL), message = SyntaxError: Unexpected token '}'

That seems odd.

> LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/processing-model-2/compile-error-cross-origin-expected.txt:1
> +Blocked access to external URL http://www1.localhost:8800/html/webappapis/scripting/processing-model-2/support/syntax-error.js

Might need to fix that test as done previously?

> LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/processing-model-2/runtime-error-cross-origin-expected.txt:1
> +Blocked access to external URL http://www1.localhost:8800/html/webappapis/scripting/processing-model-2/support/undefined-variable.js

Ditto.
Comment 3 Chris Dumez 2016-10-06 11:44:24 PDT
Created attachment 290840 [details]
Patch
Comment 4 Chris Dumez 2016-10-06 11:48:24 PDT
(In reply to comment #2)
> Comment on attachment 290828 [details]
> Patch
> 
> r=me.
> Would you be able to fix the link issues?
> And for the tests being blocked, maybe it is not worth importing them at the
> moment?
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290828&action=review
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/webappapis/animation-frames/callback-exception.html:5
> > +    <script src="../../../../../../resources/testharness.js"></script>
> 
> The links should just be src="../../../../../../resources/testharness.js".
> Maybe I miss that on your previous patches.
> In your set-up, you might need to use --no-links-conversion option I think.

I used to use -l but I did not use it this time because I thought I understood in your previous review that I shouldn't.

> 
> > LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/events/inline-event-handler-ordering-expected.txt:4
> > +Harness Error (FAIL), message = SyntaxError: Unexpected token '}'
> 
> That seems odd.
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/processing-model-2/compile-error-cross-origin-expected.txt:1
> > +Blocked access to external URL http://www1.localhost:8800/html/webappapis/scripting/processing-model-2/support/syntax-error.js
> 
> Might need to fix that test as done previously?
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/processing-model-2/runtime-error-cross-origin-expected.txt:1
> > +Blocked access to external URL http://www1.localhost:8800/html/webappapis/scripting/processing-model-2/support/undefined-variable.js
> 
> Ditto.
Comment 5 Chris Dumez 2016-10-06 12:20:17 PDT
Committed r206874: <http://trac.webkit.org/changeset/206874>
Comment 6 youenn fablet 2016-10-06 23:28:32 PDT
> > The links should just be src="../../../../../../resources/testharness.js".
> > Maybe I miss that on your previous patches.
> > In your set-up, you might need to use --no-links-conversion option I think.
> 
> I used to use -l but I did not use it this time because I thought I
> understood in your previous review that I shouldn't.

Sorry about that misunderstanding.
The -l option is not needed when importing without passing a path to the test source directory but is needed otherwise.

I was about to update https://trac.webkit.org/wiki/WebKitW3CTesting.
But it seems I do not have write access any longer.
When I'll get access, I plan to document your setup which is probably more common than mine.

So you used something like: Tools/Scripts/import-w3c-tests -l /Volumes/Data/w3c -t web-platform-tests/webappapis?
Comment 7 Chris Dumez 2016-10-07 08:54:00 PDT
(In reply to comment #6)
> > > The links should just be src="../../../../../../resources/testharness.js".
> > > Maybe I miss that on your previous patches.
> > > In your set-up, you might need to use --no-links-conversion option I think.
> > 
> > I used to use -l but I did not use it this time because I thought I
> > understood in your previous review that I shouldn't.
> 
> Sorry about that misunderstanding.
> The -l option is not needed when importing without passing a path to the
> test source directory but is needed otherwise.
> 
> I was about to update https://trac.webkit.org/wiki/WebKitW3CTesting.
> But it seems I do not have write access any longer.
> When I'll get access, I plan to document your setup which is probably more
> common than mine.
> 
> So you used something like: Tools/Scripts/import-w3c-tests -l
> /Volumes/Data/w3c -t web-platform-tests/webappapis?

I did:
Tools/Scripts/import-w3c-tests ~/web-platform-tests/html/ -t webappapis -d imported/w3c/web-platform-tests/html/ -l

This mostly worked except that the resources file has wrong relative paths and I had to fix it manually.

FYI, I tried the following but it did not import anything
Tools/Scripts/import-w3c-tests ~/web-platform-tests/html/webappapis -d imported/w3c/web-platform-tests/html/
Comment 8 youenn fablet 2016-10-10 03:10:21 PDT
> I did:
> Tools/Scripts/import-w3c-tests ~/web-platform-tests/html/ -t webappapis -d
> imported/w3c/web-platform-tests/html/ -l
> 
> This mostly worked except that the resources file has wrong relative paths
> and I had to fix it manually.

To fix the relative path, the -d should be set to imported/w3c. Something like that should work.

Tools/Scripts/import-w3c-tests ~ -t web-platform-tests/html/webappapis -d imported/w3c/ -l


> FYI, I tried the following but it did not import anything
> Tools/Scripts/import-w3c-tests ~/web-platform-tests/html/webappapis -d
> imported/w3c/web-platform-tests/html/

When no -t parameter is passed, it expects to import "csswg-test"and "web-platform-tests" sub-directories but there is none in the folder you gave.
The following should import tests and kind-of work:

Tools/Scripts/import-w3c-tests ~/web-platform-tests/html/webappapis -d imported/w3c/web-platform-tests/html/webappapis -l -t .