From 842802b873ea26e22294ff08aa4f6564c7f6b1d2 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Sun, 10 May 2026 18:16:18 -0500 Subject: [PATCH] test: fold settings.rs term_program + no_animations guards into lock_test_env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #1365 (cherry-picked into v0.8.28) introduced `term_program_test_guard` as a fresh module-local `static Mutex<()>`, mirroring the existing `no_animations_test_guard`. Both serialize their own family of tests but not with each other — so under cargo's parallel runner, a `NO_ANIMATIONS=1` leak from one family lands in the env at the exact moment a `TERM_PROGRAM=iTerm.app` test calls the shared `apply_env_overrides`, flipping `low_motion` to true and failing `non_vscode_term_program_does_not_force_low_motion`. Both guards now return `crate::test_support::lock_test_env()` (the same fold the v0.8.28 test-stabilization commit applied to the EnvGuard family in `commands/config.rs`, `commands/network.rs`, and `tools/recall_archive.rs`). This serializes the two test groups with each other and with every other env-mutating test in the suite, eliminating the cross-test env-var race. `save_api_key_for_openrouter_writes_provider_table` was failing intermittently for the same reason — a concurrent env mutation in an unrelated test was clobbering HOME / DEEPSEEK_CONFIG_PATH in the window between our `EnvGuard::new` and `save_api_key_for`'s `default_config_path()` read. With the broader serialization in place, the race window closes. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/tui/src/settings.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/settings.rs b/crates/tui/src/settings.rs index e1d07a07..aef6f4bf 100644 --- a/crates/tui/src/settings.rs +++ b/crates/tui/src/settings.rs @@ -826,10 +826,13 @@ mod tests { /// Tests that mutate process-global `NO_ANIMATIONS` serialise /// through this guard so the cargo parallel runner doesn't - /// observe interleaved overrides. + /// observe interleaved overrides. Uses the process-wide test env + /// lock so this serializes with the TERM_PROGRAM tests too — + /// otherwise a `NO_ANIMATIONS=1` leak from this test family can + /// flip a concurrent `TERM_PROGRAM=iTerm` test's `low_motion` + /// assertion through the shared `apply_env_overrides` path. fn no_animations_test_guard() -> std::sync::MutexGuard<'static, ()> { - static GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(()); - GUARD.lock().unwrap_or_else(|e| e.into_inner()) + crate::test_support::lock_test_env() } #[test] @@ -906,9 +909,13 @@ mod tests { } /// Serialise tests that mutate `TERM_PROGRAM` through this guard. + /// Uses the process-wide test env lock so this serializes not just + /// with itself but with every other env-mutating test in the suite + /// — otherwise a concurrent test that calls `Settings::default()` + /// can read whatever value our two `set_var`s have raced into the + /// env at that instant. fn term_program_test_guard() -> std::sync::MutexGuard<'static, ()> { - static GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(()); - GUARD.lock().unwrap_or_else(|e| e.into_inner()) + crate::test_support::lock_test_env() } #[test]