test_sqliteMultiReporter.xul calls SimpleTest.finish needlessly

RESOLVED FIXED in mozilla12

Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

unspecified
mozilla12
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

No description provided.
Posted patch Patch (v1)Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #587288 - Flags: review?(n.nethercote)
Comment on attachment 587288 [details] [diff] [review]
Patch (v1)

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

rs=me, you understand this better than I do, I find the SimpleTest.finish and waitForExplicitFinish stuff confusing.

BTW, how/why did you find this?  Does having it needlessly have any adverse effects?
Attachment #587288 - Flags: review?(n.nethercote) → review+
(In reply to Nicholas Nethercote [:njn] from comment #2)
> Comment on attachment 587288 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 587288 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> rs=me, you understand this better than I do, I find the SimpleTest.finish
> and waitForExplicitFinish stuff confusing.

Well, to make a long story short, tests which are async should call waitForExplicitFinish once before the load event fires, and then call SimpleTest.finish once when they're done.

> BTW, how/why did you find this?  Does having it needlessly have any adverse
> effects?

Over in bug 668728, we discovered that our mochitest framework sometimes skips a test.  I wrote a patch which does some checks when SimpleTest.finish is called to change such cases to be explicit failures as opposed to silent test skipping.  Tests which call SimpleTest.finish without waitForExplicitFinish mess up with my checks, which is how I found out about this.  :-)
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a7aa3e81b34
Target Milestone: --- → mozilla12
(In reply to Ehsan Akhgari [:ehsan] from comment #3)
> I wrote a patch which does some checks when SimpleTest.finish
> is called to change such cases to be explicit failures as opposed to silent
> test skipping.

Is this check something we can add to the harness?
https://hg.mozilla.org/mozilla-central/rev/0a7aa3e81b34

(In reply to Marco Bonardo [:mak] from comment #5)
> (In reply to Ehsan Akhgari [:ehsan] from comment #3)
> > I wrote a patch which does some checks when SimpleTest.finish
> > is called to change such cases to be explicit failures as opposed to silent
> > test skipping.
> 
> Is this check something we can add to the harness?

Yes, see bug 668728.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.