893085 - remotecppunittests.py should accept --apk option and test directories as well as files on the command line

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Dan Minor [:dminor]]

To support running cpp unit tests from the test package, it should be possibly to specify test directories as well as test files on the command line, as was done for runcppunittests.py in bug 892439.

Also, specifying a --apk option similar to the one in remotexpcshelltests.py would allow the script to "un-szip" the .so libraries before pushing them to the device, which would facilitate running tests from mozharness.

Comment 1

[Dan Minor [:dminor]]

Attachment: Patch to add --apk, --localBinDir options and accept directories on the command line

Original source for the code to handle szip is: http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/remotexpcshelltests.py#135

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 776389 [details] [diff] [review]
Patch to add --apk, --localBinDir options and accept directories on the command line

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

::: testing/remotecppunittests.py
@@ +45,5 @@
>          self.push_progs(progs)
>          self.device.chmodDir(self.remote_bin_dir)
>  
>      def push_libs(self):
> +        if self.options.local_apk:

Please file a bug on putting this in mozbase, and mention the bug number in a comment here.

@@ +46,5 @@
>          self.device.chmodDir(self.remote_bin_dir)
>  
>      def push_libs(self):
> +        if self.options.local_apk:
> +            tmpdir = tempfile.mkdtemp()

My mozfile.TemporaryDirectory patch made it to m-c, so you can use:
with TemporaryDirectory as tmpdir:

http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozfile/mozfile/mozfile.py?force=1#242

@@ +49,5 @@
> +        if self.options.local_apk:
> +            tmpdir = tempfile.mkdtemp()
> +            try:
> +                apk_contents = ZipFile(self.options.local_apk)
> +                szip = os.path.join(self.options.local_bin, '..', 'host', 'bin', 'szip')

You can get rid of options.local_bin and conditionally import MozbuildObject, like:
http://mxr.mozilla.org/mozilla-central/source/build/pgo/profileserver.py#19

those have a bindir property:
http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/base.py#227

That will work fine for the "running from an objdir" case, and the other block here takes care of running from a test package.

@@ +229,5 @@
> +        if os.path.isdir(p):
> +            #filter out .py files packaged with the unit tests
> +            progs.extend([os.path.abspath(os.path.join(p, x)) for x in os.listdir(p) if not x.endswith('.py')])
> +        else:
> +            progs.append(os.path.abspath(p))

Should we just put this in a common method to share between the local and remote harnesses?

Comment 3

[(not currently active) Ted Mielczarek]

(In reply to Ted Mielczarek [:ted.mielczarek] (post-vacation backlog) from comment #2)
> You can get rid of options.local_bin and conditionally import
> MozbuildObject, like:
> http://mxr.mozilla.org/mozilla-central/source/build/pgo/profileserver.py#19

We discussed this on IRC, I misread this code. You should just use MozbuildObject to get a useful default for local_bin, but otherwise keep this code as-is.

Comment 4

[Dan Minor [:dminor]]

Filed bug 900508 to move "de-szip" functionality to mozbase.

Comment 5

[Dan Minor [:dminor]]

Proposed common method to share between the local and remote harnesses for getting the unit tests from the arguments array:

+def extract_unittests_from_args(args):
+    """Extract unittests from args, expanding directories as needed"""
+    progs = []
+
+    for p in args:
+        if os.path.isdir(p):
+            #filter out .py files packaged with the unit tests
+            progs.extend([os.path.abspath(os.path.join(p, x)) for x in os.listdir(p) if not x.endswith('.py')])
+        else:
+            progs.append(os.path.abspath(p))
+
+    return progs
+

Comment 6

[(not currently active) Ted Mielczarek]

Sounds good.

Comment 7

[Dan Minor [:dminor]]

With review comments addressed: https://hg.mozilla.org/integration/mozilla-inbound/rev/d6d790021591

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d6d790021591