-
Notifications
You must be signed in to change notification settings - Fork 248
feat: create new pair-to-have-been-called-assertions rule
#1848
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
base: main
Are you sure you want to change the base?
feat: create new pair-to-have-been-called-assertions rule
#1848
Conversation
0f96239 to
ce3724f
Compare
ce3724f to
dd0b9b3
Compare
pair-to-have-been-called-assertions rule
G-Rath
left a comment
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.
looks like a good start - could you remove the contradictoryCallTimes logic and do any resulting simplifications, and review the istanbul comments as I'm pretty sure most should be coverable
|
|
||
| When testing mock functions, developers often use `toHaveBeenCalledWith()`, | ||
| `toBeCalledWith()`, `toHaveBeenNthCalledWith()`, `toBeNthCalledWith()`, | ||
| `toHaveBeenLastCalledWith()`, or `toBeLastCalledWith()` to verify that a |
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.
| `toHaveBeenLastCalledWith()`, or `toBeLastCalledWith()` to verify that a | |
| `toHaveBeenLastCalledWith()`, and `toBeLastCalledWith()` to verify that a |
|
|
||
| ## Examples | ||
|
|
||
| ### Incorrect |
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.
| ### Incorrect | |
| Examples of **incorrect** code for this rule: |
| expect(mockFn).toBeLastCalledWith('last'); | ||
| ``` | ||
|
|
||
| ### Correct |
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.
| ### Correct | |
| Examples of **correct** code with this rule: |
|
|
||
| ## Additional Checks | ||
|
|
||
| ### Contradictory Assertions |
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.
At this point I think this is out of scope for this rule - can we remove it for now?
|
|
||
| However, it's generally recommended to keep this rule enabled as it helps catch | ||
| bugs where functions are called more times than expected, leading to more robust | ||
| and reliable tests. |
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.
I don't think this is particularly useful to have in this section, especially as this a rule that is enabled by default
| However, it's generally recommended to keep this rule enabled as it helps catch | |
| bugs where functions are called more times than expected, leading to more robust | |
| and reliable tests. |
Summary
This PR introduces a new ESLint rule
pair-to-have-been-called-assertionsthat enforces usingtoHaveBeenCalledTimes()ortoBeCalledTimes()whenever using matchers liketoHaveBeenCalledWith(),toBeCalledWith(),toHaveBeenNthCalledWith(),toBeNthCalledWith(),toHaveBeenLastCalledWith(), ortoBeLastCalledWith().Why?
When testing mock functions, developers often use
toHaveBeenCalledWith()to verify that a function was called with specific arguments. However, without also checking the call count, the test can pass even if the mock was called more times than expected, potentially masking bugs.What does this rule do?
toHaveBeenCalledWith()(and similar matchers) withtoHaveBeenCalledTimes()toHaveBeenCalledTimes(1)assertiontoHaveBeenCalledTimes(0)withtoHaveBeenCalledWith())toHaveBeenCalledWith,toBeCalledWith,toHaveBeenNthCalledWith,toBeNthCalledWith,toHaveBeenLastCalledWith,toBeLastCalledWithExamples
❌ Incorrect:
✅ Correct:
Resolves #1820