fix: create separate test_support crates to eliminate #[allow(dead_code)] (#1667)

Because of a quirk of how implementation tests work in Rust, we had a number of `#[allow(dead_code)]` annotations that were misleading because the functions _were_ being used, just not by all integration tests in a `tests/` folder, so when compiling the test that did not use the function, clippy would complain that it was unused. This fixes things by create a "test_support" crate under the `tests/` folder that is imported as a dev dependency for the respective crate.
2025-07-24 12:19:46 -07:00
parent 2437a8d17a
commit 7af9cedbd7
14 changed files with 86 additions and 45 deletions
--- a/codex-rs/Cargo.lock
+++ b/codex-rs/Cargo.lock
@@ -663,6 +663,7 @@ dependencies = [
 "bytes",
 "codex-apply-patch",
 "codex-mcp-client",
+ "core_test_support",
 "dirs",
 "env-flags",
 "eventsource-stream",
@@ -799,6 +800,7 @@ dependencies = [
 "codex-core",
 "codex-linux-sandbox",
 "mcp-types",
+ "mcp_test_support",
 "pretty_assertions",
 "schemars 0.8.22",
 "serde",
@@ -952,6 +954,16 @@ version = "0.8.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b"

+[[package]]
+name = "core_test_support"
+version = "0.0.0"
+dependencies = [
+ "codex-core",
+ "serde_json",
+ "tempfile",
+ "tokio",
+]
+
 [[package]]
 name = "cpufeatures"
 version = "0.2.17"
@@ -2604,6 +2616,22 @@ dependencies = [
 "serde_json",
 ]

+[[package]]
+name = "mcp_test_support"
+version = "0.0.0"
+dependencies = [
+ "anyhow",
+ "assert_cmd",
+ "codex-mcp-server",
+ "mcp-types",
+ "pretty_assertions",
+ "serde_json",
+ "shlex",
+ "tempfile",
+ "tokio",
+ "wiremock",
+]
+
 [[package]]
 name = "memchr"
 version = "2.7.5"
--- a/codex-rs/core/Cargo.toml
+++ b/codex-rs/core/Cargo.toml
@@ -62,6 +62,7 @@ openssl-sys = { version = "*", features = ["vendored"] }

 [dev-dependencies]
 assert_cmd = "2"
+core_test_support = { path = "tests/common" }
 maplit = "1.0.2"
 predicates = "3"
 pretty_assertions = "1.4.1"
--- a/codex-rs/core/tests/client.rs
+++ b/codex-rs/core/tests/client.rs
@@ -5,10 +5,10 @@ use codex_core::protocol::EventMsg;
 use codex_core::protocol::InputItem;
 use codex_core::protocol::Op;
 use codex_core::protocol::SessionConfiguredEvent;
-mod test_support;
+use core_test_support::load_default_config_for_test;
+use core_test_support::load_sse_fixture_with_id;
+use core_test_support::wait_for_event;
 use tempfile::TempDir;
-use test_support::load_default_config_for_test;
-use test_support::load_sse_fixture_with_id;
 use wiremock::Mock;
 use wiremock::MockServer;
 use wiremock::ResponseTemplate;
@@ -84,14 +84,13 @@ async fn includes_session_id_and_model_headers_in_request() {
        .unwrap();

    let EventMsg::SessionConfigured(SessionConfiguredEvent { session_id, .. }) =
-        test_support::wait_for_event(&codex, |ev| matches!(ev, EventMsg::SessionConfigured(_)))
-            .await
+        wait_for_event(&codex, |ev| matches!(ev, EventMsg::SessionConfigured(_))).await
    else {
        unreachable!()
    };

    let current_session_id = Some(session_id.to_string());
-    test_support::wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
+    wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;

    // get request from the server
    let request = &server.received_requests().await.unwrap()[0];
@@ -160,7 +159,7 @@ async fn includes_base_instructions_override_in_request() {
        .await
        .unwrap();

-    test_support::wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
+    wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;

    let request = &server.received_requests().await.unwrap()[0];
    let request_body = request.body_json::<serde_json::Value>().unwrap();
--- a/codex-rs/core/tests/common/Cargo.toml
+++ b/codex-rs/core/tests/common/Cargo.toml
@@ -0,0 +1,13 @@
+[package]
+name = "core_test_support"
+version = { workspace = true }
+edition = "2024"
+
+[lib]
+path = "lib.rs"
+
+[dependencies]
+codex-core = { path = "../.." }
+serde_json = "1"
+tempfile = "3"
+tokio = { version = "1", features = ["time"] }
--- a/codex-rs/core/tests/test_support.rs
+++ b/codex-rs/core/tests/test_support.rs
@@ -1,9 +1,5 @@
 #![allow(clippy::expect_used)]

-// Helpers shared by the integration tests.  These are located inside the
-// `tests/` tree on purpose so they never become part of the public API surface
-// of the `codex-core` crate.
-
 use tempfile::TempDir;

 use codex_core::config::Config;
@@ -30,7 +26,6 @@ pub fn load_default_config_for_test(codex_home: &TempDir) -> Config {
 /// with only a `type` field results in an event with no `data:` section. This
 /// makes it trivial to extend the fixtures as OpenAI adds new event kinds or
 /// fields.
-#[allow(dead_code)]
 pub fn load_sse_fixture(path: impl AsRef<std::path::Path>) -> String {
    let events: Vec<serde_json::Value> =
        serde_json::from_reader(std::fs::File::open(path).expect("read fixture"))
@@ -55,7 +50,6 @@ pub fn load_sse_fixture(path: impl AsRef<std::path::Path>) -> String {
 /// fixture template with the supplied identifier before parsing. This lets a
 /// single JSON template be reused by multiple tests that each need a unique
 /// `response_id`.
-#[allow(dead_code)]
 pub fn load_sse_fixture_with_id(path: impl AsRef<std::path::Path>, id: &str) -> String {
    let raw = std::fs::read_to_string(path).expect("read fixture template");
    let replaced = raw.replace("__ID__", id);
@@ -77,7 +71,6 @@ pub fn load_sse_fixture_with_id(path: impl AsRef<std::path::Path>, id: &str) ->
        .collect()
 }

-#[allow(dead_code)]
 pub async fn wait_for_event<F>(
    codex: &codex_core::Codex,
    mut predicate: F,
--- a/codex-rs/core/tests/live_agent.rs
+++ b/codex-rs/core/tests/live_agent.rs
@@ -26,9 +26,8 @@ use codex_core::protocol::ErrorEvent;
 use codex_core::protocol::EventMsg;
 use codex_core::protocol::InputItem;
 use codex_core::protocol::Op;
-mod test_support;
+use core_test_support::load_default_config_for_test;
 use tempfile::TempDir;
-use test_support::load_default_config_for_test;
 use tokio::sync::Notify;
 use tokio::time::timeout;

--- a/codex-rs/core/tests/stream_no_completed.rs
+++ b/codex-rs/core/tests/stream_no_completed.rs
@@ -9,11 +9,10 @@ use codex_core::exec::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
 use codex_core::protocol::EventMsg;
 use codex_core::protocol::InputItem;
 use codex_core::protocol::Op;
-mod test_support;
+use core_test_support::load_default_config_for_test;
+use core_test_support::load_sse_fixture;
+use core_test_support::load_sse_fixture_with_id;
 use tempfile::TempDir;
-use test_support::load_default_config_for_test;
-use test_support::load_sse_fixture;
-use test_support::load_sse_fixture_with_id;
 use tokio::time::timeout;
 use wiremock::Mock;
 use wiremock::MockServer;
--- a/codex-rs/mcp-server/Cargo.toml
+++ b/codex-rs/mcp-server/Cargo.toml
@@ -37,6 +37,7 @@ uuid = { version = "1", features = ["serde", "v4"] }

 [dev-dependencies]
 assert_cmd = "2"
+mcp_test_support = { path = "tests/common" }
 pretty_assertions = "1.4.1"
 tempfile = "3"
 tokio-test = "0.4"
--- a/codex-rs/mcp-server/tests/codex_tool.rs
+++ b/codex-rs/mcp-server/tests/codex_tool.rs
@@ -1,5 +1,3 @@
-mod common;
-
 use std::collections::HashMap;
 use std::env;
 use std::path::Path;
@@ -26,11 +24,11 @@ use tempfile::TempDir;
 use tokio::time::timeout;
 use wiremock::MockServer;

-use crate::common::McpProcess;
-use crate::common::create_apply_patch_sse_response;
-use crate::common::create_final_assistant_message_sse_response;
-use crate::common::create_mock_chat_completions_server;
-use crate::common::create_shell_sse_response;
+use mcp_test_support::McpProcess;
+use mcp_test_support::create_apply_patch_sse_response;
+use mcp_test_support::create_final_assistant_message_sse_response;
+use mcp_test_support::create_mock_chat_completions_server;
+use mcp_test_support::create_shell_sse_response;

 const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);

--- a/codex-rs/mcp-server/tests/common/Cargo.toml
+++ b/codex-rs/mcp-server/tests/common/Cargo.toml
@@ -0,0 +1,24 @@
+[package]
+name = "mcp_test_support"
+version = { workspace = true }
+edition = "2024"
+
+[lib]
+path = "lib.rs"
+
+[dependencies]
+anyhow = "1"
+assert_cmd = "2"
+codex-mcp-server = { path = "../.." }
+mcp-types = { path = "../../../mcp-types" }
+pretty_assertions = "1.4.1"
+serde_json = "1"
+shlex = "1.3.0"
+tempfile = "3"
+tokio = { version = "1", features = [
+    "io-std",
+    "macros",
+    "process",
+    "rt-multi-thread",
+] }
+wiremock = "0.6"
--- a/codex-rs/mcp-server/tests/common/lib.rs
+++ b/codex-rs/mcp-server/tests/common/lib.rs
@@ -4,8 +4,6 @@ mod responses;

 pub use mcp_process::McpProcess;
 pub use mock_model_server::create_mock_chat_completions_server;
-#[allow(unused_imports)]
 pub use responses::create_apply_patch_sse_response;
-#[allow(unused_imports)]
 pub use responses::create_final_assistant_message_sse_response;
 pub use responses::create_shell_sse_response;
--- a/codex-rs/mcp-server/tests/common/mcp_process.rs
+++ b/codex-rs/mcp-server/tests/common/mcp_process.rs
@@ -191,8 +191,6 @@ impl McpProcess {
        Ok(request_id)
    }

-    // allow dead code
-    #[allow(dead_code)]
    pub async fn send_response(
        &mut self,
        id: RequestId,
@@ -220,8 +218,6 @@ impl McpProcess {
        let message = serde_json::from_str::<JSONRPCMessage>(&line)?;
        Ok(message)
    }
-    // allow dead code
-    #[allow(dead_code)]
    pub async fn read_stream_until_request_message(&mut self) -> anyhow::Result<JSONRPCRequest> {
        loop {
            let message = self.read_jsonrpc_message().await?;
@@ -244,8 +240,6 @@ impl McpProcess {
        }
    }

-    // allow dead code
-    #[allow(dead_code)]
    pub async fn read_stream_until_response_message(
        &mut self,
        request_id: RequestId,
@@ -312,8 +306,6 @@ impl McpProcess {
        }
    }

-    // allow dead code
-    #[allow(dead_code)]
    pub async fn send_notification(
        &mut self,
        method: &str,
--- a/codex-rs/mcp-server/tests/common/responses.rs
+++ b/codex-rs/mcp-server/tests/common/responses.rs
@@ -39,8 +39,6 @@ pub fn create_shell_sse_response(
    Ok(sse)
 }

-// allow dead code
-#[allow(dead_code)]
 pub fn create_final_assistant_message_sse_response(message: &str) -> anyhow::Result<String> {
    let assistant_message = json!({
        "choices": [
@@ -60,8 +58,6 @@ pub fn create_final_assistant_message_sse_response(message: &str) -> anyhow::Res
    Ok(sse)
 }

-// allow dead code
-#[allow(dead_code)]
 pub fn create_apply_patch_sse_response(
    patch_content: &str,
    call_id: &str,
--- a/codex-rs/mcp-server/tests/interrupt.rs
+++ b/codex-rs/mcp-server/tests/interrupt.rs
@@ -1,5 +1,5 @@
 #![cfg(unix)]
-mod common;
+// Support code lives in the `mcp_test_support` crate under tests/common.

 use std::path::Path;

@@ -11,9 +11,9 @@ use serde_json::json;
 use tempfile::TempDir;
 use tokio::time::timeout;

-use crate::common::McpProcess;
-use crate::common::create_mock_chat_completions_server;
-use crate::common::create_shell_sse_response;
+use mcp_test_support::McpProcess;
+use mcp_test_support::create_mock_chat_completions_server;
+use mcp_test_support::create_shell_sse_response;

 const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);