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.

Sign up for GitHub

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

Jump to bottom

Add ES6 collection support to include() #994

Merged
merged 7 commits into from
Jun 23, 2017
Merged

Add ES6 collection support to include() #994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
07de33e
fix error messages tests
shvaikalesh Jun 21, 2017
86a9ea3
add tests
shvaikalesh Jun 21, 2017
4d56c3d
add implementation
shvaikalesh Jun 21, 2017
0e809c3
performance tweaks
shvaikalesh Jun 22, 2017
defc699
add tests for SameValueZero
shvaikalesh Jun 22, 2017
039564d
drop weakmap support
shvaikalesh Jun 23, 2017
df6badd
update docs
shvaikalesh Jun 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 76 additions & 15 deletions lib/chai/core/assertions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,16 @@ module.exports = function (chai, _) {
*
* expect({a: 1, b: 2, c: 3}).to.include({a: 1, b: 2});
*
* When the target is a Set or WeakSet, `.include` asserts that the given `val` is a
* member of the target. SameValueZero equality algorithm is used.
*
* expect(new Set([1, 2])).to.include(2);
*
* When the target is a Map, `.include` asserts that the given `val` is one of
* the values of the target. SameValueZero equality algorithm is used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to explain how the SameValueZero algorithm works. I think many people could get confused by that and would end up either having to read the code to understand it or just avoiding using this assertion because they're not sure of what it does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucasfcosta Fair point. In this case, I would suggest moving the phrase "SameValueZero equality algorithm is used" from where it is currently down to the separate paragraph where strict and deep equality` is discussed. This would make the doc style more consistent anyway. And then rather than explain what "SameValueZero equality" is, we could provide a link to an explanation, just like we do with the "deep equal" algorithm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just refer to is as it is more commonly known; Object.is or if you'd like to we coud keep the phrasing but link to the equality article on MDN which mentions SameValueZero.

*
* expect(new Map([['a', 1], ['b', 2]])).to.include(2);
*
* Because `.include` does different things based on the target's type, it's
* important to check the target's type before using `.include`. See the `.a`
* doc for info on testing a target's type.
Expand All @@ -338,8 +348,8 @@ module.exports = function (chai, _) {
*
* By default, strict (`===`) equality is used to compare array members and
* object properties. Add `.deep` earlier in the chain to use deep equality
* instead. See the `deep-eql` project page for info on the deep equality
* algorithm: https://github.com/chaijs/deep-eql.
* instead (WeakSet targets are not supported). See the `deep-eql` project
* page for info on the deep equality algorithm: https://github.com/chaijs/deep-eql.
*
* // Target array deeply (but not strictly) includes `{a: 1}`
* expect([{a: 1}]).to.deep.include({a: 1});
Expand Down Expand Up @@ -449,25 +459,24 @@ module.exports = function (chai, _) {
* @api public
*/

function includeChainingBehavior () {
flag(this, 'contains', true);
function SameValueZero(a, b) {
return (_.isNaN(a) && _.isNaN(b)) || a === b;
}

function isDeepIncluded (arr, val) {
return arr.some(function (arrVal) {
return _.eql(arrVal, val);
});
function includeChainingBehavior () {
flag(this, 'contains', true);
}

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

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

var obj = flag(this, 'object')
, objType = _.type(obj).toLowerCase()
, isDeep = flag(this, 'deep')
, descriptor = isDeep ? 'deep ' : '';
, objType = _.type(obj).toLowerCase();

// This block is for asserting a subset of properties in an object.
if (objType === 'object') {
Expand Down Expand Up @@ -504,10 +513,62 @@ module.exports = function (chai, _) {
return;
}

// Assert inclusion in an array or substring in a string.
var isDeep = flag(this, 'deep')
, descriptor = isDeep ? 'deep ' : ''
, included = false;

switch (objType) {
case 'string':
included = obj.indexOf(val) !== -1;
break;

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,
ssfi
);
}

included = obj.has(val);
break;

case 'map':
var isEql = isDeep ? _.eql : SameValueZero;
obj.forEach(function (item) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @vieiralucas said, we might be able to optimize this by getting out of the loop when we found the included item.

In order to support IE and older versions of it we can throw an error inside the forEach callback and catch it.

This is a cross-platform compatible implementation I've made on Sinon for the every method. We can inspire our some implementation in that. However, even though I'd like it to be done, I won't block merging this PR because of that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucasfcosta I think this is worth exploring. I'd prefer this kind of optimization technique be implemented in a separate PR as a refactor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meeber agreed.

LGTM! Let's get this merged.

included = included || isEql(item, val);
});
break;

case 'set':
if (isDeep) {
obj.forEach(function (item) {
included = included || _.eql(item, val);
});
} else {
included = obj.has(val);
}
break;

case 'array':
if (isDeep) {
included = obj.some(function (item) {
return _.eql(item, val);
})
} else {
included = obj.indexOf(val) !== -1;
}
break;
}

// Assert inclusion in collection or substring in a string.
this.assert(
objType === 'string' || !isDeep ? ~obj.indexOf(val)
: isDeepIncluded(obj, val)
included
, 'expected #{this} to ' + descriptor + 'include ' + _.inspect(val)
, 'expected #{this} to not ' + descriptor + 'include ' + _.inspect(val));
}
Expand Down
104 changes: 96 additions & 8 deletions test/assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,42 @@ describe('assert', function () {
assert.include({foo: obj1, bar: obj2}, {foo: obj1});
assert.include({foo: obj1, bar: obj2}, {foo: obj1, bar: obj2});

if (typeof Map === 'function') {
var map = new Map();
var val = [{a: 1}];
map.set('a', val);
map.set('b', 2);
map.set('c', -0);
map.set('d', NaN);

assert.include(map, val);
assert.include(map, 2);
assert.include(map, 0);
assert.include(map, NaN);
}

if (typeof Set === 'function') {
var set = new Set();
var val = [{a: 1}];
set.add(val);
set.add(2);
set.add(-0);
set.add(NaN);

assert.include(set, val);
assert.include(set, 2);
assert.include(set, 0);
assert.include(set, NaN);
}

if (typeof WeakSet === 'function') {
var ws = new WeakSet();
var val = [{a: 1}];
ws.add(val);

assert.include(ws, val);
}

if (typeof Symbol === 'function') {
var sym1 = Symbol()
, sym2 = Symbol();
Expand All @@ -653,19 +689,19 @@ describe('assert', function () {

err(function(){
assert.include(true, true, 'blah');
}, "blah: object tested must be an array, an object, or a string, but boolean given");
}, "blah: object tested must be an array, a map, an object, a set, a string, or a weakset, but boolean given");

err(function () {
assert.include(42, 'bar');
}, "object tested must be an array, an object, or a string, but number given");
}, "object tested must be an array, a map, an object, a set, a string, or a weakset, but number given");

err(function(){
assert.include(null, 42);
}, "object tested must be an array, an object, or a string, but null given");
}, "object tested must be an array, a map, an object, a set, a string, or a weakset, but null given");

err(function () {
assert.include(undefined, 'bar');
}, "object tested must be an array, an object, or a string, but undefined given");
}, "object tested must be an array, a map, an object, a set, a string, or a weakset, but undefined given");
});

