Closed
Bug 1089536
Opened 10 years ago
Closed 10 years ago
Allow multiple test variable files to be specified on the command line
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
Tracking
(b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)
RESOLVED
FIXED
mozilla36
People
(Reporter: davehunt, Assigned: davehunt)
References
Details
Attachments
(3 files, 2 obsolete files)
3.98 KB,
patch
|
mdas
:
review+
|
Details | Diff | Splinter Review |
4.99 KB,
patch
|
mdas
:
review+
|
Details | Diff | Splinter Review |
3.93 KB,
patch
|
mdas
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8511864 -
Flags: review?(mdas)
Comment 2•10 years ago
|
||
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-
Assignee | ||
Comment 3•10 years ago
|
||
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 4•10 years ago
|
||
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+
Assignee | ||
Comment 5•10 years ago
|
||
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/d1962cb07ea5
Comment 6•10 years ago
|
||
backed out for test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3903248&repo=mozilla-inbound
Assignee | ||
Comment 7•10 years ago
|
||
(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.
Assignee | ||
Comment 8•10 years ago
|
||
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 9•10 years ago
|
||
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+
Assignee | ||
Comment 10•10 years ago
|
||
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/2e541f48b3c8
https://hg.mozilla.org/mozilla-central/rev/2e541f48b3c8
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 13•10 years ago
|
||
In order to land bug 1109005 we need to uplift this to v2.0 and v2.1.
Blocks: 1109005
Status: RESOLVED → REOPENED
status-b2g-v2.0:
--- → affected
status-b2g-v2.1:
--- → affected
Resolution: FIXED → ---
Assignee | ||
Comment 14•10 years ago
|
||
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)
Assignee | ||
Comment 15•10 years ago
|
||
Attachment #8535674 -
Flags: review?(mdas)
Comment 16•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #8535674 -
Flags: review?(mdas) → review+
Assignee | ||
Comment 17•10 years ago
|
||
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 ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Comment 18•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/72c9cb4628b3
status-b2g-v2.0M:
--- → fixed
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•