Skip to content

Commit 35a770e

Browse files
authored
Simplify request body assertions (#4845)
We'll have a lot more test like these
1 parent b09f62a commit 35a770e

File tree

10 files changed

+286
-393
lines changed

10 files changed

+286
-393
lines changed

AGENTS.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,28 @@ If you don’t have the tool:
7373
### Test assertions
7474

7575
- Tests should use pretty_assertions::assert_eq for clearer diffs. Import this at the top of the test module if it isn't already.
76+
77+
### Integration tests (core)
78+
79+
- Prefer the utilities in `core_test_support::responses` when writing end-to-end Codex tests.
80+
81+
- All `mount_sse*` helpers return a `ResponseMock`; hold onto it so you can assert against outbound `/responses` POST bodies.
82+
- Use `ResponseMock::single_request()` when a test should only issue one POST, or `ResponseMock::requests()` to inspect every captured `ResponsesRequest`.
83+
- `ResponsesRequest` exposes helpers (`body_json`, `input`, `function_call_output`, `custom_tool_call_output`, `call_output`, `header`, `path`, `query_param`) so assertions can target structured payloads instead of manual JSON digging.
84+
- Build SSE payloads with the provided `ev_*` constructors and the `sse(...)`.
85+
86+
- Typical pattern:
87+
88+
```rust
89+
let mock = responses::mount_sse_once(&server, responses::sse(vec![
90+
responses::ev_response_created("resp-1"),
91+
responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?),
92+
responses::ev_completed("resp-1"),
93+
])).await;
94+
95+
codex.submit(Op::UserTurn { ... }).await?;
96+
97+
// Assert request body if needed.
98+
let request = mock.single_request();
99+
// assert using request.function_call_output(call_id) or request.json_body() or other helpers.
100+
```

codex-rs/core/tests/common/responses.rs

Lines changed: 121 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,105 @@
1+
use std::sync::Arc;
2+
use std::sync::Mutex;
3+
14
use serde_json::Value;
25
use wiremock::BodyPrintLimit;
6+
use wiremock::Match;
37
use wiremock::Mock;
8+
use wiremock::MockBuilder;
49
use wiremock::MockServer;
510
use wiremock::Respond;
611
use wiremock::ResponseTemplate;
712
use wiremock::matchers::method;
8-
use wiremock::matchers::path;
13+
use wiremock::matchers::path_regex;
14+
15+
#[derive(Debug, Clone)]
16+
pub struct ResponseMock {
17+
requests: Arc<Mutex<Vec<ResponsesRequest>>>,
18+
}
19+
20+
impl ResponseMock {
21+
fn new() -> Self {
22+
Self {
23+
requests: Arc::new(Mutex::new(Vec::new())),
24+
}
25+
}
26+
27+
pub fn single_request(&self) -> ResponsesRequest {
28+
let requests = self.requests.lock().unwrap();
29+
if requests.len() != 1 {
30+
panic!("expected 1 request, got {}", requests.len());
31+
}
32+
requests.first().unwrap().clone()
33+
}
34+
35+
pub fn requests(&self) -> Vec<ResponsesRequest> {
36+
self.requests.lock().unwrap().clone()
37+
}
38+
}
39+
40+
#[derive(Debug, Clone)]
41+
pub struct ResponsesRequest(wiremock::Request);
42+
43+
impl ResponsesRequest {
44+
pub fn body_json(&self) -> Value {
45+
self.0.body_json().unwrap()
46+
}
47+
48+
pub fn input(&self) -> Vec<Value> {
49+
self.0.body_json::<Value>().unwrap()["input"]
50+
.as_array()
51+
.expect("input array not found in request")
52+
.clone()
53+
}
54+
55+
pub fn function_call_output(&self, call_id: &str) -> Value {
56+
self.call_output(call_id, "function_call_output")
57+
}
58+
59+
pub fn custom_tool_call_output(&self, call_id: &str) -> Value {
60+
self.call_output(call_id, "custom_tool_call_output")
61+
}
62+
63+
pub fn call_output(&self, call_id: &str, call_type: &str) -> Value {
64+
self.input()
65+
.iter()
66+
.find(|item| {
67+
item.get("type").unwrap() == call_type && item.get("call_id").unwrap() == call_id
68+
})
69+
.cloned()
70+
.unwrap_or_else(|| panic!("function call output {call_id} item not found in request"))
71+
}
72+
73+
pub fn header(&self, name: &str) -> Option<String> {
74+
self.0
75+
.headers
76+
.get(name)
77+
.and_then(|v| v.to_str().ok())
78+
.map(str::to_string)
79+
}
80+
81+
pub fn path(&self) -> String {
82+
self.0.url.path().to_string()
83+
}
84+
85+
pub fn query_param(&self, name: &str) -> Option<String> {
86+
self.0
87+
.url
88+
.query_pairs()
89+
.find(|(k, _)| k == name)
90+
.map(|(_, v)| v.to_string())
91+
}
92+
}
93+
94+
impl Match for ResponseMock {
95+
fn matches(&self, request: &wiremock::Request) -> bool {
96+
self.requests
97+
.lock()
98+
.unwrap()
99+
.push(ResponsesRequest(request.clone()));
100+
true
101+
}
102+
}
9103

10104
/// Build an SSE stream body from a list of JSON events.
11105
pub fn sse(events: Vec<Value>) -> String {
@@ -161,34 +255,40 @@ pub fn sse_response(body: String) -> ResponseTemplate {
161255
.set_body_raw(body, "text/event-stream")
162256
}
163257

164-
pub async fn mount_sse_once_match<M>(server: &MockServer, matcher: M, body: String)
258+
fn base_mock() -> (MockBuilder, ResponseMock) {
259+
let response_mock = ResponseMock::new();
260+
let mock = Mock::given(method("POST"))
261+
.and(path_regex(".*/responses$"))
262+
.and(response_mock.clone());
263+
(mock, response_mock)
264+
}
265+
266+
pub async fn mount_sse_once_match<M>(server: &MockServer, matcher: M, body: String) -> ResponseMock
165267
where
166268
M: wiremock::Match + Send + Sync + 'static,
167269
{
168-
Mock::given(method("POST"))
169-
.and(path("/v1/responses"))
170-
.and(matcher)
270+
let (mock, response_mock) = base_mock();
271+
mock.and(matcher)
171272
.respond_with(sse_response(body))
172273
.up_to_n_times(1)
173274
.mount(server)
174275
.await;
276+
response_mock
175277
}
176278

177-
pub async fn mount_sse_once(server: &MockServer, body: String) {
178-
Mock::given(method("POST"))
179-
.and(path("/v1/responses"))
180-
.respond_with(sse_response(body))
181-
.expect(1)
279+
pub async fn mount_sse_once(server: &MockServer, body: String) -> ResponseMock {
280+
let (mock, response_mock) = base_mock();
281+
mock.respond_with(sse_response(body))
282+
.up_to_n_times(1)
182283
.mount(server)
183284
.await;
285+
response_mock
184286
}
185287

186-
pub async fn mount_sse(server: &MockServer, body: String) {
187-
Mock::given(method("POST"))
188-
.and(path("/v1/responses"))
189-
.respond_with(sse_response(body))
190-
.mount(server)
191-
.await;
288+
pub async fn mount_sse(server: &MockServer, body: String) -> ResponseMock {
289+
let (mock, response_mock) = base_mock();
290+
mock.respond_with(sse_response(body)).mount(server).await;
291+
response_mock
192292
}
193293

194294
pub async fn start_mock_server() -> MockServer {
@@ -201,7 +301,7 @@ pub async fn start_mock_server() -> MockServer {
201301
/// Mounts a sequence of SSE response bodies and serves them in order for each
202302
/// POST to `/v1/responses`. Panics if more requests are received than bodies
203303
/// provided. Also asserts the exact number of expected calls.
204-
pub async fn mount_sse_sequence(server: &MockServer, bodies: Vec<String>) {
304+
pub async fn mount_sse_sequence(server: &MockServer, bodies: Vec<String>) -> ResponseMock {
205305
use std::sync::atomic::AtomicUsize;
206306
use std::sync::atomic::Ordering;
207307

@@ -228,10 +328,11 @@ pub async fn mount_sse_sequence(server: &MockServer, bodies: Vec<String>) {
228328
responses: bodies,
229329
};
230330

231-
Mock::given(method("POST"))
232-
.and(path("/v1/responses"))
233-
.respond_with(responder)
331+
let (mock, response_mock) = base_mock();
332+
mock.respond_with(responder)
234333
.expect(num_calls as u64)
235334
.mount(server)
236335
.await;
336+
337+
response_mock
237338
}

codex-rs/core/tests/suite/cli_stream.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,12 @@ async fn exec_cli_applies_experimental_instructions_file() {
106106
"data: {\"type\":\"response.created\",\"response\":{}}\n\n",
107107
"data: {\"type\":\"response.completed\",\"response\":{\"id\":\"r1\"}}\n\n"
108108
);
109-
Mock::given(method("POST"))
110-
.and(path("/v1/responses"))
111-
.respond_with(
112-
ResponseTemplate::new(200)
113-
.insert_header("content-type", "text/event-stream")
114-
.set_body_raw(sse, "text/event-stream"),
115-
)
116-
.expect(1)
117-
.mount(&server)
118-
.await;
109+
let resp_mock = core_test_support::responses::mount_sse_once_match(
110+
&server,
111+
path("/v1/responses"),
112+
sse.to_string(),
113+
)
114+
.await;
119115

120116
// Create a temporary instructions file with a unique marker we can assert
121117
// appears in the outbound request payload.
@@ -164,8 +160,8 @@ async fn exec_cli_applies_experimental_instructions_file() {
164160

165161
// Inspect the captured request and verify our custom base instructions were
166162
// included in the `instructions` field.
167-
let request = &server.received_requests().await.unwrap()[0];
168-
let body = request.body_json::<serde_json::Value>().unwrap();
163+
let request = resp_mock.single_request();
164+
let body = request.body_json();
169165
let instructions = body
170166
.get("instructions")
171167
.and_then(|v| v.as_str())

codex-rs/core/tests/suite/client.rs

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,9 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
223223

224224
// Mock server that will receive the resumed request
225225
let server = MockServer::start().await;
226-
let first = ResponseTemplate::new(200)
227-
.insert_header("content-type", "text/event-stream")
228-
.set_body_raw(sse_completed("resp1"), "text/event-stream");
229-
Mock::given(method("POST"))
230-
.and(path("/v1/responses"))
231-
.respond_with(first)
232-
.expect(1)
233-
.mount(&server)
234-
.await;
226+
let resp_mock =
227+
responses::mount_sse_once_match(&server, path("/v1/responses"), sse_completed("resp1"))
228+
.await;
235229

236230
// Configure Codex to resume from our file
237231
let model_provider = ModelProviderInfo {
@@ -277,8 +271,8 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
277271
.unwrap();
278272
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
279273

280-
let request = &server.received_requests().await.unwrap()[0];
281-
let request_body = request.body_json::<serde_json::Value>().unwrap();
274+
let request = resp_mock.single_request();
275+
let request_body = request.body_json();
282276
let expected_input = json!([
283277
{
284278
"type": "message",
@@ -372,18 +366,9 @@ async fn includes_base_instructions_override_in_request() {
372366
skip_if_no_network!();
373367
// Mock server
374368
let server = MockServer::start().await;
375-
376-
// First request – must NOT include `previous_response_id`.
377-
let first = ResponseTemplate::new(200)
378-
.insert_header("content-type", "text/event-stream")
379-
.set_body_raw(sse_completed("resp1"), "text/event-stream");
380-
381-
Mock::given(method("POST"))
382-
.and(path("/v1/responses"))
383-
.respond_with(first)
384-
.expect(1)
385-
.mount(&server)
386-
.await;
369+
let resp_mock =
370+
responses::mount_sse_once_match(&server, path("/v1/responses"), sse_completed("resp1"))
371+
.await;
387372

388373
let model_provider = ModelProviderInfo {
389374
base_url: Some(format!("{}/v1", server.uri())),
@@ -414,8 +399,8 @@ async fn includes_base_instructions_override_in_request() {
414399

415400
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
416401

417-
let request = &server.received_requests().await.unwrap()[0];
418-
let request_body = request.body_json::<serde_json::Value>().unwrap();
402+
let request = resp_mock.single_request();
403+
let request_body = request.body_json();
419404

420405
assert!(
421406
request_body["instructions"]
@@ -570,16 +555,9 @@ async fn includes_user_instructions_message_in_request() {
570555
skip_if_no_network!();
571556
let server = MockServer::start().await;
572557

573-
let first = ResponseTemplate::new(200)
574-
.insert_header("content-type", "text/event-stream")
575-
.set_body_raw(sse_completed("resp1"), "text/event-stream");
576-
577-
Mock::given(method("POST"))
578-
.and(path("/v1/responses"))
579-
.respond_with(first)
580-
.expect(1)
581-
.mount(&server)
582-
.await;
558+
let resp_mock =
559+
responses::mount_sse_once_match(&server, path("/v1/responses"), sse_completed("resp1"))
560+
.await;
583561

584562
let model_provider = ModelProviderInfo {
585563
base_url: Some(format!("{}/v1", server.uri())),
@@ -610,8 +588,8 @@ async fn includes_user_instructions_message_in_request() {
610588

611589
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
612590

613-
let request = &server.received_requests().await.unwrap()[0];
614-
let request_body = request.body_json::<serde_json::Value>().unwrap();
591+
let request = resp_mock.single_request();
592+
let request_body = request.body_json();
615593

616594
assert!(
617595
!request_body["instructions"]

codex-rs/core/tests/suite/model_tools.rs

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,11 @@ use codex_core::protocol::InputItem;
1010
use codex_core::protocol::Op;
1111
use core_test_support::load_default_config_for_test;
1212
use core_test_support::load_sse_fixture_with_id;
13+
use core_test_support::responses;
1314
use core_test_support::skip_if_no_network;
1415
use core_test_support::wait_for_event;
1516
use tempfile::TempDir;
16-
use wiremock::Mock;
1717
use wiremock::MockServer;
18-
use wiremock::ResponseTemplate;
19-
use wiremock::matchers::method;
20-
use wiremock::matchers::path;
2118

2219
fn sse_completed(id: &str) -> String {
2320
load_sse_fixture_with_id("tests/fixtures/completed_template.json", id)
@@ -44,16 +41,7 @@ async fn collect_tool_identifiers_for_model(model: &str) -> Vec<String> {
4441
let server = MockServer::start().await;
4542

4643
let sse = sse_completed(model);
47-
let template = ResponseTemplate::new(200)
48-
.insert_header("content-type", "text/event-stream")
49-
.set_body_raw(sse, "text/event-stream");
50-
51-
Mock::given(method("POST"))
52-
.and(path("/v1/responses"))
53-
.respond_with(template)
54-
.expect(1)
55-
.mount(&server)
56-
.await;
44+
let resp_mock = responses::mount_sse_once_match(&server, wiremock::matchers::any(), sse).await;
5745

5846
let model_provider = ModelProviderInfo {
5947
base_url: Some(format!("{}/v1", server.uri())),
@@ -93,13 +81,7 @@ async fn collect_tool_identifiers_for_model(model: &str) -> Vec<String> {
9381
.unwrap();
9482
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
9583

96-
let requests = server.received_requests().await.unwrap();
97-
assert_eq!(
98-
requests.len(),
99-
1,
100-
"expected a single request for model {model}"
101-
);
102-
let body = requests[0].body_json::<serde_json::Value>().unwrap();
84+
let body = resp_mock.single_request().body_json();
10385
tool_identifiers(&body)
10486
}
10587

0 commit comments

Comments
 (0)