Android cppunit and xpcshell tests fail to szip libs when --localLib used

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: gbrown, Assigned: gbrown)

Tracking

Trunk
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Remote (Android) cppunit and xpcshell tests push libraries to device to support the test executables. Libraries are either extracted from the apk (when --localApk is specified) or found in the specified directory (when --localLib is specified).

All is well when --localApk is used, but with --localLib, 'szip -d' is not invoked on the local libraries before pushing. As a result, tests fail with library load failures complaining of "bad elf magic".
Depends on: 1202101
This allows me to run cppunit-tests locally via make.

gbrown@mozpad:~/objdirs/droid$ make -C ~/objdirs/droid cppunittests-remote
make: Entering directory `/home/gbrown/objdirs/droid'
Pushing libmozglue.so..
Pushing libplugin-container.so..
Pushing libplugin-container-pie.so..
Pushing libnssckbi.so..
Pushing libfreebl3.so..
Pushing libomxplugingb235.so..
Pushing libnss3.so..
Pushing libomxpluginkk.so..
Pushing libomxpluginhc.so..
Pushing libxul.so..
Pushing libomxplugingb.so..
Pushing liblgpllibs.so..
Pushing libomxplugin.so..
Pushing libsoftokn3.so..
SUITE-START | Running 1 tests
TEST-START | TestFile
PROCESS | TestFile | 
Running nsLocalFile tests...
TEST-PASS | Setup
TEST-PASS | AppendNative with invalid file name
TEST-PASS | GetParent
TEST-PASS | Create file
TEST-PASS | Remove file
TEST-PASS | Create directory
TEST-PASS | MoveTo rename file
TEST-PASS | CopyTo copy file
TEST-PASS | MoveTo move file
TEST-PASS | MoveTo move and rename file
TEST-PASS | CopyTo copy file across directories
TEST-PASS | Normalize with native paths
TEST-PASS | Remove directory
TEST-PASS | CreateUnique file
TEST-PASS | CreateUnique directory
TEST-PASS | OpenNSPRFileDesc DELETE_ON_CLOSE
Finished running nsLocalFile tests.

TEST-PASS | TestFile | took 946ms
SUITE-END | took 0s
Result summary:
cppunittests INFO | Passed: 1
cppunittests INFO | Failed: 0
make: Leaving directory `/home/gbrown/objdirs/droid'
Attachment #8658334 - Flags: review?(dminor)
Blocks: 1090276
Comment on attachment 8658334 [details] [diff] [review]
apply 'szip -d' to local libs

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

r+ when comments are addressed.

::: testing/remotecppunittests.py
@@ +93,3 @@
>                              remote_file = posixpath.join(self.remote_bin_dir, file)
> +                            if szip:
> +                                out = subprocess.check_output([szip, '-d', local_file], stderr=subprocess.STDOUT)

Please catch CalledProcessError here and log the output if it occurs.

@@ +105,3 @@
>                              remote_file = posixpath.join(self.remote_bin_dir, file)
> +                            if szip:
> +                                out = subprocess.check_output([szip, '-d', local_file], stderr=subprocess.STDOUT)

Please catch CalledProcessError here and log the output if it occurs.

::: testing/xpcshell/remotexpcshelltests.py
@@ +460,3 @@
>                  remoteFile = remoteJoin(self.remoteBinDir, file)
> +                if szip:
> +                    out = subprocess.check_output([szip, '-d', localFile], stderr=subprocess.STDOUT)

Please catch CalledProcessError here and log the output if it occurs.

@@ +474,3 @@
>                          remoteFile = remoteJoin(self.remoteBinDir, file)
> +                        if szip:
> +                            out = subprocess.check_output([szip, '-d', localFile], stderr=subprocess.STDOUT)

Please catch CalledProcessError here and log the output if it occurs.
Attachment #8658334 - Flags: review?(dminor) → review+
https://hg.mozilla.org/mozilla-central/rev/6422d6fcc6ae
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
If there's a change needed now, please open a new bug.
Attachment #8776795 - Flags: review?(dminor)
Comment on attachment 8776795 [details]
Bug 1202102 - s/CalledProcessError/subprocess.CalledProcessError/

https://reviewboard.mozilla.org/r/68448/#review65638

This looks good to me, but like Geoff said, should be landed as a separate bug.
https://reviewboard.mozilla.org/r/68448/#review65638

Is there any benefit of creating a new bug report over directly landing the patch now? This patch merely fixes a small mistake in the first patch for bug 1202102, so I thought that it made sense to link it to that bug (since reviewboard does not support patches without bugs yet).
My concern is that this bug and its original patch have a tidy history now, with tracking flag showing that the patch landed in firefox 43, the bug status showing that the patch landed, etc. If your patch gets backed out, or needs an uplift to aurora, or whatever, the history and bug status starts to get "interesting" and can cause confusion. I think you should have opened a new bug.

That said, this is a small change and if you still want to proceed, I won't stop you.
Attachment #8776795 - Attachment is obsolete: true
See Also: → 1295323
You need to log in before you can comment on or make changes to this bug.