Bug 162331 – Import html/syntax web platform tests

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Import html/syntax web platform tests.

[Chris Dumez]

Created attachment 289426 [details]
Patch

[Build Bot]

Comment on attachment 289426 [details]
Patch

Attachment 289426 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2116369

New failing tests:
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_009.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_003.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_004.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_005.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_001.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_013.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_010.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_008.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_006.html
imported/w3c/web-platform-tests/html/syntax/parsing-html-fragments/the-input-byte-stream-004.html
imported/w3c/web-platform-tests/html/syntax/parsing-html-fragments/the-input-byte-stream-003.html
imported/w3c/web-platform-tests/html/syntax/parsing/foreign_content_011.html

[Build Bot]

Created attachment 289431 [details]
Archive of layout-test-results from ews125 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Chris Dumez]

Created attachment 289432 [details]
Patch

[youenn fablet]

Comment on attachment 289432 [details]
Patch

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

> LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing-html-fragments/the-input-byte-stream-003-expected.txt:5
> +  RenderView at (0,0) size 800x600

I guess these two tests will get fixed at some point.

[WebKit Commit Bot]

Comment on attachment 289432 [details]
Patch

Clearing flags on attachment: 289432

Committed r206201: <http://trac.webkit.org/changeset/206201>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

(In reply to comment #5)
> Comment on attachment 289432 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289432&action=review
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing-html-fragments/the-input-byte-stream-003-expected.txt:5
> > +  RenderView at (0,0) size 800x600
> 
> I guess these two tests will get fixed at some point.

I am not sure the issue is with the test itself. The test looks alright but it is in UTF16 without BOM. It could be that we are not decoding properly.

[youenn fablet]

Comment on attachment 289432 [details]
Patch

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

> LayoutTests/imported/w3c/resources/resource-files.json:26
> +        "syntax/parsing/foreign_content_013.html",

There seems to be a bug here, it should be something like:
"web-platform-tests/syntax/parsing-html-fragments/the-input-byte-stream-003.html"
"web-platform-tests/html/syntax/parsing/foreign_content_001.html"
...

That would ensure these tests are actually skipped since they are classified as resource files.
The patch does not contain any of the syntax/parsing/foreign_content*.html though.

Did you run the import script? If so, how did you run it?

The import script would also make skipping syntax/parsing-html-fragments/the-input-byte-stream-003.html probably because the python html parser is not able to parse it correctly.
We should use the WPT manifest and not the python html parser

[Chris Dumez]

(In reply to comment #9)
> Comment on attachment 289432 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289432&action=review
> 
> > LayoutTests/imported/w3c/resources/resource-files.json:26
> > +        "syntax/parsing/foreign_content_013.html",
> 
> There seems to be a bug here, it should be something like:
> "web-platform-tests/syntax/parsing-html-fragments/the-input-byte-stream-003.
> html"
> "web-platform-tests/html/syntax/parsing/foreign_content_001.html"
> ...
> 
> That would ensure these tests are actually skipped since they are classified
> as resource files.
> The patch does not contain any of the syntax/parsing/foreign_content*.html
> though.
> 
> Did you run the import script? If so, how did you run it?
> 
> The import script would also make skipping
> syntax/parsing-html-fragments/the-input-byte-stream-003.html probably
> because the python html parser is not able to parse it correctly.
> We should use the WPT manifest and not the python html parser

1. I imported them like so:
Tools/Scripts/import-w3c-tests -l /Volumes/Data/web-platform-tests-fork/html/ -t syntax/ -d imported/w3c/web-platform-tests/html/

2. I removed web-platform-tests/html/syntax/parsing/foreign_content* from my patch because they did not have expected result and it caused the iOS EWS to fail. I tried to generated expected results for them on Mac but it would not.

[youenn fablet]

> 1. I imported them like so:
> Tools/Scripts/import-w3c-tests -l
> /Volumes/Data/web-platform-tests-fork/html/ -t syntax/ -d
> imported/w3c/web-platform-tests/html/

I see. The generation of resource files probably does not work with this set of arguments.

With your setup, something like the following should work:
Tools/Scripts/import-w3c-tests -l /Volumes/Data -t web-platform-tests/html/syntax

But that would require renaming your /Volumes/Data/web-platform-tests-fork folder in /Volumes/Data/web-platform-tests.
Or moving it to /Volumes/Data/w3c/web-platform-tests and using
Tools/Scripts/import-w3c-tests -l /Volumes/Data/w3c -t web-platform-tests/html/syntax

My way of importing is the following:
1. Edit LayoutTests/imported/w3c/ImportExpectations to unskip all of web-platform-tests/html/syntax
2. Run Tools/Scripts/import-w3c-tests -t web-platform-tests/html/syntax
On the first time you do not provide a -l option, the script will checkout the wpt and csswg repositories in WebKitBuild folder, so it takes some time.

> 2. I removed web-platform-tests/html/syntax/parsing/foreign_content* from my
> patch because they did not have expected result and it caused the iOS EWS to
> fail. I tried to generated expected results for them on Mac but it would not.

Looking at these tests, they are not testharness.js based and not reference tests.
That is why the test importer classifies them as resource files.
But with the resource-files.json issue, they were still run.

[Brent Fulgham]

This change has substantially regressed Windows test bot run time, moving us from 25 minutes per test run to over an hour.

I've filed Bug 162415 about this problem.

Should we roll this change out?

[Chris Dumez]

(In reply to comment #12)
> This change has substantially regressed Windows test bot run time, moving us
> from 25 minutes per test run to over an hour.
> 
> I've filed Bug 162415 about this problem.
> 

Those tests run under 1 minute on my Mac machines and they bring important test coverage for what I am currently working on. If some of the tests are slow, I think they should be skipped on Windows and bugs should be filed. I don't think rolling out would be appropriate here as this is not a code change that caused a regression.

Chances are that Windows is missing some features and a lot of tests are timing out because of it.