Skip to content

Commit

Permalink
Use more snapshots for error responses
Browse files Browse the repository at this point in the history
We can hardcode the error messages in our tests but that makes it quite hard to update the error messages, since they have to be updated in multiple places. With insta snapshots they will still have to be updated everywhere but `cargo insta test` can automate this process, which makes it a lot more straight-forward.
  • Loading branch information
Turbo87 committed Sep 24, 2024
1 parent 94ba0c8 commit e599e46
Show file tree
Hide file tree
Showing 18 changed files with 68 additions and 239 deletions.
23 changes: 8 additions & 15 deletions src/tests/account_lock.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{util::RequestHelper, TestApp};
use chrono::{Duration, NaiveDateTime, Utc};
use chrono::{DateTime, Duration, NaiveDateTime, Utc};
use http::StatusCode;
use insta::assert_snapshot;

const URL: &str = "/api/v1/me";
const LOCK_REASON: &str = "test lock reason";
Expand Down Expand Up @@ -28,30 +29,22 @@ async fn account_locked_indefinitely() {

let response = user.get::<()>(URL).await;
assert_eq!(response.status(), StatusCode::FORBIDDEN);

let error_message = format!("This account is indefinitely locked. Reason: {LOCK_REASON}");
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": error_message }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"This account is indefinitely locked. Reason: test lock reason"}]}"#);
}

#[tokio::test(flavor = "multi_thread")]
async fn account_locked_with_future_expiry() {
let until = Utc::now().naive_utc() + Duration::days(1);
let until = "2099-12-12T12:12:12Z"
.parse::<DateTime<Utc>>()
.unwrap()
.naive_utc();

let (app, _anon, user) = TestApp::init().with_user();
lock_account(&app, user.as_model().id, Some(until));

let until = until.format("%Y-%m-%d at %H:%M:%S UTC");
let response = user.get::<()>(URL).await;
assert_eq!(response.status(), StatusCode::FORBIDDEN);

let error_message = format!("This account is locked until {until}. Reason: {LOCK_REASON}");
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": error_message }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"This account is locked until 2099-12-12 at 12:12:12 UTC. Reason: test lock reason"}]}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand Down
11 changes: 3 additions & 8 deletions src/tests/not_found_error.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
use crate::{RequestHelper, TestApp};
use http::StatusCode;
use insta::assert_snapshot;

#[tokio::test(flavor = "multi_thread")]
async fn visiting_unknown_route_returns_404() {
let (_, anon) = TestApp::init().empty();

let response = anon.get::<()>("/does-not-exist").await;
assert_eq!(response.status(), StatusCode::NOT_FOUND);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "Not Found" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Not Found"}]}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -19,8 +17,5 @@ async fn visiting_unknown_api_route_returns_404() {

let response = anon.get::<()>("/api/v1/does-not-exist").await;
assert_eq!(response.status(), StatusCode::NOT_FOUND);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "Not Found" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Not Found"}]}"#);
}
38 changes: 7 additions & 31 deletions src/tests/owners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,7 @@ async fn owners_can_remove_self() {
.remove_named_owner("owners_selfremove", username)
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required." }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required."}]}"#);

create_and_add_owner(&app, &token, "secondowner", &krate).await;

Expand All @@ -200,20 +197,14 @@ async fn owners_can_remove_self() {
.remove_named_owner("owners_selfremove", username)
.await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({ "msg": "owners successfully removed", "ok": true })
);
assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#);

// After you delete yourself, you no longer have permissions to manage the crate.
let response = token
.remove_named_owner("owners_selfremove", username)
.await;
assert_eq!(response.status(), StatusCode::FORBIDDEN);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "only owners have permission to modify owners" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"only owners have permission to modify owners"}]}"#);
}

/// Verify consistency when adidng or removing multiple owners in a single request.
Expand All @@ -235,46 +226,31 @@ async fn modify_multiple_owners() {
.remove_named_owners("owners_multiple", &[username, "user2", "user3"])
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required." }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required."}]}"#);
assert_eq!(app.db(|conn| krate.owners(conn).unwrap()).len(), 3);

// Deleting two owners at once is allowed.
let response = token
.remove_named_owners("owners_multiple", &["user2", "user3"])
.await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({ "msg": "owners successfully removed", "ok": true })
);
assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#);
assert_eq!(app.db(|conn| krate.owners(conn).unwrap()).len(), 1);

