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

Breaking: Fix .include to always use strict equality #760

Merged
merged 2 commits into from
Aug 15, 2016

Conversation

meeber
Copy link
Contributor

@meeber meeber commented Jul 28, 2016

  • Previously, .include was using strict equality for non-negated property
    inclusion, but deep equality for negated property inclusion and array
    inclusion. This fix causes .include to always use strict equality.

Note:

@keithamus
Copy link
Member

Yup, LGTM. @lucasfcosta?

@@ -221,6 +222,17 @@ module.exports = function (chai, _) {
* expect([1,2,3]).to.include(2);
* expect('foobar').to.contain('foo');
* expect({ foo: 'bar', hello: 'universe' }).to.include({ foo: 'bar' });
*
* By default, strict equality (===) 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.

Maybe this line isn't totally clear for anyone using the library.
When saying that this uses strict equality this may not be exactly clear for everyone. This is because when saying that this assertion uses this kind of comparison I would expect (if I just read the docs) the following assertion to throw an error:

expect({foo: obj1, bar: obj2}).to.include({foo: obj1});

Because I would expect it to compare the object I passed as a parameter to the include method to the one I passed to expect, and, since they would not be the same instance, strict equality would be false and therefore I would expect an Error to be raised.

Maybe we could explain this a little better, just to make things as clear as possible, I'm not sure everyone will get it right this way.

What do you guys think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lucasfcosta Agreed. Updated with an explanation.

meeber added 2 commits August 14, 2016 16:07
- Previously, `.include` was using strict equality for non-negated property
inclusion, but deep equality for negated property inclusion and array
inclusion. This fix causes `.include` to always use strict equality.
@meeber
Copy link
Contributor Author

meeber commented Aug 14, 2016

@lucasfcosta @keithamus New version pushed!

@lucasfcosta
Copy link
Member

lucasfcosta commented Aug 15, 2016

Excelent explanation. The inner working of this assertion is now very clear.
LGTM

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

Successfully merging this pull request may close these issues.

3 participants