904574 - Crashes during gtests output the failure lines with the testname as 'firefox' which breaks TBPL bug suggestions

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Ed Morley [:emorley]]

eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=26483522&tree=Mozilla-Central#error0

{
Summary

PROCESS-CRASH | firefox | application crashed [@ libxul.so + 0x1831918]
gtests TEST-UNEXPECTED-FAIL | firefox | test crashed
make[1]: *** [check] Error 1
make: *** [check] Error 2
}

TBPL then quicksearches roughly "ALL firefox keywords:intermittent-failure", which leads to many false positives.

We should:
1) Use a more specific test name (even a generic "gtest", if we can't extract the exact currently running test.
2) Not double print the crash line, since mozcrash already outputs the PROCESS-CRASH line.

Comment 1

[Ed Morley [:emorley]]

Attachment: Make crash log output compatible with TBPL

* s/firefox/gtests/
* Removes duplicate crash line

Comment 2

[Ed Morley [:emorley]]

Attachment: Fix existing pyflakes warnings

rungtests.py:8: 'tempfile' imported but unused
rungtests.py:8: 'shutil' imported but unused
rungtests.py:11: 'contextmanager' imported but unused

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 789559 [details] [diff] [review]
Make crash log output compatible with TBPL

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

::: testing/gtest/rungtests.py
@@ +31,5 @@
>          Return True if the program exits with a zero status, False otherwise.
>          """
>          self.xre_path = xre_path
>          env = self.build_environment()
> +        log.info("Running gtests")

Let's keep gtest singular to avoid confusion. (a few instance below)
Would gtest-<basename> be better? Then again I have no plans on introducing multiple gtest run so this is likely fine.

Comment 4

[Ed Morley [:emorley]]

(In reply to Benoit Girard (:BenWa) from comment #3)
> Let's keep gtest singular to avoid confusion. (a few instance below)
> Would gtest-<basename> be better? Then again I have no plans on introducing
> multiple gtest run so this is likely fine.

Singular wfm :-)

We don't add app suffix/prefixes to other psuedo test names (eg 'leakcheck'), so I think we should stick to 'gtest' for now - can always change if we add another.

Comment 5

[Ed Morley [:emorley]]

Would you also like to s/gtests/gtest/ in this existing line?
> log = mozlog.getLogger('gtests')

(results in the logger prefix eg: "gtests TEST-UNEXPECTED-FAIL | firefox | test failed with return code %d")

Comment 6

[Benoit Girard (:BenWa)]

Yes please.

Comment 7

[Ed Morley [:emorley]]

Ty :-)

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a7ceae74e71a
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/925bd252ae58

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a7ceae74e71a
https://hg.mozilla.org/mozilla-central/rev/925bd252ae58