Closed Bug 705192 Opened 8 years ago Closed 8 years ago
.py cannot execute xpcshell via SUT agent
remotexpcshelltests.py uses devicemanager.runCmd to execute xpcshell, but runCmd is not implemented in devicemanagerSUT.
I have changed launchProcess to use the devicemanager's launchProcess, and updated the adb launchProcess significantly. It works fine with adb, but there are still a couple of problems with sut.
I have found several problems trying to launch xpcshell via devicemanagerSUT: - getDeviceRoot uses the SUT testroot command, which returns /mnt/sdcard; xpcshell cannot be executed from sdcard because we cannot chmod +x it in that location; I have updated testroot to return /data/local, if /data/local/tests exists -- just like we did in devicemanagerADB - devicemanagerSUT.launchProcess appends "> <deviceroot>/process.txt" to the command line when no output file is specified. This ends up being passed as arguments to Runtime.exec(), which fails. I expect redirection is a shell responsibility and the > is not interpretted by Runtime.exec(). It's probably best to just remove this redirection -- the agent's RedirOutputThread is going to redirect output anyway. - devicemanagerSUT.launchProcess makes no allowance for its cwd parameter.
And also... - remotexpcshelltests.py surrounds some command line arguments with single quotes: eg -e '_execute_test(); quit();'. The quotes are necessary when invoked via the adb shell, but cause xpcshell to fail silently when invoked via sut. I have moved the quoting code out of remotexpcshelltests.py and into devicemanagerADB.
And also...bug 695020 affects xpcshell significantly -- xpcshell typically completes before processExists() detects it.
Can execute xpcshell via ADB or SUT with this patch. With SUT, there is a little more work required to return / report results.
Attachment #577858 - Attachment is obsolete: true
Promising, but lots of testing still to do: How do these changes affect other test suites, etc...
Attachment #596729 - Attachment is obsolete: true
Very bad try run: https://tbpl.mozilla.org/?tree=Try&rev=a295e9866dcc
Passes try now: https://tbpl.mozilla.org/?tree=Try&rev=7157e6d7824d
please check the changes in bug 728928 for devicemanager and process launching.
...actually bug 728298
It was relatively straight-forward to adapt the patch to use shell(), but I had to add some additional features to shell(): - environment handling - cwd handling - character escapes
:wlach -- note that I had 2 issues with the devicemanager shell work: - on ADB, the manual escaping of characters + list2cmdline results in double quoting of some xpcshell arguments, resulting in failures; I removed list2cmdline - _pop_last_line() fails when the file has a single line; I adjusted the character indexing
Comment on attachment 600229 [details] [diff] [review] simplified patch based on the good work in bug 728298 Looks great, sorry about the bugs in devicemanager. My only suggestion would be to run things through try before pushing.
Attachment #600229 - Flags: feedback?(wlachance) → feedback+
Comment on attachment 600229 [details] [diff] [review] simplified patch based on the good work in bug 728298 Review of attachment 600229 [details] [diff] [review]: ----------------------------------------------------------------- the changes to remoteautomation.py will affect all other unittests, so we need this on try server.
Attachment #600229 - Flags: review?(jmaher) → review+
Try run looks good: https://tbpl.mozilla.org/?tree=Try&rev=9949beffa727
are we ready to check this in? After this lands, what else is left?
Yes - ready for check-in. Bug 730152 is needed to tidy up hung xpcshell's...that's about all that I can think of.
Sorry...I meant bug 730153.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.