727152 - Robocop: FennecMochitestAssert should not rely on finalize()

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Currently FennecMochitestAssert prints out a bunch of test-end output in the finalize() function, which is risky. The finalize() function is run by the GC and may not be run at all if the VM exits before the object is GC'd (e.g. by somebody calling System.exit). At best the output will be printed at an unpredictable time. This behaviour should be made more deterministic.

Comment 1

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Keep in mind that BaseTest.tearDown explicitly calls mAsserter.finalize().

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I'm not sure that's better; that means it could get run twice instead.

Comment 3

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: do not use Asserter.finalize

As far as I can tell, there's no reason to have this code in finalize, or to define finalize for the asserter at all -- this is just code that we want to execute at the end of the test run. I have simply renamed finalize -> endTest().

We lose the possibility of getting this info dumped when tearDown is not called...but I think that's reasonable: If tearDown is not called, there's something very wrong with the test and we want to fail anyway.

Comment 4

[Geoff Brown [:gbrown] (pto Apr 11-18)]

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

Comment 5

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

Comment on attachment 650268 [details] [diff] [review]
do not use Asserter.finalize

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

looks good to me!

Comment 6

[Geoff Brown [:gbrown] (pto Apr 11-18)]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a16f00f39351