1485598 - Intermittent /webdriver/tests/new_session/default_values.py | test_no_capabilites | test_desired - assert 500 == 400

Status: NEW

(Remote Protocol :: Marionette, defect, P5)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The test fails because we error out with a SessionNotCreated error instead of InvalidArgument error.

Here what's in the WebDriver spec:

https://w3c.github.io/webdriver/#dfn-capabilities-processing

> If capabilities request is not a JSON object, return error with error code invalid argument. 

Link to the test:

https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/testing/web-platform/tests/webdriver/tests/new_session/default_values.py#20-22

Test output:

> 1535017013618	webdriver::server	DEBUG	-> POST /session {}
> 1535017013621	webdriver::server	DEBUG	<- 500 Internal Server Error {"value":{"error":"session not created","message":"Expected browser binary location, but unable to find binary in default location, no 'moz:firefoxOptions.binary' capability provided, and no binary flag set on the command line","stacktrace":""}}
> FAILED

I think the problem here is our fallback to the legacy new session parameters, which is the route geckodriver user if no `capabilities` object is present in the body of the New Session command.

So I think that as long as we have to support the legacy new session parameters we won't be able to pass this test. The only option would be to raise an invalid argument error if both the desired and required capabilities are missing.

Andreas, I wonder if that could be an option, even when we might break some clients?

Comment 1

[Andreas Tolfsen ❲:ato❳]

(In reply to Henrik Skupin (:whimboo) from comment #0)

> I think the problem here is our fallback to the legacy new
> session parameters, which is the route geckodriver user if no
> `capabilities` object is present in the body of the New Session
> command.

I would prefer for the spec compliant ones to be the fallback, but I
understand that is difficult to achieve for as long as we check for
the presence of a "capabilities" key to go into that mode.

> So I think that as long as we have to support the legacy new
> session parameters we won't be able to pass this test. The only
> option would be to raise an invalid argument error if both the
> desired and required capabilities are missing.

Maybe if we could check that the input is exactly {} (empty object)
we could use the spec compliant parser.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Wait. Maybe I have a logic issue myself... How are capabilities specified?

> {
>     "capabilities": {},
>     "desiredCapabilities": {},
>     "requiredCapabilities": {},
> }

Because right now with Serde (not sure how it was before) I check for "desired" and "required". :(

https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/testing/webdriver/src/command.rs#457
https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/testing/webdriver/src/capabilities.rs#501

Comment 3

[James Graham [:jgraham]]

I'm not sure what info is being requested here, but in general processing legacy capabilities isn't spec conforming so it's not surprising that there are some test failures here. But it seems like it ought to be possible to choose the error we get if neither desired not required is present in the keys.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The test is marked as expected fail for ages. I think that we should check if we should continue processing legacy capabilities or if we can get this now removed.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

It would be good if someone is still using it. Otherwise we could add a warning for now so that people are aware that this mode is going away soon.

Lets add a deprecation warning to the next geckodriver 0.32.0 release.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We actually have a warning implemented for legacy capabilities (desired, required) since ages:
https://searchfox.org/mozilla-central/rev/dbb83041d986f8de513d98099f66fed257565d9e/testing/webdriver/src/command.rs#481

As such I assume we do not have to do more warnings here. Also there is bug 1495476 which is about the removal of legacy capability matching.

Comment 7

[BMO Automation]

Moving bug to Remote Protocol::Marionette component per bug 1815831.

Comment 8

[Intermittent Failures Robot]

1 failures in 3880 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-release: 1

Platform and build breakdown:

• android-em-7-0-x86_64-shippable-lite-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1485598&startday=2023-06-19&endday=2023-06-25&tree=all