test: fix flaky tests with SubAgent client builder (#1007)

Author: rubenruizdegauna

agent-control/src/opamp/client_builder.rs

@@ -100,6 +100,7 @@ pub(crate) mod tests {
         Client, ClientResult, NotStartedClient, NotStartedClientResult, StartedClient,
         StartedClientResult,
     };
+    use std::thread;
 
     use super::*;
 
@@ -220,5 +221,34 @@ pub(crate) mod tests {
                 .once()
                 .return_once(move |_, _, _| Ok(client));
         }
+
+        // This is a Mock OpAMP Client Builder, which builds the Callbacks and the OpAMP Client
+        // and starts the OpAMP Client thread. This thread owns the callbacks, and these publish
+        // into the OpAMP Publisher <-- Sub Agent OpAMP Consumer
+        // Sub Agent OpAMP Consumer consumes the OpAMP events.
+        // Using the Mock, makes the OpAMP publisher to be dropped, as it's part of the expectations
+        // Until we refactor these tests (and find a better solution for this pattern) we'll
+        // spawn a thread with the publisher, just not to be dropped in the test
+        // TL;DR: Let the OpAMP Publisher leave for Duration
+        pub fn should_build_and_start_and_run(
+            &mut self,
+            agent_id: AgentID,
+            start_settings: StartSettings,
+            client: MockStartedOpAMPClientMock<C>,
+            run_for: Duration,
+        ) {
+            self.expect_build_and_start()
+                .withf(move |publisher, _sub_agent_id, _start_settings| {
+                    let publisher = publisher.clone();
+                    thread::spawn(move || {
+                        thread::sleep(run_for);
+                        drop(publisher)
+                    });
+                    //
+                    agent_id == _sub_agent_id.clone() && start_settings == *_start_settings
+                })
+                .once()
+                .return_once(move |_, _, _| Ok(client));
+        }
     }
 }

agent-control/src/sub_agent/on_host/builder.rs

@@ -240,6 +240,7 @@ mod tests {
         AgentDescription, DescriptionValueType, StartSettings,
     };
     use std::collections::HashMap;
+    use std::time::Duration;
     use tracing_test::traced_test;
 
     // TODO: tests below are testing not only the builder but also the sub-agent start/stop behavior.
@@ -277,10 +278,13 @@ mod tests {
         started_client.should_stop(1);
 
         // Infra Agent OpAMP no final stop nor health, just after stopping on reload
-        opamp_builder.should_build_and_start(
+        // TODO: We should discuss if this is a valid approach once we refactor the unit tests
+        // Build an OpAMP Client and let it run so the publisher is not dropped
+        opamp_builder.should_build_and_start_and_run(
             sub_agent_id.clone(),
             start_settings_infra,
             started_client,
+            Duration::from_millis(10),
         );
 
         let mut instance_id_getter = MockInstanceIDGetterMock::new();
@@ -375,10 +379,13 @@ mod tests {
         started_client.should_update_effective_config(1);
         started_client.should_stop(1);
 
-        opamp_builder.should_build_and_start(
+        // TODO: We should discuss if this is a valid approach once we refactor the unit tests
+        // Build an OpAMP Client and let it run so the publisher is not dropped
+        opamp_builder.should_build_and_start_and_run(
             sub_agent_id.clone(),
             start_settings_infra,
             started_client,
+            Duration::from_millis(10),
         );
 
         effective_agent_assembler.should_assemble_agent(