1014653 - Run C++ unit tests on B2G

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[(not currently active) Ted Mielczarek]

We have C++ unit tests running on desktop and Android, but not B2G. There hasn't been any demand for this, but bug 867828 does want some WebRTC C++ unit tests running there, so this would be a prerequisite.

Comment 1

[Jonathan Griffin (:jgriffin)]

What do we mean by "B2G" here?  We're in the process of standing up Mulet B2G builds in buildbot, and we'll be running C++ tests against those.  Is that sufficient?

Comment 2

[(not currently active) Ted Mielczarek]

I suspect they actually want "on device or in emulator". Mulet is just going to be in a desktop environment, right? That's probably not sufficient for this.

Comment 3

[Jonathan Griffin (:jgriffin)]

yes, Mulet is just a desktop environment.  We'll have to run them on an emulator then, since we don't have an (easy) ability to run them on device.

Comment 4

[Dan Minor [:dminor]]

I tried an emulator build today and we are already building and packaging the cppunit tests. The tests would run on an emulator using remotecppunittest.py over ADB: 20 passes, 51 failures. The failures appeared to be due to missing shared libraries, which required a bit of special handling on Android.

With luck, all we will need to get this going are some mozharness changes.

Comment 5

[Dan Minor [:dminor]]

When running this through Mozharness with appropriate --localLib, I see 6 unexpected results. These seem to be the same tests we're currently skipping on Android along with TestPLDHash, which I believe we modified last time to get it running on Android.

We'll also need to add something to remotecppunittests.py to launch the emulator. Work is under way in bug 997244 will change how this is done, so I think it makes sense to wait on that bug for now.

Comment 6

[Dan Minor [:dminor]]

Attachment: Add support for B2G emulator

This adds support for starting the b2g emulator and adds the in-tree config files required to be able to run the tests from mozharness.

Comment 7

[Geoff Brown [:gbrown]]

Comment on attachment 8455377 [details] [diff] [review]
Add support for B2G emulator

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

Did you check that Android cppunittests via adb still work?

::: testing/b2g_cppunittest_manifest.txt
@@ +5,5 @@
> +TestNativeXMLHttpRequest            # Bug 919642
> +mediapipeline_unittest              # Bug 919646
> +TestStartupCache                    # Bug 929655
> +TestStartupCacheTelemetry.manifest  # Bug 929655
> +TestStartupCacheTelemetry.js        # Bug 929655

It seems odd that a manifest and js file are included here along with executables. I know that's the same as in the android manifest, but I wonder if it's necessary, and why.

::: testing/config/mozharness/b2g_emulator_config.py
@@ +52,5 @@
> +        "--dm_trans=adb",
> +        "--symbols-path=%(symbols_path)s",
> +        "--xre-path=%(xre_path)s",
> +        "--addEnv", "LD_LIBRARY_PATH=/vendor/lib:/system/lib:/system/b2g",
> +        "--withB2GEmulator=%(b2gpath)s",

--with-b2g-emulator maybe? There are so many different naming styles in use, I'm not sure what to suggest!

::: testing/remotecppunittests.py
@@ +249,5 @@
>          result = tester.run_tests(progs, options.xre_path, options.symbols_path)
>      except Exception, e:
>          log.error(str(e))
>          result = False
> +    if options.with_b2g_emulator and runner:

It looks like "and runner" may not be necessary here.

Comment 8

[Dan Minor [:dminor]]

Try run here: https://tbpl.mozilla.org/?tree=Try&rev=3af5ebcbdcfc

Comment 9

[Dan Minor [:dminor]]

