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

[Bug]: Symbols result in misleading diffs #13809

Closed
joshkel opened this issue Jan 24, 2023 · 3 comments
Closed

[Bug]: Symbols result in misleading diffs #13809

joshkel opened this issue Jan 24, 2023 · 3 comments

Comments

@joshkel
Copy link
Contributor

joshkel commented Jan 24, 2023

Version

29.4.0

Steps to reproduce

const Foo = Symbol("foo");
test("mismatched symbols", () => {
  expect({ a: 1, [Foo]: {id: 1} }).toMatchObject({ a: 1, [Foo]: {id: 2} });
});

Expected behavior

A diff that indicates the different property:

    - Expected  - 1
    + Received  + 1

      Object {
        "a": 1,
        Symbol(foo): Object {
    -     "id": 2,
    +     "id": 1,
        },
      }

Actual behavior

A diff that acts like the received object had no symbol property at all:

    - Expected  - 3
    + Received  + 0

      Object {
        "a": 1,
    -   Symbol(foo): Object {
    -     "id": 2,
    -   },
      }

Additional context

As a result of #13639, expect.toMatchObject can now compare symbol keys. However, getObjectSubset wasn't updated to match, so the resulting diffs can be misleading or confusing.

Environment

System:
    OS: macOS 13.1
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 18.13.0 - ~/.nvm/versions/node/v18.13.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.19.3 - ~/.nvm/versions/node/v18.13.0/bin/npm
  npmPackages:
    jest: ^29.4.0 => 29.4.0
joshkel added a commit to joshkel/jest that referenced this issue Jan 24, 2023
As a result of jestjs#13639, `expect.toMatchObject` can now compare symbol keys.  However, the diffs that it generates if a symbol key doesn't match are misleading.  For example, the following assertion:

```
const Foo = Symbol("foo");
test("mismatched symbols", () => {
  expect({ a: 1, [Foo]: {id: 1} }).toMatchObject({ a: 1, [Foo]: {id: 2} });
});
```

fails as desired, but it displays the following diff:

```
    - Expected  - 3
    + Received  + 0

      Object {
        "a": 1,
    -   Symbol(foo): Object {
    -     "id": 2,
    -   },
      }
```

*Note*: In inspecting the code, I wonder if `getObjectSubset` should use the same logic as `subsetEquality` - i.e., if `subsetEquality` does not evaluate an object's inherited string keys when determining equality, then `getObjectSubset` should not evaluate an object's inherited string keys for reporting on inequality? To put it another way - jestjs#13639 appears to change `subsetEquality` from considering an object's inherited string keys to not considering inherited string keys, and I'm not sure if that was intentional in a SemVer-minor change.  For now, I've preserved the previous behavior, but let me know if this should change.

Fixes jestjs#13809
@SimenB
Copy link
Member

SimenB commented Jan 25, 2023

#13810

@SimenB SimenB closed this as completed Jan 25, 2023
@SimenB
Copy link
Member

SimenB commented Jan 26, 2023

@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 Feb 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants