From bed877760ed748e94a325adf1eb9bf8d09728a00 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Sun, 17 May 2026 14:48:10 +0700 Subject: [PATCH] test: prune client tests that assert private implementation details --- crates/irtt-client/src/client.rs | 24 --- .../irtt-client/src/client/tests/no_test.rs | 15 -- crates/irtt-client/src/client/tests/open.rs | 33 ---- .../src/client/tests/probes_replies.rs | 163 ++---------------- 4 files changed, 17 insertions(+), 218 deletions(-) diff --git a/crates/irtt-client/src/client.rs b/crates/irtt-client/src/client.rs index 2de5613..201a408 100644 --- a/crates/irtt-client/src/client.rs +++ b/crates/irtt-client/src/client.rs @@ -327,12 +327,6 @@ impl Client { Ok(all_events) } - #[cfg(test)] - pub(crate) fn recv_buffer_size(&self) -> usize { - recv_buffer_size(self.config.hmac_key.is_some(), self.negotiated.as_ref()) - .expect("test params must have a non-negative packet length") - } - /// Polls for probes that have timed out as of the current monotonic time. pub fn poll_timeouts(&mut self) -> Result, ClientError> { self.poll_timeouts_at(Instant::now()) @@ -394,24 +388,6 @@ impl Client { .map_or(0, |session| session.packets_sent) } - #[cfg(test)] - fn process_received_packet( - &mut self, - packet: &[u8], - now: ClientTimestamp, - meta: ReceiveMeta, - ) -> Result, ClientError> { - let packet_len = packet.len(); - let Some(reply) = self.decode_received_packet(packet) else { - return Ok(vec![ClientEvent::Warning { - kind: WarningKind::MalformedOrUnrelatedPacket, - message: "dropped malformed or unrelated packet".to_owned(), - at: now, - }]); - }; - self.process_echo_reply(reply, packet_len, now, meta) - } - fn decode_received_packet(&self, packet: &[u8]) -> Option { let negotiated = self .negotiated diff --git a/crates/irtt-client/src/client/tests/no_test.rs b/crates/irtt-client/src/client/tests/no_test.rs index 76306e9..02b7b07 100644 --- a/crates/irtt-client/src/client/tests/no_test.rs +++ b/crates/irtt-client/src/client/tests/no_test.rs @@ -1,20 +1,5 @@ use super::*; -#[test] -fn no_test_open_close_succeeds_on_open_reply_close() { - let mut config = default_test_config(SocketAddr::from(([127, 0, 0, 1], 1))); - config.run_mode = RunMode::NoTest; - let params = params_from_config(&config).unwrap(); - let server = no_test_server(params.clone(), 0); - config.server_addr = server.addr.to_string(); - let mut client = Client::connect(config).unwrap(); - let negotiated = assert_no_test_completed(client.open().unwrap()); - assert_eq!(negotiated.params, params); - assert_eq!(client.negotiated.as_ref(), Some(&negotiated)); - assert!(matches!(client.close(), Err(ClientError::NotOpen))); - server.join(); -} - #[test] fn no_test_success_validates_params() { let mut config = default_test_config(SocketAddr::from(([127, 0, 0, 1], 1))); diff --git a/crates/irtt-client/src/client/tests/open.rs b/crates/irtt-client/src/client/tests/open.rs index f988603..ae19c5f 100644 --- a/crates/irtt-client/src/client/tests/open.rs +++ b/crates/irtt-client/src/client/tests/open.rs @@ -1,16 +1,5 @@ use super::*; -#[test] -fn address_resolution_connects_to_local_fake_server() { - let server = start_fake_server(|socket, tx| { - let _ = recv_request(&socket, &tx); - }); - let client = Client::connect(default_test_config(server.addr)).unwrap(); - client.socket.send(b"ping").unwrap(); - assert_eq!(server.rx.recv().unwrap(), b"ping"); - server.join(); -} - #[test] fn successful_open_handshake() { let config = default_test_config(SocketAddr::from(([127, 0, 0, 1], 1))); @@ -20,7 +9,6 @@ fn successful_open_handshake() { let negotiated = assert_open_started(client.open().unwrap()); assert_eq!(negotiated.params, params); - assert!(matches!(client.phase, ClientPhase::Open { token: TOKEN })); server.join(); } @@ -69,27 +57,6 @@ fn open_timeout_after_all_timeouts() { server.join(); } -#[test] -fn open_restores_configured_read_timeout_after_timeout() { - let server = timeout_server(Duration::from_millis(700)); - let config = ClientConfig { - open_timeouts: vec![Duration::from_millis(200)], - socket_config: crate::SocketConfig { - recv_timeout: Some(Duration::from_millis(450)), - ..crate::SocketConfig::default() - }, - ..default_test_config(server.addr) - }; - let mut client = Client::connect(config).unwrap(); - assert!(matches!(client.open(), Err(ClientError::OpenTimeout))); - - let start = std::time::Instant::now(); - let mut buf = [0_u8; 1]; - assert!(client.socket.recv(&mut buf).is_err()); - assert!(start.elapsed() >= Duration::from_millis(350)); - server.join(); -} - #[test] fn protocol_version_mismatch_fails() { let mut config = default_test_config(SocketAddr::from(([127, 0, 0, 1], 1))); diff --git a/crates/irtt-client/src/client/tests/probes_replies.rs b/crates/irtt-client/src/client/tests/probes_replies.rs index d230cb9..ff38071 100644 --- a/crates/irtt-client/src/client/tests/probes_replies.rs +++ b/crates/irtt-client/src/client/tests/probes_replies.rs @@ -1057,16 +1057,6 @@ fn compute_one_way_returns_none_when_both_directions_fail() { assert!(result.is_none()); } -#[test] -fn unix_epoch_ns_i64_rejects_out_of_range_wall_time() { - let i64_max_ns = u64::try_from(i64::MAX).unwrap(); - let max = SystemTime::UNIX_EPOCH + Duration::from_nanos(i64_max_ns); - let overflow = max + Duration::from_nanos(1); - - assert_eq!(unix_epoch_ns_i64(max), Some(i64::MAX)); - assert_eq!(unix_epoch_ns_i64(overflow), None); -} - #[test] fn compute_one_way_omits_samples_when_client_wall_time_overflows_i64() { let i64_max_ns = u64::try_from(i64::MAX).unwrap(); @@ -1089,95 +1079,6 @@ fn compute_one_way_omits_samples_when_client_wall_time_overflows_i64() { assert_eq!(compute_one_way(&sent_at, &received_at, &ts), None); } -#[test] -fn matched_reply_with_reversed_monotonic_time_still_emits_event() { - let params = default_params(); - let server = silent_open_server(params.clone()); - let config = ClientConfig { - 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()); - - let base = Instant::now() + Duration::from_secs(1); - let send_ts = ClientTimestamp { - mono: base, - wall: SystemTime::now(), - }; - client.send_probe_at(send_ts).unwrap(); - - let recv_ts = ClientTimestamp { - mono: send_ts.mono - Duration::from_millis(500), - wall: send_ts.wall + Duration::from_millis(10), - }; - let reply = echo_reply_packet(TOKEN, 0, ¶ms, &TimestampFields::default(), None); - let events = client - .process_received_packet(&reply, recv_ts, ReceiveMeta::default()) - .unwrap(); - - assert_eq!(events.len(), 1); - match &events[0] { - ClientEvent::EchoReply { rtt, .. } => { - assert_eq!(rtt.raw, Duration::ZERO); - assert_eq!(rtt.effective, Duration::ZERO); - } - other => panic!("expected EchoReply, got {other:?}"), - } - - client.close().unwrap(); - server.join(); -} - -#[test] -fn process_received_packet_uses_supplied_receive_metadata() { - let params = default_params(); - let server = silent_open_server(params.clone()); - let config = ClientConfig { - 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_at(ClientTimestamp::now()).unwrap(); - let recv_ts = ClientTimestamp::now(); - let reply = echo_reply_packet(TOKEN, 0, ¶ms, &TimestampFields::default(), None); - let events = client - .process_received_packet( - &reply, - recv_ts, - ReceiveMeta { - traffic_class: Some(186), - kernel_rx_timestamp: Some(SystemTime::UNIX_EPOCH + Duration::new(12, 34)), - }, - ) - .unwrap(); - - assert_eq!(events.len(), 1); - match &events[0] { - ClientEvent::EchoReply { packet_meta, .. } => { - assert_eq!(packet_meta.traffic_class, Some(186)); - assert_eq!(packet_meta.dscp, Some(46)); - assert_eq!(packet_meta.ecn, Some(2)); - assert_eq!( - packet_meta.kernel_rx_timestamp, - Some(SystemTime::UNIX_EPOCH + Duration::new(12, 34)) - ); - } - other => panic!("expected EchoReply, got {other:?}"), - } - - client.close().unwrap(); - server.join(); -} - #[test] fn recv_available_drains_burst_replies() { let params = default_params(); @@ -1258,7 +1159,21 @@ fn short_echo_reply_does_not_emit_echo_reply() { length: 64, ..default_params() }; - let server = silent_open_server(params.clone()); + let server = start_fake_server({ + let params = params.clone(); + 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(); + let (request, _) = recv_request(&socket, &tx); + let seq = u32::from_le_bytes(request[12..16].try_into().unwrap()); + let mut reply = + echo_reply_packet(TOKEN, seq, ¶ms, &TimestampFields::default(), None); + reply.truncate(echo_packet_len(false, ¶ms) - 1); + socket.send_to(&reply, peer).unwrap(); + let _ = recv_request_timeout(&socket, &tx); + } + }); let config = ClientConfig { length: 64, socket_config: crate::SocketConfig { @@ -1271,11 +1186,7 @@ fn short_echo_reply_does_not_emit_echo_reply() { assert_open_started(client.open().unwrap()); client.send_probe().unwrap(); - let mut packet = echo_reply_packet(TOKEN, 0, ¶ms, &TimestampFields::default(), None); - packet.truncate(echo_packet_len(false, ¶ms) - 1); - let events = client - .process_received_packet(&packet, ClientTimestamp::now(), ReceiveMeta::default()) - .unwrap(); + let events = client.recv_once().unwrap(); assert!(matches!( events.as_slice(), @@ -1289,39 +1200,7 @@ fn short_echo_reply_does_not_emit_echo_reply() { } #[test] -fn overlong_echo_reply_does_not_emit_echo_reply() { - let params = default_params(); - let server = silent_open_server(params.clone()); - 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()); - client.send_probe().unwrap(); - - let mut packet = echo_reply_packet(TOKEN, 0, ¶ms, &TimestampFields::default(), None); - packet.push(0); - let events = client - .process_received_packet(&packet, ClientTimestamp::now(), ReceiveMeta::default()) - .unwrap(); - - assert!(matches!( - events.as_slice(), - [ClientEvent::Warning { - kind: WarningKind::MalformedOrUnrelatedPacket, - .. - }] - )); - client.close().unwrap(); - server.join(); -} - -#[test] -fn overlong_datagram_detection_uses_extra_receive_byte() { +fn overlong_echo_reply_datagram_does_not_emit_echo_reply() { let params = Params { length: 4096, ..default_params() @@ -1351,14 +1230,6 @@ fn overlong_datagram_detection_uses_extra_receive_byte() { }; let mut client = Client::connect(config).unwrap(); assert_open_started(client.open().unwrap()); - assert_eq!( - client.recv_buffer_size(), - echo_packet_len(false, ¶ms) + 1 - ); - assert_eq!( - client.recv_buffer.len(), - echo_packet_len(false, ¶ms) + 1 - ); client.send_probe().unwrap(); let events = client.recv_once().unwrap();