From e91f91b542c83b95d51f9916c34a3fe73a07a0cc Mon Sep 17 00:00:00 2001 From: zyycn Date: Tue, 7 Jul 2026 00:15:30 +0800 Subject: [PATCH] test: move source tests to integration suite Co-authored-by: Codex --- backend/src/infra/paths.rs | 57 -------------- .../dispatch/responses/event_recording.rs | 58 -------------- backend/src/proxy/dispatch/responses/trace.rs | 19 ----- backend/src/upstream/transport/diagnostics.rs | 31 -------- backend/tests/admin/monitoring/dashboard.rs | 16 ++-- backend/tests/infra/mod.rs | 1 + backend/tests/infra/paths.rs | 74 ++++++++++++++++++ .../tests/proxy/dispatch/chat_upstream/mod.rs | 28 ++++++- .../dispatch/chat_upstream/usage_logging.rs | 76 +++++++++++++++++++ .../tests/upstream/transport/diagnostics.rs | 26 +++++++ backend/tests/upstream/transport/mod.rs | 1 + 11 files changed, 210 insertions(+), 177 deletions(-) create mode 100644 backend/tests/infra/paths.rs create mode 100644 backend/tests/upstream/transport/diagnostics.rs diff --git a/backend/src/infra/paths.rs b/backend/src/infra/paths.rs index d9817ded6..f77b325a1 100644 --- a/backend/src/infra/paths.rs +++ b/backend/src/infra/paths.rs @@ -90,60 +90,3 @@ fn parse_installation_id(raw: &str) -> Option { fn generate_installation_id() -> String { Uuid::new_v4().to_string() } - -#[cfg(test)] -mod tests { - use std::sync::Mutex; - - use super::*; - - static ENV_LOCK: Mutex<()> = Mutex::new(()); - - #[test] - fn data_dir_should_use_xdg_data_home_directly() { - let _guard = ENV_LOCK.lock().unwrap(); - let dir = tempfile::tempdir().unwrap(); - let previous = std::env::var_os("XDG_DATA_HOME"); - std::env::set_var("XDG_DATA_HOME", dir.path()); - - let resolved = data_dir(); - - restore_env("XDG_DATA_HOME", previous); - assert_eq!(resolved, dir.path()); - } - - #[test] - fn load_or_create_installation_id_should_persist_under_data_dir_root() { - let dir = tempfile::tempdir().unwrap(); - let data_path = dir.path().join(INSTALLATION_ID_FILE_NAME); - - let installation_id = load_or_create_installation_id_from_paths(None, Some(&data_path)) - .expect("installation id should be generated"); - - assert_eq!(fs::read_to_string(data_path).unwrap(), installation_id); - } - - #[test] - fn load_or_create_installation_id_should_seed_data_dir_from_codex_desktop_id() { - let dir = tempfile::tempdir().unwrap(); - let codex_path = dir.path().join(".codex").join(INSTALLATION_ID_FILE_NAME); - let data_path = dir.path().join("data").join(INSTALLATION_ID_FILE_NAME); - let desktop_id = "018f8f6b-1d7b-7b7c-b9c8-8c9f4c6d0e1a"; - persist_installation_id(&codex_path, desktop_id).unwrap(); - - let installation_id = - load_or_create_installation_id_from_paths(Some(&codex_path), Some(&data_path)) - .expect("installation id should be seeded"); - - assert_eq!(installation_id, desktop_id); - assert_eq!(fs::read_to_string(data_path).unwrap(), desktop_id); - } - - fn restore_env(key: &str, value: Option) { - if let Some(value) = value { - std::env::set_var(key, value); - } else { - std::env::remove_var(key); - } - } -} diff --git a/backend/src/proxy/dispatch/responses/event_recording.rs b/backend/src/proxy/dispatch/responses/event_recording.rs index 7cca4aa4f..dd16d0c9c 100644 --- a/backend/src/proxy/dispatch/responses/event_recording.rs +++ b/backend/src/proxy/dispatch/responses/event_recording.rs @@ -606,61 +606,3 @@ pub(super) fn insert_websocket_pool_decision( object.insert("websocketPool".to_string(), decision.metadata_value()); } } - -#[cfg(test)] -mod tests { - use reqwest::StatusCode; - use serde_json::{Map, Value}; - - use super::*; - - #[test] - fn request_summary_should_record_shape_without_input_content() { - let secret_input = "do not persist this input"; - let mut body = Map::new(); - body.insert("model".to_string(), Value::String("gpt-5.5".to_string())); - body.insert("input".to_string(), Value::String(secret_input.to_string())); - body.insert("stream".to_string(), Value::Bool(true)); - let mut request = CodexResponsesRequest::from_body(body); - request.use_websocket = true; - - let summary = response_request_summary(&request, CodexBackendTransport::WebSocket); - - assert_eq!(summary["inputType"], "string"); - assert!(summary["inputItemsCount"].is_null()); - assert_eq!(summary["transport"], "websocket"); - assert_eq!(summary["localTransport"]["useWebsocket"], true); - assert!(!summary.to_string().contains(secret_input)); - } - - #[test] - fn upstream_dispatch_client_status_should_match_response_surface() { - let error = ResponseDispatchError::Upstream(CodexClientError::Upstream { - status: StatusCode::BAD_REQUEST, - body: "upstream bad request".to_string(), - retry_after_seconds: None, - diagnostics: CodexUpstreamDiagnostics::with_status(StatusCode::BAD_REQUEST.as_u16()), - set_cookie_headers: Vec::new(), - }); - - assert_eq!(dispatch_error_client_status_code(&error, false, false), 502); - assert_eq!(dispatch_error_client_status_code(&error, false, true), 400); - assert_eq!(dispatch_error_client_status_code(&error, true, false), 200); - } - - #[test] - fn trace_metadata_should_include_attempt_sequence() { - let mut trace = ResponseDispatchTrace::default(); - let _first = trace.start_attempt("acct_a"); - let second = trace.start_attempt("acct_b"); - let mut metadata = json!({}); - - insert_response_trace_metadata(&mut metadata, &trace, Some(&second)); - - assert_eq!(metadata["attemptIndex"], 1); - assert_eq!(metadata["attemptAccountId"], "acct_b"); - assert_eq!(metadata["attemptCount"], 2); - assert_eq!(metadata["attempts"][0]["accountId"], "acct_a"); - assert_eq!(metadata["attempts"][1]["accountId"], "acct_b"); - } -} diff --git a/backend/src/proxy/dispatch/responses/trace.rs b/backend/src/proxy/dispatch/responses/trace.rs index 881368dce..02066f3bc 100644 --- a/backend/src/proxy/dispatch/responses/trace.rs +++ b/backend/src/proxy/dispatch/responses/trace.rs @@ -37,22 +37,3 @@ impl ResponseDispatchAttempt { &self.account_id } } - -#[cfg(test)] -mod tests { - use super::ResponseDispatchTrace; - - #[test] - fn trace_should_assign_monotonic_attempt_indexes() { - let mut trace = ResponseDispatchTrace::default(); - - let first = trace.start_attempt("acct_a"); - let second = trace.start_attempt("acct_b"); - - assert_eq!(first.index(), 0); - assert_eq!(first.account_id(), "acct_a"); - assert_eq!(second.index(), 1); - assert_eq!(second.account_id(), "acct_b"); - assert_eq!(trace.attempts(), &[first, second]); - } -} diff --git a/backend/src/upstream/transport/diagnostics.rs b/backend/src/upstream/transport/diagnostics.rs index 80da25f51..8a755a961 100644 --- a/backend/src/upstream/transport/diagnostics.rs +++ b/backend/src/upstream/transport/diagnostics.rs @@ -71,34 +71,3 @@ fn header_value(headers: &HeaderMap, name: &str) -> Option { .filter(|value| !value.is_empty()) .map(ToString::to_string) } - -#[cfg(test)] -mod tests { - use reqwest::header::{HeaderMap, HeaderValue}; - - use super::CodexUpstreamDiagnostics; - - #[test] - fn diagnostics_should_extract_request_id_and_trace_headers() { - let mut headers = HeaderMap::new(); - headers.insert("x-request-id", HeaderValue::from_static("req_1")); - headers.insert("cf-ray", HeaderValue::from_static("ray_1")); - - let diagnostics = CodexUpstreamDiagnostics::from_headers(Some(429), &headers); - - assert_eq!(diagnostics.status_code, Some(429)); - assert_eq!(diagnostics.request_id.as_deref(), Some("req_1")); - assert_eq!(diagnostics.cf_ray(), Some("ray_1")); - } - - #[test] - fn diagnostics_should_not_treat_cf_ray_as_request_id() { - let mut headers = HeaderMap::new(); - headers.insert("cf-ray", HeaderValue::from_static("ray_1")); - - let diagnostics = CodexUpstreamDiagnostics::from_headers(Some(403), &headers); - - assert_eq!(diagnostics.request_id, None); - assert_eq!(diagnostics.cf_ray(), Some("ray_1")); - } -} diff --git a/backend/tests/admin/monitoring/dashboard.rs b/backend/tests/admin/monitoring/dashboard.rs index c3e99dc8a..475cb3292 100644 --- a/backend/tests/admin/monitoring/dashboard.rs +++ b/backend/tests/admin/monitoring/dashboard.rs @@ -14,7 +14,7 @@ use codex_proxy_rs::{ }, infra::{ database::connect_sqlite, - time::{china_datetime, china_day_start, china_hour, china_hour_start}, + time::{china_datetime, china_day_start, china_hour}, }, proxy::dispatch::session_affinity::SqliteSessionAffinityStore, runtime::{ @@ -222,20 +222,14 @@ async fn dashboard_latency_trend_should_use_first_token_latency() { ) .await; let now = Utc::now(); - let current_hour_start = china_hour_start(now); - let sample_hour_start = if now - current_hour_start > Duration::minutes(31) { - current_hour_start - } else { - current_hour_start - Duration::hours(1) - }; - let mut slow_completion = - usage_record_with_tokens(sample_hour_start + Duration::minutes(1), 10); + let mut slow_completion = usage_record_with_tokens(now, 10); slow_completion.latency_ms = Some(10_000); slow_completion.metadata["firstTokenMs"] = json!(100_i64); - let mut fast_completion = - usage_record_with_tokens(sample_hour_start + Duration::minutes(16), 10); + slow_completion.metadata["serviceTier"] = json!("default"); + let mut fast_completion = usage_record_with_tokens(now, 10); fast_completion.latency_ms = Some(1_000); fast_completion.metadata["firstTokenMs"] = json!(500_i64); + fast_completion.metadata["serviceTier"] = json!("priority"); store.append(&slow_completion).await.unwrap(); store.append(&fast_completion).await.unwrap(); diff --git a/backend/tests/infra/mod.rs b/backend/tests/infra/mod.rs index 2288c0f15..0d3408b5d 100644 --- a/backend/tests/infra/mod.rs +++ b/backend/tests/infra/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod format; pub(crate) mod log_rotation; +pub(crate) mod paths; pub(crate) mod storage_schema; pub(crate) mod time; diff --git a/backend/tests/infra/paths.rs b/backend/tests/infra/paths.rs new file mode 100644 index 000000000..426575577 --- /dev/null +++ b/backend/tests/infra/paths.rs @@ -0,0 +1,74 @@ +use std::{ffi::OsString, fs, sync::Mutex}; + +use codex_proxy_rs::infra::paths::{ensure_data_dir, load_or_create_installation_id}; + +const INSTALLATION_ID_FILE_NAME: &str = "installation_id"; + +static ENV_LOCK: Mutex<()> = Mutex::new(()); + +#[test] +fn data_dir_should_use_xdg_data_home_directly() { + let _guard = ENV_LOCK.lock().unwrap(); + let dir = tempfile::tempdir().unwrap(); + let _xdg_data_home = EnvVarGuard::set("XDG_DATA_HOME", dir.path().as_os_str().to_owned()); + + let resolved = ensure_data_dir().unwrap(); + + assert_eq!(resolved, dir.path()); +} + +#[test] +fn load_or_create_installation_id_should_persist_under_data_dir_root() { + let dir = tempfile::tempdir().unwrap(); + + let installation_id = + load_or_create_installation_id(Some(dir.path())).expect("installation id should generate"); + + assert_eq!( + fs::read_to_string(dir.path().join(INSTALLATION_ID_FILE_NAME)).unwrap(), + installation_id + ); +} + +#[test] +fn load_or_create_installation_id_should_seed_data_dir_from_codex_desktop_id() { + let _guard = ENV_LOCK.lock().unwrap(); + let home = tempfile::tempdir().unwrap(); + let data = tempfile::tempdir().unwrap(); + let codex_path = home.path().join(".codex").join(INSTALLATION_ID_FILE_NAME); + let desktop_id = "018f8f6b-1d7b-7b7c-b9c8-8c9f4c6d0e1a"; + fs::create_dir_all(codex_path.parent().unwrap()).unwrap(); + fs::write(&codex_path, desktop_id).unwrap(); + let _home = EnvVarGuard::set("HOME", home.path().as_os_str().to_owned()); + + let installation_id = + load_or_create_installation_id(Some(data.path())).expect("installation id should seed"); + + assert_eq!(installation_id, desktop_id); + assert_eq!( + fs::read_to_string(data.path().join(INSTALLATION_ID_FILE_NAME)).unwrap(), + desktop_id + ); +} + +struct EnvVarGuard { + key: &'static str, + previous: Option, +} + +impl EnvVarGuard { + fn set(key: &'static str, value: OsString) -> Self { + let previous = std::env::var_os(key); + std::env::set_var(key, value); + Self { key, previous } + } +} + +impl Drop for EnvVarGuard { + fn drop(&mut self) { + match &self.previous { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), + } + } +} diff --git a/backend/tests/proxy/dispatch/chat_upstream/mod.rs b/backend/tests/proxy/dispatch/chat_upstream/mod.rs index ff23ab248..85a396681 100644 --- a/backend/tests/proxy/dispatch/chat_upstream/mod.rs +++ b/backend/tests/proxy/dispatch/chat_upstream/mod.rs @@ -554,6 +554,17 @@ async fn test_app_with_two_accounts( (app, api_key, pool, dir) } +async fn test_app_with_two_accounts_and_logging( + base_url: String, +) -> (axum::Router, String, SqlitePool, tempfile::TempDir) { + let (app, _state, api_key, pool, dir) = + test_app_with_two_accounts_and_state_config(base_url, |config| { + config.logging.enabled = true; + }) + .await; + (app, api_key, pool, dir) +} + async fn test_app_with_two_accounts_and_state( base_url: String, ) -> ( @@ -562,6 +573,19 @@ async fn test_app_with_two_accounts_and_state( String, SqlitePool, tempfile::TempDir, +) { + test_app_with_two_accounts_and_state_config(base_url, |_| {}).await +} + +async fn test_app_with_two_accounts_and_state_config( + base_url: String, + configure: impl FnOnce(&mut AppConfig), +) -> ( + axum::Router, + AppState, + String, + SqlitePool, + tempfile::TempDir, ) { let dir = tempfile::tempdir().unwrap(); let db = dir.path().join("openai-responses-fallback.sqlite"); @@ -576,8 +600,10 @@ async fn test_app_with_two_accounts_and_state( "chatgpt-secondary", ) .await; + let mut config = test_config(url, base_url); + configure(&mut config); let state = test_app_state_with_pool_and_installation_id( - &test_config(url, base_url), + &config, pool.clone(), TEST_INSTALLATION_ID.to_string(), ) diff --git a/backend/tests/proxy/dispatch/chat_upstream/usage_logging.rs b/backend/tests/proxy/dispatch/chat_upstream/usage_logging.rs index 9b5c65cc3..5e2686ed4 100644 --- a/backend/tests/proxy/dispatch/chat_upstream/usage_logging.rs +++ b/backend/tests/proxy/dispatch/chat_upstream/usage_logging.rs @@ -861,6 +861,82 @@ async fn responses_should_record_request_count_when_5xx_retries_are_exhausted() assert_eq!(metadata["accountEmail"], "user@example.com"); } +#[tokio::test] +async fn responses_should_record_attempt_trace_after_retrying_another_account() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/codex/responses")) + .and(header("authorization", "Bearer access-primary")) + .respond_with(ResponseTemplate::new(429).set_body_json(json!({ + "error": {"message": "primary rate limited"} + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/codex/responses")) + .and(header("authorization", "Bearer access-secondary")) + .respond_with(ResponseTemplate::new(500).set_body_json(json!({ + "error": {"message": "secondary failed"} + }))) + .expect(3) + .mount(&server) + .await; + + let secret_input = "do not persist this input"; + let (app, api_key, pool, _dir) = test_app_with_two_accounts_and_logging(server.uri()).await; + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/v1/responses") + .header("authorization", format!("Bearer {api_key}")) + .header("content-type", "application/json") + .header("x-request-id", "req_attempt_trace_retry") + .body(Body::from( + json!({ + "model": "gpt-5.5", + "input": secret_input, + "stream": false, + "use_websocket": false + }) + .to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + let event = latest_response_usage_record(&pool).await; + let metadata: Value = serde_json::from_str(&event.metadata_json).unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_GATEWAY); + assert_eq!(event.level, "error"); + assert_eq!(event.request_id.as_deref(), Some("req_attempt_trace_retry")); + assert_eq!(event.account_id.as_deref(), Some("acct_secondary")); + assert_eq!(event.status_code, Some(500)); + assert_eq!(metadata["eventStatusCode"], 500); + assert_eq!(metadata["clientStatusCode"], 502); + assert_eq!(metadata["upstreamStatusCode"], 500); + assert_eq!(metadata["attemptIndex"], 1); + assert_eq!(metadata["attemptAccountId"], "acct_secondary"); + assert_eq!(metadata["attemptCount"], 2); + assert_eq!(metadata["attempts"][0]["index"], 0); + assert_eq!(metadata["attempts"][0]["accountId"], "acct_primary"); + assert_eq!(metadata["attempts"][1]["index"], 1); + assert_eq!(metadata["attempts"][1]["accountId"], "acct_secondary"); + assert_eq!(metadata["requestSummary"]["inputType"], "string"); + assert!(metadata["requestSummary"]["inputItemsCount"].is_null()); + assert_eq!(metadata["requestSummary"]["transport"], "http_sse"); + assert_eq!( + metadata["requestSummary"]["localTransport"]["useWebsocket"], + false + ); + assert!( + !metadata.to_string().contains(secret_input), + "request summary must not persist input content: {metadata:?}", + ); +} + #[tokio::test] async fn responses_should_record_session_affinity_for_completed_response() { let server = MockServer::start().await; diff --git a/backend/tests/upstream/transport/diagnostics.rs b/backend/tests/upstream/transport/diagnostics.rs new file mode 100644 index 000000000..8db2265fd --- /dev/null +++ b/backend/tests/upstream/transport/diagnostics.rs @@ -0,0 +1,26 @@ +use codex_proxy_rs::upstream::transport::CodexUpstreamDiagnostics; +use reqwest::header::{HeaderMap, HeaderValue}; + +#[test] +fn diagnostics_should_extract_request_id_and_trace_headers() { + let mut headers = HeaderMap::new(); + headers.insert("x-request-id", HeaderValue::from_static("req_1")); + headers.insert("cf-ray", HeaderValue::from_static("ray_1")); + + let diagnostics = CodexUpstreamDiagnostics::from_headers(Some(429), &headers); + + assert_eq!(diagnostics.status_code, Some(429)); + assert_eq!(diagnostics.request_id.as_deref(), Some("req_1")); + assert_eq!(diagnostics.cf_ray(), Some("ray_1")); +} + +#[test] +fn diagnostics_should_not_treat_cf_ray_as_request_id() { + let mut headers = HeaderMap::new(); + headers.insert("cf-ray", HeaderValue::from_static("ray_1")); + + let diagnostics = CodexUpstreamDiagnostics::from_headers(Some(403), &headers); + + assert_eq!(diagnostics.request_id, None); + assert_eq!(diagnostics.cf_ray(), Some("ray_1")); +} diff --git a/backend/tests/upstream/transport/mod.rs b/backend/tests/upstream/transport/mod.rs index 79227d642..3662207c8 100644 --- a/backend/tests/upstream/transport/mod.rs +++ b/backend/tests/upstream/transport/mod.rs @@ -37,6 +37,7 @@ use tokio_tungstenite::{ use crate::support::assertions::assert_substrings_appear_in_order; pub(crate) mod client; +pub(crate) mod diagnostics; pub(crate) mod headers; pub(crate) mod http_client; pub(crate) mod websocket;