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.

Can anyone tell me the cause of this error: ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: jar:file:///data/local/tests/xpcshell/bin/fennec-8.0a1.en-US.eabi-arm.apk!/components/DirectoryProvider.js :: <TOP_LEVEL> :: line 60" data: no] ************************************************************ I get this when I run xpcshell on Android with command line: cd /data/local/tests/xpcshell/bin && LD_LIBRARY_PATH=/data/local/tests/xpcshell/bin && GRE_HOME=/data/data/org.mozilla.fennec_unofficial && XPCSHELL_TEST_PROFILE_DIR=/data/local/tests/xpcshell/profile && ./xpcshell -r /data/local/tests/xpcshell/components/httpd.manifest --greomni /data/local/tests/xpcshell/bin/fennec-8.0a1.en-US.eabi-arm.apk -j -s -e 'const _HTTPD_JS_PATH = "/data/local/tests/xpcshell/components/httpd.js";' -e 'const _HEAD_JS_PATH = "/data/local/tests/xpcshell/scripts/head.js";' -f /data/local/tests/xpcshell/scripts/head.js -e 'const _SERVER_ADDR = "localhost"' -e 'const _HEAD_FILES = ["/data/local/tests/xpcshell/scripts/unit/head_channels.js"];' -e 'const _TAIL_FILES = ["/data/local/tests/xpcshell/scripts/unit/"];' -e 'const _TEST_FILE = ["/data/local/tests/xpcshell/scripts/unit/test_307_redirect.js"];' -e '_execute_test(); quit(0);'

That comes from someone trying to make use of the DirectoryProvider (http://mxr.mozilla.org/mozilla-central/source/mobile/components/DirectoryProvider.js) in the content process to get the APP_CACHE_PARENT_DIR, which tries to find out the profile dir and is disallowed in non-chrome processes.

Thanks Josh. Even simpler: js> Components.classes["@mozilla.org/file/directory_service;1"].getService( Components.interfaces.nsIProperties).get("ProfD", Components.interfaces.nsIFile).path uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: typein :: <TOP_LEVEL> :: line 2" data: no] ...and other tests are giving me different errors, all pointing to ProfD not being set. But I have set the XPCSHELL_TEST_PROFILE_DIR environment variable. Looking into it...

testing/xpcshell/head.js, fn do_get_profile sets ProfD from XPCSHELL_TEST_PROFILE_DIR, but do_get_profile is not being called.

I thought on windows mobile we had to hack this into the xpcshell binary during make time if --enable-tests was specified.

(In reply to comment #10) I did have some "build irregularities" and --disable-tests may have been in use for some of the problem tests!

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.

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 shows the output from: 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 ../../_tests/xpcshell/netwerk/test/unit ../../_tests/xpcshell/netwerk/test/unit_ipc --objdir=../.. --apk=../../dist/fennec-8.0a1.en-US.eabi-arm.apk &> runall.log

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.

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.

Log showing output from: cd ~/src/mozilla-central/objdir-droid/netwerk/test /usr/bin/python2.6 -u ../../../config/pythonpath.py -I../../../build -I../../../build/mobile ../../../testing/xpcshell/remotexpcshelltests.py --symbols-path=../../dist/crashreporter-symbols --build-info-json=../../mozinfo.json --dm_trans=adb ../../_tests/xpcshell/netwerk/test/unit ../../_tests/xpcshell/netwerk/test/unit_ipc --objdir=../.. --apk=../../dist/fennec-8.0a1.en-US.eabi-arm.apk Now the overall results are: INFO | Result summary: INFO | Passed: 117 INFO | Failed: 13 INFO | Todo: 0

(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.

Improved with: - copies plugin-container to device - sets CACHE_PATH - exports environment variables - fail-if added to failing tests in xpcshell.ini - fail-if and skip-if commented with bug numbers Network test results now: INFO | Result summary: INFO | Passed: 139 INFO | Failed: 0 INFO | Todo: 4 But I haven't worked through all the review comments yet...

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.)

you could require a 'make package-tests' in order to run these. That would ensure all the symlinks are resolved and related files are all in one directory.

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.

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!

There are many tests that hang and some that fail, on Android. With this patch, "make xpcshell-tests-remote" runs to completion and ends cleanly with: INFO | Result summary: INFO | Passed: 837 INFO | Failed: 0 INFO | Todo: 49

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.

Comment on attachment 551801 [details] [diff] [review] updated patch for review Review of attachment 551801 [details] [diff] [review]: ----------------------------------------------------------------- this looks good. I don't see anything specific in here to stop us from getting this running for developers! Thanks for getting these tests up and going!

http://hg.mozilla.org/integration/mozilla-inbound/rev/40e392d958e0 http://hg.mozilla.org/integration/mozilla-inbound/rev/187ac0d7b154

backed out the pushes because of orange: http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d9456378c12d

One problem was a typo: --- a/ipc/testshell/tests/xpcshell.ini +++ b/ipc/testshell/tests/xpcshell.ini @@ -1,6 +1,8 @@ +# Bug 676963: test fails consistently on Android +fail-if = os = "android" ^ should be ==

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

Oops - the getAppRoot change is handled in bug 678385; I have removed it from this patch.

updated for bitrot only

updated for bitrot only

Comment on attachment 553856 [details] [diff] [review] updated patch r=jmaher

Comment on attachment 553857 [details] [diff] [review] patch adding fail-if / skip-if "android" directives to all the xpcshell manifests r=jmaher

Pushed to try -- looks good to me. http://hg.mozilla.org/try/pushloghtml?changeset=ff5c3f0b0f6a http://tbpl.mozilla.org/?tree=Try&rev=ff5c3f0b0f6a

http://hg.mozilla.org/integration/mozilla-inbound/rev/925a6fb68f91 http://hg.mozilla.org/integration/mozilla-inbound/rev/4512e937ac29

http://hg.mozilla.org/mozilla-central/rev/4512e937ac29 http://hg.mozilla.org/mozilla-central/rev/925a6fb68f91