fix: box apply_patch test harness futures

This commit is contained in:
Michael Bolin
2026-03-25 22:35:10 -07:00
parent 6d0525ae70
commit e679a8e14b
8 changed files with 41 additions and 225 deletions

10
codex-rs/Cargo.lock generated
View File

@@ -1920,7 +1920,6 @@ dependencies = [
"codex-shell-escalation",
"codex-state",
"codex-terminal-detection",
"codex-test-macros",
"codex-utils-absolute-path",
"codex-utils-cache",
"codex-utils-cargo-bin",
@@ -2696,15 +2695,6 @@ dependencies = [
"tracing",
]
[[package]]
name = "codex-test-macros"
version = "0.0.0"
dependencies = [
"proc-macro2",
"quote",
"syn 2.0.114",
]
[[package]]
name = "codex-tui"
version = "0.0.0"

View File

@@ -80,7 +80,6 @@ members = [
"state",
"terminal-detection",
"codex-experimental-api-macros",
"test-macros",
"package-manager",
"plugin",
"artifacts",
@@ -152,7 +151,6 @@ codex-shell-escalation = { path = "shell-escalation" }
codex-skills = { path = "skills" }
codex-state = { path = "state" }
codex-stdio-to-uds = { path = "stdio-to-uds" }
codex-test-macros = { path = "test-macros" }
codex-terminal-detection = { path = "terminal-detection" }
codex-tui = { path = "tui" }
codex-tui-app-server = { path = "tui_app_server" }

View File

@@ -149,7 +149,6 @@ codex-arg0 = { workspace = true }
codex-otel = { workspace = true, features = [
"disable-default-metrics-exporter",
] }
codex-test-macros = { workspace = true }
codex-utils-cargo-bin = { workspace = true }
core_test_support = { workspace = true }
ctor = { workspace = true }

View File

@@ -786,7 +786,9 @@ impl TestCodexHarness {
}
pub async fn submit(&self, prompt: &str) -> Result<()> {
self.test.submit_turn(prompt).await
// Box the submit-and-wait path so callers do not inline the full turn
// future into their own async state.
Box::pin(self.test.submit_turn(prompt)).await
}
pub async fn submit_with_policy(
@@ -838,13 +840,17 @@ impl TestCodexHarness {
call_id: &str,
output_type: ApplyPatchModelOutput,
) -> String {
// Box the awaited output helpers so callers do not inline request
// capture and response parsing into their own async state.
match output_type {
ApplyPatchModelOutput::Freeform => self.custom_tool_call_output(call_id).await,
ApplyPatchModelOutput::Freeform => {
Box::pin(self.custom_tool_call_output(call_id)).await
}
ApplyPatchModelOutput::Function
| ApplyPatchModelOutput::Shell
| ApplyPatchModelOutput::ShellViaHeredoc
| ApplyPatchModelOutput::ShellCommandViaHeredoc => {
self.function_call_stdout(call_id).await
Box::pin(self.function_call_stdout(call_id)).await
}
}
}

View File

@@ -3,7 +3,6 @@
use anyhow::Result;
use base64::Engine;
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
use codex_test_macros::large_stack_test;
use core_test_support::responses::ev_apply_patch_call;
use core_test_support::responses::ev_apply_patch_custom_tool_call;
use core_test_support::responses::ev_shell_command_call;
@@ -51,7 +50,9 @@ async fn apply_patch_harness_with(
let builder = configure(test_codex()).with_config(|config| {
config.include_apply_patch_tool = true;
});
TestCodexHarness::with_builder(builder).await
// Box harness construction so apply_patch_cli tests do not inline the
// full test-thread startup path into each test future.
Box::pin(TestCodexHarness::with_builder(builder)).await
}
pub async fn mount_apply_patch(
@@ -87,7 +88,7 @@ fn apply_patch_responses(
]
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -134,7 +135,7 @@ D delete.txt
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -161,7 +162,7 @@ async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) ->
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -190,7 +191,7 @@ async fn apply_patch_cli_moves_file_to_new_directory(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -218,7 +219,7 @@ async fn apply_patch_cli_updates_file_appends_trailing_newline(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -244,7 +245,7 @@ async fn apply_patch_cli_insert_only_hunk_modifies_file(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -275,7 +276,7 @@ async fn apply_patch_cli_move_overwrites_existing_destination(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -338,7 +339,7 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -364,7 +365,7 @@ async fn apply_patch_cli_add_overwrites_existing_file(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -396,7 +397,7 @@ async fn apply_patch_cli_rejects_invalid_hunk_header(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -430,7 +431,7 @@ async fn apply_patch_cli_reports_missing_context(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -466,7 +467,7 @@ async fn apply_patch_cli_reports_missing_target_file(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -503,7 +504,7 @@ async fn apply_patch_cli_delete_missing_file_reports_error(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -528,7 +529,7 @@ async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -555,7 +556,7 @@ async fn apply_patch_cli_delete_directory_reports_verification_error(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -609,7 +610,7 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -664,7 +665,7 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -701,7 +702,7 @@ async fn apply_patch_cli_verification_failure_has_no_side_effects(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_shell_command_heredoc_with_cd_updates_relative_workdir() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -739,7 +740,7 @@ async fn apply_patch_shell_command_heredoc_with_cd_updates_relative_workdir() ->
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_cli_can_use_shell_command_output_as_patch_input() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -879,7 +880,7 @@ async fn apply_patch_cli_can_use_shell_command_output_as_patch_input() -> Result
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_shell_command_heredoc_with_cd_emits_turn_diff() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -964,7 +965,7 @@ async fn apply_patch_shell_command_heredoc_with_cd_emits_turn_diff() -> Result<(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -1042,7 +1043,7 @@ async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() ->
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch(
@@ -1065,7 +1066,7 @@ async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -1088,7 +1089,7 @@ async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput)
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -1123,7 +1124,7 @@ async fn apply_patch_cli_missing_second_chunk_context_rejected(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -1184,7 +1185,7 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]
@@ -1252,7 +1253,7 @@ async fn apply_patch_turn_diff_for_rename_with_content_change(
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -1322,7 +1323,7 @@ async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()>
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -1414,7 +1415,7 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result
Ok(())
}
#[large_stack_test]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Function)]
#[test_case(ApplyPatchModelOutput::Shell)]

View File

@@ -1,7 +0,0 @@
load("//:defs.bzl", "codex_rust_crate")
codex_rust_crate(
name = "test-macros",
crate_name = "codex_test_macros",
proc_macro = True,
)

View File

@@ -1,16 +0,0 @@
[package]
name = "codex-test-macros"
version.workspace = true
edition.workspace = true
license.workspace = true
[lib]
proc-macro = true
[dependencies]
proc-macro2 = "1"
quote = "1"
syn = { version = "2", features = ["full"] }
[lints]
workspace = true

View File

@@ -1,155 +0,0 @@
use proc_macro::TokenStream;
use proc_macro2::TokenStream as TokenStream2;
use quote::quote;
use syn::Attribute;
use syn::ItemFn;
use syn::parse::Nothing;
use syn::parse_macro_input;
use syn::parse_quote;
const LARGE_STACK_TEST_STACK_SIZE_BYTES: usize = 16 * 1024 * 1024;
/// Run a test body on a dedicated thread with a larger stack.
///
/// For async tests, this macro creates a Tokio multi-thread runtime with two
/// worker threads and blocks on the original async body inside the large-stack
/// thread.
#[proc_macro_attribute]
pub fn large_stack_test(attr: TokenStream, item: TokenStream) -> TokenStream {
parse_macro_input!(attr as Nothing);
let item = parse_macro_input!(item as ItemFn);
expand_large_stack_test(item).into()
}
fn expand_large_stack_test(mut item: ItemFn) -> TokenStream2 {
let attrs = filtered_attributes(&item.attrs);
item.attrs = attrs;
let is_async = item.sig.asyncness.take().is_some();
let name = &item.sig.ident;
let body = &item.block;
let thread_body = if is_async {
quote! {
{
let runtime = ::tokio::runtime::Builder::new_multi_thread()
.worker_threads(2)
.enable_all()
.build()
.unwrap_or_else(|error| {
panic!("failed to build tokio runtime for large-stack test: {error}")
});
runtime.block_on(async move #body)
}
}
} else {
quote! { #body }
};
*item.block = parse_quote!({
let handle = ::std::thread::Builder::new()
.name(::std::string::String::from(::std::stringify!(#name)))
.stack_size(#LARGE_STACK_TEST_STACK_SIZE_BYTES)
.spawn(move || #thread_body)
.unwrap_or_else(|error| {
panic!("failed to spawn large-stack test thread: {error}")
});
match handle.join() {
Ok(result) => result,
Err(payload) => ::std::panic::resume_unwind(payload),
}
});
quote! { #item }
}
fn filtered_attributes(attrs: &[Attribute]) -> Vec<Attribute> {
let mut filtered = Vec::with_capacity(attrs.len() + 1);
let mut has_test_attr = false;
for attr in attrs {
if is_tokio_test_attr(attr) {
continue;
}
if is_test_attr(attr) || is_test_case_attr(attr) {
has_test_attr = true;
}
filtered.push(attr.clone());
}
if !has_test_attr {
filtered.push(parse_quote!(#[test]));
}
filtered
}
fn is_test_attr(attr: &Attribute) -> bool {
attr.path().is_ident("test")
}
fn is_test_case_attr(attr: &Attribute) -> bool {
attr.path().is_ident("test_case")
}
fn is_tokio_test_attr(attr: &Attribute) -> bool {
let mut segments = attr.path().segments.iter();
matches!(
(segments.next(), segments.next(), segments.next()),
(Some(first), Some(second), None) if first.ident == "tokio" && second.ident == "test"
)
}
#[cfg(test)]
mod tests {
use super::expand_large_stack_test;
use syn::ItemFn;
use syn::parse_quote;
fn has_attr(item: &ItemFn, name: &str) -> bool {
item.attrs.iter().any(|attr| attr.path().is_ident(name))
}
#[test]
fn adds_test_attribute_when_missing() {
let item: ItemFn = parse_quote! {
fn sample() {}
};
let expanded_tokens = expand_large_stack_test(item);
let expanded: ItemFn = match syn::parse2(expanded_tokens) {
Ok(expanded) => expanded,
Err(error) => panic!("failed to parse expanded function: {error}"),
};
assert!(has_attr(&expanded, "test"));
let body = quote::quote!(#expanded).to_string();
assert!(body.contains("stack_size"));
}
#[test]
fn removes_tokio_test_and_keeps_test_case() {
let item: ItemFn = parse_quote! {
#[test_case(1)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn sample(value: usize) -> anyhow::Result<()> {
let _ = value;
Ok(())
}
};
let expanded_tokens = expand_large_stack_test(item);
let expanded: ItemFn = match syn::parse2(expanded_tokens) {
Ok(expanded) => expanded,
Err(error) => panic!("failed to parse expanded function: {error}"),
};
assert!(has_attr(&expanded, "test_case"));
assert!(!has_attr(&expanded, "test"));
let body = quote::quote!(#expanded).to_string();
assert!(body.contains("tokio :: runtime :: Builder"));
assert!(!body.contains("tokio :: test"));
}
}