Core: New error event for bailing on uncaught errors

[Krinkle]

Background

Historically, QUnit has tried to communicate information exclusively
through its tree structure of modules, tests, and assertions. However,
I think it is time to acknowledge that:

1. Some problems logically cannot be represented by ad-hoc executing
   a runtime-defined test.
2. Some problems are difficult to understood when represented as a test.

For example:

• When an exception happens during early "runStart" or "testStart" events,
  or their callback equivalents (QUnit.begin and QUnit.testStart),
  this results in an uncaught exception reported only with unformatted
  text going to the browser console (or stderr). As such, in a way we
  already have this concept, we're just not handling it very well, and
  in some cases handling it in a way that the text is missing and there
  is a mysterious failure (ref Details of QUnit.begin error are lost #1446).

  We also need to avoid recursion, as can happen from a faulty "testStart"
  or "beforeEach" handler.

• When an exception happens during a "testStart" event, issues can end up
  reported in a confusing backwards order. While we used prioritised queuing
  for the synthetic "global failure" test, it still has to come after the
  current test. And, when our test runs, it would cause the same problem a
  second time.

• When an exception happens during a late event, such as "suiteEnd"
  or "runEnd", we have only bad choices. If we create it as a new test
  in the current module, then we're breaking the internal counter for
  when an "after" event happens and likely either cause another exception.
  In theory this could be handled by re-starting the module, like we do
  when tests run out of order, but it just gets messy.

The one good thing about reporting uncaught errors as synthetic
assertions, is that when they happen in a "todo" test, or from hooks
for a "todo" module, that these are naturally tolerated, as they should.

Changes

• Keep the current behaviour of synthetising assertions for the common case
  of errors happening explicitly during a specific test.

• Remove the behaviour of synthetising entire tests. Instead, report them
  under the new error event.

• The error event increments the overall "bad asssertion" counter, which
  helps maintain do-what-I-mean compatibility with existing tooling for
  CI and such, which typically check this counter from a done() or
  runEnd event handler.

• The HTML Reporter reacts by rendering a block, similar to that of a
  failing test.

• The TAP Reporter (CLI) reacts by printing a "not ok" line, similar
  to that of a failing test, plus a TAP-compliant "Bail!" line to
  indicate definitively that something has gone irreversibly wrong.

• The ProcessingQueue.done() function no longer needs to do its
  own advancing/restarting of the queue and late setting of finished.
  This follows up and partly reverses the changes from Core: Fix late onerror handling #1629.

Impact

• Because this event goes straight to the reporters, there is no
  further interaction with the internal state or synchronous callbacks
  occupied with running tests, thus removing the problem of recursion,
  and avoiding the bootstrapping problem.

• The "uncaught" logic local to the CLI runner is no longer needed now.

[Krinkle]

@smcclure15 Would appreciate a look-over on this from you! 🙂

I've minised it where I could, but the patch still ended up larger than I'd like. Let me know if you see something that may be worth splitting out. The tests in particular had to change a bit, and I believe these are largely mutually exclusive (that is, I couldn't find a useful approach that would pass both before and after).

[Krinkle]

Uncaught errors now increment these. As errors can happen before/during begin(), I had to move this to config.js as the increment would fail otherwise with the stats object being undefined. We can move more of this over, but that's for another day.

[Krinkle]

The unhandled rejection in this test case happens during the loading of the test file, before QUnit.start/QUnit.begin and thus before the reporter is sees runStart. I considered moving the TAP version 13 line to the TapReporter constructor but that felt wrong as I don't think instantiating a reporter should already do something.

Perhaps in a follow-up (or in the same PR?) we could improve on this by instead making it be printed on the first relevant event, which will usually still be runStart, but would be lazily brought forward if for some reason a different event happens first (e.g. "error").

[smcclure15]

I like the approach you've taken; it really cleans up some nasty re-entrant cases that were hard to maintain.
I took a pass through for code edits, but I plan to bash this a little to ensure the edges are well-behaved.

[smcclure15]

Is there a suggested fix?
Use QUnit.on('error', ...)?

[Krinkle]

No, I did not provide a direct replacement. This was an internal method in my mind, with a rather awkward/incompete responsibility. It wasn't exactly a window.onerror callback, because it takes a custom object that one would have to fill in on the caller side, and we also need additional logic as we have in our actual window.onerror callback in html.js before callling this function. It also wasn't exactly a "push global failure" utility.

As part of 07de3c6, I factored out the part I think is re-usable, into QUnit.onUnhandledRejection(), and documented it. And in this PR, I copied the remaining logic from here to our actual window handler in html.js.

If someone out there did end up using this, as part of a custom HTML Reporter or something, and consumed this method, then a replacement would involve calling onUncaughtException(), plus whatever else this method currently does. I don't expect it to be likely, but it's something worth clarifying in the warning message, and to call out in the upcoming migration guide.

Speaking of migration guide, I've made a note of that at #1498.

TODO: Improve console warning message.

[smcclure15]

warn vs error? I'm good with either, but maybe I was surprised to find this wasn't console.error; curious if that was intentional

[Krinkle]

Yeah, it's an awkward fit, though note that we did also use warn() in onUncaughtException previously, for the same purpose (see left/red part of the diff).

This was a compromise to keep the diff minimal because we don't actually interface yet with console.error() at all. (Though I agree that we should!) It's be a bit more work to add it to our cross-realm interface and then adopt it in the places where it makes sense.

[gibson042]

I'm trying to come out of lurk mode on several projects, including QUnit. These changes look good to me, and I second @smcclure15's comments.