Created attachment 8523339 [details] [diff] [review] devicemanagerADB-launchProcess.patch 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/...
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+
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 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+
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
(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
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
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 → ---
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago → 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.