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 toMatchObject diff to match object type #6580

Closed
6 tasks done
msteen opened this issue Sep 26, 2024 · 2 comments · Fixed by #6620
Closed
6 tasks done

Expect toMatchObject diff to match object type #6580

msteen opened this issue Sep 26, 2024 · 2 comments · Fixed by #6620
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@msteen
Copy link

msteen commented Sep 26, 2024

Describe the bug

When I use expect(testObject).toMatchObject(expectedObject) I expect the diff to report the correct type for Received as it does for Expected.

For example, say I have the two classes Foo and Bar:

class Foo {
  constructor(public value: number) {}
}
class Bar {
  constructor(public value: number) {}
}

Then I expect the following test:

test("foo", () => {
  expect(new Bar(2)).toMatchObject(new Foo(1))
})

To report the types as it does in the AssertionError

 FAIL  src/test.ts > foo
AssertionError: expected Bar{ value: 2 } to match object Foo{ value: 1 }

- Expected
+ Received

- Foo {
-   "value": 1,
+ Object {
+   "value": 2,
  }

 ❯ src/test.ts:44:22
     42| }
     43| test("foo", () => {
     44|   expect(new Bar(2)).toMatchObject(new Foo(1))
       |                      ^
     45| })
     46| 

Yet it shows expecting a Foo and receiving an Object, while I would expect Object to be Bar here.

Now there have been cases (when this problem occurs in a nested object) where I had to add additional logging on failure to determine that Object is in fact Bar to pinpoint the cause of the error, which would have been unnecessary if the diff reported the correct type.

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-qmrkzs?file=test%2Fbasic.test.ts

System Info

vitest/1.6.0 linux-x64 node-v22.7.0

  System:
    OS: Linux 6.8 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    CPU: (8) x64 Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
    Memory: 2.94 GB / 31.06 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 22.7.0 - ~/.nvm/versions/node/v22.7.0/bin/node
    npm: 10.8.2 - ~/.nvm/versions/node/v22.7.0/bin/npm
    pnpm: 9.9.0 - ~/.nvm/versions/node/v22.7.0/bin/pnpm
    bun: 1.1.26 - ~/.nvm/versions/node/v22.7.0/bin/bun

Used Package Manager

pnpm

Validations

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Sep 27, 2024

toMatchObject(new Foo(1)) should probably work same as toMatchObject({ value: 1 }), but it looks like this one shows a following diff: https://stackblitz.com/edit/vitest-dev-vitest-dfkaue?file=test%2Fbasic.test.ts

 FAIL  test/basic.test.ts > x
AssertionError: expected Bar{ value: 2 } to match object { value: 1 }

- Expected
+ Received

  Object {
-   "value": 1,
+   "value": 2,
  }

 ❯ eval test/basic.test.ts:9:22
      7| 
      8| test('x', () => {
      9|   expect(new Bar(2)).toMatchObject({ value: 1 });
       |                      ^
     10| });
     11| 

So, this could be the expected behavior than showing Bar (though not sure):

- Expected
+ Received

- Foo {
-   "value": 1,
+ Bar {
+   "value": 2,
  }

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Oct 3, 2024

Looking it again, it's probably desirable to preserve prototypes inside diff, which is currently dropped "Received" side. This is due to a recent change to slim down the diff for toMatchObject #5364

I checked Jest and they also got the same bug as this code is mostly based on them. Here is a repro in Jest https://stackblitz.com/edit/github-caqski?file=basic.test.js

@hi-ogawa hi-ogawa added p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Oct 3, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants