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 tests don't work with the ES6 / ES2015 Set object #47

Closed
trevordmiller opened this issue Nov 23, 2015 · 7 comments
Closed

Expect tests don't work with the ES6 / ES2015 Set object #47

trevordmiller opened this issue Nov 23, 2015 · 7 comments

Comments

@trevordmiller
Copy link

Issue

The test below should fail, but it is passing:

it('should fail', () => {
  const actual = new Set('a');
  const expected = new Set('b');
  expect(actual).toEqual(expected);
});

expect-sets

Background & Question

I'm using the expect package from npm. I'm using Babel 5 to use Set. I'm using Node 5, so the Set being used should be native.
Am I doing something wrong, or does this look like a bug in the expect package in the way it handles Sets? I've cross posted a Stack Overflow question as I'm not sure.

p.s. I love this package and am happy I don't have to use Chai. Thank you for building it!

@mjackson
Copy link
Owner

Hi @trevordmiller – thanks for the bug report!

Under the hood we're just using deepEqual to check for equality, so we should probably open a bug report on https://github.com/substack/node-deep-equal.

@mjackson
Copy link
Owner

BTW, please be sure to reference this issue when you do so we know when we should upgrade. Thanks!

@mjackson
Copy link
Owner

Oh, wait. I just re-read your original post.

I would actually expect that test to pass. toEqual checks for equal values, but they don't have to be the same object. Use toBe for that.

@rstacruz
Copy link
Collaborator

I don't think that test should pass:

var a = new Set('a');
var b = new Set('b');

any way you look at it, equivalence or equality, they should be false. They're 2 different objects with 2 different values.

@mjackson
Copy link
Owner

Blah, sorry. You guys are right. I need to read more closely next time :P

Let's definitely open an issue on deep-equal and follow up there.

@mjackson
Copy link
Owner

Released in v 1.13.1

@trevordmiller
Copy link
Author

@mjackson Works great now! Thank you so much.

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

No branches or pull requests

3 participants