tests: reduce boilerplate of sending GET requests

This adds a `SendRequestExt` trait to h2-support, with a `get` method
that does a lot of the repeated request building stuff many test cases
were doing.

As a first step, the cleans up stream_states tests to use it.

tests/h2-support/src/client_ext.rs

@@ -0,0 +1,30 @@
+use bytes::IntoBuf;
+use http::Request;
+use h2::client::{ResponseFuture, SendRequest};
+
+/// Extend the `h2::client::SendRequest` type with convenience methods.
+pub trait SendRequestExt {
+    /// Convenience method to send a GET request and ignore the SendStream
+    /// (since GETs don't need to send a body).
+    fn get(&mut self, uri: &str) -> ResponseFuture;
+}
+
+impl<B> SendRequestExt for SendRequest<B>
+where
+    B: IntoBuf,
+    B::Buf: 'static,
+{
+    fn get(&mut self, uri: &str) -> ResponseFuture {
+        let req = Request::builder()
+            // method is GET by default
+            .uri(uri)
+            .body(())
+            .expect("valid uri");
+
+        let (fut, _tx) = self
+            .send_request(req, /*eos =*/true)
+            .expect("send_request");
+
+        fut
+    }
+}

tests/h2-support/src/future_ext.rs

@@ -72,20 +72,11 @@ pub trait FutureExt: Future {
     ///
     /// This allows the executor to poll other futures before trying this one
     /// again.
-    fn yield_once(self) -> Box<Future<Item = Self::Item, Error = Self::Error>>
+    fn yield_once(self) -> Box<dyn Future<Item = Self::Item, Error = Self::Error>>
     where
         Self: Future + Sized + 'static,
     {
-        let mut ready = false;
+        Box::new(super::util::yield_once().then(move |_| self))
-        Box::new(::futures::future::poll_fn(move || {
-            if ready {
-                Ok::<_, ()>(().into())
-            } else {
-                ready = true;
-                ::futures::task::current().notify();
-                Ok(::futures::Async::NotReady)
-            }
-        }).then(|_| self))
     }
 }
 

tests/h2-support/src/lib.rs

@@ -22,8 +22,10 @@ pub mod mock_io;
 pub mod notify;
 pub mod util;
 
+mod client_ext;
 mod future_ext;
 
+pub use client_ext::{SendRequestExt};
 pub use future_ext::{FutureExt, Unwrap};
 
 pub type WindowSize = usize;

tests/h2-support/src/mock.rs

@@ -116,7 +116,7 @@ impl Handle {
     }
 
     /// Read the client preface
