mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Reduce stack usage in async TUI tests (#52959)
## Why Large test futures can occupy their caller's stack frame even when passed directly to `Box::pin`. ## What changed - Add a non-inlined `box_future` helper and `boxed_future!` macro to construct and box futures outside the caller's poll frame. Use them for heavyweight TUI fixtures and test scenarios. - Return the shared `App` fixture as `Box<App>` to keep it on the heap. - Treat `ConnectionAborted` as a closed connection in the realtime speech test server, alongside `BrokenPipe` and `ConnectionReset`. GitOrigin-RevId: 7bbf2020a5ec12bae8e0365ddf5641deef93b544
This commit is contained in:
@@ -952,7 +952,7 @@ fn track_in_flight_recap(app: &mut App, thread_id: ThreadId) -> (RecapRequest, T
|
||||
(request, temporary_thread_id)
|
||||
}
|
||||
|
||||
async fn app_with_visible_thread(thread_id: ThreadId) -> App {
|
||||
async fn app_with_visible_thread(thread_id: ThreadId) -> Box<App> {
|
||||
let mut app = make_test_app().await;
|
||||
app.active_thread_id = Some(thread_id);
|
||||
app
|
||||
|
||||
@@ -3,21 +3,24 @@
|
||||
//! This module keeps heavyweight `App` construction and config-inspection helpers available to
|
||||
//! focused sibling test modules without making `app/tests.rs` the only practical place to test
|
||||
//! app-owned behavior.
|
||||
//! Return the large fixture on the heap so async scenarios do not embed it in their stack frames.
|
||||
|
||||
use super::*;
|
||||
use crate::chatwidget::tests::make_chatwidget_manual_with_sender;
|
||||
use crate::test_support::boxed_future;
|
||||
use codex_models_manager::test_support::construct_model_info_offline_for_tests;
|
||||
use codex_models_manager::test_support::get_model_offline_for_tests;
|
||||
|
||||
pub(crate) async fn make_test_app() -> App {
|
||||
let (mut chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
pub(crate) async fn make_test_app() -> Box<App> {
|
||||
let (mut chat_widget, app_event_tx, _rx, _op_rx) =
|
||||
boxed_future!(make_chatwidget_manual_with_sender()).await;
|
||||
let test_codex_home = chat_widget.test_codex_home.take();
|
||||
let config = chat_widget.config_ref().clone();
|
||||
let file_search = FileSearchManager::new(config.cwd.to_path_buf(), app_event_tx.clone());
|
||||
let model = get_model_offline_for_tests(config.model.as_deref());
|
||||
let session_telemetry = test_session_telemetry(&config, model.as_str());
|
||||
|
||||
App {
|
||||
Box::new(App {
|
||||
feature_write_lock: Arc::default(),
|
||||
model_catalog: chat_widget.model_catalog(),
|
||||
session_telemetry,
|
||||
@@ -121,7 +124,7 @@ pub(crate) async fn make_test_app() -> App {
|
||||
pending_hook_enabled_writes: HashMap::new(),
|
||||
recap: recap::RecapState::default(),
|
||||
_test_codex_home: test_codex_home,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn test_session_telemetry(config: &Config, model: &str) -> SessionTelemetry {
|
||||
|
||||
@@ -114,6 +114,7 @@ use crate::app_backtrack::BacktrackState;
|
||||
use crate::app_backtrack::nth_user_position;
|
||||
use crate::app_backtrack::user_count;
|
||||
use crate::app_event::HistoryBatchEntryResponse;
|
||||
use crate::test_support::boxed_future;
|
||||
|
||||
async fn drain_managed_worktree_start(app: &mut App, server: &mut AppServerSession) {
|
||||
if let Some((mode, name)) = app.pending_start_managed_worktree.take() {
|
||||
@@ -1380,7 +1381,8 @@ async fn replay_thread_snapshot_restores_the_matching_safety_buffer_prompt() {
|
||||
store.snapshot()
|
||||
};
|
||||
|
||||
let (mut chat_widget, _app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
let (mut chat_widget, _app_event_tx, _rx, _op_rx) =
|
||||
boxed_future!(make_chatwidget_manual_with_sender()).await;
|
||||
chat_widget.handle_thread_session(test_thread_session(
|
||||
ThreadId::new(),
|
||||
test_path_buf("/tmp/other-project"),
|
||||
@@ -1639,7 +1641,8 @@ async fn replay_thread_snapshot_in_progress_turn_restores_running_state_without_
|
||||
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
let thread_id = ThreadId::new();
|
||||
let session = test_thread_session(thread_id, test_path_buf("/tmp/project"));
|
||||
let (chat_widget, _app_event_tx, _rx, _new_op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
let (chat_widget, _app_event_tx, _rx, _new_op_rx) =
|
||||
boxed_future!(make_chatwidget_manual_with_sender()).await;
|
||||
app.chat_widget = chat_widget;
|
||||
app.chat_widget.handle_thread_session(session);
|
||||
|
||||
@@ -1926,7 +1929,8 @@ async fn replay_thread_snapshot_restores_collaboration_mode_without_input() {
|
||||
.capture_thread_input_state()
|
||||
.expect("expected collaboration-only input state");
|
||||
|
||||
let (chat_widget, _app_event_tx, _rx, _new_op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
let (chat_widget, _app_event_tx, _rx, _new_op_rx) =
|
||||
boxed_future!(make_chatwidget_manual_with_sender()).await;
|
||||
app.chat_widget = chat_widget;
|
||||
app.chat_widget
|
||||
.set_plan_mode_reasoning_effort(Some(ReasoningEffortConfig::Low));
|
||||
@@ -6179,7 +6183,8 @@ async fn ctrl_l_clears_owned_history_and_preserves_the_draft() -> Result<()> {
|
||||
}
|
||||
|
||||
async fn make_test_app() -> Box<App> {
|
||||
let (mut chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
let (mut chat_widget, app_event_tx, _rx, _op_rx) =
|
||||
boxed_future!(make_chatwidget_manual_with_sender()).await;
|
||||
let test_codex_home = chat_widget.test_codex_home.take();
|
||||
let config = chat_widget.config_ref().clone();
|
||||
let file_search = FileSearchManager::new(config.cwd.to_path_buf(), app_event_tx.clone());
|
||||
@@ -6298,7 +6303,8 @@ pub(super) async fn make_test_app_with_channels() -> (
|
||||
tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
tokio::sync::mpsc::UnboundedReceiver<Op>,
|
||||
) {
|
||||
let (mut chat_widget, app_event_tx, rx, op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
let (mut chat_widget, app_event_tx, rx, op_rx) =
|
||||
boxed_future!(make_chatwidget_manual_with_sender()).await;
|
||||
let test_codex_home = chat_widget.test_codex_home.take();
|
||||
let config = chat_widget.config_ref().clone();
|
||||
let file_search = FileSearchManager::new(config.cwd.to_path_buf(), app_event_tx.clone());
|
||||
@@ -8404,6 +8410,11 @@ async fn remembered_current_cwd_stays_at_launch_across_in_app_resumes() -> Resul
|
||||
|
||||
#[tokio::test]
|
||||
async fn prompt_edit_reverts_earlier_and_first_visible_prompts_in_place() -> Result<()> {
|
||||
// Allocate the whole scenario before Tokio polls it on the test thread.
|
||||
boxed_future!(prompt_edit_reverts_earlier_and_first_visible_prompts_in_place_scenario()).await
|
||||
}
|
||||
|
||||
async fn prompt_edit_reverts_earlier_and_first_visible_prompts_in_place_scenario() -> Result<()> {
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::UserMessageItem;
|
||||
use codex_protocol::models::ImageReference;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::*;
|
||||
use crate::app_event::TranscriptExportDestination;
|
||||
use crate::bottom_pane::BottomPaneView;
|
||||
use crate::test_support::boxed_future;
|
||||
use app_test_support::create_fake_paginated_rollout;
|
||||
use app_test_support::create_fake_parented_rollout_with_source;
|
||||
use app_test_support::create_fake_rollout;
|
||||
@@ -506,7 +507,7 @@ pub(super) async fn start_recording_app_server_with_realtime_speech(
|
||||
tokio_tungstenite::tungstenite::error::ProtocolError::ResetWithoutClosingHandshake,
|
||||
)) => break,
|
||||
Err(tokio_tungstenite::tungstenite::Error::Io(error))
|
||||
if matches!(error.kind(), std::io::ErrorKind::BrokenPipe | std::io::ErrorKind::ConnectionReset) => break,
|
||||
if matches!(error.kind(), std::io::ErrorKind::BrokenPipe | std::io::ErrorKind::ConnectionReset | std::io::ErrorKind::ConnectionAborted) => break,
|
||||
_ => {}
|
||||
}
|
||||
let Message::Text(text) = frame? else {
|
||||
@@ -1338,6 +1339,12 @@ async fn archive_current_thread_returns_shared_servers_to_agents() -> Result<()>
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() -> Result<()> {
|
||||
boxed_future!(local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths_scenario())
|
||||
.await
|
||||
}
|
||||
|
||||
async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths_scenario()
|
||||
-> Result<()> {
|
||||
let (mut app, events, _ops) = Box::pin(make_test_app_with_channels()).await;
|
||||
// Invalid optional worktree settings must preserve both daemon start paths.
|
||||
app.config.features.enable(Feature::Worktrees)?;
|
||||
@@ -1822,8 +1829,8 @@ async fn embedded_server_rejects_unowned_dynamic_tool_calls() -> Result<()> {
|
||||
|
||||
#[tokio::test]
|
||||
async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespace() -> Result<()> {
|
||||
check_dynamic_tool_requests(/*rollout_enabled*/ true).await?;
|
||||
check_dynamic_tool_requests(/*rollout_enabled*/ false).await
|
||||
boxed_future!(check_dynamic_tool_requests(/*rollout_enabled*/ true)).await?;
|
||||
boxed_future!(check_dynamic_tool_requests(/*rollout_enabled*/ false)).await
|
||||
}
|
||||
|
||||
async fn check_dynamic_tool_requests(rollout_enabled: bool) -> Result<()> {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::test_support::boxed_future;
|
||||
use codex_app_server_protocol::ImageGenerationItem;
|
||||
use codex_app_server_protocol::PluginAvailability;
|
||||
use codex_utils_absolute_path::test_support::PathExt;
|
||||
@@ -375,7 +376,7 @@ pub(crate) async fn make_chatwidget_manual_with_sender() -> (
|
||||
tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
tokio::sync::mpsc::UnboundedReceiver<Op>,
|
||||
) {
|
||||
let (widget, rx, op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let (widget, rx, op_rx) = boxed_future!(make_chatwidget_manual(/*model_override*/ None)).await;
|
||||
let app_event_tx = AppEventSender::new(widget.app_event_tx.app_event_tx.clone());
|
||||
(widget, app_event_tx, rx, op_rx)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use crate::legacy_core::config::ConfigOverrides;
|
||||
use crate::test_support::boxed_future;
|
||||
use codex_config::LoaderOverrides;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
@@ -88,6 +89,11 @@ async fn start_server(
|
||||
#[tokio::test]
|
||||
async fn creation_attaches_to_original_task_and_survives_service_restart() -> color_eyre::Result<()>
|
||||
{
|
||||
boxed_future!(creation_attaches_to_original_task_and_survives_service_restart_scenario()).await
|
||||
}
|
||||
|
||||
async fn creation_attaches_to_original_task_and_survives_service_restart_scenario()
|
||||
-> color_eyre::Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
let responses =
|
||||
app_test_support::create_mock_responses_server_repeating_assistant("Ready").await;
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
//! Test-only helpers shared across the TUI crate.
|
||||
|
||||
use std::future::Future;
|
||||
use std::pin::Pin;
|
||||
use std::sync::LazyLock;
|
||||
|
||||
use codex_models_manager::bundled_models_response;
|
||||
@@ -53,3 +55,18 @@ where
|
||||
panic!("app-server wire value should map to legacy helper type: {err}")
|
||||
})
|
||||
}
|
||||
|
||||
/// Construct a large test future outside its caller's poll frame before boxing it.
|
||||
/// `Box::pin(large_future())` can still reserve the future's full size on that stack.
|
||||
#[inline(never)]
|
||||
pub(crate) fn box_future<F: Future>(create: impl FnOnce() -> F) -> Pin<Box<F>> {
|
||||
Box::pin(create())
|
||||
}
|
||||
|
||||
// Keep boxed call sites shaped like ordinary calls, without closure indentation.
|
||||
macro_rules! boxed_future {
|
||||
($future:expr) => {
|
||||
$crate::test_support::box_future(|| $future)
|
||||
};
|
||||
}
|
||||
pub(crate) use boxed_future;
|
||||
|
||||
Reference in New Issue
Block a user