Skip to content

Commit 1ae820d

Browse files
authored
tests(iroh): Add some context to test errors (#3066)
## Description The flaky tests are being super weird and making no sense. This is clutching at straws for any hint. ## Breaking Changes <!-- Optional, if there are any breaking changes document them, including how to migrate older code. --> ## Notes & open questions /cc @ramfox Two slightly odd things I spotted while looking at this: - The use of 240.0.0.1 as "bad" IP address returned in discovery. Maybe one from the test nets in RFC 5737 would be better? - The endpoints accept a connection and immediately drop it. This could potentially send the CONNECTION_CLOSE before the ACK from the connection resulting in the connect call returning an error (BUT THAT WOULD BE A NORMAL READABLE TEST FAILURE). This probably doesn't happen because it's all local and very fast and Quinn has already ACKed the connection before the code manages to drop it. Still, essentially racy code. Fine, I added a fix for the 2nd. But it's not the issue. ## Change checklist - [x] Self-review. - [x] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [x] Tests if relevant. - [x] All breaking changes documented.
1 parent 423a986 commit 1ae820d

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

iroh/src/discovery.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,7 @@ mod tests {
443443
time::SystemTime,
444444
};
445445

446+
use anyhow::Context;
446447
use iroh_base::SecretKey;
447448
use rand::Rng;
448449
use tokio_util::task::AbortOnDropHandle;
@@ -604,8 +605,11 @@ mod tests {
604605
};
605606
let ep1_addr = NodeAddr::new(ep1.node_id());
606607
// wait for out address to be updated and thus published at least once
607-
ep1.node_addr().await?;
608-
let _conn = ep2.connect(ep1_addr, TEST_ALPN).await?;
608+
ep1.node_addr().await.context("waiting for NodeAddr")?;
609+
let _conn = ep2
610+
.connect(ep1_addr, TEST_ALPN)
611+
.await
612+
.context("connecting")?;
609613
Ok(())
610614
}
611615

@@ -706,10 +710,13 @@ mod tests {
706710
let handle = tokio::spawn({
707711
let ep = ep.clone();
708712
async move {
713+
// Keep connections alive until the task is dropped.
714+
let mut connections = Vec::new();
709715
// we skip accept() errors, they can be caused by retransmits
710716
while let Some(connecting) = ep.accept().await.and_then(|inc| inc.accept().ok()) {
711-
let _conn = connecting.await?;
712717
// Just accept incoming connections, but don't do anything with them.
718+
let conn = connecting.await?;
719+
connections.push(conn);
713720
}
714721

715722
anyhow::Ok(())

0 commit comments

Comments
 (0)