Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Add additional methods to RequestResult #28

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/user_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ impl Database {
fn delete_route(self) -> impl Filter<Extract = (impl Reply,), Error = Rejection> + Clone {
method::delete().and(path::param()).map(move |id| {
match self.users.lock().unwrap().remove(&id) {
Some(_) => with_status(json(&"User deleted"), StatusCode::OK),
None => with_status(json(&"Failed to delete user"), StatusCode::NOT_FOUND),
Some(_) => StatusCode::OK,
None => StatusCode::NOT_FOUND,
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions examples/user_server_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,15 @@ pub async fn delete_user() {
CONTEXT
.run(&request)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we try to use each new request at least once (so we can cover them a bit on "real life" tests)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use each new request at least once

wdym? the request here (l. 178)

let request = Request::delete(path!["users", id]);

is used twice (l. 181 & l. 189)

Copy link
Member

@cbonaudo cbonaudo Jul 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I said request, I meant methods sorry (like deserialize_body, ensure_status_ignore_body, ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh yes, of course, I'll add test cases for those!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.await
.expect_status::<String>(StatusCode::OK)
.expect_status_empty_body(StatusCode::OK)
.await;

// We try to delete the same user again and ensure that the server
// returns a 404 status code.
CONTEXT
.run(request)
.await
.expect_status::<String>(StatusCode::NOT_FOUND)
.expect_status_empty_body(StatusCode::NOT_FOUND)
.await;
}

Expand Down
128 changes: 121 additions & 7 deletions src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,53 @@ impl RequestResult {
///
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate this api because it allows us to use a basic command (ensure_status) to perform the meat of most of our needs, and have more specific commands.
Maybe it can mean that ensure_status could be renamed to reflect the 2 things it does, maybe it's not needed, not sure about that.

/// # Error
///
/// This method return an error if the server response status is not equal to
/// This method returns an error if the server response status is not equal to
/// `status` or if the body can not be deserialized to the specified type.
#[track_caller]
pub async fn ensure_status<T>(self, status: StatusCode) -> Result<T, String>
where
T: DeserializeOwned,
{
self.ensure_status_ignore_body(status)
.await?
.deserialize_body()
.await
}

/// Deserializes the body of the response into a concrete type.
///
/// This method uses `serde` internally, so the output type must implement
/// [`DeserializeOwned`].
///
/// # Error
///
/// This method returns an error if the body can not be deserialized to the
/// specified type.
#[track_caller]
pub async fn deserialize_body<T>(self) -> Result<T, String>
where
T: DeserializeOwned,
{
self.response.json().await.map_err(|err| {
format!(
"Failed to deserialize body for request '{}': {}",
self.context_description, err
)
})
}

/// Checks if the response status meets an expected status code. No check is
/// performed on the body.
///
/// # Error
///
/// This method returns an error if the server response status is not equal to
/// `status`.
#[track_caller]
pub async fn ensure_status_ignore_body(
self,
status: StatusCode,
) -> Result<RequestResult, String> {
if self.response.status() != status {
return Err(format!("Unexpected server response code for request '{}'. Body is {}",
self.context_description,
Expand All @@ -321,11 +361,85 @@ impl RequestResult {
)?));
}

self.response.json().await.map_err(|err| {
format!(
"Failed to deserialize body for request '{}': {}",
self.context_description, err
)
})
Ok(self)
}

/// Checks if the response status meets an expected status code. No check is
/// performed on the body.
///
/// # Panics
///
/// This method panics if the server response status is not equal to `status`.
#[track_caller]
pub async fn expect_status_ignore_body(self, status: StatusCode) {
match self.ensure_status_ignore_body(status).await {
Ok(_) => {}
Err(err) => panic!("{}", err),
}
}

/// Checks if the response body is empty.
///
/// The `Content-length` header is first checked; if it is missing (e.g. in case of
/// a compressed payload or a chunked transfer encoding), the body is fetched and its
/// length is checked.
///
/// # Error
///
/// This method returns an error if the body could not be checked to be empty.
#[track_caller]
pub async fn ensure_empty_body(self) -> Result<(), String> {
if matches!(self.response.content_length(), Some(0))
|| self
.response
.text()
.await
.map_err(|err| {
format!(
"Failed to check body for request '{}': {}",
self.context_description, err
)
})?
.is_empty()
{
Ok(())
} else {
Err(format!(
"Unexpected body for request '{}'",
self.context_description
))
}
}

/// Checks if the response body meets an expected status code and the body is empty.
///
/// See `ensure_empty_body` for more details.
///
/// # Error
///
/// This method returns an error if the server response status is not equal to
/// `status` or if the body could not be checked to be empty.
#[track_caller]
pub async fn ensure_status_empty_body(self, status: StatusCode) -> Result<(), String> {
self.ensure_status_ignore_body(status)
.await?
.ensure_empty_body()
.await
}

/// Checks if the response body meets an expected status code and the body is empty.
///
/// See `ensure_empty_body` for more details.
///
/// # Panics
///
/// This method panics if the server response status is not equal to `status` or if the
/// body could not be checked to be empty.
#[track_caller]
pub async fn expect_status_empty_body(self, status: StatusCode) {
match self.ensure_status_empty_body(status).await {
Ok(()) => (),
Err(err) => panic!("{}", err),
}
}
}