From 191d620707db60f18bdda9d2f5d17b87a67439b5 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Sun, 5 Oct 2025 14:58:16 -0700 Subject: [PATCH] Use response helpers when mounting SSE test responses (#4783) ## Summary - replace manual wiremock SSE mounts in the compact suite with the shared response helpers - simplify the exec auth_env integration test by using the mount_sse_once_match helper - rely on mount_sse_sequence plus server request collection to replace the bespoke SeqResponder utility in tests ## Testing - just fmt ------ https://chatgpt.com/codex/tasks/task_i_68e2e238f2a88320a337f0b9e4098093 --- codex-rs/core/tests/suite/compact.rs | 127 ++++---------------------- codex-rs/exec/tests/suite/auth_env.rs | 18 ++-- 2 files changed, 23 insertions(+), 122 deletions(-) diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index a8515296..f6db0834 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -13,12 +13,6 @@ use core_test_support::load_default_config_for_test; use core_test_support::skip_if_no_network; use core_test_support::wait_for_event; use tempfile::TempDir; -use wiremock::Mock; -use wiremock::Request; -use wiremock::Respond; -use wiremock::ResponseTemplate; -use wiremock::matchers::method; -use wiremock::matchers::path; use codex_core::codex::compact::SUMMARIZATION_PROMPT; use core_test_support::responses::ev_assistant_message; @@ -26,14 +20,10 @@ use core_test_support::responses::ev_completed; use core_test_support::responses::ev_completed_with_tokens; use core_test_support::responses::ev_function_call; use core_test_support::responses::mount_sse_once_match; +use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; -use core_test_support::responses::sse_response; use core_test_support::responses::start_mock_server; use pretty_assertions::assert_eq; -use std::sync::Arc; -use std::sync::Mutex; -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::Ordering; // --- Test helpers ----------------------------------------------------------- pub(super) const FIRST_REPLY: &str = "FIRST_REPLY"; @@ -295,12 +285,7 @@ async fn auto_compact_runs_after_token_limit_hit() { && !body.contains(SECOND_AUTO_MSG) && !body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(first_matcher) - .respond_with(sse_response(sse1)) - .mount(&server) - .await; + mount_sse_once_match(&server, first_matcher, sse1).await; let second_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); @@ -308,23 +293,13 @@ async fn auto_compact_runs_after_token_limit_hit() { && body.contains(FIRST_AUTO_MSG) && !body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(second_matcher) - .respond_with(sse_response(sse2)) - .mount(&server) - .await; + mount_sse_once_match(&server, second_matcher, sse2).await; let third_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(third_matcher) - .respond_with(sse_response(sse3)) - .mount(&server) - .await; + mount_sse_once_match(&server, third_matcher, sse3).await; let model_provider = ModelProviderInfo { base_url: Some(format!("{}/v1", server.uri())), @@ -455,12 +430,7 @@ async fn auto_compact_persists_rollout_entries() { && !body.contains(SECOND_AUTO_MSG) && !body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(first_matcher) - .respond_with(sse_response(sse1)) - .mount(&server) - .await; + mount_sse_once_match(&server, first_matcher, sse1).await; let second_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); @@ -468,23 +438,13 @@ async fn auto_compact_persists_rollout_entries() { && body.contains(FIRST_AUTO_MSG) && !body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(second_matcher) - .respond_with(sse_response(sse2)) - .mount(&server) - .await; + mount_sse_once_match(&server, second_matcher, sse2).await; let third_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(third_matcher) - .respond_with(sse_response(sse3)) - .mount(&server) - .await; + mount_sse_once_match(&server, third_matcher, sse3).await; let model_provider = ModelProviderInfo { base_url: Some(format!("{}/v1", server.uri())), @@ -582,35 +542,20 @@ async fn auto_compact_stops_after_failed_attempt() { body.contains(FIRST_AUTO_MSG) && !body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(first_matcher) - .respond_with(sse_response(sse1.clone())) - .mount(&server) - .await; + mount_sse_once_match(&server, first_matcher, sse1.clone()).await; let second_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains("You have exceeded the maximum number of tokens") }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(second_matcher) - .respond_with(sse_response(sse2.clone())) - .mount(&server) - .await; + mount_sse_once_match(&server, second_matcher, sse2.clone()).await; let third_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); !body.contains("You have exceeded the maximum number of tokens") && body.contains(SUMMARY_TEXT) }; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(third_matcher) - .respond_with(sse_response(sse3.clone())) - .mount(&server) - .await; + mount_sse_once_match(&server, third_matcher, sse3.clone()).await; let model_provider = ModelProviderInfo { base_url: Some(format!("{}/v1", server.uri())), @@ -708,49 +653,7 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_ ev_completed_with_tokens("r6", 120), ]); - #[derive(Clone)] - struct SeqResponder { - bodies: Arc>, - calls: Arc, - requests: Arc>>>, - } - - impl SeqResponder { - fn new(bodies: Vec) -> Self { - Self { - bodies: Arc::new(bodies), - calls: Arc::new(AtomicUsize::new(0)), - requests: Arc::new(Mutex::new(Vec::new())), - } - } - - fn recorded_requests(&self) -> Vec> { - self.requests.lock().unwrap().clone() - } - } - - impl Respond for SeqResponder { - fn respond(&self, req: &Request) -> ResponseTemplate { - let idx = self.calls.fetch_add(1, Ordering::SeqCst); - self.requests.lock().unwrap().push(req.body.clone()); - let body = self - .bodies - .get(idx) - .unwrap_or_else(|| panic!("unexpected request index {idx}")) - .clone(); - ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw(body, "text/event-stream") - } - } - - let responder = SeqResponder::new(vec![sse1, sse2, sse3, sse4, sse5, sse6]); - Mock::given(method("POST")) - .and(path("/v1/responses")) - .respond_with(responder.clone()) - .expect(6) - .mount(&server) - .await; + mount_sse_sequence(&server, vec![sse1, sse2, sse3, sse4, sse5, sse6]).await; let model_provider = ModelProviderInfo { base_url: Some(format!("{}/v1", server.uri())), @@ -801,10 +704,12 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_ "auto compact should not emit task lifecycle events" ); - let request_bodies: Vec = responder - .recorded_requests() + let request_bodies: Vec = server + .received_requests() + .await + .unwrap() .into_iter() - .map(|body| String::from_utf8(body).unwrap_or_default()) + .map(|request| String::from_utf8(request.body).unwrap_or_default()) .collect(); assert_eq!( request_bodies.len(), diff --git a/codex-rs/exec/tests/suite/auth_env.rs b/codex-rs/exec/tests/suite/auth_env.rs index d59f46cd..91d7bad8 100644 --- a/codex-rs/exec/tests/suite/auth_env.rs +++ b/codex-rs/exec/tests/suite/auth_env.rs @@ -1,26 +1,22 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use core_test_support::responses::ev_completed; +use core_test_support::responses::mount_sse_once_match; use core_test_support::responses::sse; -use core_test_support::responses::sse_response; use core_test_support::responses::start_mock_server; use core_test_support::test_codex_exec::test_codex_exec; -use wiremock::Mock; use wiremock::matchers::header; -use wiremock::matchers::method; -use wiremock::matchers::path; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn exec_uses_codex_api_key_env_var() -> anyhow::Result<()> { let test = test_codex_exec(); let server = start_mock_server().await; - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(header("Authorization", "Bearer dummy")) - .respond_with(sse_response(sse(vec![ev_completed("request_0")]))) - .expect(1) - .mount(&server) - .await; + mount_sse_once_match( + &server, + header("Authorization", "Bearer dummy"), + sse(vec![ev_completed("request_0")]), + ) + .await; test.cmd_with_server(&server) .arg("--skip-git-repo-check")