Merge pull request #922 from vados-cosmonic/refactor/light-testing-co…

…de-refactor refactor: various fixes to testing code
wasmCloud · Oct 19, 2023 · 0b9e1ca · 0b9e1ca
2 parents 516aa5e + 372e81e
commit 0b9e1ca
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 57 deletions.
diff --git a/Makefile b/Makefile
@@ -49,7 +49,7 @@ test-watch: ## Run unit tests continously, can optionally specify a target test
 
 test-integration: ## Run the entire integration test suite (with docker compose)
 	@$(DOCKER) compose -f ./tools/docker-compose.yml up --detach
-	@$(CARGO) nextest run $(TARGET) --profile integration -E 'kind(test)'
+	@$(CARGO) nextest run $(TARGET) --profile integration -E 'kind(test)' --nocapture
 	@$(DOCKER) compose -f ./tools/docker-compose.yml down
 
 test-integration-ci: ## Run the entire integration test suite only

diff --git a/crates/wash-lib/src/start/wasmcloud.rs b/crates/wash-lib/src/start/wasmcloud.rs
@@ -292,28 +292,25 @@ mod test {
         is_bin_installed, start_nats_server, start_wasmcloud_host, NatsConfig, NATS_SERVER_BINARY,
     };
 
-    use anyhow::{bail, Context, Result};
+    use anyhow::{Context, Result};
     use reqwest::StatusCode;
+    use std::net::{Ipv4Addr, SocketAddrV4};
     use std::{collections::HashMap, env::temp_dir};
     use tokio::fs::{create_dir_all, remove_dir_all};
-    use tokio::net::TcpStream;
+    use tokio::net::TcpListener;
     use tokio::time::Duration;
 
     const WASMCLOUD_VERSION: &str = "v0.79.0-rc3";
-    const RANDOM_PORT_RANGE_START: u16 = 5000;
-    const RANDOM_PORT_RANGE_END: u16 = 6000;
     const LOCALHOST: &str = "127.0.0.1";
 
     /// Returns an open port on the interface, searching within the range endpoints, inclusive
     async fn find_open_port() -> Result<u16> {
-        for i in RANDOM_PORT_RANGE_START..=RANDOM_PORT_RANGE_END {
-            if let Ok(conn) = TcpStream::connect((LOCALHOST, i)).await {
-                drop(conn);
-            } else {
-                return Ok(i);
-            }
-        }
-        bail!("Failed to find open port for host")
+        TcpListener::bind(SocketAddrV4::new(Ipv4Addr::LOCALHOST, 0))
+            .await
+            .context("failed to bind random port")?
+            .local_addr()
+            .map(|addr| addr.port())
+            .context("failed to get local address from opened TCP socket")
     }
 
     #[tokio::test]

