From a7d5a99cafb72d0a45e24093b9d6a95a80a65a11 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 29 Apr 2026 01:09:12 -0700 Subject: [PATCH] codex: remove large-stack TUI test harness --- codex-rs/tui/src/app/tests.rs | 474 +++++++++++++++------------------- codex-rs/tui/src/lib.rs | 151 +++++------ 2 files changed, 273 insertions(+), 352 deletions(-) diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 305644d636..1b549f9576 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -99,31 +99,6 @@ macro_rules! assert_app_snapshot { }; } -fn run_large_stack_test(body: impl FnOnce() -> T + Send + 'static) -> T -where - T: Send + 'static, -{ - std::thread::Builder::new() - .name("codex-tui-app-test".to_string()) - .stack_size(8 * 1024 * 1024) - .spawn(body) - .expect("spawn large-stack test thread") - .join() - .expect("large-stack test thread panicked") -} - -macro_rules! run_large_stack_async_test { - ($body:block) => { - run_large_stack_test(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("test runtime") - .block_on(async $body) - }) - }; -} - fn test_absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::try_from(PathBuf::from(path)).expect("absolute test path") } @@ -1243,217 +1218,198 @@ async fn token_usage_update_refreshes_status_line_with_runtime_context_window() ); } -#[test] -fn open_agent_picker_keeps_missing_threads_for_replay() -> Result<()> { - run_large_stack_async_test!({ - let mut app = make_test_app().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let thread_id = ThreadId::new(); - app.thread_event_channels - .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); +#[tokio::test] +async fn open_agent_picker_keeps_missing_threads_for_replay() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let thread_id = ThreadId::new(); + app.thread_event_channels + .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); - app.open_agent_picker(&mut app_server).await; + app.open_agent_picker(&mut app_server).await; - assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); - assert_eq!( - app.agent_navigation.get(&thread_id), - Some(&AgentPickerThreadEntry { - agent_nickname: None, - agent_role: None, - is_closed: true, - }) - ); - assert_eq!(app.agent_navigation.ordered_thread_ids(), vec![thread_id]); - Ok(()) - }) + assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); + assert_eq!( + app.agent_navigation.get(&thread_id), + Some(&AgentPickerThreadEntry { + agent_nickname: None, + agent_role: None, + is_closed: true, + }) + ); + assert_eq!(app.agent_navigation.ordered_thread_ids(), vec![thread_id]); + Ok(()) } -#[test] -fn open_agent_picker_preserves_cached_metadata_for_replay_threads() -> Result<()> { - run_large_stack_async_test!({ - let mut app = make_test_app().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let thread_id = ThreadId::new(); - app.thread_event_channels - .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); - app.agent_navigation.upsert( - thread_id, - Some("Robie".to_string()), - Some("explorer".to_string()), - /*is_closed*/ true, - ); +#[tokio::test] +async fn open_agent_picker_preserves_cached_metadata_for_replay_threads() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let thread_id = ThreadId::new(); + app.thread_event_channels + .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); + app.agent_navigation.upsert( + thread_id, + Some("Robie".to_string()), + Some("explorer".to_string()), + /*is_closed*/ true, + ); - app.open_agent_picker(&mut app_server).await; + app.open_agent_picker(&mut app_server).await; - assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); - assert_eq!( - app.agent_navigation.get(&thread_id), - Some(&AgentPickerThreadEntry { - agent_nickname: Some("Robie".to_string()), - agent_role: Some("explorer".to_string()), - is_closed: true, - }) - ); - Ok(()) - }) + assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); + assert_eq!( + app.agent_navigation.get(&thread_id), + Some(&AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: true, + }) + ); + Ok(()) } -#[test] -fn open_agent_picker_prunes_terminal_metadata_only_threads() -> Result<()> { - run_large_stack_async_test!({ - let mut app = make_test_app().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let thread_id = ThreadId::new(); - app.agent_navigation.upsert( - thread_id, - Some("Ghost".to_string()), - Some("worker".to_string()), - /*is_closed*/ false, - ); +#[tokio::test] +async fn open_agent_picker_prunes_terminal_metadata_only_threads() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let thread_id = ThreadId::new(); + app.agent_navigation.upsert( + thread_id, + Some("Ghost".to_string()), + Some("worker".to_string()), + /*is_closed*/ false, + ); - app.open_agent_picker(&mut app_server).await; + app.open_agent_picker(&mut app_server).await; - assert_eq!(app.agent_navigation.get(&thread_id), None); - assert!(app.agent_navigation.is_empty()); - Ok(()) - }) + assert_eq!(app.agent_navigation.get(&thread_id), None); + assert!(app.agent_navigation.is_empty()); + Ok(()) } -#[test] -fn open_agent_picker_marks_terminal_read_errors_closed() -> Result<()> { - run_large_stack_async_test!({ - let mut app = make_test_app().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let thread_id = ThreadId::new(); - app.thread_event_channels - .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); - app.agent_navigation.upsert( - thread_id, - Some("Robie".to_string()), - Some("explorer".to_string()), - /*is_closed*/ false, - ); +#[tokio::test] +async fn open_agent_picker_marks_terminal_read_errors_closed() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let thread_id = ThreadId::new(); + app.thread_event_channels + .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); + app.agent_navigation.upsert( + thread_id, + Some("Robie".to_string()), + Some("explorer".to_string()), + /*is_closed*/ false, + ); - app.open_agent_picker(&mut app_server).await; + app.open_agent_picker(&mut app_server).await; - assert_eq!( - app.agent_navigation.get(&thread_id), - Some(&AgentPickerThreadEntry { - agent_nickname: Some("Robie".to_string()), - agent_role: Some("explorer".to_string()), - is_closed: true, - }) - ); - Ok(()) - }) + assert_eq!( + app.agent_navigation.get(&thread_id), + Some(&AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: true, + }) + ); + Ok(()) } -#[test] -fn open_agent_picker_marks_loaded_threads_open() -> Result<()> { - run_large_stack_async_test!({ - let mut app = make_test_app().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let started = app_server - .start_thread(app.chat_widget.config_ref()) - .await?; - let thread_id = started.session.thread_id; - app.thread_event_channels - .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); +#[tokio::test] +async fn open_agent_picker_marks_loaded_threads_open() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let started = app_server + .start_thread(app.chat_widget.config_ref()) + .await?; + let thread_id = started.session.thread_id; + app.thread_event_channels + .insert(thread_id, ThreadEventChannel::new(/*capacity*/ 1)); - app.open_agent_picker(&mut app_server).await; + app.open_agent_picker(&mut app_server).await; - assert_eq!( - app.agent_navigation.get(&thread_id), - Some(&AgentPickerThreadEntry { - agent_nickname: None, - agent_role: None, - is_closed: false, - }) - ); - Ok(()) - }) + assert_eq!( + app.agent_navigation.get(&thread_id), + Some(&AgentPickerThreadEntry { + agent_nickname: None, + agent_role: None, + is_closed: false, + }) + ); + Ok(()) } -#[test] -fn attach_live_thread_for_selection_rejects_empty_non_ephemeral_fallback_threads() -> Result<()> { - run_large_stack_async_test!({ - let config = { - let app = make_test_app().await; - app.chat_widget.config_ref().clone() - }; - let mut app_server = crate::start_embedded_app_server_for_picker(&config) - .await - .expect("embedded app server"); - let started = app_server.start_thread(&config).await?; - let thread_id = started.session.thread_id; - let mut app = make_test_app().await; - app.agent_navigation.upsert( - thread_id, - Some("Scout".to_string()), - Some("worker".to_string()), - /*is_closed*/ false, - ); +#[tokio::test] +async fn attach_live_thread_for_selection_rejects_empty_non_ephemeral_fallback_threads() +-> Result<()> { + let config = { + let app = make_test_app().await; + app.chat_widget.config_ref().clone() + }; + let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&config)) + .await + .expect("embedded app server"); + let started = app_server.start_thread(&config).await?; + let thread_id = started.session.thread_id; + let mut app = make_test_app().await; + app.agent_navigation.upsert( + thread_id, + Some("Scout".to_string()), + Some("worker".to_string()), + /*is_closed*/ false, + ); - let err = app - .attach_live_thread_for_selection(&mut app_server, thread_id) - .await - .expect_err("empty fallback should not attach as a blank replay-only thread"); + let err = Box::pin(app.attach_live_thread_for_selection(&mut app_server, thread_id)) + .await + .expect_err("empty fallback should not attach as a blank replay-only thread"); - assert_eq!( - err.to_string(), - format!("Agent thread {thread_id} is not yet available for replay or live attach.") - ); - assert!(!app.thread_event_channels.contains_key(&thread_id)); - Ok(()) - }) + assert_eq!( + err.to_string(), + format!("Agent thread {thread_id} is not yet available for replay or live attach.") + ); + assert!(!app.thread_event_channels.contains_key(&thread_id)); + Ok(()) } -#[test] -fn attach_live_thread_for_selection_rejects_unmaterialized_fallback_threads() -> Result<()> { - run_large_stack_async_test!({ - let mut app = make_test_app().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let mut ephemeral_config = app.chat_widget.config_ref().clone(); - ephemeral_config.ephemeral = true; - let started = app_server.start_thread(&ephemeral_config).await?; - let thread_id = started.session.thread_id; - app.agent_navigation.upsert( - thread_id, - Some("Scout".to_string()), - Some("worker".to_string()), - /*is_closed*/ false, - ); +#[tokio::test] +async fn attach_live_thread_for_selection_rejects_unmaterialized_fallback_threads() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker( + app.chat_widget.config_ref(), + )) + .await + .expect("embedded app server"); + let mut ephemeral_config = app.chat_widget.config_ref().clone(); + ephemeral_config.ephemeral = true; + let started = app_server.start_thread(&ephemeral_config).await?; + let thread_id = started.session.thread_id; + app.agent_navigation.upsert( + thread_id, + Some("Scout".to_string()), + Some("worker".to_string()), + /*is_closed*/ false, + ); - let err = app - .attach_live_thread_for_selection(&mut app_server, thread_id) - .await - .expect_err("ephemeral fallback should not attach as a blank live thread"); + let err = Box::pin(app.attach_live_thread_for_selection(&mut app_server, thread_id)) + .await + .expect_err("ephemeral fallback should not attach as a blank live thread"); - assert_eq!( - err.to_string(), - format!("Agent thread {thread_id} is not yet available for replay or live attach.") - ); - assert!(!app.thread_event_channels.contains_key(&thread_id)); - Ok(()) - }) + assert_eq!( + err.to_string(), + format!("Agent thread {thread_id} is not yet available for replay or live attach.") + ); + assert!(!app.thread_event_channels.contains_key(&thread_id)); + Ok(()) } #[tokio::test] @@ -1482,64 +1438,58 @@ async fn should_attach_live_thread_for_selection_skips_closed_metadata_only_thre assert!(!app.should_attach_live_thread_for_selection(thread_id)); } -#[test] -fn refresh_agent_picker_thread_liveness_prunes_closed_metadata_only_threads() -> Result<()> { - run_large_stack_async_test!({ - let mut app = make_test_app().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let thread_id = ThreadId::new(); - app.agent_navigation.upsert( - thread_id, - Some("Ghost".to_string()), - Some("worker".to_string()), - /*is_closed*/ false, - ); +#[tokio::test] +async fn refresh_agent_picker_thread_liveness_prunes_closed_metadata_only_threads() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let thread_id = ThreadId::new(); + app.agent_navigation.upsert( + thread_id, + Some("Ghost".to_string()), + Some("worker".to_string()), + /*is_closed*/ false, + ); - let is_available = app - .refresh_agent_picker_thread_liveness(&mut app_server, thread_id) - .await; + let is_available = app + .refresh_agent_picker_thread_liveness(&mut app_server, thread_id) + .await; - assert!(!is_available); - assert_eq!(app.agent_navigation.get(&thread_id), None); - assert!(!app.thread_event_channels.contains_key(&thread_id)); - Ok(()) - }) + assert!(!is_available); + assert_eq!(app.agent_navigation.get(&thread_id), None); + assert!(!app.thread_event_channels.contains_key(&thread_id)); + Ok(()) } -#[test] -fn open_agent_picker_prompts_to_enable_multi_agent_when_disabled() -> Result<()> { - run_large_stack_async_test!({ - let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; - let mut app_server = - crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) - .await - .expect("embedded app server"); - let _ = app.config.features.disable(Feature::Collab); +#[tokio::test] +async fn open_agent_picker_prompts_to_enable_multi_agent_when_disabled() -> Result<()> { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let _ = app.config.features.disable(Feature::Collab); - app.open_agent_picker(&mut app_server).await; - app.chat_widget - .handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + app.open_agent_picker(&mut app_server).await; + app.chat_widget + .handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - assert_matches!( - app_event_rx.try_recv(), - Ok(AppEvent::UpdateFeatureFlags { updates }) if updates == vec![(Feature::Collab, true)] - ); - let cell = match app_event_rx.try_recv() { - Ok(AppEvent::InsertHistoryCell(cell)) => cell, - other => panic!("expected InsertHistoryCell event, got {other:?}"), - }; - let rendered = cell - .display_lines(/*width*/ 120) - .into_iter() - .map(|line| line.to_string()) - .collect::>() - .join("\n"); - assert!(rendered.contains("Subagents will be enabled in the next session.")); - Ok(()) - }) + assert_matches!( + app_event_rx.try_recv(), + Ok(AppEvent::UpdateFeatureFlags { updates }) if updates == vec![(Feature::Collab, true)] + ); + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected InsertHistoryCell event, got {other:?}"), + }; + let rendered = cell + .display_lines(/*width*/ 120) + .into_iter() + .map(|line| line.to_string()) + .collect::>() + .join("\n"); + assert!(rendered.contains("Subagents will be enabled in the next session.")); + Ok(()) } #[tokio::test] diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 2037d152ca..7a0f7cfa96 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -59,11 +59,8 @@ use codex_utils_oss::get_default_model_for_oss_provider; use color_eyre::eyre::WrapErr; use cwd_prompt::CwdPromptAction; use std::fs::OpenOptions; -use std::future::Future; use std::path::Path; use std::path::PathBuf; -#[cfg(test)] -use std::pin::Pin; use std::sync::Arc; pub use token_usage::FinalOutput; pub use token_usage::TokenUsage; @@ -445,17 +442,15 @@ pub(crate) async fn start_app_server_for_picker( } #[cfg(test)] -pub(crate) fn start_embedded_app_server_for_picker( +pub(crate) async fn start_embedded_app_server_for_picker( config: &Config, -) -> Pin> + '_>> { - Box::pin(async move { - start_app_server_for_picker( - config, - &AppServerTarget::Embedded, - Arc::new(EnvironmentManager::default_for_tests()), - ) - .await - }) +) -> color_eyre::Result { + start_app_server_for_picker( + config, + &AppServerTarget::Embedded, + Arc::new(EnvironmentManager::default_for_tests()), + ) + .await } #[allow(clippy::too_many_arguments)] @@ -1668,19 +1663,6 @@ mod tests { use serial_test::serial; use tempfile::TempDir; - fn run_large_stack_test(body: impl FnOnce() -> T + Send + 'static) -> T - where - T: Send + 'static, - { - std::thread::Builder::new() - .name("codex-tui-lib-test".to_string()) - .stack_size(8 * 1024 * 1024) - .spawn(body) - .expect("spawn large-stack test thread") - .join() - .expect("large-stack test thread panicked") - } - async fn build_config(temp_dir: &TempDir) -> std::io::Result { ConfigBuilder::default() .codex_home(temp_dir.path().to_path_buf()) @@ -1970,74 +1952,63 @@ mod tests { Ok(()) } - #[test] - fn lookup_session_target_by_name_uses_backend_title_search() -> color_eyre::Result<()> { - run_large_stack_test(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("test runtime") - .block_on(async { - let temp_dir = TempDir::new()?; - let config = build_config(&temp_dir).await?; - let thread_id = ThreadId::new(); - let rollout_path = temp_dir - .path() - .join("sessions/2025/02/01") - .join(format!("rollout-2025-02-01T10-00-00-{thread_id}.jsonl")); - let rollout_dir = rollout_path.parent().expect("rollout parent"); - std::fs::create_dir_all(rollout_dir)?; - std::fs::write(&rollout_path, "")?; + #[tokio::test] + async fn lookup_session_target_by_name_uses_backend_title_search() -> color_eyre::Result<()> { + let temp_dir = TempDir::new()?; + let config = build_config(&temp_dir).await?; + let thread_id = ThreadId::new(); + let rollout_path = temp_dir + .path() + .join("sessions/2025/02/01") + .join(format!("rollout-2025-02-01T10-00-00-{thread_id}.jsonl")); + let rollout_dir = rollout_path.parent().expect("rollout parent"); + std::fs::create_dir_all(rollout_dir)?; + std::fs::write(&rollout_path, "")?; - let state_runtime = codex_state::StateRuntime::init( - config.codex_home.to_path_buf(), - config.model_provider_id.clone(), - ) - .await - .map_err(std::io::Error::other)?; - state_runtime - .mark_backfill_complete(/*last_watermark*/ None) - .await - .map_err(std::io::Error::other)?; + let state_runtime = codex_state::StateRuntime::init( + config.codex_home.to_path_buf(), + config.model_provider_id.clone(), + ) + .await + .map_err(std::io::Error::other)?; + state_runtime + .mark_backfill_complete(/*last_watermark*/ None) + .await + .map_err(std::io::Error::other)?; - let session_cwd = temp_dir.path().join("project"); - std::fs::create_dir_all(&session_cwd)?; - let created_at = chrono::DateTime::parse_from_rfc3339("2025-02-01T10:00:00Z") - .expect("timestamp should parse") - .with_timezone(&chrono::Utc); - let mut builder = codex_state::ThreadMetadataBuilder::new( - thread_id, - rollout_path.clone(), - created_at, - serde_json::from_value(serde_json::json!("cli")) - .expect("cli session source should deserialize"), - ); - builder.cwd = session_cwd; - let mut metadata = builder.build(config.model_provider_id.as_str()); - metadata.title = "saved-session".to_string(); - metadata.first_user_message = Some("preview text".to_string()); - state_runtime - .upsert_thread(&metadata) - .await - .map_err(std::io::Error::other)?; + let session_cwd = temp_dir.path().join("project"); + std::fs::create_dir_all(&session_cwd)?; + let created_at = chrono::DateTime::parse_from_rfc3339("2025-02-01T10:00:00Z") + .expect("timestamp should parse") + .with_timezone(&chrono::Utc); + let mut builder = codex_state::ThreadMetadataBuilder::new( + thread_id, + rollout_path.clone(), + created_at, + serde_json::from_value(serde_json::json!("cli")) + .expect("cli session source should deserialize"), + ); + builder.cwd = session_cwd; + let mut metadata = builder.build(config.model_provider_id.as_str()); + metadata.title = "saved-session".to_string(); + metadata.first_user_message = Some("preview text".to_string()); + state_runtime + .upsert_thread(&metadata) + .await + .map_err(std::io::Error::other)?; - let mut app_server = - AppServerSession::new(codex_app_server_client::AppServerClient::InProcess( - start_test_embedded_app_server(config).await?, - )); - let target = lookup_session_target_by_name_with_app_server( - &mut app_server, - "saved-session", - ) - .await?; - let target = target.expect("name lookup should find the saved thread"); - assert_eq!(target.path, Some(rollout_path)); - assert_eq!(target.thread_id, thread_id); + let mut app_server = + AppServerSession::new(codex_app_server_client::AppServerClient::InProcess( + start_test_embedded_app_server(config).await?, + )); + let target = + lookup_session_target_by_name_with_app_server(&mut app_server, "saved-session").await?; + let target = target.expect("name lookup should find the saved thread"); + assert_eq!(target.path, Some(rollout_path)); + assert_eq!(target.thread_id, thread_id); - app_server.shutdown().await?; - Ok(()) - }) - }) + app_server.shutdown().await?; + Ok(()) } #[tokio::test]