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

expect(array).to.deep.include({...}) does not apply deep equality recursively #390

Closed
bitliner opened this issue Mar 6, 2015 · 24 comments
Closed

Comments

@bitliner
Copy link

bitliner commented Mar 6, 2015

As the title says,
if I run expect(array).to.deep.include(obj) I would expect to apply deep equality to each of the property of obj.

But it does not behave like this, is it a bug?

@keithamus
Copy link
Member

@bitliner thanks for the issue.

Could you please give a more concrete example? What is array and obj in this context? Below seems work fine for me (Chai 2.1.1)

expect([ {foo: {bar:true} } ]).to.deep.include({ foo: {bar:true} });

@bitliner
Copy link
Author

bitliner commented Mar 6, 2015

Sorry, my fault.
The variable arr instead of being an array it was an object, and the error message was

to have a property 'pubDate' of Sat, 26 Apr 2014 22:00:00 GMT, but got Sat, 26 Apr 2014 22:00:00 GMT

Using to.be.deep.eql it works fine.

@keithamus
Copy link
Member

Hmm. Looks like you've probably got two different Date objects that you're testing against. Despite the Date's having the same value, they wont have the same reference, and so do not equal.

I guess .deep.include could offer the same semantics as .deep.equal but I'm not sure if it makes sense to have them both share the same behaviour... thoughts?

@blah238
Copy link

blah238 commented Mar 11, 2015

I am having a similar issue: #399

I have also tried using .deep.equal but the behavior is the same for me.

@felixfbecker
Copy link

How do you do this with assert style? there is no assert.deepInclude()...

@bnord01
Copy link

bnord01 commented Mar 21, 2016

This doesn't work for nested arrays either. I need to test whether an associative array contains some entry but the following fails:

[['a','A'],['b','B']].should.deep.contain(['a','A'])
--> 
AssertionError: expected [ [ 'a', 'A' ], [ 'b', 'B' ] ] to include [ 'a', 'A' ]

What I find surprising is, that it works as expected using include.deep.members:

[['a','A'],['b','B']].should.include.deep.members([['a','A']])

@lucasfcosta
Copy link
Member

Hello guys, thanks for your reports! 😄

@bnord01 Currently we don't even differentiate .deep.contain from .contain. The problem here is that, on assertions.js L214, we check the type of the passed value using _.type, which, under the hood, calls Object.prototype.toString.call(val) and returns array for arrays, instead of object, that's why it isn't getting deeply compared, only strictly compared (using ===).

This looks like a very easy fix. We could just change this line on assertions.js L214. If this condition was written as if (_.type(obj) === 'array' && typeof val === 'object') our problem would be solved, because when calling typeof for an Array, it returns object too, and therefore we will run a deep comparison. I just tried this here and all tests passed, including the same assertion @bnord01 posted.

Hi, @felixfbecker you could just use .assert.include, because, as I explained above, currently we don't differentiate .deep.include from .include.

What is your opinion about it, @keithamus, should we add support for a deep flag on the include assertion and compare using === when it's not present or should we just fix the issue @bnord01 reported for now? In my opinion it would be nice to have a strict (===) comparison, even if it's not so common, it may be worthwhile to cover it.

@felixfbecker
Copy link

@lucasfcosta imo the default should be to search by object reference. I would then expect assert.deepInclude or should.deep.include to do a deep comparison, just like equals and deepEquals behave.

@bnord01
Copy link

bnord01 commented Mar 22, 2016

Currently we don't even differentiate .deep.contain from .contain.

@lucasfcosta that sounds not like desirable behavior to me. Indeed I was surprised that
[{}].should.not.contain({}) fails, while [{}].indexOf({}).should.equal(-1) succeeds.
If this is the intended behavior I think it should be documented with the includes method. Also in general it would be nice if a warning was issued if a non identity flag (like deep) is set on a method that doesn't support it. Looking at the documentation of deep it only says that it is used by equal and property but it is (at least) also used by members (which doesn't mention on what kind of target it can be used). Any reason not to support general ES6 iterables here?

@lucasfcosta
Copy link
Member

@felixfbecker thanks for your feedback, I agree with you. Let's just wait for a third opinion on this just to make sure this should really be done. 😃

