From 3e345ac7b68bce8a165c2468827fe40ca359d1a2 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 26 Jun 2019 12:13:02 -0700 Subject: [PATCH] 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 | 30 +++++ tests/h2-support/src/future_ext.rs | 13 +-- tests/h2-support/src/lib.rs | 2 + tests/h2-support/src/mock.rs | 34 +++--- tests/h2-support/src/mod.rs | 69 ----------- tests/h2-support/src/prelude.rs | 3 + tests/h2-tests/tests/stream_states.rs | 158 +++++++------------------- 7 files changed, 95 insertions(+), 214 deletions(-) create mode 100644 tests/h2-support/src/client_ext.rs delete mode 100644 tests/h2-support/src/mod.rs diff --git a/tests/h2-support/src/client_ext.rs b/tests/h2-support/src/client_ext.rs new file mode 100644 index 0000000..9a4d6f9 --- /dev/null +++ b/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 SendRequestExt for SendRequest +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 + } +} diff --git a/tests/h2-support/src/future_ext.rs b/tests/h2-support/src/future_ext.rs index afa216f..f08710d 100644 --- a/tests/h2-support/src/future_ext.rs +++ b/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> + fn yield_once(self) -> Box> where Self: Future + Sized + 'static, { - let mut ready = false; - Box::new(::futures::future::poll_fn(move || { - if ready { - Ok::<_, ()>(().into()) - } else { - ready = true; - ::futures::task::current().notify(); - Ok(::futures::Async::NotReady) - } - }).then(|_| self)) + Box::new(super::util::yield_once().then(move |_| self)) } } diff --git a/tests/h2-support/src/lib.rs b/tests/h2-support/src/lib.rs index 036637f..b995392 100644 --- a/tests/h2-support/src/lib.rs +++ b/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; diff --git a/tests/h2-support/src/mock.rs b/tests/h2-support/src/mock.rs index a1bf9e5..6f872cb 100644 --- a/tests/h2-support/src/mock.rs +++ b/tests/h2-support/src/mock.rs @@ -116,7 +116,7 @@ impl Handle { } /// Read the client preface - pub fn read_preface(self) -> Box> { + pub fn read_preface(self) -> Box> { 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> { + ) -> Box> { self.assert_client_handshake_with_settings(frame::Settings::default()) } @@ -137,7 +137,7 @@ impl Handle { pub fn assert_client_handshake_with_settings( mut self, settings: T, - ) -> Box> + ) -> Box> where T: Into, { @@ -189,7 +189,7 @@ impl Handle { /// Perform the H2 handshake pub fn assert_server_handshake( self, - ) -> Box> { + ) -> Box> { self.assert_server_handshake_with_settings(frame::Settings::default()) } @@ -197,7 +197,7 @@ impl Handle { pub fn assert_server_handshake_with_settings( mut self, settings: T, - ) -> Box> + ) -> Box> where T: Into, { @@ -433,7 +433,7 @@ impl AsyncWrite for Pipe { pub trait HandleFutureExt { fn recv_settings(self) - -> RecvFrame, Handle), Error = ()>>> + -> RecvFrame, Handle), Error = ()>>> where Self: Sized + 'static, Self: Future, @@ -443,7 +443,7 @@ pub trait HandleFutureExt { } fn recv_custom_settings(self, settings: T) - -> RecvFrame, Handle), Error = ()>>> + -> RecvFrame, Handle), Error = ()>>> where Self: Sized + 'static, Self: Future, @@ -454,7 +454,7 @@ pub trait HandleFutureExt { .map(|(settings, handle)| (Some(settings.into()), handle)) .unwrap(); - let boxed: Box, Handle), Error = ()>> = + let boxed: Box, Handle), Error = ()>> = Box::new(map); RecvFrame { inner: boxed, @@ -462,7 +462,7 @@ pub trait HandleFutureExt { } } - fn ignore_settings(self) -> Box> + fn ignore_settings(self) -> Box> where Self: Sized + 'static, Self: Future, @@ -497,7 +497,7 @@ pub trait HandleFutureExt { } } - fn send_bytes(self, data: &[u8]) -> Box> + fn send_bytes(self, data: &[u8]) -> Box> where Self: Future + 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> + fn idle_ms(self, ms: usize) -> Box> where Self: Sized + 'static, Self: Future, @@ -562,7 +562,7 @@ pub trait HandleFutureExt { })) } - fn buffer_bytes(self, num: usize) -> Box> + fn buffer_bytes(self, num: usize) -> Box> where Self: Sized + 'static, Self: Future, Self::Error: fmt::Debug, @@ -596,7 +596,7 @@ pub trait HandleFutureExt { })) } - fn unbounded_bytes(self) -> Box> + fn unbounded_bytes(self) -> Box> where Self: Sized + 'static, Self: Future, Self::Error: fmt::Debug, @@ -615,7 +615,7 @@ pub trait HandleFutureExt { })) } - fn then_notify(self, tx: oneshot::Sender<()>) -> Box> + fn then_notify(self, tx: oneshot::Sender<()>) -> Box> where Self: Sized + 'static, Self: Future, Self::Error: fmt::Debug, @@ -626,7 +626,7 @@ pub trait HandleFutureExt { })) } - fn wait_for(self, other: F) -> Box> + fn wait_for(self, other: F) -> Box> where F: Future + 'static, Self: Future + Sized + 'static @@ -636,7 +636,7 @@ pub trait HandleFutureExt { })) } - fn close(self) -> Box> + fn close(self) -> Box> where Self: Future + Sized + 'static, { @@ -752,7 +752,7 @@ where T: Future + 'static, T::Error: fmt::Debug, { - type Future = Box, Handle), Error = ()>>; + type Future = Box, Handle), Error = ()>>; fn into_recv_frame(self, frame: Option) -> RecvFrame { let into_fut = Box::new( diff --git a/tests/h2-support/src/mod.rs b/tests/h2-support/src/mod.rs deleted file mode 100644 index 0924afc..0000000 --- a/tests/h2-support/src/mod.rs +++ /dev/null @@ -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 = h2::Codec>; - -// This is the frame type that is sent -pub type SendFrame = h2::frame::Frame<::std::io::Cursor<::bytes::Bytes>>; diff --git a/tests/h2-support/src/prelude.rs b/tests/h2-support/src/prelude.rs index 993b2e1..11d6066 100644 --- a/tests/h2-support/src/prelude.rs +++ b/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}; diff --git a/tests/h2-tests/tests/stream_states.rs b/tests/h2-tests/tests/stream_states.rs index f22f101..ea02280 100644 --- a/tests/h2-tests/tests/stream_states.rs +++ b/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) - .unwrap() - .0.unwrap() + .get("https://example.com/") + .expect("response") .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) - .unwrap() - .0.expect("response") + .get("https://example.com/") + .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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req1 = client.send_request(request, true) - .unwrap() - .0.expect("response") + let req1 = client + .get("https://example.com") + .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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - let req2 = client.send_request(request, true) - .unwrap() - .0.then(|res| { + let req2 = client + .get("https://example.com/") + .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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req = client.send_request(request, true) - .unwrap() - .0.then(|res| { + let req = client + .get("https://example.com/") + .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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req = client.send_request(request, true) - .unwrap() - .0.expect("response") + let req = client + .get("https://example.com/") + .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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req = client.send_request(request, true) - .unwrap() - .0.expect("response") - .and_then(|resp| { + let req = client + .get("https://example.com/") + .expect("response") + .map(|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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req = client.send_request(request, true) - .unwrap() - .0.expect("response") - .and_then(|resp| { + let req = client + .get("https://example.com/") + .map(|resp| { assert_eq!(resp.status(), StatusCode::OK); // drop resp will send a reset - Ok(()) }) - .map_err(|()| -> Error { - unreachable!() + .map_err(|e| -> Error { + 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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req1 = client.send_request(request, true) - .unwrap() - .0.expect("response1") - .and_then(|resp| { + let req1 = client + .get("https://example.com/") + .map(|resp| { assert_eq!(resp.status(), StatusCode::OK); // drop resp will send a reset - Ok(()) }) - .map_err(|()| -> Error { - unreachable!() + .map_err(|e| -> Error { + unreachable!("req1 shouldn't error: {:?}", e) }); - let request = Request::builder() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req2 = client.send_request(request, true) - .unwrap() - .0.expect("response2") - .and_then(|resp| { + let req2 = client + .get("https://example.com/") + .map(|resp| { assert_eq!(resp.status(), StatusCode::OK); // drop resp will send a reset - Ok(()) }) - .map_err(|()| -> Error { - unreachable!() + .map_err(|e| -> Error { + 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() - .method(Method::GET) - .uri("https://example.com/") - .body(()) - .unwrap(); - - let req = client.send_request(request, true) - .unwrap() - .0.expect("response") + let req = client + .get("https://example.com/") + .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();