Add support for CSS WG-style manifests #90

gsnedders · 2016-07-27T22:46:15Z

This is mostly legacy. We may well want to add lints to avoid this spreading.

Note the presence of the font flag as making a test manual isn't currently needed for Servo, because they install all the possibly needed fonts, so tests with that can be run in an automated manner. However, in general we only require Ahem to be installed presently.

To-do:

get CSS manual and visual tests picked up
add all the "visual" tests (probably as a new category, for MS's sake)
add lints to avoid messiness spreading
figure out why the set of reftests hasn't changed more than it has
ignore support files
figure out what to do with reftest cycles
figure out what to do with tests referenced by other tests
make sure we don't add "visual" tests from content_is_css_visual in wpt

This change is

Ms2ger · 2016-07-28T12:31:16Z

manifest/sourcefile.py

+    @cached_property
+    def css_flag_nodes(self):
+        """List of ElementTree Elements corresponding to nodes representing a
+        to a flag <meta>"""


codecov-io · 2016-08-19T16:17:23Z

Current coverage is 83.29% (diff: 90.30%)

Merging #90 into master will increase coverage by 1.04%

@@             master        #90   diff @@
==========================================
  Files            22         24     +2   
  Lines          1668       1886   +218   
  Methods           0          0          
  Messages          0          0          
  Branches        300        324    +24   
==========================================
+ Hits           1372       1571   +199   
- Misses          237        248    +11   
- Partials         59         67     +8

Powered by Codecov. Last update 08b225a...8b6d2a6

gsnedders · 2016-08-21T20:44:06Z

https://gist.github.com/gsnedders/56386d08ceed90b871cfa2ed16361296 is the current diff in manifests. reftests are currently expected to be pretty different because we use very different logic to find the nodes of the graph which represent tests.

Ms2ger · 2016-08-22T11:35:47Z

manifest/tests/test_sourcefile.py

+<head>
+<link rel="help" href="http://www.w3.org/TR/CSS21/box.html#bidi-box-model"/>
+</head>
+<body/>


Nit: don't use /> syntax

That was so this can run as both test.html and test.xht?

<body/> doesn't quite work in HTML

gsnedders · 2016-08-24T01:54:56Z

So the things in old but not new manifest:

testharness tests in old but not new:
css-transforms-1/transform_translate.html
css-transforms-1/transform_translate_invalid.html
css-transforms-1/transform_translate_max.html
css-transforms-1/transform_translate_min.html
css-transforms-1/transform_translate_neg.html
css-transforms-1/transform_translate_second_omited.html
css-transforms-1/transform_translate_zero.html
css-ui-3/caret-color-009.html

These all use relative URLs to testharness.js

css-values-3/calc-unit-analysis.html
css-variables-1/test_variable_legal_values.html

These all use absolute URLs to testharness.js on w3ctest.org

visual tests in old but not new:
css-backgrounds-3/box-shadow-syntax-001.xht

Caused by w3c/csswg-test#1118 as the test doesn't make sense currently, no diff with that PR applied

css-shapes-1/shape-outside/shape-image/gradients/shape-outside-radial-gradient-001.html
css-shapes-1/shape-outside/shape-image/gradients/shape-outside-radial-gradient-002.html
css-shapes-1/shape-outside/shape-image/gradients/shape-outside-radial-gradient-003.html
css-shapes-1/shape-outside/shape-image/gradients/shape-outside-radial-gradient-004.html
css-transforms-1/2d-rotate-js.html

These all contain the testharness.js script in the <body> which the old generator doesn't pick up, so we have it right

manual tests in old but not new:
css-transitions-1/transition-property-044.html
css-transitions-1/transition-property-045.html

We're currently picking these up as visual instead of manual; w3c/csswg-test#1119 fixes this

reftest nodes in old but not new:
css21/syntax/at-charset-012.xht

When it comes to XML, we don't support anything but UTF-8, UTF-16, ISO-8859-1, and ASCII. That test is Shift-JIS.

So, that only leaves us to work out whether we want to handle relative paths to testharness.js and supporting Shift-JIS to get all of these right.

The ones in new but not old are mostly tests which miss <link rel=help> and hence aren't picked up by the old generator. This needs a closer review to check there isn't anything wrong there.

gsnedders · 2016-08-24T03:31:00Z

So something is wrong somewhere, because we end up with css21/syntax/at-charset-001.xht as a reftest which per @jgraham's logic it shouldn't be, as it isn't a source vertex in the graph.

gsnedders · 2016-08-31T19:24:07Z

That's now #108

gsnedders · 2016-09-13T18:04:50Z

OK, so the diff, ignoring things that are definitely just missing from what build.py/w3ctestlib choose to build…

testharness tests in new but not old
css-syntax-3/charset/page-utf16-css-no-decl-ascii-only.html
css-syntax-3/charset/page-utf16-css-no-decl.html
css-syntax-3/charset/page-windows-1251-charset-attribute-bogus.html
css-syntax-3/charset/page-windows-1251-css-at-charset-1250-charset-attribute-windows-1253.html
css-syntax-3/charset/page-windows-1251-css-at-charset-bogus-charset-attribute-windows-1250.html
css-syntax-3/charset/page-windows-1251-css-at-charset-bogus.html
css-syntax-3/charset/page-windows-1251-css-at-charset-utf16-ascii-only.html
css-syntax-3/charset/page-windows-1251-css-at-charset-utf16.html
css-syntax-3/charset/page-windows-1251-css-at-charset-utf16be.html
css-syntax-3/charset/page-windows-1251-css-at-charset-windows-1250-in-utf16.html
css-syntax-3/charset/page-windows-1251-css-at-charset-windows-1250-in-utf16be.html
css-syntax-3/charset/page-windows-1251-css-http-bogus-at-charset-windows-1250.html
css-syntax-3/charset/page-windows-1251-css-http-bogus.html
css-syntax-3/charset/page-windows-1251-css-http-windows-1250-at-charset-windows-1253.html
css-syntax-3/charset/page-windows-1251-css-no-decl.html
css-syntax-3/charset/page-windows-1251-css-utf8-bom.html
css-syntax-3/charset/xml-stylesheet-page-windows-1251-charset-attribute-windows-1250.xhtml

These all have no link to any spec.

reftest tests in new but not old
compositing-1/compositing_simple_div.html

w3c/csswg-test#1124

vendor-imports/mozilla/mozilla-central-reftests/images3/support/template-object-fit-test.html
vendor-imports/mozilla/mozilla-central-reftests/images3/support/template-object-position-test.html

These are support files and not actual tests.

reftest tests in old but not new
css-backgrounds-3/border-bottom-left-radius-003.xht
css-backgrounds-3/border-bottom-left-radius-004.xht
css-backgrounds-3/border-bottom-left-radius-005.xht
css-backgrounds-3/border-bottom-left-radius-006.xht
css-backgrounds-3/border-bottom-left-radius-007.xht
css-backgrounds-3/border-bottom-left-radius-009.xht
css-backgrounds-3/border-bottom-right-radius-003.xht
css-backgrounds-3/border-bottom-right-radius-004.xht
css-backgrounds-3/border-bottom-right-radius-005.xht
css-backgrounds-3/border-bottom-right-radius-006.xht
css-backgrounds-3/border-bottom-right-radius-007.xht
css-backgrounds-3/border-bottom-right-radius-009.xht
css-backgrounds-3/border-top-left-radius-003.xht
css-backgrounds-3/border-top-left-radius-004.xht
css-backgrounds-3/border-top-left-radius-005.xht
css-backgrounds-3/border-top-left-radius-006.xht
css-backgrounds-3/border-top-left-radius-007.xht
css-backgrounds-3/border-top-left-radius-009.xht
css-backgrounds-3/border-top-right-radius-003.xht
css-backgrounds-3/border-top-right-radius-004.xht
css-backgrounds-3/border-top-right-radius-005.xht
css-backgrounds-3/border-top-right-radius-006.xht
css-backgrounds-3/border-top-right-radius-007.xht
css-backgrounds-3/border-top-right-radius-009.xht

These are all cycles and hence don't get considered as tests. (These are likely flawed anyway…)

w3c/csswg-test#1127

css-transforms-1/transform3d-rotatex-perspective-001.html
css21/syntax/at-charset-001.xht
css21/text/text-indent-114.xht

These are referenced by other tests as references. The first by css-transforms-1/transform3d-perspective-origin-001.html (which lists a bunch of things as mismatch refs, that test included; possibly mistaking them for AND instead of OR?). The second by a whole load of tests (which probably ought reference at-charset-001-ref.xht instead). The third by text-indent-113.xht which uses it as a mismatch alongside its own ref (again I think mistaking AND/OR).

visual tests in new but not old
css-backgrounds-3/border-image-repeat-001.htm

w3c/csswg-test#1125

css21/box-display/support/viewport-004-firstcanvas.htm
css21/box-display/support/viewport-004-secondcanvas.htm

Support files.

css21/other-formats/background-color-176.html

I have no idea why this isn't getting built by w3ctestlib.

css21/other-formats/control-characters-001.html

html5lib/lxml cannot parse

css21/other-formats/xml-lang-selectors-001.html
css21/other-formats/xml-lang-selectors-002.html

I have no idea why these aren't getting built by w3ctestlib.

manual tests in new but not old
WOFF2-UserAgent/Tests/xhtml1/support/available-001a.xht
WOFF2-UserAgent/Tests/xhtml1/support/available-001b.xht

support files

css-fonts-3/font-family-name-000.xht

This has no link to any spec.

gsnedders · 2016-09-14T15:32:08Z

The files picked up as visual in web-platform-tests after web-platform-tests/wpt#3714 are:

encrypted-media/resources/clearkey-retrieve-destroy-persistent-license.html
encrypted-media/resources/clearkey-retrieve-persistent-license.html
encrypted-media/resources/drm-retrieve-destroy-persistent-license.html
encrypted-media/resources/drm-retrieve-persistent-license.html
encrypted-media/resources/drm-retrieve-persistent-usage-record.html
encrypted-media/resources/retrieve-persistent-usage-record.html
presentation-api/controlling-ua/support/iframe.html
presentation-api/controlling-ua/support/presentation.html

Ms2ger

LGTM with comments fixed.

Ms2ger · 2016-09-28T13:30:23Z

manifest/sourcefile.py

+        return rv
+
+    @cached_property
+    def content_is_css_visual(self):


Consider limiting this to css/

This doesn't work in the short-term when we want it to work on csswg-test. Or are we just not caring about that any more?

I suppose it's fine either way... We'll rename them all after the merge anyway :)

gsnedders · 2016-11-24T21:04:27Z

Will merge after #125. No point in reviewing till after the rebase, because that's gonna be messy, and I won't do that till after it lands because it will end up with all the conflicts.

gsnedders · 2016-11-28T16:51:18Z

@jgraham, @Ms2ger: r?

jgraham · 2016-12-02T11:50:11Z

Reviewed 2 of 4 files at r5, 1 of 5 files at r6, 2 of 2 files at r7, 1 of 2 files at r9, 2 of 2 files at r10.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

manifest/sourcefile.py, line 352 at r1 (raw file):

Previously, gsnedders (Geoffrey Sneddon) wrote…

(@Ms2ger admitted he mistook this for support for visual tests, which indeed would, if done naïvely, get rid of all testharness/reftest tests, so this is resolved)

Are there actually manual tests with testharness.js links in them? Otherwise I propose moving this later for the sake of efficiency. Otherwise it might be nice to have some way to detect csswg-test and only look at these possibilities if we are in that repo (or a css subdirectory).

manifest/sourcefile.py, line 42 at r10 (raw file):

                        "tools"])

    dir_path_non_test = {("css21", "archive")}

I think it might be clearer to write css21/archive and later split on / or even
{item.split('/') for item in ["css21/archive"]} if you care about performance. But I guess this way is also OK.

