diff --git a/src/controller.rs b/src/controller.rs index 5fcd0ce..3077cbe 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -1078,7 +1078,7 @@ mod tests { }; use async_trait::async_trait; - use test_log::test; + use rstest::rstest; use super::*; use crate::{ @@ -1086,6 +1086,8 @@ mod tests { engines::{EngineTickContext, ScalingEngine}, instance::{InstanceClient, ThreadPoolSnapshot}, per_vm_log::PerVmLog, + test::MockDir, + test::mockdir::mockdir, }; use tracing_subscriber::layer::SubscriberExt; @@ -1142,11 +1144,11 @@ mod tests { /// Test that actuation will not raise the pool above the VM vCPU /// count. - #[test(tokio::test)] - async fn actuation_enforces_vcpu_cap() { - let state_dir = tempfile::tempdir().unwrap(); + #[rstest] + #[tokio::test] + async fn actuation_enforces_vcpu_cap(#[from(mockdir)] _mockdir: MockDir) { let cfg = Config { - vm_state_path: PrefixedPath::new(&state_dir.path().join("ownership.json")), + vm_state_path: PrefixedPath::new("/ownership.json"), cooldown_secs: 0, ..Default::default() }; @@ -1205,11 +1207,13 @@ mod tests { // different things. /// Test that actuation clamps thread targets to configured min/max /// and blocks scale-up when host CPU is above the ceiling. - #[test(tokio::test)] - async fn actuation_enforces_controller_bounds_and_host_ceiling() { - let state_dir = tempfile::tempdir().unwrap(); + #[rstest] + #[tokio::test] + async fn actuation_enforces_controller_bounds_and_host_ceiling( + #[from(mockdir)] _mockdir: MockDir, + ) { let cfg = Config { - vm_state_path: PrefixedPath::new(&state_dir.path().join("ownership.json")), + vm_state_path: PrefixedPath::new("/ownership.json"), min_thread_count: 2, max_thread_count: 3, host_cpu_scale_up_ceiling: 0.5, @@ -1275,11 +1279,11 @@ mod tests { /// Test that a performance-validation revert arms cooldown so the next /// ordinary up/down cannot immediately undo it (the 4→3→4 flap). #[cfg(feature = "threshold-engine")] - #[test(tokio::test)] - async fn revert_arms_cooldown_blocking_immediate_retry() { - let state_dir = tempfile::tempdir().unwrap(); + #[rstest] + #[tokio::test] + async fn revert_arms_cooldown_blocking_immediate_retry(#[from(mockdir)] _mockdir: MockDir) { let cfg = Config { - vm_state_path: PrefixedPath::new(&state_dir.path().join("ownership.json")), + vm_state_path: PrefixedPath::new("/ownership.json"), cooldown_secs: 30, min_thread_count: 1, max_thread_count: 8, @@ -1332,11 +1336,11 @@ mod tests { /// Test that revert itself is not blocked by an existing cooldown, so a /// failed validation can still roll the pool back. #[cfg(feature = "threshold-engine")] - #[test(tokio::test)] - async fn revert_bypasses_existing_cooldown() { - let state_dir = tempfile::tempdir().unwrap(); + #[rstest] + #[tokio::test] + async fn revert_bypasses_existing_cooldown(#[from(mockdir)] _mockdir: MockDir) { let cfg = Config { - vm_state_path: PrefixedPath::new(&state_dir.path().join("ownership.json")), + vm_state_path: PrefixedPath::new("/ownership.json"), cooldown_secs: 60, min_thread_count: 1, max_thread_count: 8, @@ -1409,11 +1413,13 @@ mod tests { /// Test that `set_all` applies a target to every VM and reports how /// many failed when some clients error. + #[rstest] #[tokio::test] - async fn test_set_all_attempts_every_vm_and_reports_partial_failure() { - let state_dir = tempfile::tempdir().unwrap(); + async fn test_set_all_attempts_every_vm_and_reports_partial_failure( + #[from(mockdir)] _mockdir: MockDir, + ) { let cfg = Config { - vm_state_path: PrefixedPath::new(&state_dir.path().join("ownership.json")), + vm_state_path: PrefixedPath::new("/ownership.json"), ..Default::default() }; let engine = Box::new(default_threshold_engine()); @@ -1566,14 +1572,14 @@ mod tests { /// Test that discovery sync adds new VMs, retains existing ones, /// and removes disappeared ones. + #[rstest] #[tokio::test] - async fn sync_instances_adds_retains_and_removes() { + async fn sync_instances_adds_retains_and_removes(#[from(mockdir)] _mockdir: MockDir) { let added = Arc::new(AtomicUsize::new(0)); let removed = Arc::new(AtomicUsize::new(0)); let evaluated = Arc::new(AtomicUsize::new(0)); - let state_dir = tempfile::tempdir().unwrap(); let cfg = Config { - vm_state_path: PrefixedPath::new(&state_dir.path().join("ownership.json")), + vm_state_path: PrefixedPath::new("/ownership.json"), ..Default::default() }; let mut controller = Controller::new( @@ -1643,14 +1649,16 @@ mod tests { /// Test that one tick refreshes state, runs the engine, and drops /// instances that fail refresh. + #[rstest] #[tokio::test] - async fn tick_refreshes_evaluates_and_drops_failed_instances() { + async fn tick_refreshes_evaluates_and_drops_failed_instances( + #[from(mockdir)] _mockdir: MockDir, + ) { let added = Arc::new(AtomicUsize::new(0)); let removed = Arc::new(AtomicUsize::new(0)); let evaluated = Arc::new(AtomicUsize::new(0)); - let state_dir = tempfile::tempdir().unwrap(); let cfg = Config { - vm_state_path: PrefixedPath::new(&state_dir.path().join("ownership.json")), + vm_state_path: PrefixedPath::new("/ownership.json"), ..Default::default() }; let mut controller = Controller::new( diff --git a/src/lib.rs b/src/lib.rs index 395cf68..6fc09e8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,8 @@ pub mod path; pub mod per_vm_log; pub mod rolling; pub mod state; +#[cfg(test)] +pub(crate) mod test; pub(crate) trait ProcessExt { fn from_pid(pid: i32) -> procfs::ProcResult; diff --git a/src/path.rs b/src/path.rs index 6ebeb3f..ac8a955 100644 --- a/src/path.rs +++ b/src/path.rs @@ -69,12 +69,27 @@ impl fmt::Display for PrefixedPath { } } -static IO_THREAD_CONTROLLER_ROOT_PATH: LazyLock = LazyLock::new(|| { +static ROOT_PATH: LazyLock = LazyLock::new(|| { std::path::PathBuf::from( env::var("IO_THREAD_CONTROLLER_ROOT_PATH").unwrap_or(String::from("/")), ) }); +/// Returns the path new `util::Path`s will have prefixed. In non-test +/// configurations, this will be either `/` or the contents of the +/// `IO_THREAD_CONTROLLER_ROOT_PATH` env var. +/// +/// In test configurations, this may differ per thread to support concurrently +/// running unit tests. +pub(crate) fn effective_root() -> std::path::PathBuf { + #[cfg(test)] + if let Some(root) = crate::test::mockdir::ROOT_OVERRIDE.with(|slot| slot.borrow().clone()) { + return root; + } + + ROOT_PATH.clone() +} + impl PrefixedPath { pub fn new(s: &S) -> Self where @@ -82,7 +97,7 @@ impl PrefixedPath { { let mut pb = std::path::PathBuf::from(s); if pb.is_absolute() { - pb = IO_THREAD_CONTROLLER_ROOT_PATH.join(pb.strip_prefix("/").expect("absolute path")) + pb = effective_root().join(pb.strip_prefix("/").expect("absolute path")) } Self { path: pb } } diff --git a/src/per_vm_log.rs b/src/per_vm_log.rs index eb54778..b49f311 100644 --- a/src/per_vm_log.rs +++ b/src/per_vm_log.rs @@ -169,17 +169,19 @@ impl Visit for Record { #[cfg(test)] mod tests { + use rstest::rstest; use tracing_subscriber::{filter::LevelFilter, layer::SubscriberExt}; + use crate::test::{MockDir, mockdir::mockdir}; + use super::*; /// Test that only `vm`-tagged events reach the per-VM file, rendered /// as message then fields and without the `vm` field. - #[test] - fn test_writes_vm_tagged_events_without_vm_field() { - let dir = tempfile::tempdir().unwrap(); + #[rstest] + fn test_writes_vm_tagged_events_without_vm_field(#[from(mockdir)] _mockdir: MockDir) { let log = PerVmLog::default(); - log.set_dir(Some(PrefixedPath::new(dir.path()))); + log.set_dir(Some(PrefixedPath::new("/"))); let subscriber = tracing_subscriber::registry().with(log.clone()); tracing::subscriber::with_default(subscriber, || { tracing::info!(target: VM, vm = "vm-a", "status=1 2 3"); @@ -201,7 +203,10 @@ mod tests { "event=vm_added" ] ); - assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 1); + assert_eq!( + std::fs::read_dir(PrefixedPath::new("/")).unwrap().count(), + 1 + ); } #[derive(Clone, Default)] @@ -220,11 +225,10 @@ mod tests { /// Test that per-VM-only events stay out of the main log, and that the /// main log's level does not drop them from per-VM files. - #[test] - fn test_per_vm_target_bypasses_main_log() { - let dir = tempfile::tempdir().unwrap(); + #[rstest] + fn test_per_vm_target_bypasses_main_log(#[from(mockdir)] _mockdir: MockDir) { let log = PerVmLog::default(); - log.set_dir(Some(PrefixedPath::new(dir.path()))); + log.set_dir(Some(PrefixedPath::new("/"))); let main_log = Buffer::default(); let writer = main_log.clone(); let subscriber = tracing_subscriber::registry() @@ -247,15 +251,17 @@ mod tests { } /// Test that nothing is written while per-VM files are disabled. - #[test] - fn test_disabled_writes_nothing() { - let dir = tempfile::tempdir().unwrap(); + #[rstest] + fn test_disabled_writes_nothing(#[from(mockdir)] _mockdir: MockDir) { let log = PerVmLog::default(); let subscriber = tracing_subscriber::registry().with(log.clone()); tracing::subscriber::with_default(subscriber, || { tracing::info!(target: VM, vm = "vm-a", "status=1"); }); assert!(log.path("vm-a").is_none()); - assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 0); + assert_eq!( + std::fs::read_dir(PrefixedPath::new("/")).unwrap().count(), + 0 + ); } } diff --git a/src/test/mockdir.rs b/src/test/mockdir.rs new file mode 100644 index 0000000..9ae7f5d --- /dev/null +++ b/src/test/mockdir.rs @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2026 Nutanix, Inc. +// +// Author: Leonardo Forchini +// +//! `tempfile::TempDir` wrapper which controls the prefix of `util::Path`s. + +use std::{cell::RefCell, path::PathBuf}; + +use rstest::fixture; + +thread_local! { + pub static ROOT_OVERRIDE: RefCell> = const { RefCell::new(None) }; +} + +/// Scoped root override for tests that access absolute paths through +/// `util::Path`. +pub(crate) struct RootOverride { + previous: Option, +} + +impl Drop for RootOverride { + fn drop(&mut self) { + ROOT_OVERRIDE.with(|root| { + root.replace(self.previous.take()); + }); + } +} + +/// Override the root used by `util::Path` on the current test thread. +pub(crate) fn override_root(root: impl Into) -> RootOverride { + let previous = ROOT_OVERRIDE.with(|slot| slot.replace(Some(root.into()))); + RootOverride { previous } +} + +/// Temporary mock root installed as the current test thread's `util::Path` +/// prefix. +pub(crate) struct MockDir { + path: std::path::PathBuf, + _root_override: RootOverride, + _dir: tempfile::TempDir, +} + +impl MockDir { + pub(crate) fn new() -> Self { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path(); + + Self { + path: path.to_path_buf(), + _root_override: override_root(path), + _dir: dir, + } + } +} + +/// Create a new `MockDir` as an `rstest` fixture +#[fixture] +pub(crate) fn mockdir() -> MockDir { + MockDir::new() +} + +#[cfg(test)] +mod tests { + use std::path::Path; + + use rstest::rstest; + + use super::*; + use crate::path::{PrefixedPath, effective_root}; + + /// Ensure that a new `MockDir` actually changes the effective root. + #[test] + fn test_mockdir_changes_effective_root() { + let old_root = effective_root(); + + let _mockdir = MockDir::new(); + assert_ne!(old_root, effective_root()); + } + + /// Ensure new `util::Paths`s use a given `MockDir`'s prefix. + #[rstest] + fn test_mockdir_modifies_util_paths(mockdir: MockDir) { + let util_path = PrefixedPath::from("/etc/passwd"); + assert_ne!(mockdir.path, Path::new("/")); + assert_eq!(util_path.to_path_buf(), mockdir.path.join("etc/passwd")); + } + + /// Ensure paths created before a new `MockDir` are not prefixed with its + /// dir. + // TODO is this the behaviour we want? MockDirs will mostly be used as + // fixtures so I don't think it should come up too often. + #[test] + fn test_mockdir_preserves_existing_paths() { + let util_path = PrefixedPath::from("/etc/passwd"); + + let mockdir = MockDir::new(); + assert_ne!(util_path.to_path_buf(), mockdir.path.join("etc/passwd")); + } + + /// Ensure once a `MockDir` goes out of scope the effective root is + /// restored. + #[test] + fn test_mockdir_reverts_on_drop() { + let _outer = MockDir::new(); + let outer_path = effective_root(); + + let inner_path = { + let _inner = MockDir::new(); + effective_root() + }; + + let post_drop_path = effective_root(); + + assert_ne!(outer_path, inner_path); + assert_eq!(outer_path, post_drop_path); + } +} diff --git a/src/test/mod.rs b/src/test/mod.rs new file mode 100644 index 0000000..4cbd45a --- /dev/null +++ b/src/test/mod.rs @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2026 Nutanix, Inc. +// +// Author: Leonardo Forchini +// + +pub mod mockdir; + +pub use mockdir::*;