valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: use anyhow::Result for all app-server integration tests (#5836)

Browse Source 
There's a lot of visual noise in app-server's integration tests due to
the number of `.expect("<some_msg>")` lines which are largely redundant
/ not very useful. Clean them up by using `anyhow::Result` + `?`
consistently.

Replaces the existing pattern of:
```
    let codex_home = TempDir::new().expect("create temp dir");
    create_config_toml(codex_home.path()).expect("write config.toml");

    let mut mcp = McpProcess::new(codex_home.path())
        .await
        .expect("spawn mcp process");
    timeout(DEFAULT_READ_TIMEOUT, mcp.initialize())
        .await
        .expect("initialize timeout")
        .expect("initialize request");
```

With:
```
    let codex_home = TempDir::new()?;
    create_config_toml(codex_home.path())?;

    let mut mcp = McpProcess::new(codex_home.path()).await?;
    timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
```
This commit is contained in:
Owen Lin
2025-10-28 08:10:23 -07:00
committed by GitHub
parent be4bdfec93
commit 266419217e
13 changed files with 414 additions and 765 deletions
Download Patch File Download Diff File Expand all files Collapse all files

101
codex-rs/app-server/tests/suite/list_resume.rs
View File

@@ -1,6 +1,4 @@
use std::fs;
use std::path::Path;
use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::to_response;
use codex_app_server_protocol::JSONRPCNotification;
@@ -15,6 +13,8 @@ use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::SessionConfiguredNotification;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::fs;
use std::path::Path;
use tempfile::TempDir;
use tokio::time::timeout;
use uuid::Uuid;
@@ -22,38 +22,33 @@ use uuid::Uuid;
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_list_and_resume_conversations() {
async fn test_list_and_resume_conversations() -> Result<()> {
// Prepare a temporary CODEX_HOME with a few fake rollout files.
let codex_home = TempDir::new().expect("create temp dir");
let codex_home = TempDir::new()?;
create_fake_rollout(
codex_home.path(),
"2025-01-02T12-00-00",
"2025-01-02T12:00:00Z",
"Hello A",
Some("openai"),
);
)?;
create_fake_rollout(
codex_home.path(),
"2025-01-01T13-00-00",
"2025-01-01T13:00:00Z",
"Hello B",
Some("openai"),
);
)?;
create_fake_rollout(
codex_home.path(),
"2025-01-01T12-00-00",
"2025-01-01T12:00:00Z",
"Hello C",
None,
);
)?;
let mut mcp = McpProcess::new(codex_home.path())
.await
.expect("spawn mcp process");
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize())
.await
.expect("init timeout")
.expect("init failed");
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
// Request first page with size 2
let req_id = mcp
@@ -62,17 +57,14 @@ async fn test_list_and_resume_conversations() {
cursor: None,
model_providers: None,
})
.await
.expect("send listConversations");
.await?;
let resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(req_id)),
)
.await
.expect("listConversations timeout")
.expect("listConversations resp");
.await??;
let ListConversationsResponse { items, next_cursor } =
to_response::<ListConversationsResponse>(resp).expect("deserialize response");
to_response::<ListConversationsResponse>(resp)?;
assert_eq!(items.len(), 2);
// Newest first; preview text should match
@@ -90,20 +82,17 @@ async fn test_list_and_resume_conversations() {
cursor: next_cursor,
model_providers: None,
})
.await
.expect("send listConversations page 2");
.await?;
let resp2: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(req_id2)),
)
.await
.expect("listConversations page 2 timeout")
.expect("listConversations page 2 resp");
.await??;
let ListConversationsResponse {
items: items2,
next_cursor: next2,
..
} = to_response::<ListConversationsResponse>(resp2).expect("deserialize response");
} = to_response::<ListConversationsResponse>(resp2)?;
assert_eq!(items2.len(), 1);
assert_eq!(items2[0].preview, "Hello C");
assert_eq!(items2[0].model_provider, "openai");
@@ -116,7 +105,7 @@ async fn test_list_and_resume_conversations() {
"2025-01-01T11:30:00Z",
"Hello TP",
Some("test-provider"),
);
)?;
// Filtering by model provider should return only matching sessions.
let filter_req_id = mcp
@@ -125,19 +114,16 @@ async fn test_list_and_resume_conversations() {
cursor: None,
model_providers: Some(vec!["test-provider".to_string()]),
})
.await
.expect("send listConversations filtered");
.await?;
let filter_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(filter_req_id)),
)
.await
.expect("listConversations filtered timeout")
.expect("listConversations filtered resp");
.await??;
let ListConversationsResponse {
items: filtered_items,
next_cursor: filtered_next,
} = to_response::<ListConversationsResponse>(filter_resp).expect("deserialize filtered");
} = to_response::<ListConversationsResponse>(filter_resp)?;
assert_eq!(filtered_items.len(), 1);
assert_eq!(filtered_next, None);
assert_eq!(filtered_items[0].preview, "Hello TP");
@@ -150,20 +136,16 @@ async fn test_list_and_resume_conversations() {
cursor: None,
model_providers: Some(Vec::new()),
})
.await
.expect("send listConversations unfiltered");
.await?;
let unfiltered_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(unfiltered_req_id)),
)
.await
.expect("listConversations unfiltered timeout")
.expect("listConversations unfiltered resp");
.await??;
let ListConversationsResponse {
items: unfiltered_items,
next_cursor: unfiltered_next,
} = to_response::<ListConversationsResponse>(unfiltered_resp)
.expect("deserialize unfiltered response");
} = to_response::<ListConversationsResponse>(unfiltered_resp)?;
assert_eq!(unfiltered_items.len(), 4);
assert!(unfiltered_next.is_none());
@@ -173,19 +155,16 @@ async fn test_list_and_resume_conversations() {
cursor: None,
model_providers: Some(vec!["other".to_string()]),
})
.await
.expect("send listConversations filtered empty");
.await?;
let empty_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(empty_req_id)),
)
.await
.expect("listConversations filtered empty timeout")
.expect("listConversations filtered empty resp");
.await??;
let ListConversationsResponse {
items: empty_items,
next_cursor: empty_next,
} = to_response::<ListConversationsResponse>(empty_resp).expect("deserialize filtered empty");
} = to_response::<ListConversationsResponse>(empty_resp)?;
assert!(empty_items.is_empty());
assert!(empty_next.is_none());
@@ -198,20 +177,15 @@ async fn test_list_and_resume_conversations() {
..Default::default()
}),
})
.await
.expect("send resumeConversation");
.await?;
// Expect a codex/event notification with msg.type == sessionConfigured
let notification: JSONRPCNotification = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("sessionConfigured"),
)
.await
.expect("sessionConfigured notification timeout")
.expect("sessionConfigured notification");
let session_configured: ServerNotification = notification
.try_into()
.expect("deserialize sessionConfigured notification");
.await??;
let session_configured: ServerNotification = notification.try_into()?;
// Basic shape assertion: ensure event type is sessionConfigured
let ServerNotification::SessionConfigured(SessionConfiguredNotification {
model,
@@ -229,15 +203,14 @@ async fn test_list_and_resume_conversations() {
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(resume_req_id)),
)
.await
.expect("resumeConversation timeout")
.expect("resumeConversation resp");
.await??;
let ResumeConversationResponse {
conversation_id, ..
} = to_response::<ResumeConversationResponse>(resume_resp)
.expect("deserialize resumeConversation response");
} = to_response::<ResumeConversationResponse>(resume_resp)?;
// conversation id should be a valid UUID
assert!(!conversation_id.to_string().is_empty());
Ok(())
}
fn create_fake_rollout(
@@ -246,14 +219,14 @@ fn create_fake_rollout(
meta_rfc3339: &str,
preview: &str,
model_provider: Option<&str>,
) {
) -> Result<()> {
let uuid = Uuid::new_v4();
// sessions/YYYY/MM/DD/ derived from filename_ts (YYYY-MM-DDThh-mm-ss)
let year = &filename_ts[0..4];
let month = &filename_ts[5..7];
let day = &filename_ts[8..10];
let dir = codex_home.join("sessions").join(year).join(month).join(day);
fs::create_dir_all(&dir).unwrap_or_else(|e| panic!("create sessions dir: {e}"));
fs::create_dir_all(&dir)?;
let file_path = dir.join(format!("rollout-{filename_ts}-{uuid}.jsonl"));
let mut lines = Vec::new();
@@ -303,6 +276,6 @@ fn create_fake_rollout(
})
.to_string(),
);
fs::write(file_path, lines.join("\n") + "\n")
.unwrap_or_else(|e| panic!("write rollout file: {e}"));
fs::write(file_path, lines.join("\n") + "\n")?;
Ok(())
}