identity: serialise env-mutating tests on a module mutex (damocles #595 nit)

Cargo's default test runner parallelises tests within a binary,
so the original 'tests run serially' comment was wrong — two
`with_env` calls running concurrently would race the
process-wide HIVE_LABEL / HYPERHIVE_HIVE_DOMAIN state.

Added a module-scope `static ENV_LOCK: Mutex<()>` and acquire it
at the top of `with_env` so each set / run / restore window is
exclusive. Poison recovery via `unwrap_or_else(into_inner)` so a
single test panic doesn't cascade through the rest of the module.

Lighter than pulling in serial_test for one module. No new deps.
This commit is contained in:
iris 2026-05-29 19:13:01 +02:00
commit 5a0b6ca907

View file

@ -65,14 +65,27 @@ pub fn qualify(label: &str) -> String {
#[cfg(test)]
mod tests {
use super::*;
use std::sync::Mutex;
/// Per damocles's #595 review: cargo's test runner parallelises by
/// default, so a `with_env` helper that mutates process-wide env vars
/// races between tests in this module. Serialise on a module-scope
/// mutex so each `with_env` call holds the lock for its set / run /
/// restore window. Cheap (each test body is microseconds) and avoids
/// pulling in `serial_test` for just one module.
static ENV_LOCK: Mutex<()> = Mutex::new(());
/// Helper: run `f` with a clean env, restoring previous values on exit.
/// Tests run serially in this module (no `#[parallel]`) to avoid the
/// process-wide env var contention.
/// Acquires `ENV_LOCK` first so concurrent tests don't race the env-var
/// state. If a previous test panicked while holding the lock the
/// mutex would be poisoned — we use `lock().unwrap_or_else(|e| e.into_inner())`
/// to recover so a single test failure doesn't cascade through the
/// whole module.
fn with_env<F: FnOnce()>(label: Option<&str>, domain: Option<&str>, f: F) {
let _guard = ENV_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
let prev_label = env::var("HIVE_LABEL").ok();
let prev_domain = env::var("HYPERHIVE_HIVE_DOMAIN").ok();
// SAFETY: tests are single-threaded; we restore in the same scope.
// SAFETY: serialised by ENV_LOCK above; restore in the same scope.
unsafe {
match label {
Some(v) => env::set_var("HIVE_LABEL", v),