remotexpcshelltests.py cannot execute xpcshell via SUT agent

RESOLVED FIXED in mozilla13

Status

Testing
XPCShell Harness
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: gbrown, Assigned: gbrown)

Tracking

unspecified
mozilla13
x86
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

remotexpcshelltests.py uses devicemanager.runCmd to execute xpcshell, but runCmd is not implemented in devicemanagerSUT.
OS: Mac OS X → Android
Created attachment 577858 [details] [diff] [review]
wip

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.
Depends on: 695020
Created attachment 596729 [details] [diff] [review]
wip - updated for bitrot

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
Created attachment 596852 [details] [diff] [review]
wip - executes xpcshell successfully on sut and adb

Promising, but lots of testing still to do: How do these changes affect other test suites, etc...
Attachment #596729 - Attachment is obsolete: true
Created attachment 598424 [details] [diff] [review]
wip - executes xpcshell on sut and adb; mochitest-remote and mochitest-robotium work via sut and adb
Attachment #596852 - Attachment is obsolete: true
please check the changes in bug 728928 for devicemanager and process launching.
...actually bug 728298
Created attachment 599255 [details] [diff] [review]
adapted to use shell() introduced in 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
Created attachment 599771 [details] [diff] [review]
simplified patch based on the good work in bug 728298
Attachment #598424 - Attachment is obsolete: true
Attachment #599255 - Attachment is obsolete: true
Depends on: 730153
Created attachment 600229 [details] [diff] [review]
simplified patch based on the good work in bug 728298

: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
Attachment #599771 - Attachment is obsolete: true
Attachment #600225 - Attachment is obsolete: true
Attachment #600229 - Flags: review?(jmaher)
Attachment #600229 - Flags: feedback?(wlachance)
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+
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.
Keywords: checkin-needed
Sorry...I meant bug 730153.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/785345035a3b
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.