diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a2b1450e8a..9ae4e7c463 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1846,7 +1846,6 @@ dependencies = [ "codex-shell-escalation", "codex-state", "codex-terminal-detection", - "codex-test-macros", "codex-utils-absolute-path", "codex-utils-cache", "codex-utils-cargo-bin", @@ -2625,15 +2624,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" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 8d047d9232..ba287de5c5 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -80,7 +80,6 @@ members = [ "state", "terminal-detection", "codex-experimental-api-macros", - "test-macros", "package-manager", "plugin", ] @@ -150,7 +149,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" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index f99487a6b3..f64a0a5280 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -148,7 +148,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 } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index efbcf8d8ce..90ea36776e 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -792,7 +792,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( @@ -844,13 +846,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 } } } diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 15fd3b7d36..a8b429c0dd 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -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; @@ -53,7 +52,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( @@ -90,7 +91,7 @@ fn apply_patch_responses( } #[cfg(target_os = "linux")] -#[large_stack_test] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() -> Result<()> { skip_if_no_network!(Ok(())); @@ -136,7 +137,7 @@ async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() - Ok(()) } -#[large_stack_test] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] #[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] @@ -183,7 +184,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)] @@ -210,7 +211,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)] @@ -239,7 +240,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)] @@ -267,7 +268,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)] @@ -293,7 +294,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)] @@ -324,7 +325,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)] @@ -387,7 +388,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)] @@ -413,7 +414,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)] @@ -445,7 +446,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)] @@ -479,7 +480,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)] @@ -515,7 +516,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)] @@ -552,7 +553,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)] @@ -577,7 +578,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)] @@ -604,7 +605,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)] @@ -658,7 +659,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)] @@ -713,7 +714,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)] @@ -750,7 +751,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(())); @@ -788,7 +789,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(())); @@ -928,7 +929,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(())); @@ -1013,7 +1014,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(())); @@ -1091,7 +1092,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( @@ -1114,7 +1115,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)] @@ -1137,7 +1138,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)] @@ -1172,7 +1173,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)] @@ -1233,7 +1234,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)] @@ -1301,7 +1302,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(())); @@ -1371,7 +1372,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(())); @@ -1463,7 +1464,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)] diff --git a/codex-rs/test-macros/BUILD.bazel b/codex-rs/test-macros/BUILD.bazel deleted file mode 100644 index 0ff074f5e5..0000000000 --- a/codex-rs/test-macros/BUILD.bazel +++ /dev/null @@ -1,7 +0,0 @@ -load("//:defs.bzl", "codex_rust_crate") - -codex_rust_crate( - name = "test-macros", - crate_name = "codex_test_macros", - proc_macro = True, -) diff --git a/codex-rs/test-macros/Cargo.toml b/codex-rs/test-macros/Cargo.toml deleted file mode 100644 index 275580322d..0000000000 --- a/codex-rs/test-macros/Cargo.toml +++ /dev/null @@ -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 diff --git a/codex-rs/test-macros/src/lib.rs b/codex-rs/test-macros/src/lib.rs deleted file mode 100644 index 16c87f8582..0000000000 --- a/codex-rs/test-macros/src/lib.rs +++ /dev/null @@ -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 { - 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")); - } -}