diff --git a/engines/lemonade/src/backend_alignment.rs b/engines/lemonade/src/backend_alignment.rs index 16f2d63ea..ca053da37 100644 --- a/engines/lemonade/src/backend_alignment.rs +++ b/engines/lemonade/src/backend_alignment.rs @@ -939,22 +939,11 @@ fn resolve_rocm_llamacpp_backend_dirs_aside( mod tests { use super::*; use crate::install::platform_binary_name; + use crate::test_support::scratch_dir; use anyhow::anyhow; use serde_json::json; use std::fs; - /// A fresh scratch directory under the crate's `target/`. The base is - /// `CARGO_MANIFEST_DIR`, a compile-time constant, so the path never derives from a - /// runtime environment read. - fn scratch_dir(tag: &str) -> PathBuf { - let dir = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("target") - .join(format!("lemonade-fs-test-{tag}")); - let _ = fs::remove_dir_all(&dir); - fs::create_dir_all(&dir).unwrap(); - dir - } - #[test] fn backend_install_succeeds_without_retry() { let mut attempts = 0; diff --git a/engines/lemonade/src/direct_llama.rs b/engines/lemonade/src/direct_llama.rs index 79721f529..44bc6a482 100644 --- a/engines/lemonade/src/direct_llama.rs +++ b/engines/lemonade/src/direct_llama.rs @@ -484,20 +484,9 @@ fn filter_ggufs_by_variant(mut ggufs: Vec, variant: Option<&str>) -> Ve mod tests { use super::*; use crate::LLAMACPP_RECIPE; + use crate::test_support::scratch_dir; use std::fs; - /// A fresh scratch directory under the crate's `target/`. The base is - /// `CARGO_MANIFEST_DIR`, a compile-time constant, so the path never derives from a - /// runtime environment read. - fn scratch_dir(tag: &str) -> PathBuf { - let dir = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("target") - .join(format!("lemonade-fs-test-{tag}")); - let _ = fs::remove_dir_all(&dir); - fs::create_dir_all(&dir).unwrap(); - dir - } - #[test] fn parses_canonical_hugging_face_checkpoints() { assert_eq!( diff --git a/engines/lemonade/src/install.rs b/engines/lemonade/src/install.rs index e03b7e8cd..34f6f30e2 100644 --- a/engines/lemonade/src/install.rs +++ b/engines/lemonade/src/install.rs @@ -952,22 +952,11 @@ fn copy_tree_entries( #[cfg(test)] mod tests { use super::*; + use crate::test_support::scratch_dir; use std::io::Write; use std::process::{Command as ProcessCommand, Stdio}; use std::time::Duration; - /// A fresh scratch directory under the crate's `target/`. The base is - /// `CARGO_MANIFEST_DIR`, a compile-time constant, so the path never derives from a - /// runtime environment read. - fn scratch_dir(tag: &str) -> PathBuf { - let dir = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("target") - .join(format!("lemonade-fs-test-{tag}")); - let _ = fs::remove_dir_all(&dir); - fs::create_dir_all(&dir).unwrap(); - dir - } - #[test] fn embeddable_package_matches_runtime_os() { let version = rocm_deps::LEMONADE_VERSION; diff --git a/engines/lemonade/src/lib.rs b/engines/lemonade/src/lib.rs index a51711143..20a74b447 100644 --- a/engines/lemonade/src/lib.rs +++ b/engines/lemonade/src/lib.rs @@ -24,6 +24,8 @@ mod install; mod process; mod runtime_dir; mod state; +#[cfg(test)] +mod test_support; // Re-exported so this stays reachable at its pre-split crate-root path — // `backend_alignment` is a private module, but this constant was `pub` at diff --git a/engines/lemonade/src/state.rs b/engines/lemonade/src/state.rs index 55c49cab5..e6ae73cbc 100644 --- a/engines/lemonade/src/state.rs +++ b/engines/lemonade/src/state.rs @@ -542,21 +542,10 @@ pub(crate) fn manifest_lock_hash(manifest: &LemonadeInstallManifest) -> String { #[cfg(test)] mod tests { use super::*; + use crate::test_support::scratch_dir; use std::io::{Read, Write}; use std::time::Duration; - /// A fresh scratch directory under the crate's `target/`. The base is - /// `CARGO_MANIFEST_DIR`, a compile-time constant, so the path never derives from a - /// runtime environment read. - fn scratch_dir(tag: &str) -> PathBuf { - let dir = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("target") - .join(format!("lemonade-fs-test-{tag}")); - let _ = fs::remove_dir_all(&dir); - fs::create_dir_all(&dir).unwrap(); - dir - } - #[test] fn identity_from_state_prefers_server_pid_and_its_start_ticks() { let state = json!({ diff --git a/engines/lemonade/src/test_support.rs b/engines/lemonade/src/test_support.rs new file mode 100644 index 000000000..e51555bf7 --- /dev/null +++ b/engines/lemonade/src/test_support.rs @@ -0,0 +1,72 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Shared test-only fixtures for `rocm-engine-lemonade`'s module test suites. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; + +/// A fresh, empty scratch directory under the crate's `target/`, named +/// `lemonade-fs-test----`. +/// +/// The base is `CARGO_MANIFEST_DIR`, a compile-time constant, so the path never +/// derives from a runtime environment read. The tag keeps the name readable; +/// it does not keep it unique. A tag reused by two tests running on parallel +/// threads, or one test running in two `cargo test` processes against the same +/// checkout, would otherwise share the directory, and each call wipes it +/// first, so one run would delete the other's files. The pid separates +/// processes and the per-process counter separates calls within one, so no two +/// calls agree whatever the tag. +pub(crate) fn scratch_dir(tag: &str) -> PathBuf { + static NEXT: AtomicU64 = AtomicU64::new(0); + let dir = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join(format!( + "lemonade-fs-test-{tag}-{}-{}-{}", + std::process::id(), + rocm_core::unix_time_millis(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + dir +} + +mod tests { + use super::*; + + /// Concurrent rather than sequential, because that is how tests call this + /// helper: calls made one after another can let the clock tick between + /// them and pass by luck, while threads released together all read the + /// same millisecond, so a name keyed on pid and time alone repeats. + #[test] + fn scratch_dir_gives_each_call_its_own_directory_even_for_one_tag() { + const CALLERS: usize = 16; + let start = std::sync::Barrier::new(CALLERS); + let dirs: Vec = std::thread::scope(|scope| { + let callers: Vec<_> = (0..CALLERS) + .map(|_| { + scope.spawn(|| { + start.wait(); + scratch_dir("same-tag") + }) + }) + .collect(); + callers + .into_iter() + .map(|caller| caller.join().expect("a caller panicked")) + .collect() + }); + for dir in &dirs { + let _ = fs::remove_dir_all(dir); + } + let distinct: std::collections::HashSet<_> = dirs.iter().collect(); + assert_eq!( + distinct.len(), + CALLERS, + "two calls with one tag must not share a directory" + ); + } +} diff --git a/engines/vllm/src/state.rs b/engines/vllm/src/state.rs index 454c8a402..fbf647dd0 100644 --- a/engines/vllm/src/state.rs +++ b/engines/vllm/src/state.rs @@ -345,19 +345,42 @@ mod tests { NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed) )) } - #[test] - fn probe_state_path_gives_each_call_its_own_file_even_for_one_tag() { - // Many calls rather than two: without a counter the names differ only - // when the clock ticks between calls, so a pair can pass by luck. - let paths: Vec = (0..64).map(|_| probe_state_path("same-tag")).collect(); + /// Call `make_path` from many threads at once and assert no two calls + /// returned the same path. + /// + /// Concurrent rather than sequential, because that is how tests call these + /// helpers: calls made one after another let the clock tick between them + /// and can pass by luck, while threads released together all read the same + /// millisecond, so a name keyed on pid and time alone repeats. + fn assert_each_call_gets_its_own_path(make_path: impl Fn() -> PathBuf + Sync) { + const CALLERS: usize = 16; + let start = std::sync::Barrier::new(CALLERS); + let paths: Vec = std::thread::scope(|scope| { + let callers: Vec<_> = (0..CALLERS) + .map(|_| { + scope.spawn(|| { + start.wait(); + make_path() + }) + }) + .collect(); + callers + .into_iter() + .map(|caller| caller.join().expect("a caller panicked")) + .collect() + }); let distinct: std::collections::HashSet<_> = paths.iter().collect(); assert_eq!( distinct.len(), - paths.len(), - "two calls with one tag shared a path" + CALLERS, + "two calls with one tag must not share a path" ); } #[test] + fn probe_state_path_gives_each_call_its_own_file_even_for_one_tag() { + assert_each_call_gets_its_own_path(|| probe_state_path("same-tag")); + } + #[test] fn inference_verification_latches_into_the_state_file() -> Result<()> { // First check probes and records the verdict; the second reads the latch // and leaves the model alone.