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

@@ -150,26 +150,6 @@ fn copy_chunked<R: Read, W: Write>(reader: &mut R, writer: &mut W) -> io::Result
     }
 }
 
-#[test]
-fn test_copy_chunked() {
-    let mut source = Vec::<u8>::new();
-    source.resize(CHUNK_MAX_PAYLOAD_SIZE, 33);
-    source.extend_from_slice(b"hello world");
-
-    let mut dest = Vec::<u8>::new();
-    copy_chunked(&mut &source[..], &mut dest).unwrap();
-
-    let mut dest_expected = Vec::<u8>::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::<u8>::new();
+        source.resize(CHUNK_MAX_PAYLOAD_SIZE, 33);
+        source.extend_from_slice(b"hello world");
+
+        let mut dest = Vec::<u8>::new();
+        copy_chunked(&mut &source[..], &mut dest).unwrap();
+
+        let mut dest_expected = Vec::<u8>::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);
+    }
+}

src/error.rs

@@ -342,73 +342,78 @@ impl fmt::Display for ErrorKind {
     }
 }
 
-#[test]
+#[cfg(test)]
-fn status_code_error() {
+mod tests {
-    let mut response = Response::new(404, "NotFound", "").unwrap();
+    use super::*;
-    response.set_url("http://example.org/".parse().unwrap());
-    let err = Error::Status(response.status(), response);
 
-    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]
+        assert_eq!(err.to_string(), "http://example.org/: status code 404");
-fn status_code_error_redirect() {
+    }
-    use crate::{get, test};
 
-    test::set_handler("/redirect_a", |unit| {
+    #[test]
-        assert_eq!(unit.method, "GET");
+    fn status_code_error_redirect() {
-        test::make_response(
+        use crate::{get, test};
-            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();
+        test::set_handler("/redirect_a", |unit| {
-    assert_eq!(err.kind(), ErrorKind::HTTP, "{:?}", err);
+            assert_eq!(unit.method, "GET");
-    assert_eq!(
+            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]
+    #[test]
-fn io_error() {
+    fn io_error() {
-    let ioe = io::Error::new(io::ErrorKind::TimedOut, "too slow");
+        let ioe = io::Error::new(io::ErrorKind::TimedOut, "too slow");
-    let mut err = Error::new(ErrorKind::Io, Some("oops".to_string())).src(ioe);
+        let mut err = Error::new(ErrorKind::Io, Some("oops".to_string())).src(ioe);
-
+
-    err = err.url("http://example.com/".parse().unwrap());
+        err = err.url("http://example.com/".parse().unwrap());
-    assert_eq!(
+        assert_eq!(
-        err.to_string(),
+            err.to_string(),
-        "http://example.com/: Network Error: oops: too slow"
+            "http://example.com/: Network Error: oops: too slow"
-    );
+        );
-}
+    }
-
+
-#[test]
+    #[test]
-fn connection_closed() {
+    fn connection_closed() {
-    let ioe = io::Error::new(io::ErrorKind::ConnectionReset, "connection reset");
+        let ioe = io::Error::new(io::ErrorKind::ConnectionReset, "connection reset");
-    let err = ErrorKind::Io.new().src(ioe);
+        let err = ErrorKind::Io.new().src(ioe);
-    assert!(err.connection_closed());
+        assert!(err.connection_closed());
-
+
-    let ioe = io::Error::new(io::ErrorKind::ConnectionAborted, "connection aborted");
+        let ioe = io::Error::new(io::ErrorKind::ConnectionAborted, "connection aborted");
-    let err = ErrorKind::Io.new().src(ioe);
+        let err = ErrorKind::Io.new().src(ioe);
-    assert!(err.connection_closed());
+        assert!(err.connection_closed());
-}
+    }
-
+
-#[test]
+    #[test]
-fn error_is_send_and_sync() {
+    fn error_is_send_and_sync() {
-    fn takes_send(_: impl Send) {}
+        fn takes_send(_: impl Send) {}
-    fn takes_sync(_: impl Sync) {}
+        fn takes_sync(_: impl Sync) {}
-    takes_send(crate::error::ErrorKind::InvalidUrl.new());
+        takes_send(crate::error::ErrorKind::InvalidUrl.new());
-    takes_sync(crate::error::ErrorKind::InvalidUrl.new());
+        takes_sync(crate::error::ErrorKind::InvalidUrl.new());
+    }
 }

src/header.rs

@@ -145,72 +145,77 @@ impl FromStr for Header {
     }
 }
 
-#[test]
+#[cfg(test)]
-fn test_valid_name() {
+mod tests {
-    assert!(valid_name("example"));
+    use super::*;
-    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]
+    #[test]
-fn test_valid_value() {
+    fn test_valid_name() {
-    assert!(valid_value("example"));
+        assert!(valid_name("example"));
-    assert!(valid_value("foo bar"));
+        assert!(valid_name("Content-Type"));
-    assert!(valid_value(" foobar "));
+        assert!(valid_name("h-123456789"));
-    assert!(valid_value(" foo\tbar "));
+        assert!(!valid_name("Content-Type:"));
-    assert!(valid_value(" foo~"));
+        assert!(!valid_name("Content-Type "));
-    assert!(valid_value(" !bar"));
+        assert!(!valid_name(" some-header"));
-    assert!(valid_value(" "));
+        assert!(!valid_name("\"invalid\""));
-    assert!(!valid_value(" \nfoo"));
+        assert!(!valid_name("Gödel"));
-    assert!(!valid_value("foo\x7F"));
+    }
-}
 
-#[test]
+    #[test]
-fn test_parse_invalid_name() {
+    fn test_valid_value() {
-    let cases = vec![
+        assert!(valid_value("example"));
-        "Content-Type  :",
+        assert!(valid_value("foo bar"));
-        " Content-Type: foo",
+        assert!(valid_value(" foobar "));
-        "Content-Type foo",
+        assert!(valid_value(" foo\tbar "));
-        "\"some-header\": foo",
+        assert!(valid_value(" foo~"));
-        "Gödel: Escher, Bach",
+        assert!(valid_value(" !bar"));
-        "Foo: \n",
+        assert!(valid_value(" "));
-        "Foo: \nbar",
+        assert!(!valid_value(" \nfoo"));
-        "Foo: \x7F bar",
+        assert!(!valid_value("foo\x7F"));
-    ];
+    }
-    for c in cases {
+
-        let result = c.parse::<Header>();
+    #[test]
-        assert!(
+    fn test_parse_invalid_name() {
-            matches!(result, Err(ref e) if e.kind() == ErrorKind::BadHeader),
+        let cases = vec![
-            "'{}'.parse(): expected BadHeader, got {:?}",
+            "Content-Type  :",
-            c,
+            " Content-Type: foo",
-            result
+            "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::<Header>();
+            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::<Header>().unwrap();
+        assert_eq!(h.value(), "");
+    }
+
+    #[test]
+    fn value_with_whitespace() {
+        let h = "foo:      bar    ".parse::<Header>().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::<Header>().unwrap();
-    assert_eq!(h.value(), "");
-}
-
-#[test]
-fn value_with_whitespace() {
-    let h = "foo:      bar    ".parse::<Header>().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"));
-}

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<R: Read + Sized + Into<Stream>> Read for PoolReturnRead<R> {
         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);
+    }
+}

src/proxy.rs

@@ -171,8 +171,7 @@ Proxy-Connection: Keep-Alive\r\n\
 
 #[cfg(test)]
 mod tests {
-    use super::Proto;
+    use super::*;
-    use super::Proxy;
 
     #[test]
     fn parse_proxy_fakeproto() {

src/request.rs

@@ -397,25 +397,30 @@ impl Request {
     }
 }
 
-#[test]
+#[cfg(test)]
-fn request_implements_send_and_sync() {
+mod tests {
-    let _request: Box<dyn Send> = Box::new(Request::new(
+    use super::*;
-        Agent::new(),
-        "GET".to_string(),
-        "https://example.com/".to_string(),
-    ));
-    let _request: Box<dyn Sync> = Box::new(Request::new(
-        Agent::new(),
-        "GET".to_string(),
-        "https://example.com/".to_string(),
-    ));
-}
 
-#[test]
+    #[test]
-fn send_byte_slice() {
+    fn request_implements_send_and_sync() {
-    let bytes = vec![1, 2, 3];
+        let _request: Box<dyn Send> = Box::new(Request::new(
-    crate::agent()
+            Agent::new(),
-        .post("http://example.com")
+            "GET".to_string(),
-        .send(&bytes[1..2])
+            "https://example.com/".to_string(),
-        .ok();
+        ));
+        let _request: Box<dyn Sync> = 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();
+    }
 }

src/response.rs

@@ -632,15 +632,6 @@ impl<R: Read> Read for LimitedRead<R> {
     }
 }
 
-#[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<R: Read> From<LimitedRead<R>> for Stream
 where
     Stream: From<R>,
@@ -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<usize> {
+        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<usize> {
-        Err(io::Error::new(self.0.kind(), self.0.to_string()))
-    }
-}