Skip to content

Commit d85141f

Browse files
test: fix hostile-decoy env race flaking the setup-matrix and e2e suites (#135)
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 <noreply@anthropic.com>
1 parent 14eeec6 commit d85141f

8 files changed

Lines changed: 67 additions & 27 deletions

File tree

crates/socket-patch-cli/tests/e2e_safety_cow.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ impl Fixture {
235235
/// patched. This is exactly the pnpm content-store isolation
236236
/// guarantee, but exercised without a pnpm dependency.
237237
#[test]
238+
#[serial_test::serial]
238239
fn apply_breaks_hardlink_before_patching() {
239240
let fx = Fixture::new();
240241
// Materialize index.js as a hardlink to an outside file. The
@@ -290,6 +291,7 @@ fn apply_breaks_hardlink_before_patching() {
290291
/// a private regular file holding the patched bytes; the original
291292
/// target stays untouched.
292293
#[test]
294+
#[serial_test::serial]
293295
fn apply_replaces_symlink_with_private_file() {
294296
let fx = Fixture::new();
295297
let outside = fx.root().join("outside-target.js");
@@ -332,6 +334,7 @@ fn apply_replaces_symlink_with_private_file() {
332334
/// siblings should stay byte-identical. Exercises the per-file CoW
333335
/// in a loop.
334336
#[test]
337+
#[serial_test::serial]
335338
fn apply_breaks_hardlinks_on_multi_file_patch() {
336339
let fx = Fixture::new();
337340
let pkg = fx.root().join("node_modules/cow-fixture");
@@ -453,6 +456,7 @@ fn apply_breaks_hardlinks_on_multi_file_patch() {
453456
/// while writing nothing, so every CoW content assertion in this file
454457
/// would chase a no-op.
455458
#[test]
459+
#[serial_test::serial]
456460
fn run_scrubs_ambient_socket_env() {
457461
std::env::set_var("SOCKET_DRY_RUN", "true");
458462
let fx = Fixture::new();
@@ -474,6 +478,7 @@ fn run_scrubs_ambient_socket_env() {
474478
/// place via the atomic-write path. This pins the
475479
/// `CowAction::AlreadyPrivate` route.
476480
#[test]
481+
#[serial_test::serial]
477482
fn apply_against_regular_file_leaves_no_cow_litter() {
478483
let fx = Fixture::new();
479484
std::fs::write(fx.index_js(), ORIGINAL_BYTES).unwrap();
@@ -504,6 +509,7 @@ fn apply_against_regular_file_leaves_no_cow_litter() {
504509
/// state — semantically OK but observably different. This test
505510
/// pins the "no observable state change on failure" promise.
506511
#[test]
512+
#[serial_test::serial]
507513
fn apply_failure_does_not_cow_or_modify() {
508514
let fx = Fixture::new();
509515
let outside = fx.root().join("outside.js");

crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ fn copy_dir_recursive(src: &Path, dst: &Path) {
278278
// ── capstone 1: uv project flavor ─────────────────────────────────────
279279

280280
#[test]
281+
#[serial_test::serial]
281282
fn uv_vendor_fresh_checkout_frozen_offline_and_revert() {
282283
let Some(uv) = find_uv() else {
283284
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() {
476477
// ── capstone 2: requirements.txt flavor (pip + `uv pip`) ──────────────
477478

478479
#[test]
480+
#[serial_test::serial]
479481
fn pip_requirements_vendor_fresh_checkout_no_index_and_revert() {
480482
let Some(python) = find_python() else {
481483
println!("SKIP e2e_vendor_pypi_build(pip): no python3/python on PATH");

crates/socket-patch-cli/tests/setup_matrix_common/mod.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,3 +682,33 @@ fn indent(s: &str) -> String {
682682
.collect::<Vec<_>>()
683683
.join("\n")
684684
}
685+
686+
/// RAII setter for hostile ambient env decoys: sets each pair process-wide
687+
/// and removes them ALL on drop, so a panicking assertion mid-test can never
688+
/// leave the process env poisoned for the tests that run after it.
689+
///
690+
/// Process env is per-process, shared state. Any test that constructs this
691+
/// guard — and every other test in the same binary that spawns the CLI —
692+
/// must be `#[serial_test::serial]`: the child-env scrubs in the `run`
693+
/// helpers snapshot `std::env::vars_os()` and then spawn, and a concurrent
694+
/// `set_var` can land between the snapshot and the spawn, reaching the child
695+
/// un-scrubbed (the 2026-07 `setup_matrix_pypi` CI flake — the decoys made
696+
/// the sibling test's child abort at arg parse with exit 2).
697+
pub struct DecoyGuard(&'static [(&'static str, &'static str)]);
698+
699+
impl DecoyGuard {
700+
pub fn set(pairs: &'static [(&'static str, &'static str)]) -> Self {
701+
for (k, v) in pairs {
702+
std::env::set_var(k, v);
703+
}
704+
Self(pairs)
705+
}
706+
}
707+
708+
impl Drop for DecoyGuard {
709+
fn drop(&mut self) {
710+
for (k, _) in self.0 {
711+
std::env::remove_var(k);
712+
}
713+
}
714+
}

crates/socket-patch-cli/tests/setup_matrix_deno.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ mod smc;
4646
/// path that silently no-ops on skip — it is NOT a regression guard. The
4747
/// real teeth live in [`host_guard`] below.
4848
#[test]
49+
#[serial_test::serial]
4950
// Experimental ecosystem (deno): the setup-matrix aspirational cases are a
5051
// BASELINE GAP (setup does not wire deno's install hook yet). This passes on CI
5152
// only because the runners lack the `deno` toolchain (the cases soft-skip); on
@@ -172,10 +173,9 @@ mod host_guard {
172173
];
173174

174175
#[test]
176+
#[serial_test::serial]
175177
fn deno_setup_roundtrip_host() {
176-
for (k, v) in HOSTILE_DECOYS {
177-
std::env::set_var(k, v);
178-
}
178+
let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS);
179179
let tmp = tempfile::tempdir().unwrap();
180180
let root = tmp.path();
181181
std::fs::write(root.join("package.json"), PACKAGE_JSON).unwrap();
@@ -331,9 +331,5 @@ mod host_guard {
331331
Some(0),
332332
"no manifest may report configured after the hook is removed.\n{out}"
333333
);
334-
335-
for (k, _) in HOSTILE_DECOYS {
336-
std::env::remove_var(k);
337-
}
338334
}
339335
}

crates/socket-patch-cli/tests/setup_matrix_maven.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ mod smc;
4545
/// path that silently no-ops on skip — it is NOT a regression guard. The
4646
/// real teeth live in [`host_guard`] below.
4747
#[test]
48+
#[serial_test::serial]
4849
// Experimental ecosystem (maven): aspirational setup-matrix cases are a
4950
// BASELINE GAP today; this passes on CI only because the runners lack `mvn`
5051
// (cases soft-skip) and fails on any host that has it. Ignore so maven can
@@ -226,15 +227,14 @@ mod host_guard {
226227
}
227228

228229
#[test]
230+
#[serial_test::serial]
229231
fn maven_setup_is_a_clean_noop_host() {
230232
// Committed regression guard for the env scrub itself: with the old
231233
// fixed-list scrub these leaked into the child — SOCKET_STRICT /
232234
// SOCKET_VENDOR_SOURCE aborted every parse (exit 2) and
233235
// SOCKET_SETUP_EXCLUDE made the real `setup` run write
234236
// `.socket/manifest.json` into the fixture (assert_pristine RED).
235-
for (k, v) in HOSTILE_DECOYS {
236-
std::env::set_var(k, v);
237-
}
237+
let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS);
238238
let tmp = tempfile::tempdir().unwrap();
239239
let root = tmp.path();
240240
std::fs::write(root.join("pom.xml"), POM_XML).unwrap();
@@ -318,9 +318,5 @@ mod host_guard {
318318
Some(1),
319319
"positive control: exactly the package.json must count as needing configuration.\n{out}"
320320
);
321-
322-
for (k, _) in HOSTILE_DECOYS {
323-
std::env::remove_var(k);
324-
}
325321
}
326322
}

crates/socket-patch-cli/tests/setup_matrix_npm.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,25 @@
1313
mod smc;
1414

1515
#[test]
16+
#[serial_test::serial]
1617
fn npm() {
1718
smc::run_pm("npm", "npm");
1819
}
1920

2021
#[test]
22+
#[serial_test::serial]
2123
fn yarn() {
2224
smc::run_pm("npm", "yarn");
2325
}
2426

2527
#[test]
28+
#[serial_test::serial]
2629
fn pnpm() {
2730
smc::run_pm("npm", "pnpm");
2831
}
2932

3033
#[test]
34+
#[serial_test::serial]
3135
fn bun() {
3236
smc::run_pm("npm", "bun");
3337
}
@@ -40,16 +44,19 @@ fn bun() {
4044
// PASS — they're real regression guards, not gap documentation.
4145

4246
#[test]
47+
#[serial_test::serial]
4348
fn npm_workspace() {
4449
smc::run_workspace_pm("npm", "npm");
4550
}
4651

4752
#[test]
53+
#[serial_test::serial]
4854
fn pnpm_workspace() {
4955
smc::run_workspace_pm("npm", "pnpm");
5056
}
5157

5258
#[test]
59+
#[serial_test::serial]
5360
fn yarn_workspace() {
5461
smc::run_workspace_pm("npm", "yarn");
5562
}
@@ -176,15 +183,14 @@ mod host_guard {
176183
/// state at every stage. This is the assertion the soft-skipping Docker
177184
/// matrix can never make.
178185
#[test]
186+
#[serial_test::serial]
179187
fn npm_setup_roundtrip_host() {
180188
// Committed regression guard for the env scrub itself: with the old
181189
// fixed-list scrub these leaked into the child — SOCKET_STRICT /
182190
// SOCKET_VENDOR_SOURCE aborted every parse (exit 2, so the very first
183191
// `--check` assertion went red) and SOCKET_SETUP_EXCLUDE stood in for
184192
// `setup --exclude` on the real run.
185-
for (k, v) in HOSTILE_DECOYS {
186-
std::env::set_var(k, v);
187-
}
193+
let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS);
188194
let tmp = tempfile::tempdir().unwrap();
189195
let root = tmp.path();
190196
stage_project(root);

crates/socket-patch-cli/tests/setup_matrix_nuget.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ mod smc;
4949
/// path that silently no-ops on skip — it is NOT a regression guard. The
5050
/// real teeth live in [`host_guard`] below.
5151
#[test]
52+
#[serial_test::serial]
5253
// Experimental ecosystem (nuget): aspirational setup-matrix cases are a
5354
// BASELINE GAP today; this passes on CI only because the runners lack `dotnet`
5455
// (cases soft-skip) and fails on any host that has it. Ignore so nuget can
@@ -200,15 +201,14 @@ mod host_guard {
200201
/// asserting REAL on-disk + JSON state at every stage. This is the
201202
/// assertion the Docker matrix can never make for nuget.
202203
#[test]
204+
#[serial_test::serial]
203205
fn nuget_setup_roundtrip_host() {
204206
// Committed regression guard for the env scrub itself: with the old
205207
// fixed-list scrub these leaked into the child — SOCKET_STRICT /
206208
// SOCKET_VENDOR_SOURCE aborted every parse (exit 2) and
207209
// SOCKET_SETUP_EXCLUDE made the real `setup` run write
208210
// `.socket/manifest.json` into the fixture (final entries check RED).
209-
for (k, v) in HOSTILE_DECOYS {
210-
std::env::set_var(k, v);
211-
}
211+
let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS);
212212
let tmp = tempfile::tempdir().unwrap();
213213
let root = tmp.path();
214214
std::fs::write(root.join(CSPROJ_NAME), CSPROJ).unwrap();

crates/socket-patch-cli/tests/setup_matrix_pypi.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,26 +49,31 @@
4949
mod smc;
5050

5151
#[test]
52+
#[serial_test::serial]
5253
fn pip() {
5354
smc::run_pm("pypi", "pip");
5455
}
5556

5657
#[test]
58+
#[serial_test::serial]
5759
fn uv() {
5860
smc::run_pm("pypi", "uv");
5961
}
6062

6163
#[test]
64+
#[serial_test::serial]
6265
fn poetry() {
6366
smc::run_pm("pypi", "poetry");
6467
}
6568

6669
#[test]
70+
#[serial_test::serial]
6771
fn pdm() {
6872
smc::run_pm("pypi", "pdm");
6973
}
7074

7175
#[test]
76+
#[serial_test::serial]
7277
fn hatch() {
7378
smc::run_pm("pypi", "hatch");
7479
}
@@ -213,15 +218,14 @@ mod host_guard {
213218
/// a real pip project, asserting REAL on-disk + JSON state at every stage.
214219
/// This is the assertion the Docker matrix can never make for pypi.
215220
#[test]
221+
#[serial_test::serial]
216222
fn pypi_setup_roundtrip_host() {
217223
// Committed regression guard for the env scrub itself: with the old
218224
// fixed-list scrub these leaked into the child — SOCKET_STRICT /
219225
// SOCKET_VENDOR_SOURCE aborted every parse (exit 2) and
220226
// SOCKET_SETUP_EXCLUDE made the real `setup` run write
221227
// `.socket/manifest.json` into the fixture.
222-
for (k, v) in HOSTILE_DECOYS {
223-
std::env::set_var(k, v);
224-
}
228+
let _decoys = crate::smc::DecoyGuard::set(HOSTILE_DECOYS);
225229
let tmp = tempfile::tempdir().unwrap();
226230
let root = tmp.path();
227231
std::fs::write(root.join("requirements.txt"), REQ_INITIAL).unwrap();
@@ -392,10 +396,6 @@ mod host_guard {
392396
"needs_configuration",
393397
"after remove the project must report needs_configuration again:\n{v}"
394398
);
395-
396-
for (k, _) in HOSTILE_DECOYS {
397-
std::env::remove_var(k);
398-
}
399399
}
400400

401401
/// Regression: a commented-out hook line is NOT a configured project.
@@ -407,6 +407,7 @@ mod host_guard {
407407
/// project with no hook at all. Check and setup must agree on the same
408408
/// bytes.
409409
#[test]
410+
#[serial_test::serial]
410411
fn pypi_check_ignores_commented_out_hook_host() {
411412
const REQ_COMMENTED: &str = "requests==2.31.0\n# socket-patch[hook]\n";
412413
let tmp = tempfile::tempdir().unwrap();
@@ -454,6 +455,7 @@ mod host_guard {
454455
/// running the real binary against a hand-authored Poetry manifest in each
455456
/// state. Fully hermetic: `--check` neither writes nor refreshes a lockfile.
456457
#[test]
458+
#[serial_test::serial]
457459
fn poetry_check_recognizes_structural_hook_host() {
458460
// ── configured: the exact structural form `setup` emits ─────────────
459461
let tmp = tempfile::tempdir().unwrap();
@@ -520,11 +522,13 @@ mod host_guard {
520522
// these don't apply today — but the install itself must succeed.
521523

522524
#[test]
525+
#[serial_test::serial]
523526
fn pip_workspace() {
524527
smc::run_workspace_pm("pypi", "pip");
525528
}
526529

527530
#[test]
531+
#[serial_test::serial]
528532
fn uv_workspace() {
529533
smc::run_workspace_pm("pypi", "uv");
530534
}

0 commit comments

Comments
 (0)