From 2eb06d0fb36705b5c79fc7ed8de39855599f14a1 Mon Sep 17 00:00:00 2001
From: Floris Bruynooghe <flub@n0.computer>
Date: Fri, 8 Mar 2024 18:59:27 +0100
Subject: [PATCH] tests(iroh-net): Re-enable icmp probe test (#2065)

## Description

This changes the test slightly to send multiple pings, this should
make it more reliable.

## Notes & open questions

I didn't create a flaky issue for this as I thought I could address it
right away... which I just about managed.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
---
 iroh-net/src/netcheck/reportgen.rs | 47 +++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/iroh-net/src/netcheck/reportgen.rs b/iroh-net/src/netcheck/reportgen.rs
index ff06fa32ab..8e413e6ca8 100644
--- a/iroh-net/src/netcheck/reportgen.rs
+++ b/iroh-net/src/netcheck/reportgen.rs
@@ -1309,7 +1309,6 @@ mod tests {
     //
     // TODO: Not sure what about IPv6 pings using sysctl.
     #[tokio::test]
-    #[ignore = "flaky"]
     async fn test_icmp_probe_eu_derper() {
         let _logging_guard = iroh_test::logging::setup();
         let pinger = Pinger::new();
@@ -1319,17 +1318,43 @@ mod tests {
             delay: Duration::from_secs(0),
             node: Arc::new(derper),
         };
-        match run_icmp_probe(probe, addr, pinger).await {
-            Ok(report) => {
-                dbg!(&report);
-                assert_eq!(report.icmpv4, Some(true));
-                assert!(report.latency.expect("should have a latency") > Duration::from_secs(0));
-            }
-            Err(ProbeError::Error(err, _probe)) => panic!("Ping error: {err:#}"),
-            Err(ProbeError::AbortSet(_err, _probe)) => {
-                // We don't have permission, too bad.
-                // panic!("no ping permission: {err:#}");
+
+        // A singe ICMP packet might get lost.  Try several and take the first.
+        let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
+        let mut tasks = JoinSet::new();
+        for i in 0..8 {
+            let probe = probe.clone();
+            let pinger = pinger.clone();
+            let tx = tx.clone();
+            tasks.spawn(async move {
+                time::sleep(Duration::from_millis(i * 100)).await;
+                let res = run_icmp_probe(probe, addr, pinger).await;
+                tx.send(res).ok();
+            });
+        }
+        let mut last_err = None;
+        while let Some(res) = rx.recv().await {
+            match res {
+                Ok(report) => {
+                    dbg!(&report);
+                    assert_eq!(report.icmpv4, Some(true));
+                    assert!(
+                        report.latency.expect("should have a latency") > Duration::from_secs(0)
+                    );
+                    break;
+                }
+                Err(ProbeError::Error(err, _probe)) => {
+                    last_err = Some(err);
+                }
+                Err(ProbeError::AbortSet(_err, _probe)) => {
+                    // We don't have permission, too bad.
+                    // panic!("no ping permission: {err:#}");
+                    break;
+                }
             }
         }
+        if let Some(err) = last_err {
+            panic!("Ping error: {err:#}");
+        }
     }
 }