diff --git a/src/dev.rs b/src/dev.rs
@@ -229,7 +229,7 @@ pub async fn handle_command(
                 hosts
                     .into_iter()
                     .find(|h| h.id == host_id)
-                    .ok_or_else(|| anyhow!("failed to find host [{host_id}]"))?
+                    .with_context(|| format!("failed to find host [{host_id}]"))?
             } else {
                 bail!(
                     "{} hosts detected, please specify the host on which to deploy with --host-id",
@@ -373,7 +373,7 @@ pub async fn handle_command(
 
                 if !cmd.leave_host_running {
                     eprintln!("⏳ stopping wasmCloud instance...");
-                    handle_down(DownCommand::default(), output_kind).await?;
+                    handle_down(DownCommand::default(), output_kind).await.context("down command failed")?;
                     if let Some(handle) = host_subprocess.and_then(|hs| hs.into_inner())  {
                         handle.await?;
                     }

diff --git a/tests/common.rs b/tests/common.rs
@@ -1,4 +1,6 @@
 use std::fs::read_to_string;
+use std::net::TcpListener;
+use std::net::{Ipv4Addr, SocketAddrV4};
 use std::{
     env,
     fs::{create_dir_all, remove_dir_all},
@@ -9,7 +11,6 @@ use anyhow::{bail, Context, Result};
 use rand::{distributions::Alphanumeric, Rng};
 use sysinfo::{ProcessExt, SystemExt};
 use tempfile::TempDir;
-use tokio::net::TcpStream;
 use tokio::{
     fs::File,
     io::AsyncWriteExt,
@@ -18,7 +19,7 @@ use tokio::{
 };
 
 use wash_lib::cli::output::GetHostsCommandOutput;
-use wash_lib::start::{ensure_nats_server, start_nats_server, NatsConfig};
+use wash_lib::start::{ensure_nats_server, start_nats_server, NatsConfig, WASMCLOUD_HOST_BIN};
 use wasmcloud_control_interface::Host;
 
 #[allow(unused)]
@@ -79,20 +80,13 @@ pub(crate) async fn start_nats(port: u16, nats_install_dir: &PathBuf) -> Result<
     start_nats_server(nats_binary, std::process::Stdio::null(), config).await
 }
 
-const RANDOM_PORT_RANGE_START: u16 = 5000;
-const RANDOM_PORT_RANGE_END: u16 = 6000;
-const LOCALHOST: &str = "127.0.0.1";
-
 /// Returns an open port on the interface, searching within the range endpoints, inclusive
-async fn find_open_port() -> Result<u16> {
-    for i in RANDOM_PORT_RANGE_START..=RANDOM_PORT_RANGE_END {
-        if let Ok(conn) = TcpStream::connect((LOCALHOST, i)).await {
-            drop(conn);
-        } else {
-            return Ok(i);
-        }
-    }
-    bail!("Failed to find open port for host")
+pub(crate) async fn find_open_port() -> Result<u16> {
+    TcpListener::bind(SocketAddrV4::new(Ipv4Addr::LOCALHOST, 0))
+        .context("failed to bind random port")?
+        .local_addr()
+        .map(|addr| addr.port())
+        .context("failed to get local address from opened TCP socket")
 }
 
 #[allow(unused)]
@@ -110,6 +104,7 @@ impl Drop for TestWashInstance {
             test_dir, kill_cmd, ..
         } = self;
 
+        // Attempt to stop the host (this may fail)
         let kill_cmd = kill_cmd.to_string();
         let (_wash, down) = kill_cmd.trim_matches('"').split_once(' ').unwrap();
         wash()
@@ -123,17 +118,11 @@ impl Drop for TestWashInstance {
             .output()
             .expect("Could not spawn wash down process");
 
+        // Attempt to stop NATS
         self.nats
             .start_kill()
             .expect("failed to start_kill() on nats instance");
 
-        // Check to see if process was removed
-        let mut info = sysinfo::System::new_with_specifics(
-            sysinfo::RefreshKind::new().with_processes(sysinfo::ProcessRefreshKind::new()),
-        );
-
-        info.refresh_processes();
-
         remove_dir_all(test_dir).expect("failed to remove temporary directory during cleanup");
     }
 }
@@ -206,7 +195,6 @@ impl TestWashInstance {
                         }
                     }
                     _ => {
-                        println!("no host startup logs in output yet, waiting 1 second");
                         tokio::time::sleep(Duration::from_secs(1)).await;
                     }
                 }
@@ -318,7 +306,7 @@ pub(crate) async fn wait_until_process_has_count(
     })
     .await
     .context(format!(
-        "Failed to find find satisfactory amount of processes named [{filter}]"
+        "failed to find satisfactory amount of processes named [{filter}]"
     ))?;
 
     Ok(())
@@ -413,12 +401,13 @@ pub(crate) async fn init_workspace(actor_names: Vec<&str>) -> Result<WorkspaceTe
 #[allow(dead_code)]
 pub(crate) async fn wait_for_no_hosts() -> Result<()> {
     wait_until_process_has_count(
-        "wasmcloud_host",
+        WASMCLOUD_HOST_BIN,
         |v| v == 0,
         Duration::from_secs(15),
         Duration::from_millis(250),
     )
     .await
+    .context("number of hosts running is still non-zero")
 }
 
 /// Wait for NATS to start running by checking for process names.
@@ -432,6 +421,7 @@ pub(crate) async fn wait_for_nats_to_start() -> Result<()> {
         Duration::from_secs(1),
     )
     .await
+    .context("at least one nats-server process has not started")
 }
 
 /// Wait for no nats to be running by checking for process names
@@ -444,4 +434,5 @@ pub(crate) async fn wait_for_no_nats() -> Result<()> {
         Duration::from_millis(250),
     )
     .await
+    .context("number of nats-server processes should be zero")
 }
