1132465 - Parametrize marionette desired capabilities for Gaia UI tests.

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Yura Zenevich [:yzen] (please use "needinfo")]

Similar to bug 1132055 and the already existing implementation of --testvars argument when running the UI tests, it would be nice to have an ability to define

--marionette-capabilities=file.json

when for the ui tests.

Comment 1

[David Burns :automatedtester]

why does this need to be another argument? Test vars was designed to take json of things that are needed during the test. 

Adding another argument to the runner feels like it be another confusing thing that people will forget.

Comment 2

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Having this is testvars will require using a key such as 'marionette-capabilities', which I think could be more likely to cause issues than users forgetting about the additional argument. When discussing on IRC with Yura, I suggested that the marionette capabilities probably deserve to be in their own file. I wouldn't name it '--marionette-capabilites' though as the 'marionette' is superfluous in a Marionette test runner.

Comment 3

[David Burns :automatedtester]

Why does this need to be marionette-capabilities as a key? Looking at http://gaiatest.readthedocs.org/en/latest/testrunner.html#test-variables there isnt a key called capabilities.

Obviously you are welcome to do what you want in Gaiatest but adding another argument to the Marionette runtests is something I want to avoid unless I have no other option.

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to David Burns :automatedtester from comment #3)
> Why does this need to be marionette-capabilities as a key? Looking at
> http://gaiatest.readthedocs.org/en/latest/testrunner.html#test-variables
> there isnt a key called capabilities.

This is really just the 'reserved' keys for gaiatest, as keys are arbitrary. I don't think that Marionette test runner has any 'reserved' keys at the moment, so this will be the first.

> Obviously you are welcome to do what you want in Gaiatest but adding another
> argument to the Marionette runtests is something I want to avoid unless I
> have no other option.

This change needs to affect the Marionette object instantiation, so it can't really go into gaiatest. I think it's a good idea to allow any runner extending the Marionette one to benefit from injection of capabilities.

I spoke with AutomatedTester in IRC and I'm going to go with his judgement on this one (I share his loathing for superfluous command line arguments). Here's my proposal based on my discussion with David:

testvars.json:

{"capabilites": {
  "desired": {"foo": "bar"},
  "required": {"bar": "foo"}
}}

This would currently be morphed in the Marionette test runner to:

capabilities = {"foo": "bar", "requiredCapabilities": {"bar": "foo"}}
marionette.start_session(desired_capabilities=capabilities)

David: Have I understood this correctly? This should allow for the future changes to the dictionary accepted by start_session without the need to change the expectations from the testvars file. I couldn't find anything in the documentation on required capabilities though - is this a bug?

Comment 5

[Yura Zenevich [:yzen] (please use "needinfo")]

A little bit more context from IRC:
[09:31:36]  <yzen>	davehunt so if i understand correctly, I would need to add this both in gaiatest as well as in testing/marionette in gecko, right ?
[09:32:09]  <@davehunt>	you wouldn't need to add it to gaiatest, as that extends the marionette test runner
[09:32:37]  <@davehunt>	it would be a change to the marionette test runner, and a subsequent release that gaiatest can then be updated to use
[09:33:19]  <yzen>	davehunt, i see so start_session that is called in in gaia tests will check for the presence of the testvars.capabilities internally ?
[09:33:38] 	armenzg_brb is now known as armenzg
[09:33:38]  <@davehunt>	yzen: do we call start_session in gaiatest?
[09:33:45]  <yzen>	start_b2g does afaik
[09:34:12]  <yzen>	https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L602
[09:34:14]  <@davehunt>	yzen: ah, I see.. that sucks
[09:36:01]  <@davehunt>	yzen: we'd need to define the capabilities wherever we start a session
[09:36:30]  <@davehunt>	we might be able to add a start_session to BaseMarionetteTestRunner so we can centralise it
[09:37:36]  <@davehunt>	hmm.. I still think we should use a separate command line argument, as BaseMarionetteTestRunner has a capabilitiies kwarg in addition to a testvars one
[09:39:31]  <@davehunt>	yzen: for specifics I recommend asking AutomatedTester, he's more familiar with the capabilities parts than I am

Comment 6

[David Burns :automatedtester]

(In reply to Dave Hunt (:davehunt) from comment #4)
> (In reply to David Burns :automatedtester from comment #3)
> 
> testvars.json:
> 
> {"capabilites": {
>   "desired": {"foo": "bar"},
>   "required": {"bar": "foo"}
> }}

Exactly!


> 
> This would currently be morphed in the Marionette test runner to:
> 
> capabilities = {"foo": "bar", "requiredCapabilities": {"bar": "foo"}}

it actually should be capabilities = {"desiredCapabilities":{"foo": "bar"}, "requiredCapabilities": {"bar": "foo"}}

On the server we remove the desiredCapabities and requiredCapabilities keys during processing and you get a dict back with what we have set and what the browser can do (or an error if we can't meet requiredCapabilities)

> marionette.start_session(desired_capabilities=capabilities)

where capabilities = {"desiredCapabilities":{"foo": "bar"}, "requiredCapabilities": {"bar": "foo"}}

We should change the kwarg to capabilities at some point :)

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

For accessibility check purposes, we are focusing on Gij, not Gip so unassigning this but keeping open as a general feature request.

Comment 8

[BMO Automation]

Firefox OS is not being worked on