diff --git a/CLAUDE.md b/CLAUDE.md index 2b0155b..f175ddf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,6 +34,14 @@ and say so explicitly. `voro-core` requires tests; state-machine transitions and scheduler ordering are the highest-value targets. TUI code is tested where practical, not dogmatically. +A test that needs a scratch directory names it one way, with `tempfile`: +`tempfile::Builder::new().prefix("voro--").tempdir().unwrap().keep()`. +Never build the name from the process id and a clock stamp — the clock does not +advance a nanosecond at a time, so two threads reading inside one step read the +same number and land on the same directory. `keep` leaves the directory behind +deliberately, so a failing test's scratch state survives for inspection; nothing +cleans the temp dir up yet. + ## Git conventions - Feature branches, squash-merged to `main`. One logical change per PR. diff --git a/Cargo.lock b/Cargo.lock index 5b8f690..919d372 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -477,6 +477,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd2e7510819d6fbf51a5545c8f922716ecfb14df168a3242f7d33e0239efe6a1" +[[package]] +name = "fastrand" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" + [[package]] name = "filedescriptor" version = "0.8.3" @@ -1461,6 +1467,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom 0.4.3", + "once_cell", + "rustix", + "windows-sys", +] + [[package]] name = "termina" version = "0.3.3" @@ -1716,6 +1735,7 @@ version = "0.1.0" dependencies = [ "clap", "ratatui", + "tempfile", "voro-core", ] @@ -1726,6 +1746,7 @@ dependencies = [ "rusqlite", "serde", "serde_json", + "tempfile", "thiserror 2.0.18", "toml", "toml_edit", diff --git a/Cargo.toml b/Cargo.toml index e0f82f6..eaf9a20 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ toml = "0.8" toml_edit = "0.22" ratatui = { version = "0.30", features = ["unstable-rendered-line-info"] } uuid = { version = "1", features = ["v4"] } +tempfile = "3" # The profile that 'dist' will build with [profile.dist] diff --git a/crates/voro-core/Cargo.toml b/crates/voro-core/Cargo.toml index 307ae3d..c9343d8 100644 --- a/crates/voro-core/Cargo.toml +++ b/crates/voro-core/Cargo.toml @@ -18,3 +18,6 @@ thiserror.workspace = true toml.workspace = true toml_edit.workspace = true uuid.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/voro-core/src/config_edit.rs b/crates/voro-core/src/config_edit.rs index 8e0cdf4..583a4ef 100644 --- a/crates/voro-core/src/config_edit.rs +++ b/crates/voro-core/src/config_edit.rs @@ -253,14 +253,11 @@ mod tests { /// A unique scratch path per test, cleaned up by the caller. fn scratch(tag: &str) -> std::path::PathBuf { - std::env::temp_dir().join(format!( - "voro-config-edit-{tag}-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )) + tempfile::Builder::new() + .prefix(&format!("voro-config-edit-{tag}-")) + .tempdir() + .unwrap() + .keep() } #[test] diff --git a/crates/voro-core/src/store.rs b/crates/voro-core/src/store.rs index 9f795bc..f0108af 100644 --- a/crates/voro-core/src/store.rs +++ b/crates/voro-core/src/store.rs @@ -2049,16 +2049,11 @@ mod schema_guard_tests { /// A unique scratch directory per test, cleaned up by the caller. fn scratch(tag: &str) -> PathBuf { - let dir = std::env::temp_dir().join(format!( - "voro-store-{tag}-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&dir).unwrap(); - dir + tempfile::Builder::new() + .prefix(&format!("voro-store-{tag}-")) + .tempdir() + .unwrap() + .keep() } #[test] @@ -4869,9 +4864,12 @@ mod tests { /// A unique scratch database path under the OS temp dir. fn scratch_db() -> PathBuf { - static COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); - let n = COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - std::env::temp_dir().join(format!("voro-dataversion-{}-{n}.db", std::process::id())) + tempfile::Builder::new() + .prefix("voro-dataversion-") + .tempdir() + .unwrap() + .keep() + .join("voro.db") } #[test] diff --git a/crates/voro/Cargo.toml b/crates/voro/Cargo.toml index 1c68c52..8de40b0 100644 --- a/crates/voro/Cargo.toml +++ b/crates/voro/Cargo.toml @@ -14,3 +14,6 @@ categories = ["command-line-utilities"] voro-core = { path = "../voro-core", version = "0.1.0" } ratatui.workspace = true clap = { version = "4.6.1", features = ["derive"] } + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/voro/src/app.rs b/crates/voro/src/app.rs index 48289df..7d1a0b2 100644 --- a/crates/voro/src/app.rs +++ b/crates/voro/src/app.rs @@ -4145,14 +4145,11 @@ mod tests { ) -> (Store, crate::dispatch::DispatchCtx, std::path::PathBuf) { use std::process::{Command, Stdio}; - let root = std::env::temp_dir().join(format!( - "voro-app-{name}-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let root = tempfile::Builder::new() + .prefix(&format!("voro-app-{name}-")) + .tempdir() + .unwrap() + .keep(); let project_path = root.join("project"); std::fs::create_dir_all(&project_path).unwrap(); let status = Command::new("git") @@ -4189,14 +4186,11 @@ mod tests { fn resuming_a_task_with_a_live_session_spawns_no_continuation() { use std::process::{Command, Stdio}; - let root = std::env::temp_dir().join(format!( - "voro-app-resume-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let root = tempfile::Builder::new() + .prefix("voro-app-resume-") + .tempdir() + .unwrap() + .keep(); let project_path = root.join("project"); std::fs::create_dir_all(&project_path).unwrap(); let status = Command::new("git") @@ -7334,15 +7328,11 @@ mod tests { /// decides whether that directory is a git repository at all, which is the /// whole of what the press-time gate reads (DESIGN.md §8). fn pr_ready_app(with_repo: bool) -> (App, i64, std::path::PathBuf) { - let dir = std::env::temp_dir().join(format!( - "voro-review-key-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&dir).unwrap(); + let dir = tempfile::Builder::new() + .prefix("voro-review-key-") + .tempdir() + .unwrap() + .keep(); if with_repo { let status = std::process::Command::new("git") .arg("-C") @@ -7661,15 +7651,11 @@ mod tests { /// config and PATH. fn app_with_agents(agents_toml: &str) -> App { let mut app = app_with(&[TaskState::Ready]); - let dir = std::env::temp_dir().join(format!( - "voro-plan-key-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&dir).unwrap(); + let dir = tempfile::Builder::new() + .prefix("voro-plan-key-") + .tempdir() + .unwrap() + .keep(); let agents_path = dir.join("voro.toml"); std::fs::write(&agents_path, agents_toml).unwrap(); app.dispatch_ctx = crate::dispatch::DispatchCtx { diff --git a/crates/voro/src/cli.rs b/crates/voro/src/cli.rs index 2d6e323..199c2f2 100644 --- a/crates/voro/src/cli.rs +++ b/crates/voro/src/cli.rs @@ -2601,14 +2601,11 @@ mod tests { #[test] fn agent_init_then_list_through_the_cli() { - let dir = std::env::temp_dir().join(format!( - "voro-cli-agents-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let dir = tempfile::Builder::new() + .prefix("voro-cli-agents-") + .tempdir() + .unwrap() + .keep(); let agents_path = dir.join("voro/voro.toml"); let ctx = DispatchCtx { db_path: dir.join("voro.db"), @@ -2653,14 +2650,11 @@ mod tests { #[test] fn viewer_add_remove_round_trip_through_the_cli() { - let dir = std::env::temp_dir().join(format!( - "voro-cli-viewers-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let dir = tempfile::Builder::new() + .prefix("voro-cli-viewers-") + .tempdir() + .unwrap() + .keep(); let agents_path = dir.join("voro/voro.toml"); let ctx = DispatchCtx { db_path: dir.join("voro.db"), @@ -3672,15 +3666,11 @@ mod tests { /// A throwaway checkout with no remotes — the shape of a first project /// (DESIGN.md §8), which advertises `open` rather than `pr`. fn remoteless_checkout(tag: &str) -> std::path::PathBuf { - let path = std::env::temp_dir().join(format!( - "voro-cli-{tag}-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&path).unwrap(); + let path = tempfile::Builder::new() + .prefix(&format!("voro-cli-{tag}-")) + .tempdir() + .unwrap() + .keep(); git_in(&path, &["init", "-q"]); path } @@ -4622,15 +4612,11 @@ mod tests { /// viewers. The default `ctx()` points at the developer's real config, /// which these tests must not depend on (or launch viewers from). fn ctx_with_toml(toml: &str) -> DispatchCtx { - let root = std::env::temp_dir().join(format!( - "voro-cli-viewer-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&root).unwrap(); + let root = tempfile::Builder::new() + .prefix("voro-cli-viewer-") + .tempdir() + .unwrap() + .keep(); let agents_path = root.join("voro.toml"); std::fs::write(&agents_path, toml).unwrap(); DispatchCtx { @@ -5104,14 +5090,11 @@ mod tests { fn a_dead_dispatched_session_is_finalised_and_stalled_on_read() { use std::process::{Command, Stdio}; - let root = std::env::temp_dir().join(format!( - "voro-cli-reconcile-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let root = tempfile::Builder::new() + .prefix("voro-cli-reconcile-") + .tempdir() + .unwrap() + .keep(); let project = root.join("project"); std::fs::create_dir_all(&project).unwrap(); let git = |args: &[&str]| { @@ -5188,14 +5171,11 @@ mod tests { fn scratch_env(cmd: &str) -> (Store, DispatchCtx, std::path::PathBuf) { use std::process::{Command, Stdio}; - let root = std::env::temp_dir().join(format!( - "voro-cli-answer-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let root = tempfile::Builder::new() + .prefix("voro-cli-answer-") + .tempdir() + .unwrap() + .keep(); let project = root.join("project"); std::fs::create_dir_all(&project).unwrap(); let status = Command::new("git") diff --git a/crates/voro/src/dispatch.rs b/crates/voro/src/dispatch.rs index 3ab0f95..cda16f5 100644 --- a/crates/voro/src/dispatch.rs +++ b/crates/voro/src/dispatch.rs @@ -1920,20 +1920,8 @@ fn default_base_branch(repo_path: &str) -> String { #[cfg(test)] mod tests { use super::*; - use std::sync::atomic::{AtomicU64, Ordering}; use voro_core::{LivenessSource, NewTask, Priority}; - /// Distinguishes fixtures built within the same clock tick. - /// - /// A nanosecond stamp reads as though it could not repeat, and on one - /// thread it does not: consecutive `SystemTime::now()` calls always differ, - /// because each is ordered after the last. Across threads there is no such - /// ordering, and the clock does not advance a nanosecond at a time — it - /// steps roughly every 20-30ns, so any two threads reading inside one step - /// read the same number. Measured on this workstation: 4000 reads across - /// eight threads yielded 1493 duplicates. - static FIXTURE_SEQ: AtomicU64 = AtomicU64::new(0); - /// A scratch database, a freshly-`git init`ed clean project, and an /// `voro.toml` whose one agent is a stub command that just reads the /// prompt. Returns the store, the dispatch context, and the project path. @@ -1967,18 +1955,14 @@ mod tests { } /// A scratch directory no other fixture can name, however many are built - /// at once: the process id separates test binaries, the counter separates - /// fixtures within one, and the stamp keeps reruns of a recycled pid apart. + /// at once, because `tempfile` creates it exclusively and retries under a + /// different name on a clash. `keep` then leaves it behind for inspection. fn fixture_root() -> PathBuf { - std::env::temp_dir().join(format!( - "voro-dispatch-{}-{}-{}", - std::process::id(), - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos(), - FIXTURE_SEQ.fetch_add(1, Ordering::Relaxed) - )) + tempfile::Builder::new() + .prefix("voro-dispatch-") + .tempdir() + .unwrap() + .keep() } #[test] @@ -2003,35 +1987,6 @@ mod tests { assert_eq!(distinct.len(), THREADS, "roots collided: {roots:?}"); } - /// Eight fixtures spend long enough in `git init` to drift apart on the - /// clock, so the test above can pass even on a name that has no counter in - /// it. This one names four thousand roots with nothing in between, where a - /// clock-only name collides tens of times over. - #[test] - fn roots_named_back_to_back_on_many_threads_are_all_distinct() { - const THREADS: usize = 8; - const EACH: usize = 500; - let gate = std::sync::Barrier::new(THREADS); - - let roots: Vec = std::thread::scope(|scope| { - let handles: Vec<_> = (0..THREADS) - .map(|_| { - scope.spawn(|| { - gate.wait(); - (0..EACH).map(|_| fixture_root()).collect::>() - }) - }) - .collect(); - handles - .into_iter() - .flat_map(|h| h.join().unwrap()) - .collect() - }); - - let distinct: std::collections::HashSet<_> = roots.iter().collect(); - assert_eq!(distinct.len(), THREADS * EACH, "roots collided"); - } - fn git(dir: &Path, args: &[&str]) { let status = Command::new("git") .arg("-C") diff --git a/crates/voro/src/ui.rs b/crates/voro/src/ui.rs index 4bd0c3b..ef2d8e3 100644 --- a/crates/voro/src/ui.rs +++ b/crates/voro/src/ui.rs @@ -2924,16 +2924,12 @@ mod tests { use ratatui::backend::TestBackend; use voro_core::Store; - let dir = std::env::temp_dir().join(format!( - "voro-ui-config-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let dir = tempfile::Builder::new() + .prefix("voro-ui-config-") + .tempdir() + .unwrap() + .keep(); let agents_path = dir.join("voro.toml"); - std::fs::create_dir_all(&dir).unwrap(); std::fs::write(&agents_path, "[viewers.zed]\ncmd = \"zed {path}\"\n").unwrap(); let store = Store::open_in_memory().unwrap(); @@ -3004,16 +3000,12 @@ mod tests { use ratatui::backend::TestBackend; use voro_core::Store; - let dir = std::env::temp_dir().join(format!( - "voro-ui-config-many-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let dir = tempfile::Builder::new() + .prefix("voro-ui-config-many-") + .tempdir() + .unwrap() + .keep(); let agents_path = dir.join("voro.toml"); - std::fs::create_dir_all(&dir).unwrap(); let mut toml = String::from("[viewers.zed]\ncmd = \"zed {path}\"\n"); for n in 1..=4 { toml.push_str(&format!( @@ -3071,16 +3063,12 @@ mod tests { use ratatui::crossterm::event::{KeyCode, KeyEvent}; use voro_core::Store; - let dir = std::env::temp_dir().join(format!( - "voro-ui-config-scroll-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let dir = tempfile::Builder::new() + .prefix("voro-ui-config-scroll-") + .tempdir() + .unwrap() + .keep(); let agents_path = dir.join("voro.toml"); - std::fs::create_dir_all(&dir).unwrap(); let mut toml = String::from("[viewers.zed]\ncmd = \"zed {path}\"\n"); for n in 1..=6 { toml.push_str(&format!( @@ -3333,15 +3321,11 @@ mod tests { use std::process::{Command, Stdio}; use voro_core::{Action, NewTask, Store}; - let project = std::env::temp_dir().join(format!( - "voro-ui-remoteless-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&project).unwrap(); + let project = tempfile::Builder::new() + .prefix("voro-ui-remoteless-") + .tempdir() + .unwrap() + .keep(); let status = Command::new("git") .arg("-C") .arg(&project) @@ -3415,15 +3399,11 @@ mod tests { use std::process::{Command, Stdio}; use voro_core::{Action, NewTask, Store}; - let project = std::env::temp_dir().join(format!( - "voro-ui-half-report-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&project).unwrap(); + let project = tempfile::Builder::new() + .prefix("voro-ui-half-report-") + .tempdir() + .unwrap() + .keep(); let status = Command::new("git") .arg("-C") .arg(&project) @@ -6159,15 +6139,11 @@ mod tests { use ratatui::crossterm::event::{KeyCode, KeyEvent}; use voro_core::Store; - let dir = std::env::temp_dir().join(format!( - "voro-ui-mouse-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&dir).unwrap(); + let dir = tempfile::Builder::new() + .prefix("voro-ui-mouse-") + .tempdir() + .unwrap() + .keep(); let agents_path = dir.join("voro.toml"); std::fs::write( &agents_path, diff --git a/crates/voro/src/worktree.rs b/crates/voro/src/worktree.rs index d974f20..5a75643 100644 --- a/crates/voro/src/worktree.rs +++ b/crates/voro/src/worktree.rs @@ -232,7 +232,6 @@ fn pr_is_merged(url: &str) -> bool { #[cfg(test)] mod tests { use super::*; - use std::time::{SystemTime, UNIX_EPOCH}; use voro_core::{NewTask, Priority, Store, TaskState}; fn git(dir: &Path, args: &[&str]) { @@ -250,14 +249,11 @@ mod tests { /// A git repo at `/project` with one commit on `main` and a /// `git`-configured identity, ready to grow worktrees. fn repo() -> PathBuf { - let root = std::env::temp_dir().join(format!( - "voro-worktree-{}-{}", - std::process::id(), - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let root = tempfile::Builder::new() + .prefix("voro-worktree-") + .tempdir() + .unwrap() + .keep(); let project = root.join("project"); std::fs::create_dir_all(&project).unwrap(); git(&project, &["init", "-q", "-b", "main"]); diff --git a/crates/voro/tests/propose_ignores_ambient_task_id.rs b/crates/voro/tests/propose_ignores_ambient_task_id.rs index 6773a3a..310d626 100644 --- a/crates/voro/tests/propose_ignores_ambient_task_id.rs +++ b/crates/voro/tests/propose_ignores_ambient_task_id.rs @@ -32,15 +32,11 @@ fn voro(db: &Path, args: &[&str]) -> String { #[test] fn propose_ignores_ambient_voro_task_id() { - let root = std::env::temp_dir().join(format!( - "voro-it-{}-{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&root).unwrap(); + let root = tempfile::Builder::new() + .prefix("voro-it-") + .tempdir() + .unwrap() + .keep(); let db = root.join("voro.db"); voro(&db, &["project", "add", "demo", root.to_str().unwrap()]);