724578 - Add optional ability to fail if the number of tests run are not the same as expected

Status: NEW

(Testing :: Mochitest, enhancement)

Description

[David Humphrey (:humph)]

Writing some complicated async tests this past week, I really wished we had something like QUnit's expect(), which allows one to say how many tests are supposed to be run, see:

http://docs.jquery.com/QUnit/expect

This way, when doing async tests, you don't miss tests that neither pass nor fail through not being run.

Comment 1

[David Humphrey (:humph)]

Attachment: Fix

Here's a fix, which adds an optional argument to waitForExplicitFinish.  I debated doing a new method, SimpleTest.expect(), but decided it was more clear to limit this to async tests that require finish.

Comment 2

[cmtalbert]

Comment on attachment 594738 [details] [diff] [review]
Fix

Hi Dave,

Thanks for the patch.  I think this is a great idea, and a good addition to mochitest.  I have two requests with the review.
1. Either change the MDN documentation about "waitForExplicitFinish" on https://developer.mozilla.org/en/Mochitest, or if you don't want to mess with that page (and I don't blame you if you don't), set dev-doc-needed in the whiteboard and we can direct someone to take your comment out of the code and put it into that page where it would make sense.
2. Browser-chrome re-uses the simpletest module, so I'd like to see another sanity test that is run from the browser-chrome mochitest harness to ensure that this feature is available in browser-chrome tests as well (and that we don't break it in the future).

r+ with those changes.  Thanks for the patch!

Comment 3

[David Humphrey (:humph)]

I can do 1) when this lands, sure.

For 2) I'm going to need some input.  I tried to do as you say, but it seems SimpleTest isn't used by browser-chrome tests, see:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#503

and

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#535

So I can do the same logic for browser-test.js if you like, or leave it as is.  I'm fine either way.  I lean toward not changing browser-test.js.  Let me know.

Comment 4

[cmtalbert]

Ugh, I looked at this. I had remembered we did something particularly crazy with the SimpleTest methods. I had thought what we'd done was more sane and we were merely reusing them. But no, as you point out, we're re-implementing half of them and reusing the other half. 

I would prefer that we maintain the same API between the two test harnesses because not doing so will only make this more confusing for the next person who comes along.

That said I hate punishing great people like you who show up with such a helpful patch by forcing them to deal with the insanity of our test harnesses.  So if you want to punt the browser chrome API change over to a follow-on bug (and reference that bug from MDC when you change the doc) we can handle it that way.

I might just write the patch for this issue this weekend.  Thanks for the help, humph.

Comment 5

[David Humphrey (:humph)]

Clint, can this land, then, and we spin off another bug for the other?  I'd like to use this in another patch.