From 32597bab0fcf7b170b2398f546ce2dc96342d922 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 24 Oct 2025 15:31:18 -0700 Subject: [PATCH 1/3] Fixed flaky unit test --- codex-rs/app-server/tests/suite/login.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/codex-rs/app-server/tests/suite/login.rs b/codex-rs/app-server/tests/suite/login.rs index 220769b7d6e..8d0cc93eb4f 100644 --- a/codex-rs/app-server/tests/suite/login.rs +++ b/codex-rs/app-server/tests/suite/login.rs @@ -1,4 +1,5 @@ use std::path::Path; +use std::sync::OnceLock; use std::time::Duration; use app_test_support::McpProcess; @@ -14,8 +15,19 @@ use codex_app_server_protocol::LogoutChatGptResponse; use codex_app_server_protocol::RequestId; use codex_login::login_with_api_key; use tempfile::TempDir; +use tokio::sync::Mutex; +use tokio::sync::MutexGuard; use tokio::time::timeout; +static LOGIN_TEST_MUTEX: OnceLock> = OnceLock::new(); + +// Ensures that login tests that cause the oauth server to start do not +// run concurrently. The server binds to a fixed port, so concurrent tests +// would conflict. +async fn login_test_guard() -> MutexGuard<'static, ()> { + LOGIN_TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().await +} + const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); // Helper to create a config.toml; mirrors create_conversation.rs @@ -95,6 +107,7 @@ async fn logout_chatgpt_removes_auth() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn login_and_cancel_chatgpt() { + let _guard = login_test_guard().await; let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}")); @@ -209,6 +222,7 @@ async fn login_chatgpt_rejected_when_forced_api() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn login_chatgpt_includes_forced_workspace_query_param() { + let _guard = login_test_guard().await; let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); create_config_toml_forced_workspace(codex_home.path(), "ws-forced") .unwrap_or_else(|err| panic!("write config.toml: {err}")); From 38e704ac5fd702b249d44f604ee35678d53ed019 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 24 Oct 2025 15:45:17 -0700 Subject: [PATCH 2/3] Switched from mutex to `[serial(login)]` --- codex-rs/Cargo.lock | 1 + codex-rs/app-server/Cargo.toml | 1 + codex-rs/app-server/tests/suite/login.rs | 19 +++++-------------- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 0d8522a67b9..a2e49fc506c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -853,6 +853,7 @@ dependencies = [ "pretty_assertions", "serde", "serde_json", + "serial_test", "tempfile", "tokio", "toml", diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 5efbefd6b09..c1efb2ef93b 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -47,6 +47,7 @@ base64 = { workspace = true } core_test_support = { workspace = true } os_info = { workspace = true } pretty_assertions = { workspace = true } +serial_test = { workspace = true } tempfile = { workspace = true } toml = { workspace = true } wiremock = { workspace = true } diff --git a/codex-rs/app-server/tests/suite/login.rs b/codex-rs/app-server/tests/suite/login.rs index 8d0cc93eb4f..9175da0db63 100644 --- a/codex-rs/app-server/tests/suite/login.rs +++ b/codex-rs/app-server/tests/suite/login.rs @@ -1,5 +1,4 @@ use std::path::Path; -use std::sync::OnceLock; use std::time::Duration; use app_test_support::McpProcess; @@ -14,20 +13,10 @@ use codex_app_server_protocol::LoginChatGptResponse; use codex_app_server_protocol::LogoutChatGptResponse; use codex_app_server_protocol::RequestId; use codex_login::login_with_api_key; +use serial_test::serial; use tempfile::TempDir; -use tokio::sync::Mutex; -use tokio::sync::MutexGuard; use tokio::time::timeout; -static LOGIN_TEST_MUTEX: OnceLock> = OnceLock::new(); - -// Ensures that login tests that cause the oauth server to start do not -// run concurrently. The server binds to a fixed port, so concurrent tests -// would conflict. -async fn login_test_guard() -> MutexGuard<'static, ()> { - LOGIN_TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().await -} - const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); // Helper to create a config.toml; mirrors create_conversation.rs @@ -106,8 +95,9 @@ async fn logout_chatgpt_removes_auth() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] +// Serialize tests that launch the login server since it binds to a fixed port. +#[serial(login)] async fn login_and_cancel_chatgpt() { - let _guard = login_test_guard().await; let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}")); @@ -221,8 +211,9 @@ async fn login_chatgpt_rejected_when_forced_api() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] +// Serialize tests that launch the login server since it binds to a fixed port. +#[serial(login)] async fn login_chatgpt_includes_forced_workspace_query_param() { - let _guard = login_test_guard().await; let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); create_config_toml_forced_workspace(codex_home.path(), "ws-forced") .unwrap_or_else(|err| panic!("write config.toml: {err}")); From 0aa2c73b57873a8a6a71eae0d83cf6b0b817bc79 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 24 Oct 2025 16:00:47 -0700 Subject: [PATCH 3/3] Code review feedback --- codex-rs/app-server/tests/suite/login.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server/tests/suite/login.rs b/codex-rs/app-server/tests/suite/login.rs index 9175da0db63..d4d6374bc43 100644 --- a/codex-rs/app-server/tests/suite/login.rs +++ b/codex-rs/app-server/tests/suite/login.rs @@ -96,7 +96,7 @@ async fn logout_chatgpt_removes_auth() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] // Serialize tests that launch the login server since it binds to a fixed port. -#[serial(login)] +#[serial(login_port)] async fn login_and_cancel_chatgpt() { let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}")); @@ -212,7 +212,7 @@ async fn login_chatgpt_rejected_when_forced_api() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] // Serialize tests that launch the login server since it binds to a fixed port. -#[serial(login)] +#[serial(login_port)] async fn login_chatgpt_includes_forced_workspace_query_param() { let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); create_config_toml_forced_workspace(codex_home.path(), "ws-forced")