diff --git a/tests/integration.rs b/tests/integration.rs
@@ -9,8 +9,6 @@ fn integration_help_subcommand_check() {
         .expect("failed to display help text");
     let output = output_to_string(help_output).unwrap();
 
-    println!("output: \n{output}");
-
     assert!(output.contains("claims"));
     assert!(output.contains("ctl"));
     assert!(output.contains("drain"));

diff --git a/tests/integration_dev.rs b/tests/integration_dev.rs
@@ -9,7 +9,7 @@ use tokio::{process::Command, sync::RwLock, time::Duration};
 mod common;
 
 use crate::common::{
-    init, start_nats, test_dir_with_subfolder, wait_for_no_hosts, wait_for_no_nats,
+    find_open_port, init, start_nats, test_dir_with_subfolder, wait_for_no_hosts, wait_for_no_nats,
 };
 
 #[tokio::test]
@@ -28,17 +28,18 @@ async fn integration_dev_hello_actor_serial() -> Result<()> {
         .await
         .context("one or more unexpected wasmcloud instances running")?;
 
-    let mut nats = start_nats(5895, &dir).await?;
+    let nats_port = find_open_port().await?;
+    let mut nats = start_nats(nats_port, &dir).await?;
 
     let dev_cmd = Arc::new(RwLock::new(
         Command::new(env!("CARGO_BIN_EXE_wash"))
             .args([
                 "dev",
                 "--nats-port",
-                "5895",
+                nats_port.to_string().as_ref(),
                 "--nats-connect-only",
                 "--ctl-port",
-                "5895",
+                nats_port.to_string().as_ref(),
                 "--use-host-subprocess",
                 "--disable-wadm",
             ])
@@ -103,7 +104,7 @@ async fn integration_dev_hello_actor_serial() -> Result<()> {
 
     wait_for_no_nats()
         .await
-        .context("nats instance failed to exit cleanly (processes still left over")?;
+        .context("nats instance failed to exit cleanly (processes still left over)")?;
 
     Ok(())
 }
diff --git a/tests/integration_drain.rs b/tests/integration_drain.rs
@@ -189,7 +189,6 @@ fn set_smithy_cache_dir() -> (PathBuf, String) {
 
 #[cfg(any(target_os = "linux", target_os = "macos"))]
 fn test_smithy_cache_drain() {
-    println!("temp dir is {}", &std::env::temp_dir().display());
     let (_sys_tmp_cache, smithy_cache) = set_smithy_cache_dir();
     let drain_basic = wash()
         .args(["drain", "smithy", "-o", "json"])

diff --git a/tests/integration_up.rs b/tests/integration_up.rs
@@ -8,7 +8,9 @@ use tokio::{process::Command, time::Duration};
 
 mod common;
 
-use common::{start_nats, wait_for_nats_to_start, wait_for_no_hosts, wait_for_single_host};
+use common::{
+    find_open_port, start_nats, wait_for_nats_to_start, wait_for_no_hosts, wait_for_single_host,
+};
 
 const RGX_ACTOR_START_MSG: &str = r"Actor \[(?P<actor_id>[^]]+)\] \(ref: \[(?P<actor_ref>[^]]+)\]\) started on host \[(?P<host_id>[^]]+)\]";
 
@@ -25,11 +27,12 @@ async fn integration_up_can_start_wasmcloud_and_actor_serial() -> Result<()> {
 
     let host_seed = nkeys::KeyPair::new_server();
 
+    let nats_port = find_open_port().await?;
     let mut up_cmd = Command::new(env!("CARGO_BIN_EXE_wash"))
         .args([
             "up",
             "--nats-port",
-            "5893",
+            nats_port.to_string().as_ref(),
             "-o",
             "json",
             "--detached",
@@ -55,16 +58,17 @@ async fn integration_up_can_start_wasmcloud_and_actor_serial() -> Result<()> {
         Err(_e) => panic!("Unable to parse kill cmd from wash up output"),
     };
 
-    // Wait for a single host to exis
-    let host = wait_for_single_host(5893, Duration::from_secs(10), Duration::from_secs(1)).await?;
+    // Wait for a single host to exist
+    let host =
+        wait_for_single_host(nats_port, Duration::from_secs(10), Duration::from_secs(1)).await?;
 
     let start_echo = Command::new(env!("CARGO_BIN_EXE_wash"))
         .args([
             "start",
             "actor",
             "wasmcloud.azurecr.io/echo:0.3.4",
             "--ctl-port",
-            "5893",
+            nats_port.to_string().as_ref(),
             "--timeout-ms",
             "10000", // Wait up to 10 seconds for slowpoke systems
         ])
@@ -90,7 +94,7 @@ async fn integration_up_can_start_wasmcloud_and_actor_serial() -> Result<()> {
         .args(vec![
             down,
             "--ctl-port",
-            "5893",
+            nats_port.to_string().as_ref(),
             "--host-id",
             &host_seed.public_key(),
         ])
@@ -114,7 +118,7 @@ async fn integration_up_can_stop_detached_host_serial() -> Result<()> {
     let dir = test_dir_with_subfolder("can_stop_wasmcloud");
     let path = dir.join("washup.log");
     let stdout = std::fs::File::create(&path).expect("could not create log file for wash up test");
-    let nats_port: u16 = 5894;
+    let nats_port: u16 = find_open_port().await?;
 
     wait_for_no_hosts()
         .await
@@ -184,14 +188,14 @@ async fn integration_up_doesnt_kill_unowned_nats_serial() -> Result<()> {
     let dir = test_dir_with_subfolder("doesnt_kill_unowned_nats");
     let path = dir.join("washup.log");
     let stdout = std::fs::File::create(&path).expect("could not create log file for wash up test");
-    let nats_port: u16 = 5895;
+    let nats_port: u16 = find_open_port().await?;
 
     // Check that there are no host processes running
     wait_for_no_hosts()
         .await
         .context("unexpected wasmcloud instance(s) running")?;
 
-    let mut nats = start_nats(5895, &dir).await?;
+    let mut nats = start_nats(nats_port, &dir).await?;
 
     let mut up_cmd = Command::new(env!("CARGO_BIN_EXE_wash"))
         .args([