From c71d711dd4866d375f0b6562b461bd3f68ef666f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 24 Jul 2026 17:37:06 -0400 Subject: [PATCH] test: fix hostile-decoy env race flaking the setup-matrix and e2e suites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI flake (seen twice: setup_matrix_pypi host_guard::poetry_check_recognizes_structural_hook_host, exit 2 with a clap --help hint): several test binaries set hostile ambient env decoys PROCESS-WIDE (std::env::set_var) to prove the child-env scrub in their run helpers works, while sibling tests run on parallel threads. The scrub snapshots std::env::vars_os() and then spawns; a set_var landing between a sibling's snapshot and its spawn reaches the child un-scrubbed and (for SOCKET_STRICT=banana etc.) aborts its arg parse with exit 2. Mechanism proven RED/GREEN by making the schedule deterministic: a 10ms delay before the decoy set (simulating the late thread start CI hits) plus a 25ms window between snapshot and spawn fails 2/3 host_guard tests on EVERY run without the fix, and 0/5 runs with it (probes not committed). Fix, applied to every binary that mutates process env with >1 test (setup_matrix_{pypi,npm,deno,maven,nuget}, e2e_safety_cow, e2e_vendor_pypi_build): - #[serial_test::serial] on every test in the binary — serial only excludes other #[serial] tests, so annotating just the mutator would fix nothing - smc::DecoyGuard, an RAII setter that removes the decoys on drop, so a panicking assertion can't leave the process env poisoned for the tests that run after it (npm and nuget previously never removed their decoys at all) Co-Authored-By: Claude Fable 5 --- .../socket-patch-cli/tests/e2e_safety_cow.rs | 6 ++++ .../tests/e2e_vendor_pypi_build.rs | 2 ++ .../tests/setup_matrix_common/mod.rs | 30 +++++++++++++++++++ .../tests/setup_matrix_deno.rs | 10 ++----- .../tests/setup_matrix_maven.rs | 10 ++----- .../tests/setup_matrix_npm.rs | 12 ++++++-- .../tests/setup_matrix_nuget.rs | 6 ++-- .../tests/setup_matrix_pypi.rs | 18 ++++++----- 8 files changed, 67 insertions(+), 27 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_safety_cow.rs b/crates/socket-patch-cli/tests/e2e_safety_cow.rs index 27dc573..b76e536 100644 --- a/crates/socket-patch-cli/tests/e2e_safety_cow.rs +++ b/crates/socket-patch-cli/tests/e2e_safety_cow.rs @@ -235,6 +235,7 @@ impl Fixture { /// patched. This is exactly the pnpm content-store isolation /// guarantee, but exercised without a pnpm dependency. #[test] +#[serial_test::serial] fn apply_breaks_hardlink_before_patching() { let fx = Fixture::new(); // Materialize index.js as a hardlink to an outside file. The @@ -290,6 +291,7 @@ fn apply_breaks_hardlink_before_patching() { /// a private regular file holding the patched bytes; the original /// target stays untouched. #[test] +#[serial_test::serial] fn apply_replaces_symlink_with_private_file() { let fx = Fixture::new(); let outside = fx.root().join("outside-target.js"); @@ -332,6 +334,7 @@ fn apply_replaces_symlink_with_private_file() { /// siblings should stay byte-identical. Exercises the per-file CoW /// in a loop. #[test] +#[serial_test::serial] fn apply_breaks_hardlinks_on_multi_file_patch() { let fx = Fixture::new(); let pkg = fx.root().join("node_modules/cow-fixture"); @@ -453,6 +456,7 @@ fn apply_breaks_hardlinks_on_multi_file_patch() { /// while writing nothing, so every CoW content assertion in this file /// would chase a no-op. #[test] +#[serial_test::serial] fn run_scrubs_ambient_socket_env() { std::env::set_var("SOCKET_DRY_RUN", "true"); let fx = Fixture::new(); @@ -474,6 +478,7 @@ fn run_scrubs_ambient_socket_env() { /// place via the atomic-write path. This pins the /// `CowAction::AlreadyPrivate` route. #[test] +#[serial_test::serial] fn apply_against_regular_file_leaves_no_cow_litter() { let fx = Fixture::new(); std::fs::write(fx.index_js(), ORIGINAL_BYTES).unwrap(); @@ -504,6 +509,7 @@ fn apply_against_regular_file_leaves_no_cow_litter() { /// state — semantically OK but observably different. This test /// pins the "no observable state change on failure" promise. #[test] +#[serial_test::serial] fn apply_failure_does_not_cow_or_modify() { let fx = Fixture::new(); let outside = fx.root().join("outside.js"); diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs index 6f7536b..eb0bccd 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs @@ -278,6 +278,7 @@ fn copy_dir_recursive(src: &Path, dst: &Path) { // ── capstone 1: uv project flavor ───────────────────────────────────── #[test] +#[serial_test::serial] fn uv_vendor_fresh_checkout_frozen_offline_and_revert() { let Some(uv) = find_uv() else { println!("SKIP e2e_vendor_pypi_build(uv): `uv` not on PATH or at ~/.local/bin/uv"); @@ -476,6 +477,7 @@ fn uv_vendor_fresh_checkout_frozen_offline_and_revert() { // ── capstone 2: requirements.txt flavor (pip + `uv pip`) ────────────── #[test] +#[serial_test::serial] fn pip_requirements_vendor_fresh_checkout_no_index_and_revert() { let Some(python) = find_python() else { println!("SKIP e2e_vendor_pypi_build(pip): no python3/python on PATH"); diff --git a/crates/socket-patch-cli/tests/setup_matrix_common/mod.rs b/crates/socket-patch-cli/tests/setup_matrix_common/mod.rs index 61d44ce..49d54a9 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_common/mod.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_common/mod.rs @@ -682,3 +682,33 @@ fn indent(s: &str) -> String { .collect::>() .join("\n") } + +/// RAII setter for hostile ambient env decoys: sets each pair process-wide +/// and removes them ALL on drop, so a panicking assertion mid-test can never +/// leave the process env poisoned for the tests that run after it. +/// +/// Process env is per-process, shared state. Any test that constructs this +/// guard — and every other test in the same binary that spawns the CLI — +/// must be `#[serial_test::serial]`: the child-env scrubs in the `run` +/// helpers snapshot `std::env::vars_os()` and then spawn, and a concurrent +/// `set_var` can land between the snapshot and the spawn, reaching the child +/// un-scrubbed (the 2026-07 `setup_matrix_pypi` CI flake — the decoys made +/// the sibling test's child abort at arg parse with exit 2). +pub struct DecoyGuard(&'static [(&'static str, &'static str)]); + +impl DecoyGuard { + pub fn set(pairs: &'static [(&'static str, &'static str)]) -> Self { + for (k, v) in pairs { + std::env::set_var(k, v); + } + Self(pairs) + } +} + +impl Drop for DecoyGuard { + fn drop(&mut self) { + for (k, _) in self.0 { + std::env::remove_var(k); + } + } +} diff --git a/crates/socket-patch-cli/tests/setup_matrix_deno.rs b/crates/socket-patch-cli/tests/setup_matrix_deno.rs index 7776c30..1bae949 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_deno.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_deno.rs @@ -46,6 +46,7 @@ mod smc; /// path that silently no-ops on skip — it is NOT a regression guard. The /// real teeth live in [`host_guard`] below. #[test] +#[serial_test::serial] // Experimental ecosystem (deno): the setup-matrix aspirational cases are a // BASELINE GAP (setup does not wire deno's install hook yet). This passes on CI // only because the runners lack the `deno` toolchain (the cases soft-skip); on @@ -172,10 +173,9 @@ mod host_guard { ]; #[test] + #[serial_test::serial] fn deno_setup_roundtrip_host() { - for (k, v) in HOSTILE_DECOYS { - std::env::set_var(k, v); - } + let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS); let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); std::fs::write(root.join("package.json"), PACKAGE_JSON).unwrap(); @@ -331,9 +331,5 @@ mod host_guard { Some(0), "no manifest may report configured after the hook is removed.\n{out}" ); - - for (k, _) in HOSTILE_DECOYS { - std::env::remove_var(k); - } } } diff --git a/crates/socket-patch-cli/tests/setup_matrix_maven.rs b/crates/socket-patch-cli/tests/setup_matrix_maven.rs index 81d8a65..f2a121e 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_maven.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_maven.rs @@ -45,6 +45,7 @@ mod smc; /// path that silently no-ops on skip — it is NOT a regression guard. The /// real teeth live in [`host_guard`] below. #[test] +#[serial_test::serial] // Experimental ecosystem (maven): aspirational setup-matrix cases are a // BASELINE GAP today; this passes on CI only because the runners lack `mvn` // (cases soft-skip) and fails on any host that has it. Ignore so maven can @@ -226,15 +227,14 @@ mod host_guard { } #[test] + #[serial_test::serial] fn maven_setup_is_a_clean_noop_host() { // Committed regression guard for the env scrub itself: with the old // fixed-list scrub these leaked into the child — SOCKET_STRICT / // SOCKET_VENDOR_SOURCE aborted every parse (exit 2) and // SOCKET_SETUP_EXCLUDE made the real `setup` run write // `.socket/manifest.json` into the fixture (assert_pristine RED). - for (k, v) in HOSTILE_DECOYS { - std::env::set_var(k, v); - } + let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS); let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); std::fs::write(root.join("pom.xml"), POM_XML).unwrap(); @@ -318,9 +318,5 @@ mod host_guard { Some(1), "positive control: exactly the package.json must count as needing configuration.\n{out}" ); - - for (k, _) in HOSTILE_DECOYS { - std::env::remove_var(k); - } } } diff --git a/crates/socket-patch-cli/tests/setup_matrix_npm.rs b/crates/socket-patch-cli/tests/setup_matrix_npm.rs index 23cad7b..db55d45 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_npm.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_npm.rs @@ -13,21 +13,25 @@ mod smc; #[test] +#[serial_test::serial] fn npm() { smc::run_pm("npm", "npm"); } #[test] +#[serial_test::serial] fn yarn() { smc::run_pm("npm", "yarn"); } #[test] +#[serial_test::serial] fn pnpm() { smc::run_pm("npm", "pnpm"); } #[test] +#[serial_test::serial] fn bun() { smc::run_pm("npm", "bun"); } @@ -40,16 +44,19 @@ fn bun() { // PASS — they're real regression guards, not gap documentation. #[test] +#[serial_test::serial] fn npm_workspace() { smc::run_workspace_pm("npm", "npm"); } #[test] +#[serial_test::serial] fn pnpm_workspace() { smc::run_workspace_pm("npm", "pnpm"); } #[test] +#[serial_test::serial] fn yarn_workspace() { smc::run_workspace_pm("npm", "yarn"); } @@ -176,15 +183,14 @@ mod host_guard { /// state at every stage. This is the assertion the soft-skipping Docker /// matrix can never make. #[test] + #[serial_test::serial] fn npm_setup_roundtrip_host() { // Committed regression guard for the env scrub itself: with the old // fixed-list scrub these leaked into the child — SOCKET_STRICT / // SOCKET_VENDOR_SOURCE aborted every parse (exit 2, so the very first // `--check` assertion went red) and SOCKET_SETUP_EXCLUDE stood in for // `setup --exclude` on the real run. - for (k, v) in HOSTILE_DECOYS { - std::env::set_var(k, v); - } + let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS); let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); stage_project(root); diff --git a/crates/socket-patch-cli/tests/setup_matrix_nuget.rs b/crates/socket-patch-cli/tests/setup_matrix_nuget.rs index 059f5dc..c6f9bce 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_nuget.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_nuget.rs @@ -49,6 +49,7 @@ mod smc; /// path that silently no-ops on skip — it is NOT a regression guard. The /// real teeth live in [`host_guard`] below. #[test] +#[serial_test::serial] // Experimental ecosystem (nuget): aspirational setup-matrix cases are a // BASELINE GAP today; this passes on CI only because the runners lack `dotnet` // (cases soft-skip) and fails on any host that has it. Ignore so nuget can @@ -200,15 +201,14 @@ mod host_guard { /// asserting REAL on-disk + JSON state at every stage. This is the /// assertion the Docker matrix can never make for nuget. #[test] + #[serial_test::serial] fn nuget_setup_roundtrip_host() { // Committed regression guard for the env scrub itself: with the old // fixed-list scrub these leaked into the child — SOCKET_STRICT / // SOCKET_VENDOR_SOURCE aborted every parse (exit 2) and // SOCKET_SETUP_EXCLUDE made the real `setup` run write // `.socket/manifest.json` into the fixture (final entries check RED). - for (k, v) in HOSTILE_DECOYS { - std::env::set_var(k, v); - } + let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS); let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); std::fs::write(root.join(CSPROJ_NAME), CSPROJ).unwrap(); diff --git a/crates/socket-patch-cli/tests/setup_matrix_pypi.rs b/crates/socket-patch-cli/tests/setup_matrix_pypi.rs index 3d484d4..d74b64b 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_pypi.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_pypi.rs @@ -49,26 +49,31 @@ mod smc; #[test] +#[serial_test::serial] fn pip() { smc::run_pm("pypi", "pip"); } #[test] +#[serial_test::serial] fn uv() { smc::run_pm("pypi", "uv"); } #[test] +#[serial_test::serial] fn poetry() { smc::run_pm("pypi", "poetry"); } #[test] +#[serial_test::serial] fn pdm() { smc::run_pm("pypi", "pdm"); } #[test] +#[serial_test::serial] fn hatch() { smc::run_pm("pypi", "hatch"); } @@ -213,15 +218,14 @@ mod host_guard { /// a real pip project, asserting REAL on-disk + JSON state at every stage. /// This is the assertion the Docker matrix can never make for pypi. #[test] + #[serial_test::serial] fn pypi_setup_roundtrip_host() { // Committed regression guard for the env scrub itself: with the old // fixed-list scrub these leaked into the child — SOCKET_STRICT / // SOCKET_VENDOR_SOURCE aborted every parse (exit 2) and // SOCKET_SETUP_EXCLUDE made the real `setup` run write // `.socket/manifest.json` into the fixture. - for (k, v) in HOSTILE_DECOYS { - std::env::set_var(k, v); - } + let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS); let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); std::fs::write(root.join("requirements.txt"), REQ_INITIAL).unwrap(); @@ -392,10 +396,6 @@ mod host_guard { "needs_configuration", "after remove the project must report needs_configuration again:\n{v}" ); - - for (k, _) in HOSTILE_DECOYS { - std::env::remove_var(k); - } } /// Regression: a commented-out hook line is NOT a configured project. @@ -407,6 +407,7 @@ mod host_guard { /// project with no hook at all. Check and setup must agree on the same /// bytes. #[test] + #[serial_test::serial] fn pypi_check_ignores_commented_out_hook_host() { const REQ_COMMENTED: &str = "requests==2.31.0\n# socket-patch[hook]\n"; let tmp = tempfile::tempdir().unwrap(); @@ -454,6 +455,7 @@ mod host_guard { /// running the real binary against a hand-authored Poetry manifest in each /// state. Fully hermetic: `--check` neither writes nor refreshes a lockfile. #[test] + #[serial_test::serial] fn poetry_check_recognizes_structural_hook_host() { // ── configured: the exact structural form `setup` emits ───────────── let tmp = tempfile::tempdir().unwrap(); @@ -520,11 +522,13 @@ mod host_guard { // these don't apply today — but the install itself must succeed. #[test] +#[serial_test::serial] fn pip_workspace() { smc::run_workspace_pm("pypi", "pip"); } #[test] +#[serial_test::serial] fn uv_workspace() { smc::run_workspace_pm("pypi", "uv"); }