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

Merged

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

Copy link
Member

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

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

I think it's true that the only real risk here is with the internal Marionette runner; otherwise this has broadly seemed like a good idea for a while.

Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

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

I think this is OK; a Gecko implementation that falls back to the WebDriver-style external runner isn't ideal, but it's likely better than not having the feature at all.

rfcs/testdriver_in_other_types.md Outdated Show resolved Hide resolved
@jonathan-j-lee jonathan-j-lee force-pushed the testdriver-in-other-types branch from 7ee02b9 to 5b4c66a Compare December 4, 2024 19:20
@jonathan-j-lee
Copy link
Contributor Author

@web-platform-tests/wpt-core-team Ready to land?

@past past merged commit 952c114 into web-platform-tests:master Dec 10, 2024
@past
Copy link
Member

past commented Dec 10, 2024

Thanks!

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.

7 participants