Support testdriver.js in reftests

[gsnedders]

Previously, when testdriver.js we decided to punt on supporting it in reftests; we now have an actual use for it (#9346), so this needs done.

This will need changes in all the executors: the Python code needs to handle various messages to interact with the document before eventually taking a screenshot, and the JavaScript callbacks needs to deal with passing the testdriver.js messages. The right way to do this is probably by way of comparison of both bits of reftest code with the test harness code.

[gsnedders]

Myself and @jgraham have separately on various occasions remembered that this is going to be somewhat awkward for Firefox the internal Marionette reftest runner, given we'll need to reimplement the testdriver.js implementation within Marionette for that.

Perhaps the right solution here is to just put some "testdriver" flag in the manifest and then use the external reftest runner for those tests.

[jgraham]

Yeah, that seems acceptable for a v1 implemenation; we would probably want to fix this to work inside the harness later, but that could be a seperate project.

[gsnedders]

I'm concerned about the ongoing maintenance cost of having yet another implementation, especially one that wptrunner relies on (and which exists in an external repo). I'm not convinced the benefits of having the internal reftest runner support testdriver.js are actually big enough to outweigh that cost.

[jgraham]

It's probably a bad idea but we definitely could put the code in the wpt repo and ship it over to marionette in the reftest:setup.

[foolip]

@gsnedders any chance of getting this fixed before the end of the quarter?

[gsnedders]

Is there any reason to believe this is so high priority? Does it affect more tests than other untestable issues?

[foolip]

@Hexcles having dug into test writing patterns in Chromium a bit, do you have a hunch for how often this would be useful? I'm inclined to agree with @gsnedders that maybe this isn't very high prio, but don't know.

[foolip]

I've downgraded the priority. We'll probably want this to convert manual tests in the future, but at this point the request for it isn't coming from outside, it's just those of us working on wpt infra that think it's a good idea.

[Hexcles]

My research only looked at existing tests / test history. And of course, there is zero testdriver usage in reftests as it doesn't work, but I don't know how many people attempted to do that.

[gsnedders]

I think there was some discussion at TPAC suggesting this was needed for a whole bunch of tests, but I think it's pretty low priority still.

[stephenmcgruer]

So there is now at least one case of testdriver.js being used in reftests, in css/selectors/remove-hovered-element.html - which a Chromium engineer asked about today as it times out in all browsers in wpt.fyi.

Doing a quick grep, there are half a dozen cases today in the tree (some of which pass!):

$ grep -lr rel.*match  | xargs grep -lr testdriver.js 
css/css-grid/grid-model/grid-layout-stale-001.html
css/css-grid/grid-model/grid-layout-stale-002.html
css/css-shadow-parts/interaction-with-nested-pseudo-class.html
css/selectors/remove-hovered-element.html
pointerevents/pointerevent_releasepointercapture_invalid_pointerid.html
[tools/ entries]
docs/writing-tests/test-templates.md

[stephenmcgruer]

as it times out in all browsers in wpt.fyi.

Note that with the current testrunner implementation in Chromium's CI (which is not wptrunner), this test passes in Chromium's CI, which is why the engineer was surprised to discover the all-timeout status.

[josepharhar]

I am adding some new tests which need this fixed here: https://github.com/web-platform-tests/wpt/pull/34169

[foolip]

I forgot about this issue and wrote tests that don't work: https://github.com/web-platform-tests/wpt/pull/43445

[foolip]

Using git grep -l /resources/testdriver.js | xargs grep -lE 'rel.*match' I found these 10 tests that are currently timing out because this doesn't work:

https://wpt.fyi/results/css/css-shadow-parts/interaction-with-nested-pseudo-class.html https://wpt.fyi/results/css/css-text-decor/invalidation/text-decoration-thickness.html https://wpt.fyi/results/css/css-view-transitions/hit-test-unpainted-element.html https://wpt.fyi/results/css/css-view-transitions/hit-test-unrelated-element.html https://wpt.fyi/results/css/selectors/remove-hovered-element.html https://wpt.fyi/results/fullscreen/rendering/backdrop-iframe.html https://wpt.fyi/results/fullscreen/rendering/backdrop-inherit.html https://wpt.fyi/results/fullscreen/rendering/backdrop-object.html https://wpt.fyi/results/fullscreen/rendering/fullscreen-root-fills-page.html https://wpt.fyi/results/html/semantics/forms/the-selectlist-element/selectlist-option-arbitrary-content-displayed.tentative.html

I also found these two that pass because there's no class="reftest-wait", so the tests probably don't have the intended coverage:

https://wpt.fyi/results/css/css-grid/grid-model/grid-layout-stale-001.html https://wpt.fyi/results/css/css-grid/grid-model/grid-layout-stale-002.html

I fixed two tests that were a bit confused in https://github.com/web-platform-tests/wpt/pull/43937 and https://github.com/web-platform-tests/wpt/pull/43938.

[foolip]

We also have class="test-wait" for crash tests, and I've found one test like that which is also timing out:

https://wpt.fyi/results/accessibility/crashtests/svg-mouse-listener.html

[josepharhar]

I'm working on a bunch of reftests which need testdriver for user activation here: https://github.com/web-platform-tests/wpt/tree/master/html/semantics/forms/the-select-element/stylable-select

[WeizhongX]

I realized we should put such tests in wpt_internal until this issue is fixed. @josepharhar WDYT?

[josepharhar]

I realized we should put such tests in wpt_internal until this issue is fixed. @josepharhar WDYT?

That sounds fine to me

[mfreed7]

I've downgraded the priority. We'll probably want this to convert manual tests in the future, but at this point the request for it isn't coming from outside, it's just those of us working on wpt infra that think it's a good idea.

I'd like to revisit the priority of this issue. While it perhaps didn't have many users before, there is one key new feature that requires this functionality: the customizable-<select> project, whose primary use case is improving stylability and interoperability of the picker for <select>. Since we can't open the picker without testdriver functionality (e.g. test_driver.bless()), we can't effectively test the appearance of the picker via WPT.

Are there any updates, or potential solutions/workarounds to get this to work? I'm happy to use a "hack" that lets me open pickers and use a regular reference test, if such a thing exists.

[WeizhongX]

We can take a look at this in next quarter.

[jgraham]

FWIW the "hack" solution here would be to expose element screenshots via testdriver and then you could make a custom implementation of reftests in testharness tests.

[schenney-chromium]

There are several other things that require this for testing:

• spelling/grammar/etc markers (anything normally added by the user agent based on user action).
• Editing caret rendering
• The aforementioned things needing user activation

Regarding the solution of exposing screen shots in testdriver, that solution is analogous to canvas tests that extract the canvas contents to a bitmap and then check pixels. It works well if you want to know that something happened, or if you want to check a few specific pixels, but it's hard to check something like whether the correct content appears inside a popup.

[past]

After #48486 is there anything else to be done for this issue?

[gsnedders]

Not that this RFC issue should be open for, certainly; https://github.com/web-platform-tests/rfcs/pull/211 resolved what this was tracking.