@bnord01 Thanks for sharing your thoughts. I'm not sure that warning would be a good idea, but we can talk about that and come to an agreement. When it comes to the deep flag I totally agree with you both about the .deep flag documentation and its behavior when used alongside include.
I'm not sure about it, but I think we just don't support ES6 iterables because this was implemented before the ES6 implementation or because no one else noticed and told us:laughing:
I hope you wouldn't mind if we wait for other maintainer's opinion, I'm pretty new here and I don't want to rush things or take decisions alone.
Thanks again for your feedback!

@meeber
Copy link
Contributor

meeber commented Apr 12, 2016

@lucasfcosta I just ran into this issue as well. It was my hope that deep.include would perform deep equality. I can work around it but it won't be as succinct ;(

@perrin4869
Copy link

perrin4869 commented Jun 17, 2016

Just ran into this issue when testing errors.

expect(validationError).to.deep.contain({
  name: 'ValidationError',
  errors: {
    subject: { kind: 'required' },
    content: { kind: 'required' },
  },
});

Basically, I want to test that a subset of the properties match deeply, since I cannot test all the properties, therefore I can't use deep.equal.

@dmansfield
Copy link

Another basic failure case for "include" is:

expect({k1: []}).to.include({k1: []});
-->
AssertionError: expected { k1: [] } to have a property 'k1' of [], but got []

Ultimately, it's using an '===' on the value assertion.

I've tried various "deep" flags and other tricks ("contain", "keys", "members") but to no avail. How can I make such a simple assertion (without using "property" as I don't want the assertion subject to change - which is why I'm using "include" in the first place).

@meeber
Copy link
Contributor

meeber commented Sep 20, 2016

Whoops forgot to close this issue. Fixed via #761, will be released in 4.x (alpha release will be available on npm soon).

@meeber meeber closed this as completed Sep 20, 2016
@leo-thumbtack
Copy link

So what would be the proper syntax to solve this use case ?

@meeber
Copy link
Contributor

meeber commented Dec 28, 2016

@leo-thumbtack In Chai v4 (currently available via npm install chai@canary), the syntax is expect(myArray).to.deep.include(myObj).

@felixfbecker
Copy link

What about assert style?

@shvaikalesh
Copy link
Contributor

@felixfbecker assert.deepInclude or assert.notDeepInclude

@leo-thumbtack
Copy link

Thanks

@deksden
Copy link

deksden commented Aug 4, 2017

This fails with assert on chai v4.1.0:
expect([ { id: 1, name: 'first'}, { id: 2, name: 'second'}]).to.deep.include({ name: 'first' })

AssertionError: expected [ Array(2) ] to deep include { name: 'first' }

This is bug? How i can test if array include object with some specified properties?

upd: can be fixed with chai-subset plugin. Code is something like:

expect([ { id: 1, name: 'first'}, { id: 2, name: 'second'}]).to.containSubset([{ name: 'first' }])

@iraklisg
Copy link

iraklisg commented Aug 9, 2017

@deksden chai-subset plugin is what I was looking for 👍

@djom202
Copy link

djom202 commented Jul 12, 2019

Hi,

I'd like to ask about if with function could be test this condition:

expect([ 'Hender' ]).to.deep.include('end')

Thanks.

@lucasfcosta
Copy link
Member

lucasfcosta commented Jul 14, 2019

Hi @djom20, I think you could try to use a matcher or search for a plugin which would allow you to do that easily in a single assertion.

As a last resort you could also do:

const checkItemsInclude = (arr, str) => arr.some(i => i.includes(str));
expect(checkItemsInclude(yourArray, 'end')).to.be.equal(true);

If you're checking for multiple items in the same array you can make that function curried:

const arrayIncludeMatcher = arr => str => arr.some(i => i.includes(str));
const arrayIncludes = arrayIncludeMatcher(yourArray);
expect(arrayIncludes('something')).to.be.equal(true);
expect(arrayIncludes('something wrong')).to.be.equal(false);

@djom202
Copy link

djom202 commented Jul 30, 2019

Hi @djom20, I think you could try to use a matcher or search for a plugin which would allow you to do that easily in a single assertion.

As a last resort you could also do:

const checkItemsInclude = (arr, str) => arr.some(i => i.includes(str));
expect(checkItemsInclude(yourArray, 'end')).to.be.equal(true);

If you're checking for multiple items in the same array you can make that function curried:

const arrayIncludeMatcher = arr => str => arr.some(i => i.includes(str));
const arrayIncludes = arrayIncludeMatcher(yourArray);
expect(arrayIncludes('something')).to.be.equal(true);
expect(arrayIncludes('something wrong')).to.be.equal(false);

Thanks, it's working great now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests