Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Introducing mocha.throwError for better async error handling #985

Merged
merged 1 commit into from

5 participants

@jpbochi

This mocha.throwError is supposed to be called by assertion libraries.

It will allows awesome diffs (like this) to be shown even when running async tests is a browser like phantomjs.

It's an alternative to #278 and #942

I plan to send a PR to chai, with a mirroring change.

@jpbochi

@refack thanks for reviewing this.

@travisjeffery travisjeffery merged commit 9e652eb into from
@jpbochi

@travisjeffery Nice! I'll start working on an PR for chai right away.

@tj tj commented on the diff
((12 lines not shown))
if ('uncaughtException' == e) {
global.onerror = function() {};
+
+ var indexOfFn = uncaughtExceptionHandlers.indexOf(fn);
@tj Owner
tj added a note

way too verbose haha var i = would do :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@travisjeffery travisjeffery referenced this pull request from a commit
@travisjeffery travisjeffery Fix var name (#985) 5cd2add
@jpbochi jpbochi commented on the diff
support/tail.js
((12 lines not shown))
if ('uncaughtException' == e) {
global.onerror = function() {};
+
+ var indexOfFn = uncaughtExceptionHandlers.indexOf(fn);
+ if (indexOfFn != -1) { uncaughtExceptionHandlers.splice(indexOfFn, 1); }
@jpbochi
jpbochi added a note

rename indexOfFn here, too? (I know this file is actually generated from the other)

@travisjeffery Owner

nah, this is the one i fixed. the other one in mocha.js is the generated one. don't bother with that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jpbochi

@travisjeffery @visionmedia I just create this PR (chaijs/chai#222) for chai to use mocha.throwError whenever available. Please review. :neckbeard:

@christophercliff

Has this made it into should.js yet?

@jpbochi

@christophercliff nope. I made a PR to chai. I should probably have created one for should.js first, sinc it's maintained by the same folks.

I'll see if I create one this weekend.

@tj
Owner
tj commented

actually now that im reading this again it doesn't quite make sense, it's only supported in the browser, and they're not uncaught, they're just assertion errors so those var names are a little wonky

@jpbochi

@christophercliff There's one complication to implement that on should.js. Currently, there's no easy way to run its tests on a browser.

@visionmedia I'm figuring out a way to run all (or at least some) should.js's tests on a browser. Is it ok if I introduce karma there?

@jpbochi

should.js relies on try+catch of AssertionError's for negative assertions. Unfortunately, this doesn't play out nice with mocha.throwError because the error will be reported to mocha even if you catch it later.

@nchapman nchapman referenced this pull request in mozilla/fxa-content-server
Merged

test(frontend): upgrade mocha for promise support and fix async tests #766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 55 additions and 2 deletions.
  1. +19 −1 mocha.js
  2. +19 −1 support/tail.js
  3. +17 −0 test/browser/large.js
View
20 mocha.js
@@ -5510,13 +5510,18 @@ var process = {};
process.exit = function(status){};
process.stdout = {};
+var uncaughtExceptionHandlers = [];
+
/**
* Remove uncaughtException listener.
*/
-process.removeListener = function(e){
+process.removeListener = function(e, fn){
if ('uncaughtException' == e) {
global.onerror = function() {};
+
+ var indexOfFn = uncaughtExceptionHandlers.indexOf(fn);
@tj Owner
tj added a note

way too verbose haha var i = would do :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (indexOfFn != -1) { uncaughtExceptionHandlers.splice(indexOfFn, 1); }
}
};
@@ -5529,6 +5534,7 @@ process.on = function(e, fn){
global.onerror = function(err, url, line){
fn(new Error(err + ' (' + url + ':' + line + ')'));
};
+ uncaughtExceptionHandlers.push(fn);
}
};
@@ -5566,6 +5572,18 @@ Mocha.Runner.immediately = function(callback) {
};
/**
+ * Function to allow assertion libraries to throw errors directly into mocha.
+ * This is useful when running tests in a browser because window.onerror will
+ * only receive the 'message' attribute of the Error.
+ */
+mocha.throwError = function(err) {
+ uncaughtExceptionHandlers.forEach(function (fn) {
+ fn(err);
+ });
+ throw err;
+};
+
+/**
* Override ui to ensure that the ui functions are initialized.
* Normally this would happen in Mocha.prototype.loadFiles.
*/
View
20 support/tail.js
@@ -24,13 +24,18 @@ var process = {};
process.exit = function(status){};
process.stdout = {};
+var uncaughtExceptionHandlers = [];
+
/**
* Remove uncaughtException listener.
*/
-process.removeListener = function(e){
+process.removeListener = function(e, fn){
if ('uncaughtException' == e) {
global.onerror = function() {};
+
+ var indexOfFn = uncaughtExceptionHandlers.indexOf(fn);
+ if (indexOfFn != -1) { uncaughtExceptionHandlers.splice(indexOfFn, 1); }
@jpbochi
jpbochi added a note

rename indexOfFn here, too? (I know this file is actually generated from the other)

@travisjeffery Owner

nah, this is the one i fixed. the other one in mocha.js is the generated one. don't bother with that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
}
};
@@ -43,6 +48,7 @@ process.on = function(e, fn){
global.onerror = function(err, url, line){
fn(new Error(err + ' (' + url + ':' + line + ')'));
};
+ uncaughtExceptionHandlers.push(fn);
}
};
@@ -80,6 +86,18 @@ Mocha.Runner.immediately = function(callback) {
};
/**
+ * Function to allow assertion libraries to throw errors directly into mocha.
+ * This is useful when running tests in a browser because window.onerror will
+ * only receive the 'message' attribute of the Error.
+ */
+mocha.throwError = function(err) {
+ uncaughtExceptionHandlers.forEach(function (fn) {
+ fn(err) ;
+ });
+ throw err;
+};
+
+/**
* Override ui to ensure that the ui functions are initialized.
* Normally this would happen in Mocha.prototype.loadFiles.
*/
View
17 test/browser/large.js
@@ -29,4 +29,21 @@ describe('something', function(){
done();
}, 1);
})
+
+ it('should provide an even better error on phantomjs', function(done){
+ setTimeout(function(){
+ var AssertionError = function(message, actual, expected) {
+ this.message = message;
+ this.actual = actual;
+ this.expected = expected;
+ this.showDiff = true;
+ };
+ AssertionError.prototype = Object.create(Error.prototype);
+ AssertionError.prototype.name = 'AssertionError';
+ AssertionError.prototype.constructor = AssertionError;
+
+ mocha.throwError(new AssertionError('kabooom', 'text with a typo', 'text without a typo'));
+ done();
+ }, 1);
+ })
})
Something went wrong with that request. Please try again.