manifest/sourcefile.py, line 415 at r10 (raw file):

    @cached_property
    def type(self):
        rv, _ = self.manifest_items()

It seems like you probably actually want to cache manifest_items() or cache the type when computing the manifest items, depending on how this is used.d

manifest/XMLParser.py, line 1 at r10 (raw file):

from os.path import dirname, join

This file could use some documentation on why it exists. It sort of seems plausible that it works, but I didn't read too closely. Does it have a noticeable performance impact?

Comments from Reviewable

gsnedders · 2016-12-02T15:59:35Z

Review status: all files reviewed at latest revision, 8 unresolved discussions.

manifest/sourcefile.py, line 303 at r1 (raw file):

Previously, Ms2ger wrote…

"a to a"

Done.

manifest/sourcefile.py, line 352 at r1 (raw file):

Previously, jgraham wrote…

Are there actually manual tests with testharness.js links in them? Otherwise I propose moving this later for the sake of efficiency. Otherwise it might be nice to have some way to detect csswg-test and only look at these possibilities if we are in that repo (or a css subdirectory).

Does this actually matter much? Almost the entire cost is hit when we run the first content_is test because that's the point at which we actually parse, and almost the entire cost is the parse.

We definitely have manual tests in wpt which use testharness.js, at least.

manifest/XMLParser.py, line 1 at r10 (raw file):

