From 1de96027a1369e21e624e0caae262932e3dfefcf Mon Sep 17 00:00:00 2001
From: Ian Letourneau <ian@noma.to>
Date: Mon, 11 Aug 2025 23:42:08 +0000
Subject: [PATCH] fix: prevent instrumentation to run in test mode (#102)

The CI pipeline (`./check.sh`) was failing because of test errors, which was caused by the instrumentation framework complaining that no subscribers/listeners were registered.

Instead of setting up all tests to run with a dummy subscriber, move the implementation of the instrumentation behind a feature flag so that it runs only for tests.

There's a catch though: the `#[cfg(test)]` directive works only when directly testing the crate. If a crate `A` depends on another crate `B`, `B` will be compiled as usual (aka not in test mode) which will not trigger the `test` flag.

So we need to introduce our own `testing` feature flag for `harmony` core and import it with that flag (only during dev/test).

More info: https://github.com/rust-lang/rust/issues/59168

Co-authored-by: Ian Letourneau <letourneau.ian@gmail.com>
Reviewed-on: https://git.nationtech.io/NationTech/harmony/pulls/102
---
 harmony/Cargo.toml                      |  3 +++
 harmony/src/domain/instrumentation.rs   | 11 ++++++++---
 harmony/src/modules/k3d/install.rs      |  3 +--
 harmony_cli/Cargo.toml                  | 10 ++++++----
 harmony_cli/src/lib.rs                  |  2 +-
 harmony_composer/src/instrumentation.rs | 15 ++++++++++++---
 6 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/harmony/Cargo.toml b/harmony/Cargo.toml
index 3d53bba..5a42cf7 100644
--- a/harmony/Cargo.toml
+++ b/harmony/Cargo.toml
@@ -5,6 +5,9 @@ version.workspace = true
 readme.workspace = true
 license.workspace = true
 
+[features]
+testing = []
+
 [dependencies]
 rand = "0.9"
 hex = "0.4"
diff --git a/harmony/src/domain/instrumentation.rs b/harmony/src/domain/instrumentation.rs
index 79787ec..5465a9b 100644
--- a/harmony/src/domain/instrumentation.rs
+++ b/harmony/src/domain/instrumentation.rs
@@ -36,9 +36,14 @@ static HARMONY_EVENT_BUS: Lazy<broadcast::Sender<HarmonyEvent>> = Lazy::new(|| {
 });
 
 pub fn instrument(event: HarmonyEvent) -> Result<(), &'static str> {
-    match HARMONY_EVENT_BUS.send(event) {
-        Ok(_) => Ok(()),
-        Err(_) => Err("send error: no subscribers"),
+    if cfg!(any(test, feature = "testing")) {
+        let _ = event; // Suppress the "unused variable" warning for `event`
+        Ok(())
+    } else {
+        match HARMONY_EVENT_BUS.send(event) {
+            Ok(_) => Ok(()),
+            Err(_) => Err("send error: no subscribers"),
+        }
     }
 }
 
diff --git a/harmony/src/modules/k3d/install.rs b/harmony/src/modules/k3d/install.rs
index d51e005..245cf41 100644
--- a/harmony/src/modules/k3d/install.rs
+++ b/harmony/src/modules/k3d/install.rs
@@ -7,7 +7,6 @@ use serde::Serialize;
 use crate::{
     config::HARMONY_DATA_DIR,
     data::{Id, Version},
-    instrumentation::{self, HarmonyEvent},
     interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome},
     inventory::Inventory,
     score::Score,
@@ -30,7 +29,7 @@ impl Default for K3DInstallationScore {
 }
 
 impl<T: Topology> Score<T> for K3DInstallationScore {
-    fn create_interpret(&self) -> Box<dyn crate::interpret::Interpret<T>> {
+    fn create_interpret(&self) -> Box<dyn Interpret<T>> {
         Box::new(K3dInstallationInterpret {
             score: self.clone(),
         })
diff --git a/harmony_cli/Cargo.toml b/harmony_cli/Cargo.toml
index 227b39e..a887b60 100644
--- a/harmony_cli/Cargo.toml
+++ b/harmony_cli/Cargo.toml
@@ -5,6 +5,10 @@ version.workspace = true
 readme.workspace = true
 license.workspace = true
 
+[features]
+default = ["tui"]
+tui = ["dep:harmony_tui"]
+
 [dependencies]
 assert_cmd = "2.0.17"
 clap = { version = "4.5.35", features = ["derive"] }
@@ -19,7 +23,5 @@ lazy_static = "1.5.0"
 log.workspace = true
 indicatif-log-bridge = "0.2.3"
 
-
-[features]
-default = ["tui"]
-tui = ["dep:harmony_tui"]
+[dev-dependencies]
+harmony = { path = "../harmony", features = ["testing"] }
diff --git a/harmony_cli/src/lib.rs b/harmony_cli/src/lib.rs
index b6cf885..430ca4a 100644
--- a/harmony_cli/src/lib.rs
+++ b/harmony_cli/src/lib.rs
@@ -163,7 +163,7 @@ async fn init<T: Topology + Send + Sync + 'static>(
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use harmony::{
         inventory::Inventory,
         maestro::Maestro,
diff --git a/harmony_composer/src/instrumentation.rs b/harmony_composer/src/instrumentation.rs
index eafa4d0..b2e9c99 100644
--- a/harmony_composer/src/instrumentation.rs
+++ b/harmony_composer/src/instrumentation.rs
@@ -23,9 +23,18 @@ static HARMONY_COMPOSER_EVENT_BUS: Lazy<broadcast::Sender<HarmonyComposerEvent>>
     });
 
 pub fn instrument(event: HarmonyComposerEvent) -> Result<(), &'static str> {
-    match HARMONY_COMPOSER_EVENT_BUS.send(event) {
-        Ok(_) => Ok(()),
-        Err(_) => Err("send error: no subscribers"),
+    #[cfg(not(test))]
+    {
+        match HARMONY_COMPOSER_EVENT_BUS.send(event) {
+            Ok(_) => Ok(()),
+            Err(_) => Err("send error: no subscribers"),
+        }
+    }
+
+    #[cfg(test)]
+    {
+        let _ = event; // Suppress the "unused variable" warning for `event`
+        Ok(())
     }
 }