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: #34169

[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.