w3c / webdriver-bidi

Bidirectional WebDriver protocol for browser automation
https://w3c.github.io/webdriver-bidi/
336 stars 35 forks source link

[WPT] use `Undefined` to signal missing param #642

Open sadym-chromium opened 5 months ago

sadym-chromium commented 5 months ago

Moved from https://github.com/web-platform-tests/wpt/issues/44068

The current WebDriver BiDi WPT tests use None to represent two distinct cases for parameters:

This overloaded use of None can cause confusion and hinder testing scenarios where we need to pass an explicit null value.

Example:

Proposal:

Advantages:

OrKoN commented 5 months ago

I think WPT was the right place for the issue. I am in favor of using Undefined and None as used in browsingContext.setViewport. Alternatively, we could also add a Null type to be used in tests to reflect the CDDL type.

sadym-chromium commented 5 months ago

I think WPT was the right place for the issue.

According to @jgraham the issue better suits this repo.

whimboo commented 5 months ago

Hm, but what should we define in the BiDi spec? Any code change has to be made in wpt anyway. And Undefined in the spec is represented by the optional (?) field. Or what do I miss?

OrKoN commented 5 months ago

I don't think there is an action point for the spec as well.

sadym-chromium commented 5 months ago

The proposal is only about WPT tests

jgraham commented 5 months ago

I think this is the right discussion venue because it's a WG decision about test style, not a test infra decision. Not all issues here have to result in spec changes, but we might, for example, decide to document the agreed best practice in a readme.

whimboo commented 5 months ago

Beside the discussion where the actual discussion should happen I would also vote for a constant style across all of our tests.

css-meeting-bot commented 4 months ago

The Browser Testing and Tools Working Group just discussed WPT use Undefined to signal missing param.

The full IRC log of that discussion <AutomatedTester> Topic: WPT use Undefined to signal missing param
<AutomatedTester> github: https://github.com/w3c/webdriver-bidi/issues/642
<AutomatedTester> sadym (IRC): This is a technical issue with wpt. Currently we use null for sending things over the wire. This is not always correct.
<orkon> q+
<AutomatedTester> ... sometimes we use the special undefined when we want to test [missed]
<whimboo> q+
<orkon> q-
<whimboo> Python can actually not handle null and undefined with None
<AutomatedTester> ... my suggest is that we move to using a unified way to show undefined when new tests are written when fields. shouldnt be sent
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> whimboo: I agree we need somet
<AutomatedTester> ...thing better here. I mentioned this before.
<AutomatedTester> ... my initial proposal is we use null instead of undefined we make something that makes reading the spec and the tests the same
<AutomatedTester> sadym (IRC): I was played around with something that was `optional`
<AutomatedTester> whimboo: would this work as optional equal to none?
<AutomatedTester> ... as none and null are not equal
<AutomatedTester> sadym (IRC): that would work too
<AutomatedTester> ... the only issue is that none is serialised to null in JS
<AutomatedTester> whimboo: let's see if we can play with these in a test and see what works out best
<AutomatedTester> q?