From 74e75329654c98ddf64cb785ce29116eb9b1e58c Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Fri, 2 Oct 2026 12:47:27 +0700 Subject: [PATCH] tests: clean up more tests across the project --- crates/irtt-app/src/cmd/client/output.rs | 116 +------- crates/irtt-app/src/cmd/client/run.rs | 32 -- crates/irtt-app/src/cmd/tui/args.rs | 44 --- crates/irtt-app/src/cmd/tui/ui.rs | 99 ------- crates/irtt-app/src/shared/client/prepare.rs | 153 ---------- crates/irtt-client/src/client/tests/config.rs | 75 +---- crates/irtt-client/src/managed/tests.rs | 122 +------- crates/irtt-client/src/session/machine.rs | 278 ------------------ crates/irtt-client/src/session/negotiation.rs | 67 ----- crates/irtt-client/src/socket.rs | 19 -- crates/irtt-server/src/fill.rs | 90 ------ crates/irtt-server/src/set.rs | 77 ----- crates/irtt-server/src/tests/fill.rs | 64 +--- crates/irtt-stats/src/ipdv.rs | 9 - crates/irtt-stats/src/loss.rs | 18 -- crates/irtt-stats/src/retention.rs | 152 ---------- crates/irtt-stats/tests/retention.rs | 48 --- 17 files changed, 28 insertions(+), 1435 deletions(-) diff --git a/crates/irtt-app/src/cmd/client/output.rs b/crates/irtt-app/src/cmd/client/output.rs index ecc5f16..ffcbf7a 100644 --- a/crates/irtt-app/src/cmd/client/output.rs +++ b/crates/irtt-app/src/cmd/client/output.rs @@ -1264,15 +1264,6 @@ mod tests { assert_eq!(header.split_whitespace().next(), Some("target")); } - #[test] - fn default_structured_columns_always_include_target_first() { - for format in [OutputFormat::Csv, OutputFormat::Tsv, OutputFormat::Jsonl] { - let columns = default_columns(format, false); - assert_eq!(columns.first(), Some(&Column::Target)); - assert_eq!(columns, ALL_COLUMNS); - } - } - #[test] fn columns_default_keyword_matches_omitted_columns() { for format in [ @@ -1329,83 +1320,24 @@ mod tests { assert!(line.starts_with("{\"target\":\"ams\"")); } - #[test] - fn truncate_for_table_leaves_short_value_unchanged() { - assert_eq!(truncate_for_table("ams", 18), "ams"); - } - - #[test] - fn truncate_for_table_leaves_exact_width_value_unchanged() { - let value = "a".repeat(18); - assert_eq!(truncate_for_table(&value, 18), value); - } - - #[test] - fn truncate_for_table_shortens_long_value_to_width() { - let value = "a".repeat(40); - let truncated = truncate_for_table(&value, 18); - - assert_eq!(truncated.chars().count(), 18); - assert!(truncated.len() <= 18); // ASCII: byte len matches char len - } - - #[test] - fn truncate_for_table_keeps_prefix_and_appends_ellipsis_deterministically() { - let value = "abcdefghijklmnopqrstuvwxyz"; - let truncated = truncate_for_table(value, 18); - - assert_eq!(truncated, "abcdefghijklmno..."); - assert_eq!(truncated.chars().count(), 18); - } - - #[test] - fn truncate_for_table_does_not_split_multibyte_code_points() { - // Each character below is multibyte in UTF-8; a byte-index slice - // would panic or corrupt the string on a non-boundary cut. - let value = "\u{1F600}".repeat(40); // 😀 repeated, well past width 18 - let truncated = truncate_for_table(&value, 18); - - // Must not panic (validated by test completing) and must remain - // valid UTF-8 with exactly 18 chars (15 prefix chars + "..."). - assert_eq!(truncated.chars().count(), 18); - assert!(truncated.ends_with("...")); - assert_eq!(truncated.chars().filter(|c| *c == '\u{1F600}').count(), 15); - } - #[test] fn table_row_with_long_target_keeps_next_column_aligned() { let config = output_config(OutputFormat::Table, None); - let long_target = "a".repeat(40); let header = config.render_header().unwrap(); - let line = config - .render_event( - &reply(), - Some(&long_target), - Some(&EventRenderStats::default()), - ) - .unwrap(); + let event_offset = header.find("event").unwrap(); - // Target column is truncated to its table width (18), so the - // "event" column starts at the same offset for both a normal and an - // overlong target label. - let short_config = output_config(OutputFormat::Table, None); - let short_line = short_config - .render_event(&reply(), Some("ams"), Some(&EventRenderStats::default())) - .unwrap(); - - let target_width = Column::Target.table_width(); - let expected_event_offset = target_width + " ".len(); - assert_eq!(&header[expected_event_offset..][..5], "event"); - // The event column ("echo_reply") starts at the same offset - // regardless of whether the target label was short or overlong. - assert_eq!( - &line[expected_event_offset..][.."echo_reply".len()], - "echo_reply" - ); - assert_eq!( - &short_line[expected_event_offset..][.."echo_reply".len()], - "echo_reply" - ); + for label in ["ams".to_owned(), "a".repeat(40), "é".repeat(40)] { + let line = config + .render_event(&reply(), Some(&label), Some(&EventRenderStats::default())) + .unwrap(); + let prefix = line.split("echo_reply").next().unwrap(); + assert_eq!(prefix.chars().count(), event_offset, "{line}"); + if label.chars().count() > event_offset { + assert!(prefix.trim_end().ends_with("..."), "{line}"); + } else { + assert!(prefix.starts_with(&label), "{line}"); + } + } } #[test] @@ -1452,26 +1384,4 @@ mod tests { assert_eq!(line, format!("{{\"target\":\"{long_target}\",\"seq\":7}}")); } - - #[test] - fn explicit_table_columns_with_target_truncate_same_as_default() { - let config = output_config(OutputFormat::Table, Some("target,seq")); - let long_target = "d".repeat(40); - let line = config - .render_event( - &reply(), - Some(&long_target), - Some(&EventRenderStats::default()), - ) - .unwrap(); - - let target_width = Column::Target.table_width(); - let expected_prefix: String = long_target - .chars() - .take(target_width - 3) - .chain("...".chars()) - .collect(); - assert!(line.starts_with(&expected_prefix)); - assert_eq!(&line[..target_width], expected_prefix.as_str()); - } } diff --git a/crates/irtt-app/src/cmd/client/run.rs b/crates/irtt-app/src/cmd/client/run.rs index c6780e2..ab5496c 100644 --- a/crates/irtt-app/src/cmd/client/run.rs +++ b/crates/irtt-app/src/cmd/client/run.rs @@ -1052,26 +1052,6 @@ mod tests { assert!(mailbox.latest().is_none()); } - #[test] - fn eof_and_shutdown_stop_without_waiting_for_the_stdin_reader() { - let mailbox = StdinMailbox::default(); - let shutdown_requested = AtomicBool::new(false); - assert!(stdin_stop_request(&shutdown_requested, &mailbox).is_none()); - - mailbox.finish(); - assert!(matches!( - stdin_stop_request(&shutdown_requested, &mailbox), - Some(StdinStop::Eof) - )); - - let mailbox = StdinMailbox::default(); - shutdown_requested.store(true, std::sync::atomic::Ordering::Relaxed); - assert!(matches!( - stdin_stop_request(&shutdown_requested, &mailbox), - Some(StdinStop::Interrupted) - )); - } - fn status_with_targets(targets: &[TargetInstance]) -> ManagedStatus { ManagedStatus { lifecycle: ManagedLifecycle::Running, @@ -1209,18 +1189,6 @@ mod tests { assert!(finite_stats_memory_warning(&long_finite_args(), usize::MAX).is_some()); } - #[test] - fn lagged_events_are_counted_saturatingly() { - let mut dropped = 0_u64; - dropped = dropped.saturating_add(7); - dropped = dropped.saturating_add(3); - assert_eq!(dropped, 10); - assert_eq!(u64::MAX.saturating_add(1), u64::MAX); - assert!(dropped_event_warning(dropped) - .unwrap() - .contains("dropped 10 managed run events")); - } - #[test] fn target_failure_diagnostics_include_primary_and_cleanup_failures() { let target = ManagedTargetOutcome { diff --git a/crates/irtt-app/src/cmd/tui/args.rs b/crates/irtt-app/src/cmd/tui/args.rs index 5c971e0..c1397d3 100644 --- a/crates/irtt-app/src/cmd/tui/args.rs +++ b/crates/irtt-app/src/cmd/tui/args.rs @@ -2,8 +2,6 @@ use std::time::Duration; use clap::Parser; -#[cfg(test)] -use crate::shared::client::TimestampArg; use crate::shared::client::{ parse_target, parse_test_duration, prepare_managed_run, CommonClientArgs, GroupPacingArg, ManagedRunSetup, TargetArg, TargetSelection, @@ -66,11 +64,6 @@ impl TuiArgs { pub fn is_continuous(&self) -> bool { self.duration == Duration::ZERO } - - #[cfg(test)] - pub fn timestamp_mode(&self) -> TimestampArg { - self.common.tstamp - } } impl std::ops::Deref for TuiArgs { @@ -156,35 +149,6 @@ mod tests { assert!(parse(&["ams=ams.example.com:2112"]).is_ok()); } - #[test] - fn duplicate_labels_are_rejected() { - let args = parse(&["host-a:2112", "host-a:2112=host-b:2112"]).unwrap(); - let err = args.prepare().unwrap_err(); - - assert!(err.contains("duplicate target label")); - } - - #[test] - fn duplicate_positional_target_strings_get_stable_suffixes() { - let args = parse(&["host-a:2112", "host-a:2112"]).unwrap(); - let specs = args.prepare().unwrap().targets; - - assert_eq!(specs[0].label, "host-a:2112"); - assert_eq!(specs[1].label, "host-a:2112#2"); - } - - #[test] - fn duplicate_target_endpoints_are_allowed() { - let args = parse(&["127.0.0.1:2112", "127.0.0.1"]).unwrap(); - assert_eq!(args.prepare().unwrap().target_count(), 2); - } - - #[test] - fn invalid_labelled_target_syntax_is_rejected() { - assert!(parse(&["=127.0.0.1:2112"]).unwrap().prepare().is_err()); - assert!(parse(&["label="]).unwrap().prepare().is_err()); - } - #[test] fn old_target_option_is_rejected() { assert!(parse(&["--target", "eu=host.example"]).is_err()); @@ -270,14 +234,6 @@ mod tests { assert_eq!(shared.socket_config.ttl, tui.socket_config.ttl); } - #[test] - fn an_empty_hmac_key_is_rejected_via_the_shared_args() { - // CommonClientArgs is flattened into both the client and TUI parsers, - // so proving rejection once here covers both without duplicating it. - let err = parse(&["--hmac", "", "127.0.0.1:2112"]).unwrap_err(); - assert!(err.to_string().contains("HMAC key must not be empty")); - } - #[test] fn tui_help_lists_multi_target_options() { let help = TuiArgs::command().render_help().to_string(); diff --git a/crates/irtt-app/src/cmd/tui/ui.rs b/crates/irtt-app/src/cmd/tui/ui.rs index 6769e8f..49f0f9d 100644 --- a/crates/irtt-app/src/cmd/tui/ui.rs +++ b/crates/irtt-app/src/cmd/tui/ui.rs @@ -2057,19 +2057,6 @@ mod tests { } } - fn graph_sample_with_timing(seq: u32, effective_ns: i128) -> GraphSample { - GraphSample { - timestamp: Instant::now() + Duration::from_secs(u64::from(seq)), - seq, - effective_ns, - raw_ns: effective_ns + 1_000, - adjusted_ns: Some(effective_ns + 500), - client_to_server_ns: Some(effective_ns / 3), - server_to_client_ns: Some(effective_ns / 2), - server_processing_ns: Some(100_000), - } - } - fn series(data: Vec<(f64, f64)>) -> ChartSeries { ChartSeries { name: GraphMetric::EffectiveRtt.label().to_owned(), @@ -2131,18 +2118,6 @@ mod tests { assert_eq!(format_span(Duration::from_secs(90)), "1m30s"); } - #[test] - fn formats_signed_durations_and_missing_values() { - assert_eq!(format_optional_ns_i128(Some(-1_500_000)), "-1.5ms"); - assert_eq!(format_optional_ns_i128(Some(750)), "750ns"); - assert_eq!(format_optional_ns_i128(None), "-"); - assert_eq!(format_optional_ns_i128(Some(1_500)), "1.5µs"); - assert_eq!(format_optional_span(None), "-"); - assert_eq!(format_duration(Duration::from_millis(25)), "25.0ms"); - assert_eq!(format_percent_ratio(1, 4), "25.00%"); - assert_eq!(format_optional_hex(Some(0x1f)), "0x1f"); - } - #[test] fn session_started_sets_remote_session_and_running_status() { let mut state = TuiState::default(); @@ -2636,13 +2611,6 @@ mod tests { assert_eq!(state.recent_events.front().unwrap(), "event 5"); } - #[test] - fn recent_event_visible_count_tracks_panel_inner_height() { - assert_eq!(recent_events_visible_count(0), 0); - assert_eq!(recent_events_visible_count(2), 0); - assert_eq!(recent_events_visible_count(9), 7); - } - #[test] fn visible_window_selects_samples_inside_viewport() { let history: VecDeque<_> = (0..300).map(|seq| graph_sample(seq, seq.into())).collect(); @@ -2722,20 +2690,6 @@ mod tests { assert!(max_y > 12.0, "max_y={max_y}"); } - #[test] - fn y_axis_tick_labels_use_label_count_minus_one_spacing() { - let labels = y_axis_labels(-1.0, 1.0, 5); - let rendered = labels - .iter() - .map(|span| span.content.as_ref()) - .collect::>(); - - assert_eq!( - rendered, - vec!["-1.00ms", "-500µs", "0.0µs", "500µs", "1.00ms"] - ); - } - #[test] fn optional_missing_one_way_series_are_omitted_not_zero_filled() { let visible_samples = [graph_sample(1, 2_000_000)]; @@ -2750,22 +2704,6 @@ mod tests { assert_eq!(rtt[0].name, "effective RTT"); } - #[test] - fn graph_metrics_use_readable_default_series() { - let visible_samples = [graph_sample_with_timing(1, 3_000_000)]; - let visible: Vec<_> = visible_samples.iter().collect(); - let viewport = viewport_for_visible(&visible); - - assert_eq!( - graph_series(GraphMetric::EffectiveRtt, &visible, viewport)[0].data, - vec![(0.0, 3.0)] - ); - assert_eq!( - graph_series(GraphMetric::ClientToServer, &visible, viewport)[0].data, - vec![(0.0, 1.0)] - ); - } - #[test] fn multi_target_metric_series_uses_selected_sample_field() { let mut target = TuiTargetState::new("alpha".to_owned(), stats_config(true)); @@ -2848,43 +2786,6 @@ mod tests { ); } - #[test] - fn graph_metric_cycling_walks_all_metrics() { - let cases = [ - GraphMetric::EffectiveRtt, - GraphMetric::RawRtt, - GraphMetric::AdjustedRtt, - GraphMetric::ClientToServer, - GraphMetric::ServerToClient, - GraphMetric::ServerProcessing, - GraphMetric::EffectiveRtt, - ]; - let mut state = TuiState::default(); - for metric in cases { - assert_eq!(state.graph_metric, metric); - state.cycle_graph_metric(); - } - } - - #[test] - fn view_toggle_switches_between_graph_and_dashboard() { - let mut state = TuiState::default(); - - assert_eq!(state.view, TuiView::Graph); - state.toggle_view(); - assert_eq!(state.view, TuiView::Dashboard); - state.toggle_view(); - assert_eq!(state.view, TuiView::Graph); - } - - #[test] - fn target_label_span_uses_graph_series_style() { - let span = target_label_span("alpha-target", 3, 16); - - assert_eq!(span.style, target_style(3).add_modifier(Modifier::BOLD)); - assert_eq!(span.content.as_ref(), "alpha-target "); - } - #[test] fn pause_suppresses_scheduled_renders_but_allows_forced_controls() { let now = Instant::now(); diff --git a/crates/irtt-app/src/shared/client/prepare.rs b/crates/irtt-app/src/shared/client/prepare.rs index 9af920f..077fac0 100644 --- a/crates/irtt-app/src/shared/client/prepare.rs +++ b/crates/irtt-app/src/shared/client/prepare.rs @@ -121,156 +121,3 @@ pub fn prepare_managed_run( stdin_controlled: selection.stdin_controlled, }) } - -#[cfg(test)] -mod tests { - use clap::Parser; - - use super::*; - - #[derive(Parser)] - struct CommonOnlyArgs { - #[command(flatten)] - common: CommonClientArgs, - } - - fn common(args: &[&str]) -> CommonClientArgs { - let mut argv = vec!["common-only"]; - argv.extend_from_slice(args); - CommonOnlyArgs::try_parse_from(argv).unwrap().common - } - - fn unlabeled(addr: &str) -> TargetArg { - TargetArg::new(addr) - } - - fn labeled(label: &str, addr: &str) -> TargetArg { - TargetArg::new(format!("{label}={addr}")) - } - - fn prepare( - targets: &[TargetArg], - pacing: GroupPacingArg, - duration: Duration, - ) -> Result { - prepare_managed_run( - &common(&[]), - duration, - TargetSelection { - targets, - pacing, - stdin_controlled: false, - }, - ) - } - - #[test] - fn a_single_target_prepares_one_managed_target_and_a_finite_duration() { - let setup = prepare( - &[unlabeled("127.0.0.1:2112")], - GroupPacingArg::Staggered, - Duration::from_secs(30), - ) - .unwrap(); - - assert_eq!(setup.target_count(), 1); - assert!(!setup.is_multi_target()); - assert_eq!(setup.targets[0].label, "127.0.0.1:2112"); - assert_eq!(setup.targets[0].managed.server_addr, "127.0.0.1:2112"); - assert_eq!(setup.client.duration, Some(Duration::from_secs(30))); - assert_eq!(setup.pacing, ManagedPacing::Staggered); - } - - #[test] - fn a_zero_duration_prepares_a_continuous_client_config() { - let setup = prepare( - &[unlabeled("127.0.0.1:2112")], - GroupPacingArg::Staggered, - Duration::ZERO, - ) - .unwrap(); - - assert_eq!(setup.client.duration, None); - } - - #[test] - fn multiple_targets_keep_argument_order_and_share_one_client_config() { - let setup = prepare( - &[ - unlabeled("host-a:2112"), - unlabeled("host-b:2112"), - labeled("sg", "sg.example.test:2112"), - ], - GroupPacingArg::Burst, - Duration::from_secs(10), - ) - .unwrap(); - - assert!(setup.is_multi_target()); - let labels: Vec<_> = setup - .targets - .iter() - .map(|target| target.label.as_str()) - .collect(); - assert_eq!(labels, ["host-a:2112", "host-b:2112", "sg"]); - assert_eq!(setup.targets[2].managed.server_addr, "sg.example.test:2112"); - assert_eq!(setup.pacing, ManagedPacing::Burst); - - let config = setup.managed_config(); - assert_eq!(config.outcome_history_limit, 3); - assert_eq!(config.max_live_target_generations, 3); - assert_eq!(config.event_capacity, MANAGED_EVENT_CAPACITY); - assert_eq!(config.pacing, ManagedPacing::Burst); - assert_eq!(config.client, setup.client); - assert_eq!(setup.managed_targets().len(), 3); - } - - #[test] - fn the_shared_client_config_carries_no_target_of_its_own() { - // The managed driver replaces the address, and the CLI's key, per - // target, so the template must not pin either to one target. - let targets = vec![unlabeled("host-a:2112"), unlabeled("host-b:2112")]; - let setup = prepare_managed_run( - &common(&["--hmac", "secret"]), - Duration::from_secs(10), - TargetSelection { - targets: &targets, - pacing: GroupPacingArg::Staggered, - stdin_controlled: false, - }, - ) - .unwrap(); - - assert_eq!( - setup.client.server_addr, - ClientConfig::default().server_addr - ); - assert_eq!(setup.client.hmac_key, Some(b"secret".to_vec())); - // Per-target authentication is not exposed on the command line, so no - // target carries an override of the shared key. - assert!(setup - .targets - .iter() - .all(|target| target.managed.auth.is_none())); - } - - #[test] - fn an_empty_target_set_is_rejected_with_the_list_columns_hint() { - let err = prepare(&[], GroupPacingArg::Staggered, Duration::from_secs(10)).unwrap_err(); - - assert!(err.contains("at least one target is required")); - assert!(err.contains("--list-columns")); - } - - #[test] - fn a_duplicate_label_is_rejected_before_any_configuration_is_built() { - let targets = vec![ - unlabeled("host-a:2112"), - labeled("host-a:2112", "host-b:2112"), - ]; - let err = - prepare(&targets, GroupPacingArg::Staggered, Duration::from_secs(10)).unwrap_err(); - - assert!(err.contains("duplicate target label")); - } -} diff --git a/crates/irtt-client/src/client/tests/config.rs b/crates/irtt-client/src/client/tests/config.rs index f706c29..8ea5adc 100644 --- a/crates/irtt-client/src/client/tests/config.rs +++ b/crates/irtt-client/src/client/tests/config.rs @@ -1,74 +1,7 @@ use super::*; #[test] -fn params_from_config_maps_compatibility_fields() { - let config = ClientConfig { - duration: Some(Duration::from_secs(5)), - interval: Duration::from_millis(250), - length: 1472, - received_stats: ReceivedStats::Window, - stamp_at: StampAt::Midpoint, - clock: Clock::Wall, - dscp: 46, - hmac_key: Some(b"secret".to_vec()), - server_fill: Some("rand".to_owned()), - ..ClientConfig::default() - }; - - let params = params_from_config(&config).unwrap(); - assert_eq!(params.protocol_version, PROTOCOL_VERSION); - assert_eq!(params.duration_ns, 5_000_000_000); - assert_eq!(params.interval_ns, 250_000_000); - assert_eq!(params.length, 1472); - assert_eq!(params.received_stats, ReceivedStats::Window); - assert_eq!(params.stamp_at, StampAt::Midpoint); - assert_eq!(params.clock, Clock::Wall); - assert_eq!( - params.dscp, 184, - "config DSCP codepoint 46 must be shifted into raw wire byte 184" - ); - assert_eq!( - params.server_fill.as_ref().map(|fill| fill.value.as_str()), - Some("rand") - ); - assert_eq!(config.hmac_key.as_deref(), Some(b"secret".as_slice())); -} - -#[test] -fn params_from_config_accepts_boundary_values() { - for length in [0, 1, 1472, 4096, MAX_UDP_PAYLOAD_LENGTH] { - let config = ClientConfig { - length, - ..ClientConfig::default() - }; - assert_eq!( - params_from_config(&config).unwrap().length, - i64::from(length) - ); - } - - let config = ClientConfig { - dscp: 63, - ..ClientConfig::default() - }; - assert_eq!( - params_from_config(&config).unwrap().dscp, - 252, - "boundary codepoint 63 must be shifted into raw wire byte 252" - ); -} - -#[test] -fn params_from_config_encodes_continuous_duration_as_zero() { - let config = ClientConfig { - duration: None, - ..ClientConfig::default() - }; - assert_eq!(params_from_config(&config).unwrap().duration_ns, 0); -} - -#[test] -fn params_from_config_rejects_invalid_values() { +fn connect_rejects_invalid_configuration() { let i64_max_ns = u64::try_from(i64::MAX).unwrap(); let too_large = Duration::from_nanos(i64_max_ns) + Duration::from_nanos(1); let cases = [ @@ -151,10 +84,10 @@ fn params_from_config_rejects_invalid_values() { for (name, config, expected_reason) in cases { assert!( matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { reason }) if reason.contains(expected_reason) + Client::connect(config), + Err(ClientError::InvalidConfig { .. }) ), - "{name} should fail with InvalidConfig containing {expected_reason:?}" + "{name} should fail with InvalidConfig ({expected_reason})" ); } } diff --git a/crates/irtt-client/src/managed/tests.rs b/crates/irtt-client/src/managed/tests.rs index 3326bf1..3127a84 100644 --- a/crates/irtt-client/src/managed/tests.rs +++ b/crates/irtt-client/src/managed/tests.rs @@ -1,6 +1,5 @@ use std::{ future::{poll_fn, Future}, - mem, net::{SocketAddr, UdpSocket}, pin::Pin, sync::{Arc, Condvar, Mutex}, @@ -16,7 +15,7 @@ use irtt_proto::{ use tokio::runtime::{Builder, Runtime}; use super::*; -use crate::{ClientAuthConfig, NegotiationPolicy, RunMode}; +use crate::{ClientAuthConfig, RunMode}; const TOKEN: u64 = 0x1234_5678_90ab_cdef; @@ -766,52 +765,6 @@ fn terminal_targets_count_toward_retained_generation_limit() { assert!(task.snapshot().targets.is_empty()); } -#[test] -fn pruning_leaves_no_stale_target_work() { - let (mut task, _) = ManagedClient::task( - config(ManagedPacing::Staggered), - vec![ - ManagedTargetConfig::new("one", "127.0.0.1:9"), - ManagedTargetConfig::new("two", "127.0.0.1:10"), - ManagedTargetConfig::new("three", "127.0.0.1:11"), - ], - ) - .unwrap(); - task.cursor = 2; - task.timeout_cursor = 2; - task.send_cursor = 2; - task.scan_remaining = 3; - task.burst_remaining = 3; - task.stagger_remaining = 3; - - task.targets[2].desired = false; - task.targets[2].state = TargetState::Terminal; - task.prune_undesired_terminal(); - - // Timeout discovery indexes `targets` with `timeout_cursor` directly, so a - // prune must leave it safe to use; the target and send passes modulo their - // own cursors. Pending per-pass work must not outlive the targets it was - // scheduled for either. - assert_eq!(task.targets.len(), 2); - assert!(task.timeout_cursor < task.targets.len()); - assert!(task.scan_remaining <= task.targets.len()); - assert!(task.burst_remaining <= task.targets.len()); - assert!(task.stagger_remaining <= task.targets.len()); - assert!(!task.poll_timeout_pass(Instant::now())); - - for target in &mut task.targets { - target.desired = false; - target.state = TargetState::Terminal; - } - task.prune_undesired_terminal(); - - assert!(task.targets.is_empty()); - assert_eq!(task.scan_remaining, 0); - assert_eq!(task.burst_remaining, 0); - assert_eq!(task.stagger_remaining, 0); - assert!(!task.poll_timeout_pass(Instant::now())); -} - #[test] fn dynamic_rejections_leave_transaction_state_unchanged() { let mut managed = config(ManagedPacing::Staggered); @@ -1562,79 +1515,6 @@ fn staggered_pacing_fairness() { second.finish(); } -#[test] -fn stagger_gate_tracks_active_membership() { - let first = - start_server_negotiating(ServerBehavior::Echo, None, Some(Duration::from_millis(100))); - let second_gate = Arc::new(PacketGate::default()); - let second = start_server_with_gates( - ServerBehavior::Echo, - None, - Some(Duration::from_secs(1)), - Some(Arc::clone(&second_gate)), - None, - ); - let mut managed = config(ManagedPacing::Staggered); - managed.client.duration = Some(Duration::from_millis(1_250)); - managed.client.interval = Duration::from_secs(1); - managed.client.negotiation_policy = NegotiationPolicy::Loose; - managed.client.open_timeouts = vec![Duration::from_secs(2)]; - let (task, handle) = ManagedClient::task( - managed, - vec![target("first", first.addr), target("second", second.addr)], - ) - .unwrap(); - let runtime = runtime(); - let mut task = Box::pin(task); - drive_task_until(&runtime, &mut task, |task| { - task.targets[0].counters.packets_sent == 1 - }); - let one_target_last = task.last_stagger_send.unwrap(); - let one_target_gate = task.send_gate.unwrap(); - assert_eq!( - task.send_gate, - one_target_last.checked_add(Duration::from_millis(100)) - ); - second_gate.release(); - drive_task_until(&runtime, &mut task, |task| task.active_count() == 2); - let last = task.last_stagger_send.unwrap(); - let gate = task.send_gate.unwrap(); - assert_eq!(gate, last.checked_add(Duration::from_millis(50)).unwrap()); - assert!(gate <= one_target_gate); - assert!(gate > last && gate < last.checked_add(Duration::from_millis(100)).unwrap()); - drive_task_until(&runtime, &mut task, |task| { - task.targets[1].counters.packets_sent == 1 - }); - - let accepted_at = Instant::now(); - task.record_stagger_acceptance( - SendResult::Failed { accepted: true }, - Some(Duration::from_millis(50)), - accepted_at, - ); - assert_eq!( - task.send_gate, - accepted_at.checked_add(Duration::from_millis(50)) - ); - let preserved_gate = task.send_gate; - let TargetState::Active { client } = - mem::replace(&mut task.targets[0].state, TargetState::Terminal) - else { - panic!("fast target was not active before removal"); - }; - assert!(task.begin_drain(0, client, ManagedTargetEndReason::Stopped, Instant::now())); - assert_eq!(task.send_gate, preserved_gate); - drive_task_until(&runtime, &mut task, |task| { - task.targets[1].counters.packets_sent == 2 - }); - - drop(handle.stop()); - let outcome = runtime.block_on(task); - assert_eq!(outcome.failed_target_outcomes, 0); - first.finish(); - second.finish(); -} - /// The stagger gate must not fold each send's observed wakeup latency into /// every later slot. /// diff --git a/crates/irtt-client/src/session/machine.rs b/crates/irtt-client/src/session/machine.rs index 45d252d..ea90460 100644 --- a/crates/irtt-client/src/session/machine.rs +++ b/crates/irtt-client/src/session/machine.rs @@ -1177,10 +1177,6 @@ mod tests { } } - fn connected_machine(config: ClientConfig) -> SessionMachine { - SessionMachine::new(config, "127.0.0.1:2112".parse().unwrap()).unwrap() - } - fn normal_open_reply(machine: &SessionMachine, token: u64) -> OpenReply { OpenReply { flags: flags::FLAG_OPEN | flags::FLAG_REPLY, @@ -1189,210 +1185,6 @@ mod tests { } } - fn encoded_open_reply( - machine: &SessionMachine, - reply: &OpenReply, - ) -> Result, irtt_proto::ProtoError> { - irtt_proto::encode_open_reply(reply, machine.config.hmac_key.as_deref()) - } - - #[test] - fn malformed_wrong_direction_and_invalid_flags_are_ignored() { - let machine = connected_machine(ClientConfig::default()); - assert!(matches!( - machine.inspect_open_datagram(&[0_u8]), - Ok(OpenDatagramDisposition::Ignore) - )); - - let request = machine.prepare_open_request().unwrap(); - assert!(matches!( - machine.inspect_open_datagram(&request.bytes), - Ok(OpenDatagramDisposition::Ignore) - )); - - let reply = normal_open_reply(&machine, 0x1020_3040_5060_7080); - let mut reserved = encoded_open_reply(&machine, &reply).unwrap(); - reserved[3] |= 0x10; - assert!(matches!( - machine.inspect_open_datagram(&reserved), - Ok(OpenDatagramDisposition::Ignore) - )); - assert!(matches!(machine.state, MachineState::Connected)); - } - - #[test] - fn missing_unexpected_and_bad_hmac_are_ignored() { - let key = b"open-key".to_vec(); - let authenticated = connected_machine(ClientConfig { - hmac_key: Some(key.clone()), - ..ClientConfig::default() - }); - let reply = normal_open_reply(&authenticated, 0x1020_3040_5060_7080); - let plain = irtt_proto::encode_open_reply(&reply, None).unwrap(); - assert!(matches!( - authenticated.inspect_open_datagram(&plain), - Ok(OpenDatagramDisposition::Ignore) - )); - - let mut bad = encoded_open_reply(&authenticated, &reply).unwrap(); - *bad.last_mut().unwrap() ^= 0x80; - assert!(matches!( - authenticated.inspect_open_datagram(&bad), - Ok(OpenDatagramDisposition::Ignore) - )); - - let plain_machine = connected_machine(ClientConfig::default()); - let unexpected = irtt_proto::encode_open_reply(&reply, Some(&key)).unwrap(); - assert!(matches!( - plain_machine.inspect_open_datagram(&unexpected), - Ok(OpenDatagramDisposition::Ignore) - )); - } - - #[test] - fn authenticated_zero_token_is_trusted_and_terminal() { - let key = b"open-key".to_vec(); - let machine = connected_machine(ClientConfig { - hmac_key: Some(key.clone()), - ..ClientConfig::default() - }); - let reply = normal_open_reply(&machine, 0x1020_3040_5060_7080); - let mut packet = encoded_open_reply(&machine, &reply).unwrap(); - let token_offset = 4 + irtt_proto::HMAC_SIZE; - packet[token_offset..token_offset + 8].fill(0); - irtt_proto::compute_hmac_in_place(&key, &mut packet, 4).unwrap(); - - assert!(matches!( - machine.inspect_open_datagram(&packet), - Err(ClientError::Protocol(irtt_proto::ProtoError::ZeroToken)) - )); - assert!(matches!(machine.state, MachineState::Connected)); - } - - #[test] - fn trusted_invalid_parameter_encoding_is_terminal() { - let machine = connected_machine(ClientConfig::default()); - let mut packet = irtt_proto::MAGIC.to_vec(); - packet.push(flags::FLAG_OPEN | flags::FLAG_REPLY); - packet.extend_from_slice(&0x1020_3040_5060_7080_u64.to_le_bytes()); - packet.push(0x80); - - assert!(matches!( - machine.inspect_open_datagram(&packet), - Err(ClientError::Protocol( - irtt_proto::ProtoError::TruncatedVarint - )) - )); - assert!(matches!(machine.state, MachineState::Connected)); - } - - #[test] - fn trusted_rejection_version_and_run_mode_failures_do_not_open() { - let machine = connected_machine(ClientConfig::default()); - let rejection = OpenReply { - flags: flags::FLAG_OPEN | flags::FLAG_REPLY | flags::FLAG_CLOSE, - token: 0, - params: machine.requested.clone(), - }; - assert!(matches!( - machine - .prepare_open_acceptance(rejection, timestamp(Instant::now())) - .unwrap_err() - .primary, - ClientError::ServerRejected - )); - - let mut version = normal_open_reply(&machine, 0x1020_3040_5060_7080); - version.params.protocol_version += 1; - assert!(matches!( - machine - .prepare_open_acceptance(version, timestamp(Instant::now())) - .unwrap_err() - .primary, - ClientError::ProtocolVersionMismatch { .. } - )); - - let no_test = connected_machine(ClientConfig { - run_mode: RunMode::NoTest, - ..ClientConfig::default() - }); - assert!(matches!( - no_test - .prepare_open_acceptance( - normal_open_reply(&no_test, 0x1020_3040_5060_7080), - timestamp(Instant::now()), - ) - .unwrap_err() - .primary, - ClientError::UnexpectedNoTestReply - )); - assert!(matches!(machine.state, MachineState::Connected)); - assert!(matches!(no_test.state, MachineState::Connected)); - } - - #[test] - fn no_test_non_close_reply_prepares_authenticated_cleanup_without_state_change() { - let key = b"cleanup-key".to_vec(); - let machine = connected_machine(ClientConfig { - run_mode: RunMode::NoTest, - hmac_key: Some(key.clone()), - ..ClientConfig::default() - }); - let token = 0x1020_3040_5060_7080; - - let failure = machine - .prepare_open_acceptance( - normal_open_reply(&machine, token), - timestamp(Instant::now()), - ) - .unwrap_err(); - - assert!(matches!( - failure.primary, - ClientError::UnexpectedNoTestReply - )); - let cleanup = failure.cleanup_close.as_deref().unwrap(); - assert_eq!(cleanup[3], flags::FLAG_CLOSE | flags::FLAG_HMAC); - irtt_proto::verify_hmac(&key, cleanup, 4).unwrap(); - assert_eq!( - u64::from_le_bytes( - cleanup[4 + irtt_proto::HMAC_SIZE..12 + irtt_proto::HMAC_SIZE] - .try_into() - .unwrap() - ), - token - ); - drop(failure); - assert!(matches!(machine.state, MachineState::Connected)); - } - - #[test] - fn no_test_close_replies_do_not_prepare_cleanup() { - let machine = connected_machine(ClientConfig { - run_mode: RunMode::NoTest, - ..ClientConfig::default() - }); - let reply = |token| OpenReply { - flags: flags::FLAG_OPEN | flags::FLAG_REPLY | flags::FLAG_CLOSE, - token, - params: machine.requested.clone(), - }; - - let prepared = machine - .prepare_open_acceptance(reply(0), timestamp(Instant::now())) - .unwrap(); - assert!(prepared.cleanup_close_packet().is_none()); - - let failure = machine - .prepare_open_acceptance(reply(0x1020_3040_5060_7080), timestamp(Instant::now())) - .unwrap_err(); - assert!(matches!( - failure.primary, - ClientError::NonZeroNoTestToken { .. } - )); - assert!(failure.cleanup_close.is_none()); - } - #[test] fn uncommitted_probe_preparation_changes_no_authoritative_state() { let mut machine = open_machine(4, Duration::from_secs(1)); @@ -1417,30 +1209,6 @@ mod tests { assert_eq!(session.pending.len(), 0); } - #[test] - fn stale_prepared_probe_is_rejected_without_changing_state() { - let mut machine = open_machine(4, Duration::from_secs(1)); - let stale = machine.prepare_probe().unwrap(); - let accepted = machine.prepare_probe().unwrap(); - let sent_at = timestamp(Instant::now()); - let preflight = machine.preflight_probe_commit(&accepted).unwrap(); - let commit = machine.finalize_probe_commit(preflight, sent_at).unwrap(); - machine.commit_probe_sent(commit, sent_at, accepted.bytes.len()); - - assert!(matches!( - machine.preflight_probe_commit(&stale), - Err(ClientError::StalePreparedProbe { - prepared_seq: 0, - next_wire_seq: 1, - }) - )); - - let session = active(&machine); - assert_eq!(session.next_wire_seq, 1); - assert_eq!(session.packets_sent, 1); - assert_eq!(session.pending.len(), 1); - } - #[test] fn repeated_would_block_style_preflight_commits_once() { let mut machine = open_machine(4, Duration::from_secs(1)); @@ -1462,23 +1230,6 @@ mod tests { assert_eq!(session.pending.len(), 1); } - #[test] - fn pending_capacity_exhaustion_is_detected_before_commit() { - let mut machine = open_machine(1, Duration::from_secs(1)); - let first = machine.prepare_probe().unwrap(); - let sent_at = timestamp(Instant::now()); - let preflight = machine.preflight_probe_commit(&first).unwrap(); - let commit = machine.finalize_probe_commit(preflight, sent_at).unwrap(); - machine.commit_probe_sent(commit, sent_at, first.bytes.len()); - - let second = machine.prepare_probe().unwrap(); - assert!(matches!( - machine.preflight_probe_commit(&second), - Err(ClientError::PendingLimitExceeded { limit: 1 }) - )); - assert_eq!(active(&machine).packets_sent, 1); - } - #[test] fn pending_sequence_collision_is_detected_before_commit() { let mut machine = open_machine(2, Duration::from_secs(1)); @@ -1525,35 +1276,6 @@ mod tests { assert_eq!(active(&machine).pending.len(), 0); } - #[test] - fn exhaustive_timeout_polling_returns_every_due_loss() { - let mut machine = open_machine(4, Duration::from_secs(1)); - let now = Instant::now(); - for seq in [2, 1, 0] { - let sent_at = timestamp(now - Duration::from_secs(u64::from(seq) + 1)); - let session = active_mut(&mut machine); - session.pending.preflight_insert(seq).unwrap(); - session.pending.commit_insert(PendingProbe { - wire_seq: seq, - sent_at, - timeout_at: sent_at.mono + Duration::from_secs(1), - tx_not_before_wall: sent_at.wall, - kernel_tx_timestamp: None, - }); - } - - let events = machine.poll_timeouts_at(now).unwrap(); - assert!(matches!( - events.as_slice(), - [ - ClientEvent::EchoLoss { seq: 2, .. }, - ClientEvent::EchoLoss { seq: 1, .. }, - ClientEvent::EchoLoss { seq: 0, .. }, - ] - )); - assert_eq!(active(&machine).pending.len(), 0); - } - #[test] fn wrapping_sequence_from_max_to_zero_remains_valid() { let mut machine = open_machine(2, Duration::from_secs(1)); diff --git a/crates/irtt-client/src/session/negotiation.rs b/crates/irtt-client/src/session/negotiation.rs index 0aa5e78..00a48af 100644 --- a/crates/irtt-client/src/session/negotiation.rs +++ b/crates/irtt-client/src/session/negotiation.rs @@ -396,73 +396,6 @@ mod tests { } } - #[test] - fn strict_rejects_changed_negotiated_fields() { - let requested = default_params(); - - let mut returned = requested.clone(); - returned.length = 128; - assert_eq!( - rejection_reason(&requested, &returned, NegotiationPolicy::Strict), - NegotiationRestriction::LengthReduced { - requested: requested.length, - negotiated: returned.length, - } - .message() - ); - - let mut returned = requested.clone(); - returned.dscp = 0; - assert_eq!( - rejection_reason(&requested, &returned, NegotiationPolicy::Strict), - NegotiationRestriction::DscpChanged { - requested: requested.dscp >> 2, - negotiated: returned.dscp >> 2, - } - .message() - ); - - let mut returned = requested.clone(); - returned.received_stats = ReceivedStats::Count; - assert_eq!( - rejection_reason(&requested, &returned, NegotiationPolicy::Strict), - NegotiationRestriction::ReceivedStatsChanged { - requested: requested.received_stats, - negotiated: returned.received_stats, - } - .message() - ); - - let mut returned = requested.clone(); - returned.stamp_at = StampAt::Midpoint; - assert_eq!( - rejection_reason(&requested, &returned, NegotiationPolicy::Strict), - NegotiationRestriction::StampAtChanged { - requested: requested.stamp_at, - negotiated: returned.stamp_at, - } - .message() - ); - - let mut returned = requested.clone(); - returned.clock = Clock::Wall; - assert_eq!( - rejection_reason(&requested, &returned, NegotiationPolicy::Strict), - NegotiationRestriction::ClockChanged { - requested: requested.clock, - negotiated: returned.clock, - } - .message() - ); - - let mut returned = requested.clone(); - returned.server_fill = None; - assert_eq!( - rejection_reason(&requested, &returned, NegotiationPolicy::Strict), - NegotiationRestriction::ServerFillChanged.message() - ); - } - #[test] fn loose_duration_negotiation_uses_run_duration_semantics() { let requested = default_params(); diff --git a/crates/irtt-client/src/socket.rs b/crates/irtt-client/src/socket.rs index 1f32c29..249b1c7 100644 --- a/crates/irtt-client/src/socket.rs +++ b/crates/irtt-client/src/socket.rs @@ -174,25 +174,6 @@ mod tests { assert_eq!(normalize_server_addr("[::1]:1234"), "[::1]:1234"); } - #[test] - fn address_family_filtering_remains_unchanged() { - let ipv4 = SocketAddr::from(([127, 0, 0, 1], DEFAULT_PORT)); - let ipv6 = SocketAddr::from(([0, 0, 0, 0, 0, 0, 0, 1], DEFAULT_PORT)); - let mut config = ClientConfig::default(); - - assert!(address_family_allowed(&config, ipv4)); - assert!(address_family_allowed(&config, ipv6)); - - config.socket_config.ipv4_only = true; - assert!(address_family_allowed(&config, ipv4)); - assert!(!address_family_allowed(&config, ipv6)); - - config.socket_config.ipv4_only = false; - config.socket_config.ipv6_only = true; - assert!(!address_family_allowed(&config, ipv4)); - assert!(address_family_allowed(&config, ipv6)); - } - #[cfg(feature = "tokio")] #[test] fn tokio_ipv6_only_is_applied_before_bind() { diff --git a/crates/irtt-server/src/fill.rs b/crates/irtt-server/src/fill.rs index 0103894..b5c9ab8 100644 --- a/crates/irtt-server/src/fill.rs +++ b/crates/irtt-server/src/fill.rs @@ -228,75 +228,6 @@ pub(crate) fn echo_payload_len(hmac: bool, params: &Params) -> usize { mod tests { use super::*; - #[test] - fn recognized_descriptors_parse_to_their_mode() { - for (descriptor, expected) in [ - ("none", FillMode::None), - ("rand", FillMode::Random), - ("pattern:00", FillMode::Pattern(vec![0x00])), - ("pattern:ff00", FillMode::Pattern(vec![0xff, 0x00])), - // Only the hexadecimal body is case-insensitive. - ("pattern:AaBb", FillMode::Pattern(vec![0xaa, 0xbb])), - ( - "pattern:69727474", - FillMode::Pattern(DEFAULT_FILL_PATTERN.to_vec()), - ), - ] { - assert_eq!(FillMode::parse(descriptor), Some(expected), "{descriptor}"); - } - } - - #[test] - fn unrecognized_and_malformed_descriptors_parse_to_nothing() { - for descriptor in [ - "", - "bogus", - // The mode names themselves are case-sensitive. - "RAND", - "None", - "Pattern:aabb", - "rand ", - "pattern:", - "pattern:f", - "pattern:abc", - "pattern:zz", - "pattern:0g", - // A multi-byte character is not two hexadecimal digits, whatever - // its byte length. - "pattern:é", - ] { - assert_eq!(FillMode::parse(descriptor), None, "{descriptor:?}"); - } - } - - #[test] - fn the_default_descriptor_and_the_default_pattern_agree() { - // The two constants are held separately so nothing has to parse a - // string per open; this is what keeps them from drifting. - assert_eq!( - FillMode::parse(DEFAULT_FILL_DESCRIPTOR), - Some(FillMode::default_fill()) - ); - } - - #[test] - fn a_pattern_repeats_from_its_first_byte_and_stops_at_the_region() { - let mode = FillMode::Pattern(vec![0xaa, 0xbb]); - assert_eq!(mode.payload(0), Vec::::new()); - assert_eq!(mode.payload(1), vec![0xaa]); - assert_eq!( - mode.payload(7), - vec![0xaa, 0xbb, 0xaa, 0xbb, 0xaa, 0xbb, 0xaa] - ); - } - - #[test] - fn no_fill_leaves_the_region_to_the_encoder() { - // Not a length-sized run of zeroes: the encoder has already zeroed the - // region, and copying zeroes over zeroes would only cost an allocation. - assert!(FillMode::None.payload(16).is_empty()); - } - #[test] fn a_failed_random_draw_yields_zeroes() { // The real source cannot be made to fail without a production hook, and @@ -309,25 +240,4 @@ mod tests { }; assert_eq!(random_payload(6, scribble), vec![0; 6]); } - - #[test] - fn a_successful_random_draw_is_propagated_whole() { - // Deterministic on purpose. Nothing here asserts that real random bytes - // are nonzero, distinct or well distributed; those are properties of - // the operating system's source, and testing them would be a coin flip - // dressed up as an assertion. - let drawn = [1, 2, 3, 4, 5]; - let source = |buffer: &mut [u8]| -> Result<(), ()> { - assert_eq!(buffer.len(), drawn.len(), "the whole region is offered"); - buffer.copy_from_slice(&drawn); - Ok(()) - }; - assert_eq!(random_payload(drawn.len(), source), drawn); - } - - #[test] - fn an_empty_region_never_reaches_the_random_source() { - let source = |_: &mut [u8]| -> Result<(), ()> { panic!("no bytes were needed") }; - assert!(random_payload(0, source).is_empty()); - } } diff --git a/crates/irtt-server/src/set.rs b/crates/irtt-server/src/set.rs index e16677b..a8cc0f3 100644 --- a/crates/irtt-server/src/set.rs +++ b/crates/irtt-server/src/set.rs @@ -393,83 +393,6 @@ mod tests { use super::*; use crate::{token::TokenSource, ServerCore, ServerError}; - /// Which listener of a requested set has to be bound IPv6-only. - /// - /// The interesting rows are the mapped ones. `[::ffff:0.0.0.0]` is an IPv4 - /// wildcard bind wearing an IPv6 address, so it is neither a candidate for - /// `IPV6_V6ONLY` — which would refuse the traffic it exists to serve — nor - /// the IPv6 half of a same-port pair. A genuine IPv6 address paired with it - /// still is. - #[test] - fn only_a_genuine_ipv6_listener_sharing_an_explicit_port_is_bound_ipv6_only() { - for (addr, requested, ipv6_only, why) in [ - ( - "[::]:2112", - vec!["0.0.0.0:2112", "[::]:2112"], - true, - "the wildcard pair is the reason this exists", - ), - ( - "[::1]:2112", - vec!["127.0.0.1:2112", "[::1]:2112"], - true, - "and so is the explicit loopback pair", - ), - ( - "0.0.0.0:2112", - vec!["0.0.0.0:2112", "[::]:2112"], - false, - "the IPv4 half is bound exactly as it always was", - ), - ( - "[::]:2112", - vec!["[::]:2112"], - false, - "an IPv6 listener alone keeps ordinary bind semantics", - ), - ( - "[::]:2112", - vec!["0.0.0.0:2113", "[::]:2112"], - false, - "different ports cannot collide", - ), - ( - "[::]:0", - vec!["0.0.0.0:0", "[::]:0"], - false, - "two ephemeral requests resolve independently", - ), - ( - "[::ffff:0.0.0.0]:2112", - vec!["0.0.0.0:2112", "[::ffff:0.0.0.0]:2112"], - false, - "a mapped wildcard is an IPv4 listener, not an IPv6 one", - ), - ( - "[::]:2112", - vec!["[::ffff:0.0.0.0]:2112", "[::]:2112"], - true, - "and it is the IPv4 half a genuine IPv6 listener must avoid", - ), - ( - "[::ffff:127.0.0.1]:2112", - vec!["[::ffff:127.0.0.1]:2112", "[::1]:2112"], - false, - "a mapped host address is explicit IPv4 too", - ), - ] { - let requested: Vec = requested - .iter() - .map(|addr| addr.parse().expect("a test address must parse")) - .collect(); - assert_eq!( - needs_ipv6_only(addr.parse().unwrap(), &requested), - ipv6_only, - "{addr} among {requested:?}: {why}" - ); - } - } - #[tokio::test(flavor = "current_thread")] async fn a_set_serving_nothing_is_refused() { assert!(matches!( diff --git a/crates/irtt-server/src/tests/fill.rs b/crates/irtt-server/src/tests/fill.rs index e062557..6c49944 100644 --- a/crates/irtt-server/src/tests/fill.rs +++ b/crates/irtt-server/src/tests/fill.rs @@ -1,12 +1,9 @@ //! Server fill: what an open reply reports, what the session will fill with, //! and the bytes an echo reply actually carries. //! -//! The first two are deliberately different questions. A client that expressed -//! no preference keeps its absent or empty descriptor in the negotiated -//! parameters while the session uses this server's default fill, and only a -//! descriptor the server could not honor is rewritten. Asserting the reply alone -//! would not separate those, so the negotiation tests read the session's -//! effective mode too, and the payload tests then pin the bytes on the wire. +//! A client that expressed no preference keeps its absent or empty descriptor +//! in the negotiated parameters. Payload tests assert the effective fill from +//! reply bytes, without inspecting the session's internal representation. //! //! Payload lengths here are derived from `irtt-proto`'s own layout rather than //! written as offsets, and are deliberately not multiples of the pattern length. @@ -18,7 +15,6 @@ use super::support::{ expect_no_test_reply, expect_normal_open_reply, no_test_request, open_negotiated, open_request, other_peer, peer, unthrottled, ScriptedTokens, KEY, }; -use crate::{core::ServerCore, fill::FillMode}; const TOKEN_A: u64 = 0x0102_0304_0506_0708; const TOKEN_B: u64 = 0x1112_1314_1516_1718; @@ -38,8 +34,8 @@ fn requesting(descriptor: Option<&str>) -> Params { } /// Opens a session requesting `descriptor` and returns the descriptor the reply -/// carried together with the fill the session actually holds. -fn negotiated(descriptor: Option<&str>) -> (Option, FillMode) { +/// carried. +fn negotiated(descriptor: Option<&str>) -> Option { let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); let packet = core .handle_datagram(peer(), &open_request(&requesting(descriptor), None)) @@ -47,16 +43,7 @@ fn negotiated(descriptor: Option<&str>) -> (Option, FillMode) { .expect("the open must be answered"); let reply = expect_normal_open_reply(&packet, None); - let fill = effective_fill(&core, reply.token); - (reply.params.server_fill.map(|fill| fill.value), fill) -} - -/// The effective fill of the session `token` names. -fn effective_fill(core: &ServerCore, token: u64) -> FillMode { - core.session(token) - .expect("the open must have created a session") - .fill() - .clone() + reply.params.server_fill.map(|fill| fill.value) } /// The descriptor a no-test open reports for `descriptor`, which creates no @@ -75,17 +62,6 @@ fn no_test_negotiated(descriptor: Option<&str>) -> Option { reply.params.server_fill.map(|fill| fill.value) } -#[test] -fn no_preference_keeps_an_absent_descriptor_and_uses_the_default_fill() { - // The important half is the first: a client that asked for nothing must be - // answered with nothing, or a strict client rejects a restriction the - // server never actually imposed. The server's own default is an internal - // choice and stays internal. - let (returned, fill) = negotiated(None); - assert_eq!(returned, None, "an absent request stays absent"); - assert_eq!(fill, FillMode::default_fill()); -} - #[test] fn an_explicitly_empty_descriptor_is_preserved_and_uses_the_default_fill() { // A low-level peer can send this even though `ClientConfig` rejects an @@ -93,31 +69,12 @@ fn an_explicitly_empty_descriptor_is_preserved_and_uses_the_default_fill() { // with absent as "no preference", so it is neither refused nor rewritten: // the requested wire value comes back exactly, and the server picks its own // behavior behind it. - let (returned, fill) = negotiated(Some("")); + let returned = negotiated(Some("")); assert_eq!( returned.as_deref(), Some(""), "an empty request stays empty" ); - assert_eq!(fill, FillMode::default_fill()); -} - -#[test] -fn a_valid_descriptor_is_returned_exactly_as_requested() { - for (descriptor, expected) in [ - ("none", FillMode::None), - ("rand", FillMode::Random), - ("pattern:00", FillMode::Pattern(vec![0x00])), - ("pattern:ff00", FillMode::Pattern(vec![0xff, 0x00])), - // Returned with its own hexadecimal case, not normalized to the bytes - // it decodes to: rewriting it would report a restriction that did not - // happen, and a strict client would reject the session for it. - ("pattern:AaBb", FillMode::Pattern(vec![0xaa, 0xbb])), - ] { - let (returned, fill) = negotiated(Some(descriptor)); - assert_eq!(returned.as_deref(), Some(descriptor), "{descriptor}"); - assert_eq!(fill, expected, "{descriptor}"); - } } #[test] @@ -137,13 +94,12 @@ fn an_unknown_or_malformed_descriptor_is_replaced_by_the_default() { "pattern:0g", "0123456789abcdef0123456789abcdef", ] { - let (returned, fill) = negotiated(Some(descriptor)); + let returned = negotiated(Some(descriptor)); assert_eq!( returned.as_deref(), Some(crate::fill::DEFAULT_FILL_DESCRIPTOR), "{descriptor}" ); - assert_eq!(fill, FillMode::default_fill(), "{descriptor}"); } } @@ -229,8 +185,8 @@ fn repeated(pattern: &[u8], len: usize) -> Vec { #[test] fn no_preference_fills_with_the_default_pattern() { - // The wire consequence of the negotiation test above: absent descriptor, - // default bytes. 7 is not a multiple of 4, so the last repeat is partial. + // An absent descriptor uses default bytes. 7 is not a multiple of 4, so + // the last repeat is partial. assert_eq!(echo_payload(None, 7), b"irttirt".to_vec()); } diff --git a/crates/irtt-stats/src/ipdv.rs b/crates/irtt-stats/src/ipdv.rs index 1254ed2..66683f7 100644 --- a/crates/irtt-stats/src/ipdv.rs +++ b/crates/irtt-stats/src/ipdv.rs @@ -204,15 +204,6 @@ mod tests { ); } - #[test] - fn ipdv_tracker_duplicate_sequence_does_not_emit_pair_again() { - let mut tracker = IpdvTracker::new(None); - assert!(tracker.insert(ipdv_sample(0, 10)).is_empty()); - assert_eq!(tracker.insert(ipdv_sample(1, 14)).len(), 1); - - assert!(tracker.insert(ipdv_sample(1, 18)).is_empty()); - } - #[test] fn ipdv_tracker_bounded_mode_limits_sequence_state() { let limit = 4; diff --git a/crates/irtt-stats/src/loss.rs b/crates/irtt-stats/src/loss.rs index 579aeaa..5740dd3 100644 --- a/crates/irtt-stats/src/loss.rs +++ b/crates/irtt-stats/src/loss.rs @@ -91,21 +91,3 @@ pub(crate) fn loss_stats(packets: PacketCounts) -> LossStats { fn percent(numerator: f64, denominator: f64) -> f64 { 100.0 * numerator / denominator } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn duplicate_and_late_percentages_use_packets_received_denominator() { - let loss = loss_stats(PacketCounts { - packets_received: 4, - duplicates: 1, - late_packets: 2, - ..PacketCounts::default() - }); - - assert_eq!(loss.duplicate_percent, 25.0); - assert_eq!(loss.late_packets_percent, 50.0); - } -} diff --git a/crates/irtt-stats/src/retention.rs b/crates/irtt-stats/src/retention.rs index f58ad21..cfeb8df 100644 --- a/crates/irtt-stats/src/retention.rs +++ b/crates/irtt-stats/src/retention.rs @@ -34,11 +34,6 @@ const HASH_ENTRY_OVERHEAD: u64 = 1; /// Adjusted RTT, one-way delay, send/receive IPDV, and server processing only /// receive a sample when the negotiated session supplies the corresponding /// optional measurement, so counting all eleven is the upper bound. -/// -/// This count must track the metrics `CoreStats::new` builds with exact -/// retention. `exact_mode_retains_one_sample_per_counted_metric` below counts -/// the metrics a fully measured probe actually retains, so the two cannot -/// drift apart unnoticed. pub(crate) const EXACT_SAMPLES_PER_PROBE: u64 = 11; /// Normalized events a probe usually contributes to a rolling window: one send @@ -104,150 +99,3 @@ fn rolling_count_bytes(config: &StatsConfig, probe_count: u64) -> u64 { fn rolling_bytes_per_event() -> u64 { (size_of::() + size_of::()) as u64 } - -#[cfg(test)] -mod tests { - use std::time::{Duration, Instant, UNIX_EPOCH}; - - use irtt_client::{ - ClientEvent, ClientTimestamp, OneWayDelaySample, PacketMeta, ReceivedStatsSample, - RttSample, ServerTiming, SignedDuration, - }; - - use super::*; - use crate::{Snapshot, StatsCollector, TimeStats}; - - /// Every public timing metric a snapshot exposes. - fn timing_metrics(snapshot: &Snapshot) -> Vec<&TimeStats> { - vec![ - &snapshot.send_call, - &snapshot.timer_error, - &snapshot.rtt.primary, - &snapshot.rtt.raw, - &snapshot.rtt.adjusted, - &snapshot.ipdv.round_trip, - &snapshot.ipdv.send, - &snapshot.ipdv.receive, - &snapshot.one_way_delay.send_delay, - &snapshot.one_way_delay.receive_delay, - &snapshot.server_processing.processing, - ] - } - - fn timestamp(offset_ms: u64) -> ClientTimestamp { - ClientTimestamp { - mono: Instant::now() + Duration::from_millis(offset_ms), - wall: UNIX_EPOCH + Duration::from_millis(offset_ms), - } - } - - fn sent(seq: u32) -> ClientEvent { - let sent_at = timestamp(u64::from(seq) * 10); - ClientEvent::EchoSent { - seq, - remote: "127.0.0.1:2112".parse().unwrap(), - scheduled_at: Some(sent_at.mono), - sent_at, - bytes: 32, - send_call: Duration::from_micros(10 + u64::from(seq)), - timer_error: Some(Duration::from_micros(2 + u64::from(seq))), - } - } - - /// A reply carrying every optional measurement, so one probe exercises the - /// upper bound the estimate assumes. - fn fully_measured_reply(seq: u32) -> ClientEvent { - let sent_at = timestamp(u64::from(seq) * 10); - let raw = Duration::from_millis(10 + u64::from(seq)); - let received_at = ClientTimestamp { - mono: sent_at.mono + raw, - wall: sent_at.wall + raw, - }; - let base_ns = i64::from(seq) * 10_000_000; - ClientEvent::EchoReply { - seq, - remote: "127.0.0.1:2112".parse().unwrap(), - sent_at, - received_at, - rtt: RttSample { - raw, - adjusted: Some(SignedDuration::from_duration(raw)), - effective: SignedDuration::from_duration(raw), - }, - server_timing: Some(ServerTiming { - receive_mono_ns: Some(base_ns + 1_000_000), - send_mono_ns: Some(base_ns + 2_000_000), - receive_wall_ns: Some(base_ns + 1_000_000), - send_wall_ns: Some(base_ns + 2_000_000), - midpoint_mono_ns: None, - midpoint_wall_ns: None, - processing: Some(Duration::from_millis(1 + u64::from(seq))), - }), - one_way: Some(OneWayDelaySample { - client_to_server: Some(SignedDuration::from_nanos(1_000_000 + i128::from(seq))), - server_to_client: Some(SignedDuration::from_nanos(2_000_000 + i128::from(seq))), - }), - received_stats: Some(ReceivedStatsSample { - count: Some(seq + 1), - window: Some(0xff), - }), - bytes: 64, - packet_meta: PacketMeta::default(), - } - } - - /// Two probes fill every metric, including the IPDV pair metrics that need - /// adjacent replies. - fn fully_measured_snapshot(config: StatsConfig) -> Snapshot { - let mut collector = StatsCollector::new(config); - for seq in 0..2 { - collector.process(&sent(seq)); - collector.process(&fully_measured_reply(seq)); - } - collector.snapshot() - } - - /// The estimate is only honest while it counts the metrics that actually - /// retain samples. Adding or removing an exact-retaining public timing - /// metric without revisiting [`EXACT_SAMPLES_PER_PROBE`] fails here. - #[test] - fn exact_mode_retains_one_sample_per_counted_metric() { - let snapshot = fully_measured_snapshot(StatsConfig::finite()); - let with_medians = timing_metrics(&snapshot) - .into_iter() - .filter(|stats| stats.median_ns.is_some()) - .count() as u64; - - assert_eq!( - with_medians, EXACT_SAMPLES_PER_PROBE, - "the estimate counts {EXACT_SAMPLES_PER_PROBE} exact sample streams per probe, \ - but {with_medians} public timing metrics retained samples" - ); - } - - #[test] - fn running_only_retains_no_exact_samples() { - let snapshot = fully_measured_snapshot(StatsConfig::continuous()); - - for stats in timing_metrics(&snapshot) { - assert!(stats.count > 0, "the fixture should populate every metric"); - assert_eq!( - stats.median_ns, None, - "running-only mode retains no exact samples" - ); - } - } - - #[test] - fn per_sample_components_are_non_zero() { - assert!(ipdv_tracker_bytes_per_sample() > 0); - assert!(rolling_bytes_per_event() > 0); - } - - #[test] - fn running_only_stops_growing_at_the_bounded_sequence_limit() { - let limit = CONTINUOUS_SEQUENCE_LIMIT as u64; - assert_eq!(running_only_bytes(limit), running_only_bytes(limit * 100)); - assert!(running_only_bytes(limit - 1) < running_only_bytes(limit)); - } -} diff --git a/crates/irtt-stats/tests/retention.rs b/crates/irtt-stats/tests/retention.rs index b25d95d..fd21abe 100644 --- a/crates/irtt-stats/tests/retention.rs +++ b/crates/irtt-stats/tests/retention.rs @@ -27,33 +27,6 @@ fn exact_mode_grows_with_probe_count() { } } -#[test] -fn exact_mode_scales_linearly_with_probe_count() { - let config = StatsConfig::finite(); - let single = config.estimated_retained_bytes(1); - - assert_eq!(config.estimated_retained_bytes(1_000), single * 1_000); -} - -/// Exact mode retains eleven `i128` timing streams per probe alongside its -/// IPDV tracker state, so the timing samples alone are at least that much. -/// -/// This checks the estimate against the storage it claims to model. It is an -/// implementation test, not a stable public constant: the figure is free to -/// move whenever the retention model does. -#[test] -fn exact_estimate_covers_every_retained_timing_sample() { - const EXACT_TIMING_STREAMS: u64 = 11; - let per_probe = StatsConfig::finite().estimated_retained_bytes(1); - let timing_sample_bytes = EXACT_TIMING_STREAMS * std::mem::size_of::() as u64; - - assert!( - per_probe > timing_sample_bytes, - "the estimate should cover {EXACT_TIMING_STREAMS} retained timing samples plus \ - IPDV tracker state, but a probe estimated only {per_probe} bytes" - ); -} - #[test] fn enormous_probe_counts_saturate_instead_of_wrapping() { for config in [StatsConfig::finite(), StatsConfig::continuous()] { @@ -90,27 +63,6 @@ fn running_only_does_not_report_unbounded_per_probe_retention() { ); } -#[test] -fn finite_config_estimate_is_deterministic_and_usable_for_planning() { - let config = StatsConfig::finite(); - let probes = 1_000_000; - let first = config.estimated_retained_bytes(probes); - - assert_eq!(first, config.estimated_retained_bytes(probes)); - - // A million probes should land in a range a caller can reason about: well - // over a megabyte, and well under a terabyte. The exact figure is free to - // move with the retention model. - assert!( - first > 1024 * 1024, - "a million probes should estimate more than a MiB, got {first}" - ); - assert!( - first < 1024 * 1024 * 1024 * 1024, - "a million probes should estimate less than a TiB, got {first}" - ); -} - #[test] fn rolling_count_adds_bounded_retention() { let probes = 1_000_000;