Auto merge of #3097 - Turbo87:errors, r=jtgeibel

tests: Use `assert_eq!()` to check error response payload content

This PR continues #3066 and converts the remaining tests to use the same pattern of using `assert_eq!()` and the `json!()` macro to assert on the exact structure and content of the error responses.

r? `@jtgeibel`

Author: bors

src/tests/account_lock.rs

@@ -26,12 +26,17 @@ fn account_locked_indefinitely() {
     let (app, _anon, user) = TestApp::init().with_user();
     lock_account(&app, user.as_model().id, None);
 
-    user.get::<()>(URL)
-        .bad_with_status(StatusCode::FORBIDDEN)
-        .assert_error(&format!(
-            "This account is indefinitely locked. Reason: {}",
-            LOCK_REASON
-        ));
+    let response = user.get::<()>(URL);
+    response.assert_status(StatusCode::FORBIDDEN);
+
+    let error_message = format!(
+        "This account is indefinitely locked. Reason: {}",
+        LOCK_REASON
+    );
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": error_message }] })
+    );
 }
 
 #[test]
@@ -42,12 +47,17 @@ fn account_locked_with_future_expiry() {
     lock_account(&app, user.as_model().id, Some(until));
 
     let until = until.format("%Y-%m-%d at %H:%M:%S UTC");
-    user.get::<()>(URL)
-        .bad_with_status(StatusCode::FORBIDDEN)
-        .assert_error(&format!(
-            "This account is locked until {}. Reason: {}",
-            until, LOCK_REASON,
-        ));
+    let response = user.get::<()>(URL);
+    response.assert_status(StatusCode::FORBIDDEN);
+
+    let error_message = format!(
+        "This account is locked until {}. Reason: {}",
+        until, LOCK_REASON,
+    );
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": error_message }] })
+    );
 }
 
 #[test]

src/tests/all.rs

@@ -11,7 +11,7 @@ extern crate serde;
 #[macro_use]
 extern crate serde_json;
 
-use crate::util::{Bad, RequestHelper, TestApp};
+use crate::util::{RequestHelper, TestApp};
 use cargo_registry::{
     models::{Crate, CrateOwner, Dependency, NewCategory, NewTeam, NewUser, Team, User, Version},
     schema::crate_owners,
@@ -182,14 +182,6 @@ fn req(method: conduit::Method, path: &str) -> MockRequest {
     request
 }
 
-fn bad_resp(r: &mut AppResponse) -> Option<Bad> {
-    let bad = json::<Bad>(r);
-    if bad.errors.is_empty() {
-        return None;
-    }
-    Some(bad)
-}
-
 fn json<T>(r: &mut AppResponse) -> T
 where
     for<'de> T: serde::Deserialize<'de>,

src/tests/krate/dependencies.rs

@@ -26,6 +26,10 @@ fn dependencies() {
         .good();
     assert_eq!(deps.dependencies[0].crate_id, "bar_deps");
 
-    anon.get::<()>("/api/v1/crates/foo_deps/1.0.2/dependencies")
-        .bad_with_status(StatusCode::OK);
+    let response = anon.get::<()>("/api/v1/crates/foo_deps/1.0.2/dependencies");
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": "crate `foo_deps` does not have a version `1.0.2`" }] })
+    );
 }

src/tests/krate/publish.rs

@@ -86,15 +86,15 @@ fn invalid_names() {
 
     let bad_name = |name: &str, error_message: &str| {
         let crate_to_publish = PublishBuilder::new(name).version("1.0.0");
-        let json = token
-            .enqueue_publish(crate_to_publish)
-            .bad_with_status(StatusCode::OK);
-
-        assert!(
-            json.errors[0].detail.contains(error_message,),
-            "{:?}",
-            json.errors
-        );
+        let response = token.enqueue_publish(crate_to_publish);
+        response.assert_status(StatusCode::OK);
+
+        let json = response.json();
+        let json = json.as_object().unwrap();
+        let errors = json.get("errors").unwrap().as_array().unwrap();
+        let first_error = errors.first().unwrap().as_object().unwrap();
+        let detail = first_error.get("detail").unwrap().as_str().unwrap();
+        assert!(detail.contains(error_message), "{:?}", detail);
     };
 
     let error_message = "expected a valid crate name";
@@ -251,9 +251,13 @@ fn reject_new_krate_with_non_exact_dependency() {
     let crate_to_publish = PublishBuilder::new("new_dep")
         .version("1.0.0")
         .dependency(dependency);
-    token
-        .enqueue_publish(crate_to_publish)
-        .bad_with_status(StatusCode::OK);
+
+    let response = token.enqueue_publish(crate_to_publish);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": "no known crate named `foo_dep`" }] })
+    );
 }
 
 #[test]
@@ -696,19 +700,28 @@ fn bad_keywords() {
     let (_, _, _, token) = TestApp::init().with_token();
     let crate_to_publish =
         PublishBuilder::new("foo_bad_key").keyword("super-long-keyword-name-oh-no");
-    token
-        .enqueue_publish(crate_to_publish)
-        .bad_with_status(StatusCode::OK);
+    let response = token.enqueue_publish(crate_to_publish);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": "invalid upload request: invalid length 29, expected a keyword with less than 20 characters at line 1 column 221" }] })
+    );
 
     let crate_to_publish = PublishBuilder::new("foo_bad_key").keyword("?@?%");
-    token
-        .enqueue_publish(crate_to_publish)
-        .bad_with_status(StatusCode::OK);
+    let response = token.enqueue_publish(crate_to_publish);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": "invalid upload request: invalid value: string \"?@?%\", expected a valid keyword specifier at line 1 column 196" }] })
+    );
 
     let crate_to_publish = PublishBuilder::new("foo_bad_key").keyword("áccênts");
