-
Notifications
You must be signed in to change notification settings - Fork 776
Description
From #1446
I could still reproduce the issue of swalled stacktraces for uncaught errors and rejections from
QUnit.begin(), however all this needs is a.catch()handler for the queue advance step of the early callbacks.... and, that's exactly what @step2yeung already proposed with #1391.
So... this is technically true and would suffice, and it might make sense to patch and release that by itself in the name of progress. However, as part of working on this patch I figured it would make sense to add tests for failures from other callbacks as well and see how those actually get handled internally, and I'm not particularly happy with the state of that.
QUnit.begin(), QUnit.moduleStart(), and QUnit.testStart()
This is the most straight-forward in that it runs at a point where all internal state is naturally the most clean. It has its own call stack disconnected from any test, and so offers a natural point where we can tack on .catch( onUncaughtException ) and change .then( advance ) to .finally( advance ) to make sure the reporting of the "global failure" test happens correctly.
The downside of this approach is that this means we will try to run all tests to completion still, which depending on how important the begin handler was could result in a lot of noisy cascading failures. Perhaps not unreasonable in QUnit's spirit of trying to report all subsequent errors upfront, but something to keep in mind.
The real problem, however, comes when you have a failure in a testStart or moduleStart callback as well. Because we use a test() to report global failures between runStart and runEnd, this would end up reporting something like this:
not ok: global failure # from: QUnit.begin -> onUncaughtException -> new test
message: global failure: Boo # from: QUnit.testStart -> onUncaughtException -> push assertion
result: false
...
message: Boo # from: QUnit.begin -> onUncaughtException -> new test -> assertion
result: false
QUnit.testDone(), QUnit.moduleDone()
The real mess starts with the end handlers. These similarly try to create new tests. These tests either get queued to the end and reported rather late (possibly never), or if we use the prioritize flag they can be brought forward and with them the implicit "global" module that the new test would be contained by, and thus afterward another moduleDone handler. This is a problem since moduleDone handling has already closed the module hooks, but then a new test is inserted, increasing its count, and then reaching moduleDone for the second time, but this time triggering the good ol TypeError: module.hooks[handler] is undefined safeguard. In the browser this would go natively to the console, and in the CLI it would be swallowed due to recursive failures from uncaught errors, and then process.on('exit') would report Error: Process exited before tests finished running.
All of this doesn't even get into what happens when the "current" module is ignored, skipped, focussed, todo, etc. It's all rather arbitrary.
Path forward
I think we should consider following the "bail out" paradigm for uncaught exceptions. This means they would be passed directly to reporters instead of via an ad-hoc "test", and then the processing queue cancelled.
This is more or less what happens today, although mostly by accident and in a mostly silent/confusing manner. The TAP standard has already specified this feature and is in use by other systems.