-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
test(svelte-query/createMutation): improve callback assertions with 'toHaveBeenNthCalledWith' and apply shorthand property syntax in FailureExample #9630
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
Conversation
…toHaveBeenNthCalledWith' and apply shorthand property syntax in FailureExample
WalkthroughThe PR updates a Svelte test component to use object property shorthand in a createMutation call and adjusts test assertions to use nth-call checks for mock callbacks while keeping total call counts the same. No public APIs or exports are changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
View your CI Pipeline Execution ↗ for commit d049df2
☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9630 +/- ##
===========================================
+ Coverage 45.50% 89.20% +43.69%
===========================================
Files 209 18 -191
Lines 8377 176 -8201
Branches 1904 31 -1873
===========================================
- Hits 3812 157 -3655
+ Misses 4118 18 -4100
+ Partials 447 1 -446
🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/svelte-query/tests/createMutation/createMutation.test.ts (2)
51-53
: DRY up repeated nth-call assertionsMinor style: a loop keeps the expectations compact while preserving intent.
- expect(onSuccessMock).toHaveBeenNthCalledWith(1, 1) - expect(onSuccessMock).toHaveBeenNthCalledWith(2, 2) - expect(onSuccessMock).toHaveBeenNthCalledWith(3, 3) + for (const n of [1, 2, 3]) { + expect(onSuccessMock).toHaveBeenNthCalledWith(n, n) + } @@ - expect(onSettledMock).toHaveBeenNthCalledWith(1, 1) - expect(onSettledMock).toHaveBeenNthCalledWith(2, 2) - expect(onSettledMock).toHaveBeenNthCalledWith(3, 3) + for (const n of [1, 2, 3]) { + expect(onSettledMock).toHaveBeenNthCalledWith(n, n) + }Also applies to: 56-58
51-53
: Assert each onSuccess runs before its corresponding onSettledAdd immediately after the existing expectations (lines 56–58):
+ // Ensure each onSuccess occurs before its corresponding onSettled + const successOrder = onSuccessMock.mock.invocationCallOrder + const settledOrder = onSettledMock.mock.invocationCallOrder + for (let i = 0; i < 3; i++) { + expect(successOrder[i]).toBeLessThan(settledOrder[i]) + }Vitest exposes
mock.invocationCallOrder
and also providestoHaveBeenCalledBefore
/toHaveBeenCalledAfter
matchers—use whichever yields clearer intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/svelte-query/tests/createMutation/FailureExample.svelte
(1 hunks)packages/svelte-query/tests/createMutation/createMutation.test.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (1)
packages/svelte-query/tests/createMutation/FailureExample.svelte (1)
15-15
: LGTM: idiomatic object property shorthandNo behavior change; this keeps the options concise and clear.
…ons-shorthand-props
…ons-shorthand-props
Summary by CodeRabbit