From d8b580a19c267f763888c345d427f6ab44640a5f Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Sun, 10 May 2026 23:47:46 +0700 Subject: [PATCH] refactor(client): remove redundant timestamp parameter from close method --- crates/irtt-cli/src/main.rs | 2 +- crates/irtt-client/src/client.rs | 4 +- crates/irtt-client/src/client/tests/close.rs | 15 ++-- crates/irtt-client/src/client/tests/dscp.rs | 2 +- crates/irtt-client/src/client/tests/hmac.rs | 2 +- .../irtt-client/src/client/tests/no_test.rs | 10 +-- crates/irtt-client/src/client/tests/open.rs | 20 +---- .../src/client/tests/probes_replies.rs | 74 +++++++++---------- crates/irtt-client/src/client/tests/ttl.rs | 2 +- crates/irtt-client/src/managed/runner.rs | 2 +- crates/irtt-client/tests/hmac.rs | 24 ++---- crates/irtt-client/tests/large_packets.rs | 2 +- crates/irtt-client/tests/protocol_options.rs | 12 +-- 13 files changed, 69 insertions(+), 102 deletions(-) diff --git a/crates/irtt-cli/src/main.rs b/crates/irtt-cli/src/main.rs index e23eb0d..fc015e4 100644 --- a/crates/irtt-cli/src/main.rs +++ b/crates/irtt-cli/src/main.rs @@ -108,7 +108,7 @@ fn run(args: CliArgs, shutdown_requested: &AtomicBool) -> Result<(), Box Result, ClientError> { + pub fn close(&mut self) -> Result, ClientError> { let token = match self.phase { ClientPhase::Open { token } => token, ClientPhase::Closed => return Err(ClientError::AlreadyClosed), @@ -149,7 +149,7 @@ impl Client { Ok(vec![ClientEvent::SessionClosed { remote: self.remote, token, - at: now, + at: ClientTimestamp::now(), }]) } diff --git a/crates/irtt-client/src/client/tests/close.rs b/crates/irtt-client/src/client/tests/close.rs index bedd7d9..072dcc4 100644 --- a/crates/irtt-client/src/client/tests/close.rs +++ b/crates/irtt-client/src/client/tests/close.rs @@ -12,11 +12,8 @@ fn open_fails_after_close() { }); let mut client = Client::connect(default_test_config(server.addr)).unwrap(); assert_open_started(client.open().unwrap()); - client.close(ClientTimestamp::now()).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::AlreadyClosed) - )); + client.close().unwrap(); + assert!(matches!(client.open(), Err(ClientError::AlreadyClosed))); server.join(); } @@ -32,7 +29,7 @@ fn close_sends_one_close_packet_with_negotiated_token() { }); let mut client = Client::connect(default_test_config(server.addr)).unwrap(); assert_open_started(client.open().unwrap()); - let events = client.close(ClientTimestamp::now()).unwrap(); + let events = client.close().unwrap(); assert_eq!(events.len(), 1); let packets: Vec<_> = server.rx.iter().take(2).collect(); let close = &packets[1]; @@ -63,7 +60,7 @@ fn send_probe_fails_after_close() { }); let mut client = Client::connect(default_test_config(server.addr)).unwrap(); assert_open_started(client.open().unwrap()); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); assert!(matches!( client.send_probe(), Err(ClientError::AlreadyClosed) @@ -91,7 +88,7 @@ fn close_clears_timed_out_metadata() { client.poll_timeouts(ClientTimestamp::now()).unwrap(); assert_eq!(client.session.as_ref().unwrap().timed_out.len(), 1); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); assert!(client.session.is_none()); server.join(); } @@ -170,6 +167,6 @@ fn normal_echo_reply_does_not_close_session() { assert!(client.next_send_deadline().is_some()); assert!(client.send_probe().is_ok()); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } diff --git a/crates/irtt-client/src/client/tests/dscp.rs b/crates/irtt-client/src/client/tests/dscp.rs index f1bff80..2848bea 100644 --- a/crates/irtt-client/src/client/tests/dscp.rs +++ b/crates/irtt-client/src/client/tests/dscp.rs @@ -32,7 +32,7 @@ fn normal_open_applies_negotiated_dscp_after_open_and_close_clears_it() { 184 ); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); assert_eq!( socket_traffic_class(&client.socket, client.remote).unwrap(), 0 diff --git a/crates/irtt-client/src/client/tests/hmac.rs b/crates/irtt-client/src/client/tests/hmac.rs index 38b274f..b2d3361 100644 --- a/crates/irtt-client/src/client/tests/hmac.rs +++ b/crates/irtt-client/src/client/tests/hmac.rs @@ -77,7 +77,7 @@ fn hmac_close_packet_includes_valid_hmac() { config.server_addr = server.addr.to_string(); let mut client = Client::connect(config).unwrap(); assert_open_started(client.open().unwrap()); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); let packets: Vec<_> = server.rx.iter().take(2).collect(); let close = &packets[1]; assert_eq!(close[3], flags::FLAG_CLOSE | FLAG_HMAC); diff --git a/crates/irtt-client/src/client/tests/no_test.rs b/crates/irtt-client/src/client/tests/no_test.rs index 90a3db1..aa1b8bd 100644 --- a/crates/irtt-client/src/client/tests/no_test.rs +++ b/crates/irtt-client/src/client/tests/no_test.rs @@ -11,10 +11,7 @@ fn no_test_open_close_succeeds_on_open_reply_close() { 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(ClientTimestamp::now()), - Err(ClientError::NotOpen) - )); + assert!(matches!(client.close(), Err(ClientError::NotOpen))); server.join(); } @@ -117,9 +114,6 @@ fn open_fails_after_no_test_completed() { config.server_addr = server.addr.to_string(); let mut client = Client::connect(config).unwrap(); assert_no_test_completed(client.open().unwrap()); - assert!(matches!( - client.open(), - Err(ClientError::AlreadyCompleted) - )); + assert!(matches!(client.open(), Err(ClientError::AlreadyCompleted))); server.join(); } diff --git a/crates/irtt-client/src/client/tests/open.rs b/crates/irtt-client/src/client/tests/open.rs index cc0a0d9..f988603 100644 --- a/crates/irtt-client/src/client/tests/open.rs +++ b/crates/irtt-client/src/client/tests/open.rs @@ -31,10 +31,7 @@ fn open_fails_when_already_open() { let server = open_success_server(params); let mut client = Client::connect(default_test_config(server.addr)).unwrap(); assert_open_started(client.open().unwrap()); - assert!(matches!( - client.open(), - Err(ClientError::AlreadyOpen) - )); + assert!(matches!(client.open(), Err(ClientError::AlreadyOpen))); server.join(); } @@ -67,10 +64,7 @@ fn open_timeout_after_all_timeouts() { ..default_test_config(server.addr) }; let mut client = Client::connect(config).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::OpenTimeout) - )); + assert!(matches!(client.open(), Err(ClientError::OpenTimeout))); assert_eq!(server.rx.iter().take(2).count(), 2); server.join(); } @@ -87,10 +81,7 @@ fn open_restores_configured_read_timeout_after_timeout() { ..default_test_config(server.addr) }; let mut client = Client::connect(config).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::OpenTimeout) - )); + assert!(matches!(client.open(), Err(ClientError::OpenTimeout))); let start = std::time::Instant::now(); let mut buf = [0_u8; 1]; @@ -124,9 +115,6 @@ fn server_rejection_fails_in_normal_mode() { socket.send_to(&reply, peer).unwrap(); }); let mut client = Client::connect(default_test_config(server.addr)).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::ServerRejected) - )); + assert!(matches!(client.open(), Err(ClientError::ServerRejected))); server.join(); } diff --git a/crates/irtt-client/src/client/tests/probes_replies.rs b/crates/irtt-client/src/client/tests/probes_replies.rs index 4e90d83..6de2716 100644 --- a/crates/irtt-client/src/client/tests/probes_replies.rs +++ b/crates/irtt-client/src/client/tests/probes_replies.rs @@ -69,7 +69,7 @@ fn send_probe_sends_valid_echo_request() { assert_eq!(req_token, TOKEN); let seq = u32::from_le_bytes(echo_req[12..16].try_into().unwrap()); assert_eq!(seq, 0); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -115,7 +115,7 @@ fn echo_sent_reports_schedule_and_timer_error() { other => panic!("expected EchoSent, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -146,7 +146,7 @@ fn send_probe_starts_seq_at_zero_and_increments() { let seq = u32::from_le_bytes(pkt[12..16].try_into().unwrap()); assert_eq!(seq, i as u32); } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -199,7 +199,7 @@ fn send_probe_respects_finite_duration_exclusive_end() { assert!(client.session.as_ref().unwrap().sending_done); assert!(client.next_send_deadline().is_none()); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -240,7 +240,7 @@ fn continuous_duration_keeps_generating_send_deadlines() { assert!(!client.session.as_ref().unwrap().sending_done); } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -262,7 +262,7 @@ fn next_send_deadline_advances_by_interval() { let deadline2 = client.next_send_deadline().unwrap(); assert_eq!(deadline2, start + Duration::from_secs(2)); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -431,7 +431,7 @@ fn recv_once_decodes_echo_reply_and_emits_event() { } other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -520,7 +520,7 @@ fn echo_reply_metadata_is_unavailable_without_ancillary() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -563,7 +563,7 @@ fn echo_reply_metadata_propagates_observed_dscp_with_ancillary() { metadata_unavailable_skip( "echo_reply_metadata_propagates_observed_dscp_with_ancillary", ); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); return; }; @@ -574,7 +574,7 @@ fn echo_reply_metadata_propagates_observed_dscp_with_ancillary() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -595,7 +595,7 @@ fn echo_reply_metadata_preserves_observed_zero_with_ancillary() { metadata_unavailable_skip( "echo_reply_metadata_preserves_observed_zero_with_ancillary", ); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); return; }; @@ -606,7 +606,7 @@ fn echo_reply_metadata_preserves_observed_zero_with_ancillary() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -627,7 +627,7 @@ fn echo_reply_metadata_propagates_kernel_rx_timestamp_with_ancillary() { kernel_rx_timestamp_unavailable_skip( "echo_reply_metadata_propagates_kernel_rx_timestamp_with_ancillary", ); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); return; }; @@ -636,7 +636,7 @@ fn echo_reply_metadata_propagates_kernel_rx_timestamp_with_ancillary() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -654,7 +654,7 @@ fn echo_reply_rtt_uses_client_monotonic() { } else { panic!("expected EchoReply"); } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -679,7 +679,7 @@ fn server_processing_subtracted_when_valid() { } else { panic!("expected EchoReply"); } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -724,7 +724,7 @@ fn received_stats_parsed_into_sample() { } else { panic!("expected EchoReply"); } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1014,7 +1014,7 @@ fn late_reply_after_timeout_preserves_measurement_metadata() { } if *bytes == echo_packet_len(false, &default_params()) )); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1062,7 +1062,7 @@ fn late_reply_metadata_is_unavailable_without_ancillary() { other => panic!("expected LateReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1110,7 +1110,7 @@ fn late_reply_metadata_propagates_observed_dscp_with_ancillary() { metadata_unavailable_skip( "late_reply_metadata_propagates_observed_dscp_with_ancillary", ); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); return; }; @@ -1128,7 +1128,7 @@ fn late_reply_metadata_propagates_observed_dscp_with_ancillary() { other => panic!("expected LateReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1157,7 +1157,7 @@ fn pending_map_bounded() { client.send_probe(), Err(ClientError::PendingLimitExceeded { limit: 3 }) )); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1326,7 +1326,7 @@ fn pending_full_does_not_send_packet() { "no packet should be sent when pending is full" ); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1550,7 +1550,7 @@ fn matched_reply_with_reversed_monotonic_time_still_emits_event() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1596,7 +1596,7 @@ fn process_received_packet_uses_supplied_receive_metadata() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1647,7 +1647,7 @@ fn process_received_packet_uses_supplied_receive_metadata_for_late_reply() { other => panic!("expected LateReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1685,7 +1685,7 @@ fn receive_metadata_does_not_broaden_malformed_warning() { other => panic!("expected Warning, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1713,7 +1713,7 @@ fn recv_available_drains_burst_replies() { )); } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1738,7 +1738,7 @@ fn recv_available_respects_packet_budget() { assert_eq!(second.len(), 1); assert!(matches!(&second[0], ClientEvent::EchoReply { seq: 1, .. })); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1772,7 +1772,7 @@ fn send_probe_wraps_wire_sequence_at_u32_max() { .collect(); assert_eq!(seqs, vec![u32::MAX, 0]); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1863,7 +1863,7 @@ fn send_probe_after_sending_done_is_noop() { .count(); assert_eq!(echo_count, 0); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1934,7 +1934,7 @@ fn recv_buffer_uses_negotiated_packet_length() { ); assert_eq!(buf_size, 4097); assert_eq!(client.recv_buffer.len(), buf_size); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -1970,7 +1970,7 @@ fn short_echo_reply_does_not_emit_echo_reply() { .. }] )); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -2002,7 +2002,7 @@ fn overlong_echo_reply_does_not_emit_echo_reply() { .. }] )); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -2051,7 +2051,7 @@ fn overlong_datagram_detection_uses_extra_receive_byte() { .. }] )); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -2091,7 +2091,7 @@ fn exact_length_echo_reply_still_emits_echo_reply() { events.as_slice(), [ClientEvent::EchoReply { bytes, .. }] if *bytes == echo_packet_len(false, ¶ms) )); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -2113,6 +2113,6 @@ fn recv_once_at_test_helper_provides_deterministic_timestamp() { } else { panic!("expected EchoReply"); } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } diff --git a/crates/irtt-client/src/client/tests/ttl.rs b/crates/irtt-client/src/client/tests/ttl.rs index acb4652..19a806b 100644 --- a/crates/irtt-client/src/client/tests/ttl.rs +++ b/crates/irtt-client/src/client/tests/ttl.rs @@ -32,7 +32,7 @@ fn connect_applies_configured_ipv4_ttl_before_open_and_close_preserves_it() { client.open().unwrap(); server.join(); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); assert_eq!(socket_ttl(&client.socket, client.remote).unwrap(), 64); } diff --git a/crates/irtt-client/src/managed/runner.rs b/crates/irtt-client/src/managed/runner.rs index ceb8202..bc50650 100644 --- a/crates/irtt-client/src/managed/runner.rs +++ b/crates/irtt-client/src/managed/runner.rs @@ -223,7 +223,7 @@ fn run_client( } let packets_sent = client.packets_sent(); - let close_events = client.close(ClientTimestamp::now())?; + let close_events = client.close()?; publish_events(&hub, &mut counters, close_events); Ok(SessionOutcome { diff --git a/crates/irtt-client/tests/hmac.rs b/crates/irtt-client/tests/hmac.rs index 6edbe79..92980f7 100644 --- a/crates/irtt-client/tests/hmac.rs +++ b/crates/irtt-client/tests/hmac.rs @@ -101,7 +101,7 @@ fn hmac_close_success_sends_authenticated_close_and_closes_session() { let outcome = client.open().unwrap(); assert_started(outcome, ¶ms); - let events = client.close(ClientTimestamp::now()).unwrap(); + let events = client.close().unwrap(); assert!(matches!( events.as_slice(), [ClientEvent::SessionClosed { token: TOKEN, .. }] @@ -131,10 +131,7 @@ fn missing_client_key_against_hmac_required_server_times_out() { config.hmac_key = None; let mut client = Client::connect(config).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::OpenTimeout) - )); + assert!(matches!(client.open(), Err(ClientError::OpenTimeout))); let observations = server.observations(1); assert!(matches!( @@ -156,10 +153,7 @@ fn wrong_client_key_against_hmac_required_server_times_out() { config.hmac_key = Some(b"wrong-secret".to_vec()); let mut client = Client::connect(config).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::OpenTimeout) - )); + assert!(matches!(client.open(), Err(ClientError::OpenTimeout))); let observations = server.observations(1); assert!(matches!( @@ -314,7 +308,7 @@ fn backend_hmac_correct_key_succeeds() { assert_eq!(events.len(), 1); assert!(matches!(events[0], ClientEvent::EchoReply { .. })); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); } #[test] @@ -326,10 +320,7 @@ fn backend_hmac_wrong_key_fails() { config.hmac_key = Some(b"wrong-secret".to_vec()); let mut client = Client::connect(config).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::OpenTimeout) - )); + assert!(matches!(client.open(), Err(ClientError::OpenTimeout))); } #[test] @@ -341,8 +332,5 @@ fn backend_hmac_missing_key_fails() { config.hmac_key = None; let mut client = Client::connect(config).unwrap(); - assert!(matches!( - client.open(), - Err(ClientError::OpenTimeout) - )); + assert!(matches!(client.open(), Err(ClientError::OpenTimeout))); } diff --git a/crates/irtt-client/tests/large_packets.rs b/crates/irtt-client/tests/large_packets.rs index 39f8d1f..32e9517 100644 --- a/crates/irtt-client/tests/large_packets.rs +++ b/crates/irtt-client/tests/large_packets.rs @@ -136,5 +136,5 @@ fn backend_large_packet_smoke() { assert_eq!(events.len(), 1); assert!(matches!(events[0], ClientEvent::EchoReply { .. })); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); } diff --git a/crates/irtt-client/tests/protocol_options.rs b/crates/irtt-client/tests/protocol_options.rs index 44bac90..b2bad84 100644 --- a/crates/irtt-client/tests/protocol_options.rs +++ b/crates/irtt-client/tests/protocol_options.rs @@ -389,7 +389,7 @@ fn backend_basic_open_echo_close() { assert_eq!(events.len(), 1); assert!(matches!(events[0], ClientEvent::EchoReply { .. })); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); } #[test] @@ -410,7 +410,7 @@ fn backend_ttl_smoke() { assert_eq!(events.len(), 1); assert!(matches!(events[0], ClientEvent::EchoReply { .. })); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); } #[test] @@ -422,7 +422,7 @@ fn dscp_configured_open_close_smoke() { let outcome = client.open().unwrap(); assert!(matches!(outcome, irtt_client::OpenOutcome::Started { .. })); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); server.join(); } @@ -444,7 +444,7 @@ fn backend_received_stats_smoke() { assert_eq!(events.len(), 1); assert!(matches!(events[0], ClientEvent::EchoReply { .. })); - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); } } @@ -497,7 +497,7 @@ fn backend_timestamp_smoke() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); } } @@ -536,6 +536,6 @@ fn backend_clock_smoke() { other => panic!("expected EchoReply, got {other:?}"), } - client.close(ClientTimestamp::now()).unwrap(); + client.close().unwrap(); } }