-    pub fn read_preface(self) -> Box<Future<Item = Self, Error = io::Error>> {
+    pub fn read_preface(self) -> Box<dyn Future<Item = Self, Error = io::Error>> {
         let buf = vec![0; PREFACE.len()];
         let ret = read_exact(self, buf).and_then(|(me, buf)| {
             assert_eq!(buf, PREFACE);
@@ -129,7 +129,7 @@ impl Handle {
     /// Perform the H2 handshake
     pub fn assert_client_handshake(
         self,
-    ) -> Box<Future<Item = (frame::Settings, Self), Error = h2::Error>> {
+    ) -> Box<dyn Future<Item = (frame::Settings, Self), Error = h2::Error>> {
         self.assert_client_handshake_with_settings(frame::Settings::default())
     }
 
@@ -137,7 +137,7 @@ impl Handle {
     pub fn assert_client_handshake_with_settings<T>(
         mut self,
         settings: T,
-    ) -> Box<Future<Item = (frame::Settings, Self), Error = h2::Error>>
+    ) -> Box<dyn Future<Item = (frame::Settings, Self), Error = h2::Error>>
     where
         T: Into<frame::Settings>,
     {
@@ -189,7 +189,7 @@ impl Handle {
     /// Perform the H2 handshake
     pub fn assert_server_handshake(
         self,
-    ) -> Box<Future<Item = (frame::Settings, Self), Error = h2::Error>> {
+    ) -> Box<dyn Future<Item = (frame::Settings, Self), Error = h2::Error>> {
         self.assert_server_handshake_with_settings(frame::Settings::default())
     }
 
@@ -197,7 +197,7 @@ impl Handle {
     pub fn assert_server_handshake_with_settings<T>(
         mut self,
         settings: T,
-    ) -> Box<Future<Item = (frame::Settings, Self), Error = h2::Error>>
+    ) -> Box<dyn Future<Item = (frame::Settings, Self), Error = h2::Error>>
     where
         T: Into<frame::Settings>,
     {
@@ -433,7 +433,7 @@ impl AsyncWrite for Pipe {
 
 pub trait HandleFutureExt {
     fn recv_settings(self)
-        -> RecvFrame<Box<Future<Item = (Option<Frame>, Handle), Error = ()>>>
+        -> RecvFrame<Box<dyn Future<Item = (Option<Frame>, Handle), Error = ()>>>
     where
         Self: Sized + 'static,
         Self: Future<Item = (frame::Settings, Handle)>,
@@ -443,7 +443,7 @@ pub trait HandleFutureExt {
     }
 
     fn recv_custom_settings<T>(self, settings: T)
-        -> RecvFrame<Box<Future<Item = (Option<Frame>, Handle), Error = ()>>>
+        -> RecvFrame<Box<dyn Future<Item = (Option<Frame>, Handle), Error = ()>>>
     where
         Self: Sized + 'static,
         Self: Future<Item = (frame::Settings, Handle)>,
@@ -454,7 +454,7 @@ pub trait HandleFutureExt {
             .map(|(settings, handle)| (Some(settings.into()), handle))
             .unwrap();
 
-        let boxed: Box<Future<Item = (Option<Frame>, Handle), Error = ()>> =
+        let boxed: Box<dyn Future<Item = (Option<Frame>, Handle), Error = ()>> =
             Box::new(map);
         RecvFrame {
             inner: boxed,
@@ -462,7 +462,7 @@ pub trait HandleFutureExt {
         }
     }
 
-    fn ignore_settings(self) -> Box<Future<Item = Handle, Error = ()>>
+    fn ignore_settings(self) -> Box<dyn Future<Item = Handle, Error = ()>>
     where
         Self: Sized + 'static,
         Self: Future<Item = (frame::Settings, Handle)>,
@@ -497,7 +497,7 @@ pub trait HandleFutureExt {
         }
     }
 
-    fn send_bytes(self, data: &[u8]) -> Box<Future<Item = Handle, Error = Self::Error>>
+    fn send_bytes(self, data: &[u8]) -> Box<dyn Future<Item = Handle, Error = Self::Error>>
     where
         Self: Future<Item = Handle> + Sized + 'static,
         Self::Error: fmt::Debug,
@@ -536,7 +536,7 @@ pub trait HandleFutureExt {
             .recv_frame(frames::ping(payload).pong())
     }
 
-    fn idle_ms(self, ms: usize) -> Box<Future<Item = Handle, Error = Self::Error>>
+    fn idle_ms(self, ms: usize) -> Box<dyn Future<Item = Handle, Error = Self::Error>>
     where
         Self: Sized + 'static,
         Self: Future<Item = Handle>,
@@ -562,7 +562,7 @@ pub trait HandleFutureExt {
         }))
     }
 
-    fn buffer_bytes(self, num: usize) -> Box<Future<Item = Handle, Error = Self::Error>>
+    fn buffer_bytes(self, num: usize) -> Box<dyn Future<Item = Handle, Error = Self::Error>>
     where Self: Sized + 'static,
           Self: Future<Item = Handle>,
           Self::Error: fmt::Debug,
@@ -596,7 +596,7 @@ pub trait HandleFutureExt {
         }))
     }
 
-    fn unbounded_bytes(self) -> Box<Future<Item = Handle, Error = Self::Error>>
+    fn unbounded_bytes(self) -> Box<dyn Future<Item = Handle, Error = Self::Error>>
     where Self: Sized + 'static,
           Self: Future<Item = Handle>,
           Self::Error: fmt::Debug,
@@ -615,7 +615,7 @@ pub trait HandleFutureExt {
         }))
     }
 
-    fn then_notify(self, tx: oneshot::Sender<()>) -> Box<Future<Item = Handle, Error = Self::Error>>
+    fn then_notify(self, tx: oneshot::Sender<()>) -> Box<dyn Future<Item = Handle, Error = Self::Error>>
     where Self: Sized + 'static,
           Self: Future<Item = Handle>,
           Self::Error: fmt::Debug,
@@ -626,7 +626,7 @@ pub trait HandleFutureExt {
         }))
     }
 
