Install Mocha's global error handler in each test's iframe #140
Conversation
…frame. Remove it when the runner ends. Fixes jfirebaugh#136.
| window.onerror = function() { | ||
| _mochaGlobalError.apply(this, arguments); | ||
| // return false so Poltergeist doesn't try to capture the error directly | ||
| return false; |
There was a problem hiding this comment.
According to spec, return true should be used to indicate the error is handled. WebKit had it backward for some time, including the version that PhantomJS uses. I will get a return true upstream in mocha -- meanwhile see bfa5516.
There was a problem hiding this comment.
Is bfa5516 really desirable? It seems like it would be useful to catch errors that happen before the tests themselves start. In any case, I'll remove it.
There was a problem hiding this comment.
I wouldn't want it in a production test suite, but I'm not worried about it for the two poltergeist examples in Konacha's own suite.
There was a problem hiding this comment.
Oh, duh. For some reason I was thinking that would apply to apps using Konacha, which is obviously nonsense.
…o each iframe. Remove it when the runner ends. Fixes jfirebaugh#136." This reverts commit 39379f1.
|
Thanks, committed as f7fc286, and added you as a repo collaborator. |
This patch fixes #136 by copying Mocha's global error handler onto each iframe after the runner starts (Mocha sets up the handler as the last thing
Runner#rundoes, so you can't even do it on thestartevent). When the runner ends, it cleans everything up.Alternatively, you could overwrite Mocha's
process.on()shim, but that seems more like an implementation detail this prone to breakage.This also adds a test for async errors.
Also worth noting: in the process of writing this patch, I discovered Mocha's async error handling is a little buggy, which I had missed in my earlier test of it. Specifically, if an async error gets caught by Mocha and then
done()is still called later, it can screw things up a bit. Mocha doesn't actually fully clean up the test when the async error is caught, but does fire test failure events :\I think #136 is still worth fixing anyway, but I probably also need to file a bug on Mocha, too.