Skip to content

test: remove elapsed waits from hidden terminal replay checks - #29667

Merged
OrcaWin merged 1 commit into
mainfrom
OrcaWin/test-audit-hidden-replay-clock
Oct 16, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
OrcaWin/test-audit-hidden-replay-clock

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

ELI5

Two terminal replay tests spend 1.44 seconds waiting for timers. They now advance those same timers directly while still letting the real replay code and queued parser callbacks run.

What Changed

Use Vitest's clock in the two existing resize/replay cases and replace their 60ms sleeps with 60ms clock advances. Preserve all eight rounds in each loop, Promise settling, FIFO parser callbacks, module isolation, inputs, assertions and disposal. No production changes or new test hooks.

Why

The waits drive background flushing and fit retries. Keeping the same time steps and callback order removes elapsed waiting without removing the behavior checks. Sharing modules was rejected because the fixtures retain subscriptions and scheduler state across cases.

Linked Issue

Internal CI audit; no new issue. Retains existing hidden terminal recovery contracts.

Visual Proof

N/A — test scheduling only; no UI change.

Testing

  • I manually tested these changes locally (automated Linux runs and deliberate production regression controls).
  • Existing automated tests updated without deleting cases.

Three alternating full-wrapper pairs pass the same 94 cases, zero unhandled errors, pinned Node24.21.0/Bun1.4.3, one worker, background launch. Median wall 8.802 → 7.315s, module 7.151 → 5.666s, execution 6.940 → 5.451s. Collection remains similar. These are local measurements; hosted comparison remains required.

All 175 cases across five owner/sibling suites pass with shuffle seed104729: replay wrapper, startup snapshot, replay guard, fit and output scheduler. Renderer typecheck, changed-code quality, scoped lint/format and whitespace checks pass.

Two deliberate production defects fail intended assertions: dropping the later background repaint fails its text check; omitting skipped-frame recovery gets one snapshot instead of two. Production restored byte-for-byte. A speculative signal insertion before the fit branch stays green in both original/candidate tests, so it supplies no extra suppression proof; all original negative assertions remain.

AI Disclosure

Review

Independent review checks timer semantics, fixture lifetime and exact inverse preservation. Final independent preservation and hosted qualification passed. Full PR workflow 38901382074 and aggregate verify 116977636847 passed at exact head 5d435f0; all 30 checks succeeded or intentionally skipped. All five inventories reconcile, 12,362 files with no omissions or unhandled errors. Hosted wrapper module 7.970 → 7.171s, execution 7.814 → 6.994s, all 94 cases retained. This single comparison has three test-file context differences and changed worker interleaving; it is a descriptive selected-suite observation, not a whole-CI or fully causal saving claim.

Agent skill upstream boundary

  • Not applicable; no upstream skill source copied or translated.

Notes

One test support file +22/-2, production0. Fake timeout/interval/Date/performance use one clock; native Promise callbacks and existing synchronous animation-frame fixture remain. This proves the existing mocked transport/parser boundary, not real Chromium or OS timer scheduling. Local, SSH and paired-host cases remain in the full wrapper. No wire, platform or folder-workspace behavior changes.

Checklist

  • Small and focused; mechanism and alternatives explained
  • Visual proof N/A; platform/SSH compatibility considered
  • Local measurements, sibling checks and intended controls recorded
  • Independent final review and exact-head hosted measurement/full PR workflow passed before merge

@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 69009f56-4da3-4672-80d0-f7118632623b
📥 Commits

Reviewing files that changed from the base of the PR and between 78f2e9b and 5d435f0.

📒 Files selected for processing: 1
  • src/renderer/src/components/terminal-pane/pty-connection-hidden-snapshot-resize-signals.spec.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Two tests now enable fake timers for timeout, interval, date, and performance APIs. The write-settling and replay loops advance fake time by 60 ms instead of waiting for real-time delays.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 5d435

The reported affected suites passed, and teardown restores real timers between tests. No concrete merge-blocking behavior change is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly describes the main change: replacing elapsed waits in hidden terminal replay tests. It is concise and specific.
Description check Passed The description is detailed and covers the change, rationale, testing, scope, and limitations. It explains that no issue is linked because this is an internal CI audit, and it clearly identifies the r…
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.
Full details: Description check

Explanation

The description is detailed and covers the change, rationale, testing, scope, and limitations. It explains that no issue is linked because this is an internal CI audit, and it clearly identifies the remaining hosted measurement and independent review requirements.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 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.

❤️ Share

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

@OrcaWin
OrcaWin merged commit 183d142 into main Oct 16, 2026
30 checks passed
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