(In reply to Geoff Brown [:gbrown] from comment #7)
> Comment on attachment 8455377 [details] [diff] [review]
> Add support for B2G emulator
> 
> Review of attachment 8455377 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Did you check that Android cppunittests via adb still work?
I just did, and it turns out some small changes are necessary to get things working. I need to specify a retryLimit explicitly rather than using None, and I need to see check if we're running with the emulator in the final except clause before trying to clean it up. Thanks for reminding me to check that.

> 
> ::: testing/b2g_cppunittest_manifest.txt
> @@ +5,5 @@
> > +TestNativeXMLHttpRequest            # Bug 919642
> > +mediapipeline_unittest              # Bug 919646
> > +TestStartupCache                    # Bug 929655
> > +TestStartupCacheTelemetry.manifest  # Bug 929655
> > +TestStartupCacheTelemetry.js        # Bug 929655
> 
> It seems odd that a manifest and js file are included here along with
> executables. I know that's the same as in the android manifest, but I wonder
> if it's necessary, and why.
The python script attempts to run everything in the directory, so it will push these to the device and try to run them, and then mark them as failed. This was the unsophisticated way of skipping these files.

> 
> ::: testing/config/mozharness/b2g_emulator_config.py
> @@ +52,5 @@
> > +        "--dm_trans=adb",
> > +        "--symbols-path=%(symbols_path)s",
> > +        "--xre-path=%(xre_path)s",
> > +        "--addEnv", "LD_LIBRARY_PATH=/vendor/lib:/system/lib:/system/b2g",
> > +        "--withB2GEmulator=%(b2gpath)s",
> 
> --with-b2g-emulator maybe? There are so many different naming styles in use,
> I'm not sure what to suggest!
I like that better too.

> 
> ::: testing/remotecppunittests.py
> @@ +249,5 @@
> >          result = tester.run_tests(progs, options.xre_path, options.symbols_path)
> >      except Exception, e:
> >          log.error(str(e))
> >          result = False
> > +    if options.with_b2g_emulator and runner:
> 
> It looks like "and runner" may not be necessary here.
Removed.

Thanks for the review!

Comment 10

[Dan Minor [:dminor]]

With comments addressed: https://hg.mozilla.org/integration/mozilla-inbound/rev/692c3ecda749

Comment 11

[Dan Minor [:dminor]]

Attachment: Add support for B2G emulator cppunit tests to Mozharness

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/692c3ecda749

Comment 13

[Jordan Lund (:jlund)]

Comment on attachment 8456844 [details] [diff] [review]
Add support for B2G emulator cppunit tests to Mozharness

lgtm.

I am not familiar with b2g_emulator_unittest.py so just a sanity check, I noticed we explicitly set test_manifest for some reftests[1] but IIUC, we rely on this[2] if condition a lot in preflight_run_tests. 

do we need to add a condition for cppunittest in that block? Can this here[3] be None in _query_abs_base_cmd?

[1] http://mxr.mozilla.org/build/source/buildbot-configs/mozilla-tests/b2g_config.py#726
[2] http://mxr.mozilla.org/build/source/mozharness/scripts/b2g_emulator_unittest.py#316
[3] http://mxr.mozilla.org/build/source/mozharness/scripts/b2g_emulator_unittest.py#270

Comment 14

[Dan Minor [:dminor]]

Thanks Jordan, good catch.

The 'test_manifest' at [1] can be None, because that is used to interpolate values in the in-tree mozharness config file (e.g. "%(test_manifest)s".)

Since the cppunit suite doesn't use the manifest file, it is ok that it isn't specified, and I did a quick check to make sure that it won't throw an exception if you interpolate something with a value of None.

To make it clear that test_manifest isn't needed for now, I'll add a default value of None to [2].

[1] http://mxr.mozilla.org/build/source/mozharness/scripts/b2g_emulator_unittest.py#270
[2] http://mxr.mozilla.org/build/source/mozharness/scripts/b2g_emulator_unittest.py#316

Comment 15

[Jordan Lund (:jlund)]

> To make it clear that test_manifest isn't needed for now, I'll add a default
> value of None to [2].
> 

awesome. makes sense. tx :)

Comment 16

[Dan Minor [:dminor]]

Pushed to: https://hg.mozilla.org/build/mozharness/rev/ea4181d2e64f

Comment 17

[Jordan Lund (:jlund)]

Comment on attachment 8456844 [details] [diff] [review]
Add support for B2G emulator cppunit tests to Mozharness

checked into production: http://hg.mozilla.org/build/mozharness/rev/2913947db89d

Comment 18

[Dan Minor [:dminor]]

From a cedar run here [1] we are green on opt and have some unexpected results for debug:
TEST-UNEXPECTED-FAIL | ../../../gecko/storage/test/test_deadlock_detector.cpp | Sanity - deadlock not detected
TEST-UNEXPECTED-FAIL | ../../../gecko/storage/test/test_deadlock_detector.cpp | Sanity2 - deadlock not detected
TEST-UNEXPECTED-FAIL | ../../../gecko/storage/test/test_deadlock_detector.cpp | Sanity3 - deadlock not detected
TEST-UNEXPECTED-FAIL | ../../../gecko/storage/test/test_deadlock_detector.cpp | main - unknown child test
TEST-UNEXPECTED-FAIL | ../../../gecko/storage/test/test_deadlock_detector.cpp | ContentionNoDeadlock - deadlock
[1308] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file ../../../gecko/xpcom/base/nsTraceRefcnt.cppremotecppunittests TEST-UNEXPECTED-FAIL | test_deadlock_detector | test failed with return code 1
[1404] WARNIremotecppunittests TEST-UNEXPECTED-FAIL | TestCSPParser | test failed with return code 139
remotecppunittests TEST-UNEXPECTED-FAIL | TestTArray | test failed with return code 137
TEST-UNEXPECTED-FAIL | ../../../gecko/xpcom/tests/TestDeadlockDetector.cpp | main - unknown child test
TEST-UNEXPECTED-FAIL | ../../../gecko/xpcom/tests/TestDeadlockDetector.cpp | ContentionNoDeadlock - deadlock
=remotecppunittests TEST-UNEXPECTED-FAIL | TestDeadlockDetector | test failed with return code 1
Return code: 1

[1] https://tbpl.mozilla.org/?tree=Cedar&rev=f3dceebdd32e

Comment 19

[Dan Minor [:dminor]]

Attachment: Add debug only failures to b2g cppunit test manifest

These are debug only failures. The manifest format doesn't permit these tests to be skipped on debug only so I have to skip them on opt as well. The deadlock tests don't appear to be built for opt anyway.

Cedar run here: https://tbpl.mozilla.org/?tree=Cedar&rev=dbcf32e278b5.

Comment 20

[Dan Minor [:dminor]]

Thanks, pushed to: https://hg.mozilla.org/integration/mozilla-inbound/rev/8f075900becf

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8f075900becf

Comment 22

[Dan Minor [:dminor]]

Scheduled and running on trunk trees and aurora.