From e1178135785d78d722a035d3d663cf0f6dd1f035 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Mon, 18 May 2026 00:16:51 +0700 Subject: [PATCH] test: consolidate and prune redundant unit tests Simplify the test suite by merging similar test cases and removing redundant assertions that verify trivial initialization and basic container logic. --- crates/irtt-cli/src/main.rs | 94 ++++++------------- .../irtt-client/src/managed/cancellation.rs | 5 - crates/irtt-client/src/metadata.rs | 11 --- crates/irtt-client/src/probe.rs | 17 +--- crates/irtt-proto/src/close.rs | 14 --- crates/irtt-proto/src/hmac.rs | 46 --------- crates/irtt-proto/src/layout.rs | 26 ++--- crates/irtt-proto/src/params.rs | 8 +- 8 files changed, 35 insertions(+), 186 deletions(-) diff --git a/crates/irtt-cli/src/main.rs b/crates/irtt-cli/src/main.rs index 6912d54..de2353b 100644 --- a/crates/irtt-cli/src/main.rs +++ b/crates/irtt-cli/src/main.rs @@ -363,37 +363,15 @@ mod shutdown_tests { use super::*; #[test] - fn shutdown_flag_stops_run_loop() { - let shutdown = AtomicBool::new(false); - assert!(should_continue_run(true, Some(Instant::now()), &shutdown)); - - shutdown.store(true, Ordering::Relaxed); - assert!(!should_continue_run(true, Some(Instant::now()), &shutdown)); - } - - #[test] - fn shutdown_flag_suppresses_due_probe_send() { - let shutdown = AtomicBool::new(false); - let due = Instant::now() - Duration::from_millis(1); - assert!(should_send_probe(Some(due), &shutdown)); - - shutdown.store(true, Ordering::Relaxed); - assert!(!should_send_probe(Some(due), &shutdown)); - } - - #[test] - fn interrupted_continuous_run_uses_final_drain_before_close() { - assert!(should_drain_final(true, true)); - assert!(!should_drain_final(true, false)); - assert!(should_drain_final(false, false)); - } - - #[test] - fn final_summary_gate_prints_finite_or_interrupted_continuous() { - assert!(should_print_final_summary(false, false)); - assert!(should_print_final_summary(false, true)); - assert!(!should_print_final_summary(true, false)); - assert!(should_print_final_summary(true, true)); + fn final_drain_uses_capped_probe_timeout() { + assert_eq!( + final_drain_duration(Duration::from_secs(4)), + Duration::from_secs(4) + ); + assert_eq!( + final_drain_duration(Duration::from_secs(60)), + Duration::from_secs(30) + ); } } @@ -502,26 +480,7 @@ mod tests { } #[test] - fn output_does_not_print_final_summary_when_disabled() { - let mut stats = StatsCollector::new(StatsConfig::continuous()); - let mut out = Vec::new(); - { - let mut stream_output = StreamOutput { - mode: irtt_cli::OutputMode::Human, - human_options: HumanOutputOptions::default(), - print_final_summary: false, - show_running_only_summary_note: true, - out: &mut out, - stats: &mut stats, - }; - stream_output.print_summary().unwrap(); - } - - assert!(out.is_empty()); - } - - #[test] - fn continuous_interrupted_output_prints_bounded_summary_when_enabled() { + fn continuous_summary_prints_when_enabled_and_suppresses_when_disabled() { let mut stats = StatsCollector::new(StatsConfig::continuous()); let mut out = Vec::new(); { @@ -543,6 +502,22 @@ mod tests { assert!(rendered.contains("continuous mode")); assert!(rendered.contains("packets:")); assert!(rendered.contains("received=1")); + + let mut stats = StatsCollector::new(StatsConfig::continuous()); + let mut out = Vec::new(); + { + let mut stream_output = StreamOutput { + mode: irtt_cli::OutputMode::Human, + human_options: HumanOutputOptions::default(), + print_final_summary: false, + show_running_only_summary_note: true, + out: &mut out, + stats: &mut stats, + }; + stream_output.print_summary().unwrap(); + } + + assert!(out.is_empty()); } #[test] @@ -596,7 +571,7 @@ mod tests { } #[test] - fn finite_stats_memory_warning_skips_non_warning_runs() { + fn finite_stats_memory_warning_reports_only_large_finite_runs() { for args in [ cli_args(&["--duration", "0", "--interval", "1ms", "127.0.0.1:2112"]), cli_args(&[ @@ -609,10 +584,7 @@ mod tests { ] { assert_eq!(finite_stats_memory_warning(&args), None); } - } - #[test] - fn finite_stats_memory_warning_reports_threshold_tiers() { for (threshold, size, guidance) in [ ( FINITE_STATS_MEMORY_WARNING_BYTES, @@ -657,16 +629,4 @@ mod tests { fn estimate_finite_stats_memory_bytes_saturates() { assert_eq!(estimate_finite_stats_memory_bytes(u64::MAX), u64::MAX); } - - #[test] - fn final_drain_uses_capped_probe_timeout() { - assert_eq!( - final_drain_duration(Duration::from_secs(4)), - Duration::from_secs(4) - ); - assert_eq!( - final_drain_duration(Duration::from_secs(60)), - Duration::from_secs(30) - ); - } } diff --git a/crates/irtt-client/src/managed/cancellation.rs b/crates/irtt-client/src/managed/cancellation.rs index 4eca118..66b27f0 100644 --- a/crates/irtt-client/src/managed/cancellation.rs +++ b/crates/irtt-client/src/managed/cancellation.rs @@ -27,11 +27,6 @@ impl CancellationToken { mod tests { use super::*; - #[test] - fn token_starts_uncancelled() { - assert!(!CancellationToken::new().is_cancelled()); - } - #[test] fn cancel_is_idempotent_and_visible_through_clones() { let token = CancellationToken::new(); diff --git a/crates/irtt-client/src/metadata.rs b/crates/irtt-client/src/metadata.rs index 8d0d509..1b1e751 100644 --- a/crates/irtt-client/src/metadata.rs +++ b/crates/irtt-client/src/metadata.rs @@ -24,17 +24,6 @@ mod tests { use super::ReceiveMeta; use crate::event::PacketMeta; - #[test] - fn metadata_default_is_unavailable() { - let meta = ReceiveMeta::default(); - let packet_meta = PacketMeta::from(meta); - - assert_eq!(packet_meta.traffic_class, None); - assert_eq!(packet_meta.dscp, None); - assert_eq!(packet_meta.ecn, None); - assert_eq!(packet_meta.kernel_rx_timestamp, None); - } - #[test] fn metadata_observed_traffic_class_zero_preserves_observed_zero() { let packet_meta = PacketMeta::from(ReceiveMeta { diff --git a/crates/irtt-client/src/probe.rs b/crates/irtt-client/src/probe.rs index 5bfb931..b26e20b 100644 --- a/crates/irtt-client/src/probe.rs +++ b/crates/irtt-client/src/probe.rs @@ -193,18 +193,6 @@ mod tests { } } - #[test] - fn pending_map_insert_and_remove() { - let mut map = PendingMap::new(10); - let now = Instant::now(); - let probe = pending(0, now + Duration::from_secs(4)); - map.insert(probe).unwrap(); - assert_eq!(map.len(), 1); - assert!(map.remove(0).is_some()); - assert_eq!(map.len(), 0); - assert!(map.remove(0).is_none()); - } - #[test] fn pending_map_rejects_over_capacity() { let mut map = PendingMap::new(2); @@ -220,7 +208,7 @@ mod tests { } #[test] - fn completed_set_tracks_and_evicts() { + fn bounded_probe_tracking_evicts_oldest_entries() { let mut set = CompletedSet::new(3); set.insert(0); set.insert(1); @@ -232,10 +220,7 @@ mod tests { assert_eq!(set.set.len(), 3); assert!(!set.contains(0)); assert!(set.contains(3)); - } - #[test] - fn timed_out_map_tracks_and_evicts() { let mut map = TimedOutMap::new(2); let now = Instant::now(); map.insert(pending(0, now)); diff --git a/crates/irtt-proto/src/close.rs b/crates/irtt-proto/src/close.rs index 423fe35..4f876d9 100644 --- a/crates/irtt-proto/src/close.rs +++ b/crates/irtt-proto/src/close.rs @@ -31,20 +31,6 @@ mod tests { use super::*; use crate::{hmac, FLAG_HMAC}; - #[test] - fn close_request_sizes() { - let request = CloseRequest { - token: 0x7896_b6ab_8771_5213, - }; - assert_eq!(encode_close_request(&request, None).unwrap().len(), 12); - assert_eq!( - encode_close_request(&request, Some(b"testkey")) - .unwrap() - .len(), - 28 - ); - } - #[test] fn hmac_close_request_places_token_after_hmac() { let packet = encode_close_request( diff --git a/crates/irtt-proto/src/hmac.rs b/crates/irtt-proto/src/hmac.rs index 95f5626..273ba3b 100644 --- a/crates/irtt-proto/src/hmac.rs +++ b/crates/irtt-proto/src/hmac.rs @@ -98,16 +98,6 @@ mod tests { ); } - #[test] - fn compute_hmac_does_not_mutate_input() { - let packet = packet_with_hmac_field([0xa5; HMAC_SIZE]); - let original = packet.clone(); - - let _digest = compute_hmac(b"testkey", &packet, hmac_offset()).unwrap(); - - assert_eq!(packet, original); - } - #[test] fn compute_hmac_matches_copy_and_zero_semantics() { let mut packet = packet_with_hmac_field([0xa5; HMAC_SIZE]); @@ -119,42 +109,6 @@ mod tests { assert_eq!(digest, reference); } - #[test] - fn compute_hmac_in_place_does_not_set_hmac_flag() { - let mut packet = Vec::new(); - packet.extend_from_slice(&MAGIC); - packet.push(FLAG_OPEN); - packet.extend_from_slice(&[0; HMAC_SIZE]); - packet.extend_from_slice(&[1, 2, 3, 4]); - - compute_hmac_in_place(b"testkey", &mut packet, hmac_offset()).unwrap(); - - assert_eq!(packet[3], FLAG_OPEN); - assert!(packet[hmac_offset()..hmac_offset() + HMAC_SIZE] - .iter() - .any(|byte| *byte != 0)); - assert_eq!(&packet[hmac_offset() + HMAC_SIZE..], &[1, 2, 3, 4]); - } - - #[test] - fn compute_hmac_in_place_only_writes_hmac_field() { - let mut packet = packet_with_hmac_field([0xa5; HMAC_SIZE]); - packet.extend_from_slice(&[5, 6, 7, 8, 9]); - let original = packet.clone(); - - compute_hmac_in_place(b"testkey", &mut packet, hmac_offset()).unwrap(); - - assert_eq!(&packet[..hmac_offset()], &original[..hmac_offset()]); - assert_ne!( - &packet[hmac_offset()..hmac_offset() + HMAC_SIZE], - &original[hmac_offset()..hmac_offset() + HMAC_SIZE] - ); - assert_eq!( - &packet[hmac_offset() + HMAC_SIZE..], - &original[hmac_offset() + HMAC_SIZE..] - ); - } - #[test] fn verify_hmac_rejects_modified_authenticated_byte() { let mut packet = packet_with_hmac_field([0; HMAC_SIZE]); diff --git a/crates/irtt-proto/src/layout.rs b/crates/irtt-proto/src/layout.rs index 4d12edb..02b41e3 100644 --- a/crates/irtt-proto/src/layout.rs +++ b/crates/irtt-proto/src/layout.rs @@ -155,6 +155,12 @@ mod tests { #[test] fn verified_layout_lengths() { + assert_eq!(PacketLayout::open_request(false).header_len(), 4); + assert_eq!(PacketLayout::open_request(true).header_len(), 20); + assert_eq!(PacketLayout::open_reply(false).header_len(), 12); + assert_eq!(PacketLayout::open_reply(true).header_len(), 28); + assert_eq!(PacketLayout::close_request(false).header_len(), 12); + assert_eq!(PacketLayout::close_request(true).header_len(), 28); assert_eq!( echo_header_len( false, @@ -199,16 +205,6 @@ mod tests { ); } - #[test] - fn open_and_close_layout_lengths_account_for_hmac() { - assert_eq!(PacketLayout::open_request(false).header_len(), 4); - assert_eq!(PacketLayout::open_request(true).header_len(), 20); - assert_eq!(PacketLayout::open_reply(false).header_len(), 12); - assert_eq!(PacketLayout::open_reply(true).header_len(), 28); - assert_eq!(PacketLayout::close_request(false).header_len(), 12); - assert_eq!(PacketLayout::close_request(true).header_len(), 28); - } - #[test] fn layout_matrix_matches_stats_timestamps_clock_and_hmac_rules() { for stats in [ @@ -280,16 +276,6 @@ mod tests { } } - #[test] - fn no_timestamp_layout_ignores_clock() { - for clock in [Clock::Wall, Clock::Monotonic, Clock::Both] { - assert_eq!( - echo_header_len(false, ¶ms(ReceivedStats::None, StampAt::None, clock)), - 16 - ); - } - } - #[test] fn negotiated_length_never_smaller_than_header() { let mut p = params(ReceivedStats::Both, StampAt::Both, Clock::Both); diff --git a/crates/irtt-proto/src/params.rs b/crates/irtt-proto/src/params.rs index 0bbb597..0e7d573 100644 --- a/crates/irtt-proto/src/params.rs +++ b/crates/irtt-proto/src/params.rs @@ -241,7 +241,7 @@ mod tests { } #[test] - fn params_round_trip_received_stats_modes() { + fn params_round_trip_negotiated_option_modes() { for received_stats in [ ReceivedStats::None, ReceivedStats::Count, @@ -254,10 +254,7 @@ mod tests { ..Params::default() }); } - } - #[test] - fn params_round_trip_timestamp_modes() { for stamp_at in [ StampAt::None, StampAt::Send, @@ -272,10 +269,7 @@ mod tests { ..Params::default() }); } - } - #[test] - fn params_round_trip_clock_modes() { for clock in [Clock::Wall, Clock::Monotonic, Clock::Both] { assert_round_trip(Params { protocol_version: 1,