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

toEqual ignores keys with undefined properties #711

Closed
321ckatz123 opened this issue Feb 13, 2016 · 16 comments
Closed

toEqual ignores keys with undefined properties #711

321ckatz123 opened this issue Feb 13, 2016 · 16 comments

Comments

@321ckatz123
Copy link

If a property is undefined, Jest appears to ignore it when calling .toEqual().

import _ from "lodash"; describe("test", () => { it("returns an object with the correct nodes", () => { let a = {"key1": undefined, "key2": "a"}; let b = {"key2": "a"}; expect(a).toEqual(b); expect(_.isEqual(a, b)).toBeFalsy(); }); });

This simple test will pass. As you can see by adding lodash, it should be false.

My output:
Using Jest CLI v0.8.2, jasmine1
Running 1 test suite...
PASS processors\hockey__tests__\events.js (0.573s)
1 test passed (1 total in 1 test suite, run time 1.076s)
----------|----------|----------|----------|----------|----------------|
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
----------|----------|----------|----------|----------|----------------|
All files | 100 | 100 | 100 | 100 | |
----------|----------|----------|----------|----------|----------------|

@jamedranoa
Copy link

this is coming from here. @cpojer is there a reason why that check can't be removed? all the tests on the jest repository pass without it.

@cpojer
Copy link
Member

cpojer commented Feb 15, 2016

@jamedranoa yes, I plan on eventually removing this check. Unfortunately we have a shitload of tests at FB that depend on this behavior. My goal is to unfork jasmine eventually though, it'll happen in the next few months.

@jamedranoa
Copy link

Cool thank you! 👍

@scsper
Copy link

scsper commented Dec 18, 2016

@cpojer Are there still plans to remove this?

@aaronabramov
Copy link
Contributor

@scsper we still have plans, but there's still too many FB tests that depend on this behavior, so it's not going to happen soon :/

@peter-leonov
Copy link

Hey guys, what about adding a feature switch for us open source users to use isEqual() without strange JSON.stringify() related surprises?

@mnpenner
Copy link

Is there a workaround for this?

@JasonCust
Copy link

@cpojer any updates on this issue? I ran into it recently and noticed this issue has gone stale.

@SimenB
Copy link
Member

SimenB commented Dec 17, 2017

You can create your own matcher using lodash.toEqual if you want

@JasonCust
Copy link

@SimenB For now I have a couple workarounds (e.g. using toHaveProperty) but I was hoping there was some timeline on a fix or at the least a note in the docs about this issue.

@lukescott
Copy link

Hmmm... the issue says it ignores undefined. I'm seeing the opposite - it does not ignore undefined properties. It seems to depend on which side goes into expect() and vs toEqual().

Wish there was a way to toggle the behavior. In my case I want it to ignore undefined as JSON.stringify will strip undefined properties anyway (so I don't care).

@phamj88
Copy link

phamj88 commented Feb 9, 2018

@lukescott
what did your test case case look like?
Im seeing undefined gets ignored no matter which side the toEquals it is in.

@mike1808
Copy link

Is anyone concerned with this? I got a bug today because of this issue.

@SimenB
Copy link
Member

SimenB commented Sep 19, 2018

This is sorta fixed, we have toStrictEqual now.

EDIT: based on this comment, I'll call this issue fixed:

would be impossible to roll out at FB. Having a separate strict matcher in Jest is the better option.

Slightly a footgun that's it's not the default, but legacy.

If you want to enforce usage in your code base, you can use jest-community/eslint-plugin-jest#134, which can autofix your code

@SimenB SimenB closed this as completed Sep 19, 2018
tkrugg added a commit to algolia/algoliasearch-helper-js that referenced this issue Jun 18, 2019
- split the implem into merge(target, ...sources) and _merge(target, source), I think it's more simple this way.
- I reorganized some of the conditions
- I mostly kept the description and the test suite as I think they are
still relevant. Only a minor change to note in the tests:

> changed all of them to use expect().toStrictEqual instead of .toEqual
(see: jestjs/jest#711)
tkrugg added a commit to algolia/algoliasearch-helper-js that referenced this issue Jun 18, 2019
cf. jestjs/jest#711

`undefined` values being critical to in assessing merge behaviour this
is necessary
tkrugg added a commit to algolia/algoliasearch-helper-js that referenced this issue Jun 21, 2019
* fix(merge): use .toStrictEqual instead of .toEqual in tests

cf. jestjs/jest#711

`undefined` values being critical to in assessing merge behaviour this
is necessary

* fix(merge): specify description

add meaning of array-like

* fix(merge): change implementation

- split the implem into merge(target, ...sources) and _merge(target, source), I think it's more simple this way.
- I reorganized some of the conditions

The description and the test suit should not change. Implementation has
changed but behaviour and limitation is the same (Note that the lodash
inspired test suite for merge should be passing).

* fix: extra-space

Co-Authored-By: Yannick Croissant <[email protected]>

* fix: rename isObjectOrArray to isObjectOrArrayOrFunction
Haroenv pushed a commit to algolia/algoliasearch-helper-js that referenced this issue Nov 18, 2019
* fix(merge): use .toStrictEqual instead of .toEqual in tests

cf. jestjs/jest#711

`undefined` values being critical to in assessing merge behaviour this
is necessary

* fix(merge): specify description

add meaning of array-like

* fix(merge): change implementation

- split the implem into merge(target, ...sources) and _merge(target, source), I think it's more simple this way.
- I reorganized some of the conditions

The description and the test suit should not change. Implementation has
changed but behaviour and limitation is the same (Note that the lodash
inspired test suite for merge should be passing).

* fix: extra-space

Co-Authored-By: Yannick Croissant <[email protected]>

* fix: rename isObjectOrArray to isObjectOrArrayOrFunction
@glasser
Copy link

glasser commented Aug 25, 2020

It might help if the docs for toEqual mentioned the ways in which it differs from toStrictEqual instead of you having to discover toStrictEqual on your own.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests