Auto merge of #1760 - integer32llc:finish-updating-tests, r=jtgeibel

Finish updating tests

This converts the remaining tests to the newer test API, so that we can continue to move forward with improvements without some of the tests lagging behind.

I do think there's more to be done to make the tests easier to read, write, and debug, and I am introducing some duplication and abstractions I'm not happy with. Next pass though.

Author: bors

src/tests/all.rs

@@ -17,11 +17,13 @@ extern crate serde_json;
 
 use crate::util::{Bad, RequestHelper, TestApp};
 use cargo_registry::{
-    middleware::current_user::AuthenticationSource,
     models::{Crate, CrateOwner, Dependency, NewCategory, NewTeam, NewUser, Team, User, Version},
     schema::crate_owners,
     util::CargoResult,
-    views::{EncodableCrate, EncodableKeyword, EncodableOwner, EncodableVersion, GoodCrate},
+    views::{
+        EncodableCategory, EncodableCategoryWithSubcategories, EncodableCrate, EncodableKeyword,
+        EncodableOwner, EncodableVersion, GoodCrate,
+    },
     App, Config, Env, Replica, Uploader,
 };
 use std::{
@@ -32,7 +34,6 @@ use std::{
     },
 };
 
-use conduit::Request;
 use conduit_test::MockRequest;
 use diesel::prelude::*;
 use reqwest::{Client, Proxy};
@@ -47,26 +48,6 @@ macro_rules! t {
     };
 }
 
-macro_rules! ok_resp {
-    ($e:expr) => {{
-        let resp = t!($e);
-        if !crate::ok_resp(&resp) {
-            panic!("bad response: {:?}", resp.status);
-        }
-        resp
-    }};
-}
-
-macro_rules! bad_resp {
-    ($e:expr) => {{
-        let mut resp = t!($e);
-        match crate::bad_resp(&mut resp) {
-            None => panic!("ok response: {:?}", resp.status),
-            Some(b) => b,
-        }
-    }};
-}
-
 mod badge;
 mod builders;
 mod categories;
@@ -110,6 +91,23 @@ pub struct OwnerTeamsResponse {
     teams: Vec<EncodableOwner>,
 }
 #[derive(Deserialize)]
+pub struct OwnersResponse {
+    users: Vec<EncodableOwner>,
+}
+#[derive(Deserialize)]
+pub struct CategoryResponse {
+    category: EncodableCategoryWithSubcategories,
+}
+#[derive(Deserialize)]
+pub struct CategoryListResponse {
+    categories: Vec<EncodableCategory>,
+    meta: CategoryMeta,
+}
+#[derive(Deserialize)]
+pub struct CategoryMeta {
+    total: i32,
+}
+#[derive(Deserialize)]
 pub struct OkBool {
     ok: bool,
 }
@@ -254,12 +252,6 @@ fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) ->
     Ok(())
 }
 
-fn sign_in_as(req: &mut dyn Request, user: &User) {
-    req.mut_extensions().insert(user.clone());
-    req.mut_extensions()
-        .insert(AuthenticationSource::SessionCookie);
-}
-
 fn new_dependency(conn: &PgConnection, version: &Version, krate: &Crate) -> Dependency {
     use cargo_registry::schema::dependencies::dsl::*;
     use diesel::insert_into;
@@ -285,10 +277,6 @@ fn new_category<'a>(category: &'a str, slug: &'a str, description: &'a str) -> N
     }
 }
 
-fn logout(req: &mut dyn Request) {
-    req.mut_extensions().pop::<User>();
-}
-
 #[test]
 fn multiple_live_references_to_the_same_connection_can_be_checked_out() {
     use std::ptr;

src/tests/category.rs

@@ -1,24 +1,8 @@
-use crate::{app, builders::CrateBuilder, new_category, new_user, req, RequestHelper, TestApp};
-use cargo_registry::{
-    models::Category,
-    views::{EncodableCategory, EncodableCategoryWithSubcategories},
+use crate::{
+    builders::CrateBuilder, new_category, util::MockAnonymousUser, RequestHelper, TestApp,
 };
+use cargo_registry::{models::Category, views::EncodableCategoryWithSubcategories};
 
-use conduit::{Handler, Method};
-
-#[derive(Deserialize)]
-struct CategoryList {
-    categories: Vec<EncodableCategory>,
-    meta: CategoryMeta,
-}
-#[derive(Deserialize)]
-struct CategoryMeta {
-    total: i32,
-}
-#[derive(Deserialize)]
-struct GoodCategory {
-    category: EncodableCategory,
-}
 #[derive(Deserialize)]
 struct CategoryWithSubcategories {
     category: EncodableCategoryWithSubcategories,
@@ -27,10 +11,9 @@ struct CategoryWithSubcategories {
 #[test]
 fn index() {
     let (app, anon) = TestApp::init().empty();
-    let url = "/api/v1/categories";
 
     // List 0 categories if none exist
-    let json: CategoryList = anon.get(url).good();
+    let json = anon.show_category_list();
     assert_eq!(json.categories.len(), 0);
     assert_eq!(json.meta.total, 0);
 
@@ -45,7 +28,7 @@ fn index() {
     });
 
     // Only the top-level categories should be on the page
-    let json: CategoryList = anon.get(url).good();
+    let json = anon.show_category_list();
     assert_eq!(json.categories.len(), 1);
     assert_eq!(json.meta.total, 1);
     assert_eq!(json.categories[0].category, "foo");
@@ -76,94 +59,76 @@ fn show() {
 #[test]
 #[allow(clippy::cognitive_complexity)]
 fn update_crate() {
-    let (app, middle) = app();
-    let mut req = req(Method::Get, "/api/v1/categories/foo");
-    macro_rules! cnt {
-        ($req:expr, $cat:expr) => {{
-            $req.with_path(&format!("/api/v1/categories/{}", $cat));
-            let mut response = ok_resp!(middle.call($req));
-            crate::json::<GoodCategory>(&mut response)
-                .category
-                .crates_cnt as usize
-        }};
+    // Convenience function to get the number of crates in a category
+    fn count(anon: &MockAnonymousUser, category: &str) -> usize {
+        let json = anon.show_category(category);
+        json.category.crates_cnt as usize
     }
 
-    let krate = {
-        let conn = t!(app.diesel_database.get());
-        let user = t!(new_user("foo").create_or_update(&conn));
-        t!(new_category("cat1", "cat1", "Category 1 crates").create_or_update(&conn));
-        t!(new_category("Category 2", "category-2", "Category 2 crates").create_or_update(&conn));
-        CrateBuilder::new("foo_crate", user.id).expect_build(&conn)
-    };
+    let (app, anon, user) = TestApp::init().with_user();
+    let user = user.as_model();
 
-    // Updating with no categories has no effect
-    Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &[]).unwrap();
-    assert_eq!(cnt!(&mut req, "cat1"), 0);
-    assert_eq!(cnt!(&mut req, "category-2"), 0);
-
-    // Happy path adding one category
-    Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &["cat1"]).unwrap();
-    assert_eq!(cnt!(&mut req, "cat1"), 1);
-    assert_eq!(cnt!(&mut req, "category-2"), 0);
-
-    // Replacing one category with another
-    Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &["category-2"]).unwrap();
-    assert_eq!(cnt!(&mut req, "cat1"), 0);
-    assert_eq!(cnt!(&mut req, "category-2"), 1);
-
-    // Removing one category
-    Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &[]).unwrap();
-    assert_eq!(cnt!(&mut req, "cat1"), 0);
-    assert_eq!(cnt!(&mut req, "category-2"), 0);
-
-    // Adding 2 categories
-    Category::update_crate(
-        &app.diesel_database.get().unwrap(),
-        &krate,
-        &["cat1", "category-2"],
-    )
-    .unwrap();
-    assert_eq!(cnt!(&mut req, "cat1"), 1);
-    assert_eq!(cnt!(&mut req, "category-2"), 1);
-
-    // Removing all categories
-    Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &[]).unwrap();
-    assert_eq!(cnt!(&mut req, "cat1"), 0);
-    assert_eq!(cnt!(&mut req, "category-2"), 0);
-
-    // Attempting to add one valid category and one invalid category
-    let invalid_categories = Category::update_crate(
-        &app.diesel_database.get().unwrap(),
-        &krate,
-        &["cat1", "catnope"],
-    )
-    .unwrap();
-    assert_eq!(invalid_categories, vec!["catnope"]);
-    assert_eq!(cnt!(&mut req, "cat1"), 1);
-    assert_eq!(cnt!(&mut req, "category-2"), 0);
-
-    // Does not add the invalid category to the category list
-    // (unlike the behavior of keywords)
-    req.with_path("/api/v1/categories");
-    let mut response = ok_resp!(middle.call(&mut req));
-    let json: CategoryList = crate::json(&mut response);
-    assert_eq!(json.categories.len(), 2);
-    assert_eq!(json.meta.total, 2);
-
-    // Attempting to add a category by display text; must use slug
-    Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &["Category 2"]).unwrap();
-    assert_eq!(cnt!(&mut req, "cat1"), 0);
-    assert_eq!(cnt!(&mut req, "category-2"), 0);
-
-    // Add a category and its subcategory
-    {
-        let conn = t!(app.diesel_database.get());
-        t!(new_category("cat1::bar", "cat1::bar", "bar crates").create_or_update(&conn,));
+    app.db(|conn| {
+        t!(new_category("cat1", "cat1", "Category 1 crates").create_or_update(conn));
+        t!(new_category("Category 2", "category-2", "Category 2 crates").create_or_update(conn));
+        let krate = CrateBuilder::new("foo_crate", user.id).expect_build(&conn);
+
+        // Updating with no categories has no effect
+        Category::update_crate(conn, &krate, &[]).unwrap();
+        assert_eq!(count(&anon, "cat1"), 0);
+        assert_eq!(count(&anon, "category-2"), 0);
+
+        // Happy path adding one category
+        Category::update_crate(conn, &krate, &["cat1"]).unwrap();
+        assert_eq!(count(&anon, "cat1"), 1);
+        assert_eq!(count(&anon, "category-2"), 0);
+
+        // Replacing one category with another
+        Category::update_crate(conn, &krate, &["category-2"]).unwrap();
+        assert_eq!(count(&anon, "cat1"), 0);
+        assert_eq!(count(&anon, "category-2"), 1);
+
+        // Removing one category
+        Category::update_crate(conn, &krate, &[]).unwrap();
+        assert_eq!(count(&anon, "cat1"), 0);
+        assert_eq!(count(&anon, "category-2"), 0);
+
+        // Adding 2 categories
+        Category::update_crate(conn, &krate, &["cat1", "category-2"]).unwrap();
+        assert_eq!(count(&anon, "cat1"), 1);
+        assert_eq!(count(&anon, "category-2"), 1);
+
+        // Removing all categories
+        Category::update_crate(conn, &krate, &[]).unwrap();
+        assert_eq!(count(&anon, "cat1"), 0);
+        assert_eq!(count(&anon, "category-2"), 0);
+
+        // Attempting to add one valid category and one invalid category
+        let invalid_categories =
+            Category::update_crate(conn, &krate, &["cat1", "catnope"]).unwrap();
+        assert_eq!(invalid_categories, vec!["catnope"]);
+        assert_eq!(count(&anon, "cat1"), 1);
+        assert_eq!(count(&anon, "category-2"), 0);
+
+        // Does not add the invalid category to the category list
+        // (unlike the behavior of keywords)
+        let json = anon.show_category_list();
+        assert_eq!(json.categories.len(), 2);
+        assert_eq!(json.meta.total, 2);
+
+        // Attempting to add a category by display text; must use slug
+        Category::update_crate(conn, &krate, &["Category 2"]).unwrap();
+        assert_eq!(count(&anon, "cat1"), 0);
+        assert_eq!(count(&anon, "category-2"), 0);
+
+        // Add a category and its subcategory
+        t!(new_category("cat1::bar", "cat1::bar", "bar crates").create_or_update(conn));
         Category::update_crate(&conn, &krate, &["cat1", "cat1::bar"]).unwrap();
-    }
-    assert_eq!(cnt!(&mut req, "cat1"), 1);
-    assert_eq!(cnt!(&mut req, "cat1::bar"), 1);
-    assert_eq!(cnt!(&mut req, "category-2"), 0);
+
+        assert_eq!(count(&anon, "cat1"), 1);
+        assert_eq!(count(&anon, "cat1::bar"), 1);
+        assert_eq!(count(&anon, "category-2"), 0);
+    });
 }
 
 #[test]