From 3374f0bc8fd4d208b6ff139caaf1d6942915bfbb Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 3 Feb 2025 15:19:39 +0100 Subject: [PATCH 1/6] refactor: switch from iroh-test::logging to tracing-test --- Cargo.lock | 28 ++++- iroh-dns-server/Cargo.toml | 1 + iroh-dns-server/src/lib.rs | 10 +- iroh-net-report/Cargo.toml | 1 + iroh-net-report/src/dns.rs | 4 +- iroh-net-report/src/lib.rs | 6 +- iroh-net-report/src/ping.rs | 7 +- iroh-net-report/src/reportgen.rs | 10 +- iroh-net-report/src/reportgen/hairpin.rs | 8 +- iroh-net-report/src/reportgen/probes.rs | 9 +- iroh-relay/Cargo.toml | 1 + iroh-relay/src/client/connect_relay.rs | 4 +- iroh-relay/src/quic.rs | 5 +- iroh-relay/src/server.rs | 22 ++-- iroh-relay/src/server/client.rs | 7 +- iroh-relay/src/server/http_server.rs | 17 +-- iroh-test/Cargo.toml | 3 - iroh-test/src/lib.rs | 1 - iroh-test/src/logging.rs | 133 -------------------- iroh/Cargo.toml | 1 + iroh/src/discovery.rs | 21 ++-- iroh/src/discovery/local_swarm_discovery.rs | 6 +- iroh/src/discovery/pkarr/dht.rs | 3 +- iroh/src/dns.rs | 6 +- iroh/src/endpoint.rs | 16 +-- iroh/src/magicsock.rs | 25 ++-- iroh/src/magicsock/node_map.rs | 7 +- iroh/src/magicsock/relay_actor.rs | 5 +- iroh/src/magicsock/udp_conn.rs | 5 +- 29 files changed, 126 insertions(+), 246 deletions(-) delete mode 100644 iroh-test/src/logging.rs diff --git a/Cargo.lock b/Cargo.lock index 99582c76eab..dbac8dbf255 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2085,6 +2085,7 @@ dependencies = [ "tokio-util", "tracing", "tracing-subscriber", + "tracing-test", "url", "webpki-roots", "x509-parser", @@ -2177,6 +2178,7 @@ dependencies = [ "tower_governor", "tracing", "tracing-subscriber", + "tracing-test", "ttl_cache", "url", "z32", @@ -2227,6 +2229,7 @@ dependencies = [ "tokio", "tokio-util", "tracing", + "tracing-test", "url", ] @@ -2340,6 +2343,7 @@ dependencies = [ "toml", "tracing", "tracing-subscriber", + "tracing-test", "url", "webpki-roots", ] @@ -2361,9 +2365,6 @@ name = "iroh-test" version = "0.31.0" dependencies = [ "anyhow", - "tokio", - "tracing", - "tracing-subscriber", ] [[package]] @@ -4911,6 +4912,27 @@ dependencies = [ "tracing-log", ] +[[package]] +name = "tracing-test" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "557b891436fe0d5e0e363427fc7f217abf9ccd510d5136549847bdcbcd011d68" +dependencies = [ + "tracing-core", + "tracing-subscriber", + "tracing-test-macro", +] + +[[package]] +name = "tracing-test-macro" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04659ddb06c87d233c566112c1c9c5b9e98256d9af50ec3bc9c8327f873a7568" +dependencies = [ + "quote", + "syn 2.0.96", +] + [[package]] name = "try-lock" version = "0.2.5" diff --git a/iroh-dns-server/Cargo.toml b/iroh-dns-server/Cargo.toml index 32c796920ee..77c06e853ba 100644 --- a/iroh-dns-server/Cargo.toml +++ b/iroh-dns-server/Cargo.toml @@ -66,6 +66,7 @@ pkarr = { version = "2.3.1", features = ["rand"] } rand = "0.8" rand_chacha = "0.3.1" testresult = "0.4.1" +tracing-test = "0.2.5" [[bench]] name = "write" diff --git a/iroh-dns-server/src/lib.rs b/iroh-dns-server/src/lib.rs index d69b8a5ea42..c83a2aad1ec 100644 --- a/iroh-dns-server/src/lib.rs +++ b/iroh-dns-server/src/lib.rs @@ -34,6 +34,7 @@ mod tests { }; use pkarr::{PkarrClient, SignedPacket}; use testresult::TestResult; + use tracing_test::traced_test; use url::Url; use crate::{ @@ -45,8 +46,8 @@ mod tests { }; #[tokio::test] + #[traced_test] async fn pkarr_publish_dns_resolve() -> Result<()> { - iroh_test::logging::setup_multithreaded(); let (server, nameserver, http_url) = Server::spawn_for_tests().await?; let pkarr_relay_url = { let mut url = http_url.clone(); @@ -158,8 +159,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn integration_smoke() -> Result<()> { - iroh_test::logging::setup_multithreaded(); let (server, nameserver, http_url) = Server::spawn_for_tests().await?; let pkarr_relay = { @@ -190,8 +191,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn store_eviction() -> TestResult<()> { - iroh_test::logging::setup_multithreaded(); let options = ZoneStoreOptions { eviction: Duration::from_millis(100), eviction_interval: Duration::from_millis(100), @@ -220,9 +221,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn integration_mainline() -> Result<()> { - iroh_test::logging::setup_multithreaded(); - // run a mainline testnet let testnet = pkarr::mainline::dht::Testnet::new(5); let bootstrap = testnet.bootstrap.clone(); diff --git a/iroh-net-report/Cargo.toml b/iroh-net-report/Cargo.toml index 3a8078af42d..6c91e175bc3 100644 --- a/iroh-net-report/Cargo.toml +++ b/iroh-net-report/Cargo.toml @@ -43,6 +43,7 @@ iroh-test = { path = "../iroh-test" } pretty_assertions = "1.4" testresult = "0.4.0" tokio = { version = "1", default-features = false, features = ["test-util"] } +tracing-test = "0.2.5" [features] default = ["metrics"] diff --git a/iroh-net-report/src/dns.rs b/iroh-net-report/src/dns.rs index 659b6d6d3fb..032ceae1f1f 100644 --- a/iroh-net-report/src/dns.rs +++ b/iroh-net-report/src/dns.rs @@ -187,6 +187,8 @@ pub(crate) mod tests { sync::{atomic::AtomicUsize, OnceLock}, }; + use tracing_test::traced_test; + use super::*; static DNS_RESOLVER: OnceLock = OnceLock::new(); @@ -243,8 +245,8 @@ pub(crate) mod tests { } #[tokio::test] + #[traced_test] async fn stagger_basic() { - let _logging = iroh_test::logging::setup(); const CALL_RESULTS: &[Result] = &[Err(2), Ok(3), Ok(5), Ok(7)]; static DONE_CALL: AtomicUsize = AtomicUsize::new(0); let f = || { diff --git a/iroh-net-report/src/lib.rs b/iroh-net-report/src/lib.rs index 774b1d74909..0f6aee022ac 100644 --- a/iroh-net-report/src/lib.rs +++ b/iroh-net-report/src/lib.rs @@ -1006,6 +1006,7 @@ mod tests { use netwatch::IpFamily; use tokio_util::sync::CancellationToken; use tracing::info; + use tracing_test::traced_test; use super::*; use crate::{ping::Pinger, stun_utils::bind_local_stun_socket}; @@ -1143,8 +1144,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_basic() -> Result<()> { - let _guard = iroh_test::logging::setup(); let (stun_addr, stun_stats, _cleanup_guard) = stun_utils::serve("127.0.0.1".parse().unwrap()).await?; @@ -1186,9 +1187,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_udp_blocked() -> Result<()> { - let _guard = iroh_test::logging::setup(); - // Create a "STUN server", which will never respond to anything. This is how UDP to // the STUN server being blocked will look like from the client's perspective. let blackhole = tokio::net::UdpSocket::bind("127.0.0.1:0").await?; diff --git a/iroh-net-report/src/ping.rs b/iroh-net-report/src/ping.rs index 2dcebe403e7..1034bd30909 100644 --- a/iroh-net-report/src/ping.rs +++ b/iroh-net-report/src/ping.rs @@ -124,14 +124,14 @@ mod tests { use std::net::{Ipv4Addr, Ipv6Addr}; use tracing::error; + use tracing_test::traced_test; use super::*; #[tokio::test] #[ignore] // Doesn't work in CI + #[traced_test] async fn test_ping_google() -> Result<()> { - let _guard = iroh_test::logging::setup(); - // Public DNS addrs from google based on // https://developers.google.com/speed/public-dns/docs/using @@ -159,9 +159,8 @@ mod tests { // See net_report::reportgen::tests::test_icmp_probe_eu_relay for permissions to ping. #[tokio::test] + #[traced_test] async fn test_ping_localhost() { - let _guard = iroh_test::logging::setup(); - let pinger = Pinger::new(); match pinger.send(Ipv4Addr::LOCALHOST.into(), b"data").await { diff --git a/iroh-net-report/src/reportgen.rs b/iroh-net-report/src/reportgen.rs index 2c8acafdda1..b648c96949d 100644 --- a/iroh-net-report/src/reportgen.rs +++ b/iroh-net-report/src/reportgen.rs @@ -1356,12 +1356,13 @@ mod tests { use std::net::{Ipv4Addr, Ipv6Addr}; use testresult::TestResult; + use tracing_test::traced_test; use super::{super::test_utils, *}; #[tokio::test] + #[traced_test] async fn test_update_report_stun_working() { - let _logging = iroh_test::logging::setup(); let (_server_a, relay_a) = test_utils::relay().await; let (_server_b, relay_b) = test_utils::relay().await; @@ -1446,8 +1447,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_update_report_icmp() { - let _logging = iroh_test::logging::setup(); let (_server_a, relay_a) = test_utils::relay().await; let (_server_b, relay_b) = test_utils::relay().await; @@ -1556,8 +1557,8 @@ mod tests { // // TODO: Not sure what about IPv6 pings using sysctl. #[tokio::test] + #[traced_test] async fn test_icmpk_probe() { - let _logging_guard = iroh_test::logging::setup(); let pinger = Pinger::new(); let (server, node) = test_utils::relay().await; let addr = server.stun_addr().expect("test relay serves stun"); @@ -1607,7 +1608,6 @@ mod tests { #[tokio::test] async fn test_measure_https_latency() -> TestResult { - let _logging_guard = iroh_test::logging::setup(); let (server, relay) = test_utils::relay().await; let dns_resolver = crate::dns::tests::resolver(); tracing::info!(relay_url = ?relay.url , "RELAY_URL"); @@ -1626,8 +1626,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_quic_probe() -> TestResult { - let _logging_guard = iroh_test::logging::setup(); let (server, relay) = test_utils::relay().await; let client_config = iroh_relay::client::make_dangerous_client_config(); let ep = quinn::Endpoint::client(SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 0))?; diff --git a/iroh-net-report/src/reportgen/hairpin.rs b/iroh-net-report/src/reportgen/hairpin.rs index 15c67da38a8..e082ff93f6c 100644 --- a/iroh-net-report/src/reportgen/hairpin.rs +++ b/iroh-net-report/src/reportgen/hairpin.rs @@ -181,10 +181,12 @@ mod tests { use bytes::BytesMut; use tokio::sync::mpsc; use tracing::info; + use tracing_test::traced_test; use super::*; #[tokio::test] + #[traced_test] async fn test_hairpin_success() { for i in 0..100 { let now = Instant::now(); @@ -194,13 +196,12 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_hairpin_failure() { test_hairpin(false).await; } async fn test_hairpin(hairpinning_works: bool) { - let _guard = iroh_test::logging::setup(); - // Setup fake net_report and reportstate actors, hairpinning interacts with them. let (net_report_tx, mut net_report_rx) = mpsc::channel(32); let net_report_addr = net_report::Addr { @@ -275,9 +276,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_client_drop() { - let _guard = iroh_test::logging::setup(); - // Setup fake net_report and reportstate actors, hairpinning interacts with them. let (net_report_tx, _net_report_rx) = mpsc::channel(32); let net_report_addr = net_report::Addr { diff --git a/iroh-net-report/src/reportgen/probes.rs b/iroh-net-report/src/reportgen/probes.rs index 8257928cace..f9ba59bf60d 100644 --- a/iroh-net-report/src/reportgen/probes.rs +++ b/iroh-net-report/src/reportgen/probes.rs @@ -566,6 +566,7 @@ fn sort_relays<'a>( #[cfg(test)] mod tests { use pretty_assertions::assert_eq; + use tracing_test::traced_test; use super::*; use crate::{test_utils, RelayLatencies}; @@ -796,8 +797,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_plan_with_report() { - let _logging = iroh_test::logging::setup(); let (_servers, relay_map) = test_utils::relay_map(2).await; let relay_node_1 = relay_map.nodes().next().unwrap().clone(); let relay_node_2 = relay_map.nodes().nth(1).unwrap().clone(); @@ -988,8 +989,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_sort_two_latencies() { - let _logging = iroh_test::logging::setup(); let (_servers, relay_map) = test_utils::relay_map(2).await; let r1 = relay_map.nodes().next().unwrap(); let r2 = relay_map.nodes().nth(1).unwrap(); @@ -1007,8 +1008,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_sort_equal_latencies() { - let _logging = iroh_test::logging::setup(); let (_servers, relay_map) = test_utils::relay_map(2).await; let r1 = relay_map.nodes().next().unwrap(); let r2 = relay_map.nodes().nth(1).unwrap(); @@ -1049,8 +1050,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_sort_no_latency() { - let _logging = iroh_test::logging::setup(); let (_servers, relay_map) = test_utils::relay_map(2).await; let r1 = relay_map.nodes().next().unwrap(); let r2 = relay_map.nodes().nth(1).unwrap(); diff --git a/iroh-relay/Cargo.toml b/iroh-relay/Cargo.toml index 028e033875e..5e6f9d4038c 100644 --- a/iroh-relay/Cargo.toml +++ b/iroh-relay/Cargo.toml @@ -118,6 +118,7 @@ tokio = { version = "1", features = [ tracing-subscriber = { version = "0.3", features = ["env-filter"] } iroh-test = { path = "../iroh-test" } serde_json = "1" +tracing-test = "0.2.5" [build-dependencies] cfg_aliases = { version = "0.2" } diff --git a/iroh-relay/src/client/connect_relay.rs b/iroh-relay/src/client/connect_relay.rs index 329ecce1218..5445b7fbc83 100644 --- a/iroh-relay/src/client/connect_relay.rs +++ b/iroh-relay/src/client/connect_relay.rs @@ -369,13 +369,13 @@ mod tests { use std::str::FromStr; use anyhow::Result; + use tracing_test::traced_test; use super::*; #[test] + #[traced_test] fn test_host_header_value() -> Result<()> { - let _guard = iroh_test::logging::setup(); - let cases = [ ( "https://euw1-1.relay.iroh.network.", diff --git a/iroh-relay/src/quic.rs b/iroh-relay/src/quic.rs index c99fe58095f..51b1addfecf 100644 --- a/iroh-relay/src/quic.rs +++ b/iroh-relay/src/quic.rs @@ -272,16 +272,17 @@ impl QuicClient { mod tests { use std::net::Ipv4Addr; + use tracing_test::traced_test; + use super::{ server::{QuicConfig, QuicServer}, *, }; #[tokio::test] + #[traced_test] async fn quic_endpoint_basic() -> anyhow::Result<()> { let host: Ipv4Addr = "127.0.0.1".parse()?; - let _guard = iroh_test::logging::setup(); - // create a server config with self signed certificates let (_, server_config) = super::super::server::testing::self_signed_tls_certs_and_config(); let bind_addr = SocketAddr::new(host.into(), 0); diff --git a/iroh-relay/src/server.rs b/iroh-relay/src/server.rs index dfe2c4d488e..bfe00dad3b6 100644 --- a/iroh-relay/src/server.rs +++ b/iroh-relay/src/server.rs @@ -812,6 +812,7 @@ mod tests { use iroh_base::{NodeId, SecretKey}; use n0_future::{FutureExt, SinkExt}; use testresult::TestResult; + use tracing_test::traced_test; use super::*; use crate::{ @@ -856,8 +857,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_no_services() { - let _guard = iroh_test::logging::setup(); let mut server = Server::spawn(ServerConfig::<(), ()>::default()) .await .unwrap(); @@ -869,8 +870,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_conflicting_bind() { - let _guard = iroh_test::logging::setup(); let mut server = Server::spawn(ServerConfig::<(), ()> { relay: Some(RelayConfig { http_bind_addr: (Ipv4Addr::LOCALHOST, 1234).into(), @@ -893,8 +894,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_root_handler() { - let _guard = iroh_test::logging::setup(); let server = spawn_local_relay().await.unwrap(); let url = format!("http://{}", server.http_addr().unwrap()); @@ -905,8 +906,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_captive_portal_service() { - let _guard = iroh_test::logging::setup(); let server = spawn_local_relay().await.unwrap(); let url = format!("http://{}/generate_204", server.http_addr().unwrap()); let challenge = "123az__."; @@ -926,8 +927,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_client_legacy_route() { - let _guard = iroh_test::logging::setup(); let server = spawn_local_relay().await.unwrap(); // We're testing the legacy endpoint at `/derp` let endpoint_url = format!("http://{}/derp", server.http_addr().unwrap()); @@ -944,8 +945,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_clients_both_relay() -> TestResult<()> { - let _guard = iroh_test::logging::setup(); let server = spawn_local_relay().await.unwrap(); let relay_url = format!("http://{}", server.http_addr().unwrap()); let relay_url: RelayUrl = relay_url.parse().unwrap(); @@ -996,8 +997,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_clients_both_websockets() -> TestResult<()> { - let _guard = iroh_test::logging::setup(); let server = spawn_local_relay().await?; let relay_url = format!("http://{}", server.http_addr().unwrap()); @@ -1058,8 +1059,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_clients_websocket_and_relay() -> TestResult<()> { - let _guard = iroh_test::logging::setup(); let server = spawn_local_relay().await.unwrap(); let relay_url = format!("http://{}", server.http_addr().unwrap()); @@ -1114,8 +1115,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_stun() { - let _guard = iroh_test::logging::setup(); let server = Server::spawn(ServerConfig::<(), ()> { relay: None, stun: Some(StunConfig { @@ -1146,9 +1147,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_relay_access_control() -> Result<()> { - let _guard = iroh_test::logging::setup(); - let a_secret_key = SecretKey::generate(rand::thread_rng()); let a_key = a_secret_key.public(); diff --git a/iroh-relay/src/server/client.rs b/iroh-relay/src/server/client.rs index a3c541b0cc9..3e8aa934a3d 100644 --- a/iroh-relay/src/server/client.rs +++ b/iroh-relay/src/server/client.rs @@ -559,6 +559,7 @@ mod tests { use testresult::TestResult; use tokio_util::codec::Framed; use tracing::info; + use tracing_test::traced_test; use super::*; use crate::{ @@ -567,9 +568,8 @@ mod tests { }; #[tokio::test] + #[traced_test] async fn test_client_actor_basic() -> Result<()> { - let _logging = iroh_test::logging::setup(); - let (send_queue_s, send_queue_r) = mpsc::channel(10); let (disco_send_queue_s, disco_send_queue_r) = mpsc::channel(10); let (peer_gone_s, peer_gone_r) = mpsc::channel(10); @@ -678,9 +678,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_rate_limit() -> TestResult { - let _logging = iroh_test::logging::setup(); - const LIMIT: u32 = 50; const MAX_FRAMES: u32 = 100; diff --git a/iroh-relay/src/server/http_server.rs b/iroh-relay/src/server/http_server.rs index 690a19fbf0d..08e57007bee 100644 --- a/iroh-relay/src/server/http_server.rs +++ b/iroh-relay/src/server/http_server.rs @@ -706,7 +706,7 @@ mod tests { use n0_future::{SinkExt, StreamExt}; use reqwest::Url; use tracing::info; - use tracing_subscriber::{prelude::*, EnvFilter}; + use tracing_test::traced_test; use super::*; use crate::client::{ @@ -740,9 +740,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_http_clients_and_server() -> Result<()> { - let _guard = iroh_test::logging::setup(); - let a_key = SecretKey::generate(rand::thread_rng()); let b_key = SecretKey::generate(rand::thread_rng()); @@ -846,9 +845,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_https_clients_and_server() -> Result<()> { - let _logging = iroh_test::logging::setup(); - let a_key = SecretKey::generate(rand::thread_rng()); let b_key = SecretKey::generate(rand::thread_rng()); @@ -929,9 +927,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_server_basic() -> Result<()> { - let _guard = iroh_test::logging::setup(); - info!("Create the server."); let service = RelayService::new( Default::default(), @@ -1019,12 +1016,6 @@ mod tests { #[tokio::test] async fn test_server_replace_client() -> Result<()> { - tracing_subscriber::registry() - .with(tracing_subscriber::fmt::layer().with_writer(std::io::stderr)) - .with(EnvFilter::from_default_env()) - .try_init() - .ok(); - info!("Create the server."); let service = RelayService::new( Default::default(), diff --git a/iroh-test/Cargo.toml b/iroh-test/Cargo.toml index 45b90582ec6..a20f81bc655 100644 --- a/iroh-test/Cargo.toml +++ b/iroh-test/Cargo.toml @@ -17,9 +17,6 @@ workspace = true [dependencies] anyhow = "1" -tokio = { version = "1", features = ["full"] } -tracing = "0.1" -tracing-subscriber = { version = "0.3", features = ["env-filter"] } [package.metadata.docs.rs] all-features = true diff --git a/iroh-test/src/lib.rs b/iroh-test/src/lib.rs index 1a1694c6c22..55085248d16 100644 --- a/iroh-test/src/lib.rs +++ b/iroh-test/src/lib.rs @@ -1,7 +1,6 @@ //! Internal utilities to support testing. pub mod hexdump; -pub mod logging; // #[derive(derive_more::Debug)] #[allow(missing_debug_implementations)] diff --git a/iroh-test/src/logging.rs b/iroh-test/src/logging.rs deleted file mode 100644 index cdc739c4893..00000000000 --- a/iroh-test/src/logging.rs +++ /dev/null @@ -1,133 +0,0 @@ -//! Logging during tests. - -use tokio::runtime::RuntimeFlavor; -use tracing::level_filters::LevelFilter; -use tracing_subscriber::{ - layer::{Layer, SubscriberExt}, - util::SubscriberInitExt, - EnvFilter, -}; - -/// Configures logging for the current test, **single-threaded runtime only**. -/// -/// This setup can be used for any sync test or async test using a single-threaded tokio -/// runtime (the default). -/// -/// This configures logging that will interact well with tests: logs will be captured by the -/// test framework and only printed on failure. -/// -/// The logging is unfiltered, it logs all crates and modules on TRACE level. If that's too -/// much consider if your test is too large (or write a version that allows filtering...). -/// -/// # Example -/// -/// ``` -/// #[tokio::test] -/// async fn test_something() { -/// let _guard = iroh_test::logging::setup(); -/// assert!(true); -/// } -/// ``` -#[must_use = "The tracing guard must only be dropped at the end of the test"] -pub fn setup() -> tracing::subscriber::DefaultGuard { - if let Ok(handle) = tokio::runtime::Handle::try_current() { - match handle.runtime_flavor() { - RuntimeFlavor::CurrentThread => (), - RuntimeFlavor::MultiThread => { - panic!("setup_logging() does not work in a multi-threaded tokio runtime"); - } - _ => panic!("unknown runtime flavour"), - } - } - testing_subscriber().set_default() -} - -/// The first call to this function will install a global logger. -/// -/// The logger uses the `RUST_LOG` environment variable to decide on what level to log -/// anything, which is set by our CI. When running the tests with nextest the log -/// output will be captured for just the executing test. -/// -/// Logs to stdout since the assertion messages are logged on stderr by default. -pub fn setup_multithreaded() { - tracing_subscriber::registry() - .with( - tracing_subscriber::fmt::layer() - .event_format(tracing_subscriber::fmt::format().with_line_number(true)) - .with_writer(std::io::stdout), - ) - .with(EnvFilter::from_default_env()) - .try_init() - .ok(); -} - -// /// Invoke the future with test logging configured. -// /// -// /// This can be used to execute any future which uses tracing for logging, it sets up the -// /// logging as [`setup_logging`] does but in a way which will work for both single and -// /// multi-threaded tokio runtimes. -// pub(crate) async fn with_logging(f: F) -> F::Output { -// f.with_subscriber(testing_subscriber()).await -// } - -/// Returns the a [`tracing::Subscriber`] configured for our tests. -/// -/// This subscriber will ensure that log output is captured by the test's default output -/// capturing and thus is only shown with the test on failure. By default it uses -/// `RUST_LOG=trace` as configuration but you can specify the `RUST_LOG` environment -/// variable explicitly to override this. -/// -/// To use this in a tokio multi-threaded runtime use: -/// -/// ```ignore -/// use tracing_future::WithSubscriber; -/// use iroh_test::logging::testing_subscriber; -/// -/// #[tokio::test(flavor = "multi_thread")] -/// async fn test_something() -> Result<()> { -/// async move { -/// Ok(()) -/// }.with_subscriber(testing_subscriber()).await -/// } -/// ``` -pub fn testing_subscriber() -> impl tracing::Subscriber { - let var = std::env::var_os("RUST_LOG"); - let trace_log_layer = match var { - Some(_) => None, - None => Some( - tracing_subscriber::fmt::layer() - .event_format(tracing_subscriber::fmt::format().with_line_number(true)) - .with_writer(|| TestWriter) - .with_filter(LevelFilter::TRACE), - ), - }; - let env_log_layer = var.map(|_| { - tracing_subscriber::fmt::layer() - .event_format(tracing_subscriber::fmt::format().with_line_number(true)) - .with_writer(|| TestWriter) - .with_filter(EnvFilter::from_default_env()) - }); - tracing_subscriber::registry() - .with(trace_log_layer) - .with(env_log_layer) -} - -/// A tracing writer that interacts well with test output capture. -/// -/// Using this writer will make sure that the output is captured normally and only printed -/// when the test fails. See [`setup`] to actually use this. -#[derive(Debug)] -struct TestWriter; - -impl std::io::Write for TestWriter { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - print!( - "{}", - std::str::from_utf8(buf).expect("tried to log invalid UTF-8") - ); - Ok(buf.len()) - } - fn flush(&mut self) -> std::io::Result<()> { - std::io::stdout().flush() - } -} diff --git a/iroh/Cargo.toml b/iroh/Cargo.toml index 1fed04bf0fe..5d4c1b7c80c 100644 --- a/iroh/Cargo.toml +++ b/iroh/Cargo.toml @@ -129,6 +129,7 @@ iroh-test = { path = "../iroh-test" } serde_json = "1" testresult = "0.4.0" iroh-relay = { path = "../iroh-relay", default-features = false, features = ["test-utils", "server"] } +tracing-test = "0.2.5" [features] default = ["metrics"] diff --git a/iroh/src/discovery.rs b/iroh/src/discovery.rs index bd359af3b60..d42ca87d440 100644 --- a/iroh/src/discovery.rs +++ b/iroh/src/discovery.rs @@ -446,6 +446,7 @@ mod tests { use rand::Rng; use testresult::TestResult; use tokio_util::task::AbortOnDropHandle; + use tracing_test::traced_test; use super::*; use crate::RelayMode; @@ -563,8 +564,8 @@ mod tests { /// This is a smoke test for our discovery mechanism. #[tokio::test] + #[traced_test] async fn endpoint_discovery_simple_shared() -> anyhow::Result<()> { - let _guard = iroh_test::logging::setup(); let disco_shared = TestDiscoveryShared::default(); let (ep1, _guard1) = { let secret = SecretKey::generate(rand::thread_rng()); @@ -585,8 +586,8 @@ mod tests { /// This test adds an empty discovery which provides no addresses. #[tokio::test] + #[traced_test] async fn endpoint_discovery_combined_with_empty() -> anyhow::Result<()> { - let _guard = iroh_test::logging::setup(); let disco_shared = TestDiscoveryShared::default(); let (ep1, _guard1) = { let secret = SecretKey::generate(rand::thread_rng()); @@ -616,8 +617,8 @@ mod tests { /// This is to make sure that as long as one of the discoveries returns a working address, we /// will connect successfully. #[tokio::test] + #[traced_test] async fn endpoint_discovery_combined_with_empty_and_wrong() -> anyhow::Result<()> { - let _guard = iroh_test::logging::setup(); let disco_shared = TestDiscoveryShared::default(); let (ep1, _guard1) = { let secret = SecretKey::generate(rand::thread_rng()); @@ -644,8 +645,8 @@ mod tests { /// This test only has the "lying" discovery. It is here to make sure that this actually fails. #[tokio::test] + #[traced_test] async fn endpoint_discovery_combined_wrong_only() -> anyhow::Result<()> { - let _guard = iroh_test::logging::setup(); let disco_shared = TestDiscoveryShared::default(); let (ep1, _guard1) = { let secret = SecretKey::generate(rand::thread_rng()); @@ -669,8 +670,8 @@ mod tests { /// This test first adds a wrong address manually (e.g. from an outdated&node_id ticket). /// Connect should still succeed because the discovery service will be invoked (after a delay). #[tokio::test] + #[traced_test] async fn endpoint_discovery_with_wrong_existing_addr() -> anyhow::Result<()> { - let _guard = iroh_test::logging::setup(); let disco_shared = TestDiscoveryShared::default(); let (ep1, _guard1) = { let secret = SecretKey::generate(rand::thread_rng()); @@ -759,6 +760,7 @@ mod test_dns_pkarr { use iroh_relay::RelayMap; use n0_future::time::Duration; use tokio_util::task::AbortOnDropHandle; + use tracing_test::traced_test; use crate::{ discovery::pkarr::PkarrPublisher, @@ -774,9 +776,8 @@ mod test_dns_pkarr { const PUBLISH_TIMEOUT: Duration = Duration::from_secs(10); #[tokio::test] + #[traced_test] async fn dns_resolve() -> Result<()> { - let _logging_guard = iroh_test::logging::setup(); - let origin = "testdns.example".to_string(); let state = State::new(origin.clone()); let (nameserver, _dns_drop_guard) = run_dns_server(state.clone()).await?; @@ -799,9 +800,8 @@ mod test_dns_pkarr { } #[tokio::test] + #[traced_test] async fn pkarr_publish_dns_resolve() -> Result<()> { - let _logging_guard = iroh_test::logging::setup(); - let origin = "testdns.example".to_string(); let dns_pkarr_server = DnsPkarrServer::run_with_origin(origin.clone()).await?; @@ -832,9 +832,8 @@ mod test_dns_pkarr { const TEST_ALPN: &[u8] = b"TEST"; #[tokio::test] + #[traced_test] async fn pkarr_publish_dns_discover() -> Result<()> { - let _logging_guard = iroh_test::logging::setup(); - let dns_pkarr_server = DnsPkarrServer::run().await?; let (relay_map, _relay_url, _relay_guard) = run_relay_server().await?; diff --git a/iroh/src/discovery/local_swarm_discovery.rs b/iroh/src/discovery/local_swarm_discovery.rs index e1e53cc2c55..dd4be23d0c3 100644 --- a/iroh/src/discovery/local_swarm_discovery.rs +++ b/iroh/src/discovery/local_swarm_discovery.rs @@ -404,12 +404,13 @@ mod tests { use iroh_base::SecretKey; use n0_future::StreamExt; use testresult::TestResult; + use tracing_test::traced_test; use super::super::*; #[tokio::test] + #[traced_test] async fn local_swarm_discovery_publish_resolve() -> TestResult { - let _guard = iroh_test::logging::setup(); let (_, discovery_a) = make_discoverer()?; let (node_id_b, discovery_b) = make_discoverer()?; @@ -441,9 +442,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn local_swarm_discovery_subscribe() -> TestResult { - let _guard = iroh_test::logging::setup(); - let num_nodes = 5; let mut node_ids = BTreeSet::new(); let mut discoverers = vec![]; diff --git a/iroh/src/discovery/pkarr/dht.rs b/iroh/src/discovery/pkarr/dht.rs index c29a09e54ad..0a047de96d1 100644 --- a/iroh/src/discovery/pkarr/dht.rs +++ b/iroh/src/discovery/pkarr/dht.rs @@ -410,13 +410,14 @@ mod tests { use iroh_base::RelayUrl; use pkarr::mainline::dht::DhtSettings; use testresult::TestResult; + use tracing_test::traced_test; use super::*; #[tokio::test] #[ignore = "flaky"] + #[traced_test] async fn dht_discovery_smoke() -> TestResult { - let _logging_guard = iroh_test::logging::setup(); let ep = crate::Endpoint::builder().bind().await?; let secret = ep.secret_key().clone(); let testnet = pkarr::mainline::dht::Testnet::new(2); diff --git a/iroh/src/dns.rs b/iroh/src/dns.rs index 6ac764f23be..7795ab40ba7 100644 --- a/iroh/src/dns.rs +++ b/iroh/src/dns.rs @@ -383,14 +383,16 @@ async fn stagger_call Fut, Fut: Future>>( pub(crate) mod tests { use std::sync::atomic::AtomicUsize; + use tracing_test::traced_test; + use super::*; use crate::defaults::staging::NA_RELAY_HOSTNAME; const TIMEOUT: Duration = Duration::from_secs(5); const STAGGERING_DELAYS: &[u64] = &[200, 300]; #[tokio::test] + #[traced_test] async fn test_dns_lookup_ipv4_ipv6() { - let _logging = iroh_test::logging::setup(); let resolver = default_resolver(); let res: Vec<_> = resolver .lookup_ipv4_ipv6_staggered(NA_RELAY_HOSTNAME, TIMEOUT, STAGGERING_DELAYS) @@ -402,8 +404,8 @@ pub(crate) mod tests { } #[tokio::test] + #[traced_test] async fn stagger_basic() { - let _logging = iroh_test::logging::setup(); const CALL_RESULTS: &[Result] = &[Err(2), Ok(3), Ok(5), Ok(7)]; static DONE_CALL: AtomicUsize = AtomicUsize::new(0); let f = || { diff --git a/iroh/src/endpoint.rs b/iroh/src/endpoint.rs index 9a4df20be2d..db841f99e56 100644 --- a/iroh/src/endpoint.rs +++ b/iroh/src/endpoint.rs @@ -1728,6 +1728,7 @@ mod tests { use n0_future::StreamExt; use rand::SeedableRng; use tracing::{error_span, info, info_span, Instrument}; + use tracing_test::traced_test; use super::*; use crate::test_utils::{run_relay_server, run_relay_server_with}; @@ -1735,8 +1736,8 @@ mod tests { const TEST_ALPN: &[u8] = b"n0/iroh/test"; #[tokio::test] + #[traced_test] async fn test_connect_self() { - let _guard = iroh_test::logging::setup(); let ep = Endpoint::builder() .alpns(vec![TEST_ALPN.to_vec()]) .bind() @@ -1755,8 +1756,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn endpoint_connect_close() { - let _guard = iroh_test::logging::setup(); let (relay_map, relay_url, _guard) = run_relay_server().await.unwrap(); let server_secret_key = SecretKey::generate(rand::thread_rng()); let server_peer_id = server_secret_key.public(); @@ -1849,9 +1850,8 @@ mod tests { /// Test that peers are properly restored #[tokio::test] + #[traced_test] async fn restore_peers() { - let _guard = iroh_test::logging::setup(); - let secret_key = SecretKey::generate(rand::thread_rng()); /// Create an endpoint for the test. @@ -1903,8 +1903,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn endpoint_relay_connect_loop() { - let _logging_guard = iroh_test::logging::setup(); let start = Instant::now(); let n_clients = 5; let n_chunks_per_client = 2; @@ -2010,8 +2010,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn endpoint_bidi_send_recv() { - let _logging_guard = iroh_test::logging::setup(); let ep1 = Endpoint::builder() .alpns(vec![TEST_ALPN.to_vec()]) .relay_mode(RelayMode::Disabled) @@ -2100,9 +2100,9 @@ mod tests { } #[tokio::test] + #[traced_test] async fn endpoint_conn_type_stream() { const TIMEOUT: Duration = std::time::Duration::from_secs(15); - let _logging_guard = iroh_test::logging::setup(); let (relay_map, _relay_url, _relay_guard) = run_relay_server().await.unwrap(); let mut rng = rand_chacha::ChaCha8Rng::seed_from_u64(42); let ep1_secret_key = SecretKey::generate(&mut rng); @@ -2184,8 +2184,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_direct_addresses_no_stun_relay() { - let _guard = iroh_test::logging::setup(); let (relay_map, _, _guard) = run_relay_server_with(None, false).await.unwrap(); let ep = Endpoint::builder() diff --git a/iroh/src/magicsock.rs b/iroh/src/magicsock.rs index 9435568104e..98a33b9020a 100644 --- a/iroh/src/magicsock.rs +++ b/iroh/src/magicsock.rs @@ -3141,6 +3141,7 @@ mod tests { use iroh_test::CallOnDrop; use rand::RngCore; use tokio_util::task::AbortOnDropHandle; + use tracing_test::traced_test; use super::*; use crate::{ @@ -3426,9 +3427,8 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] + #[traced_test] async fn test_two_devices_roundtrip_quinn_magic() -> Result<()> { - iroh_test::logging::setup_multithreaded(); - let m1 = MagicStack::new(RelayMode::Disabled).await?; let m2 = MagicStack::new(RelayMode::Disabled).await?; @@ -3462,9 +3462,9 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_regression_network_change_rebind_wakes_connection_driver( ) -> testresult::TestResult { - let _ = iroh_test::logging::setup(); let m1 = MagicStack::new(RelayMode::Disabled).await?; let m2 = MagicStack::new(RelayMode::Disabled).await?; @@ -3501,6 +3501,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] + #[traced_test] async fn test_two_devices_roundtrip_network_change() -> Result<()> { time::timeout( Duration::from_secs(90), @@ -3512,8 +3513,6 @@ mod tests { /// Same structure as `test_two_devices_roundtrip_quinn_magic`, but interrupts regularly /// with (simulated) network changes. async fn test_two_devices_roundtrip_network_change_impl() -> Result<()> { - iroh_test::logging::setup_multithreaded(); - let m1 = MagicStack::new(RelayMode::Disabled).await?; let m2 = MagicStack::new(RelayMode::Disabled).await?; @@ -3616,8 +3615,8 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] + #[traced_test] async fn test_two_devices_setup_teardown() -> Result<()> { - iroh_test::logging::setup_multithreaded(); for i in 0..10 { println!("-- round {i}"); println!("setting up magic stack"); @@ -3639,9 +3638,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_two_devices_roundtrip_quinn_raw() -> Result<()> { - let _guard = iroh_test::logging::setup(); - let make_conn = |addr: SocketAddr| -> anyhow::Result { let key = SecretKey::generate(rand::thread_rng()); let conn = std::net::UdpSocket::bind(addr)?; @@ -3788,9 +3786,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_two_devices_roundtrip_quinn_rebinding_conn() -> Result<()> { - let _guard = iroh_test::logging::setup(); - fn make_conn(addr: SocketAddr) -> anyhow::Result { let key = SecretKey::generate(rand::thread_rng()); let conn = UdpConn::bind(addr)?; @@ -3981,8 +3978,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_local_endpoints() { - let _guard = iroh_test::logging::setup(); let ms = Handle::new(Default::default()).await.unwrap(); // See if we can get endpoints. @@ -4105,11 +4102,10 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_try_send_no_send_addr() { // Regression test: if there is no send_addr we should keep being able to use the // Endpoint. - let _guard = iroh_test::logging::setup(); - let secret_key_1 = SecretKey::from_bytes(&[1u8; 32]); let secret_key_2 = SecretKey::from_bytes(&[2u8; 32]); let node_id_2 = secret_key_2.public(); @@ -4196,11 +4192,10 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_try_send_no_udp_addr_or_relay_url() { // This specifically tests the `if udp_addr.is_none() && relay_url.is_none()` // behaviour of MagicSock::try_send. - let _logging_guard = iroh_test::logging::setup(); - let secret_key_1 = SecretKey::from_bytes(&[1u8; 32]); let secret_key_2 = SecretKey::from_bytes(&[2u8; 32]); let node_id_2 = secret_key_2.public(); diff --git a/iroh/src/magicsock/node_map.rs b/iroh/src/magicsock/node_map.rs index 1e789f3fcee..65efa3074b6 100644 --- a/iroh/src/magicsock/node_map.rs +++ b/iroh/src/magicsock/node_map.rs @@ -698,6 +698,7 @@ mod tests { use std::net::Ipv4Addr; use iroh_base::SecretKey; + use tracing_test::traced_test; use super::{node_state::MAX_INACTIVE_DIRECT_ADDRESSES, *}; @@ -715,9 +716,8 @@ mod tests { /// Test persisting and loading of known nodes. #[tokio::test] + #[traced_test] async fn restore_from_vec() { - let _guard = iroh_test::logging::setup(); - let node_map = NodeMap::default(); let mut rng = rand::thread_rng(); @@ -781,9 +781,8 @@ mod tests { } #[test] + #[traced_test] fn test_prune_direct_addresses() { - let _guard = iroh_test::logging::setup(); - let node_map = NodeMap::default(); let public_key = SecretKey::generate(rand::thread_rng()).public(); let id = node_map diff --git a/iroh/src/magicsock/relay_actor.rs b/iroh/src/magicsock/relay_actor.rs index 6d2d4637374..1ceeee05318 100644 --- a/iroh/src/magicsock/relay_actor.rs +++ b/iroh/src/magicsock/relay_actor.rs @@ -1200,6 +1200,7 @@ mod tests { use testresult::TestResult; use tokio_util::task::AbortOnDropHandle; use tracing::info; + use tracing_test::traced_test; use super::*; use crate::test_utils; @@ -1360,8 +1361,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_active_relay_reconnect() -> TestResult { - let _guard = iroh_test::logging::setup(); let (_relay_map, relay_url, _server) = test_utils::run_relay_server().await?; let (peer_node, _echo_node_task) = start_echo_node(relay_url.clone()); @@ -1447,8 +1448,8 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_active_relay_inactive() -> TestResult { - let _guard = iroh_test::logging::setup(); let (_relay_map, relay_url, _server) = test_utils::run_relay_server().await?; let secret_key = SecretKey::from_bytes(&[1u8; 32]); diff --git a/iroh/src/magicsock/udp_conn.rs b/iroh/src/magicsock/udp_conn.rs index 3cbd37cd603..50d6e1a2739 100644 --- a/iroh/src/magicsock/udp_conn.rs +++ b/iroh/src/magicsock/udp_conn.rs @@ -136,6 +136,7 @@ mod tests { use netwatch::IpFamily; use tokio::sync::mpsc; use tracing::{info_span, Instrument}; + use tracing_test::traced_test; use super::*; use crate::tls; @@ -160,14 +161,14 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_rebinding_conn_send_recv_ipv4() -> Result<()> { - let _guard = iroh_test::logging::setup(); rebinding_conn_send_recv(IpFamily::V4).await } #[tokio::test] + #[traced_test] async fn test_rebinding_conn_send_recv_ipv6() -> Result<()> { - let _guard = iroh_test::logging::setup(); if !net_report::os_has_ipv6() { return Ok(()); } From 784222ee1e4755440e7dbfbef62f78fb840399eb Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 3 Feb 2025 15:27:11 +0100 Subject: [PATCH 2/6] refactor: remove CallOnDrop from iroh-test we can just use AbortOnDropHandle where needed --- iroh-test/src/lib.rs | 18 ------------------ iroh/src/endpoint.rs | 15 --------------- iroh/src/magicsock.rs | 26 ++++++++------------------ 3 files changed, 8 insertions(+), 51 deletions(-) diff --git a/iroh-test/src/lib.rs b/iroh-test/src/lib.rs index 55085248d16..9dc9a9dd151 100644 --- a/iroh-test/src/lib.rs +++ b/iroh-test/src/lib.rs @@ -1,21 +1,3 @@ //! Internal utilities to support testing. pub mod hexdump; - -// #[derive(derive_more::Debug)] -#[allow(missing_debug_implementations)] -pub struct CallOnDrop(Option>); - -impl CallOnDrop { - pub fn new(f: impl FnOnce() + 'static) -> Self { - Self(Some(Box::new(f))) - } -} - -impl Drop for CallOnDrop { - fn drop(&mut self) { - if let Some(f) = self.0.take() { - f(); - } - } -} diff --git a/iroh/src/endpoint.rs b/iroh/src/endpoint.rs index db841f99e56..9102ebb2a29 100644 --- a/iroh/src/endpoint.rs +++ b/iroh/src/endpoint.rs @@ -1724,7 +1724,6 @@ mod tests { use std::time::Instant; - use iroh_test::CallOnDrop; use n0_future::StreamExt; use rand::SeedableRng; use tracing::{error_span, info, info_span, Instrument}; @@ -1952,10 +1951,6 @@ mod tests { .instrument(error_span!("server")), ) }; - let abort_handle = server.abort_handle(); - let _server_guard = CallOnDrop::new(move || { - abort_handle.abort(); - }); for i in 0..n_clients { let round_start = Instant::now(); @@ -2166,17 +2161,7 @@ mod tests { }; let res_ep1 = tokio::spawn(tokio::time::timeout(TIMEOUT, ep1_side)); - - let ep1_abort_handle = res_ep1.abort_handle(); - let _ep1_guard = CallOnDrop::new(move || { - ep1_abort_handle.abort(); - }); - let res_ep2 = tokio::spawn(tokio::time::timeout(TIMEOUT, ep2_side)); - let ep2_abort_handle = res_ep2.abort_handle(); - let _ep2_guard = CallOnDrop::new(move || { - ep2_abort_handle.abort(); - }); let (r1, r2) = tokio::try_join!(res_ep1, res_ep2).unwrap(); r1.expect("ep1 timeout").unwrap(); diff --git a/iroh/src/magicsock.rs b/iroh/src/magicsock.rs index 98a33b9020a..bdb089b303c 100644 --- a/iroh/src/magicsock.rs +++ b/iroh/src/magicsock.rs @@ -3138,7 +3138,6 @@ impl NetInfo { #[cfg(test)] mod tests { use anyhow::Context; - use iroh_test::CallOnDrop; use rand::RngCore; use tokio_util::task::AbortOnDropHandle; use tracing_test::traced_test; @@ -3215,7 +3214,7 @@ mod tests { /// /// When the returned drop guard is dropped, the tasks doing this updating are stopped. #[instrument(skip_all)] - async fn mesh_stacks(stacks: Vec) -> Result { + async fn mesh_stacks(stacks: Vec) -> Result>> { /// Registers endpoint addresses of a node to all other nodes. fn update_direct_addrs( stacks: &[MagicStack], @@ -3239,22 +3238,19 @@ mod tests { // For each node, start a task which monitors its local endpoints and registers them // with the other nodes as local endpoints become known. - let mut tasks = JoinSet::new(); + let mut tasks = Vec::new(); for (my_idx, m) in stacks.iter().enumerate() { let m = m.clone(); let stacks = stacks.clone(); - tasks.spawn(async move { + tasks.push(AbortOnDropHandle::new(tokio::task::spawn(async move { let me = m.endpoint.node_id().fmt_short(); let mut stream = m.endpoint.direct_addresses().stream().filter_map(|i| i); while let Some(new_eps) = stream.next().await { info!(%me, "conn{} endpoints update: {:?}", my_idx + 1, new_eps); update_direct_addrs(&stacks, my_idx, new_eps); } - }); + }))); } - let guard = CallOnDrop::new(move || { - tasks.abort_all(); - }); // Wait for all nodes to be registered with each other. time::timeout(Duration::from_secs(10), async move { @@ -3279,7 +3275,7 @@ mod tests { .await .context("failed to connect nodes")?; info!("all nodes meshed"); - Ok(guard) + Ok(tasks) } #[instrument(skip_all, fields(me = %ep.endpoint.node_id().fmt_short()))] @@ -3534,9 +3530,7 @@ mod tests { time::sleep(offset()).await; } }); - CallOnDrop::new(move || { - task.abort(); - }) + AbortOnDropHandle::new(task) }; for i in 0..rounds { @@ -3563,9 +3557,7 @@ mod tests { time::sleep(offset()).await; } }); - CallOnDrop::new(move || { - task.abort(); - }) + AbortOnDropHandle::new(task) }; for i in 0..rounds { @@ -3593,9 +3585,7 @@ mod tests { m2.endpoint.magic_sock().force_network_change(true).await; time::sleep(offset()).await; }); - CallOnDrop::new(move || { - task.abort(); - }) + AbortOnDropHandle::new(task) }; for i in 0..rounds { From 8dac679c0c9f0bba3420d3c7f2f6ce327ba4a5a6 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 3 Feb 2025 15:55:44 +0100 Subject: [PATCH 3/6] refactor: replace remaining iroh-test --- Cargo.lock | 24 ----- Cargo.toml | 1 - README.md | 1 - iroh-base/Cargo.toml | 1 - iroh-base/src/key.rs | 10 +- iroh-base/src/ticket/node.rs | 32 +++--- iroh-dns-server/Cargo.toml | 1 - iroh-net-report/Cargo.toml | 1 - iroh-relay/Cargo.toml | 1 - iroh-relay/src/protos/relay.rs | 16 ++- iroh-test/Cargo.toml | 22 ---- iroh-test/README.md | 21 ---- iroh-test/src/hexdump.rs | 180 --------------------------------- iroh-test/src/lib.rs | 3 - iroh/Cargo.toml | 1 - 15 files changed, 37 insertions(+), 278 deletions(-) delete mode 100644 iroh-test/Cargo.toml delete mode 100644 iroh-test/README.md delete mode 100644 iroh-test/src/hexdump.rs delete mode 100644 iroh-test/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index dbac8dbf255..8e61f825ff6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2055,7 +2055,6 @@ dependencies = [ "iroh-quinn-proto", "iroh-quinn-udp", "iroh-relay", - "iroh-test 0.31.0", "n0-future", "netdev", "netwatch", @@ -2101,7 +2100,6 @@ dependencies = [ "derive_more", "ed25519-dalek", "getrandom", - "iroh-test 0.28.0", "postcard", "proptest", "rand", @@ -2153,7 +2151,6 @@ dependencies = [ "humantime-serde", "iroh", "iroh-metrics", - "iroh-test 0.31.0", "lru", "n0-future", "pkarr", @@ -2215,7 +2212,6 @@ dependencies = [ "iroh-metrics", "iroh-quinn", "iroh-relay", - "iroh-test 0.31.0", "n0-future", "netwatch", "portmapper", @@ -2311,7 +2307,6 @@ dependencies = [ "iroh-metrics", "iroh-quinn", "iroh-quinn-proto", - "iroh-test 0.31.0", "lru", "n0-future", "num_enum", @@ -2348,25 +2343,6 @@ dependencies = [ "webpki-roots", ] -[[package]] -name = "iroh-test" -version = "0.28.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f909a839e5aafd4c4ca473e6e143bacdd6a8483385e64186cacfa62e91f4081d" -dependencies = [ - "anyhow", - "tokio", - "tracing", - "tracing-subscriber", -] - -[[package]] -name = "iroh-test" -version = "0.31.0" -dependencies = [ - "anyhow", -] - [[package]] name = "is-terminal" version = "0.4.13" diff --git a/Cargo.toml b/Cargo.toml index 9ad1da77802..3621046a2a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,6 @@ members = [ "iroh-base", "iroh-dns-server", "iroh", - "iroh-test", "iroh/bench", "iroh-relay", "iroh-net-report", diff --git a/README.md b/README.md index 9f966e031bd..1798c8fa553 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,6 @@ This repository contains a workspace of crates: - `iroh`: The core library for hole-punching & communicating with relays. - `iroh-relay`: The relay server implementation. This is the code we run in production (and you can, too!). - `iroh-base`: Common types like `Hash`, key types or `RelayUrl`. -- `iroh-test`: Test utilities. - `iroh-dns-server`: DNS server implementation powering the `n0_discovery` for NodeIds, running at dns.iroh.link. - `iroh-net-report`: Analyzes your host's networking ability & NAT. diff --git a/iroh-base/Cargo.toml b/iroh-base/Cargo.toml index 000944f274e..9bf16bcfcfc 100644 --- a/iroh-base/Cargo.toml +++ b/iroh-base/Cargo.toml @@ -29,7 +29,6 @@ thiserror = { version = "2", optional = true } getrandom = { version = "0.2", default-features = false, optional = true } [dev-dependencies] -iroh-test = "0.28.0" postcard = { version = "1", features = ["use-std"] } proptest = "1.0.0" rand = "0.8" diff --git a/iroh-base/src/key.rs b/iroh-base/src/key.rs index b1ce6f40aed..5f4a6511b1d 100644 --- a/iroh-base/src/key.rs +++ b/iroh-base/src/key.rs @@ -350,7 +350,7 @@ fn decode_base32_hex(s: &str) -> Result<[u8; 32], KeyParsingError> { #[cfg(test)] mod tests { - use iroh_test::{assert_eq_hex, hexdump::parse_hexdump}; + use data_encoding::HEXLOWER; use super::*; @@ -360,10 +360,10 @@ mod tests { PublicKey::from_str("ae58ff8833241ac82d6ff7611046ed67b5072d142c588d0063e942d9a75502b6") .unwrap(); let bytes = postcard::to_stdvec(&public_key).unwrap(); - let expected = - parse_hexdump("ae58ff8833241ac82d6ff7611046ed67b5072d142c588d0063e942d9a75502b6") - .unwrap(); - assert_eq_hex!(bytes, expected); + let expected = HEXLOWER + .decode(b"ae58ff8833241ac82d6ff7611046ed67b5072d142c588d0063e942d9a75502b6") + .unwrap(); + assert_eq!(bytes, expected); } #[test] diff --git a/iroh-base/src/ticket/node.rs b/iroh-base/src/ticket/node.rs index 0b424ab7b33..25836b7afc0 100644 --- a/iroh-base/src/ticket/node.rs +++ b/iroh-base/src/ticket/node.rs @@ -135,10 +135,9 @@ impl<'de> Deserialize<'de> for NodeTicket { mod tests { use std::net::{Ipv4Addr, SocketAddr}; - use iroh_test::{assert_eq_hex, hexdump::parse_hexdump}; - use super::*; use crate::key::{PublicKey, SecretKey}; + use data_encoding::HEXLOWER; fn make_ticket() -> NodeTicket { let peer = SecretKey::generate(&mut rand::thread_rng()).public(); @@ -188,15 +187,24 @@ mod tests { .as_bytes(), ) .unwrap(); - let expected = parse_hexdump(" - 00 # variant - ae58ff8833241ac82d6ff7611046ed67b5072d142c588d0063e942d9a75502b6 # node id, 32 bytes, see above - 01 # relay url present - 10 687474703a2f2f646572702e6d652e2f # relay url, 16 bytes, see above - 01 # one direct address - 00 # ipv4 - 7f000001 8008 # address, see above - ").unwrap(); - assert_eq_hex!(base32, expected); + let expected = [ + // variant + "00", + // node id, 32 bytes, see above + "ae58ff8833241ac82d6ff7611046ed67b5072d142c588d0063e942d9a75502b6", + // relay url present + "01", + // relay url, 16 bytes, see above + "10", + "687474703a2f2f646572702e6d652e2f", + // one direct address + "01", + // ipv4 + "00", + // address, see above + "7f0000018008", + ]; + let expected = HEXLOWER.decode(expected.concat().as_bytes()).unwrap(); + assert_eq!(base32, expected); } } diff --git a/iroh-dns-server/Cargo.toml b/iroh-dns-server/Cargo.toml index 77c06e853ba..45868612f8b 100644 --- a/iroh-dns-server/Cargo.toml +++ b/iroh-dns-server/Cargo.toml @@ -61,7 +61,6 @@ z32 = "1.1.1" criterion = "0.5.1" hickory-resolver = "=0.25.0-alpha.4" iroh = { path = "../iroh" } -iroh-test = { path = "../iroh-test" } pkarr = { version = "2.3.1", features = ["rand"] } rand = "0.8" rand_chacha = "0.3.1" diff --git a/iroh-net-report/Cargo.toml b/iroh-net-report/Cargo.toml index 6c91e175bc3..97ca6d074b8 100644 --- a/iroh-net-report/Cargo.toml +++ b/iroh-net-report/Cargo.toml @@ -39,7 +39,6 @@ url = { version = "2.4" } [dev-dependencies] iroh-relay = { path = "../iroh-relay", features = ["test-utils", "server"] } -iroh-test = { path = "../iroh-test" } pretty_assertions = "1.4" testresult = "0.4.0" tokio = { version = "1", default-features = false, features = ["test-util"] } diff --git a/iroh-relay/Cargo.toml b/iroh-relay/Cargo.toml index 5e6f9d4038c..48ebed934ae 100644 --- a/iroh-relay/Cargo.toml +++ b/iroh-relay/Cargo.toml @@ -116,7 +116,6 @@ tokio = { version = "1", features = [ "test-util", ] } tracing-subscriber = { version = "0.3", features = ["env-filter"] } -iroh-test = { path = "../iroh-test" } serde_json = "1" tracing-test = "0.2.5" diff --git a/iroh-relay/src/protos/relay.rs b/iroh-relay/src/protos/relay.rs index 1179fa492a2..5c5f449f428 100644 --- a/iroh-relay/src/protos/relay.rs +++ b/iroh-relay/src/protos/relay.rs @@ -604,6 +604,7 @@ pub(crate) async fn recv_frame> + Unpin>( #[cfg(test)] mod tests { + use data_encoding::HEXLOWER; use tokio_util::codec::{FramedRead, FramedWrite}; use super::*; @@ -726,10 +727,17 @@ mod tests { for (frame, expected_hex) in frames { let bytes = frame.encode_for_ws_msg(); - // To regenerate the hexdumps: - // let hexdump = iroh_test::hexdump::print_hexdump(bytes, []); - // println!("{hexdump}"); - let expected_bytes = iroh_test::hexdump::parse_hexdump(expected_hex)?; + let stripped: Vec = expected_hex + .chars() + .filter_map(|s| { + if s.is_ascii_whitespace() { + None + } else { + Some(s as u8) + } + }) + .collect(); + let expected_bytes = HEXLOWER.decode(&stripped).unwrap(); assert_eq!(bytes, expected_bytes); } diff --git a/iroh-test/Cargo.toml b/iroh-test/Cargo.toml deleted file mode 100644 index a20f81bc655..00000000000 --- a/iroh-test/Cargo.toml +++ /dev/null @@ -1,22 +0,0 @@ -[package] -name = "iroh-test" -version = "0.31.0" -edition = "2021" -readme = "README.md" -description = "Internal utilities to support testing of iroh." -license = "MIT OR Apache-2.0" -authors = ["n0 team"] -repository = "https://github.com/n0-computer/iroh" -publish = true - -# Sadly this also needs to be updated in .github/workflows/ci.yml -rust-version = "1.81" - -[lints] -workspace = true - -[dependencies] -anyhow = "1" - -[package.metadata.docs.rs] -all-features = true diff --git a/iroh-test/README.md b/iroh-test/README.md deleted file mode 100644 index d4dea492a84..00000000000 --- a/iroh-test/README.md +++ /dev/null @@ -1,21 +0,0 @@ -# iroh-test - -This crate contains testing support code for iroh. It is only used -for running the iroh test suite. - -# License - -This project is licensed under either of - - * Apache License, Version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or - http://www.apache.org/licenses/LICENSE-2.0) - * MIT license ([LICENSE-MIT](LICENSE-MIT) or - http://opensource.org/licenses/MIT) - -at your option. - -### Contribution - -Unless you explicitly state otherwise, any contribution intentionally submitted -for inclusion in this project by you, as defined in the Apache-2.0 license, -shall be dual licensed as above, without any additional terms or conditions. diff --git a/iroh-test/src/hexdump.rs b/iroh-test/src/hexdump.rs deleted file mode 100644 index 4c5e06727dd..00000000000 --- a/iroh-test/src/hexdump.rs +++ /dev/null @@ -1,180 +0,0 @@ -use anyhow::{ensure, Context, Result}; - -/// Parses a commented multi line hexdump into a vector of bytes. -/// -/// This is useful to write wire level protocol tests. -pub fn parse_hexdump(s: &str) -> Result> { - let mut result = Vec::new(); - - for (line_number, line) in s.lines().enumerate() { - let data_part = line.split('#').next().unwrap_or(""); - let cleaned: String = data_part.chars().filter(|c| !c.is_whitespace()).collect(); - - ensure!( - cleaned.len() % 2 == 0, - "Non-even number of hex chars detected on line {}.", - line_number + 1 - ); - - for i in (0..cleaned.len()).step_by(2) { - let byte_str = &cleaned[i..i + 2]; - let byte = u8::from_str_radix(byte_str, 16) - .with_context(|| format!("Invalid hex data on line {}.", line_number + 1))?; - - result.push(byte); - } - } - - Ok(result) -} - -/// Returns a hexdump of the given bytes in multiple lines as a String. -pub fn print_hexdump(bytes: impl AsRef<[u8]>, line_lengths: impl AsRef<[usize]>) -> String { - let line_lengths = line_lengths.as_ref(); - let mut bytes_iter = bytes.as_ref().iter(); - let default_line_length = line_lengths - .last() - .filter(|x| **x != 0) - .copied() - .unwrap_or(16); - let mut line_lengths_iter = line_lengths.iter(); - let mut output = String::new(); - - loop { - let line_length = line_lengths_iter - .next() - .copied() - .unwrap_or(default_line_length); - if line_length == 0 { - output.push('\n'); - } else { - let line: Vec<_> = bytes_iter.by_ref().take(line_length).collect(); - - if line.is_empty() { - break; - } - - for byte in &line { - output.push_str(&format!("{:02x} ", byte)); - } - output.pop(); // Remove the trailing space - output.push('\n'); - } - } - - output -} - -/// This is a macro to assert that two byte slices are equal. -/// -/// It is like assert_eq!, but it will print a nicely formatted hexdump of the -/// two slices if they are not equal. This makes it much easier to track down -/// a difference in a large byte slice. -#[macro_export] -macro_rules! assert_eq_hex { - ($a:expr, $b:expr) => { - assert_eq_hex!($a, $b, []) - }; - ($a:expr, $b:expr, $hint:expr) => { - let a = $a; - let b = $b; - let hint = $hint; - let ar: &[u8] = a.as_ref(); - let br: &[u8] = b.as_ref(); - let hintr: &[usize] = hint.as_ref(); - if ar != br { - panic!( - "assertion failed: `(left == right)`\nleft:\n{}\nright:\n{}\n", - ::iroh_test::hexdump::print_hexdump(ar, hintr), - ::iroh_test::hexdump::print_hexdump(br, hintr), - ) - } - }; -} - -#[cfg(test)] -mod tests { - use super::{parse_hexdump, print_hexdump}; - - #[test] - fn test_basic() { - let input = r" - a1b2 # comment - 3c4d - "; - let result = parse_hexdump(input).unwrap(); - assert_eq!(result, vec![0xa1, 0xb2, 0x3c, 0x4d]); - } - - #[test] - fn test_upper_case() { - let input = r" - A1B2 # comment - 3C4D - "; - let result = parse_hexdump(input).unwrap(); - assert_eq!(result, vec![0xa1, 0xb2, 0x3c, 0x4d]); - } - - #[test] - fn test_mixed_case() { - let input = r" - a1B2 # comment - 3C4d - "; - let result = parse_hexdump(input).unwrap(); - assert_eq!(result, vec![0xa1, 0xb2, 0x3c, 0x4d]); - } - - #[test] - fn test_odd_characters() { - let input = r" - a1b - "; - let result = parse_hexdump(input); - assert!(result.is_err()); - } - - #[test] - fn test_invalid_characters() { - let input = r" - a1g2 # 'g' is not valid in hex - "; - let result = parse_hexdump(input); - assert!(result.is_err()); - } - #[test] - fn test_basic_hexdump() { - let data: &[u8] = &[0x1, 0x2, 0x3, 0x4, 0x5]; - let output = print_hexdump(data, [1, 2]); - assert_eq!(output, "01\n02 03\n04 05\n"); - } - - #[test] - fn test_newline_insertion() { - let data: &[u8] = &[0x1, 0x2, 0x3, 0x4]; - let output = print_hexdump(data, [1, 0, 2]); - assert_eq!(output, "01\n\n02 03\n04\n"); - } - - #[test] - fn test_indefinite_line_length() { - let data: &[u8] = &[0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8]; - let output = print_hexdump(data, [2, 4]); - assert_eq!(output, "01 02\n03 04 05 06\n07 08\n"); - } - - #[test] - fn test_empty_data() { - let data: &[u8] = &[]; - let output = print_hexdump(data, [1, 2]); - assert_eq!(output, ""); - } - - #[test] - fn test_zeros_then_default() { - let data: &[u8] = &[0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8]; - let output = print_hexdump(data, [1, 0, 0, 2]); - assert_eq!(output, "01\n\n\n02 03\n04 05\n06 07\n08\n"); - } -} diff --git a/iroh-test/src/lib.rs b/iroh-test/src/lib.rs deleted file mode 100644 index 9dc9a9dd151..00000000000 --- a/iroh-test/src/lib.rs +++ /dev/null @@ -1,3 +0,0 @@ -//! Internal utilities to support testing. - -pub mod hexdump; diff --git a/iroh/Cargo.toml b/iroh/Cargo.toml index 5d4c1b7c80c..6cd68a627ee 100644 --- a/iroh/Cargo.toml +++ b/iroh/Cargo.toml @@ -125,7 +125,6 @@ tokio = { version = "1", features = [ "test-util", ] } tracing-subscriber = { version = "0.3", features = ["env-filter"] } -iroh-test = { path = "../iroh-test" } serde_json = "1" testresult = "0.4.0" iroh-relay = { path = "../iroh-relay", default-features = false, features = ["test-utils", "server"] } From 07deb83014fbf49dd2e1ff5637023c4cecc92623 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 3 Feb 2025 15:57:50 +0100 Subject: [PATCH 4/6] ci: remove iroh-test --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 5db3e4943e4..17e174ee191 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -23,7 +23,7 @@ env: RUSTFLAGS: -Dwarnings RUSTDOCFLAGS: -Dwarnings SCCACHE_CACHE_SIZE: "10G" - CRATES_LIST: "iroh,iroh-bench,iroh-test,iroh-dns-server,iroh-relay,iroh-net-report" + CRATES_LIST: "iroh,iroh-bench,iroh-dns-server,iroh-relay,iroh-net-report" IROH_FORCE_STAGING_RELAYS: "1" jobs: From bb949d53ee37e321b2c137205c52c70df55b692a Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 3 Feb 2025 16:00:16 +0100 Subject: [PATCH 5/6] cargo fmt --- iroh-base/src/ticket/node.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/iroh-base/src/ticket/node.rs b/iroh-base/src/ticket/node.rs index 25836b7afc0..e964def5dd8 100644 --- a/iroh-base/src/ticket/node.rs +++ b/iroh-base/src/ticket/node.rs @@ -135,9 +135,10 @@ impl<'de> Deserialize<'de> for NodeTicket { mod tests { use std::net::{Ipv4Addr, SocketAddr}; + use data_encoding::HEXLOWER; + use super::*; use crate::key::{PublicKey, SecretKey}; - use data_encoding::HEXLOWER; fn make_ticket() -> NodeTicket { let peer = SecretKey::generate(&mut rand::thread_rng()).public(); From f4f9ec9870ec7b689435bde1db89b39284e8bab7 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 3 Feb 2025 16:30:09 +0100 Subject: [PATCH 6/6] apply CR --- iroh/src/magicsock.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/iroh/src/magicsock.rs b/iroh/src/magicsock.rs index bdb089b303c..82ed12ddd23 100644 --- a/iroh/src/magicsock.rs +++ b/iroh/src/magicsock.rs @@ -3214,7 +3214,7 @@ mod tests { /// /// When the returned drop guard is dropped, the tasks doing this updating are stopped. #[instrument(skip_all)] - async fn mesh_stacks(stacks: Vec) -> Result>> { + async fn mesh_stacks(stacks: Vec) -> Result> { /// Registers endpoint addresses of a node to all other nodes. fn update_direct_addrs( stacks: &[MagicStack], @@ -3238,18 +3238,18 @@ mod tests { // For each node, start a task which monitors its local endpoints and registers them // with the other nodes as local endpoints become known. - let mut tasks = Vec::new(); + let mut tasks = JoinSet::new(); for (my_idx, m) in stacks.iter().enumerate() { let m = m.clone(); let stacks = stacks.clone(); - tasks.push(AbortOnDropHandle::new(tokio::task::spawn(async move { + tasks.spawn(async move { let me = m.endpoint.node_id().fmt_short(); let mut stream = m.endpoint.direct_addresses().stream().filter_map(|i| i); while let Some(new_eps) = stream.next().await { info!(%me, "conn{} endpoints update: {:?}", my_idx + 1, new_eps); update_direct_addrs(&stacks, my_idx, new_eps); } - }))); + }); } // Wait for all nodes to be registered with each other.