Skip to content

Commit

Permalink
tests(iroh-net): Re-enable icmp probe test (#2065)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
flub authored Mar 8, 2024
1 parent 9e7605d commit 2eb06d0
Showing 1 changed file with 36 additions and 11 deletions.
47 changes: 36 additions & 11 deletions iroh-net/src/netcheck/reportgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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:#}");
}
}
}

0 comments on commit 2eb06d0

Please sign in to comment.