From 6d04be6c625edae32ef5eefdce501467e15546fd Mon Sep 17 00:00:00 2001 From: atlas Date: Tue, 3 Nov 2026 08:11:16 +0100 Subject: [PATCH] hive-priv: test seams are traits, not closure bags --- hive-priv/src/container.rs | 82 +++++++--------- hive-priv/src/shared.rs | 50 +++++++++- hive-priv/src/storage.rs | 194 +++++++++++++++++++++++++------------ 3 files changed, 213 insertions(+), 113 deletions(-) diff --git a/hive-priv/src/container.rs b/hive-priv/src/container.rs index 40ee26e3..5fc7f1a1 100644 --- a/hive-priv/src/container.rs +++ b/hive-priv/src/container.rs @@ -12,18 +12,13 @@ use tokio::io::{AsyncWriteExt, BufReader}; use tokio::net::unix::OwnedWriteHalf; use tokio::process::Command; -use crate::shared::{container_system_name, validate_agent_name}; - -/// The program `program` names, ready for arguments. The one place the real -/// tools are resolved, so a test can substitute stand-ins. -fn real_command(program: &str) -> Command { - Command::new(program) -} +use crate::shared::{RealSpawn, Spawn, container_system_name, validate_agent_name}; /// What the lifecycle ops run and how long they wait for it: the host's /// tools and timings in [`Host::REAL`], stand-ins in tests. -struct Host<'a> { - spawn: &'a (dyn Fn(&str) -> Command + Sync), +struct Host { + /// Starts the tools the ops run. + spawn: S, /// How long to wait for machined to drop a machine's registration after a /// shutdown has been asked for. Generous: a container with slow-stopping /// units legitimately takes a while, and escalating early would SIGKILL a @@ -38,18 +33,18 @@ struct Host<'a> { probe_timeout: std::time::Duration, } -impl Host<'static> { +impl Host { const REAL: Self = Host { - spawn: &real_command, + spawn: RealSpawn, release_timeout: std::time::Duration::from_secs(20), release_poll: std::time::Duration::from_millis(500), probe_timeout: std::time::Duration::from_secs(5), }; } -impl Host<'_> { +impl Host { fn command(&self, program: &str) -> Command { - (self.spawn)(program) + self.spawn.command(program) } } @@ -76,7 +71,7 @@ pub(crate) async fn start_container(name: &str) -> Result<(String, String)> { start_container_with(&Host::REAL, name).await } -async fn start_container_with(host: &Host<'_>, name: &str) -> Result<(String, String)> { +async fn start_container_with(host: &Host, name: &str) -> Result<(String, String)> { validate_agent_name(name)?; let machine = container_system_name(name); // Silenced: a first-time create means the unit doesn't exist yet, and @@ -158,7 +153,7 @@ pub(crate) async fn start_failure_log(name: &str, since_unix: u64) -> Result<(St /// `journalctl` with `argv`: its stdout, if it exits 0 within `limit`. async fn run_journalctl( - host: &Host<'_>, + host: &Host, limit: std::time::Duration, argv: Vec, ) -> Result { @@ -182,7 +177,7 @@ pub(crate) async fn stop_container(name: &str) -> Result<(String, String)> { stop_container_with(&Host::REAL, name).await } -async fn stop_container_with(host: &Host<'_>, name: &str) -> Result<(String, String)> { +async fn stop_container_with(host: &Host, name: &str) -> Result<(String, String)> { validate_agent_name(name)?; stop_and_release(host, &container_system_name(name)).await } @@ -195,7 +190,7 @@ pub(crate) async fn kill_container(name: &str) -> Result<(String, String)> { kill_container_with(&Host::REAL, name).await } -async fn kill_container_with(host: &Host<'_>, name: &str) -> Result<(String, String)> { +async fn kill_container_with(host: &Host, name: &str) -> Result<(String, String)> { validate_agent_name(name)?; let machine = container_system_name(name); machinectl_run(host, &["kill", &machine, "--signal=SIGKILL"]).await @@ -206,7 +201,7 @@ pub(crate) async fn destroy_container(name: &str) -> Result<(String, String)> { destroy_container_with(&Host::REAL, name).await } -async fn destroy_container_with(host: &Host<'_>, name: &str) -> Result<(String, String)> { +async fn destroy_container_with(host: &Host, name: &str) -> Result<(String, String)> { validate_agent_name(name)?; container_run_with(host, &["destroy", &container_system_name(name)]).await } @@ -240,7 +235,7 @@ pub(crate) async fn container_toplevel_action( /// [`container_toplevel_action`] past the check that `toplevel` is the /// closure realised for `name`. async fn apply_toplevel( - host: &Host<'_>, + host: &Host, verb: &str, name: &str, toplevel: &str, @@ -272,7 +267,7 @@ async fn apply_toplevel( /// wedged `systemctl reload`) is exactly when the caller most wants the /// live line in the build log, not just a summary error afterward. async fn swap_container_profile( - host: &Host<'_>, + host: &Host, system_name: &str, toplevel: &str, stream: bool, @@ -448,7 +443,7 @@ pub(crate) async fn container_run(args: &[&str]) -> Result<(String, String)> { container_run_with(&Host::REAL, args).await } -async fn container_run_with(host: &Host<'_>, args: &[&str]) -> Result<(String, String)> { +async fn container_run_with(host: &Host, args: &[&str]) -> Result<(String, String)> { let out = host .command("nixos-container") .args(args) @@ -484,7 +479,7 @@ async fn container_run_with(host: &Host<'_>, args: &[&str]) -> Result<(String, S /// Invoke `machinectl` with the given args, log output to journald. /// Used for operations that nixos-container doesn't expose (e.g. sending /// signals to running containers). -async fn machinectl_run(host: &Host<'_>, args: &[&str]) -> Result<(String, String)> { +async fn machinectl_run(host: &Host, args: &[&str]) -> Result<(String, String)> { let out = host .command("machinectl") .args(args) @@ -525,7 +520,7 @@ async fn machinectl_run(host: &Host<'_>, args: &[&str]) -> Result<(String, Strin /// Errors and timeouts answer "still registered". Being wrong that way costs /// a redundant SIGKILL to something already gone; being wrong the other way /// hands back a stop that silently leaked the name. -async fn machine_registered(host: &Host<'_>, machine: &str) -> bool { +async fn machine_registered(host: &Host, machine: &str) -> bool { let probe = host .command("machinectl") .args(["show", machine, "--property=Name"]) @@ -547,7 +542,7 @@ async fn machine_registered(host: &Host<'_>, machine: &str) -> bool { /// Poll [`machine_registered`] until the name is free or the timeout expires. /// Returns true once the registration is gone. -async fn wait_name_released(host: &Host<'_>, machine: &str) -> bool { +async fn wait_name_released(host: &Host, machine: &str) -> bool { let deadline = tokio::time::Instant::now() + host.release_timeout; loop { if !machine_registered(host, machine).await { @@ -589,7 +584,7 @@ async fn wait_name_released(host: &Host<'_>, machine: &str) -> bool { /// that every stop gets it: dashboard, reconcile, destroy, cold-start /// fallback. The start path already distrusts its own exit code the same way; /// this is the missing half of that pair. -async fn stop_and_release(host: &Host<'_>, machine: &str) -> Result<(String, String)> { +async fn stop_and_release(host: &Host, machine: &str) -> Result<(String, String)> { let stop = container_run_with(host, &["stop", machine]).await; if wait_name_released(host, machine).await { // Happy path, and also the path where a stop that reported failure @@ -620,7 +615,7 @@ async fn stop_and_release(host: &Host<'_>, machine: &str) -> Result<(String, Str /// on success (all output was streamed); the error string includes stderr /// tail on failure. async fn container_run_streaming( - host: &Host<'_>, + host: &Host, args: &[&str], writer: &mut OwnedWriteHalf, ) -> Result<(String, String)> { @@ -862,7 +857,7 @@ mod host_tests { use tokio::net::UnixStream; use super::*; - use crate::shared::test_support::scratch; + use crate::shared::test_support::{MissingTool, ScriptSpawn, scratch}; const TOPLEVEL: &str = "/nix/store/0123456789abcdfghijklmnpqrsvwxyz-nixos-system-bob"; @@ -872,7 +867,7 @@ mod host_tests { /// `$D` is the stub's scratch directory. struct Stub { dir: PathBuf, - spawn: Box Command + Sync>, + script: String, } impl Stub { @@ -882,21 +877,18 @@ mod host_tests { "D={d}\necho \"$0 $*\" >> \"$D/calls\"\n{behave}", d = dir.display() ); - let spawn = move |program: &str| { - let mut cmd = Command::new("sh"); - cmd.arg("-c").arg(&script).arg(program); - cmd - }; - Self { - dir, - spawn: Box::new(spawn), - } + Self { dir, script } } /// Timings short enough for a test that waits them out. - fn host(&self) -> Host<'_> { + fn host(&self) -> Host { + Self::host_over(ScriptSpawn(self.script.clone())) + } + + /// [`Stub::host`]'s timings over another `spawn`. + fn host_over(spawn: S) -> Host { Host { - spawn: &*self.spawn, + spawn, release_timeout: Duration::from_millis(300), release_poll: Duration::from_millis(20), probe_timeout: Duration::from_secs(10), @@ -1104,11 +1096,7 @@ mod host_tests { #[tokio::test] async fn a_missing_tool_is_reported_by_name() { - let gone = |_: &str| Command::new("/nonexistent/hive-priv-test-tool"); - let host = Host { - spawn: &gone, - ..Stub::new("").host() - }; + let host = Stub::host_over(MissingTool); let msg = err_text(container_run_with(&host, &["list"]).await); assert!(msg.contains("invoke nixos-container"), "{msg}"); let msg = err_text(machinectl_run(&host, &["list"]).await); @@ -1216,11 +1204,7 @@ mod host_tests { let free = Stub::new("exit 1"); assert!(!machine_registered(&free.host(), "h-bob").await); - let gone = |_: &str| Command::new("/nonexistent/hive-priv-test-tool"); - let host = Host { - spawn: &gone, - ..free.host() - }; + let host = Stub::host_over(MissingTool); assert!( machine_registered(&host, "h-bob").await, "a probe that cannot run must answer registered" diff --git a/hive-priv/src/shared.rs b/hive-priv/src/shared.rs index a8ae7332..bda35d6d 100644 --- a/hive-priv/src/shared.rs +++ b/hive-priv/src/shared.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context as _, Result, bail}; use hive_priv_sock::AGENT_PREFIX; +use tokio::process::Command; /// Return the system container name for a logical agent name. /// All agents (including the manager) use the `h-` prefix. @@ -210,13 +211,60 @@ impl Drop for StagedFile<'_> { } } -/// Scratch-directory helpers shared by every module's tests. One counter for +/// How an op starts a host program. [`RealSpawn`] in production; tests +/// substitute stand-ins, so no root, container or btrfs filesystem is needed. +pub(crate) trait Spawn { + /// The `program` command, ready for arguments. + fn command(&self, program: &str) -> Command; +} + +/// Starts the program `program` names on `PATH`. +pub(crate) struct RealSpawn; + +impl Spawn for RealSpawn { + fn command(&self, program: &str) -> Command { + Command::new(program) + } +} + +/// Scratch directories and stand-in tools shared by every module's tests. One counter for /// the whole crate, so no two tests get the same directory. #[cfg(test)] pub(crate) mod test_support { use std::path::PathBuf; use std::sync::atomic::{AtomicU32, Ordering}; + use tokio::process::Command; + + use super::Spawn; + + /// Runs every program as `sh -c