From 8c8598b4fd1ece7d5ba959f2a352b2271ffd3336 Mon Sep 17 00:00:00 2001 From: Jeb Rosen Date: Wed, 14 Aug 2019 11:29:55 -0700 Subject: [PATCH] Fix 'static_files' and 'serve' tests. --- contrib/lib/tests/static_files.rs | 92 +++++++++++++++++------------- core/lib/src/local/request.rs | 22 +++---- examples/static_files/src/main.rs | 2 + examples/static_files/src/tests.rs | 34 ++++++----- scripts/test.sh | 8 +-- 5 files changed, 89 insertions(+), 69 deletions(-) diff --git a/contrib/lib/tests/static_files.rs b/contrib/lib/tests/static_files.rs index 0d290ac1..1986c9c2 100644 --- a/contrib/lib/tests/static_files.rs +++ b/contrib/lib/tests/static_files.rs @@ -45,7 +45,7 @@ mod static_tests { "inner/", ]; - fn assert_file(client: &Client, prefix: &str, path: &str, exists: bool) { + async fn assert_file(client: &Client, prefix: &str, path: &str, exists: bool) { let full_path = format!("/{}/{}", prefix, path); let mut response = client.get(full_path).dispatch(); if exists { @@ -59,50 +59,60 @@ mod static_tests { let mut file = File::open(path).expect("open file"); let mut expected_contents = String::new(); file.read_to_string(&mut expected_contents).expect("read file"); - assert_eq!(response.body_string_wait(), Some(expected_contents)); + assert_eq!(response.body_string().await, Some(expected_contents)); } else { assert_eq!(response.status(), Status::NotFound); } } - fn assert_all(client: &Client, prefix: &str, paths: &[&str], exist: bool) { - paths.iter().for_each(|path| assert_file(client, prefix, path, exist)) + async fn assert_all(client: &Client, prefix: &str, paths: &[&str], exist: bool) { + for path in paths.iter() { + assert_file(client, prefix, path, exist).await; + } } #[test] fn test_static_no_index() { - let client = Client::new(rocket()).expect("valid rocket"); - assert_all(&client, "no_index", REGULAR_FILES, true); - assert_all(&client, "no_index", HIDDEN_FILES, false); - assert_all(&client, "no_index", INDEXED_DIRECTORIES, false); + rocket::async_test(async { + let client = Client::new(rocket()).expect("valid rocket"); + assert_all(&client, "no_index", REGULAR_FILES, true).await; + assert_all(&client, "no_index", HIDDEN_FILES, false).await; + assert_all(&client, "no_index", INDEXED_DIRECTORIES, false).await; + }) } #[test] fn test_static_hidden() { - let client = Client::new(rocket()).expect("valid rocket"); - assert_all(&client, "dots", REGULAR_FILES, true); - assert_all(&client, "dots", HIDDEN_FILES, true); - assert_all(&client, "dots", INDEXED_DIRECTORIES, false); + rocket::async_test(async { + let client = Client::new(rocket()).expect("valid rocket"); + assert_all(&client, "dots", REGULAR_FILES, true).await; + assert_all(&client, "dots", HIDDEN_FILES, true).await; + assert_all(&client, "dots", INDEXED_DIRECTORIES, false).await; + }) } #[test] fn test_static_index() { - let client = Client::new(rocket()).expect("valid rocket"); - assert_all(&client, "index", REGULAR_FILES, true); - assert_all(&client, "index", HIDDEN_FILES, false); - assert_all(&client, "index", INDEXED_DIRECTORIES, true); + rocket::async_test(async { + let client = Client::new(rocket()).expect("valid rocket"); + assert_all(&client, "index", REGULAR_FILES, true).await; + assert_all(&client, "index", HIDDEN_FILES, false).await; + assert_all(&client, "index", INDEXED_DIRECTORIES, true).await; - assert_all(&client, "default", REGULAR_FILES, true); - assert_all(&client, "default", HIDDEN_FILES, false); - assert_all(&client, "default", INDEXED_DIRECTORIES, true); + assert_all(&client, "default", REGULAR_FILES, true).await; + assert_all(&client, "default", HIDDEN_FILES, false).await; + assert_all(&client, "default", INDEXED_DIRECTORIES, true).await; + }) } #[test] fn test_static_all() { - let client = Client::new(rocket()).expect("valid rocket"); - assert_all(&client, "both", REGULAR_FILES, true); - assert_all(&client, "both", HIDDEN_FILES, true); - assert_all(&client, "both", INDEXED_DIRECTORIES, true); + rocket::async_test(async { + let client = Client::new(rocket()).expect("valid rocket"); + assert_all(&client, "both", REGULAR_FILES, true).await; + assert_all(&client, "both", HIDDEN_FILES, true).await; + assert_all(&client, "both", INDEXED_DIRECTORIES, true).await; + }) } #[test] @@ -121,29 +131,31 @@ mod static_tests { #[test] fn test_forwarding() { - use rocket::http::RawStr; - use rocket::{get, routes}; + rocket::async_test(async { + use rocket::http::RawStr; + use rocket::{get, routes}; - #[get("/", rank = 20)] - fn catch_one(value: String) -> String { value } + #[get("/", rank = 20)] + fn catch_one(value: String) -> String { value } - #[get("//", rank = 20)] - fn catch_two(a: &RawStr, b: &RawStr) -> String { format!("{}/{}", a, b) } + #[get("//", rank = 20)] + fn catch_two(a: &RawStr, b: &RawStr) -> String { format!("{}/{}", a, b) } - let rocket = rocket().mount("/default", routes![catch_one, catch_two]); - let client = Client::new(rocket).expect("valid rocket"); + let rocket = rocket().mount("/default", routes![catch_one, catch_two]); + let client = Client::new(rocket).expect("valid rocket"); - let mut response = client.get("/default/ireallydontexist").dispatch(); - assert_eq!(response.status(), Status::Ok); - assert_eq!(response.body_string_wait().unwrap(), "ireallydontexist"); + let mut response = client.get("/default/ireallydontexist").dispatch(); + assert_eq!(response.status(), Status::Ok); + assert_eq!(response.body_string().await.unwrap(), "ireallydontexist"); - let mut response = client.get("/default/idont/exist").dispatch(); - assert_eq!(response.status(), Status::Ok); - assert_eq!(response.body_string_wait().unwrap(), "idont/exist"); + let mut response = client.get("/default/idont/exist").dispatch(); + assert_eq!(response.status(), Status::Ok); + assert_eq!(response.body_string().await.unwrap(), "idont/exist"); - assert_all(&client, "both", REGULAR_FILES, true); - assert_all(&client, "both", HIDDEN_FILES, true); - assert_all(&client, "both", INDEXED_DIRECTORIES, true); + assert_all(&client, "both", REGULAR_FILES, true).await; + assert_all(&client, "both", HIDDEN_FILES, true).await; + assert_all(&client, "both", INDEXED_DIRECTORIES, true).await; + }) } #[test] diff --git a/core/lib/src/local/request.rs b/core/lib/src/local/request.rs index 4b545f42..f45cf87c 100644 --- a/core/lib/src/local/request.rs +++ b/core/lib/src/local/request.rs @@ -1,5 +1,5 @@ use std::fmt; -use std::rc::Rc; +use std::sync::Arc; use std::net::SocketAddr; use std::ops::{Deref, DerefMut}; use std::borrow::Cow; @@ -67,16 +67,16 @@ use crate::local::Client; /// [`mut_dispatch`]: #method.mut_dispatch pub struct LocalRequest<'c> { client: &'c Client, - // This pointer exists to access the `Rc` mutably inside of + // This pointer exists to access the `Arc` mutably inside of // `LocalRequest`. This is the only place that a `Request` can be accessed // mutably. This is accomplished via the private `request_mut()` method. ptr: *mut Request<'c>, - // This `Rc` exists so that we can transfer ownership to the `LocalResponse` + // This `Arc` exists so that we can transfer ownership to the `LocalResponse` // selectively on dispatch. This is necessary because responses may point // into the request, and thus the request and all of its data needs to be // alive while the response is accessible. // - // Because both a `LocalRequest` and a `LocalResponse` can hold an `Rc` to + // Because both a `LocalRequest` and a `LocalResponse` can hold an `Arc` to // the same `Request`, _and_ the `LocalRequest` can mutate the request, we // must ensure that 1) neither `LocalRequest` not `LocalResponse` are `Sync` // or `Send` and 2) mutations carried out in `LocalRequest` are _stable_: @@ -85,7 +85,7 @@ pub struct LocalRequest<'c> { // even if the `Request` is mutated by a `LocalRequest`, those mutations are // not observable by `LocalResponse`. // - // The first is ensured by the embedding of the `Rc` type which is neither + // The first is ensured by the embedding of the `Arc` type which is neither // `Send` nor `Sync`. The second is more difficult to argue. First, observe // that any methods of `LocalRequest` that _remove_ values from `Request` // only remove _Copy_ values, in particular, `SocketAddr`. Second, the @@ -94,7 +94,7 @@ pub struct LocalRequest<'c> { // `Response`. And finally, observe how all of the data stored in `Request` // is converted into its owned counterpart before insertion, ensuring stable // addresses. Together, these properties guarantee the second condition. - request: Rc>, + request: Arc>, data: Vec, uri: Cow<'c, str>, } @@ -118,8 +118,8 @@ impl<'c> LocalRequest<'c> { } // See the comments on the structure for what's going on here. - let mut request = Rc::new(request); - let ptr = Rc::get_mut(&mut request).unwrap() as *mut Request<'_>; + let mut request = Arc::new(request); + let ptr = Arc::get_mut(&mut request).unwrap() as *mut Request<'_>; LocalRequest { client, ptr, request, uri, data: vec![] } } @@ -150,7 +150,7 @@ impl<'c> LocalRequest<'c> { fn long_lived_request<'a>(&mut self) -> &'a mut Request<'c> { // See the comments in the structure for the argument of correctness. // Additionally, the caller must ensure that the owned instance of - // `Rc` remains valid as long as the returned reference can be + // `Arc` remains valid as long as the returned reference can be // accessed. unsafe { &mut *self.ptr } } @@ -393,7 +393,7 @@ impl<'c> LocalRequest<'c> { fn _dispatch( client: &'c Client, request: &'c mut Request<'c>, - owned_request: Rc>, + owned_request: Arc>, uri: &str, data: Vec ) -> LocalResponse<'c> { @@ -454,7 +454,7 @@ impl fmt::Debug for LocalRequest<'_> { /// when invoking methods, a `LocalResponse` can be treated exactly as if it /// were a `Response`. pub struct LocalResponse<'c> { - _request: Rc>, + _request: Arc>, response: Response<'c>, } diff --git a/examples/static_files/src/main.rs b/examples/static_files/src/main.rs index ce4a1776..14991223 100644 --- a/examples/static_files/src/main.rs +++ b/examples/static_files/src/main.rs @@ -1,3 +1,5 @@ +#![feature(async_await)] + extern crate rocket; extern crate rocket_contrib; diff --git a/examples/static_files/src/tests.rs b/examples/static_files/src/tests.rs index e61c3785..30aa9dba 100644 --- a/examples/static_files/src/tests.rs +++ b/examples/static_files/src/tests.rs @@ -6,14 +6,14 @@ use rocket::http::Status; use super::rocket; -fn test_query_file (path: &str, file: T, status: Status) +async fn test_query_file (path: &str, file: T, status: Status) where T: Into> { let client = Client::new(rocket()).unwrap(); let mut response = client.get(path).dispatch(); assert_eq!(response.status(), status); - let body_data = response.body_bytes_wait(); + let body_data = response.body_bytes().await; if let Some(filename) = file.into() { let expected_data = read_file_content(filename); assert!(body_data.map_or(false, |s| s == expected_data)); @@ -30,27 +30,35 @@ fn read_file_content(path: &str) -> Vec { #[test] fn test_index_html() { - test_query_file("/", "static/index.html", Status::Ok); - test_query_file("/?v=1", "static/index.html", Status::Ok); - test_query_file("/?this=should&be=ignored", "static/index.html", Status::Ok); + rocket::async_test(async { + test_query_file("/", "static/index.html", Status::Ok).await; + test_query_file("/?v=1", "static/index.html", Status::Ok).await; + test_query_file("/?this=should&be=ignored", "static/index.html", Status::Ok).await; + }) } #[test] fn test_hidden_file() { - test_query_file("/hidden/hi.txt", "static/hidden/hi.txt", Status::Ok); - test_query_file("/hidden/hi.txt?v=1", "static/hidden/hi.txt", Status::Ok); - test_query_file("/hidden/hi.txt?v=1&a=b", "static/hidden/hi.txt", Status::Ok); + rocket::async_test(async { + test_query_file("/hidden/hi.txt", "static/hidden/hi.txt", Status::Ok).await; + test_query_file("/hidden/hi.txt?v=1", "static/hidden/hi.txt", Status::Ok).await; + test_query_file("/hidden/hi.txt?v=1&a=b", "static/hidden/hi.txt", Status::Ok).await; + }) } #[test] fn test_icon_file() { - test_query_file("/rocket-icon.jpg", "static/rocket-icon.jpg", Status::Ok); - test_query_file("/rocket-icon.jpg", "static/rocket-icon.jpg", Status::Ok); + rocket::async_test(async { + test_query_file("/rocket-icon.jpg", "static/rocket-icon.jpg", Status::Ok).await; + test_query_file("/rocket-icon.jpg", "static/rocket-icon.jpg", Status::Ok).await; + }) } #[test] fn test_invalid_path() { - test_query_file("/thou_shalt_not_exist", None, Status::NotFound); - test_query_file("/thou/shalt/not/exist", None, Status::NotFound); - test_query_file("/thou/shalt/not/exist?a=b&c=d", None, Status::NotFound); + rocket::async_test(async { + test_query_file("/thou_shalt_not_exist", None, Status::NotFound).await; + test_query_file("/thou/shalt/not/exist", None, Status::NotFound).await; + test_query_file("/thou/shalt/not/exist?a=b&c=d", None, Status::NotFound).await; + }) } diff --git a/scripts/test.sh b/scripts/test.sh index c4d539ad..06c85c82 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -67,8 +67,7 @@ if [ "$1" = "--contrib" ]; then msgpack tera_templates handlebars_templates -# TODO.async: serve needs tests to use tokio runtime, blocked on #1071 -# serve + serve helmet diesel_postgres_pool diesel_sqlite_pool @@ -87,9 +86,8 @@ if [ "$1" = "--contrib" ]; then pushd "${CONTRIB_LIB_ROOT}" > /dev/null 2>&1 -# TODO.async: default_features includes `serve` -# echo ":: Building and testing contrib [default]..." -# CARGO_INCREMENTAL=0 cargo test + echo ":: Building and testing contrib [default]..." + CARGO_INCREMENTAL=0 cargo test for feature in "${FEATURES[@]}"; do echo ":: Building and testing contrib [${feature}]..."