945975 - Add mach command for Android xpcshell tests

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Geoff Brown [:gbrown]]

We should have mach commands for running tests for Firefox for Android. Let's start with xpcshell tests.

I want this to be a replacement for the existing "make xpcshell-tests-remote", which runs all of the tests in the xpcshell_android.ini manifest. As well, I want to add back the ability to run a single test, or a directory of tests (accidentally deleted from the make targets -- bug 938911).

Comment 1

[Geoff Brown [:gbrown]]

There is some room for discussion about the command name. 
 - The existing make target is "xpcshell-tests-remote" (note plural "tests").
 - The existing (desktop) mach command is "xpcshell-test" (singular).
 - B2G tests are using "mochitest-remote", "reftest-remote", but no xpcshell support yet (bug 932362).

I assume B2G and Android tests are going to "share" the xxx-remote command names, launching different test harnesses based on configuration...but I have not thought that out entirely.

My prefered command name is "xpcshell-test-remote".

Comment 2

[Geoff Brown [:gbrown]]

Attachment: add mach command for xpcshell-test-remote

This works for me:

$ ./mach xpcshell-test-remote
From _tests: Kept 10922 existing; Added/updated 0; Removed 0 files and 0 directories.
using APK: /home/mozdev/objdirs/droid/dist/fennec-28.0a1.en-US.android-arm.apk
Pushing assets/libmozalloc.so..
Pushing assets/libnss3.so..
Pushing assets/libxul.so..
Pushing assets/libnssckbi.so..
Pushing assets/libfreebl3.so..
Pushing assets/libsoftokn3.so..
Pushing assets/libomxplugin.so..
Pushing assets/libomxplugingb.so..
Pushing assets/libomxplugingb235.so..
Pushing assets/libomxpluginhc.so..
Pushing assets/libomxpluginfroyo.so..
Pushing lib/armeabi-v7a/libplugin-container.so..
Pushing lib/armeabi-v7a/libmozglue.so..
pushing /home/mozdev/objdirs/droid/_tests/xpcshell
 3:38.15 INFO | Running tests sequentially.
...

$ ./mach xpcshell-test-remote --devicemanager sut --ip 192.168.0.70 --no-setup
From _tests: Kept 10922 existing; Added/updated 0; Removed 0 files and 0 directories.
using APK: /home/mozdev/objdirs/droid/dist/fennec-28.0a1.en-US.android-arm.apk
 0:04.52 INFO | Running tests sequentially.
 0:07.92 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_abi.js | test passed (time: 1032.048ms)
 0:13.76 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_bug292789.js | test passed (time: 807.606ms)
 0:21.44 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_bug380398.js | test passed (time: 735.249ms)
 0:27.38 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_bug397073.js | test passed (time: 723.703ms)
 0:33.34 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_bug399707.js | test passed (time: 709.649ms)
 0:39.04 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_bug401153.js | test passed (time: 724.110ms)
 0:46.07 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_bug415367.js | test passed (time: 721.554ms)
 0:52.38 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/chrome/test/unit/test_bug519468.js | test passed (time: 773.512ms)
...

