When running xpcshell tests, the command line options passed to xpcshell are often complex. On desktop, the script testing/xpcshell/runxpcshelltests.py can be used to generate the xpcshell command line and execute tests. A similar facility is required to run xpcshell tests on Android devices.
(It may be possible to leverage testing/xpcshell/remotexpcshelltests.py, developed for Windows Mobile.)

(In reply to comment #0)
> When running xpcshell tests, the command line options passed to xpcshell are
> often complex. On desktop, the script testing/xpcshell/runxpcshelltests.py
> can be used to generate the xpcshell command line and execute tests. A
> similar facility is required to run xpcshell tests on Android devices.
>
> (It may be possible to leverage testing/xpcshell/remotexpcshelltests.py,
> developed for Windows Mobile.)
Geoff,
It'd be awesome to have your help with updating the remotexpcshelltests.py. You can follow the mechanism that we used in mochitest [1] and reftest[2] in order to easily update it to work with android.
It's been on our list for a while, but we've been helping get the rest of the buildbot automation to a reliable state for mochitest and reftest that xpcshell slipped off our list.
One nice thing about continuing to work with the remoted class structure is that it will make it easy to run the xpcshell tests in the buildbot automation once they are finished.
In general there are a few things that we'll need to change (from windows mobile):
* Process name cannot be hardcoded. In the other harnesses we made it a command line option --app
* File names/paths cannot be depended to be writeable - we use the devicemanager::getDeviceRoot API to get to a writeable location on the phone - if you are copying a profile or tests to the phone, create directories in that location.
* Feed devicemanager paths with forward slashes i.e. foo/bar/baz. The underlying agents (ADB, SUTAgent, etc) are responsible for getting the slashes right on device. (This was something we went back and forth about for windows mobile, so some of the slash swapping code might still be in the remoted xpcshell classes, if so feel free to remove it).
And if you have any other questions or if you need to ask what we were smoking when we first wrote that code, please feel free. You can comment here or find Joel (jmaher) and I (ctalbert) in #mobile or #ateam on IRC.
[1] Mochitest remoting: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsremote.py
[2] Reftest remoting: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/remotereftest.py

I have started updating remotexpcshelltests.py. I tried using the same devicemanager selection found in the mochitest and reftest remoting and tested with devicemanagerADB, running ADB over USB. I found some problems in devicemanagerADB; I can probably work around these but filed bug 669549 anyway.

runxpcshelltests.py normally calls xpcshell "-g <xrePath>", where <xrePath> is typically the path to xpcshell. For the remote case, I think -g is unnecessary if I follow the run_xpctest.py (bug 661282) convention of setting LD_LIBRARY_PATH, GRE_HOME, and specifying --greomni; can anyone confirm?

I think you're right. From my read, the xrepath is replaced by the <objdir> parameter of the runxpcshell script. The script is going to copy all your libraries into one directory, so you won't need xrepath.

Created attachment 547114[details][diff][review]
work in progress patch
With this patch, I can run tests with something like:
cd ~/src/mozilla-central/objdir-droid/netwerk/test
/usr/bin/python2.6 -u ../../../config/pythonpath.py -I../../../build -I../../build -I../../../build/mobile ../../../testing/xpcshell/remotexpcshelltests.py --symbols-path=../../dist/crashreporter-symbols --build-info-json=../../mozinfo.json --dm_trans=adb --test-path=test_simple.js --objdir=~/src/mozilla-central/objdir-droid --apk=~/src/mozilla-central/objdir-droid/dist/fennec-8.0a1.en-US.eabi-arm.apk ../../_tests/xpcshell/netwerk/test/unit ../../_tests/xpcshell/netwerk/test/unit_ipc
The test passes and output is very similar to make SOLO_FILE=test_simple.js -C netwerk/test check-one.
However, other tests fail -- cause(s) still under investigation.

Created attachment 547857[details][diff][review]
work in progress patch
With these changes, I can run most tests successfully. I have commented out half a dozen or so tests from unit/xpcshell.ini and there are an additional 8 tests that fail, but otherwise, it looks good:
INFO | Result summary:
INFO | Passed: 132
INFO | Failed: 8
INFO | Todo: 0

This is great stuff Geoff. Rather than commenting out the tests, you should be able to use 'skip-if os == "android"'. Let's just skip what doesn't run or fails for now, get this checked in and file follow up bugs for what gets skipped.

Created attachment 548331[details][diff][review]
patch for review
I found and fixed some problems in the earlier patch (not all files were being copied, not running in correct directory). I also discovered that the unit_ipc tests were not running, and that they hang if they are run. Now all unit_ipc and a few unit tests are skip-if'd.

(In reply to comment #15)
> This is great stuff Geoff. Rather than commenting out the tests, you should
> be able to use 'skip-if os == "android"'. Let's just skip what doesn't run
> or fails for now, get this checked in and file follow up bugs for what gets
> skipped.
If something is hanging or crashing, you can skip it. If something is just failing, please mark it fails-if = os == "android", so we still run the tests. (Minor difference, but it'd help if we introduced a new crash, say.)

Comment on attachment 548331[details][diff][review]
patch for review
Review of attachment 548331[details][diff][review]:
-----------------------------------------------------------------
I ended up doing this:
mkdir ${xpcshell_tests}
cd ${xpcshell_tests}
wget fennec-8.0a1.multi.eabi-arm.tests.zip
unzip fennec-8.0a1.multi.eabi-arm.tests.zip
mkdir fennec
cd fennec
wget fennec-8.0a1.multi.eabi-arm.apk
unzip fennec-8.0a1.multi.eabi-arm.apk
cd ../xpcshell
python remotexpcshelltests.py --build-info-json=mozinfo.json --dm_trans=adb --apk=${xpcshell_tests}/fennec/fennec-8.0a1.multi.eabi-arm.apk --objdir=${xpcshell_tests} tests/netwerk/test/unit
this was using /data/local/tests and setting packageName='' on an unrooted NexusS.
We should fix a lot of these little things and I will review again.
::: build/mobile/devicemanagerADB.py
@@ +30,5 @@
> + files = self.listFiles("/data/data")
> + if (len(files) == 1):
> + if (files[0].find("Permission denied") != -1):
> + print "NOT running as root"
> + raise Exception("not running as root")
do we want to raise an exception here? Is root required?
::: netwerk/test/unit_ipc/xpcshell.ini
@@ +43,2 @@
> [test_xmlhttprequest_wrap.js]
> +skip-if = os == "android"
instead of skip-if on all of these conditions, we can do it in the master xpcshell.ini for the whole include. Unless you think we will be fixing these tests one at a time in the near future?
::: testing/xpcshell/remotexpcshelltests.py
@@ +89,3 @@
>
> + local = os.path.join(objdir, "dist/bin/xpcshell")
> + self.device.pushFile(local, self.remoteBinDir)
I found that in a packaged environment, we needed 'objdir/bin/xpcshell', so we should remove the hardcoded dist for now and append it to objdir if it exists (os.path.exists(os.path.join(objdir, 'dist'))
@@ +104,5 @@
> + local = os.path.join(objdir, "dist/fennec")
> + for file in os.listdir(local):
> + if (file.endswith(".so")):
> + self.device.pushFile(os.path.join(local, file), self.remoteBinDir)
> +
in a packaged build (unzip fennec.apk), we have the .so files in lib/ and libmozutils.js lives in lib/armeabi-v7a. I am not sure of the reason for putting libmozutils.js in a different directory, but we should put some logic in here to handle an unpacked directory. I think it is safe to assume objdir/fennec/lib/libmoz*.so
@@ +114,1 @@
>
this is great for a single directory of tests, but I think it could be a lengthy setup process for all tests. Maybe we can just move this to a different function and only copy the current directory of tests that we are running?
I am leaning towards a setupUtilities() and a setupTestDir().
@@ +166,5 @@
> + shellArgs += " && GRE_HOME="+self.device.getAppRoot()
> + shellArgs += " && XPCSHELL_TEST_PROFILE_DIR="+self.profileDir
> + shellArgs += " && "+xpcshell+" "
> + shellArgs += " ".join(cmd[1:])
> +
this is not going to work in automation, but the goal of landing this is for developer tests. I will refactor this into various commands (cd, xpcshell) instead of a && command. For sut agent, we do this:
['cd dir', '"LD_LIBRARY_PATH=dir;GRE_HOME=root;..." xpcshell args']. I think it would be fair to ask that we setup a dict of env variables and do something like:
for var in envvars:
shellArgs += " && %s=%s" % (var, envvars[var])
Then I think the rest of the changes needed for automation can be refactored into the runCmd() function.

I have been trying to run the "global" set of xpcshell tests: make xpcshell-tests-remote from $objdir; this is the 1200 or so tests in all the manifests referenced by testing/xpcshell/xpcshell.ini. Currently about 100 tests hang and about 500 tests fail. I notice that many failures are related to missing files. For example: toolkit/components/places/tests/expiration/head_expiration.js references toolkit/components/places/tests/head_common.js, but head_common.js is not copied to device, because toolkit/components/places/tests does not contain any tests.
The current setup strategy is to push every directory containing a test, as determined by the manifest reader...but that is not good enough if tests reference files in other directories. I thought I would try to push the entire $objdir/_tests/xpcshell directory, but this is also problematic since $objdir/_tests/xpcshell/tests is a symbolic link to $objdir/..! (And we have other cases where tests fail if symbolic links are not followed and pushed.)

I found that 'make package-tests' also failed: It went into an infinite loop of copying, again because of the symbolic link from $objdir/_tests/xpcshell/tests to $objdir/..; the symbolic link was being created by my makefile patch (bug 668351), xpcshell-tests-remote target, which was copied from the reftest-remote target:
reftest-remote:
@if test -f ${MOZ_HOST_BIN}/xpcshell && [ "${TEST_DEVICE}" != "" -o "$(DM_TRANS)" = "adb" ]; \
then ln -s $(abspath $(topsrcdir)) _tests/reftest/tests;$(call REMOTE_REFTEST,tests/$(TEST_PATH)); $(CHECK_TEST_ERROR); \
Why do we do this?

I found that adb has a 1024 character limit to its arguments. That is, if len("shell " + args) > 1024, adb complains "service name too long" and does not execute the command. This is somewhat verified here: http://groups.google.com/group/android-developers/browse_thread/thread/6319be6a93f1efde#.
The current patch exceeds this limit for xpcshell tests in deep sub-directories. I will shorten path names where possible to reduce the length of the adb shell command used to launch the xpcshell binary. Also, I will add a warning to remotexpcshelltests.py, to be reported when the command length approaches 1024 characters.

Created attachment 551801[details][diff][review]
updated patch for review
Patch updated based on review, with additional improvements for more successful test runs.
>::: build/mobile/devicemanagerADB.py
>@@ +30,5 @@
>> + files = self.listFiles("/data/data")
>> + if (len(files) == 1):
>> + if (files[0].find("Permission denied") != -1):
>> + print "NOT running as root"
>> + raise Exception("not running as root")
>
>do we want to raise an exception here? Is root required?
Root is not required for remote xpcshell, but this is in devicemanager, so root might be required in some cases. Note that the raise is inside an existing try/except; we just run "adb root" if it looks like we are not already running as root (and if "adb root" fails, we just warn about it and continue anyway).
> instead of skip-if on all of these conditions, we can do it in the master
> xpcshell.ini for the whole include. Unless you think we will be fixing these
> tests one at a time in the near future?
Thanks for the tip - now I have used this technique a few times, whenever there were a large number of tests failing in the same manifest.
> ::: testing/xpcshell/remotexpcshelltests.py
> @@ +89,3 @@
>>
>> + local = os.path.join(objdir, "dist/bin/xpcshell")
>> + self.device.pushFile(local, self.remoteBinDir)
>
> I found that in a packaged environment, we needed 'objdir/bin/xpcshell', so
> we should remove the hardcoded dist for now and append it to objdir if it
> exists (os.path.exists(os.path.join(objdir, 'dist'))
Done. (And similar allowance made for lib.)
@@ +114,1 @@
> > this is great for a single directory of tests, but I think it could be a
> lengthy setup process for all tests. Maybe we can just move this to a
> different function and only copy the current directory of tests that we are
> running?
>
> I am leaning towards a setupUtilities() and a setupTestDir().
Setup is definitely time consuming. I have split out setupUtilities/TestDir, as suggested, but currently, both are called unless the --noSetup argument is passed. Optimization is significantly complicated by the issues raised in comments 21 - 23: If tests reference files in other directories, we really need to push the whole $objdir/_tests/xpcshell directory and operate within the remote version of that, even if just one test is being run.
>@@ +166,5 @@
>> + shellArgs += " && GRE_HOME="+self.device.getAppRoot()
>> + shellArgs += " && XPCSHELL_TEST_PROFILE_DIR="+self.profileDir
>> + shellArgs += " && "+xpcshell+" "
>> + shellArgs += " ".join(cmd[1:])
>> +
>
> this is not going to work in automation, but the goal of landing this is for
> developer tests. I will refactor this into various commands (cd, xpcshell)
> instead of a && command. For sut agent, we do this:
>
> ['cd dir', '"LD_LIBRARY_PATH=dir;GRE_HOME=root;..." xpcshell args']. I think
> it would be fair to ask that we setup a dict of env variables and do something
> like:
> for var in envvars:
> shellArgs += " && %s=%s" % (var, envvars[var])
>
> Then I think the rest of the changes needed for automation can be refactored
> into the runCmd() function.
I have made some minor changes to this but did not want to get bogged down by the potential SUT agent / automation details...probably best if I leave that to the experts!

For the benefit of others trying to run these tests, here are notes about my environment:
- apply both patches for this bug, 668349
- apply patch for bug 668351
- apply follow-up patch for bug 661282
- normal Linux/Android build environment
- normal Android make with objdir .../objdir-droid
- cd .../objdir-droid; make package
- ensure adb is in your $PATH
- ensure supported Android device is connected (I used a Samsung Galaxy S)
- I used a rooted device, but this should not be necessary
- make directory /data/local/tests on the device:
adb shell
$ cd /data/local
$ mkdir tests
- cd .../objdir-droid; make xpcshell-tests-remote
OR make -C netwerk/test xpcshell-tests-remote
OR make SOLO_FILE=test_simple.js -C netwerk/test check-one-remote
Setup takes a long time (over 30 minutes!); progress messages are displayed every few seconds.

Created attachment 552597[details][diff][review]
update patch
Updated from reviewed version with just a couple of tweaks:
- if the --apk argument is not specified, try searching $objdir for a likely APK
- update devicemanagerADB's getAppRoot to allow for org.mozilla.fennec_$USER