From 44a50c0df68664172aef18c907f6598d2284c50a Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 19 Aug 2026 06:18:15 +0200 Subject: [PATCH] test(hive-c0re): serialise the remaining env-mutating tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every one of these carried a "SAFETY: single-threaded mutation of an env var no other test asserts on" comment. Each claim was true of the module and false of the process: env vars are one process-global and every #[test] in this crate lands in the same binary at default parallelism, so "no other test" has to mean no other test in the BINARY — and two of them set HIVE_FORGE_URL outright. They now take the crate lock #3483 added, including the two whose variables nothing else touches: what makes a variable safe is that every mutator routes through one lock, not that today's set happens not to collide. The doc comment on push_forwarded_var_options said the render-level tests race each other; they serialise now, so it says that. It is deliberately not an intra-doc link — test_env is #[cfg(test)], so rustdoc cannot resolve it and -D rustdoc::broken-intra-doc-links fails the docs check. --- hive-c0re/src/meta.rs | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index b3f220a6..7698250f 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -733,8 +733,9 @@ const FORWARDED_VAR_OPTIONS: &[(&str, &str)] = &[ /// Render the forwarded-var option assignments for one agent's module block. /// /// Split out of `render_flake` so it can be tested without touching process -/// env: the render-level tests have to `set_var`, which makes them race each -/// other under the default parallel test runner. A pure function over the +/// env: the render-level tests have to `set_var`, which forces them to +/// serialise on the crate's `test_env` lock (a `#[cfg(test)]` module, so this +/// deliberately isn't an intra-doc link). A pure function over the /// already-collected pairs has no such hazard. /// /// A var that isn't present emits nothing rather than a guess. For an optional @@ -1907,8 +1908,11 @@ mod tests { // also appear in `systemd.globalEnvironment`, which every unit + // shell in the container inherits. // - // SAFETY: single-threaded mutation of a process env var the other - // tests don't assert the absence of; restored before returning. + // Serialised against every other env-mutating test in the crate: + // `render_flake_sets_service_url_options_from_forwarded_env` sets this + // same variable, and both land in the one test binary. + let _env = crate::test_env::lock(); + // SAFETY: serialised by the guard above; restored before returning. unsafe { std::env::set_var("HIVE_FORGE_URL", "http://forge.example.test"); } @@ -2050,8 +2054,11 @@ mod tests { // nothing. It has to land before the environment blocks that close the // module out. // - // SAFETY: single-threaded mutation of process env vars, restored - // before returning. + // Serialised against every other env-mutating test in the crate: + // `render_flake_forwards_env_into_global_environment` sets HIVE_FORGE_URL + // too, and both land in the one test binary. + let _env = crate::test_env::lock(); + // SAFETY: serialised by the guard above; restored before returning. unsafe { std::env::set_var("HIVE_FORGE_URL", "http://forge.example.test"); std::env::set_var("HIVE_MATRIX_URL", "http://matrix.example.test"); @@ -2126,8 +2133,10 @@ mod tests { // the signal, no reference is emitted (so the flake doesn't point at // a file that was never embedded). // - // SAFETY: single-threaded mutation of a process env var the other - // tests don't assert the absence of; restored before returning. + // Serialised against every other env-mutating test in the crate. Taken + // before the fixture files so the guard covers the whole window in + // which HIVE_TLS_CA_PATH / HIVE_PEER_CA_PATHS are perturbed. + let _env = crate::test_env::lock(); let ca_file = std::env::temp_dir().join(format!("hive-ca-test-{}.pem", std::process::id())); std::fs::write( &ca_file, @@ -2160,8 +2169,8 @@ mod tests { ) .expect("write stale peer CA"); - // All env mutations are serialised within this one test (no other - // test asserts on these vars), restored before returning. + // SAFETY: serialised by the guard at the top of this test; restored + // before returning. unsafe { std::env::remove_var("HIVE_PEER_CA_PATHS"); std::env::set_var("HIVE_TLS_CA_PATH", &ca_file); @@ -2298,8 +2307,10 @@ mod tests { // on hive-c0re's unit -> `hyperhive.github.enable = false` injected into // every agent. On by default, so nothing is emitted unless disabled. // - // SAFETY: single-threaded mutation of an env var no other test asserts - // on; restored before returning. + // Serialised against every other env-mutating test in the crate — the + // variable is this module's alone, but the process it lives in is not. + let _env = crate::test_env::lock(); + // SAFETY: serialised by the guard above; restored before returning. let render = || { render_flake( "github:example/hyperhive",