Closed
Bug 727152
Opened 12 years ago
Closed 12 years ago
Robocop: FennecMochitestAssert should not rely on finalize()
Categories
(Testing :: Mochitest, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla17
People
(Reporter: kats, Assigned: gbrown)
References
Details
Attachments
(1 file)
3.32 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
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.
Blocks: mobile-automation
Assignee | ||
Comment 1•12 years ago
|
||
Keep in mind that BaseTest.tearDown explicitly calls mAsserter.finalize().
Reporter | ||
Comment 2•12 years ago
|
||
I'm not sure that's better; that means it could get run twice instead.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → gbrown
Assignee | ||
Comment 3•12 years ago
|
||
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.
Attachment #650268 -
Flags: review?(jmaher)
Assignee | ||
Comment 4•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=8fe573bfad2d
Comment 5•12 years ago
|
||
Comment on attachment 650268 [details] [diff] [review] do not use Asserter.finalize Review of attachment 650268 [details] [diff] [review]: ----------------------------------------------------------------- looks good to me!
Attachment #650268 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a16f00f39351
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a16f00f39351
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•