Skip to content

[CI] Reduce test imports, clock waits, and headless cleanup costs - #29312

Merged
nwparker merged 13 commits into
mainfrom
nwparker/ci-transport-store-boundary-20261016
Oct 16, 2026
Merged

nwparker merged 13 commits into
mainfrom
nwparker/ci-transport-store-boundary-20261016

Conversation

@nwparker

@nwparker nwparker commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 28 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​532 $\color{#cf222e}{\Huge{\mathbf{−}}}$​120 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​412
Prod 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​62 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​60

ELI5

CI spends less time loading unrelated app code, waiting on test clocks, repeating replay formatting, and closing headless hosts. Existing behavior checks stay in place.

What Changed

  • Reuse smaller stores in transport and feedback tests, and the existing mobile runtime fixture for real pairing/listener tests.
  • Advance updater and native-chat observation clocks while still awaiting actual host work. Restore real timers before teardown so disposal deadlines can fire after a failure.
  • Count replay cells with the existing public width predicate instead of formatting descriptors in the test oracle.
  • Request existing daemon cleanup during final headless disposal and failed startup; keep restart behavior and public-close checks.
  • Exclude four exact test helpers from Computer E2E and Windows serve-switch smoke triggers. Their unit/static checks and package selection remain; production and mixed changes still select the smoke checks.
  • Disable background throttling in the hidden popup fixture while retaining hidden windows, first-frame checks, native assertions, and timeout limits.

Why

These changes remove setup and waits that the tests do not need. Reusing existing fixture APIs keeps real actions, subscriptions, IPC, server lifecycle, and behavioral assertions in place. The popup setting addresses hidden compositor scheduling; the original intermittent first-frame failure did not reproduce locally, so its cause remains unproved.

Local comparisons apply to separate cohorts and must not be added into a fleet speed estimate. The mobile four-root cohort retained all 47 cases across four matched AB/BA pairs: median paired command saving 3.558s, with sampled process-group RSS 20.45–26.98% lower in every pair. One pair regressed in summed worker phases, so full-shard savings remain unproved. The final combined checks below are a correctness refresh, not another benchmark.

Linked Issue

N/A — maintainer-requested CI capacity work.

Visual Proof

N/A — test and workflow changes; no product UI or interaction change. Hidden Electron qualification runs on an isolated display or CI.

Testing

  • Independent reviews of store boundaries, replay equivalence, routing, fixture cleanup, and clock behavior; the clock cleanup finding was corrected before publication.
  • Rebase preservation: all 11 imported patches matched their reviewed predecessors; all seven upstream worktree-create files and both reviewed mobile source hashes were retained before the separate cleanup correction.
  • Constituent Linux qualification at b461: 391 affected passes with two existing skips, 53 routing passes, 45 hidden Electron passes, and all four stock Bun CI compiler projects passed. Fresh-build headless qualification passed all 18 scenarios with no retries/skips/errors, matching 116 findings and 112 client-evidence records; independent process cleanup audit found no owned processes left. These results do not qualify the final integrated commit.
  • Final integrated commit b8a5ab93ca7427f106896792925072cbc31ef224: 439 affected tests passed with the same two existing skips (441 cases); all prior 393 constituent case identities/outcomes retained, all 47 mobile cases retained, and one cleanup regression added. All 53 routing cases passed. Direct Node compatibility also passed the 126-case clock cohort.
  • All four native TypeScript and four pinned Bun 1.4.3 CI compiler projects passed. Changed-code quality, formatting, ordinary Oxlint, and full anti-slop passed. Full pnpm lint stopped at five existing mobile import-cycle warnings across four untouched files; the later checks in that chained command were not claimed as run.

The first combined invocation omitted the Node-worker executable binding and exited nonzero with several roots unexecuted. The corrected repository-wrapper invocation above passed with explicit Node 24.20.0; its complete case catalog was verified. No source change or assertion retry was used to resolve that launcher error.

  • Exact final-head hosted qualification passed: full PR, Headless Node server, Computer E2E, Windows serve switch, macOS updater, LOC, and Mobile. The physical full-PR aggregate 116842549252 passed; Mobile was checked separately. Linux package qualification passed all 45 Electron boundary cases, including 39 popup cases with the real first-frame prerequisite retained. This does not prove permanent flake elimination.
  • Latest-main integration: all eight deadline/reattach cases passed on a clean temporary merge with main 7cf0c2a5, including its new regression and the strict store mock; original branch/head restored clean.

Hosted observations against the earlier qualified batch: the same 26 changed roots totalled 79.354→37.284 worker-seconds; the slowest unit test step was 355→339 seconds and summed five-shard test-step occupancy 1711→1652 seconds. Sources, shard assignments and diagnostic overhead differ, and this is one observation per condition. These are not isolated causal or fleet savings; no candidate import-phase split is available.

Review

Test isolation, case assertions, timeout budgets, retries, native/process checks, SSH host authority, and mixed-version RPC behavior are preserved. No library or native dependency update is included. Independent source reviews and the hosted checks above qualify the exact published commit; all executed required checks passed.

Agent skill upstream boundary

  • Not applicable; no upstream skill content copied.

Checklist

  • Before/after, mechanism, and choice explained.
  • Self-reviewed for security, cross-platform, SSH/remote, and compatibility impact.
  • Behavioral checks retained; no product UI change.
  • Final local checks and their limitations disclosed.
  • Exact published-head hosted checks passed.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

  • Last reviewed commit: c1687a6e69668c61696a165b4db7d4c2e2f71de7
  • Latest commit, not reviewed: b8a5ab9

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: 769e11e2-ce88-411e-9a4a-08810d02e70c
📥 Commits

Reviewing files that changed from the base of the PR and between a37adbf and c1687a6e69668c61696a165b4db7d4c2e2f71de7.

📒 Files selected for processing: 2
  • mobile/src/session/MobileNativeChatView.test.ts
  • src/shared/git-binary-compatibility.test.ts

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

📝 Walkthrough

Terminal-pane transport tests now fail if their fixtures read app-store ownership. Grid serialization uses buffer-line and cell-width checks to count clipped wide cells. Updater failure-cause tests use fake timers. Git fast-forward compatibility coverage moved to a shared test case. The mobile chat view test derives its override type from the component props.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to c1687

No actionable merge-blocking issue is identified; run the required checks on the reviewed head before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 22 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
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.
Title check Passed The title clearly summarizes the main CI optimization work: reducing test imports, clock waits, and headless cleanup costs. It is concise and specific.
Description check Passed The description is complete and covers the required explanation, changes, rationale, issue status, visual proof, testing, review, scope boundary, notes, and checklist information. It also documents li…
  • 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.

@nwparker
nwparker force-pushed the nwparker/ci-transport-store-boundary-20261016 branch from cebc515 to 26461cd Compare October 16, 2026 00:54
@nwparker nwparker changed the title [CI] Reduce repeated store imports in transport tests [CI] Reduce test work and avoid a Bun worker crash Oct 16, 2026
@nwparker
nwparker force-pushed the nwparker/ci-transport-store-boundary-20261016 branch from 26461cd to 30b6a1c Compare October 16, 2026 01:20
@nwparker
nwparker force-pushed the nwparker/ci-transport-store-boundary-20261016 branch from a37adbf to c1687a6 Compare October 16, 2026 02:35
@nwparker nwparker changed the title [CI] Reduce test work and avoid a Bun worker crash [CI] Reduce repeated imports and waits in tests Oct 16, 2026
@nwparker
nwparker force-pushed the nwparker/ci-transport-store-boundary-20261016 branch 4 times, most recently from a7ea99b to 4fd0456 Compare October 16, 2026 04:55
@nwparker nwparker changed the title [CI] Reduce repeated imports and waits in tests [CI] Reduce repeated imports, waits and headless test cleanup Oct 16, 2026
@nwparker
nwparker force-pushed the nwparker/ci-transport-store-boundary-20261016 branch from 4fd0456 to b8a5ab9 Compare October 16, 2026 05:34
@nwparker nwparker changed the title [CI] Reduce repeated imports, waits and headless test cleanup [CI] Reduce test imports, clock waits, and headless cleanup costs Oct 16, 2026
@nwparker
nwparker merged commit 629db0a into main Oct 16, 2026
59 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