openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-11 20:36:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: remove apply_patch large-stack test workaround

Browse Source
This commit is contained in:
Michael Bolin
2026-03-25 22:35:10 -07:00
parent 6d0525ae70
commit a0c1ff4d71
8 changed files with 50 additions and 242 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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"

2
codex-rs/Cargo.toml
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" }

1
codex-rs/core/Cargo.toml
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 }

38
codex-rs/core/tests/common/test_codex.rs
View File

@@ -661,11 +661,11 @@ impl TestCodex {
}
pub async fn submit_turn(&self, prompt: &str) -> Result<()> {
self.submit_turn_with_policies(
Box::pin(self.submit_turn_with_policies(
prompt,
AskForApproval::Never,
SandboxPolicy::DangerFullAccess,
)
))
.await
}
@@ -674,7 +674,7 @@ impl TestCodex {
prompt: &str,
sandbox_policy: SandboxPolicy,
) -> Result<()> {
self.submit_turn_with_policies(prompt, AskForApproval::Never, sandbox_policy)
Box::pin(self.submit_turn_with_policies(prompt, AskForApproval::Never, sandbox_policy))
.await
}
@@ -683,12 +683,12 @@ impl TestCodex {
prompt: &str,
service_tier: Option<ServiceTier>,
) -> Result<()> {
self.submit_turn_with_context(
Box::pin(self.submit_turn_with_context(
prompt,
AskForApproval::Never,
SandboxPolicy::DangerFullAccess,
Some(service_tier),
)
))
.await
}
@@ -698,12 +698,12 @@ impl TestCodex {
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
) -> Result<()> {
self.submit_turn_with_context(
Box::pin(self.submit_turn_with_context(
prompt,
approval_policy,
sandbox_policy,
/*service_tier*/ None,
)
))
.await
}
@@ -756,16 +756,16 @@ pub struct TestCodexHarness {
impl TestCodexHarness {
pub async fn new() -> Result<Self> {
Self::with_builder(test_codex()).await
Box::pin(Self::with_builder(test_codex())).await
}
pub async fn with_config(mutator: impl FnOnce(&mut Config) + Send + 'static) -> Result<Self> {
Self::with_builder(test_codex().with_config(mutator)).await
Box::pin(Self::with_builder(test_codex().with_config(mutator))).await
}
pub async fn with_builder(mut builder: TestCodexBuilder) -> Result<Self> {
let server = start_mock_server().await;
let test = builder.build(&server).await?;
let test = Box::pin(builder.build(&server)).await?;
Ok(Self { server, test })
}
@@ -786,7 +786,7 @@ impl TestCodexHarness {
}
pub async fn submit(&self, prompt: &str) -> Result<()> {
self.test.submit_turn(prompt).await
Box::pin(self.test.submit_turn(prompt)).await
}
pub async fn submit_with_policy(
@@ -794,9 +794,7 @@ impl TestCodexHarness {
prompt: &str,
sandbox_policy: SandboxPolicy,
) -> Result<()> {
self.test
.submit_turn_with_policy(prompt, sandbox_policy)
.await
Box::pin(self.test.submit_turn_with_policy(prompt, sandbox_policy)).await
}
pub async fn request_bodies(&self) -> Vec<Value> {
@@ -815,12 +813,12 @@ impl TestCodexHarness {
}
pub async fn function_call_output_value(&self, call_id: &str) -> Value {
let bodies = self.request_bodies().await;
let bodies = Box::pin(self.request_bodies()).await;
function_call_output(&bodies, call_id).clone()
}
pub async fn function_call_stdout(&self, call_id: &str) -> String {
self.function_call_output_value(call_id)
Box::pin(self.function_call_output_value(call_id))
.await
.get("output")
.and_then(Value::as_str)
@@ -829,7 +827,7 @@ impl TestCodexHarness {
}
pub async fn custom_tool_call_output(&self, call_id: &str) -> String {
let bodies = self.request_bodies().await;
let bodies = Box::pin(self.request_bodies()).await;
custom_tool_call_output_text(&bodies, call_id)
}
@@ -839,12 +837,14 @@ impl TestCodexHarness {
output_type: ApplyPatchModelOutput,
) -> String {
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
}
}
}

63
codex-rs/core/tests/suite/apply_patch_cli.rs
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;
@@ -42,7 +41,7 @@ use wiremock::matchers::method;
use wiremock::matchers::path_regex;
pub async fn apply_patch_harness() -> Result<TestCodexHarness> {
apply_patch_harness_with(|builder| builder).await
Box::pin(apply_patch_harness_with(|builder| builder)).await
}
async fn apply_patch_harness_with(
@@ -51,7 +50,7 @@ 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::pin(TestCodexHarness::with_builder(builder)).await
}
pub async fn mount_apply_patch(
@@ -87,7 +86,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 +133,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 +160,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 +189,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 +217,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 +243,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 +274,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 +337,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 +363,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 +395,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 +429,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 +465,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 +502,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 +527,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 +554,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 +608,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 +663,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 +700,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 +738,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 +878,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 +963,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 +1041,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 +1064,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 +1087,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 +1122,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 +1183,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 +1251,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 +1321,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 +1413,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)]

7
codex-rs/test-macros/BUILD.bazel
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,
)

16
codex-rs/test-macros/Cargo.toml
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

155
codex-rs/test-macros/src/lib.rs
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"));
}
}