fix: onhost supervisor reloading test (#2681)

* fix: fix onhost supervisor reloading test

* fix nits

Author: vjripoll

Cargo.lock

@@ -44,6 +44,7 @@ chrono = "0.4.45"
 jsonwebtoken = { version = "10.4.0", features = ["aws_lc_rs"] }
 reqwest = { version = "0.13.4", features = ["blocking", "socks"] }
 rstest = "0.26.1"
+serial_test = "3.5.0"
 windows = "0.62.2"
 windows-sys = "0.61.2"
 aws-lc-rs = "1.17.1"

Cargo.toml

@@ -116,7 +116,7 @@ fs = { path = "../fs", features = ["mocks"] }
 tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }
 fake = { version = "5.1.0", features = ["derive", "http"] }
 httpmock = { version = "0.8.3", features = ["proxy"] }
-serial_test = "3.5.0"
+serial_test = { workspace = true }
 futures = "0.3.32"
 rcgen = { version = "0.14.8", features = ["crypto"] }
 rstest = { workspace = true }

agent-control/Cargo.toml

@@ -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,13 +156,13 @@ 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 =
         get_instance_id(&AgentID::try_from(agent_id).unwrap(), dirs.base_paths());
 
-    // Give some time for the echo output to be captured in the log files
+    // Let the first run write its log before the reload can change/disable logging.
     std::thread::sleep(Duration::from_secs(5));
 
     // Trigger a reload by sending a new remote config via OpAMP with updated values
@@ -169,10 +171,8 @@ fn run_file_logging_scenario(
         format!("message: \"{reload_message}\"\nenable_file_logging: \"{reload_file_logging}\"\n"),
     );
 
-    // Give some time for the echo output to be captured in the log files
-    std::thread::sleep(Duration::from_secs(5));
-
-    dirs
+    // AC stays alive via the returned handle; the caller's retry waits for the post-reload logs.
+    (dirs, agent_control)
 }
 
 /// File logging enable/disable combinations with before and after reload checks
@@ -188,7 +188,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 +198,25 @@ 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"
-    );
 
     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 +228,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);

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

@@ -15,8 +15,9 @@ tempfile = { workspace = true }
 
 [dev-dependencies]
 assert_matches = { workspace = true }
-assert_cmd = "2.2.2"
-predicates = "3.1.4"
+assert_cmd = { workspace = true }
+predicates = { workspace = true }
+serial_test = { workspace = true }
 
 [target.'cfg(target_os = "windows")'.dev-dependencies]
 windows = { workspace = true, features = [

self-replacer/Cargo.toml

@@ -10,6 +10,7 @@ use tempfile::TempDir;
 
 mod test_helpers;
 use self_replacer::{BinaryReplacer, SelfReplacer};
+use serial_test::serial;
 use test_helpers::{copy_example_binary, create_modified_binary};
 
 use self_replacer::BACKUP_SUFFIX;
@@ -18,7 +19,14 @@ use self_replacer::BACKUP_SUFFIX;
 // Common tests that run on all platforms
 // ============================================================================
 
+// Must run serially: this test writes an executable and then execs it. If another test runs
+// in parallel, its `Command` fork+exec can inherit our still-open write fd to the freshly
+// written binary during the fork->exec window, so our exec fails with ETXTBSY ("text file
+// busy", os error 26). O_CLOEXEC does not help because the fd is only closed on exec, not fork.
+// Serializing removes the write/exec overlap. (test_rollback_on_invalid_path needs no serial:
+// it never execs a freshly written binary.)
 #[test]
+#[serial]
 fn test_self_replacement_with_real_binary() {
     let temp_dir = TempDir::new().unwrap();
     let test_dir = temp_dir.path();
@@ -128,7 +136,10 @@ mod unix_specific {
 
     const TEST_EXEC_MODE: u32 = 0o754; // rwxr-xr--
 
+    // Serial for the same reason as test_self_replacement_with_real_binary: writing then
+    // exec-ing a binary races another test's fork+exec and hits ETXTBSY (os error 26).
     #[test]
+    #[serial]
     fn test_permission_preservation_with_real_binary() {
         let temp_dir = TempDir::new().unwrap();
         let test_dir = temp_dir.path();

self-replacer/tests/integration_tests.rs

@@ -29,7 +29,7 @@ pub fn get_example_binary() -> PathBuf {
     };
     path.push(binary_name);
 
-    // Build the example if it doesn't exist
+    // 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")