tests use mockall for expectation-style doubles, starting with the builder and hive-priv seams

Cargo.lock

@@ -1171,6 +1171,12 @@ dependencies = [
  "syn 2.0.119",
 ]
 
+[[package]]
+name = "downcast"
+version = "0.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1435fa1053d8b2fbbe9be7e97eca7f33d37b28409959813daefc1446a14247f1"
+
 [[package]]
 name = "dunce"
 version = "1.0.5"
@@ -1421,6 +1427,15 @@ dependencies = [
  "percent-encoding",
 ]
 
+[[package]]
+name = "fragile"
+version = "2.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8878864ba14bb86e818a412bfd6f18f9eabd4ec0f008a28e8f7eb61db532fcf9"
+dependencies = [
+ "futures-core",
+]
+
 [[package]]
 name = "fs_extra"
 version = "1.3.0"
@@ -1989,6 +2004,7 @@ dependencies = [
  "anyhow",
  "hive-priv-sock",
  "libc",
+ "mockall",
  "rustix",
  "serde_json",
  "tokio",
@@ -3143,6 +3159,32 @@ dependencies = [
  "windows-sys 0.61.2",
 ]
 
+[[package]]
+name = "mockall"
+version = "0.15.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1a6ceddfe3ce334925e96bf420fdb2dcee5bed6c632a168ece622676dadeaf8a"
+dependencies = [
+ "cfg-if",
+ "downcast",
+ "fragile",
+ "mockall_derive",
+ "predicates",
+ "predicates-tree",
+]
+
+[[package]]
+name = "mockall_derive"
+version = "0.15.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9cfe16fbe8a314aeec0b861ac24e60b1e123e97634bab045475b9d6a18416fd8"
+dependencies = [
+ "cfg-if",
+ "proc-macro2",
+ "quote",
+ "syn 2.0.119",
+]
+
 [[package]]
 name = "nats-jwt"
 version = "0.3.0"
@@ -3550,6 +3592,32 @@ version = "0.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c"
 
+[[package]]
+name = "predicates"
+version = "3.1.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ada8f2932f28a27ee7b70dd6c1c39ea0675c55a36879ab92f3a715eaa1e63cfe"
+dependencies = [
+ "anstyle",
+ "predicates-core",
+]
+
+[[package]]
+name = "predicates-core"
+version = "1.0.10"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cad38746f3166b4031b1a0d39ad9f954dd291e7854fcc0eed52ee41a0b50d144"
+
+[[package]]
+name = "predicates-tree"
+version = "1.0.13"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d0de1b847b39c8131db0467e9df1ff60e6d0562ab8e9a16e568ad0fdb372e2f2"
+dependencies = [
+ "predicates-core",
+ "termtree",
+]
+
 [[package]]
 name = "problem_details"
 version = "0.9.0"
@@ -4861,6 +4929,7 @@ dependencies = [
  "async-nats",
  "futures-util",
  "hive-log",
+ "mockall",
  "serde_json",
  "swarm-queue-client",
  "swarm-secret-client",
@@ -5304,6 +5373,12 @@ dependencies = [
  "new_debug_unreachable",
 ]
 
+[[package]]
+name = "termtree"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683"
+
 [[package]]
 name = "thiserror"
 version = "1.0.69"

Cargo.toml

@@ -287,3 +287,8 @@ utoipa-axum = "0.2"
 # re-issue it at half its life. `der` and `spki` are already in the tree
 # through `pkcs8`.
 x509-cert = { version = "0.2", default-features = false, features = ["pem"] }
+# Test doubles with expectations (canned replies, call counts, argument
+# matchers). Dev-dependency only, and used behind `#[cfg(test)]`, so release
+# builds never compile it. See "Testing seams and test doubles" in
+# `docs/process/conventions.md`.
+mockall = "0.15"

docs/process/conventions.md

@@ -521,6 +521,27 @@ gate locally: `nix flake check` (shares the build farm, use
 sparingly) or build just the suspect check, for example `nix build
 .#checks.x86_64-linux.hivectl-docs`.
 
+## Testing seams and test doubles
+
+- **A new test seam is a trait, used generically** (`impl Seam` or a type
+  parameter). No `dyn Fn` closure bags: a trait names each question the code
+  asks and gives the test one place to answer it.
+- **A double that only answers and records uses `mockall`** (a
+  dev-dependency): put `#[cfg_attr(test, mockall::automock)]` on the trait,
+  set canned replies with `returning`, and assert calls with `times` and
+  `withf`. The attribute keeps `mockall` out of release builds. Examples:
+  `Probes` in `hive-priv/src/storage.rs`, `Publisher` and `Tokens` in
+  `swarm-builder/src/main.rs`.
+- **`automock` can't mock a trait with a supertrait** such as `Clone`: a mock
+  doesn't implement it. Declare the mock with `mockall::mock!` as a plain
+  struct and implement the trait for `Arc<Mock…>`, as `swarm-builder` does.
+  Better, leave the supertrait off the trait and bound the callers.
+- **Stateful fakes stay hand-written**: in-memory stores whose reads see earlier
+  writes. Real-process stubs such as `ScriptSpawn` also stay, since they run a
+  real `sh` on purpose.
+- **Nothing migrates in one sweep.** Move an existing expectation-style fake
+  to `mockall` when you touch its file for another reason.
+
 ## Best-effort oneshot services
 
 The harness ships a family of one-shot systemd services that

hive-priv/Cargo.toml

@@ -16,3 +16,6 @@ serde_json.workspace = true
 tokio.workspace = true
 tracing.workspace = true
 tracing-subscriber.workspace = true
+
+[dev-dependencies]
+mockall.workspace = true

hive-priv/src/storage.rs

@@ -179,6 +179,7 @@ fn is_subvolume_root(on_btrfs: bool, ino: Option<u64>) -> bool {
 
 /// The two questions the storage ops ask the state filesystem. [`RealProbes`]
 /// is production; tests answer without a btrfs filesystem.
+#[cfg_attr(test, mockall::automock)]
 trait Probes {
     /// Whether `path` lives on a btrfs filesystem; an error when it cannot be
     /// told.
@@ -1009,7 +1010,7 @@ async fn set_subvolume_quota(
 #[cfg(test)]
 mod tests {
     use super::{
-        Command, Env, Probes, agent_disk_paths, cleanup_stale_path, delete_agent_snapshot,
+        Command, Env, MockProbes, agent_disk_paths, cleanup_stale_path, delete_agent_snapshot,
         delete_agent_subvolume, du_command, du_total, ensure_agent_subvolume,
         ensure_btrfs_quota_in, exec_delete_agent_snapshot, exec_delete_agent_subvolume,
         exec_ensure_agent_subvolume, exec_measure_agent_disk, exec_read_subvolume_usage,
@@ -1412,21 +1413,31 @@ mod tests {
                 .collect()
         }
 
-        /// An [`Env`] over this tree, running `btrfs` and the two probes given.
+        /// An [`Env`] over this tree, running `btrfs` and probes that answer
+        /// with the functions given.
         fn env<'a, S: Spawn>(
             &'a self,
             spawn: &'a S,
             on_btrfs: fn(&Path) -> anyhow::Result<bool>,
             is_subvolume: fn(&Path) -> bool,
-        ) -> Env<'a, &'a S, FnProbes> {
+        ) -> Env<'a, &'a S, MockProbes> {
+            let mut probes = MockProbes::new();
+            probes.expect_on_btrfs().returning(on_btrfs);
+            probes.expect_is_subvolume().returning(is_subvolume);
+            self.env_with(spawn, probes)
+        }
+
+        /// An [`Env`] over this tree, running `btrfs` and the `probes` given.
+        fn env_with<'a, S: Spawn>(
+            &'a self,
+            spawn: &'a S,
+            probes: MockProbes,
+        ) -> Env<'a, &'a S, MockProbes> {
             Env {
                 state_root: &self.state,
                 staging_root: &self.staging,
                 spawn,
-                probes: FnProbes {
+                probes,
-                    on_btrfs,
-                    is_subvolume,
-                },
             }
         }
 
@@ -1440,22 +1451,6 @@ mod tests {
         }
     }
 
-    /// Probes answered by the functions given.
-    struct FnProbes {
-        on_btrfs: fn(&Path) -> anyhow::Result<bool>,
-        is_subvolume: fn(&Path) -> bool,
-    }
-
-    impl Probes for FnProbes {
-        fn on_btrfs(&self, path: &Path) -> anyhow::Result<bool> {
-            (self.on_btrfs)(path)
-        }
-
-        fn is_subvolume(&self, path: &Path) -> bool {
-            (self.is_subvolume)(path)
-        }
-    }
-
     /// Creates, deletes and snapshots as btrfs does; `send` writes `STREAM`.
     const LIKE_BTRFS: &str = r#"case "$1 $2" in
   "subvolume create") mkdir "$3";;
@@ -1490,7 +1485,8 @@ esac"#;
         let fx = Fx::new();
         std::fs::create_dir(fx.agent("iris")).unwrap();
         let btrfs = fx.btrfs(LIKE_BTRFS);
-        let env = fx.env(&btrfs, on_btrfs, not_subvol);
+        // No expectation is set: a probe call would panic.
+        let env = fx.env_with(&btrfs, MockProbes::new());
         assert_eq!(
             ensure_agent_subvolume(&env, "iris").await.unwrap(),
             (String::new(), String::new())

swarm-builder/Cargo.toml

@@ -26,6 +26,7 @@ tracing.workspace = true
 # `test-util` for `#[tokio::test(start_paused = true)]`: the slot tests wait
 # out the build slot's wait limit on paused time.
 [dev-dependencies]
+mockall.workspace = true
 tokio = { workspace = true, features = ["test-util"] }
 tracing-subscriber.workspace = true
 

swarm-builder/src/main.rs

@@ -177,7 +177,7 @@ trait Publisher: Clone + Send + Sync + 'static {
 }
 
 /// The secrets a request reads from the store, once per request. [`Store`] in
-/// production; tests answer from memory.
+/// production; tests answer with canned replies.
 trait Tokens: Clone + Send + Sync + 'static {
     /// The forge token config repos are fetched with.
     fn forge_token(&self) -> impl Future<Output = Result<String>> + Send;
@@ -970,57 +970,79 @@ mod tests {
         Read::Stuck
     }
 
-    #[derive(Clone)]
+    // `Publisher` and `Tokens` have `Clone` as a supertrait, which `automock`
-    struct FakeTokens {
+    // cannot express (a mock is not `Clone`), so the two mocks are declared by
-        forge: Read,
+    // hand with `mock!` as plain structs. The traits are implemented for
-        push: Read,
+    // `Arc<Mock…>`: every clone the daemon makes shares one set of
-    }
+    // expectations, which are checked when the last clone drops.
-
+    mockall::mock! {
-    impl Tokens for FakeTokens {
+        Publisher {
-        async fn forge_token(&self) -> Result<String> {
+            fn publish(
-            self.forge.get().await
+                &self,
-        }
+                subject: Subject,
-
+                body: Vec<u8>,
-        async fn push_token(&self) -> Result<String> {
+            ) -> impl Future<Output = Result<(), String>> + Send;
-            self.push.get().await
         }
     }
 
-    /// A publisher whose first `failing` publishes fail; the rest are recorded.
+    impl Publisher for Arc<MockPublisher> {
-    #[derive(Clone)]
+        fn publish(
-    struct Sink {
+            &self,
-        sent: tokio::sync::mpsc::UnboundedSender<(String, Value)>,
+            subject: Subject,
-        attempts: Arc<std::sync::atomic::AtomicUsize>,
+            body: Vec<u8>,
-        failing: usize,
+        ) -> impl Future<Output = Result<(), String>> + Send {
+            MockPublisher::publish(self, subject, body)
+        }
     }
 
-    impl Publisher for Sink {
+    mockall::mock! {
-        async fn publish(&self, subject: Subject, body: Vec<u8>) -> Result<(), String> {
+        Tokens {
-            if self.attempts.fetch_add(1, Ordering::SeqCst) < self.failing {
+            fn forge_token(&self) -> impl Future<Output = Result<String>> + Send;
-                return Err("the connection is closed".to_owned());
+            fn push_token(&self) -> impl Future<Output = Result<String>> + Send;
-            }
+        }
-            let body = serde_json::from_slice(&body).expect("a JSON reply");
+    }
-            self.sent
+
-                .send((subject.to_string(), body))
+    impl Tokens for Arc<MockTokens> {
-                .expect("test is listening");
+        fn forge_token(&self) -> impl Future<Output = Result<String>> + Send {
-            Ok(())
+            MockTokens::forge_token(self)
+        }
+
+        fn push_token(&self) -> impl Future<Output = Result<String>> + Send {
+            MockTokens::push_token(self)
         }
     }
 
     struct Rig {
-        sink: Sink,
+        sink: Arc<MockPublisher>,
-        tokens: FakeTokens,
+        tokens: Arc<MockTokens>,
     }
 
+    /// A rig whose token reads answer `forge` and `push`, and whose first
+    /// `failing` publishes fail; the rest are sent to the returned receiver.
     fn rig_with(forge: Read, push: Read, failing: usize) -> (Rig, Sent) {
         let (sent, rx) = tokio::sync::mpsc::unbounded_channel();
-        let sink = Sink {
+        let mut sink = MockPublisher::new();
-            sent,
+        sink.expect_publish()
-            attempts: Arc::default(),
+            .times(failing)
-            failing,
+            .returning(|_, _| Box::pin(async { Err("the connection is closed".to_owned()) }));
+        sink.expect_publish().returning(move |subject, body| {
+            let body = serde_json::from_slice(&body).expect("a JSON reply");
+            sent.send((subject.to_string(), body))
+                .expect("test is listening");
+            Box::pin(async { Ok(()) })
+        });
+        let mut tokens = MockTokens::new();
+        tokens
+            .expect_forge_token()
+            .returning(move || Box::pin(forge.get()));
+        tokens
+            .expect_push_token()
+            .returning(move || Box::pin(push.get()));
+        let rig = Rig {
+            sink: Arc::new(sink),
+            tokens: Arc::new(tokens),
         };
-        let tokens = FakeTokens { forge, push };
+        (rig, rx)
-        (Rig { sink, tokens }, rx)
     }
 
     fn env() -> (Rig, Sent) {