Closed
Bug 1099475
Opened 10 years ago
Closed 9 years ago
devicemanagerADB.py - Do not quote uri in launchProcess since it is passed in an argument array in _checkCmd
Categories
(Testing :: Mozbase, defect)
Testing
Mozbase
Tracking
(Not tracked)
RESOLVED
WONTFIX
mozilla37
People
(Reporter: bc, Assigned: bc)
References
()
Details
Attachments
(1 file)
1.18 KB,
patch
|
wlach
:
review+
jmaher
:
feedback+
|
Details | Diff | Splinter Review |
http://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/devicemanagerADB.py#371 if uri != "": acmd.append("-d") acmd.append(''.join(['\'',uri, '\''])); This ends up double quoting the uri since _checkCmd uses Popen and an array of arguments. The arguments are passed down to one of the os.exec family and then to the underlying OS's exec. I don't think the quoting is necessary because exec knows the argument boundaries since they are contained in an array. Note adb_device.py doesn't do the quote thingy. I found this problem trying to run the mochitests using adb instead of sut (adjust to suit your environment): python -u mochitest/runtestsremote.py --run-only-tests=android.json --certificate-path=certs --close-when-done --autorun --console-level=DEBUG --log-tbpl=dummy-mochitest-1-samsung-gs3-3.log --log-tbpl-level=DEBUG --dm_trans=adb --deviceSerial=501a2aa7 --app=org.mozilla.fennec --xre-path=/mozilla/projects/autophone/downloads/firefox --utility-path=/mozilla/projects/autophone/downloads/tests/bin --timeout=4800 --remote-webserver=192.168.1.50 --http-port=54313 --ssl-port=60558 --total-chunks=16 --this-chunk=1 --pidfile=mochitest-samsung-gs3-3.pid --symbols-path=/mozilla/projects/autophone/src/autophone/builds/aHR0cDovL2Z0cC5tb3ppbGxhLm9yZy9wdWIvbW96aWxsYS5vcmcvbW9iaWxlL3RpbmRlcmJveC1idWlsZHMvbW96aWxsYS1pbmJvdW5kLWFuZHJvaWQvMTQxNTkzNTIyNC9mZW5uZWMtMzYuMGExLmVuLVVTLmFuZHJvaWQtYXJtLmFwaw==/symbols The symptom was that the mochitest was loaded using a url of the form 'http://mochi.test:8888/... rather than http://mochi.test.8888/...
Attachment #8523339 -
Flags: review?(wlachance)
Attachment #8523339 -
Flags: feedback?(jmaher)
Comment 1•10 years ago
|
||
Comment on attachment 8523339 [details] [diff] [review] devicemanagerADB-launchProcess.patch Review of attachment 8523339 [details] [diff] [review]: ----------------------------------------------------------------- the code change looks fine. The reason for quoting is when we have & and ? in a url it would cause problems with the shell when we would launch fennec. Assuming this change works for mochitest and reftest via adb, I suspect this is fine. it could be a specific version of android had issues, but at the time it was probably all we were testing on.
Attachment #8523339 -
Flags: feedback?(jmaher) → feedback+
Assignee | ||
Comment 2•10 years ago
|
||
I'll double check reftests though I didn't see a problem with them. I think that is due to the way reftests are run via the extension instead of loading a url.
Comment 3•10 years ago
|
||
Comment on attachment 8523339 [details] [diff] [review] devicemanagerADB-launchProcess.patch Makes sense, I think. I kinda wish we could make this method go away.
Attachment #8523339 -
Flags: review?(wlachance) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb92abb8a0f6 I had tried a try run but could not get android tests to run. I finally gave up and pushed anyway. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4c0312072512
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Bob Clary [:bc:] from comment #4) > https://hg.mozilla.org/integration/mozilla-inbound/rev/fb92abb8a0f6 should have been: https://hg.mozilla.org/integration/mozilla-inbound/rev/4386c2d95db3
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4386c2d95db3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 7•9 years ago
|
||
This caused bug 1119962, breaking adb-based local mochitests for Android. See discussion in 1119962. Backed-out: https://hg.mozilla.org/integration/mozilla-inbound/rev/6485d16e2d79
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•9 years ago
|
Status: REOPENED → RESOLVED
Closed: 10 years ago → 9 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•