test: de-flake intermittent CI tests (parallel-load resilience) - #252
Merged
Conversation
Two shared test-harness helpers used by multiple flaky suites under
process-per-test nextest parallelism:
1. `git_program()` resolves `git` to an absolute path once per process
(with an optional `GIT` override), so no per-spawn PATH walk happens.
Under heavy concurrent fork/exec a bare `Command::new("git")` can
transiently fail with ENOENT ("No such file or directory") even though
git is installed. Callers pair this with a small ENOENT retry.
2. `http_call_with_retry()` + `is_transient_connection_error()` retry a
request that fails at the connection level (peer disconnected /
connection refused / reset) with bounded backoff. A freshly started
dashboard server can accept-then-drop a socket during startup, racing
the client. HTTP status errors are NOT retried (the agent is built with
`http_status_as_error(false)`), so only never-completed requests retry.
`get_json` now uses it, and `wait_for_dashboard` polls until the server
returns a real 2xx, not just until a bare connect succeeds.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flake #1: `plans_branch_switch_from_session_worktree_against_worktree_root` and `plans_ambiguous_git_change_from_session_worktree_with_worktree_branch` intermittently failed with `git ...: No such file or directory` under CI's process-per-test nextest parallelism. Root cause: a bare `Command::new("git")` PATH lookup can transiently fail the spawn with ENOENT under heavy concurrent fork/exec load. Fix (test only): `run_git` now resolves git to an absolute path once (via a process-local `OnceLock` mirroring the shared `common::git_program()`), asserts the cwd exists before spawning, and retries a transient spawn ENOENT a few times with a short backoff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flake #2: dashboard API tests (e.g. `automation_config_is_dashboard_controllable_and_persistent`, `automation_jobs_crud_and_manual_run_are_dashboard_controllable`) intermittently failed with `... failed: io: Peer disconnected` when the client raced the dashboard server's startup under parallel load. Fix (shared harness): the dashboard POST/PATCH/DELETE helpers now route through `common::http_call_with_retry`, so a request that fails at the connection level (peer disconnected / refused / reset) is retried with bounded backoff. Combined with `wait_for_dashboard` now polling for a real 2xx before the first request, all dashboard tests benefit. The local `git` helper also uses the resilient absolute-path lookup + ENOENT retry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flake #3: `broker_bounds_lsp_document_write_hangs` and `broker_drops_lsp_client_after_partial_diagnostics_frame_timeout` intermittently failed on loaded runners. Both tests force an engine crash, then re-run a healthy fake LSP server and assert it recovers. The recovery collection paid a tight 150ms (`FAKE_LSP_TIMEOUT`) wall-clock budget that must cover a real python spawn plus a didOpen -> publishDiagnostics round trip. Under load that spawn can exceed 150ms, so the recovery `refresh_documents(...).unwrap()` false-fired with a spurious timeout. Fix (test only): the two recovery calls now use a dedicated generous `FAKE_LSP_RECOVERY_TIMEOUT` (3s), well under the 6s outer async bound so a genuine hang is still caught. The crash-forcing first half keeps its tight 50ms internal timeout — it is *meant* to time out, and load only makes that expected outcome more certain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flake #4: `cursor_workspace_open_hook_notifies_daemon` (and the sibling hook-notification tests sharing `assert_hook_notification`) intermittently panicked under parallel load. The fake-daemon harness bounded several waits at a tight 2s: the socket accept deadline and the `recv_timeout` for the observed request both depend on spawning and running the real `tracedecay` CLI as a child process, whose fork/exec + init can be scheduled slowly under nextest's process-per-test parallelism. Fix (test only): introduce `CLI_ROUNDTRIP_TIMEOUT` (20s) for waits gated on the external CLI and `LOCAL_READY_TIMEOUT` (10s) for in-process readiness signals, replacing the 2s bounds. Generous ceilings that still fail fast on a genuine hang (the CLI normally connects in well under a second). The local `git` helper also gains the resilient absolute-path lookup + ENOENT retry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flake #5: the codex app-server / model backend tests (the success-path `codex_app_server_*` / `fake_codex_app_server_*` cases) flaked under parallel execution. `fake_codex_response_timeout()` was 5s on non-Windows — the budget for the fake codex child (a real python interpreter) to spawn and complete its scripted turn. Under process-per-test parallelism that spawn can be scheduled slowly enough to exceed 5s, yielding a spurious "timed out waiting for codex app-server response". Fix (test only): use a uniform generous 30s success-path budget (matching the prior Windows value). Tests that deliberately exercise the timeout path pass their own tight `Duration` (e.g. 300ms) and are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the intermittently-failing tests that have been dodging CI — each passes in isolation but fails under CI's parallel
nextest(process-per-test) load. All changes are test-harness/test-side only; no product behavior changes. Reproduced by saturating CPU (load avg ~455 on 64 cores) and confirmed stable there.Fixes
src/mcp/hook_events.rs) — bareCommand::new("git")does a PATH walk per spawn and transiently ENOENTs under heavy fork/exec.run_gitnow resolvesgitto an absolute path once (OnceLock,GIToverride), asserts cwd exists, and retries transient spawn ENOENT with backoff (sharedcommon::git_program()). 25/25 under load.tests/common/mod.rs+dashboard_api_support.rs) — client raced server startup ("Peer disconnected"). Newhttp_call_with_retry/is_transient_connection_errorretry connection-level failures only (not HTTP status errors, so POST/PATCH/DELETE stay safe);wait_for_dashboardpolls until a real 2xx. 6/6 automation suite + 53/53 dashboard suite under load.lsp_code_diagnostics_test.rs) — the crash-recovery half re-ran a healthy fake LSP under a 150ms budget that must cover a python spawn + round trip; gave recovery a load-tolerantFAKE_LSP_RECOVERY_TIMEOUT(3s, still under the 6s outer bound). The crash-forcing half keeps its tight 50ms (meant to fire). 25/25 under load.tool_daemon_test.rs) — 2s accept/recv bounds depended on spawning the real CLI child; raised toCLI_ROUNDTRIP_TIMEOUT(20s) /LOCAL_READY_TIMEOUT(10s) — generous ceilings that still fail fast on a genuine hang. 25/25 under load.automation_runner_test/backend.rs) — 5s success budget for the fake codex python child; raised to 30s. Deliberate-timeout tests pass their own 300ms and are unaffected. 20/20 under load.Validation
cargo fmt/cargo clippy --all-targets -- -D warningsclean (only vendored libsql warnings).cargo nextest run --lib --test hooks_lsp_suite --test core_cli_suite --test automation_runner_test: 1235/1235.cargo nextest run --test dashboard_api_test: 53/53.No assertions weakened, no
#[ignore]added — the fixes make the waits deterministic/load-tolerant, not the tests laxer.🤖 Generated with Claude Code