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 does not treat non-enumerable properties equally #6392

Closed
mweststrate opened this issue Jun 4, 2018 · 2 comments · Fixed by #6398
Closed

toEqual does not treat non-enumerable properties equally #6392

mweststrate opened this issue Jun 4, 2018 · 2 comments · Fixed by #6398

Comments

@mweststrate
Copy link
Contributor

mweststrate commented Jun 4, 2018

🐛 Bug Report

I discovered some inconsistent behavior in the toEqual matcher in Jest; when comparing to objects, it ignores non-enumerable properties, but only if they are not symbols. This leads to quite inconsistent behavior:

This test passes:

const actual1 = {
        x: 3
}
Object.defineProperty(actual1, "test", {
        enumerable: false,
        value: 5
})
expect(actual1).toEqual({ x: 3 })

But this one doesn't:

const actual2 = {
        x: 3
}
const mySymbol = Symbol("test")
Object.defineProperty(actual2, mySymbol, {
        enumerable: false,
        value: 5
})
expect(actual2).toEqual({ x: 3 })

Where one would expect both to behave the same.

The cause of the issue can be found here. To find the string properties the for..in loop is used which ignores non-enumerable properties, but for the symbolic properties getOwnPropertySymbols is used, which will include non-enumerable properties.

To Reproduce

See link below

Expected behavior

Non-enumerable symbolic properties should be treated the same as non-enumerable non-symbolic properties when running toEqual. Either both should be included, or excluded. Given that symbolic properties are lot more exotic, I think the behavior of string properties should be followed and non-enumerable symbolic properties should be excluded from deep equality.

I am willing to file a PR myself

Link to repl or repo (highly encouraged)

https://repl.it/@mweststrate/IckyMeanProgrammers

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: Linux 4.15 Ubuntu 18.04 LTS (Bionic Beaver)
    CPU: x64 Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
  Binaries:
    Node: 9.3.0 - ~/.nvm/versions/node/v9.3.0/bin/node
    Yarn: 1.6.0 - /usr/bin/yarn
    npm: 5.5.1 - ~/.nvm/versions/node/v9.3.0/bin/npm
  npmPackages:
    @types/jest: ^21.1.9 => 21.1.10 
    jest: ^22.0.4 => 22.4.4 
@mweststrate
Copy link
Contributor Author

Added PR: #6398, as that is often more clarifying than a bug report :)

cpojer pushed a commit that referenced this issue Jun 5, 2018
…ties.… (#6398)

* `expect` no longer tries to equal non-enumerable symbolic properties. Fixes #6392

* Updated changelog with PR number

* Fixed linting errors

* 'fixed' jest error
@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 12, 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

Successfully merging a pull request may close this issue.

1 participant