-
Notifications
You must be signed in to change notification settings - Fork 242
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(rules): add no-expect-to-match-object rule #545
feat(rules): add no-expect-to-match-object rule #545
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this - sorry for the delay in reviewing!
I've got a few changes to request; I've also asked if you could chuck in a bunch more tests, just because they're really cheap - some of them might seem odd, but they're just-in-case to catch weird edge cases we've hit in the past with other rules.
@SimenB could you do a sense check in this for me? I think the underlying concept is good, but I always get the rules on comparing objects with jest mixed up, so want to make sure this is what we want to encourage, or otherwise adjust it to encourage the right stuff.
Also looking over the rule, if the behaviour is positive, I'm wondering if it might be better to focus on objectContaining
; so call the rule prefer-object-containing
, focus on saying "prefer expect.objectContaining" (i.e because you can use it with toStrictEqual
, but also with other matchers), and maybe even implement an autofix that replaces toMatchObject
-> toStrictEqual(expect.objectContaining)
?
That last one might be going a bit too far for now, but let me know what you think :)
Thank you for your review, Sorry also for the delay in processing your feedback!
I am happy to adapt this rule in any way. The motivation for me was more on the safety of a nested equality check. I believe an auto fix would be making too many assumptions since the options are to either explicitly state what you are expecting the object to be like: expect(obj).toStrictEqual({ a: 1, b: 2 }); or to check specific properties but still be aware of other properties: expect(obj).toStrictEqual(
expect.objectContaining({
a: 1,
b: expect.any(Number),
}),
); |
🎉 This issue has been resolved in version 23.11.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Motivation
Jest allows you to to check that a JavaScript object matches a subset of the properties of an object using
expect().toMatchObject(object)
.Checking just a subset of an object might lead to not catching changes in an object in the tests, this rule bans
expect().toMatchObject
in favor ofexpect().toStrictEqual
for a deep equality check orexpect().objectContaining
to consciously check the properties that are interesting.