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

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

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

Comment 1

[Geoff Brown [:gbrown]]

Attachment: apply 'szip -d' to local libs

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'

Comment 2

[Dan Minor [:dminor]]

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.

Comment 3

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2308d5c7ea3a

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6422d6fcc6ae

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6422d6fcc6ae

Comment 6

[Rob Wu [:robwu]]

Attachment: Bug 1202102 - s/CalledProcessError/subprocess.CalledProcessError/

Review commit: https://reviewboard.mozilla.org/r/68448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68448/

Comment 7

[Geoff Brown [:gbrown]]

If there's a change needed now, please open a new bug.

Comment 8

[Dan Minor [: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.

Comment 9

[Rob Wu [:robwu]]

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

Comment 10

[Geoff Brown [:gbrown]]

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.