From 91cb0ce5fc9751c02f43f661c08e215af8f5aba4 Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Sun, 14 Mar 2021 11:44:01 +0100 Subject: [PATCH] Move unit tests inside conditionally compiled mod tests { } blocks Idiomatic rust organizes unit tests into `mod tests { }` blocks using conditional compilation `[cfg(test)]` to decide whether to compile the code in that block. This commit moves "bare" test functions into such blocks, and puts the block at the bottom of respective file. --- src/body.rs | 45 +++++++------ src/error.rs | 129 ++++++++++++++++++------------------ src/header.rs | 135 +++++++++++++++++++------------------- src/pool.rs | 169 +++++++++++++++++++++++++----------------------- src/proxy.rs | 3 +- src/request.rs | 45 +++++++------ src/response.rs | 40 ++++++------ 7 files changed, 295 insertions(+), 271 deletions(-) diff --git a/src/body.rs b/src/body.rs index 6cb285d..6420bef 100644 --- a/src/body.rs +++ b/src/body.rs @@ -150,26 +150,6 @@ fn copy_chunked(reader: &mut R, writer: &mut W) -> io::Result } } -#[test] -fn test_copy_chunked() { - let mut source = Vec::::new(); - source.resize(CHUNK_MAX_PAYLOAD_SIZE, 33); - source.extend_from_slice(b"hello world"); - - let mut dest = Vec::::new(); - copy_chunked(&mut &source[..], &mut dest).unwrap(); - - let mut dest_expected = Vec::::new(); - dest_expected.extend_from_slice(format!("{:x}\r\n", CHUNK_MAX_PAYLOAD_SIZE).as_bytes()); - dest_expected.resize(dest_expected.len() + CHUNK_MAX_PAYLOAD_SIZE, 33); - dest_expected.extend_from_slice(b"\r\n"); - - dest_expected.extend_from_slice(b"b\r\nhello world\r\n"); - dest_expected.extend_from_slice(b"0\r\n\r\n"); - - assert_eq!(dest, dest_expected); -} - /// Helper to send a body, either as chunked or not. pub(crate) fn send_body( mut body: SizedReader, @@ -184,3 +164,28 @@ pub(crate) fn send_body( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_copy_chunked() { + let mut source = Vec::::new(); + source.resize(CHUNK_MAX_PAYLOAD_SIZE, 33); + source.extend_from_slice(b"hello world"); + + let mut dest = Vec::::new(); + copy_chunked(&mut &source[..], &mut dest).unwrap(); + + let mut dest_expected = Vec::::new(); + dest_expected.extend_from_slice(format!("{:x}\r\n", CHUNK_MAX_PAYLOAD_SIZE).as_bytes()); + dest_expected.resize(dest_expected.len() + CHUNK_MAX_PAYLOAD_SIZE, 33); + dest_expected.extend_from_slice(b"\r\n"); + + dest_expected.extend_from_slice(b"b\r\nhello world\r\n"); + dest_expected.extend_from_slice(b"0\r\n\r\n"); + + assert_eq!(dest, dest_expected); + } +} diff --git a/src/error.rs b/src/error.rs index 15a9aed..a01420c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -342,73 +342,78 @@ impl fmt::Display for ErrorKind { } } -#[test] -fn status_code_error() { - let mut response = Response::new(404, "NotFound", "").unwrap(); - response.set_url("http://example.org/".parse().unwrap()); - let err = Error::Status(response.status(), response); +#[cfg(test)] +mod tests { + use super::*; - assert_eq!(err.to_string(), "http://example.org/: status code 404"); -} + #[test] + fn status_code_error() { + let mut response = Response::new(404, "NotFound", "").unwrap(); + response.set_url("http://example.org/".parse().unwrap()); + let err = Error::Status(response.status(), response); -#[test] -fn status_code_error_redirect() { - use crate::{get, test}; + assert_eq!(err.to_string(), "http://example.org/: status code 404"); + } - test::set_handler("/redirect_a", |unit| { - assert_eq!(unit.method, "GET"); - test::make_response( - 302, - "Go here", - vec!["Location: test://example.edu/redirect_b"], - vec![], - ) - }); - test::set_handler("/redirect_b", |unit| { - assert_eq!(unit.method, "GET"); - test::make_response( - 302, - "Go here", - vec!["Location: http://example.com/status/500"], - vec![], - ) - }); + #[test] + fn status_code_error_redirect() { + use crate::{get, test}; - let err = get("test://example.org/redirect_a").call().unwrap_err(); - assert_eq!(err.kind(), ErrorKind::HTTP, "{:?}", err); - assert_eq!( + test::set_handler("/redirect_a", |unit| { + assert_eq!(unit.method, "GET"); + test::make_response( + 302, + "Go here", + vec!["Location: test://example.edu/redirect_b"], + vec![], + ) + }); + test::set_handler("/redirect_b", |unit| { + assert_eq!(unit.method, "GET"); + test::make_response( + 302, + "Go here", + vec!["Location: http://example.com/status/500"], + vec![], + ) + }); + + let err = get("test://example.org/redirect_a").call().unwrap_err(); + assert_eq!(err.kind(), ErrorKind::HTTP, "{:?}", err); + assert_eq!( err.to_string(), "http://example.com/status/500: status code 500 (redirected from test://example.org/redirect_a)" ); -} - -#[test] -fn io_error() { - let ioe = io::Error::new(io::ErrorKind::TimedOut, "too slow"); - let mut err = Error::new(ErrorKind::Io, Some("oops".to_string())).src(ioe); - - err = err.url("http://example.com/".parse().unwrap()); - assert_eq!( - err.to_string(), - "http://example.com/: Network Error: oops: too slow" - ); -} - -#[test] -fn connection_closed() { - let ioe = io::Error::new(io::ErrorKind::ConnectionReset, "connection reset"); - let err = ErrorKind::Io.new().src(ioe); - assert!(err.connection_closed()); - - let ioe = io::Error::new(io::ErrorKind::ConnectionAborted, "connection aborted"); - let err = ErrorKind::Io.new().src(ioe); - assert!(err.connection_closed()); -} - -#[test] -fn error_is_send_and_sync() { - fn takes_send(_: impl Send) {} - fn takes_sync(_: impl Sync) {} - takes_send(crate::error::ErrorKind::InvalidUrl.new()); - takes_sync(crate::error::ErrorKind::InvalidUrl.new()); + } + + #[test] + fn io_error() { + let ioe = io::Error::new(io::ErrorKind::TimedOut, "too slow"); + let mut err = Error::new(ErrorKind::Io, Some("oops".to_string())).src(ioe); + + err = err.url("http://example.com/".parse().unwrap()); + assert_eq!( + err.to_string(), + "http://example.com/: Network Error: oops: too slow" + ); + } + + #[test] + fn connection_closed() { + let ioe = io::Error::new(io::ErrorKind::ConnectionReset, "connection reset"); + let err = ErrorKind::Io.new().src(ioe); + assert!(err.connection_closed()); + + let ioe = io::Error::new(io::ErrorKind::ConnectionAborted, "connection aborted"); + let err = ErrorKind::Io.new().src(ioe); + assert!(err.connection_closed()); + } + + #[test] + fn error_is_send_and_sync() { + fn takes_send(_: impl Send) {} + fn takes_sync(_: impl Sync) {} + takes_send(crate::error::ErrorKind::InvalidUrl.new()); + takes_sync(crate::error::ErrorKind::InvalidUrl.new()); + } } diff --git a/src/header.rs b/src/header.rs index 74cdde1..122cee2 100644 --- a/src/header.rs +++ b/src/header.rs @@ -145,72 +145,77 @@ impl FromStr for Header { } } -#[test] -fn test_valid_name() { - assert!(valid_name("example")); - assert!(valid_name("Content-Type")); - assert!(valid_name("h-123456789")); - assert!(!valid_name("Content-Type:")); - assert!(!valid_name("Content-Type ")); - assert!(!valid_name(" some-header")); - assert!(!valid_name("\"invalid\"")); - assert!(!valid_name("Gödel")); -} +#[cfg(test)] +mod tests { + use super::*; -#[test] -fn test_valid_value() { - assert!(valid_value("example")); - assert!(valid_value("foo bar")); - assert!(valid_value(" foobar ")); - assert!(valid_value(" foo\tbar ")); - assert!(valid_value(" foo~")); - assert!(valid_value(" !bar")); - assert!(valid_value(" ")); - assert!(!valid_value(" \nfoo")); - assert!(!valid_value("foo\x7F")); -} + #[test] + fn test_valid_name() { + assert!(valid_name("example")); + assert!(valid_name("Content-Type")); + assert!(valid_name("h-123456789")); + assert!(!valid_name("Content-Type:")); + assert!(!valid_name("Content-Type ")); + assert!(!valid_name(" some-header")); + assert!(!valid_name("\"invalid\"")); + assert!(!valid_name("Gödel")); + } -#[test] -fn test_parse_invalid_name() { - let cases = vec![ - "Content-Type :", - " Content-Type: foo", - "Content-Type foo", - "\"some-header\": foo", - "Gödel: Escher, Bach", - "Foo: \n", - "Foo: \nbar", - "Foo: \x7F bar", - ]; - for c in cases { - let result = c.parse::
(); - assert!( - matches!(result, Err(ref e) if e.kind() == ErrorKind::BadHeader), - "'{}'.parse(): expected BadHeader, got {:?}", - c, - result - ); + #[test] + fn test_valid_value() { + assert!(valid_value("example")); + assert!(valid_value("foo bar")); + assert!(valid_value(" foobar ")); + assert!(valid_value(" foo\tbar ")); + assert!(valid_value(" foo~")); + assert!(valid_value(" !bar")); + assert!(valid_value(" ")); + assert!(!valid_value(" \nfoo")); + assert!(!valid_value("foo\x7F")); + } + + #[test] + fn test_parse_invalid_name() { + let cases = vec![ + "Content-Type :", + " Content-Type: foo", + "Content-Type foo", + "\"some-header\": foo", + "Gödel: Escher, Bach", + "Foo: \n", + "Foo: \nbar", + "Foo: \x7F bar", + ]; + for c in cases { + let result = c.parse::
(); + assert!( + matches!(result, Err(ref e) if e.kind() == ErrorKind::BadHeader), + "'{}'.parse(): expected BadHeader, got {:?}", + c, + result + ); + } + } + + #[test] + fn empty_value() { + let h = "foo:".parse::
().unwrap(); + assert_eq!(h.value(), ""); + } + + #[test] + fn value_with_whitespace() { + let h = "foo: bar ".parse::
().unwrap(); + assert_eq!(h.value(), "bar"); + } + + #[test] + fn name_and_value() { + let header: Header = "X-Forwarded-For: 127.0.0.1".parse().unwrap(); + assert_eq!("X-Forwarded-For", header.name()); + assert_eq!("127.0.0.1", header.value()); + assert!(header.is_name("X-Forwarded-For")); + assert!(header.is_name("x-forwarded-for")); + assert!(header.is_name("X-FORWARDED-FOR")); } } - -#[test] -fn empty_value() { - let h = "foo:".parse::
().unwrap(); - assert_eq!(h.value(), ""); -} - -#[test] -fn value_with_whitespace() { - let h = "foo: bar ".parse::
().unwrap(); - assert_eq!(h.value(), "bar"); -} - -#[test] -fn name_and_value() { - let header: Header = "X-Forwarded-For: 127.0.0.1".parse().unwrap(); - assert_eq!("X-Forwarded-For", header.name()); - assert_eq!("127.0.0.1", header.value()); - assert!(header.is_name("X-Forwarded-For")); - assert!(header.is_name("x-forwarded-for")); - assert!(header.is_name("X-FORWARDED-FOR")); -} diff --git a/src/pool.rs b/src/pool.rs index bc20475..26ad5a1 100644 --- a/src/pool.rs +++ b/src/pool.rs @@ -219,88 +219,6 @@ impl PoolKey { } } -#[test] -fn poolkey_new() { - // Test that PoolKey::new() does not panic on unrecognized schemes. - PoolKey::new(&Url::parse("zzz:///example.com").unwrap(), None); -} - -#[test] -fn pool_connections_limit() { - // Test inserting connections with different keys into the pool, - // filling and draining it. The pool should evict earlier connections - // when the connection limit is reached. - let pool = ConnectionPool::new_with_limits(10, 1); - let hostnames = (0..pool.max_idle_connections * 2).map(|i| format!("{}.example", i)); - let poolkeys = hostnames.map(|hostname| PoolKey { - scheme: "https".to_string(), - hostname, - port: Some(999), - proxy: None, - }); - for key in poolkeys.clone() { - pool.add(key, Stream::from_vec(vec![])) - } - assert_eq!(pool.len(), pool.max_idle_connections); - - for key in poolkeys.skip(pool.max_idle_connections) { - let result = pool.remove(&key); - assert!(result.is_some(), "expected key was not in pool"); - } - assert_eq!(pool.len(), 0) -} - -#[test] -fn pool_per_host_connections_limit() { - // Test inserting connections with the same key into the pool, - // filling and draining it. The pool should evict earlier connections - // when the per-host connection limit is reached. - let pool = ConnectionPool::new_with_limits(10, 2); - let poolkey = PoolKey { - scheme: "https".to_string(), - hostname: "example.com".to_string(), - port: Some(999), - proxy: None, - }; - - for _ in 0..pool.max_idle_connections_per_host * 2 { - pool.add(poolkey.clone(), Stream::from_vec(vec![])) - } - assert_eq!(pool.len(), pool.max_idle_connections_per_host); - - for _ in 0..pool.max_idle_connections_per_host { - let result = pool.remove(&poolkey); - assert!(result.is_some(), "expected key was not in pool"); - } - assert_eq!(pool.len(), 0); -} - -#[test] -fn pool_checks_proxy() { - // Test inserting different poolkeys with same address but different proxies. - // Each insertion should result in an additional entry in the pool. - let pool = ConnectionPool::new_with_limits(10, 1); - let url = Url::parse("zzz:///example.com").unwrap(); - - pool.add(PoolKey::new(&url, None), Stream::from_vec(vec![])); - assert_eq!(pool.len(), 1); - - pool.add( - PoolKey::new(&url, Some(Proxy::new("localhost:9999").unwrap())), - Stream::from_vec(vec![]), - ); - assert_eq!(pool.len(), 2); - - pool.add( - PoolKey::new( - &url, - Some(Proxy::new("user:password@localhost:9999").unwrap()), - ), - Stream::from_vec(vec![]), - ); - assert_eq!(pool.len(), 3); -} - /// Read wrapper that returns the stream to the pool once the /// read is exhausted (reached a 0). /// @@ -360,3 +278,90 @@ impl> Read for PoolReturnRead { Ok(amount) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn poolkey_new() { + // Test that PoolKey::new() does not panic on unrecognized schemes. + PoolKey::new(&Url::parse("zzz:///example.com").unwrap(), None); + } + + #[test] + fn pool_connections_limit() { + // Test inserting connections with different keys into the pool, + // filling and draining it. The pool should evict earlier connections + // when the connection limit is reached. + let pool = ConnectionPool::new_with_limits(10, 1); + let hostnames = (0..pool.max_idle_connections * 2).map(|i| format!("{}.example", i)); + let poolkeys = hostnames.map(|hostname| PoolKey { + scheme: "https".to_string(), + hostname, + port: Some(999), + proxy: None, + }); + for key in poolkeys.clone() { + pool.add(key, Stream::from_vec(vec![])) + } + assert_eq!(pool.len(), pool.max_idle_connections); + + for key in poolkeys.skip(pool.max_idle_connections) { + let result = pool.remove(&key); + assert!(result.is_some(), "expected key was not in pool"); + } + assert_eq!(pool.len(), 0) + } + + #[test] + fn pool_per_host_connections_limit() { + // Test inserting connections with the same key into the pool, + // filling and draining it. The pool should evict earlier connections + // when the per-host connection limit is reached. + let pool = ConnectionPool::new_with_limits(10, 2); + let poolkey = PoolKey { + scheme: "https".to_string(), + hostname: "example.com".to_string(), + port: Some(999), + proxy: None, + }; + + for _ in 0..pool.max_idle_connections_per_host * 2 { + pool.add(poolkey.clone(), Stream::from_vec(vec![])) + } + assert_eq!(pool.len(), pool.max_idle_connections_per_host); + + for _ in 0..pool.max_idle_connections_per_host { + let result = pool.remove(&poolkey); + assert!(result.is_some(), "expected key was not in pool"); + } + assert_eq!(pool.len(), 0); + } + + #[test] + fn pool_checks_proxy() { + // Test inserting different poolkeys with same address but different proxies. + // Each insertion should result in an additional entry in the pool. + let pool = ConnectionPool::new_with_limits(10, 1); + let url = Url::parse("zzz:///example.com").unwrap(); + + pool.add(PoolKey::new(&url, None), Stream::from_vec(vec![])); + assert_eq!(pool.len(), 1); + + pool.add( + PoolKey::new(&url, Some(Proxy::new("localhost:9999").unwrap())), + Stream::from_vec(vec![]), + ); + assert_eq!(pool.len(), 2); + + pool.add( + PoolKey::new( + &url, + Some(Proxy::new("user:password@localhost:9999").unwrap()), + ), + Stream::from_vec(vec![]), + ); + assert_eq!(pool.len(), 3); + } +} diff --git a/src/proxy.rs b/src/proxy.rs index 0daea67..0aed467 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -171,8 +171,7 @@ Proxy-Connection: Keep-Alive\r\n\ #[cfg(test)] mod tests { - use super::Proto; - use super::Proxy; + use super::*; #[test] fn parse_proxy_fakeproto() { diff --git a/src/request.rs b/src/request.rs index 1505fcc..0113990 100644 --- a/src/request.rs +++ b/src/request.rs @@ -397,25 +397,30 @@ impl Request { } } -#[test] -fn request_implements_send_and_sync() { - let _request: Box = Box::new(Request::new( - Agent::new(), - "GET".to_string(), - "https://example.com/".to_string(), - )); - let _request: Box = Box::new(Request::new( - Agent::new(), - "GET".to_string(), - "https://example.com/".to_string(), - )); -} +#[cfg(test)] +mod tests { + use super::*; -#[test] -fn send_byte_slice() { - let bytes = vec![1, 2, 3]; - crate::agent() - .post("http://example.com") - .send(&bytes[1..2]) - .ok(); + #[test] + fn request_implements_send_and_sync() { + let _request: Box = Box::new(Request::new( + Agent::new(), + "GET".to_string(), + "https://example.com/".to_string(), + )); + let _request: Box = Box::new(Request::new( + Agent::new(), + "GET".to_string(), + "https://example.com/".to_string(), + )); + } + + #[test] + fn send_byte_slice() { + let bytes = vec![1, 2, 3]; + crate::agent() + .post("http://example.com") + .send(&bytes[1..2]) + .ok(); + } } diff --git a/src/response.rs b/src/response.rs index b38a81b..97b6c54 100644 --- a/src/response.rs +++ b/src/response.rs @@ -632,15 +632,6 @@ impl Read for LimitedRead { } } -#[test] -fn short_read() { - use std::io::Cursor; - let mut lr = LimitedRead::new(Cursor::new(vec![b'a'; 3]), 10); - let mut buf = vec![0; 1000]; - let result = lr.read_to_end(&mut buf); - assert!(result.err().unwrap().kind() == io::ErrorKind::UnexpectedEof); -} - impl From> for Stream where Stream: From, @@ -667,10 +658,30 @@ pub(crate) fn charset_from_content_type(header: Option<&str>) -> &str { .unwrap_or(DEFAULT_CHARACTER_SET) } +// ErrorReader returns an error for every read. +// The error is as close to a clone of the underlying +// io::Error as we can get. +struct ErrorReader(io::Error); + +impl Read for ErrorReader { + fn read(&mut self, _buf: &mut [u8]) -> io::Result { + Err(io::Error::new(self.0.kind(), self.0.to_string())) + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn short_read() { + use std::io::Cursor; + let mut lr = LimitedRead::new(Cursor::new(vec![b'a'; 3]), 10); + let mut buf = vec![0; 1000]; + let result = lr.read_to_end(&mut buf); + assert!(result.err().unwrap().kind() == io::ErrorKind::UnexpectedEof); + } + #[test] fn content_type_without_charset() { let s = "HTTP/1.1 200 OK\r\n\ @@ -823,14 +834,3 @@ mod tests { assert_eq!(hist, ["http://1.example.com/", "http://2.example.com/"]) } } - -// ErrorReader returns an error for every read. -// The error is as close to a clone of the underlying -// io::Error as we can get. -struct ErrorReader(io::Error); - -impl Read for ErrorReader { - fn read(&mut self, _buf: &mut [u8]) -> io::Result { - Err(io::Error::new(self.0.kind(), self.0.to_string())) - } -}