From 2d385136a161ac9e3bcb2195cbb57bacda55be82 Mon Sep 17 00:00:00 2001 From: jimmyfraiture Date: Fri, 10 Oct 2025 16:42:02 +0200 Subject: [PATCH 1/2] test: reduce time dependency on test harness --- codex-rs/Cargo.lock | 110 +++++++++++++- codex-rs/core/tests/common/Cargo.toml | 2 + codex-rs/core/tests/common/lib.rs | 143 ++++++++++++++++++ codex-rs/core/tests/suite/cli_stream.rs | 114 ++++---------- .../core/tests/suite/user_notification.rs | 13 +- 5 files changed, 282 insertions(+), 100 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 0c12880c94..b02a673ee2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1577,10 +1577,12 @@ dependencies = [ "anyhow", "assert_cmd", "codex-core", + "notify", "regex-lite", "serde_json", "tempfile", "tokio", + "walkdir", "wiremock", ] @@ -1645,7 +1647,7 @@ dependencies = [ "bitflags 2.9.1", "crossterm_winapi", "futures-core", - "mio", + "mio 1.0.4", "parking_lot", "rustix 0.38.44", "signal-hook", @@ -2299,6 +2301,18 @@ dependencies = [ "winapi", ] +[[package]] +name = "filetime" +version = "0.2.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc0505cd1b6fa6580283f6bdf70a73fcf4aba1184038c90902b92b3dd0df63ed" +dependencies = [ + "cfg-if", + "libc", + "libredox", + "windows-sys 0.60.2", +] + [[package]] name = "fixed_decimal" version = "0.7.0" @@ -2371,6 +2385,15 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fsevent-sys" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76ee7a02da4d231650c7cea31349b889be2f45ddb3ef3032d2ec8185f6313fd2" +dependencies = [ + "libc", +] + [[package]] name = "futures" version = "0.3.31" @@ -3057,6 +3080,26 @@ version = "2.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f4c7245a08504955605670dbf141fceab975f15ca21570696aebe9d2e71576bd" +[[package]] +name = "inotify" +version = "0.9.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8069d3ec154eb856955c1c0fbffefbf5f3c40a104ec912d4797314c1801abff" +dependencies = [ + "bitflags 1.3.2", + "inotify-sys", + "libc", +] + +[[package]] +name = "inotify-sys" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e05c02b5e89bff3b946cedeca278abc628fe811e604f027c45a8aa3cf793d0eb" +dependencies = [ + "libc", +] + [[package]] name = "inout" version = "0.1.4" @@ -3257,6 +3300,26 @@ dependencies = [ "zeroize", ] +[[package]] +name = "kqueue" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eac30106d7dce88daf4a3fcb4879ea939476d5074a9b7ddd0fb97fa4bed5596a" +dependencies = [ + "kqueue-sys", + "libc", +] + +[[package]] +name = "kqueue-sys" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed9625ffda8729b85e45cf04090035ac368927b8cebc34898e7c120f52e4838b" +dependencies = [ + "bitflags 1.3.2", + "libc", +] + [[package]] name = "lalrpop" version = "0.19.12" @@ -3334,6 +3397,7 @@ checksum = "4488594b9328dee448adb906d8b126d9b7deb7cf5c22161ee591610bb1be83c0" dependencies = [ "bitflags 2.9.1", "libc", + "redox_syscall", ] [[package]] @@ -3534,6 +3598,18 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "mio" +version = "0.8.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c" +dependencies = [ + "libc", + "log", + "wasi 0.11.1+wasi-snapshot-preview1", + "windows-sys 0.48.0", +] + [[package]] name = "mio" version = "1.0.4" @@ -3656,6 +3732,25 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" +[[package]] +name = "notify" +version = "6.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6205bd8bb1e454ad2e27422015fb5e4f2bcc7e08fa8f27058670d208324a4d2d" +dependencies = [ + "bitflags 2.9.1", + "crossbeam-channel", + "filetime", + "fsevent-sys", + "inotify", + "kqueue", + "libc", + "log", + "mio 0.8.11", + "walkdir", + "windows-sys 0.48.0", +] + [[package]] name = "nu-ansi-term" version = "0.50.1" @@ -5380,7 +5475,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34db1a06d485c9142248b7a054f034b349b212551f3dfd19c94d45a754a217cd" dependencies = [ "libc", - "mio", + "mio 1.0.4", "signal-hook", ] @@ -5948,7 +6043,7 @@ dependencies = [ "bytes", "io-uring", "libc", - "mio", + "mio 1.0.4", "parking_lot", "pin-project-lite", "signal-hook-registry", @@ -6846,6 +6941,15 @@ dependencies = [ "windows-targets 0.42.2", ] +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets 0.48.5", +] + [[package]] name = "windows-sys" version = "0.52.0" diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 6ecc54937f..8d254e6c7b 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -15,3 +15,5 @@ serde_json = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["time"] } wiremock = { workspace = true } +notify = "6" +walkdir = { workspace = true } diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 2c012b9b35..5944fa9481 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -164,6 +164,149 @@ pub fn sandbox_network_env_var() -> &'static str { codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR } +pub mod fs_wait { + use anyhow::Result; + use anyhow::anyhow; + use notify::RecursiveMode; + use notify::Watcher; + use std::path::Path; + use std::path::PathBuf; + use std::sync::mpsc; + use std::sync::mpsc::RecvTimeoutError; + use std::time::Duration; + use std::time::Instant; + use tokio::task; + use walkdir::WalkDir; + + pub async fn wait_for_path_exists( + path: impl Into, + timeout: Duration, + ) -> Result { + let path = path.into(); + task::spawn_blocking(move || wait_for_path_exists_blocking(path, timeout)).await? + } + + pub async fn wait_for_matching_file( + root: impl Into, + timeout: Duration, + predicate: impl FnMut(&Path) -> bool + Send + 'static, + ) -> Result { + let root = root.into(); + task::spawn_blocking(move || { + let mut predicate = predicate; + blocking_find_matching_file(root, timeout, &mut predicate) + }) + .await? + } + + fn wait_for_path_exists_blocking(path: PathBuf, timeout: Duration) -> Result { + if path.exists() { + return Ok(path); + } + + let watch_root = nearest_existing_ancestor(&path); + let (tx, rx) = mpsc::channel(); + let mut watcher = notify::recommended_watcher(move |res| { + let _ = tx.send(res); + })?; + watcher.watch(&watch_root, RecursiveMode::Recursive)?; + + let deadline = Instant::now() + timeout; + loop { + if path.exists() { + return Ok(path.clone()); + } + let now = Instant::now(); + if now >= deadline { + break; + } + let remaining = deadline.saturating_duration_since(now); + match rx.recv_timeout(remaining) { + Ok(Ok(_event)) => { + if path.exists() { + return Ok(path.clone()); + } + } + Ok(Err(err)) => return Err(err.into()), + Err(RecvTimeoutError::Timeout) => break, + Err(RecvTimeoutError::Disconnected) => break, + } + } + + if path.exists() { + Ok(path) + } else { + Err(anyhow!("timed out waiting for {:?}", path)) + } + } + + fn blocking_find_matching_file( + root: PathBuf, + timeout: Duration, + predicate: &mut impl FnMut(&Path) -> bool, + ) -> Result { + let root = wait_for_path_exists_blocking(root, timeout)?; + + if let Some(found) = scan_for_match(&root, predicate) { + return Ok(found); + } + + let (tx, rx) = mpsc::channel(); + let mut watcher = notify::recommended_watcher(move |res| { + let _ = tx.send(res); + })?; + watcher.watch(&root, RecursiveMode::Recursive)?; + + let deadline = Instant::now() + timeout; + + while Instant::now() < deadline { + let remaining = deadline.saturating_duration_since(Instant::now()); + match rx.recv_timeout(remaining) { + Ok(Ok(_event)) => { + if let Some(found) = scan_for_match(&root, predicate) { + return Ok(found); + } + } + Ok(Err(err)) => return Err(err.into()), + Err(RecvTimeoutError::Timeout) => break, + Err(RecvTimeoutError::Disconnected) => break, + } + } + + if let Some(found) = scan_for_match(&root, predicate) { + Ok(found) + } else { + Err(anyhow!("timed out waiting for matching file in {:?}", root)) + } + } + + fn scan_for_match(root: &Path, predicate: &mut impl FnMut(&Path) -> bool) -> Option { + for entry in WalkDir::new(root).into_iter().filter_map(Result::ok) { + let path = entry.path(); + if !entry.file_type().is_file() { + continue; + } + if predicate(path) { + return Some(path.to_path_buf()); + } + } + None + } + + fn nearest_existing_ancestor(path: &Path) -> PathBuf { + let mut current = path; + loop { + if current.exists() { + return current.to_path_buf(); + } + match current.parent() { + Some(parent) => current = parent, + None => return PathBuf::from("."), + } + } + } +} + #[macro_export] macro_rules! skip_if_sandbox { () => {{ diff --git a/codex-rs/core/tests/suite/cli_stream.rs b/codex-rs/core/tests/suite/cli_stream.rs index f9408d5a9c..497730926a 100644 --- a/codex-rs/core/tests/suite/cli_stream.rs +++ b/codex-rs/core/tests/suite/cli_stream.rs @@ -1,12 +1,11 @@ use assert_cmd::Command as AssertCommand; use codex_core::RolloutRecorder; use codex_core::protocol::GitInfo; +use core_test_support::fs_wait; use core_test_support::skip_if_no_network; use std::time::Duration; -use std::time::Instant; use tempfile::TempDir; use uuid::Uuid; -use walkdir::WalkDir; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -211,12 +210,12 @@ async fn responses_api_stream_cli() { /// End-to-end: create a session (writes rollout), verify the file, then resume and confirm append. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn integration_creates_and_checks_session_file() { +async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { // Honor sandbox network restrictions for CI parity with the other tests. - skip_if_no_network!(); + skip_if_no_network!(Ok(())); // 1. Temp home so we read/write isolated session files. - let home = TempDir::new().unwrap(); + let home = TempDir::new()?; // 2. Unique marker we'll look for in the session log. let marker = format!("integration-test-{}", Uuid::new_v4()); @@ -254,63 +253,20 @@ async fn integration_creates_and_checks_session_file() { // Wait for sessions dir to appear. let sessions_dir = home.path().join("sessions"); - let dir_deadline = Instant::now() + Duration::from_secs(5); - while !sessions_dir.exists() && Instant::now() < dir_deadline { - std::thread::sleep(Duration::from_millis(50)); - } - assert!(sessions_dir.exists(), "sessions directory never appeared"); + fs_wait::wait_for_path_exists(&sessions_dir, Duration::from_secs(5)).await?; // Find the session file that contains `marker`. - let deadline = Instant::now() + Duration::from_secs(10); - let mut matching_path: Option = None; - while Instant::now() < deadline && matching_path.is_none() { - for entry in WalkDir::new(&sessions_dir) { - let entry = match entry { - Ok(e) => e, - Err(_) => continue, - }; - if !entry.file_type().is_file() { - continue; - } - if !entry.file_name().to_string_lossy().ends_with(".jsonl") { - continue; - } - let path = entry.path(); - let Ok(content) = std::fs::read_to_string(path) else { - continue; - }; - let mut lines = content.lines(); - if lines.next().is_none() { - continue; - } - for line in lines { - if line.trim().is_empty() { - continue; - } - let item: serde_json::Value = match serde_json::from_str(line) { - Ok(v) => v, - Err(_) => continue, - }; - if item.get("type").and_then(|t| t.as_str()) == Some("response_item") - && let Some(payload) = item.get("payload") - && payload.get("type").and_then(|t| t.as_str()) == Some("message") - && let Some(c) = payload.get("content") - && c.to_string().contains(&marker) - { - matching_path = Some(path.to_path_buf()); - break; - } - } + let marker_clone = marker.clone(); + let path = fs_wait::wait_for_matching_file(&sessions_dir, Duration::from_secs(10), move |p| { + if p.extension().and_then(|ext| ext.to_str()) != Some("jsonl") { + return false; } - if matching_path.is_none() { - std::thread::sleep(Duration::from_millis(50)); - } - } - - let path = match matching_path { - Some(p) => p, - None => panic!("No session file containing the marker was found"), - }; + let Ok(content) = std::fs::read_to_string(p) else { + return false; + }; + content.contains(&marker_clone) + }) + .await?; // Basic sanity checks on location and metadata. let rel = match path.strip_prefix(&sessions_dir) { @@ -418,42 +374,25 @@ async fn integration_creates_and_checks_session_file() { assert!(output2.status.success(), "resume codex-cli run failed"); // Find the new session file containing the resumed marker. - let deadline = Instant::now() + Duration::from_secs(10); - let mut resumed_path: Option = None; - while Instant::now() < deadline && resumed_path.is_none() { - for entry in WalkDir::new(&sessions_dir) { - let entry = match entry { - Ok(e) => e, - Err(_) => continue, - }; - if !entry.file_type().is_file() { - continue; - } - if !entry.file_name().to_string_lossy().ends_with(".jsonl") { - continue; + let marker2_clone = marker2.clone(); + let resumed_path = + fs_wait::wait_for_matching_file(&sessions_dir, Duration::from_secs(10), move |p| { + if p.extension().and_then(|ext| ext.to_str()) != Some("jsonl") { + return false; } - let p = entry.path(); - let Ok(c) = std::fs::read_to_string(p) else { - continue; - }; - if c.contains(&marker2) { - resumed_path = Some(p.to_path_buf()); - break; - } - } - if resumed_path.is_none() { - std::thread::sleep(Duration::from_millis(50)); - } - } + std::fs::read_to_string(p) + .map(|content| content.contains(&marker2_clone)) + .unwrap_or(false) + }) + .await?; - let resumed_path = resumed_path.expect("No resumed session file found containing the marker2"); // Resume should write to the existing log file. assert_eq!( resumed_path, path, "resume should create a new session file" ); - let resumed_content = std::fs::read_to_string(&resumed_path).unwrap(); + let resumed_content = std::fs::read_to_string(&resumed_path)?; assert!( resumed_content.contains(&marker), "resumed file missing original marker" @@ -462,6 +401,7 @@ async fn integration_creates_and_checks_session_file() { resumed_content.contains(&marker2), "resumed file missing resumed marker" ); + Ok(()) } /// Integration test to verify git info is collected and recorded in session files. diff --git a/codex-rs/core/tests/suite/user_notification.rs b/codex-rs/core/tests/suite/user_notification.rs index 2ad87c7b6c..3390f4a65a 100644 --- a/codex-rs/core/tests/suite/user_notification.rs +++ b/codex-rs/core/tests/suite/user_notification.rs @@ -5,6 +5,7 @@ use std::os::unix::fs::PermissionsExt; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; +use core_test_support::fs_wait; use core_test_support::responses; use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodex; @@ -17,8 +18,7 @@ use responses::ev_assistant_message; use responses::ev_completed; use responses::sse; use responses::start_mock_server; -use tokio::time::Duration; -use tokio::time::sleep; +use std::time::Duration; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn summarize_context_three_requests_and_instructions() -> anyhow::Result<()> { @@ -60,14 +60,7 @@ echo -n "${@: -1}" > $(dirname "${0}")/notify.txt"#, wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; // We fork the notify script, so we need to wait for it to write to the file. - for _ in 0..100u32 { - if notify_file.exists() { - break; - } - sleep(Duration::from_millis(100)).await; - } - - assert!(notify_file.exists()); + fs_wait::wait_for_path_exists(¬ify_file, Duration::from_secs(5)).await?; Ok(()) } From 2690405e9e09812f18b359706371f8c33bfcff75 Mon Sep 17 00:00:00 2001 From: jimmyfraiture Date: Fri, 10 Oct 2025 16:43:32 +0200 Subject: [PATCH 2/2] Cargo nit --- codex-rs/Cargo.lock | 63 ++++++++------------------- codex-rs/Cargo.toml | 1 + codex-rs/core/tests/common/Cargo.toml | 4 +- 3 files changed, 20 insertions(+), 48 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index b02a673ee2..5576b7cba9 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1647,7 +1647,7 @@ dependencies = [ "bitflags 2.9.1", "crossterm_winapi", "futures-core", - "mio 1.0.4", + "mio", "parking_lot", "rustix 0.38.44", "signal-hook", @@ -2301,18 +2301,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "filetime" -version = "0.2.26" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc0505cd1b6fa6580283f6bdf70a73fcf4aba1184038c90902b92b3dd0df63ed" -dependencies = [ - "cfg-if", - "libc", - "libredox", - "windows-sys 0.60.2", -] - [[package]] name = "fixed_decimal" version = "0.7.0" @@ -3082,11 +3070,11 @@ checksum = "f4c7245a08504955605670dbf141fceab975f15ca21570696aebe9d2e71576bd" [[package]] name = "inotify" -version = "0.9.6" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8069d3ec154eb856955c1c0fbffefbf5f3c40a104ec912d4797314c1801abff" +checksum = "f37dccff2791ab604f9babef0ba14fbe0be30bd368dc541e2b08d07c8aa908f3" dependencies = [ - "bitflags 1.3.2", + "bitflags 2.9.1", "inotify-sys", "libc", ] @@ -3397,7 +3385,6 @@ checksum = "4488594b9328dee448adb906d8b126d9b7deb7cf5c22161ee591610bb1be83c0" dependencies = [ "bitflags 2.9.1", "libc", - "redox_syscall", ] [[package]] @@ -3598,18 +3585,6 @@ dependencies = [ "simd-adler32", ] -[[package]] -name = "mio" -version = "0.8.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c" -dependencies = [ - "libc", - "log", - "wasi 0.11.1+wasi-snapshot-preview1", - "windows-sys 0.48.0", -] - [[package]] name = "mio" version = "1.0.4" @@ -3734,23 +3709,28 @@ checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" [[package]] name = "notify" -version = "6.1.1" +version = "8.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6205bd8bb1e454ad2e27422015fb5e4f2bcc7e08fa8f27058670d208324a4d2d" +checksum = "4d3d07927151ff8575b7087f245456e549fea62edf0ec4e565a5ee50c8402bc3" dependencies = [ "bitflags 2.9.1", - "crossbeam-channel", - "filetime", "fsevent-sys", "inotify", "kqueue", "libc", "log", - "mio 0.8.11", + "mio", + "notify-types", "walkdir", - "windows-sys 0.48.0", + "windows-sys 0.60.2", ] +[[package]] +name = "notify-types" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e0826a989adedc2a244799e823aece04662b66609d96af8dff7ac6df9a8925d" + [[package]] name = "nu-ansi-term" version = "0.50.1" @@ -5475,7 +5455,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34db1a06d485c9142248b7a054f034b349b212551f3dfd19c94d45a754a217cd" dependencies = [ "libc", - "mio 1.0.4", + "mio", "signal-hook", ] @@ -6043,7 +6023,7 @@ dependencies = [ "bytes", "io-uring", "libc", - "mio 1.0.4", + "mio", "parking_lot", "pin-project-lite", "signal-hook-registry", @@ -6941,15 +6921,6 @@ dependencies = [ "windows-targets 0.42.2", ] -[[package]] -name = "windows-sys" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" -dependencies = [ - "windows-targets 0.48.5", -] - [[package]] name = "windows-sys" version = "0.52.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index ca40b1a536..1c1640be3f 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -122,6 +122,7 @@ log = "0.4" maplit = "1.0.2" mime_guess = "2.0.5" multimap = "0.10.0" +notify = "8.2.0" nucleo-matcher = "0.3.1" openssl-sys = "*" opentelemetry = "0.30.0" diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 8d254e6c7b..b3082dc548 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -10,10 +10,10 @@ path = "lib.rs" anyhow = { workspace = true } assert_cmd = { workspace = true } codex-core = { workspace = true } +notify = { workspace = true } regex-lite = { workspace = true } serde_json = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["time"] } -wiremock = { workspace = true } -notify = "6" walkdir = { workspace = true } +wiremock = { workspace = true }