Supporting <link rel=> for finding ref tests

# Simon Fraser (5 days ago)

I'd like to land a patch to support finding test references via <link rel="match/mismatch">:

bugs.webkit.org/show_bug.cgi?id=203784, bugs.webkit.org/show_bug.cgi?id=203784

There has been some discussion about this in the past: lists.webkit.org/pipermail/webkit-dev/2011-November/018470.html, lists.webkit.org/pipermail/webkit-dev/2011-November/018470.html

But I think the benefits outweigh the drawbacks. As that mail states:

Link element approach Pros:

  • Can reuse same ref. file for multiple tests

Still true.

  • Can have multiple ref. files for single test

True but no something that we support, and I haven't see any WPT use this (our importer throws an error if it sees this)

  • Information is self-contained in the test file

Still true

  • We may get away with test suite build step

It certainly simplifies WPT test import. Currently importing some CSS suites (e.g. css-backgrounds) results in broken -expected.html files because copying them breaks references to sub resources.

(It turns out that we can't convert W3C ref tests to use WebKit conventions due to the first two points.)

We're doing this much more now, and the "multiple references" point is moot, so I think we can import WPT tests mostly as-is.

Cons:

  • Requires us modifying each port's DRT to support this format

No, it just requires webkitpy hacking which I've done in the patch.

  • Adding link elements itself may affect tests (all W3C tests are required to have link elements at the moment)

I haven't seen this be an issue.

  • Hard to understand relationship between files. e.g. if we want to figure out which tests use ref.html, we must look at all test files

This is true, but I don't really see it being a problem in practice. What I have seen is us importing CSS 2.1 tests that have foo.html and foo-ref.html, and treating foo-ref.html as a test so generating foo-expected.txt and foo-ref-expected.txt. That seems worse.

  • Other browser vendors (Firefox and Opera) prefer manifest file approach

This is no longer true. "reftest.list" files are deprecated (webkit.org/b/203783, web-platform-tests/wpt#20060, web-platform-tests/wpt#20060).

So now that WPT is heavily invested in <link rel=> I think we should follow suite. It will simplify WPT import, and reduced the number of cloned -expected.html files significantly.

Simon

Contact us to advertise here
# Ryosuke Niwa (5 days ago)

On Fri, Nov 8, 2019 at 2:01 PM Simon Fraser <simon.fraser at apple.com> wrote: >

I'd like to land a patch to support finding test references via <link rel="match/mismatch">: bugs.webkit.org/show_bug.cgi?id=203784

There has been some discussion about this in the past: lists.webkit.org/pipermail/webkit-dev/2011-November/018470.html

But I think the benefits outweigh the drawbacks. As that mail states:

Link element approach Pros:

  • Can reuse same ref. file for multiple tests

Still true.

  • Can have multiple ref. files for single test

True but no something that we support, and I haven't see any WPT use this (our importer throws an error if it sees this)

  • Information is self-contained in the test file

Still true

  • We may get away with test suite build step

It certainly simplifies WPT test import.

Currently importing some CSS suites (e.g. css-backgrounds) results in broken -expected.html files because copying them breaks references to sub resources.

(It turns out that we can't convert W3C ref tests to use WebKit conventions due to the first two points.)

We're doing this much more now, and the "multiple references" point is moot, so I think we can import WPT tests mostly as-is.

Cons:

  • Requires us modifying each port's DRT to support this format

No, it just requires webkitpy hacking which I've done in the patch.

I'm not certain writing a bunch of regular expressions in webkitpy is a reliable mechanism to find expected results. Another issue I found back then was that it significantly slowed run-webkit-tests' startup time because WPT has a workflow to find all tests & their expected results upfront before any tests could run. >

  • Adding link elements itself may affect tests (all W3C tests are required to have link elements at the moment)

I haven't seen this be an issue.

Another issue is that if you were to modify a test which happens to be also used as a reference or a mismatch result (worse) for some other test, then you may not notice that without inspecting every other test in existence.

  • Hard to understand relationship between files. e.g. if we want to figure out which tests use ref.html, we must look at all test files

This is true, but I don't really see it being a problem in practice.

This definitely is an issue. It's possible WPT has improved things but we've definitely had an experience where tests were used as reference for other tests, etc... and having to think about this issue every time I touch test drove me nuts.

What I have seen is us importing CSS 2.1 tests that have foo.html and foo-ref.html, and treating foo-ref.html as a test so generating foo-expected.txt and foo-ref-expected.txt. That seems worse.

Seems like we can treat "-ref" as a special suffix like we already do with support directory and resources directory.

So now that WPT is heavily invested in <link rel=> I think we should follow suite. It will simplify WPT import, and reduced the number of cloned -expected.html files significantly.

I really don't want to deal with tests being used as references for other tests. I'm okay with this approach if we forbid that.

  • R. Niwa
# Simon Fraser (5 days ago)

On Nov 8, 2019, at 2:07 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Fri, Nov 8, 2019 at 2:01 PM Simon Fraser <simon.fraser at apple.com> wrote:

I'd like to land a patch to support finding test references via <link rel="match/mismatch">: bugs.webkit.org/show_bug.cgi?id=203784

There has been some discussion about this in the past: lists.webkit.org/pipermail/webkit-dev/2011-November/018470.html

But I think the benefits outweigh the drawbacks. As that mail states:

Link element approach Pros:

  • Can reuse same ref. file for multiple tests

Still true.

  • Can have multiple ref. files for single test

True but no something that we support, and I haven't see any WPT use this (our importer throws an error if it sees this)

  • Information is self-contained in the test file

Still true

  • We may get away with test suite build step

It certainly simplifies WPT test import.

Currently importing some CSS suites (e.g. css-backgrounds) results in broken -expected.html files because copying them breaks references to sub resources.

(It turns out that we can't convert W3C ref tests to use WebKit conventions due to the first two points.)

We're doing this much more now, and the "multiple references" point is moot, so I think we can import WPT tests mostly as-is.

Cons:

  • Requires us modifying each port's DRT to support this format

No, it just requires webkitpy hacking which I've done in the patch.

I'm not certain writing a bunch of regular expressions in webkitpy is a reliable mechanism to find expected results. Another issue I found back then was that it significantly slowed run-webkit-tests' startup time because WPT has a workflow to find all tests & their expected results upfront before any tests could run.

The patch uses html5lib (via BeautifulSoup), which is exactly what WPT, and our importer use to find the ref tests.

We don't find references up-front; only when running each test. This patch does add some overhead for parsing each test file, which I measured to be about 1-2 sec on a directory which took 30s to run. I think this slight slowdown is worthwhile (we could probably eliminate it with some webkitpy optimizations).

  • Adding link elements itself may affect tests (all W3C tests are required to have link elements at the moment)

I haven't seen this be an issue.

Another issue is that if you were to modify a test which happens to be also used as a reference or a mismatch result (worse) for some other test, then you may not notice that without inspecting every other test in existence.

EWS will tell you. Also, generally a file won't act as both a test and a reference; I don't see us deviating from our current "-expected.html" naming conventions. BTW, you don't have to add a <link> tag; we'll still fall

back to the current search behavior. If you have both, then webkitpy will warn.

  • Hard to understand relationship between files. e.g. if we want to figure out which tests use ref.html, we must look at all test files

This is true, but I don't really see it being a problem in practice.

This definitely is an issue. It's possible WPT has improved things but we've definitely had an experience where tests were used as reference for other tests, etc... and having to think about this issue every time I touch test drove me nuts.

Do you have any examples? If this does happen in WPT, we should discourage it (and can fix it via PRs).

Generally references live in a resources/ or references/ subdirectory, or have "-ref" in the name.

What I have seen is us importing CSS 2.1 tests that have foo.html and foo-ref.html, and treating foo-ref.html as a test so generating foo-expected.txt and foo-ref-expected.txt. That seems worse.

Seems like we can treat "-ref" as a special suffix like we already do with support directory and resources directory.

All that works now.

So now that WPT is heavily invested in <link rel=> I think we should follow suite. It will simplify WPT import, and reduced the number of cloned -expected.html files significantly.

I really don't want to deal with tests being used as references for other tests. I'm okay with this approach if we forbid that.

I'm OK with that (enforced by code review unless we see the need for tooling).

Simon

# Robert Ma (5 days ago)

WPT has recently passed an RFC web-platform-tests/rfcs/blob/master/rfcs/reftest_simplification.md

trying to simplify the reftest graph (although it has not been implemented yet), which eliminates a lot of the complexities and concerns.

# Simon Fraser (5 days ago)

On Nov 8, 2019, at 2:15 PM, Robert Ma <robertma at chromium.org> wrote:

WPT has recently passed an RFC web-platform-tests/rfcs/blob/master/rfcs/reftest_simplification.md trying to simplify the reftest graph (although it has not been implemented yet), which eliminates a lot of the complexities and concerns.

That sounds like a very sensible proposal.

It will take some work in webkitpy to support multiple <link rel=>, but since we don't support that when importing now anyway, I don't see it as a blocker to adding <link rel=> support.

I filed webkit.org/b/204022 webkit.org/b/204022 to implement support for multiple <link rel=>.

Simon

# Ryosuke Niwa (5 days ago)

On Fri, Nov 8, 2019 at 2:15 PM Simon Fraser <simon.fraser at apple.com> wrote: >

On Nov 8, 2019, at 2:07 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Fri, Nov 8, 2019 at 2:01 PM Simon Fraser <simon.fraser at apple.com> wrote: >

I'd like to land a patch to support finding test references via <link rel="match/mismatch">: bugs.webkit.org/show_bug.cgi?id=203784

There has been some discussion about this in the past: lists.webkit.org/pipermail/webkit-dev/2011-November/018470.html

But I think the benefits outweigh the drawbacks. As that mail states:

Link element approach Pros:

  • Can reuse same ref. file for multiple tests

Still true.

  • Can have multiple ref. files for single test

True but no something that we support, and I haven't see any WPT use this (our importer throws an error if it sees this)

  • Information is self-contained in the test file

Still true

  • We may get away with test suite build step

It certainly simplifies WPT test import.

Currently importing some CSS suites (e.g. css-backgrounds) results in broken -expected.html files because copying them breaks references to sub resources.

(It turns out that we can't convert W3C ref tests to use WebKit conventions due to the first two points.)

We're doing this much more now, and the "multiple references" point is moot, so I think we can import WPT tests mostly as-is.

Cons:

  • Requires us modifying each port's DRT to support this format

No, it just requires webkitpy hacking which I've done in the patch.

I'm not certain writing a bunch of regular expressions in webkitpy is a reliable mechanism to find expected results. Another issue I found back then was that it significantly slowed run-webkit-tests' startup time because WPT has a workflow to find all tests & their expected results upfront before any tests could run.

The patch uses html5lib (via BeautifulSoup), which is exactly what WPT, and our importer use to find the ref tests.

We don't find references up-front; only when running each test. This patch does add some overhead for parsing each test file, which I measured to be about 1-2 sec on a directory which took 30s to run. I think this slight slowdown is worthwhile (we could probably eliminate it with some webkitpy optimizations).

Hm... that's ~3% overhead.

  • Adding link elements itself may affect tests (all W3C tests are required to have link elements at the moment)

I haven't seen this be an issue.

Another issue is that if you were to modify a test which happens to be also used as a reference or a mismatch result (worse) for some other test, then you may not notice that without inspecting every other test in existence.

EWS will tell you. Also, generally a file won't act as both a test and a reference; I don't see us deviating from our current "-expected.html" naming conventions. BTW, you don't have to add a <link> tag; we'll still fall back to the current search behavior. If you have both, then webkitpy will warn.

I think we should enforce that in our tooling then.

  • Hard to understand relationship between files. e.g. if we want to figure out which tests use ref.html, we must look at all test files

This is true, but I don't really see it being a problem in practice.

This definitely is an issue. It's possible WPT has improved things but we've definitely had an experience where tests were used as reference for other tests, etc... and having to think about this issue every time I touch test drove me nuts.

Do you have any examples? If this does happen in WPT, we should discourage it (and can fix it via PRs).

Oh yeah, it's discouraged but it still happens. If we're doing this in WebKit, run-webkit-tests should outright refuse to run such tests.

Generally references live in a resources/ or references/ subdirectory, or have "-ref" in the name.

We need to enforce that in tool.

What I have seen is us importing CSS 2.1 tests that have foo.html and foo-ref.html, and treating foo-ref.html as a test so generating foo-expected.txt and foo-ref-expected.txt. That seems worse.

Seems like we can treat "-ref" as a special suffix like we already do with support directory and resources directory.

All that works now.

Cool. Now that I think about it, I may have added that LOL.

So now that WPT is heavily invested in <link rel=> I think we should follow suite. It will simplify WPT import, and reduced the number of cloned -expected.html files significantly.

I really don't want to deal with tests being used as references for other tests. I'm okay with this approach if we forbid that.

I'm OK with that (enforced by code review unless we see the need for tooling).

I think we should make run-webkit-tests not allow tests that use other tests as reference files, or ones that don't follow our file naming convention.

  • R. Niwa
# Frédéric Wang (2 days ago)

On 09/11/2019 04:02, Ryosuke Niwa wrote: >

  • Requires us modifying each port's DRT to support this format

No, it just requires webkitpy hacking which I've done in the patch. I'm not certain writing a bunch of regular expressions in webkitpy is a reliable mechanism to find expected results. Another issue I found back then was that it significantly slowed run-webkit-tests' startup time because WPT has a workflow to find all tests & their expected results upfront before any tests could run. The patch uses html5lib (via BeautifulSoup), which is exactly what WPT, and our importer use to find the ref tests.

We don't find references up-front; only when running each test. This patch does add some overhead for parsing each test file, which I measured to be about 1-2 sec on a directory which took 30s to run. I think this slight slowdown is worthwhile (we could probably eliminate it with some webkitpy optimizations). Hm... that's ~3% overhead.

@Simon: I agree with Ryosuke that 3% sounds big. IIUC you are parsing the HTML file when running each test? I thought that there is a MANIFEST.json file which is supposed to cache that information, why can't we use it?

# Simon Fraser (a day ago)

On Nov 12, 2019, at 4:52 AM, Frédéric Wang <fwang at igalia.com> wrote:

On 09/11/2019 04:02, Ryosuke Niwa wrote:

  • Requires us modifying each port's DRT to support this format

No, it just requires webkitpy hacking which I've done in the patch. I'm not certain writing a bunch of regular expressions in webkitpy is a reliable mechanism to find expected results. Another issue I found back then was that it significantly slowed run-webkit-tests' startup time because WPT has a workflow to find all tests & their expected results upfront before any tests could run. The patch uses html5lib (via BeautifulSoup), which is exactly what WPT, and our importer use to find the ref tests.

We don't find references up-front; only when running each test. This patch does add some overhead for parsing each test file, which I measured to be about 1-2 sec on a directory which took 30s to run. I think this slight slowdown is worthwhile (we could probably eliminate it with some webkitpy optimizations). Hm... that's ~3% overhead.

@Simon: I agree with Ryosuke that 3% sounds big. IIUC you are parsing the HTML file when running each test? I thought that there is a MANIFEST.json file which is supposed to cache that information, why can't we use it?

I don't see any files call MANIFEST.json in the wpt repo.

There are reftest.list files but these are obsolete.

I hope to get that 3% back by other webkitpy perf optimizations (python optimization hints welcome).

Simon

# Frédéric Wang (a day ago)

On 12/11/2019 20:06, Simon Fraser wrote:

On Nov 12, 2019, at 4:52 AM, Frédéric Wang <fwang at igalia.com> wrote:

On 09/11/2019 04:02, Ryosuke Niwa wrote:

  • Requires us modifying each port's DRT to support this format

No, it just requires webkitpy hacking which I've done in the patch. I'm not certain writing a bunch of regular expressions in webkitpy is a reliable mechanism to find expected results. Another issue I found back then was that it significantly slowed run-webkit-tests' startup time because WPT has a workflow to find all tests & their expected results upfront before any tests could run. The patch uses html5lib (via BeautifulSoup), which is exactly what WPT, and our importer use to find the ref tests.

We don't find references up-front; only when running each test. This patch does add some overhead for parsing each test file, which I measured to be about 1-2 sec on a directory which took 30s to run. I think this slight slowdown is worthwhile (we could probably eliminate it with some webkitpy optimizations). Hm... that's ~3% overhead. @Simon: I agree with Ryosuke that 3% sounds big. IIUC you are parsing the HTML file when running each test? I thought that there is a MANIFEST.json file which is supposed to cache that information, why can't we use it? I don't see any files call MANIFEST.json in the wpt repo.

This is a generated file: web-platform-tests/wpt/blob/7c50c216081d6ea3c9afe553ee7b64534020a1b2/tools/runner/update_manifest.py#L12

# Robert Ma (a day ago)

MANIFEST.json mostly stays out of the way / happens under the hood. It is cached as GitHub releases, and ignored by .gitignore. One of its purposes is a cache of the reftest graph.

Once you run any test or explicitly run ./wpt manifest, the manifest will be generated/updated in the root of your WPT checkout.

Want more features?

Request early access to our private beta of readable email premium.