859705 - Enabling on-demand decompression on android breaks xpcshell tests

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Mike Hommey [:glandium]]

When enabling on-demand decompression, the result of the unzip done in remotexpcshelltests.py is .so files unsuitable for direct use with xpcshell.

Comment 1

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #0)
> unzip done in remotexpcshelltests.py 

It's done by the tinderbox script, which is even worse.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Un-szip libraries before pushing them on the device for xpcshell tests

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 737369 [details] [diff] [review]
Un-szip libraries before pushing them on the device for xpcshell tests

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

::: testing/testsuite-targets.mk
@@ +456,5 @@
>  stage-android: make-stage-dir
> +ifdef MOZ_ENABLE_SZIP
> +# Tinderbox scripts are not unzipping everything, so the file needs to be in a directory it unzips
> +	$(NSINSTALL) -D $(PKG_STAGE)/bin/host
> +	$(NSINSTALL) $(DIST)/host/bin/szip $(PKG_STAGE)/bin/host

The nsinstall -D isn't actually necessary, since nsinstall will create the dir, right?

::: testing/xpcshell/remotexpcshelltests.py
@@ +139,5 @@
> +                szip = os.path.join(self.localBin, '..', 'host', 'bin', 'szip')
> +                if not os.path.exists(szip):
> +                    # Tinderbox scripts are are not unzipping everything, so
> +                    # the executable is places somewhere it unzips, which
> +                    # doesn't match the location in a local objdir.

I would just say "tinderbox builds must run szip from the test package".

@@ +142,5 @@
> +                    # the executable is places somewhere it unzips, which
> +                    # doesn't match the location in a local objdir.
> +                    szip = os.path.join(self.localBin, 'host', 'szip')
> +                if not os.path.exists(szip):
> +                    szip = None

We should probably error in this case, since the tests aren't going to work.

@@ +143,5 @@
> +                    # doesn't match the location in a local objdir.
> +                    szip = os.path.join(self.localBin, 'host', 'szip')
> +                if not os.path.exists(szip):
> +                    szip = None
> +                for info in self.localAPKContents.infolist():

You could just use namelist() here to get the list of filenames since you're not using any other properties of the ZipInfo.

@@ +155,5 @@
> +                            print >> sys.stderr, os.path.exists(szip)
> +                            subprocess.call(['ldd', szip])
> +                            print >> sys.stderr, "Running %s -d %s" % (szip, file)
> +                            out = subprocess.check_output([szip, '-d', file], stderr=subprocess.STDOUT)
> +                            print >>sys.stderr, out

This seems really verbose. Do we really need this much log output?

Comment 4

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> > +                    # the executable is places somewhere it unzips, which
> > +                    # doesn't match the location in a local objdir.
> > +                    szip = os.path.join(self.localBin, 'host', 'szip')
> > +                if not os.path.exists(szip):
> > +                    szip = None
> 
> We should probably error in this case, since the tests aren't going to work.

If the test package doesn't contain szip, it means the files are not szipped in the test package

Comment 5

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b8dcd393805

Comment 6

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> You could just use namelist() here to get the list of filenames since you're
> not using any other properties of the ZipInfo.

It's used for extract(), and avoids rescanning the file list for a matching name.

Comment 7

[Mike Hommey [:glandium]]

I had removed one line too many before landing.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7610346a34ee

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3b8dcd393805

Comment 9

[Ryan VanderMeulen [:RyanVM]]

This was backed out for xpcshell failures. Callek had me backout both csets to be safe.

https://tbpl.mozilla.org/php/getParsedLog.php?id=21971752&tree=Mozilla-Inbound

TEST-INFO | /builds/tegra-148/test/build/tests/xpcshell/tests/gfx/tests/unit/test_nsIScriptableRegion.js | running test ...
TEST-UNEXPECTED-FAIL | /builds/tegra-148/test/build/tests/xpcshell/tests/gfx/tests/unit/test_nsIScriptableRegion.js | test failed (with xpcshell return code: 255), see following log:
>>>>>>>
xpcw: cd /mnt/sdcard/tests/xpcshell/gfx/tests/unit
xpcw: xpcshell -r /mnt/sdcard/tests/xpcshell/c/httpd.manifest --greomni /data/local/xpcb/fennec-23.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 = []; -e const _TAIL_FILES = []; -e const _TEST_FILE = ["test_nsIScriptableRegion.js"]; -e _execute_test(); quit(0);
link_image[2046]: failed to link /data/local/xpcb/xpcshell
CANNOT LINK EXECUTABLE

<<<<<<<
INFO | Result summary:
INFO | Passed: 0
INFO | Failed: 1
INFO | Todo: 0
program finished with exit code 1

Comment 10

[Mike Hommey [:glandium]]

Relanded with a small fixup (a os.path.basename when building the remote file name)
https://hg.mozilla.org/integration/mozilla-inbound/rev/91e97665da3e

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/91e97665da3e