mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
test(desktop): make runtime subprocess tests PATH-independent
Wren and Dawn both reproduced two more parallel-test flakes at the
merge head, same class as the grandchild_inherits_pgid fix one commit
earlier: claude_spawn_uses_the_probed_cli_executable holds
lock_path_mutex while swapping PATH to a tempdir, and any sibling test
doing a bare-name PATH lookup in that window fails with NotFound.
- own_group_grandchild_detected_by_ancestor_walk: bare `sh` ->
absolute /bin/sh, and the script's inner `sleep` -> /bin/sleep
(a child spawned during the swap window inherits the poisoned PATH
for life, so inner lookups must be absolute too)
- make_pair_runtime_placeholder (kill_stale_live_pair_is_not_touched):
bare `true` -> /usr/bin/true on unix (present on macOS and Linux);
Windows keeps the PATH lookup since no test there swaps PATH
- grandchild_inherits_pgid_of_process_group_leader: inner `sleep`
lookups also made absolute, closing the same residual window
Chose PATH-independence over taking lock_path_mutex (Wren's noted
preference): no new lock scope, no serialization of unrelated tests.
Test-only. Repro at 0ec684477: 5/8 scoped parallel runs failed
(cargo test --lib managed_agents::runtime::tests -- --test-threads=16).
After: 10/10 scoped runs green, 4/4 full managed_agents parallel runs
green (805/805 each).
Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
0ec684477d
commit
3aa4f69b7e
@@ -710,15 +710,20 @@ fn grandchild_inherits_pgid_of_process_group_leader() {
|
||||
// getpgid returned -1). The group is killed in cleanup, so the sleep
|
||||
// never runs to term.
|
||||
//
|
||||
// `/bin/sh` (absolute) rather than `sh`: parallel tests holding
|
||||
// `lock_path_mutex` legitimately swap PATH to a tempdir, and this test
|
||||
// doesn't need the lock — but a PATH lookup during the swap window
|
||||
// fails with NotFound (observed flake).
|
||||
// Absolute `/bin/sh` and `/bin/sleep` rather than bare names: parallel
|
||||
// tests holding `lock_path_mutex` legitimately swap PATH to a tempdir,
|
||||
// and this test doesn't need the lock — but a PATH lookup during the
|
||||
// swap window fails with NotFound, and a child spawned during it
|
||||
// inherits the poisoned PATH for its lifetime, so the script's inner
|
||||
// lookups must be absolute too (observed flake).
|
||||
let mut harness = {
|
||||
let mut cmd = Command::new("/bin/sh");
|
||||
cmd.args(["-c", "/bin/sh -c 'sleep 10 & echo $!' & wait $!; sleep 10"])
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.process_group(0);
|
||||
cmd.args([
|
||||
"-c",
|
||||
"/bin/sh -c '/bin/sleep 10 & echo $!' & wait $!; /bin/sleep 10",
|
||||
])
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.process_group(0);
|
||||
cmd.spawn().expect("spawn harness")
|
||||
};
|
||||
|
||||
@@ -802,9 +807,15 @@ fn own_group_grandchild_detected_by_ancestor_walk() {
|
||||
// The test process is the "harness". Spawn an intermediate with its own
|
||||
// process group (mirrors the node shim). It backgrounds a grandchild
|
||||
// (sleep 30) and prints the grandchild PID so we can inspect it.
|
||||
//
|
||||
// Absolute `/bin/sh` and `/bin/sleep` — no PATH lookups anywhere in this
|
||||
// tree. Parallel tests holding `lock_path_mutex` legitimately swap PATH to
|
||||
// a tempdir; the outer spawn during that window fails with NotFound, and a
|
||||
// child spawned during it inherits the poisoned PATH for its lifetime, so
|
||||
// inner lookups must be absolute too (observed flake).
|
||||
let mut intermediate = {
|
||||
let mut cmd = Command::new("sh");
|
||||
cmd.args(["-c", "sleep 30 & echo $!; wait"])
|
||||
let mut cmd = Command::new("/bin/sh");
|
||||
cmd.args(["-c", "/bin/sleep 30 & echo $!; wait"])
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.process_group(0);
|
||||
cmd.spawn().expect("spawn intermediate")
|
||||
@@ -1232,7 +1243,16 @@ fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRun
|
||||
use std::process::{Command, Stdio};
|
||||
// Spawn a real child so ManagedAgentProcess's Child field is satisfied.
|
||||
// `true` exits immediately with 0 — just a handle we need for type purposes.
|
||||
let child = Command::new("true")
|
||||
//
|
||||
// Absolute `/usr/bin/true` on unix (present on both macOS and Linux):
|
||||
// parallel tests holding `lock_path_mutex` swap PATH to a tempdir, and a
|
||||
// bare `true` lookup during that window fails with NotFound (observed
|
||||
// flake). Windows keeps the PATH lookup — no test there swaps PATH.
|
||||
#[cfg(unix)]
|
||||
let program = "/usr/bin/true";
|
||||
#[cfg(windows)]
|
||||
let program = "true";
|
||||
let child = Command::new(program)
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
|
||||
Reference in New Issue
Block a user