$ ./mach xpcshell-test-remote --no-setup netwerk/test/unit/test_simple.js
From _tests: Kept 10922 existing; Added/updated 0; Removed 0 files and 0 directories.
using APK: /home/mozdev/objdirs/droid/dist/fennec-28.0a1.en-US.android-arm.apk
 0:06.15 INFO | Running tests sequentially.
 0:11.42 TEST-INFO | profile dir is /mnt/sdcard/tests/xpcshell/p
 0:11.52 TEST-INFO | /home/mozdev/objdirs/droid/_tests/xpcshell/netwerk/test/unit/test_simple.js | full command: ['/data/local/xpcb/xpcw', '-r', '/mnt/sdcard/tests/xpcshell/c/httpd.manifest', '--greomni', u'/data/local/xpcb/fennec-28.0a1.en-US.android-arm.apk', '-m', '-n', '-s', '-e', 'const _HTTPD_JS_PATH = "/mnt/sdcard/tests/xpcshell/c/httpd.js";', '-e', 'const _HEAD_JS_PATH = "/mnt/sdcard/tests/xpcshell/head.js";', '-e', 'const _TESTING_MODULES_DIR = "/mnt/sdcard/tests/xpcshell/m";', '-f', '/mnt/sdcard/tests/xpcshell/head.js', '-e', 'const _SERVER_ADDR = "localhost"', '-e', u'const _HEAD_FILES = ["/mnt/sdcard/tests/xpcshell/netwerk/test/unit/head_channels.js", "/mnt/sdcard/tests/xpcshell/netwerk/test/unit/head_cache.js", "/mnt/sdcard/tests/xpcshell/netwerk/test/unit/head_cache2.js"];', '-e', 'const _TAIL_FILES = [];', '-e', u'const _TEST_FILE = ["test_simple.js"];', '-e', '_execute_test(); quit(0);']
 0:11.53 TEST-INFO | /home/mozdev/objdirs/droid/_tests/xpcshell/netwerk/test/unit/test_simple.js | current directory: u'/mnt/sdcard/tests/xpcshell/netwerk/test/unit'
 0:11.53 TEST-INFO | /home/mozdev/objdirs/droid/_tests/xpcshell/netwerk/test/unit/test_simple.js | environment: {'MOZ_CRASHREPORTER': '1', 'XPCSHELL_TEST_TEMP_DIR': '/mnt/sdcard/tests/xpcshell/tmp', 'XPCOM_DEBUG_BREAK': 'stack-and-abort', 'XPCSHELL_MINIDUMP_DIR': '/mnt/sdcard/tests/xpcshell/minidumps', 'GRE_HOME': '/data/data/org.mozilla.fennec_mozdev', 'XPCSHELL_TEST_PROFILE_DIR': '/mnt/sdcard/tests/xpcshell/p', 'MOZ_CRASHREPORTER_NO_REPORT': '1', 'HOME': '/mnt/sdcard/tests/xpcshell/p', 'MOZ_LINKER_CACHE': '/data/local/xpcb', 'LD_LIBRARY_PATH': '/data/local/xpcb', 'TMPDIR': '/mnt/sdcard/tests/xpcshell/tmp', 'NS_TRACE_MALLOC_DISABLE_STACKS': '1'}
 0:17.53 TEST-PASS | /home/mozdev/objdirs/droid/_tests/xpcshell/netwerk/test/unit/test_simple.js | test passed (time: 6004.892ms)
 0:17.53 >>>>>>>
 0:17.53 xpcw: cd /mnt/sdcard/tests/xpcshell/netwerk/test/unit
 0:17.53 xpcw: xpcshell -r /mnt/sdcard/tests/xpcshell/c/httpd.manifest --greomni /data/local/xpcb/fennec-28.0a1.en-US.android-arm.apk -m -n -s -e const _HTTPD_JS_PATH = "/mnt/sdcard/tests/xpcshell/c/httpd.js"; -e const _HEAD_JS_PATH = "/mnt/sdcard/tests/xpcshell/head.js"; -e const _TESTING_MODULES_DIR = "/mnt/sdcard/tests/xpcshell/m"; -f /mnt/sdcard/tests/xpcshell/head.js -e const _SERVER_ADDR = "localhost" -e const _HEAD_FILES = ["/mnt/sdcard/tests/xpcshell/netwerk/test/unit/head_channels.js", "/mnt/sdcard/tests/xpcshell/netwerk/test/unit/head_cache.js", "/mnt/sdcard/tests/xpcshell/netwerk/test/unit/head_cache2.js"]; -e const _TAIL_FILES = []; -e const _TEST_FILE = ["test_simple.js"]; -e _execute_test(); quit(0);
 0:17.53 TEST-INFO | (xpcshell/head.js) | test MAIN run_test pending (1)
 0:17.53 TEST-INFO | (xpcshell/head.js) | test pending (2)
 0:17.53 TEST-INFO | (xpcshell/head.js) | test MAIN run_test finished (2)
 0:17.53 TEST-INFO | (xpcshell/head.js) | running event loop
 0:17.53 TEST-PASS | /mnt/sdcard/tests/xpcshell/netwerk/test/unit/head_channels.js | [ChannelListener.prototype.onStopRequest : 162] 10 == 10
 0:17.53 TEST-PASS | test_simple.js | [checkRequest : 51] "0123456789" == "0123456789"
 0:17.53 TEST-INFO | (xpcshell/head.js) | test finished (1)
 0:17.53 TEST-INFO | (xpcshell/head.js) | exiting test
 0:17.53 TEST-PASS | (xpcshell/head.js) | 2 (+ 0) check(s) passed
 0:17.53 TEST-INFO | (xpcshell/head.js) | 0 check(s) todo
 0:17.53 
 0:17.53 <<<<<<<
 0:18.45 INFO | Result summary:
 0:18.45 INFO | Passed: 1
 0:18.45 INFO | Failed: 0
 0:18.45 INFO | Todo: 0
 0:18.45 INFO | Retried: 0

