RFC 212: Introduce require_webdriver_bidi metadata tag
#212
Conversation
sadym-chromium
changed the title
require_webdriver_bidi metadata tagrequire_webdriver_bidi metadata tag
sadym-chromium
force-pushed
the
sadym/require_webdriver_bidi
branch
from
8770a51
to
9a6e20a
Compare
sadym-chromium
force-pushed
the
sadym/require_webdriver_bidi
branch
from
0f55b8b
to
da8d841
Compare
|
|
||
| ### Potential false-negative due to missing `require_webdriver_bidi` tag | ||
|
|
||
| Some implementations can enable BiDi by default, while others would still need enabling it via `require_webdriver_bidi`. This could lead to false-negative test passes by the tests requiring BiDi but not having the `require_webdriver_bidi` tag. This risk can be mitigated by adding documentation describing the logic behind. |
There was a problem hiding this comment.
The BiDi testdriver APIs could enforce a check that they are only being called by a test with this flag set, perhaps? At least when they're called from a top-level test. I don't think documentation alone will be enough here.
|
|
||
| In this alternative approach, the `require_webdriver_bidi` metadata would be added to the corresponding `.ini` file. However, this approach was ultimately declined because the `.ini` file metadata is primarily concerned with the expectations of the test run rather than the specific API requirements. On the other hand, the `require_webdriver_bidi` metadata is directly related to the test API itself. | ||
|
|
||
| ### Extract testdriver BiDi API into a separate file |
There was a problem hiding this comment.
A different alternative here would be to make testdriver.js have a features query parameter that it could check before enabling the features, and which could be supported in the manifest to extract the required features. So the author would need to write <script src="/resources/testdriver.js?features=bidi"></script>, and that would be used both by the js code to decide whether to enable the feature, and also the manifest code to set the flag on the test. The problem would be cases where you include testdriver in a support file but not in the top-level test.
There was a problem hiding this comment.
To clarify: do you think it worse mentioning as an alternative, or you insist we should implement this way instead?
There was a problem hiding this comment.
I thought of it when reviewing, and I think it's worth discussing here :)
The fact that it doesn't really work for tests that are using testdriver outside the test document seems like a notable downside because you'd still need some mechanism e.g. the meta tag in the test document. But it does naturally fail closed in the sense that if you don't specify the feature you can't use it, which is a notable advantage.
There was a problem hiding this comment.
I don't think think this approach would allow adding or not adding bidi in the test_driver object. So in fact in both cases of testdriver.js?features=bidi and testdriver.js, the bidi will be in the test_driver object. However, it's up to the backend to restrict the bidi calls if "bidi" parameter is not set. Please confirm I understood your proposal correctly, and if so, I think it makes sense and I will re-write the proposal accordingly.
There was a problem hiding this comment.
UPD: actually, testdriver can use document.currentScript.src to get it's query parameters, and enable/disable BiDi based on this.
There was a problem hiding this comment.
There was a problem hiding this comment.
UPD: actually,
testdrivercan usedocument.currentScript.srcto get it's query parameters, and enable/disable BiDi based on this.
Note that this doesn’t work in <script type=module>. Is that a use case testdriver needs/wants to support?
There was a problem hiding this comment.
I started drafting this alternative in the #214
There was a problem hiding this comment.
Note that this doesn’t work in <script type=module>. Is that a use case testdriver needs/wants to support?
I think testdriver.is currently not a module but import.meta.url could be used if it becomes a module.
sadym-chromium
force-pushed
the
sadym/require_webdriver_bidi
branch
from
5b9a8f5
to
2fa805b
Compare
Preview