From b7032e65bf0e1decafd7b47f1a61cd5f3fcfe601 Mon Sep 17 00:00:00 2001 From: Zavian Wang <36817799+Zavianx@users.noreply.github.com> Date: Thu, 14 May 2026 05:22:32 +0800 Subject: [PATCH] test(local-ai): serialize Ollama env mutations (#1656) --- src/openhuman/local_ai/install.rs | 10 ++-- src/openhuman/local_ai/mod.rs | 7 +++ src/openhuman/local_ai/ollama_api.rs | 4 +- .../local_ai/service/ollama_admin_tests.rs | 48 +++++-------------- .../local_ai/service/public_infer_tests.rs | 20 ++------ .../local_ai/service/vision_embed.rs | 8 +--- src/openhuman/memory/store/factories.rs | 30 ++++++------ src/openhuman/voice/postprocess.rs | 28 +++-------- 8 files changed, 52 insertions(+), 103 deletions(-) diff --git a/src/openhuman/local_ai/install.rs b/src/openhuman/local_ai/install.rs index 607f01a73..780dffe1f 100644 --- a/src/openhuman/local_ai/install.rs +++ b/src/openhuman/local_ai/install.rs @@ -334,16 +334,14 @@ pub(crate) fn find_system_ollama_binary() -> Option { mod tests { use super::*; use std::ffi::OsString; - use std::sync::Mutex; /// Serialises tests that mutate process-global environment variables - /// (OLLAMA_BIN, PATH). Without this, cargo's test runner can interleave - /// their set/remove calls and cause flakes. - static ENV_LOCK: Mutex<()> = Mutex::new(()); + /// (OLLAMA_BIN, PATH) with other local-AI tests that also read these + /// variables. Without this, cargo's test runner can interleave set/remove + /// calls and cause flakes. fn env_lock() -> std::sync::MutexGuard<'static, ()> { - // Recover from a prior test's panic so one failure doesn't cascade. - ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()) + crate::openhuman::local_ai::local_ai_test_guard() } /// RAII guard: records the prior value of `var` on construction and diff --git a/src/openhuman/local_ai/mod.rs b/src/openhuman/local_ai/mod.rs index 42a6e38b5..a7b5f57ab 100644 --- a/src/openhuman/local_ai/mod.rs +++ b/src/openhuman/local_ai/mod.rs @@ -4,6 +4,13 @@ pub(crate) static LOCAL_AI_TEST_MUTEX: once_cell::sync::Lazy> = once_cell::sync::Lazy::new(|| std::sync::Mutex::new(())); +#[cfg(test)] +pub(crate) fn local_ai_test_guard() -> std::sync::MutexGuard<'static, ()> { + LOCAL_AI_TEST_MUTEX + .lock() + .unwrap_or_else(|p| p.into_inner()) +} + mod core; pub mod device; pub mod gif_decision; diff --git a/src/openhuman/local_ai/ollama_api.rs b/src/openhuman/local_ai/ollama_api.rs index d44fd4a14..f794c433d 100644 --- a/src/openhuman/local_ai/ollama_api.rs +++ b/src/openhuman/local_ai/ollama_api.rs @@ -341,9 +341,7 @@ mod tests { } fn test_lock() -> std::sync::MutexGuard<'static, ()> { - crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()) + crate::openhuman::local_ai::local_ai_test_guard() } #[test] diff --git a/src/openhuman/local_ai/service/ollama_admin_tests.rs b/src/openhuman/local_ai/service/ollama_admin_tests.rs index 9e34ad1dd..fa7b51ea4 100644 --- a/src/openhuman/local_ai/service/ollama_admin_tests.rs +++ b/src/openhuman/local_ai/service/ollama_admin_tests.rs @@ -24,9 +24,7 @@ async fn spawn_mock(app: Router) -> String { #[tokio::test] async fn has_model_detects_exact_and_prefixed_tag() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/tags", @@ -58,9 +56,7 @@ async fn has_model_detects_exact_and_prefixed_tag() { #[tokio::test] async fn has_model_errors_on_non_success_tags_response() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/tags", @@ -83,9 +79,7 @@ async fn has_model_errors_on_non_success_tags_response() { #[tokio::test] async fn ollama_healthy_returns_true_on_200_tags_response() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route("/api/tags", get(|| async { Json(json!({ "models": [] })) })); let base = spawn_mock(app).await; @@ -104,9 +98,7 @@ async fn ollama_healthy_returns_true_on_200_tags_response() { #[tokio::test] async fn ollama_healthy_returns_false_on_unreachable_url() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); // Point at a port we never bind → connect fails → healthy = false. unsafe { @@ -122,9 +114,7 @@ async fn ollama_healthy_returns_false_on_unreachable_url() { #[tokio::test] async fn diagnostics_reports_server_unreachable_when_url_unbound() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); unsafe { std::env::set_var("OPENHUMAN_OLLAMA_BASE_URL", "http://127.0.0.1:1"); @@ -160,9 +150,7 @@ async fn diagnostics_reports_server_unreachable_when_url_unbound() { #[tokio::test] async fn diagnostics_with_running_server_but_missing_models_flags_issues() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route("/api/tags", get(|| async { Json(json!({ "models": [] })) })); let base = spawn_mock(app).await; @@ -200,9 +188,7 @@ async fn diagnostics_with_running_server_but_missing_models_flags_issues() { #[tokio::test] async fn diagnostics_ok_when_expected_models_are_present() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let config = Config::default(); let chat = crate::openhuman::local_ai::model_ids::effective_chat_model_id(&config); @@ -257,9 +243,7 @@ async fn diagnostics_ok_when_expected_models_are_present() { #[tokio::test] async fn resolve_binary_path_finds_binary_via_ollama_bin_env() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let tmp = tempfile::tempdir().unwrap(); let fake_bin = tmp.path().join(if cfg!(windows) { @@ -292,9 +276,7 @@ async fn resolve_binary_path_finds_binary_via_ollama_bin_env() { #[tokio::test] async fn diagnostics_repair_actions_include_start_server_when_binary_known() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let tmp = tempfile::tempdir().unwrap(); let fake_bin = tmp.path().join(if cfg!(windows) { @@ -335,9 +317,7 @@ async fn diagnostics_repair_actions_include_start_server_when_binary_known() { async fn diagnostics_repair_actions_field_always_present() { // Verifies that the "repair_actions" key is always present in the diagnostics // JSON, regardless of the server state, so the UI can always iterate over it. - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); unsafe { std::env::set_var("OPENHUMAN_OLLAMA_BASE_URL", "http://127.0.0.1:1"); @@ -358,9 +338,7 @@ async fn diagnostics_repair_actions_field_always_present() { #[tokio::test] async fn list_models_returns_parsed_payload() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/tags", @@ -391,9 +369,7 @@ async fn list_models_returns_parsed_payload() { #[tokio::test] async fn list_models_errors_on_non_success() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/tags", diff --git a/src/openhuman/local_ai/service/public_infer_tests.rs b/src/openhuman/local_ai/service/public_infer_tests.rs index ed8096ade..97ccff727 100644 --- a/src/openhuman/local_ai/service/public_infer_tests.rs +++ b/src/openhuman/local_ai/service/public_infer_tests.rs @@ -28,9 +28,7 @@ fn ready_service(config: &Config) -> LocalAiService { #[tokio::test] async fn inference_hits_ollama_generate_and_returns_response() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai test mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/generate", @@ -67,9 +65,7 @@ async fn inference_hits_ollama_generate_and_returns_response() { #[tokio::test] async fn inference_errors_on_non_success_status() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai test mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/generate", @@ -92,9 +88,7 @@ async fn inference_errors_on_non_success_status() { #[tokio::test] async fn inference_errors_on_empty_response_when_allow_empty_false() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai test mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/generate", @@ -183,9 +177,7 @@ async fn inline_complete_interactive_disabled_returns_empty_string() { /// the permit it would deadlock or time out. #[tokio::test] async fn inline_complete_interactive_does_not_block_on_held_permit() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai test mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); // Hold the global LLM permit for the duration of the test. let _held = crate::openhuman::scheduler_gate::gate::try_acquire_llm_permit() @@ -242,9 +234,7 @@ async fn inline_complete_interactive_does_not_block_on_held_permit() { // safer trade-off. See PR #1524. #[ignore = "flaky timing under full-suite load — see PR #1524"] async fn gated_inline_complete_blocks_on_held_permit() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai test mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let held = crate::openhuman::scheduler_gate::gate::try_acquire_llm_permit() .expect("test must start with a free permit"); diff --git a/src/openhuman/local_ai/service/vision_embed.rs b/src/openhuman/local_ai/service/vision_embed.rs index 28d4650e0..950f70b84 100644 --- a/src/openhuman/local_ai/service/vision_embed.rs +++ b/src/openhuman/local_ai/service/vision_embed.rs @@ -250,9 +250,7 @@ mod tests { #[tokio::test] async fn embed_against_mock_returns_vectors_with_dimensions() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); let app = mock_with_tags_and( "/api/embed", @@ -283,9 +281,7 @@ mod tests { #[tokio::test] async fn embed_rejects_all_empty_inputs_before_network_call() { - let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .expect("local ai mutex"); + let _guard = crate::openhuman::local_ai::local_ai_test_guard(); // Even without a working mock server, entirely-empty inputs must be // rejected before any HTTP call. diff --git a/src/openhuman/memory/store/factories.rs b/src/openhuman/memory/store/factories.rs index 44d80bf97..7d89c2b17 100644 --- a/src/openhuman/memory/store/factories.rs +++ b/src/openhuman/memory/store/factories.rs @@ -416,15 +416,13 @@ pub fn create_memory_for_migration( #[cfg(test)] mod tests { use super::*; - use crate::openhuman::config::TEST_ENV_LOCK as ENV_LOCK; use axum::{routing::get, Json, Router}; use std::ffi::OsString; use std::net::SocketAddr; /// RAII helper that swaps `OPENHUMAN_OLLAMA_BASE_URL` to `value` for the - /// duration of the scope while holding `TEST_ENV_LOCK` so concurrent - /// tests can't race on the process-global env. The previous value (if - /// any) is restored on drop. + /// duration of the scope while holding the local-AI domain test mutex. + /// The previous value (if any) is restored on drop. struct EnvGuard { _lock: std::sync::MutexGuard<'static, ()>, prev: Option, @@ -432,11 +430,11 @@ mod tests { impl EnvGuard { fn set(value: &str) -> Self { - let lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let lock = crate::openhuman::local_ai::local_ai_test_guard(); let prev = std::env::var_os("OPENHUMAN_OLLAMA_BASE_URL"); // SAFETY: env mutation is wrapped because Rust 2024 marks it - // unsafe; the call is gated by TEST_ENV_LOCK so no other test in - // this binary is observing the env concurrently. + // unsafe; the call is gated by the local-AI domain mutex so no + // other local-AI test is observing the env concurrently. unsafe { std::env::set_var("OPENHUMAN_OLLAMA_BASE_URL", value); } @@ -446,7 +444,7 @@ mod tests { impl Drop for EnvGuard { fn drop(&mut self) { - // SAFETY: same justification as `set` — still under TEST_ENV_LOCK. + // SAFETY: same justification as `set` — still under the same lock. unsafe { match self.prev.take() { Some(v) => std::env::set_var("OPENHUMAN_OLLAMA_BASE_URL", v), @@ -535,8 +533,8 @@ mod tests { } /// Sets `OPENHUMAN_OLLAMA_BASE_URL` to a deliberately unreachable address - /// under `TEST_ENV_LOCK`, then verifies that the probed settings fall - /// back to cloud when the user has opted into local embeddings. + /// under the local-AI domain mutex, then verifies that the probed settings + /// fall back to cloud when the user has opted into local embeddings. #[tokio::test] async fn probed_settings_fall_back_to_cloud_when_ollama_unreachable() { let _env = EnvGuard::set("http://127.0.0.1:1"); @@ -602,14 +600,14 @@ mod tests { /// observe the Sentry side effect directly here, but the boolean return /// value is the gate's contract — covers the once-per-process guarantee. /// - /// Acquires `TEST_ENV_LOCK` to serialize with `probed_settings_*` tests - /// that also touch the latch; without that, parallel test execution can - /// reset the flag between this test's two `report_ollama_health_gate_once` - /// calls and turn the second one into a fresh "first" — flaking the - /// suppression assertion. + /// Acquires the local-AI domain mutex to serialize with `probed_settings_*` + /// tests that also touch the latch; without that, parallel test execution + /// can reset the flag between this test's two + /// `report_ollama_health_gate_once` calls and turn the second one into a + /// fresh "first", flaking the suppression assertion. #[test] fn ollama_health_gate_reports_at_most_once_per_process() { - let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let _lock = crate::openhuman::local_ai::local_ai_test_guard(); reset_health_gate_for_test(); assert!( diff --git a/src/openhuman/voice/postprocess.rs b/src/openhuman/voice/postprocess.rs index bd48a364a..52b17ab35 100644 --- a/src/openhuman/voice/postprocess.rs +++ b/src/openhuman/voice/postprocess.rs @@ -225,9 +225,7 @@ mod tests { #[tokio::test] async fn disabled_cleanup_returns_raw_text() { - let _g = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()); + let _g = crate::openhuman::local_ai::local_ai_test_guard(); let mut config = Config::default(); config.local_ai.voice_llm_cleanup_enabled = false; let service = local_ai::global(&config); @@ -243,9 +241,7 @@ mod tests { // Covers the branch where cleanup is enabled in config but the // local LLM hasn't reached the ready/degraded state yet — // cleanup must gracefully fall back to the raw Whisper output. - let _g = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()); + let _g = crate::openhuman::local_ai::local_ai_test_guard(); let config = Config::default(); // voice_llm_cleanup_enabled = true by default let service = local_ai::global(&config); let previous = service.status.lock().state.clone(); @@ -286,9 +282,7 @@ mod tests { #[tokio::test] async fn ready_llm_returns_trimmed_cleanup_or_falls_back() { - let _g = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()); + let _g = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/generate", post(|| async { @@ -311,9 +305,7 @@ mod tests { #[tokio::test] async fn ready_llm_empty_response_falls_back_to_raw_text() { - let _g = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()); + let _g = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/generate", post(|| async { Json(json!({"model":"test","response":" ","done": true})) }), @@ -331,9 +323,7 @@ mod tests { #[tokio::test] async fn ready_llm_error_response_falls_back_to_raw_text() { - let _g = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()); + let _g = crate::openhuman::local_ai::local_ai_test_guard(); let app = Router::new().route( "/api/generate", post(|| async { @@ -359,9 +349,7 @@ mod tests { // glued the conversation context in front of the raw text when // the LLM ran. If the global state raced away from "ready" the // call short-circuits to raw — still valid, just the other branch. - let _g = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()); + let _g = crate::openhuman::local_ai::local_ai_test_guard(); #[derive(serde::Deserialize)] struct Body { prompt: String, @@ -398,9 +386,7 @@ mod tests { // "Conversation context:" header regardless of which branch // runs — the LLM path uses the raw-text-only prompt, and the // short-circuit path never builds a prompt at all. - let _g = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX - .lock() - .unwrap_or_else(|p| p.into_inner()); + let _g = crate::openhuman::local_ai::local_ai_test_guard(); #[derive(serde::Deserialize)] struct Body { prompt: String,