Currently we only run C++ unit tests during "make check". We should run them on the test machines from the test package instead.

If we fix bug 811411, we could also run the C++ unit tests on mobile, which would get us better test coverage.

Work in progress to stage cpptests. Next step will be to move logic to run tests out of 'make check'.

I wouldn't worry about removing them from make check yet. Once we've got them split off into a separate job we can deal with that.

(Also, apparently this patch belongs on bug 811404, FWIW.)

Work in progress to run desktop cppunittests.

Comment on attachment 769149 [details] [diff] [review] Patch to run cppunittests from mozharness. Review of attachment 769149 [details] [diff] [review]: ----------------------------------------------------------------- ::: scripts/desktop_unittest.py @@ +316,5 @@ > + abs_app_plugins_dir = os.path.join(abs_app_dir, 'plugins') > + if suites: # there are cppunittest suites to run > + cppunittests = map(lambda f: os.path.join(dirs['abs_cppunittest_dir'], f), glob.glob(os.path.join(dirs['abs_cppunittest_dir'], '*'))) > + for i in xrange(0, len(cppunittests)): > + suites['cppunittest%d' % i] = [cppunittests[i]] This is...a little weird. You're setting up sub-suites for each thing in the tests dir? I guess this is because runcppunittests.py expects to get binary names on the commandline? We could pretty easily change it to accept either filenames or directory names, and just pass the directory here and force it to enumerate the contents. (Seems like that would simplify this.) @@ +361,5 @@ > env['MINIDUMP_STACKWALK'] = c['minidump_stackwalk_path'] > if c.get('minidump_save_path'): > env['MINIDUMP_SAVE_PATH'] = c['minidump_save_path'] > + if c.get('ld_library_path'): > + env['LD_LIBRARY_PATH'] = c['ld_library_path'] runcppunittests.py should already be taking care of this, no? http://mxr.mozilla.org/mozilla-central/source/testing/runcppunittests.py#93

I set up sub-suites so that runcppunittests.py is invoked for each executable file individually as it gave more stable test runs than attempting to run all of the tests from the same call to runcppunittests.py. I haven't (yet) spent the time to determine as to why this would be the case. I wasn't aware that runcppunittests was attempting to set up LD_LIBRARY_PATH. It doesn't work properly (at least with mozharness), but I'll try to fix it there rather than through the mozharness script.

Let's scope this to only cover desktop tests for now, we can do Android tests as a followup. We know the desktop tests work since we already run them. We don't run the tests on Android, so bringing them up might involve disabling or fixing some of them.

I'll also attach a sample config file that I've been using for local testing.

Comment on attachment 774606 [details] [diff] [review] Patch to add support for running c++ unittests to desktop_unittest.py. Review of attachment 774606 [details] [diff] [review]: ----------------------------------------------------------------- ::: scripts/desktop_unittest.py @@ +317,5 @@ > + #copy shared libs from application to test bin dir > + libs = glob.glob(os.path.join(abs_app_dir, '*.so')) > + for lib in libs: > + shutil.copy2(os.path.join(abs_app_dir, lib), > + os.path.join(dirs['abs_test_bin_dir'])) You shouldn't need to do this, you should be able to just call runcppunittests with --xre-path=abs_app_dir. This is exactly what we do for running them in-tree: http://mxr.mozilla.org/mozilla-central/source/config/rules.mk#179

Comment on attachment 774609 [details] Sample config file to run cpp unittests on linux. Ah, I see, you just have a wrong xre path here. This wants to be the app dir, I don't know how you'd represent that in a mozharness config. (xre stands for XUL Runtime Environment, but basically it's just wherever libxul is.)

Ted: thanks! I was under the mistaken impression that --xre-path had to point to where xulrunner lives, not libxul. I'll update the patch.

Comment on attachment 774654 [details] [diff] [review] Patch to add support for running c++ unittests to desktop_unittest.py. mozcrash depends on mozfile+mozlog, so I think it needs to come after those two in the list.

Thanks! Revised version of the patch.

Comment on attachment 775766 [details] [diff] [review] Patch to add support for running c++ unittests to desktop_unittest.py. https://hg.mozilla.org/build/mozharness/rev/9abede48fb56

Backed out due to bustage, e.g. https://tbpl.mozilla.org/php/getParsedLog.php?id=25301015&tree=Mozilla-Aurora : 15:53:16 ERROR - Return code: 1 15:53:17 ERROR - Return code: 1 16:04:52 FATAL - Uncaught exception: Traceback (most recent call last): 16:04:52 FATAL - File "C:\slave\test\scripts\mozharness\base\script.py", line 1005, in run 16:04:52 FATAL - self.run_action(action) 16:04:52 FATAL - File "C:\slave\test\scripts\mozharness\base\script.py", line 947, in run_action 16:04:52 FATAL - self._possibly_run_method(method_name, error_if_missing=True) 16:04:52 FATAL - File "C:\slave\test\scripts\mozharness\base\script.py", line 888, in _possibly_run_method 16:04:52 FATAL - return getattr(self, method_name)() 16:04:52 FATAL - File "scripts/scripts/desktop_unittest.py", line 286, in run_tests 16:04:52 FATAL - self._run_category_suites('cppunittest') 16:04:52 FATAL - File "scripts/scripts/desktop_unittest.py", line 312, in _run_category_suites 16:04:52 FATAL - abs_base_cmd = self._query_abs_base_cmd(suite_category) 16:04:52 FATAL - File "scripts/scripts/desktop_unittest.py", line 192, in _query_abs_base_cmd 16:04:52 FATAL - run_file = c['run_file_names'][suite_category] 16:04:52 FATAL - KeyError: 'cppunittest' 16:04:52 FATAL - Exiting -1

Sorry I missed that. Looks like this can't be landed without updating the configuration files as wel (or adding code to handle a missing suite_category)

Ash run for these changes is here: https://tbpl.mozilla.org/?tree=Ash&rev=87e0aaba801d. (The Talos failures are due to me forgetting to update ash-mozharness to the latest mozharness prior to applying my patch.)

Comment on attachment 781686 [details] [diff] [review] Patch to desktop_unittests.py and config files to run c++ unit tests. Thanks! It wouldn't hurt to watch Cedar after this lands on default, and inbound once it lands on production, since we had issues the first time around. The Ash run helps a lot, though.

Comment on attachment 781686 [details] [diff] [review] Patch to desktop_unittests.py and config files to run c++ unit tests. http://hg.mozilla.org/build/mozharness/rev/3605ab526175

In production

We're now running from the test package and not from make check.