From 458bb24be15368ef7b910ca2b712471760ae708d Mon Sep 17 00:00:00 2001 From: Sijie Yang Date: Sat, 25 Nov 2023 10:47:50 +0800 Subject: [PATCH] Add more unit tests --- src/congestion_control/congestion_control.rs | 3 +- src/connection/cid.rs | 2 +- src/connection/connection.rs | 1 + src/connection/path.rs | 14 +--- src/connection/recovery.rs | 1 - src/connection/rtt.rs | 1 - src/connection/space.rs | 1 - src/endpoint.rs | 57 +++++++++++-- src/error.rs | 87 ++++++++++++++++++-- src/h3/error.rs | 47 ++++++++++- src/h3/h3.rs | 23 ++++++ src/h3/qpack/qpack.rs | 5 -- src/h3/qpack/static_table.rs | 39 ++++++++- src/timer_queue.rs | 4 - src/window.rs | 3 + 15 files changed, 248 insertions(+), 40 deletions(-) diff --git a/src/congestion_control/congestion_control.rs b/src/congestion_control/congestion_control.rs index b36095f69..91741b2ab 100644 --- a/src/congestion_control/congestion_control.rs +++ b/src/congestion_control/congestion_control.rs @@ -33,11 +33,12 @@ pub use hystart_plus_plus::HystartPlusPlus; /// Available congestion control algorithm #[repr(C)] -#[derive(Eq, PartialEq, Debug, Clone)] +#[derive(Eq, PartialEq, Debug, Clone, Copy, Default)] pub enum CongestionControlAlgorithm { /// CUBIC uses a cubic function instead of a linear window increase function /// of the current TCP standards to improve scalability and stability under /// fast and long-distance networks.. + #[default] Cubic, /// BBR uses recent measurements of a transport connection's delivery rate, diff --git a/src/connection/cid.rs b/src/connection/cid.rs index 02a2ec330..adca8e0d8 100644 --- a/src/connection/cid.rs +++ b/src/connection/cid.rs @@ -23,7 +23,7 @@ use crate::Result; const MAX_CIDS_COUNT: u64 = 16; /// Connection Id and related metadata. -#[derive(Debug, Default)] +#[derive(Default)] pub struct ConnectionIdItem { /// The Connection ID. pub cid: ConnectionId, diff --git a/src/connection/connection.rs b/src/connection/connection.rs index d7dcce126..d1cdd82b6 100644 --- a/src/connection/connection.rs +++ b/src/connection/connection.rs @@ -4208,6 +4208,7 @@ pub(crate) mod tests { conf.set_ack_delay_exponent(3); conf.set_max_ack_delay(25); conf.set_reset_token_key([1u8; 64]); + conf.set_address_token_lifetime(3600); conf.set_send_batch_size(2); conf.set_max_handshake_timeout(0); conf.set_multipath(false); diff --git a/src/connection/path.rs b/src/connection/path.rs index 50d319839..2958aa279 100644 --- a/src/connection/path.rs +++ b/src/connection/path.rs @@ -480,16 +480,6 @@ impl PathMap { } } - /// Process a PATH_ABANDON frame on the give path - pub(super) fn on_path_abandon(&mut self, path_id: usize, error_code: u64, reason: Vec) { - // TODO - } - - /// Process a PATH_STATUS frame on the give path - pub(super) fn on_path_status(&mut self, path_id: usize, seq_num: u64, status: u64) { - // TODO - } - /// Handle the sent event of PATH_CHALLENGE. pub fn on_path_chal_sent( &mut self, @@ -650,6 +640,10 @@ mod tests { assert_eq!(path_mgr.get_mut(pid)?.state, PathState::Validated); assert_eq!(path_mgr.get_mut(pid)?.sent_chals.len(), 0); + // Fake receiving of depulicated PATH_RESPONSE + path_mgr.on_path_resp_received(pid, data); + assert_eq!(path_mgr.get_mut(pid)?.validated(), true); + // Timeout event path_mgr.on_path_chal_timeout(now + time::Duration::from_millis(INITIAL_CHAL_TIMEOUT)); assert_eq!(path_mgr.get_mut(pid)?.lost_chal, 0); diff --git a/src/connection/recovery.rs b/src/connection/recovery.rs index f5edd3257..85e7cdb15 100644 --- a/src/connection/recovery.rs +++ b/src/connection/recovery.rs @@ -46,7 +46,6 @@ const MAX_PTO_PROBES_COUNT: usize = 2; /// An implementation of the loss detection mechanisms described in /// RFC 9002 Section 6 and Appendix A. -#[derive(Debug)] pub struct Recovery { /// The maximum amount of time by which the receiver intends to delay /// acknowledgments for packets in the Application Data packet number space. diff --git a/src/connection/rtt.rs b/src/connection/rtt.rs index daab3fede..8ceb17660 100644 --- a/src/connection/rtt.rs +++ b/src/connection/rtt.rs @@ -19,7 +19,6 @@ use crate::TIMER_GRANULARITY; /// RTT estimation for a network path /// See RFC 9001 Section 5 -#[derive(Copy, Clone, Debug)] pub struct RttEstimator { /// The most recent RTT sample. latest_rtt: Duration, diff --git a/src/connection/space.rs b/src/connection/space.rs index ce8207f2c..c5200f58f 100644 --- a/src/connection/space.rs +++ b/src/connection/space.rs @@ -343,7 +343,6 @@ impl std::fmt::Debug for SentPacket { } /// Metadata of acknowledged packet -#[derive(Clone)] pub struct AckedPacket { /// The packet number of the sent packet. pub pkt_num: u64, diff --git a/src/endpoint.rs b/src/endpoint.rs index 1782ac48a..50aeaa169 100644 --- a/src/endpoint.rs +++ b/src/endpoint.rs @@ -951,6 +951,7 @@ mod tests { use super::*; use crate::connection; use crate::Config; + use crate::CongestionControlAlgorithm; use crate::Error; use crate::TlsConfig; use bytes::Buf; @@ -1050,11 +1051,13 @@ mod tests { } /// Run client/server endpoint with default config. - fn run_with_test_config(&mut self, hdr_opt: CaseConf) -> Result<()> { - let cli_conf = TestPair::new_test_config(false)?; - let srv_conf = TestPair::new_test_config(true)?; + fn run_with_test_config(&mut self, case_conf: CaseConf) -> Result<()> { + let mut cli_conf = TestPair::new_test_config(false)?; + cli_conf.set_congestion_control_algorithm(case_conf.cc_algor); + let mut srv_conf = TestPair::new_test_config(true)?; + srv_conf.set_congestion_control_algorithm(case_conf.cc_algor); - self.run(cli_conf, srv_conf, hdr_opt) + self.run(cli_conf, srv_conf, case_conf) } /// Run event loop for the endpoint. @@ -1461,6 +1464,7 @@ mod tests { token: Option>, request_num: u32, request_size: usize, + cc_algor: CongestionControlAlgorithm, packet_loss: u32, packet_delay: u32, packet_reorder: u32, @@ -2244,13 +2248,56 @@ mod tests { } #[test] - fn transfer_single_stream_with_packet_loss() -> Result<()> { + fn transfer_single_stream_cubic_with_packet_loss() -> Result<()> { let mut t = TestPair::new(); let mut case_conf = CaseConf::default(); case_conf.request_num = 1; case_conf.request_size = 1024 * 16; case_conf.packet_loss = 1; + case_conf.cc_algor = CongestionControlAlgorithm::Cubic; + + t.run_with_test_config(case_conf)?; + Ok(()) + } + + #[test] + fn transfer_single_stream_bbr_with_packet_loss() -> Result<()> { + let mut t = TestPair::new(); + + let mut case_conf = CaseConf::default(); + case_conf.request_num = 1; + case_conf.request_size = 1024 * 16; + case_conf.packet_loss = 1; + case_conf.cc_algor = CongestionControlAlgorithm::Bbr; + + t.run_with_test_config(case_conf)?; + Ok(()) + } + + #[test] + fn transfer_single_stream_bbr3_with_packet_loss() -> Result<()> { + let mut t = TestPair::new(); + + let mut case_conf = CaseConf::default(); + case_conf.request_num = 1; + case_conf.request_size = 1024 * 16; + case_conf.packet_loss = 1; + case_conf.cc_algor = CongestionControlAlgorithm::Bbr3; + + t.run_with_test_config(case_conf)?; + Ok(()) + } + + #[test] + fn transfer_single_stream_copa_with_packet_loss() -> Result<()> { + let mut t = TestPair::new(); + + let mut case_conf = CaseConf::default(); + case_conf.request_num = 1; + case_conf.request_size = 1024 * 16; + case_conf.packet_loss = 1; + case_conf.cc_algor = CongestionControlAlgorithm::Copa; t.run_with_test_config(case_conf)?; Ok(()) diff --git a/src/error.rs b/src/error.rs index 80e3ef846..e9ea6d695 100644 --- a/src/error.rs +++ b/src/error.rs @@ -16,12 +16,16 @@ use crate::frame::Frame; +use strum::IntoEnumIterator; +use strum_macros::EnumIter; + /// QUIC transport error. #[allow(clippy::enum_variant_names)] -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Default, PartialEq, Eq, EnumIter)] pub enum Error { /// An endpoint uses this with CONNECTION_CLOSE to signal that the /// connection is being closed abruptly in the absence of any error. + #[default] NoError, /// The endpoint encountered an internal error and cannot continue with the @@ -175,8 +179,7 @@ impl Error { } } - #[cfg(feature = "ffi")] - pub(crate) fn to_c(self) -> libc::ssize_t { + pub(crate) fn to_c(&self) -> libc::ssize_t { match self { Error::NoError => 0, Error::InternalError => -1, @@ -253,10 +256,82 @@ impl std::fmt::Debug for ConnectionError { write!(f, "is_app={:?} ", self.is_app)?; write!(f, "error_code={:?} ", self.error_code)?; match std::str::from_utf8(&self.reason) { - Ok(v) => write!(f, "reason={:?}", v), - Err(_) => write!(f, "reason={:?}", self.reason), - }?; + Ok(v) => write!(f, "reason={:?}", v)?, + Err(_) => write!(f, "reason={:?}", self.reason)?, + }; Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::Error::IoError; + + #[test] + fn error_to_wire() { + let mut found_internal_err = false; + for err in Error::iter() { + if err == Error::NoError { + assert!(err.to_wire() == 0); + continue; + } + if err == Error::Done { + found_internal_err = true; + } + if found_internal_err { + assert_eq!(err.to_wire(), 0); + continue; + } + if let Error::CryptoError(_) = err { + assert_eq!(err.to_wire(), 0); + } else { + assert!(err.to_wire() > 0); + } + } + } + + #[test] + fn error_to_c() { + for err in Error::iter() { + if err == Error::NoError { + assert_eq!(err.to_c(), 0); + } else { + assert!(err.to_c() < 0); + } + } + } + + #[test] + fn io_error() { + use std::error::Error; + let e = std::io::Error::from(std::io::ErrorKind::UnexpectedEof); + let e = super::Error::from(e); + + assert_eq!(format!("{}", e), "IoError(\"unexpected end of file\")"); + assert!(e.source().is_none()); + } + + #[test] + fn connection_error() { + let e = ConnectionError { + is_app: false, + error_code: 0, + frame: None, + reason: vec![], + }; + assert_eq!(format!("{:?}", e), "is_app=false error_code=0 reason=\"\""); + + let e = ConnectionError { + is_app: true, + error_code: 1, + frame: None, + reason: vec![0x97, 0x61, 0x6C], + }; + assert_eq!( + format!("{:?}", e), + "is_app=true error_code=1 reason=[151, 97, 108]" + ); + } +} diff --git a/src/h3/error.rs b/src/h3/error.rs index 392bf04fb..a98fa63bd 100644 --- a/src/h3/error.rs +++ b/src/h3/error.rs @@ -15,9 +15,12 @@ use std::fmt; use std::fmt::Write; +use strum::IntoEnumIterator; +use strum_macros::EnumIter; + /// An HTTP/3 error. /// See RFC 9114 and RFC 9204. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, EnumIter)] pub enum Http3Error { /// This is used when the connection or stream needs to be closed, but there /// is no error to signal. @@ -134,8 +137,7 @@ impl Http3Error { } } - #[cfg(feature = "ffi")] - pub(crate) fn to_c(self) -> libc::ssize_t { + pub(crate) fn to_c(&self) -> libc::ssize_t { match self { Http3Error::NoError => 0, Http3Error::Done => -1, @@ -186,3 +188,42 @@ impl std::convert::From for Http3Error { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn error_to_wire() { + let mut found_internal_err = false; + for err in Http3Error::iter() { + assert!(err.to_wire() > 0); + if let Http3Error::TransportError(_) = err { + found_internal_err = true; + } + if found_internal_err { + assert_eq!(err.to_wire(), 0x102); + } + } + } + + #[test] + fn error_to_c() { + for err in Http3Error::iter() { + if err == Http3Error::NoError { + assert_eq!(err.to_c(), 0); + } else { + assert!(err.to_c() < 0); + } + } + } + + #[test] + fn internal_error() { + let e = Http3Error::InternalError; + assert_eq!(format!("{}", e), "InternalError"); + + use std::error::Error; + assert!(e.source().is_none()); + } +} diff --git a/src/h3/h3.rs b/src/h3/h3.rs index f0d539caf..f977cd898 100644 --- a/src/h3/h3.rs +++ b/src/h3/h3.rs @@ -207,6 +207,29 @@ pub trait Http3Handler { fn on_conn_goaway(&self, stream_id: u64); } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fmt_readable() { + let h = Header::new(b"proto", b"h3"); + assert_eq!(format!("{:?}", h), "\"proto: h3\""); + + let h = Header::new(b"proto", &vec![0x97, 0x61, 0x6c]); + assert_eq!(format!("{:?}", h), "\"proto: [151, 97, 108]\""); + } + + #[test] + fn header_ref() { + let name = b"x-powered-by"; + let value = b"tquic"; + let h = HeaderRef::new(name, value); + assert_eq!(h.name(), name); + assert_eq!(h.value(), value); + } +} + pub use error::Http3Error; #[path = "qpack/qpack.rs"] diff --git a/src/h3/qpack/qpack.rs b/src/h3/qpack/qpack.rs index 30761954f..99f0b382e 100644 --- a/src/h3/qpack/qpack.rs +++ b/src/h3/qpack/qpack.rs @@ -358,11 +358,6 @@ mod tests { fn static_table() { let mut encoded = [0u8; 158]; - for (k, v, index) in static_table::STATIC_ENCODE_TABLE { - assert_eq!(static_table::STATIC_DECODE_TABLE[index as usize].0, k); - assert_eq!(static_table::STATIC_DECODE_TABLE[index as usize].1, v); - } - let headers = vec![ h3::Header::new(b":authority", b""), h3::Header::new(b":path", b"/"), diff --git a/src/h3/qpack/static_table.rs b/src/h3/qpack/static_table.rs index 4207a919c..846214e83 100644 --- a/src/h3/qpack/static_table.rs +++ b/src/h3/qpack/static_table.rs @@ -503,6 +503,14 @@ mod tests { assert_eq!(STATIC_DECODE_TABLE.len(), 99); } + #[test] + fn static_table_index() { + for (name, value, index) in STATIC_ENCODE_TABLE { + assert_eq!(STATIC_DECODE_TABLE[index as usize].0, name); + assert_eq!(STATIC_DECODE_TABLE[index as usize].1, value); + } + } + #[test] fn encode_static_with_name_and_value() { let name: &[u8] = b":method"; @@ -519,9 +527,36 @@ mod tests { #[test] fn encode_static_with_value_not_match() { - let name: &[u8] = b":method"; + let names = vec![ + "agf", + "varz", + "rangf", + "serves", + "referes", + "locatioo", + "forwardef", + "user-agenu", + "content-typf", + "last-modifiee", + "content-lengti", + "x-frame-optiont", + "x-xss-protectioo", + "if-modified-sincf", + "timing-allow-origio", + "x-content-type-optiont", + "content-security-policz", + "strict-transport-securitz", + "access-control-allow-origio", + "access-control-allow-headert", + "access-control-expose-headert", + "access-control-request-headert", + "access-control-allow-credentialt", + ]; + let value: &[u8] = b"UNKNOWN"; - assert_eq!(encode_static(&(name, value)), Some((15, false))); + for name in names { + assert_eq!(encode_static(&(name.as_bytes(), value)), None); + } } #[test] diff --git a/src/timer_queue.rs b/src/timer_queue.rs index 03e17b886..fc5cddf95 100644 --- a/src/timer_queue.rs +++ b/src/timer_queue.rs @@ -79,10 +79,6 @@ impl TimerQueue { } None } - - fn pop_next_expire(&mut self) -> Option { - self.timers.pop_min().map(|(idx, _)| idx) - } } impl Default for TimerQueue { diff --git a/src/window.rs b/src/window.rs index d41a3e566..308d734d1 100644 --- a/src/window.rs +++ b/src/window.rs @@ -109,6 +109,9 @@ mod tests { assert!(!win.contains(137)); assert!(win.contains(10)); assert!(win.contains(0)); + + win.insert(8); + assert!(win.contains(8)); } #[test]