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

2026-09-16 12:13:30 +00:00 · 2025-07-23 21:59:03 -07:00
parent d2be0720b5
commit 3f00f1a74a
14 changed files with 94 additions and 74 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 = "0.0.0"
+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,18 +1,11 @@
 #![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;
 use codex_core::config::ConfigOverrides;
 use codex_core::config::ConfigToml;

-/// Returns a default `Config` whose on-disk state is confined to the provided
-/// temporary directory. Using a per-test directory keeps tests hermetic and
-/// avoids clobbering a developer’s real `~/.codex`.
 pub fn load_default_config_for_test(codex_home: &TempDir) -> Config {
    Config::load_from_base_config_with_overrides(
        ConfigToml::default(),
@@ -22,15 +15,6 @@ pub fn load_default_config_for_test(codex_home: &TempDir) -> Config {
    .expect("defaults for test should always succeed")
 }

-/// Builds an SSE stream body from a JSON fixture.
-///
-/// The fixture must contain an array of objects where each object represents a
-/// single SSE event with at least a `type` field matching the `event:` value.
-/// Additional fields become the JSON payload for the `data:` line. An object
-/// 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"))
@@ -51,11 +35,6 @@ pub fn load_sse_fixture(path: impl AsRef<std::path::Path>) -> String {
        .collect()
 }

-/// Same as [`load_sse_fixture`], but replaces the placeholder `__ID__` in the
-/// 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 +56,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,4 +1,4 @@
-mod common;
+// Support code lives in the `mcp_test_support` crate under tests/common.

 use std::collections::HashMap;
 use std::env;
@@ -26,11 +26,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);

@@ -60,8 +60,7 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> {

    let McpHandle {
        process: mut mcp_process,
-        server: _server,
-        dir: _dir,
+        ..
    } = create_mcp_process(vec![
        create_shell_sse_response(
            shell_command.clone(),
@@ -204,8 +203,7 @@ async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> {

    let McpHandle {
        process: mut mcp_process,
-        server: _server,
-        dir: _dir,
+        ..
    } = create_mcp_process(vec![
        create_apply_patch_sse_response(&patch_content, "call1234")?,
        create_final_assistant_message_sse_response("Patch has been applied successfully!")?,
@@ -395,11 +393,9 @@ fn create_expected_patch_approval_elicitation_request(
 pub struct McpHandle {
    pub process: McpProcess,
    /// Retain the server for the lifetime of the McpProcess.
-    #[allow(dead_code)]
-    server: MockServer,
+    _server: MockServer,
    /// Retain the temporary directory for the lifetime of the McpProcess.
-    #[allow(dead_code)]
-    dir: TempDir,
+    _dir: TempDir,
 }

 async fn create_mcp_process(responses: Vec<String>) -> anyhow::Result<McpHandle> {
@@ -410,8 +406,8 @@ async fn create_mcp_process(responses: Vec<String>) -> anyhow::Result<McpHandle>
    timeout(DEFAULT_READ_TIMEOUT, mcp_process.initialize()).await??;
    Ok(McpHandle {
        process: mcp_process,
-        server,
-        dir: codex_home,
+        _server: server,
+        _dir: codex_home,
    })
 }

--- a/codex-rs/mcp-server/tests/common/Cargo.toml
+++ b/codex-rs/mcp-server/tests/common/Cargo.toml
@@ -0,0 +1,19 @@
+[package]
+name = "mcp_test_support"
+version = "0.0.0"
+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
@@ -1,11 +1,12 @@
+// Re-export the items that tests use. This crate lives under
+// `tests/common` so it is only built for tests via a path dev-dependency.
+
 mod mcp_process;
 mod mock_model_server;
 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
@@ -33,10 +33,8 @@ use tokio::process::Command;
 pub struct McpProcess {
    next_request_id: AtomicI64,
    /// Retain this child process until the client is dropped. The Tokio runtime
-    /// will make a "best effort" to reap the process after it exits, but it is
-    /// not a guarantee. See the `kill_on_drop` documentation for details.
-    #[allow(dead_code)]
-    process: Child,
+    /// will make a best effort to reap the process after it exits.
+    _process: Child,
    stdin: ChildStdin,
    stdout: BufReader<ChildStdout>,
 }
@@ -71,7 +69,7 @@ impl McpProcess {
        let stdout = BufReader::new(stdout);
        Ok(Self {
            next_request_id: AtomicI64::new(0),
-            process,
+            _process: process,
            stdin,
            stdout,
        })
@@ -191,8 +189,6 @@ impl McpProcess {
        Ok(request_id)
    }

-    // allow dead code
-    #[allow(dead_code)]
    pub async fn send_response(
        &mut self,
        id: RequestId,
@@ -220,8 +216,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 +238,6 @@ impl McpProcess {
        }
    }

-    // allow dead code
-    #[allow(dead_code)]
    pub async fn read_stream_until_response_message(
        &mut self,
        request_id: RequestId,
@@ -312,8 +304,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);