Skip to content

Commit

Permalink
fix: .include to work with all objects
Browse files Browse the repository at this point in the history
Overly strict type-checking was causing `.include` to reject `Error`
objects and objects with a custom `@@toStringTag`.
  • Loading branch information
meeber committed Jul 31, 2017
1 parent 1847ef8 commit c01cf30
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 48 deletions.
104 changes: 56 additions & 48 deletions lib/chai/core/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -469,53 +469,18 @@ module.exports = function (chai, _) {

function include (val, msg) {
if (msg) flag(this, 'message', msg);

_.expectTypes(this, [
'array', 'object', 'string',
'map', 'set', 'weakset',
]);


var obj = flag(this, 'object')
, objType = _.type(obj).toLowerCase();

// This block is for asserting a subset of properties in an object.
if (objType === 'object') {
var props = Object.keys(val)
, negate = flag(this, 'negate')
, firstErr = null
, numErrs = 0;

props.forEach(function (prop) {
var propAssertion = new Assertion(obj);
_.transferFlags(this, propAssertion, true);
flag(propAssertion, 'lockSsfi', true);

if (!negate || props.length === 1) {
propAssertion.property(prop, val[prop]);
return;
}

try {
propAssertion.property(prop, val[prop]);
} catch (err) {
if (!_.checkError.compatibleConstructor(err, AssertionError)) throw err;
if (firstErr === null) firstErr = err;
numErrs++;
}
}, this);

// When validating .not.include with multiple properties, we only want
// to throw an assertion error if all of the properties are included,
// in which case we throw the first property assertion error that we
// encountered.
if (negate && props.length > 1 && numErrs === props.length) throw firstErr;
, objType = _.type(obj).toLowerCase()
, flagMsg = flag(this, 'message')
, negate = flag(this, 'negate')
, ssfi = flag(this, 'ssfi')
, isDeep = flag(this, 'deep')
, descriptor = isDeep ? 'deep ' : '';

return;
}
flagMsg = flagMsg ? flagMsg + ': ' : '';

var isDeep = flag(this, 'deep')
, descriptor = isDeep ? 'deep ' : ''
, included = false;
var included = false;

switch (objType) {
case 'string':
Expand All @@ -524,10 +489,6 @@ module.exports = function (chai, _) {

case 'weakset':
if (isDeep) {
var flagMsg = flag(this, 'message')
, ssfi = flag(this, 'ssfi');
flagMsg = flagMsg ? flagMsg + ': ' : '';

throw new AssertionError(
flagMsg + 'unable to use .deep.include with WeakSet',
undefined,
Expand Down Expand Up @@ -564,6 +525,53 @@ module.exports = function (chai, _) {
included = obj.indexOf(val) !== -1;
}
break;

default:
// This block is for asserting a subset of properties in an object.
// `_.expectTypes` isn't used here because `.include` should work with
// objects with a custom `@@toStringTag`.
if (val !== Object(val)) {
throw new AssertionError(
flagMsg + 'object tested must be an array, a map, an object,'
+ ' a set, a string, or a weakset, but ' + objType + ' given',
undefined,
ssfi
);
}

var props = Object.keys(val)
, firstErr = null
, numErrs = 0;

props.forEach(function (prop) {
var propAssertion = new Assertion(obj);
_.transferFlags(this, propAssertion, true);
flag(propAssertion, 'lockSsfi', true);

if (!negate || props.length === 1) {
propAssertion.property(prop, val[prop]);
return;
}

try {
propAssertion.property(prop, val[prop]);
} catch (err) {
if (!_.checkError.compatibleConstructor(err, AssertionError)) {
throw err;
}
if (firstErr === null) firstErr = err;
numErrs++;
}
}, this);

// When validating .not.include with multiple properties, we only want
// to throw an assertion error if all of the properties are included,
// in which case we throw the first property assertion error that we
// encountered.
if (negate && props.length > 1 && numErrs === props.length) {
throw firstErr;
}
return;
}

// Assert inclusion in collection or substring in a string.
Expand Down
11 changes: 11 additions & 0 deletions test/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,17 @@ describe('assert', function () {
assert.include('', '');
assert.include([ 1, 2, 3], 3);

// .include should work with Error objects and objects with a custom
// `@@toStringTag`.
assert.include(new Error('foo'), {message: 'foo'});
if (typeof Symbol !== 'undefined'
&& typeof Symbol.toStringTag !== 'undefined') {
var customObj = {a: 1};
customObj[Symbol.toStringTag] = 'foo';

assert.include(customObj, {a: 1});
}

var obj1 = {a: 1}
, obj2 = {b: 2};
assert.include([obj1, obj2], obj1);
Expand Down
11 changes: 11 additions & 0 deletions test/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -1884,6 +1884,17 @@ describe('expect', function () {

expect({a: 1}).to.include({'toString': Object.prototype.toString});

// .include should work with Error objects and objects with a custom
// `@@toStringTag`.
expect(new Error('foo')).to.include({message: 'foo'});
if (typeof Symbol !== 'undefined'
&& typeof Symbol.toStringTag !== 'undefined') {
var customObj = {a: 1};
customObj[Symbol.toStringTag] = 'foo';

expect(customObj).to.include({a: 1});
}

var obj1 = {a: 1}
, obj2 = {b: 2};
expect([obj1, obj2]).to.include(obj1);
Expand Down
11 changes: 11 additions & 0 deletions test/should.js
Original file line number Diff line number Diff line change
Expand Up @@ -1540,6 +1540,17 @@ describe('should', function() {
['foo', 'bar'].should.not.include('baz');
['foo', 'bar'].should.not.include(1);

// .include should work with Error objects and objects with a custom
// `@@toStringTag`.
(new Error('foo')).should.include({message: 'foo'});
if (typeof Symbol !== 'undefined'
&& typeof Symbol.toStringTag !== 'undefined') {
var customObj = {a: 1};
customObj[Symbol.toStringTag] = 'foo';

customObj.should.include({a: 1});
}

({a: 1}).should.include({'toString': Object.prototype.toString});

var obj1 = {a: 1}
Expand Down

0 comments on commit c01cf30

Please sign in to comment.