1377876 - Don't run test_crash.py for code-coverage builds because crash reporter is disabled

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Greg Mierzwinski [:sparky]]

This bug is for disabling the marionette unit tests in the manifest 'unit/unit-tests.ini' when they run on linux64-ccov.

The cause of this problem is a slow build that seems to delay the creation of the minidump files that many of these tests rely on (like 'asan').

Comment 1

[Greg Mierzwinski [:sparky]]

Attachment: Bug 1377876 - Disable 'test_crash.py' when running on linux64-ccov.

This patch adds a flag to the 'mozinfo.json' that can be used to disable tests when they are running on linux64-ccov. Then, this flag is used to prevent the marionnette test manifest 'unit/unit-tests.ini' from running on linux64-ccov.

Review commit: https://reviewboard.mozilla.org/r/154014/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154014/

Comment 2

[Greg Mierzwinski [:sparky]]

Here's a test run with it passing on linux64-ccov: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c49d77df94a0ad11d1f84eaa5bb6ac2c450d5c2f

And here's a test run with linux64 which shows that mozinfo.json contains the coverage field now by default and is set to false: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d3cb7ed3eedb3ec9b30fc415e549166bf3fe196

There is more work that needs to be done to modify how the mozinfo is used in other test suites and this will be done in a follow-up bug because linux64-jsdcov needs a new variable like 'MOZ_CODE_COVERAGE' to be able to have it set by default instead of when the '*/runtests.py' files are being run.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Greg, I thought that we (including Kyle) agreed on to only disable this specific test file for now.

Comment 4

[Greg Mierzwinski [:sparky]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8149ce11d948770311297547f761cedb19129261

Comment 5

[Greg Mierzwinski [:sparky]]

Comment on attachment 8883053 [details]
Bug 1377876 - Disable 'test_crash.py' when running on linux64-ccov.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154014/diff/1-2/

Comment 6

[Greg Mierzwinski [:sparky]]

I misunderstood, I'm sorry about that. I've started another test (just in case) and submitted a new patch for review. I've also left a small formatting change in the 'unit-tests.ini' file I previously had modified.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8883053 [details]
Bug 1377876 - Disable 'test_crash.py' when running on linux64-ccov.

https://reviewboard.mozilla.org/r/154014/#review159224

I'm not a valid reviewer of build config changes. But this looks simple enough and is doing what it should do. As best the patch might have been splitted into two different commits (one per component). But to stop this perma failure I will get it landed now. Thanks for getting this implemented!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44222e50fcf1
Disable 'test_crash.py' when running on linux64-ccov. r=whimboo

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/44222e50fcf1

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Marking as disabled until the underlying bug has been fixed.

Comment 11

[Greg Mierzwinski [:sparky]]

Not a problem :whimboo! And thanks for your comment, I'll keep that in mind for my next patches.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As being informed on bug 1376795 the code coverage builds do not have the crash reporter turned on! As such the unit test should not be run because it relies on its existence.

With my patch on bug 1381403 we now dynamically skip each test if the crash reporter is not available in the build.

Comment 14

[Marco Castelluccio [:marco]]

We have enabled the crash reporter in the code coverage builds now, we should make sure this test is being run.

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

it appears we already run this- the change to skip for 'coverage' doesn't exist, I believe bug 1381403 makes this work more reliably.