clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-29 08:56:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: box apply_patch test harness futures (#15835)

Browse Source 
## Why

`#[large_stack_test]` made the `apply_patch_cli` tests pass by giving
them more stack, but it did not address why those tests needed the extra
stack in the first place.

The real problem is the async state built by the `apply_patch_cli`
harness path. Those tests await three helper boundaries directly:
harness construction, turn submission, and apply-patch output
collection. If those helpers inline their full child futures, the test
future grows to include the whole harness startup and request/response
path.

This change replaces the workaround from #12768 with the same basic
approach used in #13429, but keeps the fix narrower: only the helper
boundaries awaited directly by `apply_patch_cli` stay boxed.

## What Changed

- removed `#[large_stack_test]` from
`core/tests/suite/apply_patch_cli.rs`
- restored ordinary `#[tokio::test(flavor = "multi_thread",
worker_threads = 2)]` annotations in that suite
- deleted the now-unused `codex-test-macros` crate and removed its
workspace wiring
- boxed only the three helper boundaries that the suite awaits directly:
  - `apply_patch_harness_with(...)`
  - `TestCodexHarness::submit(...)`
  - `TestCodexHarness::apply_patch_output(...)`
- added comments at those boxed boundaries explaining why they remain
boxed

## Testing

- `cargo test -p codex-core --test all suite::apply_patch_cli --
--nocapture`

## References

- #12768
- #13429
This commit is contained in:
Michael Bolin
2026-03-26 10:32:04 -07:00
committed by GitHub
parent e7139e14a2
commit e36ebaa3da
8 changed files with 42 additions and 226 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
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;
@@ -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)]