-
Notifications
You must be signed in to change notification settings - Fork 78
Tentative testdriver methods #226
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
base: main
Are you sure you want to change the base?
Conversation
| To answer these questions, it would be helpful if browser vendors could collaborate on a tentative but working implementation. | ||
|
|
||
| ## Details | ||
| 1. There must be publicly documented (e.g. as part of an Interop Investigation Area) consensus between at least two browser vendors and intent to work towards a specification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the tests for these tentative methods should be marked with tentative. But I think we should treat these tentative methods similarly to how we treat tentative tests for the sake of consistency. Given tentative tests do not require consensus currently, I would remove this requirement from tentative methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue that tentative testdriver API pose a greater risk, because removing tests that rely on undocumented API is painful and hard — see #172.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both the tests and the method name will be clearly marked as tentative until in the spec with two implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gsnedders
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of my comments below are really "let's make this more explicit", rather than any real objection.
And mostly a question for the Core Team, but I do wonder what standard we thing we're holding things to based on #127: is an explainer enough for a testdriver API, or do we want a spec? Because if an explainer is enough, I'd question how often we'd actually add tentative API, because we probably do want something defining what the behaviour of the endpoint is, even if only at a high-level.
rfcs/tentative_testdriver_methods.md
Outdated
| # RFC 226: Tentative testdriver methods | ||
|
|
||
| ## Summary | ||
| Allow tentative methods to be added to testdriver.js that are not yet included in the WebDriver or WebDriver BiDi specifications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. I had assumed that the existence of a WebDriver endpoint implied it needed to be in the WebDriver spec. So how is WebDriver endpoint defined, given that Gecko uses Marionette for WPT and no other vendors implement this WebDriver endpoint yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, https://webbluetoothcg.github.io/web-bluetooth/#automated-testing defines a whole load of endpoints.
The policy is that it requires a spec for the endpoint, not that that spec be in any specific standard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Now I better understand your original question regarding whether an explainer is sufficient. I agree that at least having an explainer makes sense, so if the consensus is that an explainer is acceptable, we probably don't need this RFC. RFC 127 just says "an existing WebDriver endpoint", which isn't explicit about a formal spec, though I guess that could be inferred. So perhaps we still need an RFC making it clear that an explainer is sufficient.
That said, one difference between a spec and an explainer is that an explainer is far more likely to change than a spec. So that might still be one reason to mark these methods as tentative: to indicate that the details are still being ironed out.
| As part of the [Interop 2025 Accessibility Investigation Area](https://github.com/web-platform-tests/interop-accessibility/issues/148), the intention is to extend browsers and WPT to allow additional accessibility properties to be tested by web platform tests. | ||
| However, there are open questions regarding the shape of the API for exposing these properties, what properties should be exposed, how the tests should be written, etc. | ||
| While the end goal is to extend the WebDriver specification with the required new endpoints, these open questions need to be answered before this is feasible. | ||
| To answer these questions, it would be helpful if browser vendors could collaborate on a tentative but working implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For things like the Accessibility Investigation, is there any intention for there to be any, even high-level, explainer of what the proposed endpoints are? I'm mostly concerned about the risk of ending up somewhere where it is hard for anyone else to start passing the tests written using the tentative endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is WICG/aom#203. However, that's an evolving issue, not a document. I could create an explainer document if that's what we need, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Goal is to have:
- the original problem description and original proposal explainer
- this RFC
- two implementations (nearly there in Gecko and WebKit)
- tentative tests in WPT WebDriver and WPT ARIA
- and a TBD WebDriver spec update
I don't think we need another explainer... We just need to work out the remaining differences and make sure they are clearly documented in the relevant locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One open question here is whether we need this RFC at all or whether an explainer is sufficient. See #226 (comment). If an explainer is sufficient, is it sufficient for that explainer to be a GitHub issue (as is the case now) or does it need to be a document (to make it easier to follow)?
| To answer these questions, it would be helpful if browser vendors could collaborate on a tentative but working implementation. | ||
|
|
||
| ## Details | ||
| 1. There must be publicly documented (e.g. as part of an Interop Investigation Area) consensus between at least two browser vendors and intent to work towards a specification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue that tentative testdriver API pose a greater risk, because removing tests that rely on undocumented API is painful and hard — see #172.
Ms2ger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no fundamental concerns with this, assuming we'd use this for experimentation, not things that stick around indefinitely.
…cification, not just the WebDriver specifications.
jcscottiii
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed this today during the RFC meeting. I'm still opposed to the wording in Detail 1. But the rest of the RFC looks fine. And in the absence of better alternatives for that one point, I will approve this.
|
@gsnedders Can you take another look at this? |
|
An alternative to having |
cookiecrook
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+ with the addition of the new assertion expectation in tentative methods.
I think it's okay to dismiss @gsnedders review since it's 5 months old, was self-described as not a "real objection", and I believe has fully addressed in the latest iteration. |
Rendered.
CC @zcorpan, @cookiecrook.