From 372e81e2da3a60ee8cbf3f2525bf27284dc62332 Mon Sep 17 00:00:00 2001 From: Victor Adossi Date: Thu, 19 Oct 2023 15:17:57 +0900 Subject: [PATCH] refactor: various fixes to testing code This commit refactors some of the testing code to: - ensure we always print integration test output (save time root causing in CI and elsewhere) - consistent use of TARGET to choose which test to run - use system provided randomized ports (port 0) - fix some uses of context - remove some process scanning that was never used This commit also includes changes test flake fixes from https://github.com/wasmCloud/wash/pull/921 Signed-off-by: Victor Adossi --- Makefile | 2 +- crates/wash-lib/src/start/wasmcloud.rs | 21 ++++++------- src/dev.rs | 4 +-- tests/common.rs | 41 ++++++++++---------------- tests/integration.rs | 2 -- tests/integration_dev.rs | 11 +++---- tests/integration_drain.rs | 1 - tests/integration_up.rs | 22 ++++++++------ 8 files changed, 47 insertions(+), 57 deletions(-) diff --git a/Makefile b/Makefile index beede58a..3d2f6f64 100644 --- 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 index 19421b78..349270d3 100644 --- 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 { - 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 index fae08eee..c7bb86cb 100644 --- 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 index b451e0cd..febc38a4 100644 --- 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 { - 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 { + 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 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 index f8ac4932..5704a5e1 100644 --- 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 index cb86929c..e3b85a8c 100644 --- 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 index deec1cf3..5c00451a 100644 --- 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 index deee13b3..3e4af77b 100644 --- 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[^]]+)\] \(ref: \[(?P[^]]+)\]\) started on host \[(?P[^]]+)\]"; @@ -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,8 +58,9 @@ 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([ @@ -64,7 +68,7 @@ async fn integration_up_can_start_wasmcloud_and_actor_serial() -> Result<()> { "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([