Skip to content

Commit

Permalink
Fix throws assertion when using not and both args
Browse files Browse the repository at this point in the history
  • Loading branch information
@lucasfcosta
lucasfcosta committed Apr 23, 2016
1 parent 3ef5e84 commit 7eed9ab
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 70 deletions.
85 changes: 52 additions & 33 deletions lib/chai/core/assertions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1287,22 +1287,22 @@ module.exports = function (chai, _) {
* @alias throws
* @alias Throw
* @param {Error|ErrorConstructor} errorLike
* @param {String|RegExp} errMatcher error message
* @param {String|RegExp} errMsgMatcher error message
* @param {String} message _optional_
* @see https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Error#Error_types
* @returns error for chaining (null if no error)
* @namespace BDD
* @api public
*/

function assertThrows (errorLike, errMatcher, msg) {
function assertThrows (errorLike, errMsgMatcher, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object');
var negate = flag(this, 'negate');
new Assertion(obj, msg).is.a('function');

if (errorLike instanceof RegExp || typeof errorLike === 'string' && arguments.length < 2) {
errMatcher = errorLike;
if (errorLike instanceof RegExp || typeof errorLike === 'string') {
errMsgMatcher = errorLike;
errorLike = null;
}

Expand All @@ -1315,11 +1315,10 @@ module.exports = function (chai, _) {

// If we have the negate flag enabled and at least one valid argument it means we do expect an error
// but we want it to match a given set of criteria
var everyArgIsUndefined = Array.prototype.slice.call(arguments).every(function(el) { return el === undefined });
var everyArgIsUndefined = errorLike === undefined && errMsgMatcher === undefined;

// Checking if error was thrown
if (everyArgIsUndefined || !everyArgIsUndefined && !negate) {

// We need this to display results correctly according to their types
var errorLikeString = 'an error';
if (errorLike instanceof Error) {
Expand All @@ -1338,49 +1337,69 @@ module.exports = function (chai, _) {
);
}

// If an error was caught and `errorLike` exists we check instances and constructors
if (caughtErr && errorLike) {
var expectedConstructorName = _.checkError.getConstructorName(errorLike);
var actualConstructorName = _.checkError.getConstructorName(caughtErr);
// If we've got the negate flag enabled and both args, we should only fail if both aren't compatible
// See Issue #551@GitHub
var everyArgIsDefined = errorLike && errMsgMatcher;
var constructorError;
var messageError;

if (errorLike && caughtErr) {
// We should compare instances only if `errorLike` is an instance of `Error`
if (errorLike instanceof Error) {
var hasCompatibleInstance = _.checkError.compatibleInstance(caughtErr, errorLike)
this.assert(
hasCompatibleInstance
this.assert(
_.checkError.compatibleInstance(caughtErr, errorLike)
, 'expected #{this} to throw #{exp} but #{act} was thrown'
, 'expected #{this} to not throw #{exp}' + (caughtErr && !negate ? ' but #{act} was thrown' : '')
, errorLike.toString()
, caughtErr.toString()
);
}

this.assert(
_.checkError.compatibleConstructor(caughtErr, errorLike)
, 'expected #{this} to throw #{exp} but #{act} was thrown'
, 'expected #{this} to not throw #{exp}' + (caughtErr ? ' but #{act} was thrown' : '')
, (errorLike instanceof Error ? errorLike.toString() : expectedConstructorName)
, (caughtErr instanceof Error ? caughtErr.toString() : actualConstructorName)
);
try {
this.assert(
_.checkError.compatibleConstructor(caughtErr, errorLike)
, 'expected #{this} to throw #{exp} but #{act} was thrown'
, 'expected #{this} to not throw #{exp}' + (caughtErr ? ' but #{act} was thrown' : '')
, (errorLike instanceof Error ? errorLike.toString() : errorLike && _.checkError.getConstructorName(errorLike))
, (caughtErr instanceof Error ? caughtErr.toString() : caughtErr && _.checkError.getConstructorName(caughtErr))
);
} catch(e) {
if (everyArgIsDefined && negate) {
constructorError = e;
} else {
throw e;
}
}
}

// We only check the message if an `errMatcher` arg has been provided
if (errMatcher) {
var expectedMessage = errMatcher;
var actualMessage = _.checkError.getMessage(caughtErr);

if (errMsgMatcher) {
// Here we check compatible messages
var placeholder = 'including';
if (errMatcher instanceof RegExp) {
if (errMsgMatcher instanceof RegExp) {
placeholder = 'matching'
}

this.assert(
_.checkError.compatibleMessage(caughtErr, errMatcher),
'expected #{this} to throw error ' + placeholder + ' #{exp} but got #{act}',
'expected #{this} to throw error not ' + placeholder + ' #{exp}',
expectedMessage,
actualMessage
);
try {
this.assert(
_.checkError.compatibleMessage(caughtErr, errMsgMatcher)
, 'expected #{this} to throw error ' + placeholder + ' #{exp} but got #{act}'
, 'expected #{this} to throw error not ' + placeholder + ' #{exp}'
, errMsgMatcher
, _.checkError.getMessage(caughtErr)
);
} catch(e) {
if (everyArgIsDefined && negate) {
messageError = e;
} else {
throw e;
}
}
}

if (everyArgIsDefined && negate) {
if (constructorError && messageError) {
throw constructorError;
}
}

flag(this, 'object', caughtErr);
Expand Down
71 changes: 42 additions & 29 deletions lib/chai/utils/checkError.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/*!
'use strict';

/* !
* Chai - checkError utility
* Copyright(c) 2012-2016 Jake Luer <jake@alogicalparadox.com>
* MIT Licensed
Expand All @@ -12,14 +14,6 @@
* @api public
*/

module.exports = {
compatibleInstance: compatibleInstance,
compatibleConstructor: compatibleConstructor,
compatibleMessage: compatibleMessage,
getMessage: getMessage,
getConstructorName: getConstructorName
};

/**
* ### .compatibleInstance(thrown, errorLike)
*
Expand All @@ -28,16 +22,18 @@ module.exports = {
*
* @name compatibleInstance
* @param {Error} thrown error
* @param {Error|ErrorConstructor} errorLike object to compare against
* @param {Error} errorLike object to compare against
* @namespace Utils
* @api public
*/

function compatibleInstance(thrown, errorLike) {
var isCompatible = false;
if (errorLike instanceof Error) {
return thrown === errorLike;
} else {
return true;
isCompatible = thrown === errorLike;
}

return isCompatible;
}

/**
Expand All @@ -55,6 +51,7 @@ function compatibleInstance(thrown, errorLike) {
* @namespace Utils
* @api public
*/

function compatibleConstructor(thrown, errorLike) {
if (errorLike instanceof Error) {
// If `errorLike` is an instance of any error we compare their constructors
Expand All @@ -80,53 +77,69 @@ function compatibleConstructor(thrown, errorLike) {
* @namespace Utils
* @api public
*/

function compatibleMessage(thrown, errMatcher) {
var comparisonString = typeof thrown === 'string' ? thrown : thrown.message;

if (errMatcher instanceof RegExp) {
return errMatcher.test(comparisonString);
} else if (typeof errMatcher === 'string') {
return comparisonString.indexOf(errMatcher) !== -1;
return comparisonString.indexOf(errMatcher) !== -1; // eslint-disable-line no-magic-numbers
}

return false;
}

/**
* ### .getConstructorName(err)
* ### .getConstructorName(errorLike)
*
* Gets the constructor name for an Error instance or constructor itself.
*
* @name getConstructorName
* @param {Error|ErrorConstructor} err
* @param {Error|ErrorConstructor} errorLike
* @namespace Utils
* @api public
*/
function getConstructorName(err) {
if (err instanceof Error) {
return err.constructor.name;
} else {

function getConstructorName(errorLike) {
var constructorName = errorLike;
if (errorLike instanceof Error) {
constructorName = errorLike.constructor.name;
} else if (typeof errorLike === 'function') {
// if `err` is not an instance of Error it is an error constructor itself
return typeof err === 'function' ? new err().name : err;
constructorName = new errorLike().name; // eslint-disable-line new-cap
}

return constructorName;
}

/**
* ### .getMessage(err)
* ### .getMessage(errorLike)
*
* Gets the error message from an error.
* If `err` is a String itself, we return it.
* If the error has no message, we return an empty string.
*
* @name getMessage
* @param {Error|String} err
* @param {Error|String} errorLike
* @namespace Utils
* @api public
*/
function getMessage(err) {
if (err && err.message) {
return err.message;
} else {
return typeof err === 'string' ? err : '';

function getMessage(errorLike) {
var msg = '';
if (errorLike && errorLike.message) {
msg = errorLike.message;
} else if (typeof errorLike === 'string') {
msg = errorLike;
}

return msg;
}

module.exports = {
compatibleInstance: compatibleInstance,
compatibleConstructor: compatibleConstructor,
compatibleMessage: compatibleMessage,
getMessage: getMessage,
getConstructorName: getConstructorName,
};
7 changes: 7 additions & 0 deletions test/expect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,9 @@ describe('expect', function () {
expect(badFn).to.throw(Error, /testing/);
expect(badFn).to.throw(Error, 'testing');

expect(badFn).to.not.throw(Error, 'I am the wrong error message');
expect(badFn).to.not.throw(TypeError, 'testing');

err(function(){
expect(goodFn).to.throw();
}, "expected [Function] to throw an error");
Expand Down Expand Up @@ -1171,6 +1174,10 @@ describe('expect', function () {
err(function () {
(customErrFn).should.not.throw();
}, "expected [Function] to not throw an error but 'CustomError: foo' was thrown");

err(function(){
expect(badFn).to.not.throw(Error, 'testing');
}, "expected [Function] to not throw 'Error' but 'Error: testing' was thrown");
});

it('respondTo', function(){
Expand Down
7 changes: 7 additions & 0 deletions test/should.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,9 @@ describe('should', function() {
should.throw(badFn, Error, /testing/);
should.throw(badFn, Error, 'testing');

(badFn).should.not.throw(Error, 'I am the wrong error message');
(badFn).should.not.throw(TypeError, 'testing');

err(function(){
(goodFn).should.throw();
}, "expected [Function] to throw an error");
Expand Down Expand Up @@ -1027,6 +1030,10 @@ describe('should', function() {
err(function () {
(customErrFn).should.not.throw();
}, "expected [Function] to not throw an error but 'CustomError: foo' was thrown");

err(function(){
(badFn).should.not.throw(Error, 'testing');
}, "expected [Function] to not throw 'Error' but 'Error: testing' was thrown");
});

it('respondTo', function(){
Expand Down
16 changes: 8 additions & 8 deletions test/utilities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,13 @@ describe('utilities', function () {
hp(1, arr).should.be.true;
hp(3, arr).should.be.false;
});

it('should handle literal types', function() {
var s = 'string literal';
hp('length', s).should.be.true;
hp(3, s).should.be.true;
hp(14, s).should.be.false;

hp('foo', 1).should.be.false;
});

Expand Down Expand Up @@ -767,13 +767,13 @@ describe('utilities', function () {

it('returns enumerable symbols only', function () {
if (typeof Symbol !== 'function') return;

var cat = Symbol('cat')
, dog = Symbol('dog')
, frog = Symbol('frog')
, cow = 'cow'
, obj = {};

obj[cat] = 'meow';
obj[dog] = 'woof';

Expand Down Expand Up @@ -816,14 +816,14 @@ describe('utilities', function () {

it('returns enumerable property names and symbols', function () {
if (typeof Symbol !== 'function') return;

var cat = Symbol('cat')
, dog = Symbol('dog')
, frog = Symbol('frog')
, bird = 'bird'
, cow = 'cow'
, obj = {};

obj[cat] = 'meow';
obj[dog] = 'woof';
obj[bird] = 'chirp';
Expand Down Expand Up @@ -859,8 +859,8 @@ describe('utilities', function () {

expect(checkError.compatibleInstance(errorInstance, sameInstance)).to.be.true;
expect(checkError.compatibleInstance(errorInstance, otherInstance)).to.be.false;
expect(checkError.compatibleInstance(errorInstance, Error)).to.be.true;
expect(checkError.compatibleInstance(errorInstance, aNumber)).to.be.true;
expect(checkError.compatibleInstance(errorInstance, Error)).to.be.false;
expect(checkError.compatibleInstance(errorInstance, aNumber)).to.be.false;
});

it('compatibleConstructor', function () {
Expand Down

0 comments on commit 7eed9ab

Please sign in to comment.