Core: ensure process-queue advances with rejected promises

[step2yeung]

Problem: processTaskQueue processes each task from the task queue as a promise. When the task throws an error, causing the promise to reject, the processing-queue does not continue to process the rest of the tasks queue. The user effect of this problem is qunit hanging, without continuing to process the rest of the task queue, meaning the test will not finish.

In our emberjs app that is uses qunit, the test execution will hang. Testem (the test runner) will recognize the browser has be hanging (using a timeout value) and exit the browser with some number of tests not executed.

Fix:

• Make sure we advance() the processing-queue when a promise is rejected.
• Make sure errors thrown from callbacks are handled, and subsequent callbacks are called

The fix will throw the error, and continue to process the tasks, so the test run will actually run to completion.

Fixes https://github.com/qunitjs/qunit/issues/1446.

[rwjblue]

@step2yeung - Would it be possible to split this into two changes (one for the actual bug being fixed along with its test, and another for the refactorings)?

[stefanpenner]

@step2yeung could you elaborate on the "problem" in the description above, can you relate your description to a user facing example. Currently, I'm having some trouble connecting the two.

[step2yeung]

@stefanpenner Updated the description, hope it helps clarify the user facing issue.

@rwjblue Separated out the fix and the refactor (will submit that afterwards)

[Krinkle]

You may ignore these inline comments. I've uncovered a couple of other things that I'll summarise at https://github.com/qunitjs/qunit/issues/1446.

[Krinkle]

The one part I believe we still haven't fixed is advancing the process-queue. It makes sense to advance the queue, but, I'd also like to make sure (to avoid future regressions) that the negative impact of this is quantified in an integration test.

For the QUnit CLI (TAP) this is probably not particularly significant as Node.js processes naturally end at this point, and the error is reported with TAP-formatted bail out. For browsers, I imagine the process would remain stuck pending timeout, exactly as before, as we'd not proceed and thus not reach the runEnd event or done() callback. This does mean that we'd run all the tests as well, which is potentially incorrect, but that's kind of the established pattern and before/beforeEach failures are handled the same way.