Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC 211: Support testdriver.js in other test types #211

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonathan-j-lee
Copy link
Contributor

@jonathan-j-lee jonathan-j-lee commented Oct 8, 2024

@jcscottiii jcscottiii changed the title RFC 209: Support testdriver.js in other test types RFC 211: Support testdriver.js in other test types Oct 21, 2024
Copy link
Contributor

@WeizhongX WeizhongX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chrishtr
Copy link

chrishtr commented Nov 7, 2024

@jgraham could you review? We need this in order to test customizable select menus, among other things. Thanks!

@past
Copy link
Member

past commented Nov 7, 2024

Here are the relevant meeting notes from this week's discussion.

@jgraham
Copy link
Contributor

jgraham commented Nov 11, 2024

So, in principle I support the idea of this. In practice it's really hard for Gecko to implement at the moment, because we have a specific internal endpoint for running reftests, and although it's not impossible to imagine a mechanism to allow it to be interrupted, it feels likely to be challenging. In theory we could fall back to the more standard reftest implementation only for these reftests, allowing an implementation much more similar to the WebDriver one, although that might represent a surprising amount of behaviour change.

Also in the meeting @gsnedders brought up some concerns that even with this support some of the use cases listed might be challenging to support cross-browser. In particular, testing <select> might end up in poorly-defined territory; whether the popup window should be accessible to WebDriver seems unclear, although maybe the customizable select work itself makes that clearer.

@WeizhongX
Copy link
Contributor

So, in principle I support the idea of this. In practice it's really hard for Gecko to implement at the moment, because we have a specific internal endpoint for running reftests,

I thought with this change, reftests do not use testdriver should continue to work as usual?

Jonathan has a draft PR here: web-platform-tests/wpt#48486.

Also in the meeting @gsnedders brought up some concerns that even with this support some of the use cases listed might be challenging to support cross-browser. In particular, testing <select> might end up in poorly-defined territory; whether the popup window should be accessible to WebDriver seems unclear, although maybe the customizable select work itself makes that clearer.

Maybe in this RFC our goal is to enable testdriver for reftests, crash tests etc? Other issues should be addressed separately. Popup window is an issue even for now, right?

@mfreed7
Copy link

mfreed7 commented Nov 11, 2024

Also in the meeting @gsnedders brought up some concerns that even with this support some of the use cases listed might be challenging to support cross-browser. In particular, testing <select> might end up in poorly-defined territory; whether the popup window should be accessible to WebDriver seems unclear, although maybe the customizable select work itself makes that clearer.

Just to clarify, there are two "modes" that <select> can use for popups. One is the old/existing one, where (in all mainstream browsers) the popup window is actually a separate OS window. I agree that making this interoperably-testable will be tough. However, the other mode is the new "customizable-<select>" mode, where select, ::picker(select) {appearance: base-select} is used. In that case, the picker is defined to be a popover, so it renders right in the same renderer. There are no separate popup windows. That mode is the one for which we badly need this capability. And hopefully it doesn't present any issues, right?

@jonathan-j-lee
Copy link
Contributor Author

Edited the "Risks" section a bit to capture the practical implementation challenges discussed. PTAL @jgraham @gsnedders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants