fix: fix onhost supervisor reloading test

Author: vjripoll

agent-control/tests/on_host/scenarios/file_logging.rs

@@ -2,8 +2,10 @@ use std::{fs, io, path::Path, time::Duration};
 
 use crate::{
     common::{
-        agent_control::start_agent_control_with_custom_config, base_paths::TempBasePaths,
-        retry::retry, runtime::tokio_runtime,
+        agent_control::{StartedAgentControl, start_agent_control_with_custom_config},
+        base_paths::TempBasePaths,
+        retry::retry,
+        runtime::tokio_runtime,
     },
     on_host::tools::{
         config::{OnHostAgentControlConfigBuilder, create_file, create_local_config},
@@ -123,7 +125,7 @@ fn run_file_logging_scenario(
     initial_message: &str,
     reload_file_logging: bool,
     reload_message: &str,
-) -> TempBasePaths {
+) -> (TempBasePaths, StartedAgentControl) {
     let mut opamp_server = FakeServer::start(tokio_runtime().handle());
 
     let dirs = TempBasePaths::default();
@@ -154,7 +156,7 @@ fn run_file_logging_scenario(
         dirs.local_dir(),
     );
 
-    let _agent_control =
+    let agent_control =
         start_agent_control_with_custom_config(dirs.base_paths(), AGENT_CONTROL_MODE_ON_HOST);
 
     let sub_agent_instance_id =
@@ -172,7 +174,8 @@ fn run_file_logging_scenario(
     // Give some time for the echo output to be captured in the log files
     std::thread::sleep(Duration::from_secs(5));
 
-    dirs
+    // Return the handle so AC keeps running while the caller waits for the post-reload logs.
+    (dirs, agent_control)
 }
 
 /// File logging enable/disable combinations with before and after reload checks
@@ -188,7 +191,8 @@ fn test_file_logging_reload(
 ) {
     let agent_id = format!("file-logging-agent-{first_run_enabled}-{second_run_enabled}");
 
-    let dirs = run_file_logging_scenario(
+    // Keep the handle alive so AC keeps running during the wait below.
+    let (dirs, _agent_control) = run_file_logging_scenario(
         &agent_id,
         first_run_enabled,
         first_run_message,
@@ -197,24 +201,26 @@ fn test_file_logging_reload(
     );
 
     let log_dir_path = dirs.log_dir();
-    let agent_logs_dir = log_dir_path.join(&agent_id);
-    assert!(
-        agent_logs_dir.exists(),
-        "Log directory {agent_logs_dir:?} does not exist"
-    );
 
+    // Wait (AC still running) until the logs match the expected end state after the reload.
     let all_contents = retry(60, Duration::from_secs(1), || {
-        Ok(collect_stdout_logs(&log_dir_path, &agent_id)?)
+        let contents = collect_stdout_logs(&log_dir_path, &agent_id)?;
+        if first_run_enabled == contents.contains(first_run_message)
+            && second_run_enabled == contents.contains(second_run_message)
+        {
+            Ok(contents)
+        } else {
+            Err(format!("log state not settled yet. Log contents: {contents}").into())
+        }
     });
 
-    // If the logs are enabled for the run the string must be found, same for disabled and not found
     assert!(
         first_run_enabled == all_contents.contains(first_run_message),
-        "First run log not found (pre-reload). Log contents: {all_contents}"
+        "First run log mismatch (pre-reload). Log contents: {all_contents}"
     );
     assert!(
         second_run_enabled == all_contents.contains(second_run_message),
-        "Second run log not found (post-reload). Log contents: {all_contents}"
+        "Second run log mismatch (post-reload). Log contents: {all_contents}"
     );
 }
 
@@ -226,7 +232,8 @@ fn onhost_supervisor_reloading_keeps_file_logging_disabled() {
     let unique_str_2 = "keeps_disabled_run2";
     let agent_id = "test-agent-logs-always-disabled";
 
-    let dirs = run_file_logging_scenario(agent_id, false, unique_str_1, false, unique_str_2);
+    let (dirs, _agent_control) =
+        run_file_logging_scenario(agent_id, false, unique_str_1, false, unique_str_2);
 
     let log_dir_path = dirs.log_dir();
     let agent_logs_dir = log_dir_path.join(agent_id);

self-replacer/tests/integration_tests.rs

@@ -10,7 +10,7 @@ use tempfile::TempDir;
 
 mod test_helpers;
 use self_replacer::{BinaryReplacer, SelfReplacer};
-use test_helpers::{copy_example_binary, create_modified_binary};
+use test_helpers::{copy_example_binary, create_modified_binary, exec_serial_guard};
 
 use self_replacer::BACKUP_SUFFIX;
 
@@ -20,6 +20,9 @@ use self_replacer::BACKUP_SUFFIX;
 
 #[test]
 fn test_self_replacement_with_real_binary() {
+    // Serialize with the other write+exec test to avoid ETXTBSY (see exec_serial_guard).
+    let _guard = exec_serial_guard();
+
     let temp_dir = TempDir::new().unwrap();
     let test_dir = temp_dir.path();
 
@@ -130,6 +133,9 @@ mod unix_specific {
 
     #[test]
     fn test_permission_preservation_with_real_binary() {
+        // Serialize with the other write+exec test to avoid ETXTBSY (see exec_serial_guard).
+        let _guard = exec_serial_guard();
+
         let temp_dir = TempDir::new().unwrap();
         let test_dir = temp_dir.path();
 

self-replacer/tests/test_helpers.rs

@@ -2,6 +2,27 @@
 
 use std::fs;
 use std::path::{Path, PathBuf};
+use std::sync::{Mutex, Once};
+
+/// Serializes the integration tests that write a binary and then execute it.
+///
+/// Running them concurrently races on Linux: while one test's `fs::copy`/`fs::write` holds a
+/// writable fd to a binary, another test spawning a process (`fork` + `exec`) inherits that fd, so
+/// exec'ing the just-written binary can fail with `ETXTBSY` ("Text file busy"). Holding this lock
+/// for the duration of each such test keeps only one write+exec sequence in flight at a time.
+static EXEC_SERIAL_LOCK: Mutex<()> = Mutex::new(());
+
+/// Acquires the [`EXEC_SERIAL_LOCK`], recovering from poisoning (a failing test that panicked while
+/// holding it must not cascade-poison the others).
+pub fn exec_serial_guard() -> std::sync::MutexGuard<'static, ()> {
+    EXEC_SERIAL_LOCK
+        .lock()
+        .unwrap_or_else(|poisoned| poisoned.into_inner())
+}
+
+/// Ensures the example binary is built at most once, even when parallel tests call
+/// [`get_example_binary`] concurrently.
+static BUILD_EXAMPLE: Once = Once::new();
 
 /// Returns path to the example binary, building it automatically if needed.
 ///
@@ -29,23 +50,26 @@ pub fn get_example_binary() -> PathBuf {
     };
     path.push(binary_name);
 
-    // Build the example if it doesn't exist
-    if !path.exists() {
-        eprintln!("Example binary not found, building it...");
-        let output = std::process::Command::new("cargo")
-            .arg("build")
-            .arg("--example")
-            .arg("self_replacing_binary")
-            .output()
-            .expect("Failed to run cargo build");
-
-        if !output.status.success() {
-            panic!(
-                "Failed to build example binary:\n{}",
-                String::from_utf8_lossy(&output.stderr)
-            );
+    // Build the example if it doesn't exist, but only once across all (parallel) tests.
+    let build_path = path.clone();
+    BUILD_EXAMPLE.call_once(move || {
+        if !build_path.exists() {
+            eprintln!("Example binary not found, building it...");
+            let output = std::process::Command::new("cargo")
+                .arg("build")
+                .arg("--example")
+                .arg("self_replacing_binary")
+                .output()
+                .expect("Failed to run cargo build");
+
+            if !output.status.success() {
+                panic!(
+                    "Failed to build example binary:\n{}",
+                    String::from_utf8_lossy(&output.stderr)
+                );
+            }
         }
-    }
+    });
 
     assert!(
         path.exists(),