Repository navigation
test: stabilize Darwin cancellation drain assertions - #29477
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
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 | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly identifies the test-only change: stabilizing Darwin cancellation drain assertions. |
| Description check | |
The description is detailed and covers the change, motivation, testing, scope, compatibility considerations, and checklist. It identifies the observed failure in issue #29471, explains why visual proo… |
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
ELI5
A macOS terminal-name cancellation test failed in CI because real timers no longer provided the observation window it assumed. Control the clock in the two affected cases so a busy runner cannot finish the active read before cancellation is checked.
What Changed
Use scoped fake timers for the candidate-read and directory-scan cancellation tests. Each starts a read, aborts, proves the operation remains pending while that read is active, then drains it and verifies no later reads were admitted. Cleanup drains pending work and restores real timers even after a failed assertion. Production behavior is unchanged.
Why
The failed run 38856466593 expected an operation to remain pending after a real 2 ms delay followed by a zero-delay timer, while the simulated read lasted 10 ms. Scheduler delays can invalidate that window. The adjacent scan test had the same assumption. Controlling time preserves both distinct cancellation contracts without increasing sleeps or removing the failing assertion.
Linked Issue
Internal test audit; observed failure in #29471.
Visual Proof
N/A — test-only change with no application UI or interaction change.
Testing
All 42 owner cases pass in 12 alternating original/candidate runs across Bun 1.4.3 and Node 24.21, and shuffled runs on both runtimes. All 54 owner/sibling cases pass. Node typecheck, changed-code-quality checks, normal/anti-slop lint, formatting and whitespace checks pass. All 31 declarations remain; only these two bodies and the timer import change.
Four deliberate owner regressions fail at their intended guards: admitting later candidate reads, admitting later scan reads, rejecting before a candidate read drains, and rejecting before a scan read drains. Production source restored byte-for-byte. The fake filesystem exercises the Darwin contract on Linux; no macOS device or desktop run claimed.
Three alternating pairs per runtime, original first; single worker, official module reporter:
These small timing differences establish no overall speed improvement. Hosted run 38858886806 attempt 2 passed the full PR aggregate and all five unit shards with all 42 cases passing. Official module duration was 228 → 207 ms; ordinary execution was 176 ms and is a separate measure. Baseline and candidate used Node 24.19/Bun 1.4.3, four workers/five shards and Blacksmith Ubuntu 24.04 X64 runners. The actual candidate test and unchanged production owner were source-verified. Broader source/dependency changes, HOME setup, workload (12,205 → 12,230 files), runner instance and scheduling differ, so this unpaired result establishes no causal or whole-workflow speedup. Attempt 1 lacked timing artifacts and is excluded.
Review
Independent diagnosis, pre-review and final preservation review approved the scoped-clock repair, both distinct cancellation contracts, cleanup and all four mutation controls.
Agent skill upstream boundary
Notes
No production, wire, SSH, folder workspace, Windows or Linux behavior changes.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)