enhance(test): Auto-cleanup after each test#3774
Merged
Conversation
Register a module-level afterEach hook in makeRenderDataClient that automatically drains all active manager cleanups. Manual renderDataHook.cleanup() calls in afterEach are no longer needed. Also call .unref() on GCPolicy's setInterval to prevent it from blocking Node.js process exit in test runners. Made-with: Cursor
🦋 Changeset detectedLatest commit: ce42b01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Size Change: +23 B (+0.03%) Total Size: 80.5 kB
ℹ️ View Unchanged
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3774 +/- ##
=======================================
Coverage 98.05% 98.05%
=======================================
Files 151 151
Lines 2832 2834 +2
Branches 554 555 +1
=======================================
+ Hits 2777 2779 +2
Misses 11 11
Partials 44 44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: ce42b01 | Previous: 1a20f4e | Ratio |
|---|---|---|---|
normalizeLong |
429 ops/sec (±2.24%) |
460 ops/sec (±1.03%) |
1.07 |
normalizeLong Values |
408 ops/sec (±0.13%) |
408 ops/sec (±0.82%) |
1 |
denormalizeLong |
293 ops/sec (±2.32%) |
286 ops/sec (±2.53%) |
0.98 |
denormalizeLong Values |
267 ops/sec (±2.11%) |
264 ops/sec (±2.36%) |
0.99 |
denormalizeLong donotcache |
1054 ops/sec (±0.12%) |
1051 ops/sec (±0.12%) |
1.00 |
denormalizeLong Values donotcache |
778 ops/sec (±0.18%) |
772 ops/sec (±0.16%) |
0.99 |
denormalizeShort donotcache 500x |
1540 ops/sec (±0.37%) |
1585 ops/sec (±0.12%) |
1.03 |
denormalizeShort 500x |
850 ops/sec (±2.23%) |
864 ops/sec (±2.04%) |
1.02 |
denormalizeShort 500x withCache |
6393 ops/sec (±0.10%) |
6357 ops/sec (±0.22%) |
0.99 |
queryShort 500x withCache |
2740 ops/sec (±0.21%) |
2734 ops/sec (±0.13%) |
1.00 |
buildQueryKey All |
52110 ops/sec (±0.27%) |
55181 ops/sec (±0.40%) |
1.06 |
query All withCache |
6498 ops/sec (±0.43%) |
5942 ops/sec (±0.14%) |
0.91 |
denormalizeLong with mixin Entity |
282 ops/sec (±2.11%) |
283 ops/sec (±2.32%) |
1.00 |
denormalizeLong withCache |
6915 ops/sec (±0.23%) |
6807 ops/sec (±0.13%) |
0.98 |
denormalizeLong Values withCache |
5234 ops/sec (±0.12%) |
5132 ops/sec (±0.29%) |
0.98 |
denormalizeLong All withCache |
6377 ops/sec (±0.17%) |
5781 ops/sec (±0.18%) |
0.91 |
denormalizeLong Query-sorted withCache |
6665 ops/sec (±0.18%) |
6026 ops/sec (±0.11%) |
0.90 |
denormalizeLongAndShort withEntityCacheOnly |
1700 ops/sec (±0.22%) |
1719 ops/sec (±0.23%) |
1.01 |
getResponse |
4797 ops/sec (±0.68%) |
4672 ops/sec (±0.35%) |
0.97 |
getResponse (null) |
10158776 ops/sec (±0.73%) |
9245068 ops/sec (±0.88%) |
0.91 |
getResponse (clear cache) |
268 ops/sec (±2.00%) |
272 ops/sec (±1.97%) |
1.01 |
getSmallResponse |
3291 ops/sec (±0.19%) |
3418 ops/sec (±0.11%) |
1.04 |
getSmallInferredResponse |
2551 ops/sec (±0.12%) |
2506 ops/sec (±0.07%) |
0.98 |
getResponse Collection |
4508 ops/sec (±0.41%) |
4638 ops/sec (±0.32%) |
1.03 |
get Collection |
4504 ops/sec (±0.21%) |
4601 ops/sec (±0.21%) |
1.02 |
get Query-sorted |
5286 ops/sec (±0.13%) |
5275 ops/sec (±0.44%) |
1.00 |
setLong |
453 ops/sec (±0.32%) |
458 ops/sec (±0.24%) |
1.01 |
setLongWithMerge |
260 ops/sec (±0.21%) |
261 ops/sec (±0.16%) |
1.00 |
setLongWithSimpleMerge |
275 ops/sec (±0.18%) |
275 ops/sec (±0.15%) |
1 |
setSmallResponse 500x |
932 ops/sec (±0.21%) |
935 ops/sec (±0.52%) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Jest's "worker process has failed to exit gracefully" warning appears when running all 3 test projects together due to open handles (GCPolicy's
setInterval, pending NetworkManager fetches) surviving past test completion. Every test file must manually callrenderDataHook.cleanup()inafterEach, which is easy to forget and leads to resource leaks.Solution
Two systemic fixes, modeled after
@testing-library/react's auto-cleanup pattern:@data-client/test— auto-cleanup viaafterEachmakeRenderDataClientregisters a module-levelafterEachhook (like RTL does) that drains all active manager cleanups after every test. EachrenderDataHook()call adds its cleanup to a sharedSet; the hook drains and clears it. ManualrenderDataHook.cleanup()/renderDataClient.cleanup()calls inafterEachare no longer needed.Removed manual cleanup from all 34 test files.
@data-client/core—.unref()on GCPolicy intervalThe 5-minute GC sweep
setIntervalnow calls.unref()in Node.js environments, preventing it from keeping Jest workers alive.flowchart TD RTL["RTL afterEach auto-cleanup"] -->|unmounts component| DP["DataProvider unmount"] DP -->|useEffect cleanup| Init["initManager cleanup"] Init --> NM["NetworkManager.cleanup()"] Init --> SM["SubscriptionManager.cleanup()"] Init --> GC["GCPolicy.cleanup()"] Auto["@data-client/test afterEach auto-cleanup"] -->|drains activeCleanups Set| Full["Full cleanup: reject fetches + clear managers"]Made with Cursor
Note
Medium Risk
Changes test teardown semantics by adding a global
afterEachhook and alters timer lifecycle behavior inGCPolicy; regressions would mainly surface as unexpected cleanup timing or lingering/early-disposed resources in tests.Overview
@data-client/testnow registers a module-levelafterEachthat automatically runs and clears all activemakeRenderDataClient()/renderDataHook()cleanups, reducing leaked managers/fetches and removing the need for per-test-filecleanup()boilerplate.@data-client/coreupdatesGCPolicyto call.unref()on its sweepsetIntervalwhen supported (Node), so the timer no longer blocks process exit. Docs/release notes are updated accordingly, and numerous tests drop manualrenderDataClient.cleanup()/renderDataHook.cleanup()calls (except where ordering with fake timers still requires it).Written by Cursor Bugbot for commit ce42b01. This will update automatically on new commits. Configure here.