Skip to content

test: make five CI-flaky unit and contract tests reliable - #1653

Merged
devswha merged 5 commits into
mainfrom
fix/flaky-tests
Oct 22, 2026
Merged

devswha merged 5 commits into
mainfrom
fix/flaky-tests

Conversation

@devswha

@devswha devswha commented Oct 22, 2026

Copy link
Copy Markdown
Owner

Change

Makes five tests that failed on CI pass reliably. All five failures were in the tests' own setup, not in product code; no product code changes. No timeout, retry, fixed sleep or weakened assertion was added.

  • omo multiline paste (server/core/omo-pane-send.test.ts, failed on main 10-22 05:29Z and 08:15Z: expected bracketed paste, received bare a\nb).
    • Cause: the fixture shell printed "ready" and then exec'd sleep. During that exec, Linux can briefly show an empty /proc/<pid>/environ, so the composer missed OMO_CODING_AGENT_DIR and the holder record.
    • Fix: the ready shell now stays blocked on stdin, and cleanup waits for it to exit.
    • Under load: 7 failures in 500 runs before, 0 in 500 after.
  • Heavy paging tests (opencode.test.ts 1.x and long-session paging and /undo, conversation.test.ts Devin cache and Agy ledger; 60 s timeouts at 65-108 s on CI).
    • Cause: the SQLite fixtures synced every row to disk. Devin alone spent 11.2 s in 496 fsync calls.
    • Fix: the throwaway fixture writers set PRAGMA synchronous = OFF; committed visibility, locking, data volume and assertions are unchanged. This follows test(sessions): build the rollback-journal fixture without setup syncs #1578.
    • Per test, alone: 5-20 s before, under 0.2 s after.
    • With eight heavy files at once: 1,506 pass, 0 fail.
  • OpenCode v2 contract tests (opencode.contract.test.ts: @effect queued while the pane works, failed 3 times, once on main; first key never waits on the event stream).
    • Cause: the pending receipt confirmed the PTY write before the stand-in had read Enter, and cases moved on before the previous send was consumed.
    • Fix: each case waits for the stand-in's byte reports to confirm consumption, and matches reports by their offset in the shared log.
    • Results: 1 failure in 5 runs before, 0 in 13 after. The whole file run 3 times: 60 pass.

Also checked: the Portal "unrelated process" failures on main (10-22 06:17Z and 06:33Z) predate #1638, which fixed them. That fixture fails 3 times in 500 runs without #1638's change and never with it.

Validation

  • bun run check fast: passes.
  • The changed files on this branch: unit 170 pass, contract 20 pass.

Test-only changes; no changelog fragment.

The ready shell exec'd sleep while the composer read its environment. Linux can expose an empty /proc/pid/environ during exec, hiding the fixture's store and dropping bracketed paste markers. Keep the ready shell blocked on stdin instead, validate its signal, and await fixture exit during cleanup.
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 4dd760f and 20cb46b.

📒 Files selected for processing: 4
  • server/conversation.test.ts
  • server/core/omo-pane-send.test.ts
  • server/opencode.contract.test.ts
  • server/opencode.test.ts

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

📝 Summary

Summary by CodeRabbit

  • Tests
    • SQLite-based test fixtures now use reduced synchronous disk-write settings while retaining their existing coverage of committed visibility and locking.
    • Process cleanup and terminal-input checks now wait for child processes to exit and for complete input to be captured, improving the reliability of test results.

Test fixtures now disable SQLite synchronous writes, wait for Omo child processes to exit, and synchronize OpenCode contract tests with typed-input reports and byte offsets.

Changes: Test reliability

Layer / File(s) Summary
SQLite fixture setup
server/conversation.test.ts, server/opencode.test.ts
The test database fixtures disable SQLite synchronous writes. Comments in the OpenCode fixtures clarify that they test committed visibility and locking, not crash durability.
Omo test process lifecycle
server/core/omo-pane-send.test.ts
The fixture checks its readiness output while keeping the shell process alive. Cleanup waits for tracked children to exit after sending termination signals.
Typed-input report synchronization
server/opencode.contract.test.ts
The stand-in reports input chunks with starting byte offsets. Tests wait for typed input ending in Enter and, for queued delivery, the corresponding pending-message removal.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Other

Possibly related issues

  • devswha/herdr-web-ui#1610: The PR updates the Omo test fixture setup, but does not implement the issue’s proposed server validation.
  • devswha/herdr-web-ui#1510: The PR changes the contract test’s byte-report synchronization for the load-sensitive failure.
  • devswha/herdr-web-ui#1556: The PR disables synchronous writes in the Devin history-cache fixture associated with the reported timeout.

Suggested reviewers: ploppy3

Merge Risk: ⚪ Minimal · up to 20cb4

The test-reliability changes preserve the tested behaviors without an established merge-blocking risk.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: improving reliability for five flaky CI tests.
Description check Passed The description includes the required Change and Validation sections. It explains the failure causes, test-fixture changes, scope, reported results, and validation commands.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • 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.

@devswha
devswha merged commit 4ad674c into main Oct 22, 2026
17 checks passed
@devswha
devswha deleted the fix/flaky-tests branch October 22, 2026 13:47
devswha added a commit that referenced this pull request Oct 23, 2026
…their limits under load (#1728)

## Change

Test cost only, no assertion removed:

- The Agy paging case uses 51 prompts instead of 80, still crossing the
50-prompt page boundary.
- Devin fixtures insert inside transactions, and the seven-read
history-cache case gets a deadline sized to its work instead of bun's 5
s default.

The OpenCode paging tests #1471 also names already got a mitigation in
#1653 and are unchanged.

Fixes #1471
Fixes #1556

## Validation

- `bun test server/conversation.test.ts server/devin.test.ts`: 154 pass.
Local per-test times dropped (Devin real-store cases from about 1.3 s to
under 130 ms each); the loaded CI host itself was not reproduced.
- `bun run check fast`: pass.
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