Closed Bug 1089536 Opened 10 years ago Closed 9 years ago

Allow multiple test variable files to be specified on the command line

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
normal

Tracking

(b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
mozilla36
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: davehunt, Assigned: davehunt)

References

Details

Attachments

(3 files, 2 obsolete files)

At the moment we have duplicate data across our test variable files. There was talk recently of adding Marketplace credentials to these, so I think it makes sense to allow multiple files to be specified. As the test variable dictionary could contain nested dictionaries we would need to perform a deep update in order to merge the files.
Attachment #8511864 - Flags: review?(mdas)
Comment on attachment 8511864 [details] [diff] [review]
Allow multiple testvars files. v1.0

Review of attachment 8511864 [details] [diff] [review]:
-----------------------------------------------------------------

Something isn't quite right with the update function. If I have one file, testvars.json, containing:

{"asdf": 3}

and another, testvars2.json, containing:

{"asdf": {"ee":3}, "asdfe":3}

Then if I run with --testvars=testvars.json --testvars=testvars2.json, I get the following error:

  File "runtests.py", line 40, in <module>
    cli()
  File "runtests.py", line 35, in cli
    runner = startTestRunner(runner_class, options, tests)
  File "runtests.py", line 19, in startTestRunner
    runner = runner_class(**vars(options))
  File "runtests.py", line 14, in __init__
    BaseMarionetteTestRunner.__init__(self, **kwargs)
  File "/Users/mdas/Code/m-c/testing/marionette/client/marionette/runner/base.py", line 527, in __init__
    self.testvars = update(self.testvars, json.loads(f.read()))
  File "/Users/mdas/Code/m-c/testing/marionette/client/marionette/runner/base.py", line 516, in update
    d[k] = update(d.get(k, {}), v)
  File "/Users/mdas/Code/m-c/testing/marionette/client/marionette/runner/base.py", line 518, in update
    d[k] = u[k]
TypeError: 'int' object does not support item assignment
Attachment #8511864 - Flags: review?(mdas) → review-
This was because an integer value was attempted to be updated by a dictionary. I've updated the patch so that if the original value is not a dictionary but the new value is, the new value takes precedent.
Attachment #8511864 - Attachment is obsolete: true
Attachment #8517420 - Flags: review?(mdas)
Comment on attachment 8517420 [details] [diff] [review]
Allow multiple testvars files. v1.1

Review of attachment 8517420 [details] [diff] [review]:
-----------------------------------------------------------------

works great, thanks
Attachment #8517420 - Flags: review?(mdas) → review+
(In reply to Carsten Book [:Tomcat] from comment #6)
> backed out for test failures like
> https://treeherder.mozilla.org/ui/logviewer.
> html#?job_id=3903248&repo=mozilla-inbound

Sorry about that, I realised immediately after pushing that I didn't run on Try first, so I was watching the push on Treeherder. Thanks for backing it out Tomcat, I'm running a new patch on Try now.
This version of the patch allows testvars to be None. Try:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9da9adeb47e1
Attachment #8517420 - Attachment is obsolete: true
Attachment #8522987 - Flags: review?(mdas)
Comment on attachment 8522987 [details] [diff] [review]
Allow multiple testvars files. v1.2

Review of attachment 8522987 [details] [diff] [review]:
-----------------------------------------------------------------

Heh whoops, I should have tested that case as well :/ Thanks for the fix!
Attachment #8522987 - Flags: review?(mdas) → review+
https://hg.mozilla.org/mozilla-central/rev/2e541f48b3c8
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
In order to land bug 1109005 we need to uplift this to v2.0 and v2.1.
Blocks: 1109005
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Malini: Do you know if there's anything I need to do to get this approved for the release branches? I've not done an uplift for quite a while...
Attachment #8535650 - Flags: review?(mdas)
Comment on attachment 8535650 [details] [diff] [review]
Uplift for mozilla-b2g32_v2_0

Review of attachment 8535650 [details] [diff] [review]:
-----------------------------------------------------------------

ah sorry, I was pto that day. You can just land as a=test-only, you don't need additional r?s so long as the patch doesn't have to be heavily modified to land.
Attachment #8535650 - Flags: review?(mdas) → review+
Attachment #8535674 - Flags: review?(mdas) → review+
Landed as:
mozilla-b2g34_v2_1: https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/e4139bd6f0ec
mozilla-b2g32_v2_0: https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/72c9cb4628b3
Status: REOPENED → RESOLVED
Closed: 10 years ago9 years ago
Resolution: --- → FIXED
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.