Repository: chaijs/chai
Pull request #742 (2016-06-26 18:43:07)

Throws polishment assert interface
Hello guys!
This includes polishments for the `Assert` interface due to the changes on the `throws` assertion.
I have also updated the docStrings for each one of the `throws` related methods in order to make sense with the new behavior, but since I'm not a native english speaker I would appreciate if you guys could review it carefully just to make sure everything is correct.

While doing this I also noticed that we had problems in the way we were handling empty strings for errors, so I included that in a separate commit. Now we do an explicit check to see if the `caughtErr` or the `errMsgMatcher` are `null` or `undefined`, because empty Strings evaluate to `false`.

I was also thinking if we should handle throwing raw empty Strings or other kinds of objects instead of `Error` objects, what do you think? Should we handle those too?

--------------------------
**Edit**: I also improved the DocStrings for the `.throws` assertion explaining which kind of comparison is used for each type of argument.


Patch:
diff --git a/README.md b/README.md
index 015c545..5b7353d 100644
--- a/README.md
+++ b/README.md
@@ -15,7 +15,7 @@
 
 [![Slack Status](https://chai-slack.herokuapp.com/badge.svg)]( https://chai-slack.herokuapp.com/)
 [![Join the chat at https://gitter.im/chaijs/chai](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/chaijs/chai)
-[![OpenCollective](https://opencollective.com/chaijs/backers/badge.svg)](https://opencollective.com/chaijs) 
+[![OpenCollective](https://opencollective.com/chaijs/backers/badge.svg)](#backers) 
 
 
 Chai is a BDD / TDD assertion library for [node](http://nodejs.org) and the browser that
diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js
index 6c3d109..3e0eba2 100644
--- a/lib/chai/core/assertions.js
+++ b/lib/chai/core/assertions.js
@@ -1285,8 +1285,12 @@ module.exports = function (chai, _) {
    * ### .throw(constructor)
    *
    * Asserts that the function target will throw a specific error, or specific type of error
-   * (as determined using `instanceof`), optionally with a RegExp or string inclusion test
+   * (as determined using `instanceof`), optionally with a RegExp or String inclusion test
    * for the error's message.
+   * If an Error instance is provided, it asserts that the error thrown and the instance
+   * provided are the same.
+   * If an Error constructor is provided, it asserts that the error thrown is an instance
+   * of the constructor provided.
    *
    *     var err = new ReferenceError('This is a bad function.');
    *     var fn = function () { throw err; }
@@ -1351,7 +1355,7 @@ module.exports = function (chai, _) {
       if (errorLike instanceof Error) {
         errorLikeString = '#{exp}';
       } else if (errorLike) {
-        errorLikeString = _.checkError.getConstructorName(errorLike);
+        errorLikeString = errorLike.name;
       }
 
       this.assert(
@@ -1360,8 +1364,7 @@ module.exports = function (chai, _) {
         , 'expected #{this} to not throw an error but #{act} was thrown'
         , errorLike && errorLike.toString()
         , (caughtErr instanceof Error ?
-            caughtErr.toString() : (typeof caughtErr === 'string' ? caughtErr : caughtErr &&
-                                    _.checkError.getConstructorName(caughtErr)))
+            caughtErr.toString() : (typeof caughtErr === 'string' ? caughtErr : caughtErr && caughtErr.name))
       );
     }
 
@@ -1403,7 +1406,7 @@ module.exports = function (chai, _) {
       }
     }
 
-    if (errMsgMatcher) {
+    if (caughtErr && errMsgMatcher !== undefined && errMsgMatcher !== null) {
       // Here we check compatible messages
       var placeholder = 'including';
       if (errMsgMatcher instanceof RegExp) {
diff --git a/lib/chai/interface/assert.js b/lib/chai/interface/assert.js
index 1acdb3b..850ac9f 100644
--- a/lib/chai/interface/assert.js
+++ b/lib/chai/interface/assert.js
@@ -1232,66 +1232,82 @@ module.exports = function (chai, util) {
   }
 
   /**
-   * ### .throws(function, [constructor/string/regexp], [string/regexp], [message])
+   * ### .throws(fn, [errorLike/string/regexp], [string/regexp], [message])
    *
-   * Asserts that `function` will throw an error that is an instance of
-   * `constructor`, or alternately that it will throw an error with message
-   * matching `regexp`.
+   * If `errorLike` is an `Error` constructor, asserts that `fn` will throw an error that is an
+   * instance of `errorLike`.
+   * If `errorLike` is an `Error` instance, asserts that the error thrown is the same
+   * instance as `errorLike`.
+   * If `errMsgMatcher` is provided, it also asserts that the error thrown will have a
+   * message matching `errMsgMatcher`.
    *
    *     assert.throws(fn, 'function throws a reference error');
    *     assert.throws(fn, /function throws a reference error/);
    *     assert.throws(fn, ReferenceError);
-   *     assert.throws(fn, ReferenceError, 'function throws a reference error');
-   *     assert.throws(fn, ReferenceError, /function throws a reference error/);
+   *     assert.throws(fn, errorInstance);
+   *     assert.throws(fn, ReferenceError, 'Error thrown must be a ReferenceError and have this msg');
+   *     assert.throws(fn, errorInstance, 'Error thrown must be the same errorInstance and have this msg');
+   *     assert.throws(fn, ReferenceError, /Error thrown must be a ReferenceError and match this/);
+   *     assert.throws(fn, errorInstance, /Error thrown must be the same errorInstance and match this/);
    *
    * @name throws
    * @alias throw
    * @alias Throw
-   * @param {Function} function
-   * @param {ErrorConstructor} constructor
-   * @param {RegExp} regexp
+   * @param {Function} fn
+   * @param {ErrorConstructor/Error} errorLike
+   * @param {RegExp/String} errMsgMatcher
    * @param {String} message
    * @see https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Error#Error_types
    * @namespace Assert
    * @api public
    */
 
-  assert.throws = function (fn, errt, errs, msg) {
-    if ('string' === typeof errt || errt instanceof RegExp) {
-      errs = errt;
-      errt = null;
+  assert.throws = function (fn, errorLike, errMsgMatcher, msg) {
+    if ('string' === typeof errorLike || errorLike instanceof RegExp) {
+      errMsgMatcher = errorLike;
+      errorLike = null;
     }
 
-    var assertErr = new Assertion(fn, msg).to.throw(errt, errs);
+    var assertErr = new Assertion(fn, msg).to.throw(errorLike, errMsgMatcher);
     return flag(assertErr, 'object');
   };
 
   /**
-   * ### .doesNotThrow(function, [constructor/regexp], [message])
+   * ### .doesNotThrow(fn, [errorLike/string/regexp], [string/regexp], [message])
    *
-   * Asserts that `function` will _not_ throw an error that is an instance of
-   * `constructor`, or alternately that it will not throw an error with message
-   * matching `regexp`.
+   * If `errorLike` is an `Error` constructor, asserts that `fn` will _not_ throw an error that is an
+   * instance of `errorLike`.
+   * If `errorLike` is an `Error` instance, asserts that the error thrown is _not_ the same
+   * instance as `errorLike`.
+   * If `errMsgMatcher` is provided, it also asserts that the error thrown will _not_ have a
+   * message matching `errMsgMatcher`.
    *
-   *     assert.doesNotThrow(fn, Error, 'function does not throw');
+   *     assert.doesNotThrow(fn, 'Any Error thrown must not have this message');
+   *     assert.doesNotThrow(fn, /Any Error thrown must not match this/);
+   *     assert.doesNotThrow(fn, Error);
+   *     assert.doesNotThrow(fn, errorInstance);
+   *     assert.doesNotThrow(fn, Error, 'Error must not have this message');
+   *     assert.doesNotThrow(fn, errorInstance, 'Error must not have this message');
+   *     assert.doesNotThrow(fn, Error, /Error must not match this/);
+   *     assert.doesNotThrow(fn, errorInstance, /Error must not match this/);
    *
    * @name doesNotThrow
-   * @param {Function} function
-   * @param {ErrorConstructor} constructor
-   * @param {RegExp} regexp
+   * @param {Function} fn
+   * @param {ErrorConstructor} errorLike
+   * @param {RegExp/String} errMsgMatcher
    * @param {String} message
    * @see https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Error#Error_types
    * @namespace Assert
    * @api public
    */
 
-  assert.doesNotThrow = function (fn, type, msg) {
-    if ('string' === typeof type) {
-      msg = type;
-      type = null;
+  assert.doesNotThrow = function (fn, errorLike, errMsgMatcher, msg) {
+    if ('string' === typeof errorLike || errorLike instanceof RegExp) {
+      errMsgMatcher = errorLike;
+      errorLike = null;
     }
 
-    new Assertion(fn, msg).to.not.Throw(type);
+    new Assertion(fn, msg).to.not.throw(errorLike, errMsgMatcher);
   };
 
   /**


Test patch:
diff --git a/test/assert.js b/test/assert.js
index bf5d763..bb13a5a 100644
--- a/test/assert.js
+++ b/test/assert.js
@@ -989,10 +989,13 @@ describe('assert', function () {
   it('throws / throw / Throw', function() {
     ['throws', 'throw', 'Throw'].forEach(function (throws) {
       assert[throws](function() { throw new Error('foo'); });
+      assert[throws](function() { throw new Error(''); }, '');
       assert[throws](function() { throw new Error('bar'); }, 'bar');
       assert[throws](function() { throw new Error('bar'); }, /bar/);
       assert[throws](function() { throw new Error('bar'); }, Error);
       assert[throws](function() { throw new Error('bar'); }, Error, 'bar');
+      assert[throws](function() { throw new Error(''); }, Error, '');
+      assert[throws](function() { throw new Error('foo') }, '');
 
       var thrownErr = assert[throws](function() { throw new Error('foo'); });
       assert(thrownErr instanceof Error, 'assert.' + throws + ' returns error');
@@ -1000,30 +1003,30 @@ describe('assert', function () {
 
       err(function () {
         assert[throws](function() { throw new Error('foo') }, TypeError);
-       }, "expected [Function] to throw 'TypeError' but 'Error: foo' was thrown")
+      }, "expected [Function] to throw 'TypeError' but 'Error: foo' was thrown")
 
       err(function () {
         assert[throws](function() { throw new Error('foo') }, 'bar');
-       }, "expected [Function] to throw error including 'bar' but got 'foo'")
+      }, "expected [Function] to throw error including 'bar' but got 'foo'")
 
       err(function () {
         assert[throws](function() { throw new Error('foo') }, Error, 'bar');
-       }, "expected [Function] to throw error including 'bar' but got 'foo'")
+      }, "expected [Function] to throw error including 'bar' but got 'foo'")
 
       err(function () {
         assert[throws](function() { throw new Error('foo') }, TypeError, 'bar');
-       }, "expected [Function] to throw 'TypeError' but 'Error: foo' was thrown")
+      }, "expected [Function] to throw 'TypeError' but 'Error: foo' was thrown")
 
       err(function () {
         assert[throws](function() {});
-       }, "expected [Function] to throw an error");
+      }, "expected [Function] to throw an error");
 
       err(function () {
-          assert[throws](function() { throw new Error('') }, 'bar');
+        assert[throws](function() { throw new Error('') }, 'bar');
       }, "expected [Function] to throw error including 'bar' but got ''");
 
       err(function () {
-          assert[throws](function() { throw new Error('') }, /bar/);
+        assert[throws](function() { throw new Error('') }, /bar/);
       }, "expected [Function] to throw error matching /bar/ but got ''");
     });
   });
@@ -1033,18 +1036,79 @@ describe('assert', function () {
         this.name = 'CustomError';
         this.message = message;
     }
-    CustomError.prototype = Error.prototype;
+    CustomError.prototype = Object.create(Error.prototype);
 
     assert.doesNotThrow(function() { });
     assert.doesNotThrow(function() { }, 'foo');
+    assert.doesNotThrow(function() { }, '');
+
+    assert.doesNotThrow(function() {
+      throw new Error('This is a message');
+    }, TypeError);
+
+    assert.doesNotThrow(function() {
+      throw new Error('This is a message');
+    }, 'Another message');
+
+    assert.doesNotThrow(function() {
+      throw new Error('This is a message');
+    }, /Another message/);
+
+    assert.doesNotThrow(function() {
+      throw new Error('This is a message');
+    }, Error, 'Another message');
+
+    assert.doesNotThrow(function() {
+      throw new Error('This is a message');
+    }, Error, /Another message/);
+
+    assert.doesNotThrow(function() {
+      throw new Error('This is a message');
+    }, TypeError, 'Another message');
+
+    assert.doesNotThrow(function() {
+      throw new Error('This is a message');
+    }, TypeError, /Another message/);
 
     err(function () {
       assert.doesNotThrow(function() { throw new Error('foo'); });
-     }, "expected [Function] to not throw an error but 'Error: foo' was thrown");
+    }, "expected [Function] to not throw an error but 'Error: foo' was thrown");
 
     err(function () {
-        assert.doesNotThrow(function() { throw new CustomError('foo'); });
+      assert.doesNotThrow(function() { throw new CustomError('foo'); });
     }, "expected [Function] to not throw an error but 'CustomError: foo' was thrown");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new Error('foo'); }, Error);
+    }, "expected [Function] to not throw 'Error' but 'Error: foo' was thrown");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new CustomError('foo'); }, CustomError);
+    }, "expected [Function] to not throw 'CustomError' but 'CustomError: foo' was thrown");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new Error('foo'); }, 'foo');
+    }, "expected [Function] to throw error not including 'foo'");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new Error('foo'); }, /foo/);
+    }, "expected [Function] to throw error not matching /foo/");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new Error('foo'); }, Error, 'foo');
+    }, "expected [Function] to not throw 'Error' but 'Error: foo' was thrown");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new CustomError('foo'); }, CustomError, 'foo');
+    }, "expected [Function] to not throw 'CustomError' but 'CustomError: foo' was thrown");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new Error(''); }, '');
+    }, "expected [Function] to throw error not including ''");
+
+    err(function () {
+      assert.doesNotThrow(function() { throw new Error(''); }, Error, '');
+    }, "expected [Function] to not throw 'Error' but 'Error' was thrown");
   });
 
   it('ifError', function() {
diff --git a/test/expect.js b/test/expect.js
index 5706b41..4020777 100644
--- a/test/expect.js
+++ b/test/expect.js
@@ -1160,7 +1160,9 @@ describe('expect', function () {
       , refErrFn = function () { throw new ReferenceError('hello'); }
       , ickyErrFn = function () { throw new PoorlyConstructedError(); }
       , specificErrFn = function () { throw specificError; }
-      , customErrFn = function() { throw new CustomError('foo'); };
+      , customErrFn = function() { throw new CustomError('foo'); }
+      , emptyErrFn = function () { throw new Error(); }
+      , emptyStringErrFn = function () { throw new Error(''); };
 
     expect(goodFn).to.not.throw();
     expect(goodFn).to.not.throw(Error);
@@ -1180,13 +1182,19 @@ describe('expect', function () {
     expect(ickyErrFn).to.not.throw(specificError);
     expect(specificErrFn).to.throw(specificError);
 
+    expect(goodFn).to.not.throw('testing');
+    expect(goodFn).to.not.throw(/testing/);
     expect(badFn).to.throw(/testing/);
     expect(badFn).to.not.throw(/hello/);
     expect(badFn).to.throw('testing');
     expect(badFn).to.not.throw('hello');
+    expect(emptyStringErrFn).to.throw('');
+    expect(emptyStringErrFn).to.not.throw('testing');
+    expect(badFn).to.throw('');
 
     expect(badFn).to.throw(Error, /testing/);
     expect(badFn).to.throw(Error, 'testing');
+    expect(emptyErrFn).to.not.throw(Error, 'testing');
 
     expect(badFn).to.not.throw(Error, 'I am the wrong error message');
     expect(badFn).to.not.throw(TypeError, 'testing');
@@ -1266,6 +1274,14 @@ describe('expect', function () {
     err(function(){
       expect(badFn).to.not.throw(Error, 'testing');
     }, "expected [Function] to not throw 'Error' but 'Error: testing' was thrown");
+
+    err(function(){
+      expect(emptyStringErrFn).to.not.throw(Error, '');
+    }, "expected [Function] to not throw 'Error' but 'Error' was thrown");
+
+    err(function(){
+      expect(emptyStringErrFn).to.not.throw('');
+    }, "expected [Function] to throw error not including ''");
   });
 
   it('respondTo', function(){
diff --git a/test/should.js b/test/should.js
index 0aec0d6..4f5645b 100644
--- a/test/should.js
+++ b/test/should.js
@@ -981,7 +981,9 @@ describe('should', function() {
       , refErrFn = function () { throw new ReferenceError('hello'); }
       , ickyErrFn = function () { throw new PoorlyConstructedError(); }
       , specificErrFn = function () { throw specificError; }
-      , customErrFn = function() { throw new CustomError('foo'); };
+      , customErrFn = function () { throw new CustomError('foo'); }
+      , emptyErrFn = function () { throw new Error(); }
+      , emptyStringErrFn = function () { throw new Error(''); };
 
     (goodFn).should.not.throw();
     (goodFn).should.not.throw(Error);
@@ -1004,12 +1006,19 @@ describe('should', function() {
     (ickyErrFn).should.not.throw(specificError);
     (specificErrFn).should.throw(specificError);
 
+    (goodFn).should.not.throw('testing');
+    (goodFn).should.not.throw(/testing/);
     (badFn).should.throw(/testing/);
     (badFn).should.throw('testing');
     (badFn).should.not.throw(/hello/);
     (badFn).should.not.throw('hello');
+    (emptyStringErrFn).should.throw('');
+    (emptyStringErrFn).should.not.throw('testing');
+    (badFn).should.throw('');
+
     (badFn).should.throw(Error, /testing/);
     (badFn).should.throw(Error, 'testing');
+    (emptyErrFn).should.not.throw(Error, 'testing');
 
     (stringErrFn).should.throw(/testing/);
     (stringErrFn).should.throw('testing');
@@ -1122,6 +1131,14 @@ describe('should', function() {
     err(function(){
       (badFn).should.not.throw(Error, 'testing');
     }, "expected [Function] to not throw 'Error' but 'Error: testing' was thrown");
+
+    err(function(){
+      (emptyStringErrFn).should.not.throw(Error, '');
+    }, "expected [Function] to not throw 'Error' but 'Error' was thrown");
+
+    err(function(){
+      (emptyStringErrFn).should.not.throw('');
+    }, "expected [Function] to throw error not including ''");
   });
 
   it('respondTo', function(){