Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Convert .throw to chainable method #1

Open
wants to merge 1 commit into
base: overhaul-throw
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 36 additions & 28 deletions lib/chai/core/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -2532,14 +2532,16 @@ module.exports = function (chai, _) {
}

/**
* ### .throw
* ### .throw([errLike[, errMsgMatcher[, msg]]])
* ### .throw([errMsgMatcher[, msg]])
*
* When invoked without any arguments, `.throw` invokes the target function
* and asserts that it throws.
* When used as a property assertion or when invoked without any arguments,
* `.throw` invokes the target function and asserts that it throws.
*
* var badFn = function () { throw TypeError('Illegal salmon!'); };
*
* expect(badFn).to.throw;
* expect(badFn).to.throw();
*
* When invoked with one argument, and it's a function (typically a
Expand Down Expand Up @@ -2602,19 +2604,19 @@ module.exports = function (chai, _) {
*
* expect(badFn).to.throw(TypeError).with.property('code', 42);
*
* When invoked without any arguments, `.throw` can be negated by adding
* `.not` earlier in the chain. However, `.throw` cannot be negated when
* invoking it with one or more arguments. Doing so would create uncertain
* expectations by asserting that the target function either doesn't throw, or
* that it throws but the thrown value doesn't match the criteria described by
* one or more of the arguments.
* When used as a property assertion or when invoked without any arguments,
* `.throw` can be negated by adding `.not` earlier in the chain. However,
* `.throw` cannot be negated when invoking it with one or more arguments.
* Doing so would create uncertain expectations by asserting that the target
* function either doesn't throw, or that it throws but the thrown value
* doesn't match the criteria described by one or more of the arguments.
*
* When the target function isn't expected to throw, it's best to assert
* exactly that.
*
* var goodFn = function () {};
*
* expect(goodFn).to.not.throw();
* expect(goodFn).to.not.throw;
*
* When the target function is expected to throw, it's often best to assert
* that it throws a value that matches some criteria, rather than asserting
Expand All @@ -2626,7 +2628,7 @@ module.exports = function (chai, _) {
* expect(badFn).to.throw(TypeError('Illegal salmon!')); // Recommended
* expect(badFn).to.not.throw(ReferenceError); // Not supported
* // Supported but not recommended:
* expect(badFn).to.throw().but.not.an.instanceof(ReferenceError);
* expect(badFn).to.throw.but.not.an.instanceof(ReferenceError);
*
* `.throw` accepts an optional `msg` argument which is a custom error message
* to show when the assertion fails. The message can also be given as the
Expand All @@ -2636,7 +2638,7 @@ module.exports = function (chai, _) {
* var goodFn = function () {};
*
* expect(goodFn).to.throw(TypeError, 'x', 'nooo why fail??');
* expect(goodFn, 'nooo why fail??').to.throw();
* expect(goodFn, 'nooo why fail??').to.throw;
*
* Due to limitations in ES5, using `.throw` with arguments may not always
* work as expected when using a transpiler such as Babel or TypeScript. In
Expand All @@ -2653,14 +2655,14 @@ module.exports = function (chai, _) {
* `fn` throws, provide `fn` instead of `fn()` as the target for the
* assertion.
*
* expect(fn).to.throw(); // Good! Tests `fn` as desired
* expect(fn()).to.throw(); // Bad! Tests result of `fn()`, not `fn`
* expect(fn).to.throw; // Good! Tests `fn` as desired
* expect(fn()).to.throw; // Bad! Tests result of `fn()`, not `fn`
*
* If you need to assert that your function `fn` throws when passed certain
* arguments, then wrap a call to `fn` inside of another function.
*
* expect(function () { fn(42); }).to.throw(); // Function expression
* expect(() => fn(42)).to.throw(); // ES6 arrow function
* expect(function () { fn(42); }).to.throw; // Function expression
* expect(() => fn(42)).to.throw; // ES6 arrow function
*
* Another common mistake is to provide an object method (or any stand-alone
* function that relies on `this`) as the target of the assertion. Doing so is
Expand All @@ -2670,9 +2672,9 @@ module.exports = function (chai, _) {
* method or function call inside of another function. Another solution is to
* use `bind`.
*
* expect(function () { cat.meow(); }).to.throw(); // Function expression
* expect(() => cat.meow()).to.throw(); // ES6 arrow function
* expect(cat.meow.bind(cat)).to.throw(); // Bind
* expect(function () { cat.meow(); }).to.throw; // Function expression
* expect(() => cat.meow()).to.throw; // ES6 arrow function
* expect(cat.meow.bind(cat)).to.throw; // Bind
*
* Finally, it's worth mentioning that it's a best practice in JavaScript to
* only throw `Error` instances. This includes `Error` instances created from
Expand Down Expand Up @@ -2708,14 +2710,10 @@ module.exports = function (chai, _) {
* @api public
*/

