From 213fb1f5c1819ecde784e9b6c570db979bb8d003 Mon Sep 17 00:00:00 2001 From: jossduff Date: Sun, 6 Oct 2024 12:31:02 -0400 Subject: [PATCH 1/2] fix(test): run tests serially to eliminate cross-test priory instance communication --- sigil/Cargo.lock | 41 ++++++++++++++++++++++++ sigil/Cargo.toml | 1 + sigil/README.md | 4 ++- sigil/tests/integration.rs | 46 +++++++++++++++------------ sigil/tests/test_configs/a.toml | 2 +- sigil/tests/test_configs/default.toml | 1 + sigil/tests/test_helper.rs | 20 +++++++++++- 7 files changed, 92 insertions(+), 23 deletions(-) diff --git a/sigil/Cargo.lock b/sigil/Cargo.lock index c703343..5cf351a 100644 --- a/sigil/Cargo.lock +++ b/sigil/Cargo.lock @@ -3508,6 +3508,15 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f3cb5ba0dc43242ce17de99c180e96db90b235b8a9fdc9543c96d2209116bd9f" +[[package]] +name = "scc" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "836f1e0f4963ef5288b539b643b35e043e76a32d0f4e47e67febf69576527f50" +dependencies = [ + "sdd", +] + [[package]] name = "schannel" version = "0.1.24" @@ -3523,6 +3532,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "sdd" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "60a7b59a5d9b0099720b417b6325d91a52cbf5b3dcb5041d864be53eefa58abc" + [[package]] name = "security-framework" version = "2.11.1" @@ -3646,6 +3661,31 @@ dependencies = [ "syn 2.0.77", ] +[[package]] +name = "serial_test" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b4b487fe2acf240a021cf57c6b2b4903b1e78ca0ecd862a71b71d2a51fed77d" +dependencies = [ + "futures", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82fe9db325bcef1fbcde82e078a5cc4efdf787e96b3b9cf45b50b529f2083d67" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.77", +] + [[package]] name = "sha1" version = "0.10.6" @@ -3698,6 +3738,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "serial_test", "testcontainers", "tokio", "toml", diff --git a/sigil/Cargo.toml b/sigil/Cargo.toml index 57d83c1..380642c 100644 --- a/sigil/Cargo.toml +++ b/sigil/Cargo.toml @@ -26,3 +26,4 @@ tracing-subscriber = { version = "0.3.17", features = [ "env-filter", ] } toml = "0.8.19" +serial_test = "3.1.1" diff --git a/sigil/README.md b/sigil/README.md index 4679a11..81fb1d8 100644 --- a/sigil/README.md +++ b/sigil/README.md @@ -20,7 +20,9 @@ It is our hope that we have successfully encapsulated this admittedly-convoluted ## Testing -Once built, the project may be tested as usual using the standard `cargo test`. Some integration tests rely on the ability to access a Docker image of the client to test inter-client communications. +Once built, the project may be tested as usual using the standard `cargo test`. Some integration tests rely on the ability to access a Docker image of the client to test inter-client communications. + +* If the tests are manually cancelled, make sure to also manually stop any docker containers started by the test. ## Running diff --git a/sigil/tests/integration.rs b/sigil/tests/integration.rs index 293a0fd..52854e0 100644 --- a/sigil/tests/integration.rs +++ b/sigil/tests/integration.rs @@ -1,6 +1,7 @@ use anyhow::{Context, Result}; use reqwest::Client; use serde_json::json; +use serial_test::serial; use std::panic::AssertUnwindSafe; use std::string::String; use testcontainers::{ @@ -13,43 +14,47 @@ mod test_helper; use test_helper::SigilTestInstance; #[tokio::test] +#[serial] async fn test_2_mdns_connections() { let sigil_a = SigilTestInstance::new("a.toml").await; let sigil_b = SigilTestInstance::new("b.toml").await; - // TODO: make assertions about all this - let peer_id_a = sigil_a.rpc("my_peer_id", None).await.unwrap(); - println!("\npeer a id: {}", peer_id_a); + let sigil_a_peer_id = sigil_a.rpc("my_peer_id", None).await.unwrap(); + let sigil_b_peer_id = sigil_b.rpc("my_peer_id", None).await.unwrap(); - let connected_peers_a = sigil_a.rpc("connected_peers", None).await.unwrap(); - println!("connected peers a: {}", connected_peers_a); - - let gossipsub_peers_a = sigil_a.rpc("gossipsub_mesh_peers", None).await.unwrap(); - println!("gossipsub mesh peers a: {}", gossipsub_peers_a); + sigil_a + .rpc_with_expected("connected_peers", None, &sigil_b_peer_id) + .await + .unwrap(); - let kademlia_routing_table_peers = sigil_a - .rpc("kademlia_routing_table_peers", None) + sigil_a + .rpc_with_expected("gossipsub_mesh_peers", None, &sigil_b_peer_id) .await .unwrap(); - println!("kademlia routing table peers a: {kademlia_routing_table_peers}\n"); - let peer_id_b = sigil_b.rpc("my_peer_id", None).await.unwrap(); - println!("peer b id: {}", peer_id_b); + sigil_a + .rpc_with_expected("kademlia_routing_table_peers", None, &sigil_b_peer_id) + .await + .unwrap(); - let connected_peers_b = sigil_b.rpc("connected_peers", None).await.unwrap(); - println!("connected peers b: {}", connected_peers_b); + sigil_b + .rpc_with_expected("connected_peers", None, &sigil_a_peer_id) + .await + .unwrap(); - let gossipsub_peers_b = sigil_b.rpc("gossipsub_mesh_peers", None).await.unwrap(); - println!("gossipsub mesh peers b: {}", gossipsub_peers_b); + sigil_b + .rpc_with_expected("gossipsub_mesh_peers", None, &sigil_a_peer_id) + .await + .unwrap(); - let kademlia_routing_table_peers = sigil_b - .rpc("kademlia_routing_table_peers", None) + sigil_b + .rpc_with_expected("kademlia_routing_table_peers", None, &sigil_a_peer_id) .await .unwrap(); - println!("kademlia routing table peers b: {kademlia_routing_table_peers}"); } #[tokio::test] +#[serial] async fn test_no_connections_default_config() { let sigil = SigilTestInstance::new("default.toml").await; @@ -70,6 +75,7 @@ async fn test_no_connections_default_config() { } #[tokio::test] +#[serial] async fn test_hello_sigil() { let sigil = SigilTestInstance::new("default.toml").await; diff --git a/sigil/tests/test_configs/a.toml b/sigil/tests/test_configs/a.toml index c763f97..23aab78 100644 --- a/sigil/tests/test_configs/a.toml +++ b/sigil/tests/test_configs/a.toml @@ -1,2 +1,2 @@ [priory] -secret_key_seed = 1 +secret_key_seed = 3 diff --git a/sigil/tests/test_configs/default.toml b/sigil/tests/test_configs/default.toml index 382c4e2..c763f97 100644 --- a/sigil/tests/test_configs/default.toml +++ b/sigil/tests/test_configs/default.toml @@ -1 +1,2 @@ [priory] +secret_key_seed = 1 diff --git a/sigil/tests/test_helper.rs b/sigil/tests/test_helper.rs index 55b4bc3..da0de5e 100644 --- a/sigil/tests/test_helper.rs +++ b/sigil/tests/test_helper.rs @@ -1,9 +1,11 @@ use anyhow::{anyhow, Context, Result}; +use serde_json::Value; use testcontainers::{ core::{ContainerAsync, IntoContainerPort, WaitFor}, runners::AsyncRunner, GenericImage, ImageExt, }; +use tokio::time::Duration; // TODO: there are a lot of constansts in here. They will either be params or real CONST vars. @@ -29,6 +31,9 @@ impl SigilTestInstance { .await .expect("Failed to start sigil container"); + // give it a chance to make connections or crash + tokio::time::sleep(Duration::from_millis(500)).await; + let host_port = container .get_host_port_ipv4(port) .await @@ -99,7 +104,20 @@ impl SigilTestInstance { method, params ))?; - response.text().await.context("Failed to get response body") + let body = response + .text() + .await + .context("Failed to get response body")?; + + let json: Value = serde_json::from_str(&body) + .context(format!("parse rpc response from {method} as json"))?; + + json["result"] + .as_str() + .context(format!( + "extract result field from json response to {method}" + )) + .map(|s| s.into()) } pub async fn get_container_logs(&self) -> String { From 135dd5828c5e2c95f394bb475bae0e4d6640bc95 Mon Sep 17 00:00:00 2001 From: jossduff Date: Tue, 8 Oct 2024 12:03:43 -0400 Subject: [PATCH 2/2] fix(test): 1 minute timeout on tests and refactor_ --- sigil/tests/integration.rs | 86 +++++++++++++++++++++++---------- sigil/tests/test_configs/c.toml | 3 ++ sigil/tests/test_helper.rs | 9 ++-- 3 files changed, 70 insertions(+), 28 deletions(-) create mode 100644 sigil/tests/test_configs/c.toml diff --git a/sigil/tests/integration.rs b/sigil/tests/integration.rs index 52854e0..a58b3e7 100644 --- a/sigil/tests/integration.rs +++ b/sigil/tests/integration.rs @@ -15,42 +15,78 @@ use test_helper::SigilTestInstance; #[tokio::test] #[serial] -async fn test_2_mdns_connections() { +async fn test_3_mdns_connections() { let sigil_a = SigilTestInstance::new("a.toml").await; let sigil_b = SigilTestInstance::new("b.toml").await; + let sigil_c = SigilTestInstance::new("c.toml").await; let sigil_a_peer_id = sigil_a.rpc("my_peer_id", None).await.unwrap(); let sigil_b_peer_id = sigil_b.rpc("my_peer_id", None).await.unwrap(); + let sigil_c_peer_id = sigil_c.rpc("my_peer_id", None).await.unwrap(); - sigil_a - .rpc_with_expected("connected_peers", None, &sigil_b_peer_id) - .await - .unwrap(); + let sigil_instances = vec![sigil_a, sigil_b, sigil_c]; + let sigil_peer_ids = vec![sigil_a_peer_id, sigil_b_peer_id, sigil_c_peer_id]; - sigil_a - .rpc_with_expected("gossipsub_mesh_peers", None, &sigil_b_peer_id) - .await - .unwrap(); + for (i, sigil_instance) in sigil_instances.iter().enumerate() { + for (j, sigil_peer_id) in sigil_peer_ids.iter().enumerate() { + // skip checking for the the peer_id of the current sigil_instance + if i == j { + continue; + } - sigil_a - .rpc_with_expected("kademlia_routing_table_peers", None, &sigil_b_peer_id) - .await - .unwrap(); + sigil_instance + .rpc_with_expected("connected_peers", None, sigil_peer_id) + .await + .unwrap(); - sigil_b - .rpc_with_expected("connected_peers", None, &sigil_a_peer_id) - .await - .unwrap(); + sigil_instance + .rpc_with_expected("kademlia_routing_table_peers", None, sigil_peer_id) + .await + .unwrap(); - sigil_b - .rpc_with_expected("gossipsub_mesh_peers", None, &sigil_a_peer_id) - .await - .unwrap(); + sigil_instance + .rpc_with_expected("gossipsub_mesh_peers", None, sigil_peer_id) + .await + .unwrap(); + } + } +} - sigil_b - .rpc_with_expected("kademlia_routing_table_peers", None, &sigil_a_peer_id) - .await - .unwrap(); +#[tokio::test] +#[serial] +async fn test_2_mdns_connections() { + let sigil_a = SigilTestInstance::new("a.toml").await; + let sigil_b = SigilTestInstance::new("b.toml").await; + + let sigil_a_peer_id = sigil_a.rpc("my_peer_id", None).await.unwrap(); + let sigil_b_peer_id = sigil_b.rpc("my_peer_id", None).await.unwrap(); + + let sigil_instances = vec![sigil_a, sigil_b]; + let sigil_peer_ids = vec![sigil_a_peer_id, sigil_b_peer_id]; + + for (i, sigil_instance) in sigil_instances.iter().enumerate() { + for (j, sigil_peer_id) in sigil_peer_ids.iter().enumerate() { + // skip checking for the the peer_id of the current sigil_instance + if i == j { + continue; + } + + sigil_instance + .rpc_with_expected("connected_peers", None, sigil_peer_id) + .await + .unwrap(); + + sigil_instance + .rpc_with_expected("kademlia_routing_table_peers", None, sigil_peer_id) + .await + .unwrap(); + + sigil_instance + .rpc_with_expected("gossipsub_mesh_peers", None, sigil_peer_id) + .await + .unwrap(); + } + } } #[tokio::test] diff --git a/sigil/tests/test_configs/c.toml b/sigil/tests/test_configs/c.toml new file mode 100644 index 0000000..7a0b87a --- /dev/null +++ b/sigil/tests/test_configs/c.toml @@ -0,0 +1,3 @@ + +[priory] +secret_key_seed = 4 diff --git a/sigil/tests/test_helper.rs b/sigil/tests/test_helper.rs index da0de5e..727a093 100644 --- a/sigil/tests/test_helper.rs +++ b/sigil/tests/test_helper.rs @@ -32,7 +32,7 @@ impl SigilTestInstance { .expect("Failed to start sigil container"); // give it a chance to make connections or crash - tokio::time::sleep(Duration::from_millis(500)).await; + tokio::time::sleep(Duration::from_millis(1000)).await; let host_port = container .get_host_port_ipv4(port) @@ -45,7 +45,10 @@ impl SigilTestInstance { .context("get internal ports") .unwrap(); - let reqwest_client = reqwest::Client::new(); + let reqwest_client = reqwest::ClientBuilder::new() + .timeout(Duration::from_secs(60)) + .build() + .unwrap(); Self { container, @@ -65,7 +68,7 @@ impl SigilTestInstance { if !response.contains(expected) { anyhow::bail!( - "Response does not contain expected text. \nexpected: {}\nactual: {}\n", + "Response to method {method} with params {params:?} does not contain expected text. \nexpected: {}\nactual: {}\n", expected, response );