Previously, jgraham wrote…

This file could use some documentation on why it exists. It sort of seems plausible that it works, but I didn't read too closely. Does it have a noticeable performance impact?

Its performance is comparable to non-extension-module xml.etree.ElementTree (i.e., not xml.etree.cElementTree), except in the XHTML case we take a perf hit because of the size of the XHTML DTD (we could in theory not include it, and reparse the file with it on a parse error, but I doubt it's worth it?).

Comments from Reviewable

gsnedders · 2016-12-02T16:11:34Z

manifest/sourcefile.py, line 415 at r10 (raw file):

Previously, jgraham wrote…

It seems like you probably actually want to cache manifest_items() or cache the type when computing the manifest items, depending on how this is used.d

It's only used by the lint currently, so we in principle don't actually care about the items.

Comments from Reviewable

gsnedders · 2016-12-02T16:29:03Z

manifest/XMLParser.py, line 1 at r10 (raw file):

Previously, gsnedders (Geoffrey Sneddon) wrote…

Its performance is comparable to non-extension-module xml.etree.ElementTree (i.e., not xml.etree.cElementTree), except in the XHTML case we take a perf hit because of the size of the XHTML DTD (we could in theory not include it, and reparse the file with it on a parse error, but I doubt it's worth it?).

To give a rough idea:

On Py2.7.12, with the upstream XMLParser, on w-p-t + csswg-test:

real	1m44.461s
user	1m29.551s
sys	0m7.883s

And with this:

real	2m24.404s
user	2m5.471s
sys	0m11.000s

Obviously, with hashes we shouldn't be reparsing everything that often anyway.

Comments from Reviewable

gsnedders · 2016-12-02T16:47:06Z

Review status: 6 of 8 files reviewed at latest revision, 8 unresolved discussions.

manifest/sourcefile.py, line 42 at r10 (raw file):

Previously, jgraham wrote…

I think it might be clearer to write css21/archive and later split on / or even
{item.split('/') for item in ["css21/archive"]} if you care about performance. But I guess this way is also OK.

We can do whatever here. It doesn't really matter, IMO.

Comments from Reviewable

gsnedders · 2016-12-13T22:43:50Z

@jgraham @Ms2ger ping

Ms2ger · 2016-12-14T16:33:24Z

OK

Reviewed 2 of 4 files at r5, 1 of 5 files at r6, 1 of 2 files at r7, 1 of 2 files at r10, 3 of 3 files at r11.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Ms2ger · 2016-12-14T16:34:29Z

I don't see a reason to keep blocking this; if there's bugs left, we'll catch them at some point.

* Respect multiple headers with the same name in .headers files. Fixes #90.

Ms2ger reviewed Jul 28, 2016
View reviewed changes

gsnedders force-pushed the css_manifest branch from 9f869db to d44082c Compare August 19, 2016 16:11

Ms2ger reviewed Aug 22, 2016
View reviewed changes

gsnedders force-pushed the css_manifest branch 2 times, most recently from bb70a03 to 38f209b Compare August 31, 2016 16:50

gsnedders force-pushed the css_manifest branch from 38f209b to 17a0c06 Compare September 5, 2016 17:12

gsnedders force-pushed the css_manifest branch 2 times, most recently from 4403e37 to 49b4738 Compare September 16, 2016 15:35

Ms2ger force-pushed the css_manifest branch from 49b4738 to 3d78f9b Compare September 28, 2016 11:55

Ms2ger suggested changes Sep 28, 2016

View reviewed changes

gsnedders mentioned this pull request Nov 15, 2016

[meta] Merge repo into web-platform-tests w3c/csswg-test#1102

Closed

7 tasks

gsnedders force-pushed the css_manifest branch from 3d78f9b to f97da69 Compare November 15, 2016 18:25

gsnedders force-pushed the css_manifest branch 3 times, most recently from c673be5 to ef6d904 Compare November 28, 2016 16:42

gsnedders requested a review from jgraham December 13, 2016 22:43

gsnedders self-assigned this Dec 13, 2016

gsnedders added 11 commits December 14, 2016 16:19

Mark manual CSS WG-style tests as being manual tests

321e622

Add initial support for visual tests

c27a0b1

Add support for legacy CSS visual tests

e1e4b3a

Add a few paths to the blacklist from csswg-test

1a1f791

Update name_is_reference to match everything from csswg-test

11f2e85

Add our own XML parser which handles XHTML entities

38c44f5

Use our XMLParser for XML files

5feefc1

Add a really ugly hack to support more encodings for XML

5bbb21d

Silence ElementTree FutureWarnings

78455bb

Add lint for CSS style tests

a5c34e8

Update name_is_reference to match everything from web-platform-tests

8b6d2a6

gsnedders force-pushed the css_manifest branch from b580e62 to 8b6d2a6 Compare December 14, 2016 16:20

Ms2ger approved these changes Dec 14, 2016

View reviewed changes

gsnedders merged commit b1beaa6 into w3c:master Dec 14, 2016

gsnedders deleted the css_manifest branch December 14, 2016 16:44

gsnedders mentioned this pull request Dec 14, 2016

Fix the test runner #144

Closed

jgraham pushed a commit that referenced this pull request Apr 5, 2017

Respect multiple headers with the same name in .headers files. (#95)

f58dace

* Respect multiple headers with the same name in .headers files. Fixes #90.

wpt-issue-mover mentioned this pull request Apr 7, 2017

[meta] Merge repo into web-platform-tests web-platform-tests/wpt#5486

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for CSS WG-style manifests #90

Add support for CSS WG-style manifests #90

gsnedders commented Jul 27, 2016 •

edited

Loading

Ms2ger Jul 28, 2016

codecov-io commented Aug 19, 2016 •

edited

Loading

gsnedders commented Aug 21, 2016

Ms2ger Aug 22, 2016

gsnedders Aug 22, 2016

Ms2ger Sep 28, 2016

gsnedders commented Aug 24, 2016

gsnedders commented Aug 24, 2016

gsnedders commented Aug 31, 2016

gsnedders commented Sep 13, 2016 •

edited

Loading

gsnedders commented Sep 14, 2016

Ms2ger left a comment

Ms2ger Sep 28, 2016

gsnedders Sep 28, 2016

Ms2ger Sep 28, 2016

gsnedders commented Nov 24, 2016

gsnedders commented Nov 28, 2016

jgraham commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 13, 2016

Ms2ger commented Dec 14, 2016

Ms2ger commented Dec 14, 2016

Add support for CSS WG-style manifests #90

Add support for CSS WG-style manifests #90

Conversation

gsnedders commented Jul 27, 2016 • edited Loading

Ms2ger Jul 28, 2016

Choose a reason for hiding this comment

codecov-io commented Aug 19, 2016 • edited Loading

Current coverage is 83.29% (diff: 90.30%)

gsnedders commented Aug 21, 2016

Ms2ger Aug 22, 2016

Choose a reason for hiding this comment

gsnedders Aug 22, 2016

Choose a reason for hiding this comment

Ms2ger Sep 28, 2016

Choose a reason for hiding this comment

gsnedders commented Aug 24, 2016

gsnedders commented Aug 24, 2016

gsnedders commented Aug 31, 2016

gsnedders commented Sep 13, 2016 • edited Loading

gsnedders commented Sep 14, 2016

Ms2ger left a comment

Choose a reason for hiding this comment

Ms2ger Sep 28, 2016

Choose a reason for hiding this comment

gsnedders Sep 28, 2016

Choose a reason for hiding this comment

Ms2ger Sep 28, 2016

Choose a reason for hiding this comment

gsnedders commented Nov 24, 2016

gsnedders commented Nov 28, 2016

jgraham commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 2, 2016

gsnedders commented Dec 13, 2016

Ms2ger commented Dec 14, 2016

Ms2ger commented Dec 14, 2016

gsnedders commented Jul 27, 2016 •

edited

Loading

codecov-io commented Aug 19, 2016 •

edited

Loading

gsnedders commented Sep 13, 2016 •

edited

Loading