From 00fd0ebb15077c8cdc1358c6563b1f796c36615d Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Sun, 17 May 2026 00:34:18 +0700 Subject: [PATCH] test: consolidate duplicate CLI and client config cases --- crates/irtt-cli/src/lib.rs | 55 ++---- crates/irtt-cli/src/main.rs | 108 +++++------ crates/irtt-client/src/client/tests/config.rs | 178 ++++++++---------- 3 files changed, 138 insertions(+), 203 deletions(-) diff --git a/crates/irtt-cli/src/lib.rs b/crates/irtt-cli/src/lib.rs index 962c46b..dfc9fd8 100644 --- a/crates/irtt-cli/src/lib.rs +++ b/crates/irtt-cli/src/lib.rs @@ -971,15 +971,12 @@ mod tests { } #[test] - fn duration_zero_selects_continuous_config() { + fn cli_options_map_to_client_config() { let args = parse(&["--duration", "0", "127.0.0.1:2112"]).unwrap(); assert!(args.is_continuous()); assert_eq!(args.duration, Duration::ZERO); assert_eq!(args.to_client_config().duration, None); - } - #[test] - fn parses_output_clock_and_timestamp_modes() { let args = parse(&[ "--output", "machine", @@ -994,6 +991,7 @@ mod tests { assert_eq!(args.clock, ClockArg::Wall); assert_eq!(args.timestamp_mode(), TimestampArg::Send); assert_eq!(args.to_client_config().stamp_at, StampAt::Send); + assert_eq!(args.to_client_config().clock, Clock::Wall); let args = parse(&[ "--output", @@ -1009,13 +1007,11 @@ mod tests { assert_eq!(args.clock, ClockArg::Monotonic); assert_eq!(args.timestamp_mode(), TimestampArg::Receive); assert_eq!(args.to_client_config().stamp_at, StampAt::Receive); + assert_eq!(args.to_client_config().clock, Clock::Monotonic); let args = parse(&["--output", "human", "127.0.0.1:2112"]).unwrap(); assert_eq!(args.output, OutputMode::Human); - } - #[test] - fn maps_tstamp_modes() { for (value, expected) in [ ("none", StampAt::None), ("send", StampAt::Send), @@ -1026,29 +1022,11 @@ mod tests { let args = parse(&["--tstamp", value, "127.0.0.1:2112"]).unwrap(); assert_eq!(args.to_client_config().stamp_at, expected); } - } - #[test] - fn timestamps_alias_accepts_midpoint() { let args = parse(&["--timestamps", "midpoint", "127.0.0.1:2112"]).unwrap(); assert_eq!(args.timestamp_mode(), TimestampArg::Midpoint); assert_eq!(args.to_client_config().stamp_at, StampAt::Midpoint); - } - #[test] - fn rejects_duplicate_timestamp_options() { - assert!(parse(&[ - "--tstamp", - "send", - "--timestamps", - "receive", - "127.0.0.1:2112" - ]) - .is_err()); - } - - #[test] - fn maps_received_stats_modes() { for (value, expected) in [ ("none", ReceivedStats::None), ("count", ReceivedStats::Count), @@ -1061,10 +1039,7 @@ mod tests { let args = parse(&["127.0.0.1:2112"]).unwrap(); assert_eq!(args.to_client_config().received_stats, ReceivedStats::Both); - } - #[test] - fn maps_sfill_server_fill_options() { let args = parse(&["--sfill", "abc", "127.0.0.1:2112"]).unwrap(); assert_eq!(args.to_client_config().server_fill.as_deref(), Some("abc")); @@ -1074,18 +1049,12 @@ mod tests { let max = "0123456789abcdef0123456789abcdef"; let args = parse(&["--sfill", max, "127.0.0.1:2112"]).unwrap(); assert_eq!(args.to_client_config().server_fill.as_deref(), Some(max)); - } - #[test] - fn maps_dscp_codepoints() { for value in ["0", "46", "63"] { let args = parse(&["--dscp", value, "127.0.0.1:2112"]).unwrap(); assert_eq!(args.to_client_config().dscp, value.parse::().unwrap()); } - } - #[test] - fn maps_ttl_values() { for value in ["1", "64", "255"] { let args = parse(&["--ttl", value, "127.0.0.1:2112"]).unwrap(); assert_eq!( @@ -1093,10 +1062,7 @@ mod tests { Some(value.parse::().unwrap()) ); } - } - #[test] - fn maps_length_option() { let args = parse(&["--length", "1472", "127.0.0.1:2112"]).unwrap(); assert_eq!(args.length, 1472); assert_eq!(args.to_client_config().length, 1472); @@ -1104,10 +1070,7 @@ mod tests { let args = parse(&["127.0.0.1:2112"]).unwrap(); assert_eq!(args.length, 0); assert_eq!(args.to_client_config().length, 0); - } - #[test] - fn maps_loose_negotiation() { let args = parse(&["127.0.0.1:2112"]).unwrap(); assert_eq!( args.to_client_config().negotiation_policy, @@ -1121,6 +1084,18 @@ mod tests { ); } + #[test] + fn rejects_duplicate_timestamp_options() { + assert!(parse(&[ + "--tstamp", + "send", + "--timestamps", + "receive", + "127.0.0.1:2112" + ]) + .is_err()); + } + #[test] fn help_lists_advanced_protocol_options() { let help = CliArgs::command().render_help().to_string(); diff --git a/crates/irtt-cli/src/main.rs b/crates/irtt-cli/src/main.rs index 7ce011d..d87f4df 100644 --- a/crates/irtt-cli/src/main.rs +++ b/crates/irtt-cli/src/main.rs @@ -599,75 +599,53 @@ mod tests { } #[test] - fn finite_stats_memory_warning_skips_continuous_mode() { - let args = cli_args(&["--duration", "0", "--interval", "1ms", "127.0.0.1:2112"]); - - assert_eq!(finite_stats_memory_warning(&args), None); + fn finite_stats_memory_warning_skips_non_warning_runs() { + for args in [ + cli_args(&["--duration", "0", "--interval", "1ms", "127.0.0.1:2112"]), + cli_args(&[ + "--duration", + "1000ms", + "--interval", + "1ms", + "127.0.0.1:2112", + ]), + ] { + assert_eq!(finite_stats_memory_warning(&args), None); + } } #[test] - fn finite_stats_memory_warning_skips_small_finite_run() { - let args = cli_args(&[ - "--duration", - "1000ms", - "--interval", - "1ms", - "127.0.0.1:2112", - ]); + fn finite_stats_memory_warning_reports_threshold_tiers() { + for (threshold, size, guidance) in [ + ( + FINITE_STATS_MEMORY_WARNING_BYTES, + "about 128 MiB", + "bounded-memory long-running tests", + ), + ( + FINITE_STATS_MEMORY_STRONG_WARNING_BYTES, + "about 512 MiB", + "shortening the run", + ), + ( + FINITE_STATS_MEMORY_VERY_STRONG_WARNING_BYTES, + "about 1 GiB", + "memory-constrained systems", + ), + ] { + let expected_probes = threshold.div_ceil(FINITE_STATS_BYTES_PER_PROBE); + let args = cli_args(&[ + "--duration", + &format!("{expected_probes}ms"), + "--interval", + "1ms", + "127.0.0.1:2112", + ]); - assert_eq!(finite_stats_memory_warning(&args), None); - } - - #[test] - fn finite_stats_memory_warning_reports_128_mib_tier() { - let expected_probes = - FINITE_STATS_MEMORY_WARNING_BYTES.div_ceil(FINITE_STATS_BYTES_PER_PROBE); - let args = cli_args(&[ - "--duration", - &format!("{expected_probes}ms"), - "--interval", - "1ms", - "127.0.0.1:2112", - ]); - - let warning = finite_stats_memory_warning(&args).unwrap(); - assert!(warning.contains("about 128 MiB")); - assert!(warning.contains("bounded-memory long-running tests")); - } - - #[test] - fn finite_stats_memory_warning_reports_512_mib_tier() { - let expected_probes = - FINITE_STATS_MEMORY_STRONG_WARNING_BYTES.div_ceil(FINITE_STATS_BYTES_PER_PROBE); - let args = cli_args(&[ - "--duration", - &format!("{expected_probes}ms"), - "--interval", - "1ms", - "127.0.0.1:2112", - ]); - - let warning = finite_stats_memory_warning(&args).unwrap(); - assert!(warning.contains("about 512 MiB")); - assert!(warning.contains("shortening the run")); - assert!(warning.contains("increasing the interval")); - } - - #[test] - fn finite_stats_memory_warning_reports_1_gib_tier() { - let expected_probes = - FINITE_STATS_MEMORY_VERY_STRONG_WARNING_BYTES.div_ceil(FINITE_STATS_BYTES_PER_PROBE); - let args = cli_args(&[ - "--duration", - &format!("{expected_probes}ms"), - "--interval", - "1ms", - "127.0.0.1:2112", - ]); - - let warning = finite_stats_memory_warning(&args).unwrap(); - assert!(warning.contains("about 1 GiB")); - assert!(warning.contains("memory-constrained systems")); + let warning = finite_stats_memory_warning(&args).unwrap(); + assert!(warning.contains(size)); + assert!(warning.contains(guidance)); + } } #[test] diff --git a/crates/irtt-client/src/client/tests/config.rs b/crates/irtt-client/src/client/tests/config.rs index d9c1991..b7f3f5b 100644 --- a/crates/irtt-client/src/client/tests/config.rs +++ b/crates/irtt-client/src/client/tests/config.rs @@ -51,7 +51,7 @@ fn params_from_config_maps_compatibility_fields() { } #[test] -fn params_from_config_accepts_udp_payload_length_boundary_values() { +fn params_from_config_accepts_boundary_values() { for length in [0, 1, 1472, 4096, MAX_UDP_PAYLOAD_LENGTH] { let config = ClientConfig { length, @@ -62,18 +62,12 @@ fn params_from_config_accepts_udp_payload_length_boundary_values() { i64::from(length) ); } -} -#[test] -fn params_from_config_rejects_oversized_udp_payload_length() { let config = ClientConfig { - length: MAX_UDP_PAYLOAD_LENGTH + 1, + dscp: 63, ..ClientConfig::default() }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { reason }) if reason.contains("packet length") - )); + assert_eq!(params_from_config(&config).unwrap().dscp, 63); } #[test] @@ -86,97 +80,85 @@ fn params_from_config_encodes_continuous_duration_as_zero() { } #[test] -fn params_from_config_rejects_zero_finite_duration() { - let config = ClientConfig { - duration: Some(Duration::ZERO), - ..ClientConfig::default() - }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { reason }) - if reason == "duration must be greater than zero; use None for continuous mode" - )); -} - -#[test] -fn params_from_config_rejects_zero_interval() { - let config = ClientConfig { - interval: Duration::ZERO, - ..ClientConfig::default() - }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { reason }) if reason == "interval must be greater than zero" - )); -} - -#[test] -fn params_from_config_rejects_nanosecond_encoding_overflow_as_invalid_config() { +fn params_from_config_rejects_invalid_values() { let i64_max_ns = u64::try_from(i64::MAX).unwrap(); - let config = ClientConfig { - duration: Some(Duration::from_nanos(i64_max_ns) + Duration::from_nanos(1)), - ..ClientConfig::default() - }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { reason }) - if reason == "duration is too large to encode as nanoseconds" - )); + let too_large = Duration::from_nanos(i64_max_ns) + Duration::from_nanos(1); + let cases = [ + ( + "oversized UDP payload", + ClientConfig { + length: MAX_UDP_PAYLOAD_LENGTH + 1, + ..ClientConfig::default() + }, + "packet length", + ), + ( + "zero finite duration", + ClientConfig { + duration: Some(Duration::ZERO), + ..ClientConfig::default() + }, + "duration must be greater than zero; use None for continuous mode", + ), + ( + "zero interval", + ClientConfig { + interval: Duration::ZERO, + ..ClientConfig::default() + }, + "interval must be greater than zero", + ), + ( + "duration nanosecond overflow", + ClientConfig { + duration: Some(too_large), + ..ClientConfig::default() + }, + "duration is too large to encode as nanoseconds", + ), + ( + "interval nanosecond overflow", + ClientConfig { + interval: too_large, + ..ClientConfig::default() + }, + "interval is too large to encode as nanoseconds", + ), + ( + "invalid DSCP codepoint", + ClientConfig { + dscp: 64, + ..ClientConfig::default() + }, + "dscp", + ), + ( + "empty server fill", + ClientConfig { + server_fill: Some("".to_owned()), + ..ClientConfig::default() + }, + "server_fill", + ), + ( + "oversized server fill", + ClientConfig { + server_fill: Some("0123456789abcdef0123456789abcdefx".to_owned()), + ..ClientConfig::default() + }, + "server_fill", + ), + ]; - let config = ClientConfig { - interval: Duration::from_nanos(i64_max_ns) + Duration::from_nanos(1), - ..ClientConfig::default() - }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { reason }) - if reason == "interval is too large to encode as nanoseconds" - )); -} - -#[test] -fn params_from_config_accepts_max_dscp_codepoint() { - let config = ClientConfig { - dscp: 63, - ..ClientConfig::default() - }; - assert_eq!(params_from_config(&config).unwrap().dscp, 63); -} - -#[test] -fn params_from_config_rejects_invalid_dscp_codepoint() { - let config = ClientConfig { - dscp: 64, - ..ClientConfig::default() - }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { .. }) - )); -} - -#[test] -fn params_from_config_rejects_empty_server_fill() { - let config = ClientConfig { - server_fill: Some("".to_owned()), - ..ClientConfig::default() - }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { .. }) - )); -} - -#[test] -fn params_from_config_rejects_oversized_server_fill() { - let config = ClientConfig { - server_fill: Some("0123456789abcdef0123456789abcdefx".to_owned()), - ..ClientConfig::default() - }; - assert!(matches!( - params_from_config(&config), - Err(ClientError::InvalidConfig { .. }) - )); + for (name, config, expected_reason) in cases { + assert!( + matches!( + params_from_config(&config), + Err(ClientError::InvalidConfig { reason }) if reason.contains(expected_reason) + ), + "{name} should fail with InvalidConfig containing {expected_reason:?}" + ); + } } #[test]