844852 - Run GTest on buildbot

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Benoit Girard (:BenWa)]

Blocked by bug 844288. Once this is solved this should be straightforward.

Comment 1

[(not currently active) Ted Mielczarek]

We'll probably want to wrap them in a little bit of Python harness in order to facilitate getting stacks out of them when they crash etc. We can probably just reuse the runcppunittests harness I wrote:
http://mxr.mozilla.org/mozilla-central/source/testing/runcppunittests.py

This will also make it easier to run them on the test slaves if we so desire, and run them on mobile (there's already a remotecppunittest.py).

Related: if we don't already, we should enable crash reporting in the gtest harness when we do this.

Comment 2

[Benoit Girard (:BenWa)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> Related: if we don't already, we should enable crash reporting in the gtest
> harness when we do this.

Perhaps this deserve a bug. It's not obvious to me why a test suite would want crash reporting.

Comment 3

[(not currently active) Ted Mielczarek]

So that when your tests eventually trigger a crash you can figure out where. :)

Comment 4

[Benoit Girard (:BenWa)]

I though crash reporter was to submit crash reports to crash-stats and we used something else to catch test crashes. Can you point me to an example of how other suite enables it?

Comment 5

[(not currently active) Ted Mielczarek]

We enable Breakpad's exception handler while running unit tests to catch crashes. You can see this in the C++ unit test harness I linked in comment 1:
http://mxr.mozilla.org/mozilla-central/source/testing/runcppunittests.py#74

Then the Python harnesses look for minidumps and process them to spit out a stack:
http://mxr.mozilla.org/mozilla-central/source/testing/runcppunittests.py#54

This is how we get stack traces for crashes in unit test runs on TBPL.

Comment 6

[Ed Morley [:emorley]]

(Tinderbox doesn't exist for gecko)

Comment 7

[Benoit Girard (:BenWa)]

Attachment: Add testing/gtest/rungtests.py

I made very few changes to runcppunittest.cpp but here's a list:
* Renamed symbols
* Replaced multiple binary with a single firefox/gecko binary
* Define MOZ_TBPL_PARSER=1 to true to replace the GTest output

This script does not hang off any make check:: target. I expect this will only be ran via releng. Before running this you need a top level build + 'make -C objdir/testing/gtest gtest'.

Comment 8

[Benoit Girard (:BenWa)]

CC ateam to get the ball rolling in deploying this so it doesn't become the critical path. What are the requirements to getting this deployed on buildbot/integration tests?

With bug 844288 + this patch the test package should include everything required to run GTest by running rungtests.py. Are there any additional requirements I should cover now?

I think initially this will only run on Desktop so we don't need to block on bug 846356.

Comment 9

[Benoit Girard (:BenWa)]

Attachment: Add testing/gtest/rungtests.py

Missed qref

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 741943 [details] [diff] [review]
Add testing/gtest/rungtests.py

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

r=me with a few nits fixed.

::: testing/gtest/rungtests.py
@@ +37,5 @@
> +        Return True if the program exits with a zero status, False otherwise.
> +        """
> +        basename = os.path.basename(prog)
> +        log.info("Running test %s", basename)
> +        with TemporaryDirectory() as tempdir:

If none of your tests need a temporary directory you can do without this. It was convenient for the C++ tests because they need a profile directory.

@@ +67,5 @@
> +        """
> +        env["MOZILLA_FIVE_HOME"] = self.xre_path
> +        env["MOZ_XRE_DIR"] = self.xre_path
> +        #TODO: switch this to just abort once all C++ unit tests have
> +        # been fixed to enable crash reporting

This comment is misleading.

@@ +70,5 @@
> +        #TODO: switch this to just abort once all C++ unit tests have
> +        # been fixed to enable crash reporting
> +        env["XPCOM_DEBUG_BREAK"] = "stack-and-abort"
> +        env["MOZ_CRASHREPORTER_NO_REPORT"] = "1"
> +        env["MOZ_CRASHREPORTER"] = "1"

This isn't actually going to enable crash reporting for you, you'll have to handle that inside your gtest runner. It shouldn't be too hard, something like:
http://mxr.mozilla.org/mozilla-central/source/media/mtransport/test/mtransport_test_utils.h#55

I guess that might be the other reason I used a temp dir in the C++ unit tests, to have a known location to write minidumps.

@@ +100,5 @@
> +        return env
> +
> +    def run_gtest(self, prog, xre_path, symbols_path=None):
> +        """
> +        Run a set of C++ unit test programs.

This comment is sort of wrong. Also, you can just merge this and run_one_test, since you don't have multiple test binaries.

Comment 11

[Benoit Girard (:BenWa)]

Attachment: Add testing/gtest/rungtests.py

https://tbpl.mozilla.org/?tree=Try&rev=f5867fd03f2b

Comment 12

[Benoit Girard (:BenWa)]

Attachment: Add testing/gtest/rungtests.py v3

https://tbpl.mozilla.org/?tree=Try&rev=9d27929872cb

Comment 13

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb8b971070eb

Comment 14

[Ryan VanderMeulen [:RyanVM]]

(In reply to Benoit Girard (:BenWa) from comment #13)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/eb8b971070eb

Backed out for checktest crashes.
https://hg.mozilla.org/integration/mozilla-inbound/rev/31aa900a060b

https://tbpl.mozilla.org/php/getParsedLog.php?id=24127568&tree=Mozilla-Inbound

Comment 15

[(not currently active) Ted Mielczarek]

Exciting. Also realizing that you're linking gtest-libxul after buildsymbols happens, so you don't have symbols for it. :-/

Comment 16

[Benoit Girard (:BenWa)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #15)
> Exciting. Also realizing that you're linking gtest-libxul after buildsymbols
> happens, so you don't have symbols for it. :-/

I'll file a follow up bug for that. I want to start running this per checkin right away before people check in platform specific tests. Should I modify 'make -c testing/gtest check' to make symbols for gtest-libxul?

Comment 17

[Benoit Girard (:BenWa)]

Attachment: Add testing/gtest/rungtests.py v4

https://tbpl.mozilla.org/?tree=Try&rev=896c086c610b

Comment 18

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ec58d112f88

Comment 19

[(not currently active) Ted Mielczarek]

(In reply to Benoit Girard (:BenWa) from comment #16)
> I'll file a follow up bug for that. I want to start running this per checkin
> right away before people check in platform specific tests. Should I modify
> 'make -c testing/gtest check' to make symbols for gtest-libxul?

You can probably do that by just running $(PYTHON) $(PYTHON) $(topsrcdir)/toolkit/crashreporter/tools/symbolstore.py $(DUMP_SYMS_BIN) $(DIST)/crashreporter-symbols <path to gtest-libxul binary>, where DUMP_SYMS_BIN is like:
http://mxr.mozilla.org/mozilla-central/source/Makefile.in#89

(We could move that definition to configure or config.mk or something if that's easier.)

We'll have to rethink this all when we move this our of make check to a separate test job.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9ec58d112f88