it('notInclude', function () {
Expand All @@ -678,6 +714,38 @@ describe('assert', function () {
assert.notInclude({foo: obj1, bar: obj2}, {foo: {a: 1}});
assert.notInclude({foo: obj1, bar: obj2}, {foo: obj1, bar: {b: 2}});

if (typeof Map === 'function') {
var map = new Map();
var val = [{a: 1}];
map.set('a', val);
map.set('b', 2);

assert.notInclude(map, [{a: 1}]);
assert.notInclude(map, 3);
}

if (typeof Set === 'function') {
var set = new Set();
var val = [{a: 1}];
set.add(val);
set.add(2);

assert.include(set, val);
assert.include(set, 2);

assert.notInclude(set, [{a: 1}]);
assert.notInclude(set, 3);
}

if (typeof WeakSet === 'function') {
var ws = new WeakSet();
var val = [{a: 1}];
ws.add(val);

assert.notInclude(ws, [{a: 1}]);
assert.notInclude(ws, {});
}

if (typeof Symbol === 'function') {
var sym1 = Symbol()
, sym2 = Symbol()
Expand All @@ -699,19 +767,19 @@ describe('assert', function () {

err(function(){
assert.notInclude(true, true, 'blah');
}, "blah: object tested must be an array, an object, or a string, but boolean given");
}, "blah: object tested must be an array, a map, an object, a set, a string, or a weakset, but boolean given");

err(function () {
assert.notInclude(42, 'bar');
}, "object tested must be an array, an object, or a string, but number given");
}, "object tested must be an array, a map, an object, a set, a string, or a weakset, but number given");

err(function(){
assert.notInclude(null, 42);
}, "object tested must be an array, an object, or a string, but null given");
}, "object tested must be an array, a map, an object, a set, a string, or a weakset, but null given");

err(function () {
assert.notInclude(undefined, 'bar');
}, "object tested must be an array, an object, or a string, but undefined given");
}, "object tested must be an array, a map, an object, a set, a string, or a weakset, but undefined given");

err(function () {
assert.notInclude('foobar', 'bar');
Expand All @@ -731,6 +799,26 @@ describe('assert', function () {
assert.notDeepInclude({foo: obj1, bar: obj2}, {baz: {a: 1}});
assert.notDeepInclude({foo: obj1, bar: obj2}, {foo: {a: 1}, bar: {b: 9}});

if (typeof Map === 'function') {
var map = new Map();
map.set(1, [{a: 1}]);

assert.deepInclude(map, [{a: 1}]);
}

if (typeof Set === 'function') {
var set = new Set();
set.add([{a: 1}]);

assert.deepInclude(set, [{a: 1}]);
}

if (typeof WeakSet === 'function') {
err(function() {
assert.deepInclude(new WeakSet(), {}, 'foo');
}, 'foo: unable to use .deep.include with WeakSet');
}

err(function () {
assert.deepInclude([obj1, obj2], {a: 9}, 'blah');
}, "blah: expected [ { a: 1 }, { b: 2 } ] to deep include { a: 9 }");
Expand Down