Fix throws assertion when using not and both args

lib/chai/core/assertions.js

@@ -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');
+    var negate = flag(this, 'negate') || false;
     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;
     }
 
@@ -1315,11 +1315,16 @@ 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;
+
+    // If we've got the negate flag enabled and both args, we should only fail if both aren't compatible
+    // See Issue #551 and PR #683@GitHub
+    var everyArgIsDefined = Boolean(errorLike && errMsgMatcher);
+    var errorLikeFail = false;
+    var errMsgMatcherFail = false;
 
     // 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) {
@@ -1338,48 +1343,75 @@ 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 (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
-          , '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()
-        );
+        var isCompatibleInstance = _.checkError.compatibleInstance(caughtErr, errorLike);
+
+        if (isCompatibleInstance === negate) {
+          // These checks were created to ensure we won't fail too soon when we've got both args and a negate
+          // See Issue #551 and PR #683@GitHub
+          if (everyArgIsDefined && negate) {
+            errorLikeFail = true;
+          } else {
+            this.assert(
+                negate
+              , '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)
-      );
+      var isCompatibleConstructor = _.checkError.compatibleConstructor(caughtErr, errorLike);
+      if (isCompatibleConstructor === negate) {
+        if (everyArgIsDefined && negate) {
+            errorLikeFail = true;
+        } else {
+          this.assert(
+              negate
+            , '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))
+          );
+        }
+      }
     }
 
-    // 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'
       }
 
+      var isCompatibleMessage = _.checkError.compatibleMessage(caughtErr, errMsgMatcher);
+      if (isCompatibleMessage === negate) {
+        if (everyArgIsDefined && negate) {
+            errMsgMatcherFail = true;
+        } else {
+          this.assert(
+            negate
+            , 'expected #{this} to throw error ' + placeholder + ' #{exp} but got #{act}'
+            , 'expected #{this} to throw error not ' + placeholder + ' #{exp}'
+            ,  errMsgMatcher
+            ,  _.checkError.getMessage(caughtErr)
+          );
+        }
+      }
+    }
+
+    // If both assertions failed and both should've matched we throw an error
+    if (errorLikeFail && errMsgMatcherFail) {
       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
+        negate
+        , '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))
       );
     }
 

lib/chai/utils/index.js

@@ -151,4 +151,4 @@ exports.getOwnEnumerableProperties = require('./getOwnEnumerableProperties');
  * Checks error against a given set of criteria
  */
 
-exports.checkError = require('./checkError');
+exports.checkError = require('check-error');

package.json

@@ -34,6 +34,7 @@
   },
   "dependencies": {
     "assertion-error": "^1.0.1",
+    "check-error": "^1.0.1",
     "deep-eql": "^0.1.3",
     "type-detect": "^1.0.0"
   },

test/expect.js

@@ -1104,6 +1104,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");
@@ -1175,6 +1178,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(){

test/should.js

@@ -944,6 +944,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");
@@ -1031,6 +1034,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(){

test/utilities.js

@@ -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;
     });
 
@@ -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';
 
@@ -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';