Enabling on-demand decompression on android breaks xpcshell tests

RESOLVED FIXED in mozilla23

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla23
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

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

Comment 1

5 years ago
(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.
(Assignee)

Comment 2

5 years ago
Created attachment 737369 [details] [diff] [review]
Un-szip libraries before pushing them on the device for xpcshell tests
Attachment #737369 - Flags: review?(ted)
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?
Attachment #737369 - Flags: review?(ted) → review+
(Assignee)

Comment 4

5 years ago
(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
(Assignee)

Comment 6

5 years ago
(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.
(Assignee)

Comment 7

5 years ago
I had removed one line too many before landing.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7610346a34ee
https://hg.mozilla.org/mozilla-central/rev/3b8dcd393805
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla23 → ---
(Assignee)

Comment 10

5 years ago
Relanded with a small fixup (a os.path.basename when building the remote file name)
https://hg.mozilla.org/integration/mozilla-inbound/rev/91e97665da3e
https://hg.mozilla.org/mozilla-central/rev/91e97665da3e
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.