-    token
-        .enqueue_publish(crate_to_publish)
-        .bad_with_status(StatusCode::OK);
+    let response = token.enqueue_publish(crate_to_publish);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": "invalid upload request: invalid value: string \"áccênts\", expected a valid keyword specifier at line 1 column 201" }] })
+    );
 }
 
 #[test]

src/tests/krate/search.rs

@@ -676,19 +676,19 @@ fn pagination_parameters_only_accept_integers() {
         CrateBuilder::new("pagination_links_3", user.id).expect_build(conn);
     });
 
-    let invalid_per_page_json = anon
-        .get_with_query::<()>("/api/v1/crates", "page=1&per_page=100%22%EF%BC%8Cexception")
-        .bad_with_status(StatusCode::BAD_REQUEST);
+    let response =
+        anon.get_with_query::<()>("/api/v1/crates", "page=1&per_page=100%22%EF%BC%8Cexception");
+    response.assert_status(StatusCode::BAD_REQUEST);
     assert_eq!(
-        invalid_per_page_json.errors[0].detail,
-        "invalid digit found in string"
+        response.json(),
+        json!({ "errors": [{ "detail": "invalid digit found in string" }] })
     );
 
-    let invalid_page_json = anon
-        .get_with_query::<()>("/api/v1/crates", "page=100%22%EF%BC%8Cexception&per_page=1")
-        .bad_with_status(StatusCode::BAD_REQUEST);
+    let response =
+        anon.get_with_query::<()>("/api/v1/crates", "page=100%22%EF%BC%8Cexception&per_page=1");
+    response.assert_status(StatusCode::BAD_REQUEST);
     assert_eq!(
-        invalid_page_json.errors[0].detail,
-        "invalid digit found in string"
+        response.json(),
+        json!({ "errors": [{ "detail": "invalid digit found in string" }] })
     );
 }

src/tests/krate/yanking.rs

@@ -109,12 +109,11 @@ fn yank_by_a_non_owner_fails() {
             .expect_build(conn);
     });
 
-    let json = token
-        .yank("foo_not", "1.0.0")
-        .bad_with_status(StatusCode::OK);
+    let response = token.yank("foo_not", "1.0.0");
+    response.assert_status(StatusCode::OK);
     assert_eq!(
-        json.errors[0].detail,
-        "must already be an owner to yank or unyank"
+        response.json(),
+        json!({ "errors": [{ "detail": "must already be an owner to yank or unyank" }] })
     );
 }
 

src/tests/owners.rs

@@ -135,28 +135,30 @@ fn owners_can_remove_self() {
         .db(|conn| CrateBuilder::new("owners_selfremove", user.as_model().id).expect_build(conn));
 
     // Deleting yourself when you're the only owner isn't allowed.
-    let json = token
-        .remove_named_owner("owners_selfremove", username)
-        .bad_with_status(StatusCode::OK);
-    assert!(json.errors[0]
-        .detail
-        .contains("cannot remove all individual owners of a crate"));
+    let response = token.remove_named_owner("owners_selfremove", username);
+    response.assert_status(StatusCode::OK);
+    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." }] })
+    );
 
     create_and_add_owner(&app, &token, "secondowner", &krate);
 
     // Deleting yourself when there are other owners is allowed.
-    let json = token
-        .remove_named_owner("owners_selfremove", username)
-        .good();
-    assert!(json.ok);
+    let response = token.remove_named_owner("owners_selfremove", username);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "msg": "owners successfully removed", "ok": true })
+    );
 
     // After you delete yourself, you no longer have permisions to manage the crate.
-    let json = token
-        .remove_named_owner("owners_selfremove", username)
-        .bad_with_status(StatusCode::OK);
-    assert!(json.errors[0]
-        .detail
-        .contains("only owners have permission to modify owners",));
+    let response = token.remove_named_owner("owners_selfremove", username);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "errors": [{ "detail": "only owners have permission to modify owners" }] })
+    );
 }
 
 // Verify consistency when adidng or removing multiple owners in a single request.
@@ -172,35 +174,46 @@ fn modify_multiple_owners() {
     let user3 = create_and_add_owner(&app, &token, "user3", &krate);
 
     // Deleting all owners is not allowed.
-    let json = token
-        .remove_named_owners("owners_multiple", &[username, "user2", "user3"])
-        .bad_with_status(StatusCode::OK);
-    assert!(&json.errors[0]
-        .detail
-        .contains("cannot remove all individual owners of a crate"));
+    let response = token.remove_named_owners("owners_multiple", &[username, "user2", "user3"]);
+    response.assert_status(StatusCode::OK);
+    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_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 3);
 
     // Deleting two owners at once is allowed.
-    let json = token
-        .remove_named_owners("owners_multiple", &["user2", "user3"])
-        .good();
-    assert!(json.ok);
+    let response = token.remove_named_owners("owners_multiple", &["user2", "user3"]);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "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 json = token
-        .add_named_owners("owners_multiple", &["user2", username])
-        .bad_with_status(StatusCode::OK);
-    assert!(&json.errors[0].detail.contains("is already an owner"));
+    let response = token.add_named_owners("owners_multiple", &["user2", username]);
+    response.assert_status(StatusCode::OK);
+    assert_eq!(
+        response.json(),
+        json!({ "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 json = token
-        .add_named_owners("owners_multiple", &["user2", "user3"])
-        .good();
-    assert!(json.ok);
+    let response = token.add_named_owners("owners_multiple", &["user2", "user3"]);
+    response.assert_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,
+        })
+    );
+
     user2.accept_ownership_invitation(&krate.name, krate.id);
     user3.accept_ownership_invitation(&krate.name, krate.id);
+
     assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 3);
 }