Comment 3

[(not currently active) Ted Mielczarek]

We could also plausibly just reuse "mach xpcshell-test" here, and have the command do the right thing if your build is for Android. What do you think about that?

Comment 4

[Andrew Halberstadt [:ahal]]

+1 to ted's proposal. I already filed a similar bug for mochitest: bug 938712

The current situation of creating a new command per platform was a result of the blind following the blind, and I think we should nip that practice in the bud now. I'll file a bug to make b2g use xpcshell-test as well.

Geoff, I already started creating some common conditions (http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/base.py#565) you should add one to detect android and use that to funnel control flow to the proper places.

Comment 5

[Andrew Halberstadt [:ahal]]

(In reply to Andrew Halberstadt [:ahal] from comment #4)
> I'll file a bug to make b2g use xpcshell-test as well.

Oh you're right, it doesn't exist yet :p

Comment 6

[Geoff Brown [:gbrown]]

This sounds good -- I'll make that change. 

There are slightly different options for desktop vs Android -- sorting those out is one of the only down-sides to consolidation that I see. 

$ ./mach help xpcshell-test
usage: mach [global arguments] xpcshell-test [command arguments]

Run XPCOM Shell tests.

Global Arguments:
  -v, --verbose         Print verbose output.
  -l FILENAME, --log-file FILENAME
                        Filename to write log data to.
  --log-interval        Prefix log line with interval from last message rather
                        than relative time. Note that this is NOT execution
                        time if there are parallel operations.
  --log-no-times        Do not prefix log lines with times. By default, mach
                        will prefix each output line with the time since
                        command start.

Command Arguments:
  TEST                  Test to run. Can be specified as a single JS file, a
                        directory, or omitted. If omitted, the entire test
                        suite is executed.
  --debugger DEBUGGER   Run xpcshell under the given debugger.
  --debugger-args ARGS  pass the given args to the debugger _before_ the
                        application on the command line
  --debugger-interactive
                        prevents the test harness from redirecting stdout and
                        stderr for interactive debuggers
  --interactive, -i     Open an xpcshell prompt before running tests.
  --keep-going, -k      Continue running tests after a SIGINT is received.
  --sequential          Run the tests sequentially.
  --shuffle, -s         Randomize the execution order of tests.
  --rerun-failures      Reruns failures from last time.


$ ./mach help xpcshell-test-remote
usage: mach [global arguments] xpcshell-test-remote [command arguments]

Run remote XPCOM Shell tests.

Global Arguments:
  -v, --verbose         Print verbose output.
  -l FILENAME, --log-file FILENAME
                        Filename to write log data to.
  --log-interval        Prefix log line with interval from last message rather
                        than relative time. Note that this is NOT execution
                        time if there are parallel operations.
  --log-no-times        Do not prefix log lines with times. By default, mach
                        will prefix each output line with the time since
                        command start.

Command Arguments:
  TEST                  Test to run. Can be specified as a single JS file, a
                        directory, or omitted. If omitted, the entire test
                        suite is executed.
  --devicemanager DEVICEMANAGER
                        Type of devicemanager to use for communication: adb or
                        sut
  --ip IP               IP address of device
  --port PORT           Port of device
  --remote_test_root REMOTE_TEST_ROOT
                        Remote test root such as /mnt/sdcard or /data/local
  --no-setup            Do not copy files to device.
  --local-apk LOCAL_APK
                        Use specified Fennec APK.
  --keep-going, -k      Continue running tests after a SIGINT is received.

Comment 7

[Andrew Halberstadt [:ahal]]

(In reply to Geoff Brown [:gbrown] from comment #6)
> This sounds good -- I'll make that change. 
> 
> There are slightly different options for desktop vs Android -- sorting those
> out is one of the only down-sides to consolidation that I see. 

Yeah, that's a good point. It would be awesome if mach could dynamically set options at run time, but that would be a pretty big change.

On the other side, we could try to clean up our options across the various platforms for something resembling a unified set of options. But this is easier said than done.

Comment 8

[Geoff Brown [:gbrown]]

Attachment: support Android in mach xpcshell-test

This feels a little awkward, but might be sufficient.

$ ./mach help xpcshell-test
usage: mach [global arguments] xpcshell-test [command arguments]

Run XPCOM Shell tests.

Global Arguments:
  -v, --verbose         Print verbose output.
  -l FILENAME, --log-file FILENAME
                        Filename to write log data to.
  --log-interval        Prefix log line with interval from last message rather
                        than relative time. Note that this is NOT execution
                        time if there are parallel operations.
  --log-no-times        Do not prefix log lines with times. By default, mach
                        will prefix each output line with the time since
                        command start.

Command Arguments:
  TEST                  Test to run. Can be specified as a single JS file, a
                        directory, or omitted. If omitted, the entire test
                        suite is executed.
  --debugger DEBUGGER   Run xpcshell under the given debugger.
  --debugger-args ARGS  pass the given args to the debugger _before_ the
                        application on the command line
  --debugger-interactive
                        prevents the test harness from redirecting stdout and
                        stderr for interactive debuggers
  --interactive, -i     Open an xpcshell prompt before running tests.
  --keep-going, -k      Continue running tests after a SIGINT is received.
  --sequential          Run the tests sequentially.
  --shuffle, -s         Randomize the execution order of tests.
  --rerun-failures      Reruns failures from last time.
  --devicemanager DEVICEMANAGER
                        (Android) Type of devicemanager to use for
                        communication: adb or sut
  --ip IP               (Android) IP address of device
  --port PORT           (Android) Port of device
  --remote_test_root REMOTE_TEST_ROOT
                        (Android) Remote test root such as /mnt/sdcard or
                        /data/local
  --no-setup            (Android) Do not copy files to device
  --local-apk LOCAL_APK
                        (Android) Use specified Fennec APK

Comment 9

[Andrew Halberstadt [:ahal]]

Comment on attachment 8342824 [details] [diff] [review]
support Android in mach xpcshell-test

Review of attachment 8342824 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. I think ideally most of the logic in our mach commands should live in the actual harness, but our harnesses are too much of a jumbled mess at the moment :)

::: testing/xpcshell/mach_commands.py
@@ +201,5 @@
>          return int(not result)
>  
> +class AndroidXPCShellRunner(MozbuildObject):
> +    """Run Android xpcshell tests."""
> +    def run_test(self, 

nit: whitepsace

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 8342824 [details] [diff] [review]
support Android in mach xpcshell-test

Review of attachment 8342824 [details] [diff] [review]:
-----------------------------------------------------------------

Overall this looks good, just a few nits and comments.

::: python/mozbuild/mozbuild/base.py
@@ +591,5 @@
> +    @staticmethod
> +    def is_android(cls):
> +        """Must have an Android build."""
> +        if hasattr(cls, 'substs'):
> +            return cls.substs.get('MOZ_WIDGET_TOOLKIT') == 'android'

You could also use OS_TARGET == 'Android', but it probably doesn't matter.

::: testing/xpcshell/mach_commands.py
@@ +214,5 @@
> +
> +        import remotexpcshelltests
> +        from mozdevice import devicemanagerADB, devicemanagerSUT
> +
> +        if (devicemanager == "adb"):

nit: don't need parens (here and elsewhere)

@@ +223,5 @@
> +        else:
> +            if (ip):
> +                dm = devicemanagerSUT.DeviceManagerSUT(ip, port, deviceRoot=remote_test_root)
> +            else:
> +                raise Exception("You must provide a device IP to connect to via the --ip option")

This feels like something we'll want to pull out into a utility method if we support mach commands for other remote harnesses.

@@ +276,5 @@
> +
> +        self.log_manager.terminal_handler.removeFilter(xpcshell_filter)
> +        self.log_manager.disable_unstructured()
> +
> +        return int(not result)

It does suck that our test harness requires this much setup in the mach command, but we can fix that in followup bugs.

Comment 11

[Geoff Brown [:gbrown]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #10)
> nit: don't need parens (here and elsewhere)
> 
> @@ +223,5 @@
> > +        else:
> > +            if (ip):
> > +                dm = devicemanagerSUT.DeviceManagerSUT(ip, port, deviceRoot=remote_test_root)
> > +            else:
> > +                raise Exception("You must provide a device IP to connect to via the --ip option")
> 
> This feels like something we'll want to pull out into a utility method if we
> support mach commands for other remote harnesses.

I pulled this out into a local function for now. I'll have a look at moving it elsewhere for the next remote harness (Android mochitest?).

With nits addressed: https://hg.mozilla.org/integration/mozilla-inbound/rev/00db24c44565

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/00db24c44565