From 59bea8acdaed868e36094ecaaff6e4e3761023e1 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Sat, 16 May 2026 23:14:31 +0700 Subject: [PATCH] test(client): remove implementation-coupled client tests --- crates/irtt-client/src/client.rs | 10 +- crates/irtt-client/src/client/tests/close.rs | 25 ---- .../src/client/tests/probes_replies.rs | 117 +----------------- .../irtt-client/src/client/tests/sequence.rs | 27 +--- 4 files changed, 15 insertions(+), 164 deletions(-) diff --git a/crates/irtt-client/src/client.rs b/crates/irtt-client/src/client.rs index 3354553..2de5613 100644 --- a/crates/irtt-client/src/client.rs +++ b/crates/irtt-client/src/client.rs @@ -457,7 +457,7 @@ impl Client { .is_some_and(|h| sequence_is_before(wire_seq, h)); let highest_seen = session.highest_received_seq.unwrap_or(wire_seq); - update_highest_received(session, wire_seq); + update_highest_received(&mut session.highest_received_seq, wire_seq); session.completed.insert(wire_seq); let mut events = Vec::new(); @@ -494,7 +494,7 @@ impl Client { } Ok(events) } else if session.completed.contains(wire_seq) { - update_highest_received(session, wire_seq); + update_highest_received(&mut session.highest_received_seq, wire_seq); Ok(vec![ClientEvent::DuplicateReply { seq: wire_seq, remote: self.remote, @@ -507,7 +507,7 @@ impl Client { let one_way = compute_one_way(&timed_out.sent_at, &now, &reply.timestamps); let received_stats = build_received_stats(&reply); let highest_seen = session.highest_received_seq.unwrap_or(wire_seq); - update_highest_received(session, wire_seq); + update_highest_received(&mut session.highest_received_seq, wire_seq); session.completed.insert(wire_seq); let mut events = vec![ClientEvent::LateReply { @@ -690,8 +690,8 @@ impl Client { } } -fn update_highest_received(session: &mut ActiveSession, wire_seq: u32) { - session.highest_received_seq = Some(session.highest_received_seq.map_or(wire_seq, |h| { +fn update_highest_received(highest_received_seq: &mut Option, wire_seq: u32) { + *highest_received_seq = Some(highest_received_seq.map_or(wire_seq, |h| { if sequence_is_after(wire_seq, h) { wire_seq } else { diff --git a/crates/irtt-client/src/client/tests/close.rs b/crates/irtt-client/src/client/tests/close.rs index 0727728..4b3d73e 100644 --- a/crates/irtt-client/src/client/tests/close.rs +++ b/crates/irtt-client/src/client/tests/close.rs @@ -68,31 +68,6 @@ fn send_probe_fails_after_close() { server.join(); } -#[test] -fn close_clears_timed_out_metadata() { - let params = default_params(); - let server = silent_open_server(params); - let config = ClientConfig { - probe_timeout: Duration::from_millis(40), - socket_config: crate::SocketConfig { - recv_timeout: Some(Duration::from_millis(50)), - ..Default::default() - }, - ..default_test_config(server.addr) - }; - let mut client = Client::connect(config).unwrap(); - assert_open_started(client.open().unwrap()); - - client.send_probe().unwrap(); - thread::sleep(Duration::from_millis(60)); - client.poll_timeouts().unwrap(); - assert_eq!(client.session.as_ref().unwrap().timed_out.len(), 1); - - client.close().unwrap(); - assert!(client.session.is_none()); - server.join(); -} - #[test] fn close_flagged_echo_reply_emits_reply_then_closes_without_sending_close() { let params = default_params(); diff --git a/crates/irtt-client/src/client/tests/probes_replies.rs b/crates/irtt-client/src/client/tests/probes_replies.rs index f0382bc..d230cb9 100644 --- a/crates/irtt-client/src/client/tests/probes_replies.rs +++ b/crates/irtt-client/src/client/tests/probes_replies.rs @@ -86,7 +86,7 @@ fn echo_sent_reports_schedule_and_timer_error() { wall: SystemTime::now(), }; assert_open_started(client.open().unwrap()); - let session_start = client.session.as_ref().unwrap().start_mono; + let session_start = client.next_send_deadline().unwrap(); assert!( session_start >= start.mono, "probe schedule must start after open begins" @@ -170,8 +170,7 @@ fn send_probe_respects_finite_duration_exclusive_end() { let mut client = Client::connect(config).unwrap(); assert_open_started(client.open().unwrap()); - let session = client.session.as_ref().unwrap(); - let start = session.start_mono; + let start = client.next_send_deadline().unwrap(); let interval = Duration::from_millis(500); let now0 = ClientTimestamp { @@ -192,7 +191,6 @@ fn send_probe_respects_finite_duration_exclusive_end() { }; let events = client.send_probe_at(now2).unwrap(); assert!(events.is_empty()); - assert!(client.session.as_ref().unwrap().sending_done); assert!(client.next_send_deadline().is_none()); client.close().unwrap(); @@ -223,7 +221,7 @@ fn continuous_duration_keeps_generating_send_deadlines() { let mut client = Client::connect(config).unwrap(); assert_open_started(client.open().unwrap()); - let start = client.session.as_ref().unwrap().start_mono; + let start = client.next_send_deadline().unwrap(); let interval = Duration::from_millis(500); for seq in 0..4 { let now = ClientTimestamp { @@ -233,7 +231,6 @@ fn continuous_duration_keeps_generating_send_deadlines() { let events = client.send_probe_at(now).unwrap(); assert_eq!(events.len(), 1); assert!(client.next_send_deadline().is_some()); - assert!(!client.session.as_ref().unwrap().sending_done); } client.close().unwrap(); @@ -245,55 +242,20 @@ fn next_send_deadline_advances_by_interval() { let params = default_params(); let (mut client, server) = open_client_with_echo_server(¶ms); - let session = client.session.as_ref().unwrap(); - let start = session.start_mono; let deadline0 = client.next_send_deadline().unwrap(); - assert_eq!(deadline0, start); client.send_probe().unwrap(); let deadline1 = client.next_send_deadline().unwrap(); - assert_eq!(deadline1, start + Duration::from_secs(1)); + assert_eq!(deadline1, deadline0 + Duration::from_secs(1)); client.send_probe().unwrap(); let deadline2 = client.next_send_deadline().unwrap(); - assert_eq!(deadline2, start + Duration::from_secs(2)); + assert_eq!(deadline2, deadline0 + Duration::from_secs(2)); client.close().unwrap(); server.join(); } -#[test] -fn send_probe_reports_schedule_overflow() { - let params = Params { - protocol_version: 1, - duration_ns: 0, - interval_ns: 1_000_000_000, - received_stats: ReceivedStats::Both, - stamp_at: StampAt::Both, - clock: Clock::Both, - ..Params::default() - }; - let server = silent_open_server(params); - let config = ClientConfig { - duration: None, - interval: Duration::from_secs(1), - socket_config: crate::SocketConfig { - recv_timeout: Some(Duration::from_millis(200)), - ..Default::default() - }, - ..default_test_config(server.addr) - }; - let mut client = Client::connect(config).unwrap(); - assert_open_started(client.open().unwrap()); - client.session.as_mut().unwrap().packets_sent = u64::MAX - 1; - - assert!(matches!( - client.send_probe(), - Err(ClientError::DurationOverflow) - )); - server.join(); -} - #[test] fn recv_once_returns_empty_on_timeout() { let params = default_params(); @@ -803,8 +765,6 @@ fn late_reply_after_timeout_preserves_measurement_metadata() { thread::sleep(Duration::from_millis(60)); let losses = client.poll_timeouts().unwrap(); assert!(matches!(&losses[0], ClientEvent::EchoLoss { seq: 0, .. })); - assert_eq!(client.session.as_ref().unwrap().pending.len(), 0); - assert_eq!(client.session.as_ref().unwrap().timed_out.len(), 1); let late = client.recv_once().unwrap(); match &late[0] { @@ -830,8 +790,6 @@ fn late_reply_after_timeout_preserves_measurement_metadata() { } other => panic!("expected stats-eligible LateReply, got {other:?}"), } - assert_eq!(client.session.as_ref().unwrap().timed_out.len(), 0); - let duplicate = client.recv_once().unwrap(); assert!(matches!( &duplicate[0], @@ -1057,12 +1015,6 @@ fn unmatched_future_reply_does_not_update_highest_received_seq() { let ev_future = client.recv_once().unwrap(); assert!(matches!(&ev_future[0], ClientEvent::Warning { .. })); - assert_eq!( - client.session.as_ref().unwrap().highest_received_seq, - Some(0), - "highest_received_seq should not be updated by unmatched future reply" - ); - thread::sleep(Duration::from_millis(30)); let ev1 = client.recv_once().unwrap(); assert!( @@ -1279,65 +1231,6 @@ fn recv_available_respects_packet_budget() { server.join(); } -#[test] -fn wrapped_reply_after_u32_max_is_not_late() { - let params = default_params(); - let server = start_fake_server(move |socket, tx| { - let (_, peer) = recv_request(&socket, &tx); - let reply = open_reply(FLAG_OPEN | FLAG_REPLY, TOKEN, ¶ms, None); - socket.send_to(&reply, peer).unwrap(); - - socket - .set_read_timeout(Some(Duration::from_secs(2))) - .unwrap(); - let mut seqs = Vec::new(); - for _ in 0..2 { - let mut buf = [0_u8; 2048]; - let (size, _) = socket.recv_from(&mut buf).unwrap(); - tx.send(buf[..size].to_vec()).unwrap(); - seqs.push(u32::from_le_bytes(buf[12..16].try_into().unwrap())); - } - assert_eq!(seqs, vec![u32::MAX, 0]); - - let ts = TimestampFields::default(); - let reply_max = echo_reply_packet(TOKEN, u32::MAX, ¶ms, &ts, None); - socket.send_to(&reply_max, peer).unwrap(); - thread::sleep(Duration::from_millis(10)); - let reply_zero = echo_reply_packet(TOKEN, 0, ¶ms, &ts, None); - socket.send_to(&reply_zero, peer).unwrap(); - }); - let config = ClientConfig { - socket_config: crate::SocketConfig { - recv_timeout: Some(Duration::from_millis(200)), - ..Default::default() - }, - ..default_test_config(server.addr) - }; - let mut client = Client::connect(config).unwrap(); - assert_open_started(client.open().unwrap()); - let session = client.session.as_mut().unwrap(); - session.next_wire_seq = u32::MAX; - - client.send_probe().unwrap(); - client.send_probe().unwrap(); - thread::sleep(Duration::from_millis(50)); - - let first = client.recv_once().unwrap(); - assert!(matches!( - &first[0], - ClientEvent::EchoReply { seq: u32::MAX, .. } - )); - - thread::sleep(Duration::from_millis(30)); - let second = client.recv_once().unwrap(); - assert!( - matches!(&second[0], ClientEvent::EchoReply { seq: 0, .. }), - "freshly wrapped reply should not be late, got {:?}", - second[0] - ); - server.join(); -} - #[test] fn compute_one_way_returns_available_direction_samples() { let sent_at = ClientTimestamp { diff --git a/crates/irtt-client/src/client/tests/sequence.rs b/crates/irtt-client/src/client/tests/sequence.rs index 1703ff2..52dc8f5 100644 --- a/crates/irtt-client/src/client/tests/sequence.rs +++ b/crates/irtt-client/src/client/tests/sequence.rs @@ -1,10 +1,4 @@ use super::*; -use std::time::Instant; - -use crate::{ - probe::{CompletedSet, PendingMap, TimedOutMap}, - session::ActiveSession, -}; #[test] fn sequence_ordering_handles_normal_and_wrapped_values() { @@ -24,21 +18,10 @@ fn sequence_ordering_handles_normal_and_wrapped_values() { #[test] fn highest_received_seq_updates_across_wrap() { - let mut session = ActiveSession { - next_wire_seq: 0, - highest_received_seq: Some(u32::MAX), - packets_sent: 0, - start_mono: Instant::now(), - end_mono: None, - next_send_at: Instant::now(), - pending: PendingMap::new(8), - timed_out: TimedOutMap::new(8), - completed: CompletedSet::new(8), - sending_done: false, - }; + let mut highest = Some(u32::MAX); - update_highest_received(&mut session, 0); - assert_eq!(session.highest_received_seq, Some(0)); - update_highest_received(&mut session, u32::MAX); - assert_eq!(session.highest_received_seq, Some(0)); + update_highest_received(&mut highest, 0); + assert_eq!(highest, Some(0)); + update_highest_received(&mut highest, u32::MAX); + assert_eq!(highest, Some(0)); }