mozilla/browserid — pull request #3836 by jrgm (2013-08-27 03:29:56+00:00)
Title: Update to qunit 1.12.0 (fixes #3797, but some open issues for tests that never assert)

(Obviously not ready for merging since it has failing tests.)

So, I was looking into setting up a `window.onerror` handler to deal with compile failures in QUnit tests (GH-3797). But in looking at QUnit source code, I realized that this already had been added some time ago. So by upgrading to latest QUnit, we fix GH-3797 for free.

However, along the way, QUnit also added the requirement that tests must have at least one assertion (which is sensible). There were about 13 tests that did not meet this criterion, but most of them are cases where they only want to assert failure (e.g., this callback must never be called after xhr module shutdown). This PR fixes most of those, but I'm left with some cases that I don't grok and at least #2 and #3 look like bugs in the tests.

To ease review of these changes, I've put a `git diff -U12` with more context at http://people.mozilla.org/~jmorrison/issue-3797-diff-U12.txt. I've also setup https://jrgmnewqunit.personatest.org which has this PR, and https://jrgmoldqunit.personatest.org which is the same changes to the tests minus upgrading QUnit version (and that instance passes (modulo known failures GH-3779 on IE9 and IE8)).

The remaining failures:

- https://github.com/mozilla/browserid/blob/dev/resources/static/test/cases/dialog/js/misc/state.js#L389-L397
```
dialog/js/misc/state: primary_user_authenticating stops all modules (1, 0, 1) Rerun 4 ms
Expected at least one assertion, but none were run - call expect(0) to accept zero assertions.
Source:
@https://jrgmnewqunit.personatest.org/test/cases/dialog/js/misc/state.js:389
@https://jrgmnewqunit.personatest.org/test/cases/dialog/js/misc/state.js:4
```
This one is flaky. The code when run in isolation from any other tests it always passes (i.e., no exception is thrown). But when run with all the other tests, most of the time an exception **is** thrown, but occasionally passes (calls `equal()`). Given the comment "ignore exception, it tries shutting down all the modules", perhaps this is acceptable, in which case we could just add `equal(true, "This is ok too")` in the `catch` clause. @shane-tomlinson: you wrote this test. Comments?

- https://github.com/mozilla/browserid/blob/dev/resources/static/test/cases/pages/js/verify_secondary_address.js#L169-L195
```
pages/js/verify_secondary_address: redirect: message shows with correct timeout (1, 0, 1) Rerun 45 ms
Expected at least one assertion, but none were run - call expect(0) to accept zero assertions.
Source:
QUnit.asyncTest@https://jrgmnewqunit.personatest.org/test/qunit/qunit.js:401
```
In this case, the mocked out replaceFormWithNotice callback is never called. @seanmonstar - Comments?

- https://github.com/mozilla/browserid/blob/dev/resources/static/test/cases/dialog/js/modules/primary_user_provisioned.js#L117-L131
```
dialog/js/modules/primary_user_provisioned: start controller with `add: true` adds email to user's list (1, 0, 1)Rerun30 ms
Expected at least one assertion, but none were run - call expect(0) to accept zero assertions.
Source:
QUnit.asyncTest@https://jrgmnewqunit.personatest.org/test/qunit/qunit.js:401
```
In this case, the `ready` callback is never called. @shane-tomlinson?