From a40cd7e64d076b0558f0f836f6906c4acdb26979 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Mon, 1 Jun 2026 18:41:51 +0530 Subject: [PATCH] test: fix flaky/stale tests blocking Rust E2E + coverage CI on main (#3147) Co-authored-by: Cyrus Gray --- .../OpenhumanLinkModal.notifications.test.tsx | 4 +-- .../intelligence/memoryGraphLayout.test.ts | 10 +++---- .../config_auth_app_state_connectivity_e2e.rs | 30 +++++++++++++++---- ...provider_admin_round22_raw_coverage_e2e.rs | 29 ++++++++++++++++-- 4 files changed, 57 insertions(+), 16 deletions(-) diff --git a/app/src/components/__tests__/OpenhumanLinkModal.notifications.test.tsx b/app/src/components/__tests__/OpenhumanLinkModal.notifications.test.tsx index 2508964eb..bcbb85196 100644 --- a/app/src/components/__tests__/OpenhumanLinkModal.notifications.test.tsx +++ b/app/src/components/__tests__/OpenhumanLinkModal.notifications.test.tsx @@ -49,7 +49,7 @@ describe('OpenhumanLinkModal notifications test flow', () => { await flushAsyncWork(); expect( - screen.getByText(/Test notification sent\. If you didn’t receive it/i) + screen.getByText(/Test notification sent\. If you didn't receive it/i) ).toBeInTheDocument(); expect(showNativeNotification).toHaveBeenCalledWith( expect.objectContaining({ tag: 'welcome-notification-test' }) @@ -115,7 +115,7 @@ describe('OpenhumanLinkModal notifications test flow', () => { await flushAsyncWork(); expect( - screen.getByText(/Test notification sent\. If you didn’t receive it/i) + screen.getByText(/Test notification sent\. If you didn't receive it/i) ).toBeInTheDocument(); expect(showNativeNotification).toHaveBeenCalledTimes(1); }); diff --git a/app/src/components/intelligence/memoryGraphLayout.test.ts b/app/src/components/intelligence/memoryGraphLayout.test.ts index 4b8adf7de..7ccef7f44 100644 --- a/app/src/components/intelligence/memoryGraphLayout.test.ts +++ b/app/src/components/intelligence/memoryGraphLayout.test.ts @@ -41,12 +41,12 @@ describe('memoryGraphLayout', () => { expect(nodeColor(contact())).toBe(CONTACT_COLOR); }); - it('nodeRadius shrinks with level and is fixed for chunk/contact', () => { - expect(nodeRadius(summary({ level: 0 }))).toBe(10); - expect(nodeRadius(summary({ level: 3 }))).toBeCloseTo(7.6); - expect(nodeRadius(summary({ level: 99 }))).toBe(4); // floored + it('nodeRadius grows with summary level and is fixed for chunk/contact', () => { + expect(nodeRadius(summary({ level: 0 }))).toBe(5); + expect(nodeRadius(summary({ level: 3 }))).toBeCloseTo(12.5); + expect(nodeRadius(summary({ level: 99 }))).toBe(252.5); expect(nodeRadius(contact())).toBe(9); - expect(nodeRadius(chunk())).toBe(4); + expect(nodeRadius(chunk())).toBe(3); }); it('only summary/contact nodes glow', () => { diff --git a/tests/config_auth_app_state_connectivity_e2e.rs b/tests/config_auth_app_state_connectivity_e2e.rs index a1b889b76..93ea2c1ea 100644 --- a/tests/config_auth_app_state_connectivity_e2e.rs +++ b/tests/config_auth_app_state_connectivity_e2e.rs @@ -5444,14 +5444,32 @@ fn credentials_profile_store_recovers_dropped_entries_empty_files_and_datetime_e .to_string(), ) .expect("write missing oauth secret fixture"); - let missing_secret_err = AuthProfilesStore::new(&missing_oauth_secret_dir, false) + // An OAuth profile missing its access_token must not poison the whole + // store: it is dropped just like a bad-kind entry (see #3125), so the load + // succeeds with that single profile purged rather than returning an error. + let recovered_missing_secret = AuthProfilesStore::new(&missing_oauth_secret_dir, false) .load() - .expect_err("oauth profile missing access token should fail"); + .expect("oauth profile missing access_token should be dropped, not fail the whole load"); assert!( - missing_secret_err - .to_string() - .contains("OAuth profile missing access_token"), - "unexpected missing oauth secret error: {missing_secret_err:#}" + !recovered_missing_secret + .profiles + .contains_key("github:missing-access"), + "oauth profile missing access_token should be dropped on load: {recovered_missing_secret:#?}" + ); + assert!( + !recovered_missing_secret.active_profiles.contains_key("github"), + "active profile pointing at a dropped profile should be purged: {recovered_missing_secret:#?}" + ); + let rewritten_missing_secret: Value = serde_json::from_str( + &std::fs::read_to_string(missing_oauth_secret_dir.join("auth-profiles.json")) + .expect("read rewritten missing-oauth-secret store"), + ) + .expect("rewritten missing-oauth-secret store should be json"); + assert!( + rewritten_missing_secret + .pointer("/profiles/github:missing-access") + .is_none(), + "dropped oauth profile should be purged from persisted store: {rewritten_missing_secret}" ); let public_api_dir = tmp.path().join("public-api-errors"); diff --git a/tests/inference_provider_admin_round22_raw_coverage_e2e.rs b/tests/inference_provider_admin_round22_raw_coverage_e2e.rs index b7e09579d..0730975f8 100644 --- a/tests/inference_provider_admin_round22_raw_coverage_e2e.rs +++ b/tests/inference_provider_admin_round22_raw_coverage_e2e.rs @@ -8,7 +8,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::{ atomic::{AtomicUsize, Ordering}, - Arc, Mutex, + Arc, Mutex, OnceLock, }; use async_trait::async_trait; @@ -81,19 +81,38 @@ impl Drop for EnvVarGuard { fn drop(&mut self) { match &self.previous { Some(value) => { - // SAFETY: this integration test is validated with --test-threads=1. + // SAFETY: mutation is serialized by `env_lock()` (see below). unsafe { std::env::set_var(self.key, value) } } None => { - // SAFETY: this integration test is validated with --test-threads=1. + // SAFETY: mutation is serialized by `env_lock()` (see below). unsafe { std::env::remove_var(self.key) } } } } } +/// Serializes the whole suite's process-global env access. +/// +/// Several tests mutate `OPENHUMAN_WORKSPACE` / `OPENHUMAN_OLLAMA_BASE_URL` / +/// `PATH` via [`EnvVarGuard`]. `cargo test` (and `cargo llvm-cov`) run a +/// binary's tests on multiple threads by default, so without this lock those +/// mutations race and a test reads another test's workspace/config — observed +/// as a flaky failure under `cargo llvm-cov` (the coverage job does not pass +/// `--test-threads=1`). Every test takes this guard up front so the suite is +/// effectively serialized regardless of the runner's thread count. +static ENV_LOCK: OnceLock> = OnceLock::new(); + +fn env_lock() -> std::sync::MutexGuard<'static, ()> { + ENV_LOCK + .get_or_init(|| Mutex::new(())) + .lock() + .unwrap_or_else(|e| e.into_inner()) +} + #[tokio::test] async fn compatible_provider_covers_responses_fallback_auth_and_merge_system_edges() { + let _env = env_lock(); let (base, state) = serve_mock().await; let fallback = OpenAiCompatibleProvider::new( @@ -209,6 +228,7 @@ async fn compatible_provider_covers_responses_fallback_auth_and_merge_system_edg #[tokio::test] async fn provider_admin_model_listing_covers_openrouter_validation_and_local_synthesis() { + let _env = env_lock(); let (base, state) = serve_mock().await; let tmp = tempdir().expect("tempdir"); let mut config = temp_config(&tmp); @@ -291,6 +311,7 @@ async fn provider_admin_model_listing_covers_openrouter_validation_and_local_syn #[tokio::test] async fn factory_covers_legacy_api_key_scoping_and_abstract_model_errors() { + let _env = env_lock(); let (base, state) = serve_mock().await; let tmp = tempdir().expect("tempdir"); let mut config = temp_config(&tmp); @@ -372,6 +393,7 @@ async fn factory_covers_legacy_api_key_scoping_and_abstract_model_errors() { #[tokio::test] async fn reliable_provider_covers_chat_tools_streaming_and_context_bail_edges() { + let _env = env_lock(); let calls = Arc::new(AtomicUsize::new(0)); let provider = ReliableProvider::new( vec![( @@ -491,6 +513,7 @@ async fn reliable_provider_covers_chat_tools_streaming_and_context_bail_edges() #[tokio::test] async fn local_admin_covers_diagnostics_errors_assets_status_and_shutdown_with_fake_bins() { + let _env = env_lock(); let (base, _state) = serve_mock().await; let tmp = tempdir().expect("tempdir"); let mut config = temp_config(&tmp);