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

feat: improve deep equality check failing message for toBe #1383

Merged
merged 7 commits into from Jun 6, 2022
Merged

feat: improve deep equality check failing message for toBe #1383

merged 7 commits into from Jun 6, 2022

Conversation

TommyDew42
Copy link
Contributor

What does this PR do?

A PR for issue #1290.

We would like to have a message to remind users they should use deep equality checks, when object comparison fails with toBe. The error message would look like this now:
Screenshot 2022-05-27 at 12 12 49 PM

The implementation is similar to how jest does it.

How the change is tested?

Unit tests

@netlify
Copy link

netlify bot commented May 27, 2022

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 368e204
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/62979f15102dea0008da855b
😎 Deploy Preview https://deploy-preview-1383--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@TommyDew42 TommyDew42 changed the title Improve deep equality check failing message for toBe feat: improve deep equality check failing message for toBe May 27, 2022
@ghiscoding
Copy link
Contributor

Thanks for working on the issue that I opened, however I still prefer how Jest shows the error, the text is more aligned especially around the expectations of expected/received. Also I see you added square brackets around the "serializes to the same string" but I find that a little bit confusing, I didn't know (until I looked at your code change) that the bracket was something that you added, I wasn't sure if the text inside it was dynamic or not (it's not) and that made it confusing, it's also not an array so why use brackets and not double quotes? Nonetheless, it's a great improvement compare to current code, so I'd be curious to see what the core team will reply. Thanks again for working on the issue

I like how Jest shows it, everything is aligned and more readable

expect(received).toBe(expected) // Object.is equality

    If it should pass with deep equality, replace "toBe" with "toStrictEqual"

    Expected: [{"age": 30, "id": 123, "name": "John"}]
    Received: serializes to the same string

      at Object.<anonymous> (/src/index.spec.ts:34:50)

@TommyDew42
Copy link
Contributor Author

@ghiscoding @sheremet-va

Does the current error message look good to us?
Screenshot 2022-06-02 at 1 03 00 AM

@ghiscoding
Copy link
Contributor

@ghiscoding @sheremet-va

Does the current error message look good to us? Screenshot 2022-06-02 at 1 03 00 AM

Yeah this is much closer to Jest message, love it

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

Successfully merging this pull request may close these issues.

None yet

4 participants