From 374b50a23bb3debd6b28cc24f549144e254a1f5a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 9 Sep 2022 16:56:53 +1000 Subject: [PATCH 1/5] Rewrite test using `async_std::test` --- swarm/src/lib.rs | 49 ++++++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 230de4631db..4e450b0961f 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -2425,8 +2425,8 @@ mod tests { assert!(!swarm.is_connected(&peer_id)); } - #[test] - fn multiple_addresses_err() { + #[async_std::test] + async fn multiple_addresses_err() { // Tries dialing multiple addresses, and makes sure there's one dialing error per address. let target = PeerId::random(); @@ -2450,35 +2450,26 @@ mod tests { ) .unwrap(); - futures::executor::block_on(future::poll_fn(|cx| -> Poll> { - loop { - match swarm.poll_next_unpin(cx) { - Poll::Ready(Some(SwarmEvent::OutgoingConnectionError { - peer_id, - // multiaddr, - error: DialError::Transport(errors), - })) => { - assert_eq!(peer_id.unwrap(), target); - - let failed_addresses = - errors.into_iter().map(|(addr, _)| addr).collect::>(); - assert_eq!( - failed_addresses, - addresses - .clone() - .into_iter() - .map(|addr| addr.with(Protocol::P2p(target.into()))) - .collect::>() - ); + match swarm.next().await.unwrap() { + SwarmEvent::OutgoingConnectionError { + peer_id, + // multiaddr, + error: DialError::Transport(errors), + } => { + assert_eq!(peer_id.unwrap(), target); - return Poll::Ready(Ok(())); - } - Poll::Ready(_) => unreachable!(), - Poll::Pending => break Poll::Pending, - } + let failed_addresses = errors.into_iter().map(|(addr, _)| addr).collect::>(); + assert_eq!( + failed_addresses, + addresses + .clone() + .into_iter() + .map(|addr| addr.with(Protocol::P2p(target.into()))) + .collect::>() + ); } - })) - .unwrap(); + e => panic!("Unexpected event: {e:?}"), + } } #[test] From ccd33d740a6dc72bef26830a179f65a099a6877e Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 9 Sep 2022 16:59:00 +1000 Subject: [PATCH 2/5] Use `HashSet` to avoid rare test failure In case we accidentally generate the same port twice, we will try to issue two dial attempts to the same address but also expect two dial errors which is exactly what this test is trying to catch. Unfortunately, the assertion is badly written and does not catch duplicate inputs. --- swarm/src/lib.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 4e450b0961f..783ed73ed70 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1624,7 +1624,6 @@ mod tests { use libp2p_core::transport::TransportEvent; use libp2p_core::Endpoint; use quickcheck::{quickcheck, Arbitrary, Gen, QuickCheck}; - use rand::prelude::SliceRandom; use rand::Rng; // Test execution state. @@ -2433,19 +2432,18 @@ mod tests { let mut swarm = new_test_swarm::<_, ()>(DummyConnectionHandler::default()).build(); - let mut addresses = Vec::new(); + let mut addresses = HashSet::new(); for _ in 0..3 { - addresses.push(multiaddr![Ip4([0, 0, 0, 0]), Tcp(rand::random::())]); + addresses.insert(multiaddr![Ip4([0, 0, 0, 0]), Tcp(rand::random::())]); } for _ in 0..5 { - addresses.push(multiaddr![Udp(rand::random::())]); + addresses.insert(multiaddr![Udp(rand::random::())]); } - addresses.shuffle(&mut rand::thread_rng()); swarm .dial( DialOpts::peer_id(target) - .addresses(addresses.clone()) + .addresses(addresses.iter().cloned().collect()) .build(), ) .unwrap(); From aedd5c79ac39b18f80ad3fe7c406d7115e728f0d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 9 Sep 2022 17:00:04 +1000 Subject: [PATCH 3/5] Swap expected and failed values in assertion Not sure if it is just me, but I'd expect the expectation to come first. --- swarm/src/lib.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 783ed73ed70..d494089d69c 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -2454,17 +2454,16 @@ mod tests { // multiaddr, error: DialError::Transport(errors), } => { - assert_eq!(peer_id.unwrap(), target); + assert_eq!(target, peer_id.unwrap()); let failed_addresses = errors.into_iter().map(|(addr, _)| addr).collect::>(); - assert_eq!( - failed_addresses, - addresses - .clone() - .into_iter() - .map(|addr| addr.with(Protocol::P2p(target.into()))) - .collect::>() - ); + let expected_addresses = addresses + .clone() + .into_iter() + .map(|addr| addr.with(Protocol::P2p(target.into()))) + .collect::>(); + + assert_eq!(expected_addresses, failed_addresses); } e => panic!("Unexpected event: {e:?}"), } From 0d42c0a87a4cba72a85770a0dc8ed8b037521dd9 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 9 Sep 2022 17:03:45 +1000 Subject: [PATCH 4/5] Unroll loops --- swarm/src/lib.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index d494089d69c..9ddc7b19e22 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -2432,13 +2432,16 @@ mod tests { let mut swarm = new_test_swarm::<_, ()>(DummyConnectionHandler::default()).build(); - let mut addresses = HashSet::new(); - for _ in 0..3 { - addresses.insert(multiaddr![Ip4([0, 0, 0, 0]), Tcp(rand::random::())]); - } - for _ in 0..5 { - addresses.insert(multiaddr![Udp(rand::random::())]); - } + let addresses = HashSet::from([ + multiaddr![Ip4([0, 0, 0, 0]), Tcp(rand::random::())], + multiaddr![Ip4([0, 0, 0, 0]), Tcp(rand::random::())], + multiaddr![Ip4([0, 0, 0, 0]), Tcp(rand::random::())], + multiaddr![Udp(rand::random::())], + multiaddr![Udp(rand::random::())], + multiaddr![Udp(rand::random::())], + multiaddr![Udp(rand::random::())], + multiaddr![Udp(rand::random::())], + ]); swarm .dial( From 4f9d8391d2109caa6146f9e82e307b82528dea6d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 9 Sep 2022 17:05:51 +1000 Subject: [PATCH 5/5] Remove unnecessary clone --- swarm/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 9ddc7b19e22..7faaa43b44d 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -2461,7 +2461,6 @@ mod tests { let failed_addresses = errors.into_iter().map(|(addr, _)| addr).collect::>(); let expected_addresses = addresses - .clone() .into_iter() .map(|addr| addr.with(Protocol::P2p(target.into()))) .collect::>();