From 55bea498fe8b7021670677f6a41e46cb93a63a44 Mon Sep 17 00:00:00 2001 From: masashiono0611 Date: Wed, 12 Aug 2026 00:05:25 +0900 Subject: [PATCH] fix(tests): widen _wait_pidfile's window and name what it saw (#595) A watcher relaunch is a real fork + lock-acquire + SIGTERM-the-predecessor + self-write before the pidfile reflects it, and the 3s this polled for could lose that race on a loaded CI runner -- the flake #595 caught on PR #436's macos-latest 4/4 shard, at _wait_pidfile "$pf" "$w2". Of #595's four sites, three are already fixed independently: marker-gc and codex-monitor by #606, the launcher re-registration race by #615. This is the fourth and last: _wait_pidfile in test_watch.bats itself, still on its original budget. - Widened to 10s, matching the launcher suite's wait_for_child_count budget (test_codex_bridge_launcher.bats) rather than inventing a new number. - On timeout, reports the pidfile path, the wanted pid, and what it last read instead -- #595 asked for a message that can tell "never arrived" from "arrived as something else"; a bare assertion failure at this line couldn't. Not a hoist into a shared helper: #595 suggested one, but the launcher suite's wait_for_child_count is a different predicate shape (process count, not pidfile content) with no shared caller today, and unifying that pattern is a design call for whoever wants it, not implied by fixing this flake. Verified: full tests/test_watch.bats (27/27) before and after; the target test individually; and the timeout path's message format tested standalone against a forced-unreachable pid. --- tests/test_watch.bats | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/test_watch.bats b/tests/test_watch.bats index 2ad35e370..43a649abd 100644 --- a/tests/test_watch.bats +++ b/tests/test_watch.bats @@ -395,13 +395,21 @@ _max_message_id() { # --- #93: parallel --continue/--resume sessions sharing a session_id --- -# Poll up to ~3s for to record . +# Poll up to ~10s for to record . A watcher relaunch does +# a real fork + lock-acquire + SIGTERM-the-predecessor + self-write before the +# pidfile reflects it, and a loaded CI runner can push that past the 3s this +# used to allow -- the flake #595 caught on a macos-latest shard. On timeout, +# reports what it was waiting for and what it last saw, per #595's ask for a +# failure message that distinguishes "never arrived" from "arrived as +# something else" rather than a bare assertion failure. _wait_pidfile() { - local pf="$1" want="$2" i - for i in $(seq 1 30); do - [ -f "$pf" ] && [ "$(cat "$pf" 2>/dev/null)" = "$want" ] && return 0 + local pf="$1" want="$2" i seen + for i in $(seq 1 100); do + seen="$(cat "$pf" 2>/dev/null || true)" + [ -f "$pf" ] && [ "$seen" = "$want" ] && return 0 sleep 0.1 done + echo "_wait_pidfile: timed out waiting for '$pf' to record pid $want (last saw: '${seen:-}')" >&2 return 1 }