Skip to content

test(query-core): expand notifyManager unit tests 🤖🤖🤖 - #11407

Open
pengzh1 wants to merge 1 commit into
TanStack:mainfrom
pengzh1:test/notify-manager-unit-tests
Open

test(query-core): expand notifyManager unit tests 🤖🤖🤖#11407
pengzh1 wants to merge 1 commit into
TanStack:mainfrom
pengzh1:test/notify-manager-unit-tests

Conversation

@pengzh1

@pengzh1 pengzh1 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🎯 Changes

Expands the unit test coverage for notifyManager in packages/query-core. The existing suite covered the happy paths of batching and custom notifyFn/batchNotifyFn/scheduler configuration; this PR adds behavioral coverage for the gaps that were not exercised:

  • Empty batches: a batch() call that schedules nothing does not invoke the scheduler at all (the flush no-op path).
  • Scheduling outside of a batch: each schedule() notifies its callback individually and does not route through batchNotifyFn, pinning the two distinct code paths of schedule.
  • Nested batches: ending an inner batch does not flush; queued callbacks are dispatched only after the outermost batch ends.
  • Return value: batch() passes the callback's return value through to the caller.
  • Error recovery: after a batch throws, the transaction counter is released, so a subsequent schedule() is dispatched immediately instead of being queued forever.
  • Default scheduler semantics: notifications scheduled with the default scheduler run on a macrotask (setTimeout(0)) — draining microtasks alone does not flush them.

Key assertions were verified against intentionally broken implementations (empty-batch flush guard, nested-batch flush guard) and go red, so they test behavior rather than implementation details.

Test-only change: no production source is modified.

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested code changes locally with pnpm run test:pr, or these tests do not apply to this pull request.
  • I fully understand the code in this pull request, including any code generated with AI assistance.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Tests
    • Added coverage for notification batching, including nested batches and empty batches.
    • Verified scheduler behavior for notifications made inside and outside batches.
    • Confirmed batched callbacks return their results and recover correctly after errors.
    • Added coverage ensuring default notifications run on a later task rather than during microtask processing.

Cover previously untested behaviors: empty batches do not invoke the
scheduler, schedules outside of a batch notify each callback individually
without the batch notify function, nested batches only flush at the
outermost boundary, the batch callback's return value is passed through,
schedules dispatch immediately after a batch threw (transaction counter
is released), and the default scheduler defers notifications to a
macrotask.
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: a0d12f82-69b0-482d-9af7-3b8b7d752f03

📥 Commits

Reviewing files that changed from the base of the PR and between 1893a96 and d5f3b62.

📒 Files selected for processing (1)
  • packages/query-core/src/__tests__/notifyManager.test.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds six notifyManager tests. The tests cover batch flushing, standalone scheduling, nested batches, callback results, error recovery, and default macrotask scheduling.

Changes

notifyManager behavior

Layer / File(s) Summary
Batch lifecycle behavior
packages/query-core/src/__tests__/notifyManager.test.tsx
Tests cover empty batches, standalone notifications, nested batch flushing, and batch return values.
Scheduling and recovery behavior
packages/query-core/src/__tests__/notifyManager.test.tsx
Tests cover transaction release after errors and default macrotask scheduling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d5f3b

This change adds coverage for notifyManager batching, error recovery, return values, and scheduling behavior without changing runtime implementation. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the test-only changes, lists the covered behaviors, completes all required template sections, and records testing and release impact.
Title check ✅ Passed The title clearly identifies the query-core unit test expansion for notifyManager. The emojis add minor noise but do not reduce clarity.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant