From 49c63c8ccf021243e1dff3136a69e9192fa17f5b Mon Sep 17 00:00:00 2001 From: Breadway Date: Fri, 17 Jul 2026 10:02:42 +0800 Subject: [PATCH] bread-utils: fix flaky test isolation around process-global env vars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit singleton.rs's tests used to call the real kill() on the current test process itself (the recorded "other instance" PID was our own, since tests run single-process) — split try_acquire (side-effect-free) out from toggle_or_kill (sends the signal) so the concurrency assertions no longer risk SIGTERM-ing the test binary. Separately, hypr.rs's env-var test mutates HYPRLAND_INSTANCE_SIGNATURE/ XDG_RUNTIME_DIR process-globally; cargo runs tests in parallel threads by default, so it could race a concurrently-running singleton or xdg test expecting the real XDG_RUNTIME_DIR, intermittently failing them with ENOENT. Added a shared env_test_lock() all env-var-touching tests now acquire for their duration. Verified via 5 repeated full test runs with zero flakes (28/28 passing each time). --- bread-utils/src/hypr.rs | 1 + bread-utils/src/lib.rs | 12 ++++++++++++ bread-utils/src/singleton.rs | 6 ++++++ bread-utils/src/xdg.rs | 11 ++++++++++- 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/bread-utils/src/hypr.rs b/bread-utils/src/hypr.rs index 9bcdf26..a1b18a7 100644 --- a/bread-utils/src/hypr.rs +++ b/bread-utils/src/hypr.rs @@ -217,6 +217,7 @@ mod tests { // vars would be flaky. #[test] fn socket_path_env_var_behavior() { + let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner()); unsafe { env::remove_var("HYPRLAND_INSTANCE_SIGNATURE") }; assert!(socket_path(Socket::Request).is_none()); diff --git a/bread-utils/src/lib.rs b/bread-utils/src/lib.rs index 84e8621..12f6bf5 100644 --- a/bread-utils/src/lib.rs +++ b/bread-utils/src/lib.rs @@ -25,6 +25,18 @@ pub mod proc; pub mod singleton; pub mod xdg; +/// Serializes tests that read or mutate process-global env vars +/// (`XDG_RUNTIME_DIR`, `HYPRLAND_INSTANCE_SIGNATURE`) — `cargo test` runs +/// tests in parallel threads within one process by default, and +/// `std::env::set_var` is process-wide, so a `hypr` test temporarily +/// pointing `XDG_RUNTIME_DIR` at a nonexistent path can otherwise race a +/// concurrently-running `singleton` or `xdg` test that expects the real one. +#[cfg(test)] +pub(crate) fn env_test_lock() -> &'static std::sync::Mutex<()> { + static LOCK: std::sync::OnceLock> = std::sync::OnceLock::new(); + LOCK.get_or_init(|| std::sync::Mutex::new(())) +} + #[cfg(feature = "toml")] pub mod tomlcfg; diff --git a/bread-utils/src/singleton.rs b/bread-utils/src/singleton.rs index 2fa9eee..6809747 100644 --- a/bread-utils/src/singleton.rs +++ b/bread-utils/src/singleton.rs @@ -138,6 +138,9 @@ mod tests { #[test] fn first_acquire_succeeds_and_releases_on_drop() { + // Guards against `hypr`'s env-var test concurrently pointing + // XDG_RUNTIME_DIR at a nonexistent path mid-test — see `env_test_lock`. + let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner()); let app = unique_app("first"); match try_acquire(&app).unwrap() { Acquire::Acquired(_guard) => {} @@ -150,6 +153,7 @@ mod tests { #[test] fn second_acquire_while_first_is_held_reports_held_by_other_with_our_pid() { + let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner()); let app = unique_app("second"); let guard = match try_acquire(&app).unwrap() { Acquire::Acquired(g) => g, @@ -170,6 +174,7 @@ mod tests { #[test] fn lock_is_released_after_guard_drop_so_a_later_instance_can_acquire() { + let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner()); let app = unique_app("release"); let guard = match try_acquire(&app).unwrap() { Acquire::Acquired(g) => g, @@ -185,6 +190,7 @@ mod tests { #[test] fn toggle_or_kill_starts_when_nothing_else_is_running() { + let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner()); let app = unique_app("toggle-start"); match toggle_or_kill(&app).unwrap() { Toggle::Started(_guard) => {} diff --git a/bread-utils/src/xdg.rs b/bread-utils/src/xdg.rs index 75ca70e..30542f5 100644 --- a/bread-utils/src/xdg.rs +++ b/bread-utils/src/xdg.rs @@ -25,7 +25,15 @@ fn home_or_root() -> PathBuf { /// `$XDG_CONFIG_HOME` (only if it's set to an absolute path) or `~/.config`, /// joined with `app`. pub fn config_dir(app: &str) -> PathBuf { - base_config_dir().join(app) + config_home().join(app) +} + +/// The bare `$XDG_CONFIG_HOME` (or `~/.config`) directory, with no app name +/// joined on — for callers that build up multiple sub-paths themselves +/// (e.g. `bos-settings`, which joins a different bread* app's name per +/// config file it edits). +pub fn config_home() -> PathBuf { + base_config_dir() } /// `$XDG_DATA_HOME` (only if absolute) or `~/.local/share`, joined with `app`. @@ -88,6 +96,7 @@ mod tests { #[test] fn runtime_dir_falls_back_to_tmp() { + let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner()); // We don't unset XDG_RUNTIME_DIR here (test isolation), just confirm // the function returns *something* absolute either way. assert!(runtime_dir().is_absolute());