function assertThrows (errLike, errMsgMatcher, msg) {
if (msg) flag(this, 'message', msg);
function assertThrows () {
var obj = flag(this, 'object');
var ssfi = flag(this, 'ssfi');
var flagMsg = flag(this, 'message');
var negate = flag(this, 'negate');

var cri = createErrCriteria(errLike, errMsgMatcher, negate);

new Assertion(obj, flagMsg, ssfi, true).is.a('function');

Expand All @@ -2735,24 +2733,34 @@ module.exports = function (chai, _) {
wasErrThrown,
'expected #{this} to throw',
'expected #{this} to not throw but #{act} was thrown',
errLike || errMsgMatcher,
wasErrThrown,
caughtErr
);

// Set the caught error as the target of future assertions.
flag(this, 'object', caughtErr);
}

function assertThrowsCriteria (errLike, errMsgMatcher, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object');
var ssfi = flag(this, 'ssfi');
var flagMsg = flag(this, 'message');
var negate = flag(this, 'negate');

var cri = createErrCriteria(errLike, errMsgMatcher, negate);

assertErrCriteria(
caughtErr,
obj,
cri,
flagMsg,
ssfi
);
}

Assertion.addMethod('throw', assertThrows);
Assertion.addMethod('throws', assertThrows);
Assertion.addMethod('Throw', assertThrows);
Assertion.addChainableMethod('throw', assertThrowsCriteria, assertThrows);
Assertion.addChainableMethod('throws', assertThrowsCriteria, assertThrows);
Assertion.addChainableMethod('Throw', assertThrowsCriteria, assertThrows);

/**
* ### .respondTo(method[, msg])
Expand Down
26 changes: 24 additions & 2 deletions test/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -2992,6 +2992,28 @@ describe('expect', function () {
var badFnObj = function () { throw {a: 1}; };
var badFnStr = function () { throw "Illegal salmon!"; };

describe("used as a property-based assertion", function () {
describe("normal", function () {
it("passes when target throws", function () {
expect(badFnErr).to.throw;
});
it("fails when target doesn't throw", function () {
err(function () {
expect(goodFn, 'blah').to.throw;
}, /^blah: expected \[Function(: goodFn)*\] to throw$/);
});
});
describe("negated", function () {
it("passes when target doesn't throw", function () {
expect(goodFn).to.not.throw;
});
it("fails when target throws", function () {
err(function () {
expect(badFnErr, 'blah').to.not.throw;
}, /^blah: expected \[Function(: badFnErr)*\] to not throw but \[Error: Illegal salmon!\] was thrown$/);
});
});
});
describe("invoked with no args", function () {
describe("normal", function () {
it("passes when target throws", function () {
Expand Down Expand Up @@ -3204,12 +3226,12 @@ describe('expect', function () {
describe("negated", function () {
it("fails when errLike is defined", function () {
err(function () {
expect(badFnErr).to.not.throw(TypeError);
expect(goodFn).to.not.throw(TypeError);
}, /^errLike and errMsgMatcher must both be undefined when negate is true$/, true);
});
it("fails when errMsgMatcher is defined", function () {
err(function () {
expect(badFnErr).to.not.throw(undefined, 'salmon');
expect(goodFn).to.not.throw(undefined, 'salmon');
}, /^errLike and errMsgMatcher must both be undefined when negate is true$/, true);
});
});
Expand Down