main is red: tui_review_test is flaky and order-dependent #82

New issue

Closed

opened 2026-08-13 07:07:57 +00:00 by a73x · 0 comments

a73x commented

2026-08-13 07:07:57 +00:00

(Migrated from git.a73x.sh)

main at 15bc9a3 fails cargo test. Exit 101.

I merged it on a green full-suite run in the agent's worktree, so the verification passed and was not reliable — that is the part worth fixing, not just the test.

Diagnosis so far:

how it is run	result
the single test alone	passes, 3.7s
whole binary, default parallelism	fails — 1 or 2 tests, varying between runs
whole binary, `--test-threads=1`	fails — 1 test, 74s for 12 tests

Failing names seen: an_editor_killed_mid_write_records_nothing, a_comment_only_body_aborts_and_records_nothing. Which of the two fails varies; sometimes both.

Passing alone but failing in-binary rules out the test's own logic. Serial failure rules out plain parallel interference. What is left is shared state between tests in the binary, or a timing assumption that holds when a test starts on an idle machine and not when it starts after eleven pty sessions. 74 seconds for 12 tests says the harness spends most of its time waiting on fixed delays, which is the usual source of exactly this.

Both failures are in the $EDITOR path, which is the newest and most timing-dependent part of run_dashboard_driven — it spawns a pty, drives stdin, and waits for an external process to write a file and exit.

Wanted: make the harness wait on observable conditions rather than elapsed time — the editor's output file existing, the process having exited, the expected text appearing on screen — with a generous timeout as a backstop rather than as the mechanism. A test that passes because a sleep was long enough is a test that fails on someone else's machine.

Also worth having: run the file repeatedly in CI, or at least once with --test-threads=1, so an order dependency cannot hide again.

Migrated from git-collab issue 79fd7273620072de46a8501915defb79edf09fc6.

Original author: a73x dev@a73x.sh.

Close reason: merged in patch 4a2dba54

`main` at 15bc9a3 fails `cargo test`. Exit 101. I merged it on a green full-suite run in the agent's worktree, so the verification passed and was not reliable — that is the part worth fixing, not just the test. Diagnosis so far: | how it is run | result | |---|---| | the single test alone | **passes**, 3.7s | | whole binary, default parallelism | fails — 1 or 2 tests, varying between runs | | whole binary, `--test-threads=1` | fails — 1 test, 74s for 12 tests | Failing names seen: `an_editor_killed_mid_write_records_nothing`, `a_comment_only_body_aborts_and_records_nothing`. Which of the two fails varies; sometimes both. Passing alone but failing in-binary rules out the test's own logic. Serial failure rules out plain parallel interference. What is left is shared state between tests in the binary, or a timing assumption that holds when a test starts on an idle machine and not when it starts after eleven pty sessions. 74 seconds for 12 tests says the harness spends most of its time waiting on fixed delays, which is the usual source of exactly this. Both failures are in the `$EDITOR` path, which is the newest and most timing-dependent part of `run_dashboard_driven` — it spawns a pty, drives stdin, and waits for an external process to write a file and exit. Wanted: make the harness wait on **observable conditions** rather than elapsed time — the editor's output file existing, the process having exited, the expected text appearing on screen — with a generous timeout as a backstop rather than as the mechanism. A test that passes because a sleep was long enough is a test that fails on someone else's machine. Also worth having: run the file repeatedly in CI, or at least once with `--test-threads=1`, so an order dependency cannot hide again. --- Migrated from git-collab issue `79fd7273620072de46a8501915defb79edf09fc6`. Original author: a73x <dev@a73x.sh>. Close reason: merged in patch 4a2dba54