1598274 - Improve test_crash.py to wait for expected content crashes by using the CrashManager

Status: NEW

(Testing :: Marionette Client and Harness, enhancement, P5)

Description

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

Over on bug 1420363 comment 55 Gabriele brought up an interesting behavior for mochitests related to expected crashes:

(In reply to Gabriele Svelto [:gsvelto] from bug 1420363 comment #57)

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #56)

This is interesting. I wonder if we should also use that in those Marionette tests which explicitly check for expected crashes. Do you have some more information about that, or just links? Thanks.

It's pretty simple to use, given one or more crash IDs you call CrashManager.ensureCrashIsPresent() and wait on the promises it returns. This is how it's done in SpecialPowersParent.jsm, we gather promises for every crash here:

https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/testing/specialpowers/content/SpecialPowersParent.jsm#776-779

Then we wait for them before touching the .dmp/.extra files:

https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/testing/specialpowers/content/SpecialPowersChild.jsm#711-713

BrowserTestUtils.jsm does something similar:

https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm#1686-1756

This would be worthwhile to check if we could get the same implemented for test_crash.py where we currently wait on the file system level only. This is a bit racy specifically on slow systems

This would most likely only apply to content crashes but not when the parent process crashes.

Comment 1

[Gabriele Svelto [:gsvelto]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #0)

This would most likely only apply to content crashes but not when the parent process crashes.

Yes, the files of a main process crash are modified by the crash reporter client but AFAIK that's always disabled in tests so once they've been written out they won't be modified anymore.

BTW this use case made me think about the Rust rewrite we've started (bug 1588530): writing and processing .dmp/.extra files will happen in an external process, so we might make the test harnesses talk to it directly instead of having to look around for the files left after each run.