-    fn wait_for<F>(self, other: F) -> Box<Future<Item = Self::Item, Error = Self::Error>>
+    fn wait_for<F>(self, other: F) -> Box<dyn Future<Item = Self::Item, Error = Self::Error>>
     where
         F: Future + 'static,
         Self: Future + Sized + 'static
@@ -636,7 +636,7 @@ pub trait HandleFutureExt {
         }))
     }
 
-    fn close(self) -> Box<Future<Item = (), Error = ()>>
+    fn close(self) -> Box<dyn Future<Item = (), Error = ()>>
     where
         Self: Future<Error = ()> + Sized + 'static,
     {
@@ -752,7 +752,7 @@ where
     T: Future<Item = Handle> + 'static,
     T::Error: fmt::Debug,
 {
-    type Future = Box<Future<Item = (Option<Frame>, Handle), Error = ()>>;
+    type Future = Box<dyn Future<Item = (Option<Frame>, Handle), Error = ()>>;
 
     fn into_recv_frame(self, frame: Option<Frame>) -> RecvFrame<Self::Future> {
         let into_fut = Box::new(

tests/h2-support/src/mod.rs

@@ -1,69 +0,0 @@
-//! Utilities to support tests.
-
-#[cfg(not(feature = "unstable"))]
-compile_error!(
-    "Tests depend on the 'unstable' feature on h2. \
-    Retry with `cargo test --features unstable`"
-);
-
-pub extern crate bytes;
-pub extern crate env_logger;
-pub extern crate futures;
-pub extern crate h2;
-pub extern crate http;
-pub extern crate string;
-pub extern crate tokio_io;
-
-// Kind of annoying, but we can't use macros from crates that aren't specified
-// at the root.
-macro_rules! try_ready {
-    ($e:expr) => ({
-        use $crate::support::futures::Async;
-        match $e {
-            Ok(Async::Ready(t)) => t,
-            Ok(Async::NotReady) => return Ok(Async::NotReady),
-            Err(e) => return Err(From::from(e)),
-        }
-    })
-}
-
-macro_rules! try_nb {
-    ($e:expr) => ({
-        use ::std::io::ErrorKind::WouldBlock;
-        use $crate::support::futures::Async;
-
-        match $e {
-            Ok(t) => t,
-            Err(ref e) if e.kind() == WouldBlock => {
-                return Ok(Async::NotReady)
-            }
-            Err(e) => return Err(e.into()),
-        }
-    })
-}
-
-#[macro_use]
-mod assert;
-
-#[macro_use]
-pub mod raw;
-
-pub mod frames;
-pub mod prelude;
-pub mod mock;
-pub mod mock_io;
-pub mod notify;
-pub mod util;
-
-mod future_ext;
-
-pub use self::future_ext::{FutureExt, Unwrap};
-
-pub type WindowSize = usize;
-pub const DEFAULT_WINDOW_SIZE: WindowSize = (1 << 16) - 1;
-
-// This is our test Codec type
-pub type Codec<T> = h2::Codec<T, ::std::io::Cursor<::bytes::Bytes>>;
-
-// This is the frame type that is sent
-pub type SendFrame = h2::frame::Frame<::std::io::Cursor<::bytes::Bytes>>;

tests/h2-support/src/prelude.rs

@@ -31,6 +31,9 @@ pub use self::futures::{Future, IntoFuture, Sink, Stream};
 // And our Future extensions
 pub use super::future_ext::{FutureExt, Unwrap};
 
+// Our client_ext helpers
+pub use super::client_ext::{SendRequestExt};
+
 // Re-export HTTP types
 pub use self::http::{uri, HeaderMap, Method, Request, Response, StatusCode, Version};
 

tests/h2-tests/tests/stream_states.rs

@@ -202,16 +202,9 @@ fn errors_if_recv_frame_exceeds_max_frame_size() {
     let (io, mut srv) = mock::new();
 
     let h2 = client::handshake(io).unwrap().and_then(|(mut client, h2)| {
-        let request = Request::builder()
-            .method(Method::GET)
-            .uri("https://example.com/")
-            .body(())
-            .unwrap();
-
         let req = client
-            .send_request(request, true)
+            .get("https://example.com/")
-            .unwrap()
+            .expect("response")
-            .0.unwrap()
             .and_then(|resp| {
                 assert_eq!(resp.status(), StatusCode::OK);
                 let body = resp.into_parts().1;
@@ -261,16 +254,9 @@ fn configure_max_frame_size() {
         .handshake::<_, Bytes>(io)
         .expect("handshake")
         .and_then(|(mut client, h2)| {
-            let request = Request::builder()
-                .method(Method::GET)
-                .uri("https://example.com/")
-                .body(())
-                .unwrap();
-
             let req = client
-                .send_request(request, true)
+                .get("https://example.com/")
-                .unwrap()
+                .expect("response")
-                .0.expect("response")
                 .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     let body = resp.into_parts().1;
@@ -330,15 +316,9 @@ fn recv_goaway_finishes_processed_streams() {
     let h2 = client::handshake(io)
         .expect("handshake")
         .and_then(|(mut client, h2)| {
-            let request = Request::builder()
+            let req1 = client
-                .method(Method::GET)
+                .get("https://example.com")
-                .uri("https://example.com/")
+                .expect("response")
-                .body(())
-                .unwrap();
-
-            let req1 = client.send_request(request, true)
-                .unwrap()
-                .0.expect("response")
                 .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     let body = resp.into_parts().1;
@@ -349,16 +329,10 @@ fn recv_goaway_finishes_processed_streams() {
                     Ok(())
                 });
 
-
             // this request will trigger a goaway
-            let request = Request::builder()
+            let req2 = client
-                .method(Method::GET)
+                .get("https://example.com/")
-                .uri("https://example.com/")
+                .then(|res| {
-                .body(())
-                .unwrap();
-            let req2 = client.send_request(request, true)
-                .unwrap()
-                .0.then(|res| {
                     let err = res.unwrap_err();
                     assert_eq!(err.to_string(), "protocol error: not a result of an error");
                     Ok::<(), ()>(())
@@ -436,15 +410,9 @@ fn skipped_stream_ids_are_implicitly_closed() {
             .handshake::<_, Bytes>(io)
             .expect("handshake")
             .and_then(|(mut client, h2)| {
-                let request = Request::builder()
+                let req = client
-                    .method(Method::GET)
+                    .get("https://example.com/")
-                    .uri("https://example.com/")
+                    .then(|res| {
-                    .body(())
-                    .unwrap();
-
-                let req = client.send_request(request, true)
-                    .unwrap()
-                    .0.then(|res| {
                         let err = res.unwrap_err();
                         assert_eq!(
                             err.to_string(),
@@ -496,15 +464,9 @@ fn send_rst_stream_allows_recv_data() {
     let client = client::handshake(io)
         .expect("handshake")
         .and_then(|(mut client, conn)| {
-            let request = Request::builder()
+            let req = client
-                .method(Method::GET)
+                .get("https://example.com/")
-                .uri("https://example.com/")
+                .expect("response")
-                .body(())
-                .unwrap();
-
-            let req = client.send_request(request, true)
-                .unwrap()
-                .0.expect("response")
                 .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     // drop resp will send a reset
@@ -545,19 +507,12 @@ fn send_rst_stream_allows_recv_trailers() {
     let client = client::handshake(io)
         .expect("handshake")
         .and_then(|(mut client, conn)| {
-            let request = Request::builder()
+            let req = client
-                .method(Method::GET)
+                .get("https://example.com/")
-                .uri("https://example.com/")
+                .expect("response")
-                .body(())
+                .map(|resp| {
-                .unwrap();
-
-            let req = client.send_request(request, true)
-                .unwrap()
-                .0.expect("response")
-                .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     // drop resp will send a reset
-                    Ok(())
                 });
 
             conn.expect("client")
@@ -598,26 +553,18 @@ fn rst_stream_expires() {
         .handshake::<_, Bytes>(io)
         .expect("handshake")
         .and_then(|(mut client, conn)| {
-            let request = Request::builder()
+            let req = client
-                .method(Method::GET)
+                .get("https://example.com/")
-                .uri("https://example.com/")
+                .map(|resp| {
-                .body(())
-                .unwrap();
-
-            let req = client.send_request(request, true)
-                .unwrap()
-                .0.expect("response")
-                .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     // drop resp will send a reset
-                    Ok(())
                 })
-                .map_err(|()| -> Error {
+                .map_err(|e| -> Error {
-                    unreachable!()
+                    unreachable!("req shouldn't error: {:?}", e)
                 });
 
             conn.drive(req)
-                .and_then(|(conn, _)| conn.expect_err("client"))
+                .and_then(|(conn, _)| conn.expect_err("client should error"))
                 .map(|err| {
                     assert_eq!(
                         err.to_string(),
@@ -627,7 +574,6 @@ fn rst_stream_expires() {
                 })
         });
 
-
     client.join(srv).wait().expect("wait");
 }
 
@@ -671,40 +617,24 @@ fn rst_stream_max() {
         .handshake::<_, Bytes>(io)
         .expect("handshake")
         .and_then(|(mut client, conn)| {
-            let request = Request::builder()
+            let req1 = client
-                .method(Method::GET)
+                .get("https://example.com/")
-                .uri("https://example.com/")
+                .map(|resp| {
-                .body(())
-                .unwrap();
-
-            let req1 = client.send_request(request, true)
-                .unwrap()
-                .0.expect("response1")
-                .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     // drop resp will send a reset
-                    Ok(())
                 })
-                .map_err(|()| -> Error {
+                .map_err(|e| -> Error {
-                    unreachable!()
+                    unreachable!("req1 shouldn't error: {:?}", e)
                 });
 
-            let request = Request::builder()
+            let req2 = client
-                .method(Method::GET)
+                .get("https://example.com/")
-                .uri("https://example.com/")
+                .map(|resp| {
-                .body(())
-                .unwrap();
-
-            let req2 = client.send_request(request, true)
-                .unwrap()
-                .0.expect("response2")
-                .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     // drop resp will send a reset
-                    Ok(())
                 })
-                .map_err(|()| -> Error {
+                .map_err(|e| -> Error {
-                    unreachable!()
+                    unreachable!("req2 shouldn't error: {:?}", e)
                 });
 
             conn.drive(req1.join(req2))
@@ -751,15 +681,9 @@ fn reserved_state_recv_window_update() {
     let client = client::handshake(io)
         .expect("handshake")
         .and_then(|(mut client, conn)| {
-            let request = Request::builder()
+            let req = client
-                .method(Method::GET)
+                .get("https://example.com/")
-                .uri("https://example.com/")
+                .expect("response")
-                .body(())
-                .unwrap();
-
-            let req = client.send_request(request, true)
-                .unwrap()
-                .0.expect("response")
                 .and_then(|resp| {
                     assert_eq!(resp.status(), StatusCode::OK);
                     Ok(())
@@ -829,7 +753,7 @@ fn rst_while_closing() {
         .recv_settings()
         .recv_frame(
             frames::headers(1)
-                .request("GET", "https://example.com/")
+                .request("POST", "https://example.com/")
         )
         .send_frame(frames::headers(1).response(200))
         .send_frame(frames::headers(1).eos())
@@ -848,7 +772,7 @@ fn rst_while_closing() {
         .expect("handshake")
         .and_then(|(mut client, conn)| {
             let request = Request::builder()
-                .method(Method::GET)
+                .method(Method::POST)
                 .uri("https://example.com/")
                 .body(())
                 .unwrap();