Repository navigation
[CI] Speed up terminal probe tests with controlled policy clocks - #26327
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe interactive-wait visibility tests now use fake timers to reach the configured probe timeout. The concurrent-request test still checks that only one foreground probe runs. The wedged-probe tests check that the wait resolves to Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The timeout tests retain their stated assertions while avoiding real-time waits. No identified issue prevents merging after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
nwparker
changed the title
1 participant
ELI5
Two terminal approval tests now advance a controlled clock instead of waiting for real two-second probe deadlines. The same 27 tests and production behavior remain covered.
What Changed
After each test creates its terminal pane, fake only Date, setTimeout and clearTimeout while checking the injected foreground probe that intentionally never answers. Advance the existing production timeout constant, await the original results, and restore real timers in finally and afterEach. Other 25 test callbacks and every case name remain unchanged. Production code, native setup and real process checks are untouched.
Why
The probes are fully injected; elapsed desktop time adds no evidence to their timeout policy. Advancing that policy clock preserves the full deadline and single-flight assertions while removing roughly six seconds of deliberate waiting. Shorter production deadlines or dropped tests would weaken coverage.
Linked Issue
N/A — internal test performance work.
Visual Proof
N/A — test-only change; no rendered UI or interaction behavior changes.
Testing
pnpm tc:node), targeted formatting, changed-code quality and diff checks pass on base0f9f1993223f90991db9c60fe6e30ce2b7abcb78.pnpm testsamples pass all 27 cases; all source/configuration/routing and natural-close checks pass. Root independently replays the preserved collector and verifies every raw artifact hash.ORCA_BACKGROUND_LAUNCH=1.Before/after measurements
Three counted pairs on one held Linux ARM runner with four CPUs, stock four workers, isolation enabled, six setups, and Vitest filesystem module cache disabled. Node 24.21, Bun 1.4.2 (
744846f84), Vitest 5.0.3; frozen measured source29ad785b1cd000b580ba3b3da87affb835a601ad.Order: before/after, after/before, before/after. Every first command and slow sample is retained; no warmup, retry, discard or report normalization. Commands share Bun/runtime and OS caches, so arms are not independently cold even with Vitest filesystem cache off. These measurements apply to this single-file cohort; they do not establish a full-suite speedup, fleet latency or the overall 2× goal. The current target and relevant runtime dependencies retain the measured bytes; unrelated Node runtime registry additions are retained on current main. Full current integration still needs CI.
Review
Clock ownership, original assertions, timeout boundary, rejection observation and restoration were independently checked. No production timer, process cleanup, runtime contract or test exclusion changes.
Agent skill upstream boundary
Notes
The test policy is platform-independent and uses the existing runtime harness. No SSH host ownership, folder workspace, mobile or wire behavior changes. Linux ARM runtime qualification is measured; Windows/macOS execution is not inferred from it.
Checklist
Final integration and review evidence
Full run
37687898131, attempt 1, tests synthetic source3981dbe4c44c956373adad2321fd7780058a1222with parents current main0f9f1993223f90991db9c60fe6e30ce2b7abcb78and shipping headd504992b80552ef3d21a5290c57181cbaf6d765d. Its tree equals the shipping tree; only the one qualified test file differs from incoming main. The measured original/candidate test bytes remain exact.Root read all ten raw logs and assignment/timing artifacts and compared them with actual stock discovery: 11,700 physical files appear exactly once, 11,560 passed and 140 stock-skipped; 112,846 passed cases, three expected failures and 1,052 skips, with zero failed cases or unhandled errors. The changed file runs under Bun and passes all 27 cases. Upstream main adds 14 paths and retires 29 between the earlier 7b revision and this base; those main changes are preserved, and none are deleted by this PR. Counts do not imply full cross-revision named-case equivalence: stock full-CI artifacts provide aggregates, while complete original/candidate target identities were separately checked in the 12-command runtime qualification.
CodeRabbit explicitly reviews
0f9f1993… → d504992b…and reports zero actionable comments. The final review-thread query has zero threads and no omitted page. Independent peers and root separately verify every six-sample raw measurement/source/configuration/routing/natural-close gate; the table above is already present before merge, and all samples remain retained. These are cohort measurements on their frozen source, not a new measurement from this integration run.Actual merge source
Merged at 21:38:56 UTC as
31a5b42b0d272526c405458154eea932cd0ded8e. Two concurrent upstream PRs, #26071 and #25947, advanced main after the qualified CI base; the actual squash parent is661cba999e65350f7abdbdb98988fe55af4c2d5c. The merged tree therefore differs from the earlier tested synthetic tree. Root confirms this merge changes only the one qualified test file relative to that actual parent, preserves the exact shipping candidate bytes, and retains every named dependency/configuration guard from the premerge source assessment. The qualified CI coverage counts above remain observations on3981dbe4…; they are not a claim that additional concurrent upstream tests ran in that earlier run. The before/after table was published and verified before merge and remains unchanged.