1136754 - Update test variables to support remote device lab

Status: RESOLVED FIXED

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

Description

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

The remote device lab we're setting up provides a JSON file with slightly different key/values. We need to update our existing test variable files and the tests so that we're ready to run the full suite of tests in parallel.

Comment 1

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

It'd be worth getting a preview of what they're sending before we decide how to use it on our side in the tests and start fielding specific patches. Talking design, at least in terms of test-side concerns, before implementation might be appropriate here.

One of the things mentioned earlier re: a key changing type depending on situation is a little concerning in terms of what that would mean for test code. In cases like that, might be worth having a translation layer that standardizes the value before the tests get it.

Comment 2

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Looking at the code, specifically I'd probably explore wrapping testvars at the gaia ui test level in a "environment" object with properties, rather than coding straight to json schema. That would cover issues like this in the future.

Comment 3

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

Attachment: Example testvars file from remote lab

This is an example device testvars file from our remote lab.

Comment 4

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

Attachment: Link to Github pull-request: https://github.com/mozilla/webqa-credentials/pull/194

Comment 5

[GitHub Autolander Bot]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/28492/files

Comment 6

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8569800 [details] [review]
Link to Github pull-request: https://github.com/mozilla/webqa-credentials/pull/194

Looks great.

Comment 7

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

Attachment: Link to Github pull-request: https://github.com/mozilla/webqa-credentials/pull/195

This will land once the gaiatest patch has landed.

Comment 8

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8569801 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/28492/files

Rolling up our IRC conversation, I'd be more comfortable with GaiaDevice sticking with calls that query the device itself. That way it stays cohesive as a Device Information API, as opposed to reflecting what we -think- the device should be showing.

I'm going to look to see if we have somewhere more appropriate we can put these in current code. If so, I'll update with that. 

If not, then we'll go with this so that we can complete the switchover, but I'll file a follow-on bug to refactor it myself into a GaiaEnv vs. GaiaDevice type split, where GaiaEnv ultimately becomes an abstraction over testvars per comment 2.

Comment 9

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8569801 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/28492/files

I reviewed gaia_test.py more closely, and there's at least precedent for consulting testvars in GaiaDevice in the is_android_build/is_emulator/is_desktop_b2g methods. Also, GaiaData has a bunch of device stuff in it re: bluetooth, etc., so we've already blown cohesion.

The is_ routines do fall back on API queries if the information isn't supplied in testvars, though, which might be a good pattern to follow here as well. However, I'm also fine the way it is.

So r+'ing.

I do think we should talk at some point about the long-term plans for Gaia UI Test, though. If it's going to stay in active development via other suites, it'd be worth refactoring GaiaDevice and GaiaData to strictly device queries and user data queries respectively, and creating GaiaEnvironment to handle testvars and other externally-supplied stuff.

Comment 10

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8569983 [details] [review]
Link to Github pull-request: https://github.com/mozilla/webqa-credentials/pull/195

r+ pending the other changes landing.

Comment 11

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

Comment on attachment 8569801 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/28492/files

I hope you don't mind me switching your review here - I just don't want autolander to merge it. After having some time to think about it I believe you're right, I'll revisit the patch today.

Comment 12

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

Comment on attachment 8569801 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/28492/files

Okay, I've pushed another commit. If it looks good I'll squash before merging.

Comment 13

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8569801 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/28492/files

This looks great! 

I think over time, we can move some of the other testvars stuff into there and get away from writing straight to json schematic dependencies in the tests. 

The opportunity to calculate values and standardize the types should be a big help. It might conceivably even let us support multiple input formats or sources in the future if we want to tackle that.

Thanks for considering this! Enthusiastic r+

Comment 14

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

Thanks, I've triggered a couple of adhocs against our remote lab too. I'll hold off to see how they do before merging.

1 SIM: http://jenkins1.qa.scl3.mozilla.com:8080/view/UI/job/flame-kk.ui.adhoc.bitbar/12
2 SIMs: http://jenkins1.qa.scl3.mozilla.com:8080/view/UI/job/flame-kk.ui.adhoc.bitbar/13

Comment 15

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

Landed in:
https://github.com/mozilla-b2g/gaia/commit/1a99204f23e9b84290f2301be550e848794130a3

Comment 16

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

Reminder to myself to merge the final webqa-credentials patch.

Comment 17

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

Done.