// Adding multiple users fails if one of them already is an owner.
let response = token
.add_named_owners("owners_multiple", &["user2", username])
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "`foo` is already an owner" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`foo` is already an owner"}]}"#);
assert_eq!(app.db(|conn| krate.owners(conn).unwrap()).len(), 1);

// Adding multiple users at once succeeds.
let response = token
.add_named_owners("owners_multiple", &["user2", "user3"])
.await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({
"msg": "user user2 has been invited to be an owner of crate owners_multiple,user user3 has been invited to be an owner of crate owners_multiple",
"ok": true,
})
);
assert_snapshot!(response.text(), @r#"{"msg":"user user2 has been invited to be an owner of crate owners_multiple,user user3 has been invited to be an owner of crate owners_multiple","ok":true}"#);

assert_snapshot!(app.emails_snapshot());

Expand Down
7 changes: 2 additions & 5 deletions src/tests/pagination.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::builders::CrateBuilder;
use crate::util::{RequestHelper, TestApp};
use http::status::StatusCode;
use insta::assert_snapshot;
use ipnetwork::IpNetwork;
use serde_json::json;

#[tokio::test(flavor = "multi_thread")]
async fn pagination_blocks_ip_from_cidr_block_list() {
Expand All @@ -24,8 +24,5 @@ async fn pagination_blocks_ip_from_cidr_block_list() {
.get_with_query::<()>("/api/v1/crates", "page=2&per_page=1")
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "Page 2 is unavailable for performance reasons. Please take a look at https://crates.io/data-access for alternatives." }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Page 2 is unavailable for performance reasons. Please take a look at https://crates.io/data-access for alternatives."}]}"#);
}
10 changes: 2 additions & 8 deletions src/tests/routes/crates/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,19 +1045,13 @@ async fn pagination_parameters_only_accept_integers() {
.get_with_query::<()>("/api/v1/crates", "page=1&per_page=100%22%EF%BC%8Cexception")
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "invalid digit found in string" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid digit found in string"}]}"#);

let response = anon
.get_with_query::<()>("/api/v1/crates", "page=100%22%EF%BC%8Cexception&per_page=1")
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "invalid digit found in string" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid digit found in string"}]}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand Down
7 changes: 2 additions & 5 deletions src/tests/routes/crates/new.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::builders::PublishBuilder;
use crate::util::{RequestHelper, TestApp};
use http::StatusCode;
use insta::assert_snapshot;

#[tokio::test(flavor = "multi_thread")]
async fn daily_limit() {
Expand All @@ -15,9 +16,5 @@ async fn daily_limit() {
let crate_to_publish = PublishBuilder::new("foo_daily_limit", "1.0.0");
let response = user.publish_crate(crate_to_publish).await;
assert_eq!(response.status(), StatusCode::TOO_MANY_REQUESTS);
let json = response.json();
assert_eq!(
json["errors"][0]["detail"],
"You have published too many versions of this crate in the last 24 hours"
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"You have published too many versions of this crate in the last 24 hours"}]}"#);
}
52 changes: 8 additions & 44 deletions src/tests/routes/crates/owners/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ async fn owner_change_via_cookie() {
let body = serde_json::to_vec(&body).unwrap();
let response = cookie.put::<()>(&url, body).await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
);
assert_snapshot!(response.text(), @r#"{"msg":"user user-2 has been invited to be an owner of crate foo_crate","ok":true}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -84,10 +81,7 @@ async fn owner_change_via_token() {
let body = serde_json::to_vec(&body).unwrap();
let response = token.put::<()>(&url, body).await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
);
assert_snapshot!(response.text(), @r#"{"msg":"user user-2 has been invited to be an owner of crate foo_crate","ok":true}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -106,10 +100,7 @@ async fn owner_change_via_change_owner_token() {
let body = serde_json::to_vec(&body).unwrap();
let response = token.put::<()>(&url, body).await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
);
assert_snapshot!(response.text(), @r#"{"msg":"user user-2 has been invited to be an owner of crate foo_crate","ok":true}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -129,10 +120,7 @@ async fn owner_change_via_change_owner_token_with_matching_crate_scope() {
let body = serde_json::to_vec(&body).unwrap();
let response = token.put::<()>(&url, body).await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
);
assert_snapshot!(response.text(), @r#"{"msg":"user user-2 has been invited to be an owner of crate foo_crate","ok":true}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand Down Expand Up @@ -249,27 +237,15 @@ async fn invite_already_invited_user() {
// Invite the user the first time
let response = owner.add_named_owner("crate_name", "invited_user").await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({
"msg": "user invited_user has been invited to be an owner of crate crate_name",
"ok": true,
})
);
assert_snapshot!(response.text(), @r#"{"msg":"user invited_user has been invited to be an owner of crate crate_name","ok":true}"#);

// Check one email was sent, this will be the ownership invite email
assert_eq!(app.emails().len(), 1);

// Then invite the user a second time, the message should point out the user is already invited
let response = owner.add_named_owner("crate_name", "invited_user").await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({
"msg": "user invited_user already has a pending invitation to be an owner of crate crate_name",
"ok": true,
})
);
assert_snapshot!(response.text(), @r#"{"msg":"user invited_user already has a pending invitation to be an owner of crate crate_name","ok":true}"#);

// Check that no new email is sent after the second invitation
assert_eq!(app.emails().len(), 1);
Expand All @@ -288,13 +264,7 @@ async fn invite_with_existing_expired_invite() {
// Invite the user the first time
let response = owner.add_named_owner("crate_name", "invited_user").await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({
"msg": "user invited_user has been invited to be an owner of crate crate_name",
"ok": true,
})
);
assert_snapshot!(response.text(), @r#"{"msg":"user invited_user has been invited to be an owner of crate crate_name","ok":true}"#);

// Check one email was sent, this will be the ownership invite email
assert_eq!(app.emails().len(), 1);
Expand All @@ -305,13 +275,7 @@ async fn invite_with_existing_expired_invite() {
// Then invite the user a second time, a new invite is created as the old one expired
let response = owner.add_named_owner("crate_name", "invited_user").await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.json(),
json!({
"msg": "user invited_user has been invited to be an owner of crate crate_name",
"ok": true,
})
);
assert_snapshot!(response.text(), @r#"{"msg":"user invited_user has been invited to be an owner of crate crate_name","ok":true}"#);

// Check that the email for the second invite was sent
assert_eq!(app.emails().len(), 2);
Expand Down
11 changes: 3 additions & 8 deletions src/tests/routes/crates/versions/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::builders::{CrateBuilder, VersionBuilder};
use crate::util::{RequestHelper, TestApp};
use crates_io::views::EncodableDependency;
use http::StatusCode;
use insta::assert_snapshot;

#[derive(Deserialize)]
pub struct Deps {
Expand Down Expand Up @@ -31,17 +32,11 @@ async fn dependencies() {
.get::<()>("/api/v1/crates/missing-crate/1.0.0/dependencies")
.await;
assert_eq!(response.status(), StatusCode::NOT_FOUND);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "crate `missing-crate` does not exist" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"crate `missing-crate` does not exist"}]}"#);

let response = anon
.get::<()>("/api/v1/crates/foo_deps/1.0.2/dependencies")
.await;
assert_eq!(response.status(), StatusCode::NOT_FOUND);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "crate `foo_deps` does not have a version `1.0.2`" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"crate `foo_deps` does not have a version `1.0.2`"}]}"#);
}
6 changes: 2 additions & 4 deletions src/tests/routes/crates/versions/yank_unyank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::builders::{CrateBuilder, PublishBuilder};
use crate::util::{RequestHelper, Response, TestApp};
use crate::OkBool;
use http::StatusCode;
use insta::assert_snapshot;

pub trait YankRequestHelper {
/// Yank the specified version of the specified crate and run all pending background jobs
Expand Down Expand Up @@ -41,10 +42,7 @@ async fn yank_by_a_non_owner_fails() {

let response = token.yank("foo_not", "1.0.0").await;
assert_eq!(response.status(), StatusCode::FORBIDDEN);
assert_eq!(
response.json(),
json!({ "errors": [{ "detail": "must already be an owner to yank or unyank" }] })
);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"must already be an owner to yank or unyank"}]}"#);
}

#[tokio::test(flavor = "multi_thread")]
Expand Down
Loading

0 comments on commit e599e46

Please sign in to comment.