Skip to content

test(cli): kill the CLI when a process test fails, so the run cannot hang - #104

Open
tee8z wants to merge 1 commit into
masterfrom
test/terminal-harness-cleanup
Open

tee8z wants to merge 1 commit into
masterfrom
test/terminal-harness-cleanup

Conversation

@tee8z

@tee8z tee8z commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A test that drives the CLI and fails before it reaps the process leaves the process running. For example, a terminal test can time out waiting for a prompt. The blocking task that reads the process's terminal, or waits for its exit, then never returns. The Tokio test runtime waits for blocking tasks when it shuts down, so one failure hangs the whole cargo test run, instead of failing it.

This happened during reviews. On the head of #94, the integration suite stopped for 28 minutes in login_on_a_terminal_reports_why_no_profile_was_saved_and_still_succeeds, with a voltage login process still waiting at its prompt. In the same reviews, the #75 terminal tests also failed 5 times at their 10-second limits, while instrumented builds ran in parallel. Each passed when rerun.

Change

Before After
A test fails while the CLI still runs The process keeps running, and the run hangs KillOnDrop kills the process as the test unwinds, and the test fails
A process does not exit in time timeout(...).unwrap() panics, and the blocking wait remains, or each test kills the process itself exit_within fails with a message, and the guard kills the process
A terminal step (on_a_terminal_with) 10 seconds to show its prompt, and 10 seconds to exit 30 seconds for each (TERMINAL_STEP)
  • KillOnDrop is created right after each spawn. exit_within disarms it once the process is reaped, because a reaped process ID can be reused.
  • Every async test that waits for a CLI process with spawn_blocking now uses exit_within: the terminal and hidden-prompt harnesses, the Ctrl-C tests, the checkout stream, progress, and early-closing-pipe tests, fully_on_a_terminal_ending, and the four payments list --watch tests from feat(cli): follow payments live with payments list --watch #111. The three hand-written kill -KILL blocks are removed. A --watch process runs until Ctrl-C, so a watch test that failed before its interrupt would hang the run without the guard.
  • A device login waits out its 5-second poll interval before its first prompt, so 10 seconds left little margin on a loaded machine. A passing step finishes well within the new limit, which only bounds a failure.

Only tests/cli_integration.rs changes, and no behavior of the CLI changes.

Checks

  • The hang, reproduced: I changed one terminal test to wait for a prompt that never appears. On master, the run was still going after 60 seconds, and an outer 120-second timeout killed it (exit 124). On this branch, the test fails after 30.01 seconds with exit 101, and no CLI process is left. That change is not committed.
  • Verification gate, in nix develop: cargo fmt --check, cargo clippy --locked --all-targets -- -D warnings, cargo machete --with-metadata, cargo test --locked (197 unit, 149 integration) and python3 scripts/check-coverage.py pass.
  • CRAP: 0 of 841 measured functions are at or above 20. The highest is wait_for_payment at 18.10, which is unchanged.
  • Watch tests: after the rebase onto master, I changed one watch test's assertion so it failed before its Ctrl-C. The test failed in 1.1 seconds and left no CLI process. That change is not committed.
  • I reviewed the diff with voltage-pr-review.

Limits

  • I could not reproduce the timing failures on demand. The two login tests passed 3 of 3 runs on both branches under 32 busy loops. Three full integration suites at once passed 6 of 6 per branch. Four loops beside a CRAP build passed on master. The 30-second limit is a margin based on the failures I saw, and it is not proven to fix them.
  • A drain still waits if another process holds the pseudo-terminal open, which needs a concurrent spawn between openpty and the FD_CLOEXEC calls. This PR does not change that.

…hang

When a test that drives the CLI failed before it reaped the process, for
example at a prompt that never appeared, the process kept running. So did
the blocking task that read its terminal or waited for it, and the test
runtime waits for such tasks at shutdown, so the whole run hung.

Each of these tests now holds a KillOnDrop guard from spawn, and waits
through exit_within, which kills the process on a timeout and disarms
the guard once the process is reaped. A terminal step may now take 30
seconds instead of 10, because a device login waits out a 5-second poll
interval before its first prompt.
@tee8z
tee8z force-pushed the test/terminal-harness-cleanup branch from c4e9c64 to 5278048 Compare October 26, 2026 23:06
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