Refactor some test code

src/render.rs

@@ -357,7 +357,7 @@ mod tests {
     // in comrak 0.1.8 but was fixed in 0.1.9.
     #[test]
     fn text_with_fancy_single_quotes() {
-        let text = r#"wb’"#;
+        let text = "wb’";
         let result = markdown_to_html(text, None);
         assert_eq!(result, "<p>wb’</p>\n");
     }

src/tests/all.rs

@@ -15,26 +15,19 @@ extern crate tracing;
 
 use crate::util::{RequestHelper, TestApp};
 use cargo_registry::{
-    models::{Crate, CrateOwner, Dependency, NewCategory, NewTeam, NewUser, Team, User, Version},
+    models::{Crate, CrateOwner, NewCategory, NewTeam, NewUser, Team, User},
     schema::crate_owners,
-    util::AppResponse,
     views::{
         EncodableCategory, EncodableCategoryWithSubcategories, EncodableCrate, EncodableKeyword,
         EncodableOwner, EncodableVersion, GoodCrate,
     },
-    App, Config, Env, Replica, Uploader,
 };
 use std::{
     borrow::Cow,
-    sync::{
-        atomic::{AtomicUsize, Ordering},
-        Arc,
-    },
+    sync::atomic::{AtomicUsize, Ordering},
 };
 
-use conduit::Body;
 use diesel::prelude::*;
-use reqwest::{blocking::Client, Proxy};
 
 mod account_lock;
 mod authentication;
@@ -105,66 +98,6 @@ pub struct OkBool {
     ok: bool,
 }
 
-fn app() -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
-    build_app(simple_config(), None)
-}
-
-fn simple_config() -> Config {
-    let uploader = Uploader::S3 {
-        bucket: s3::Bucket::new(
-            String::from("alexcrichton-test"),
-            None,
-            dotenv::var("S3_ACCESS_KEY").unwrap_or_default(),
-            dotenv::var("S3_SECRET_KEY").unwrap_or_default(),
-            // When testing we route all API traffic over HTTP so we can
-            // sniff/record it, but everywhere else we use https
-            "http",
-        ),
-        cdn: None,
-    };
-
-    Config {
-        uploader,
-        session_key: "test this has to be over 32 bytes long".to_string(),
-        gh_client_id: dotenv::var("GH_CLIENT_ID").unwrap_or_default(),
-        gh_client_secret: dotenv::var("GH_CLIENT_SECRET").unwrap_or_default(),
-        gh_base_url: "http://api.github.com".to_string(),
-        db_url: env("TEST_DATABASE_URL"),
-        replica_db_url: None,
-        env: Env::Test,
-        max_upload_size: 3000,
-        max_unpack_size: 2000,
-        mirror: Replica::Primary,
-        // When testing we route all API traffic over HTTP so we can
-        // sniff/record it, but everywhere else we use https
-        api_protocol: String::from("http"),
-        publish_rate_limit: Default::default(),
-        blocked_traffic: Default::default(),
-        domain_name: "crates.io".into(),
-        allowed_origins: Vec::new(),
-    }
-}
-
-fn build_app(
-    config: Config,
-    proxy: Option<String>,
-) -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
-    let client = if let Some(proxy) = proxy {
-        let mut builder = Client::builder();
-        builder = builder
-            .proxy(Proxy::all(&proxy).expect("Unable to configure proxy with the provided URL"));
-        Some(builder.build().expect("TLS backend cannot be initialized"))
-    } else {
-        None
-    };
-
-    let app = App::new(config, client);
-    assert_ok!(assert_ok!(app.primary_database.get()).begin_test_transaction());
-    let app = Arc::new(app);
-    let handler = cargo_registry::build_handler(Arc::clone(&app));
-    (app, handler)
-}
-
 // Return the environment variable only if it has been defined
 #[track_caller]
 fn env(var: &str) -> String {
@@ -178,27 +111,6 @@ fn env(var: &str) -> String {
     }
 }
 
-fn json<T>(r: &mut AppResponse) -> T
-where
-    for<'de> T: serde::Deserialize<'de>,
-{
-    use conduit::Body::*;
-
-    let mut body = Body::empty();
-    std::mem::swap(r.body_mut(), &mut body);
-    let body: std::borrow::Cow<'static, [u8]> = match body {
-        Static(slice) => slice.into(),
-        Owned(vec) => vec.into(),
-        File(_) => unimplemented!(),
-    };
-
-    let s = std::str::from_utf8(&body).unwrap();
-    match serde_json::from_str(s) {
-        Ok(t) => t,
-        Err(e) => panic!("failed to decode: {:?}\n{}", e, s),
-    }
-}
-
 static NEXT_GH_ID: AtomicUsize = AtomicUsize::new(0);
 
 fn new_user(login: &str) -> NewUser<'_> {
@@ -240,23 +152,6 @@ fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) ->
     Ok(())
 }
 
-fn new_dependency(conn: &PgConnection, version: &Version, krate: &Crate) -> Dependency {
-    use cargo_registry::schema::dependencies::dsl::*;
-    use diesel::insert_into;
-
-    insert_into(dependencies)
-        .values((
-            version_id.eq(version.id),
-            crate_id.eq(krate.id),
-            req.eq(">= 0"),
-            optional.eq(false),
-            default_features.eq(false),
-            features.eq(Vec::<String>::new()),
-        ))
-        .get_result(conn)
-        .unwrap()
-}
-
 fn new_category<'a>(category: &'a str, slug: &'a str, description: &'a str) -> NewCategory<'a> {
     NewCategory {
         category,
@@ -265,11 +160,15 @@ fn new_category<'a>(category: &'a str, slug: &'a str, description: &'a str) -> N
     }
 }
 
+// This reflects the configuration of our test environment. In the production application, this
+// does not hold true.
 #[test]
 fn multiple_live_references_to_the_same_connection_can_be_checked_out() {
     use std::ptr;
 
-    let (app, _) = app();
+    let (app, _) = TestApp::init().empty();
+    let app = app.as_inner();
+
     let conn1 = app.primary_database.get().unwrap();
     let conn2 = app.primary_database.get().unwrap();
     let conn1_ref: &PgConnection = &conn1;

src/tests/authentication.rs

@@ -1,45 +1,32 @@
-use crate::{util::RequestHelper, TestApp};
+use crate::util::{RequestHelper, Response};
+use crate::TestApp;
 
 use crate::util::encode_session_header;
-use conduit::{header, Handler, HandlerResult, Method, StatusCode};
-use conduit_test::MockRequest;
+use conduit::{header, Body, Method, StatusCode};
 
 static URL: &str = "/api/v1/me/updates";
-static MUST_LOGIN: &[u8] =
-    b"{\"errors\":[{\"detail\":\"must be logged in to perform that action\"}]}";
+static MUST_LOGIN: &[u8] = br#"{"errors":[{"detail":"must be logged in to perform that action"}]}"#;
 static INTERNAL_ERROR_NO_USER: &str =
     "user_id from cookie not found in database caused by NotFound";
 
-fn call(app: &TestApp, mut request: MockRequest) -> HandlerResult {
-    app.as_middleware().call(&mut request)
-}
-
-fn into_parts(response: HandlerResult) -> (StatusCode, std::borrow::Cow<'static, [u8]>) {
-    use conduit_test::ResponseExt;
-
-    let response = response.unwrap();
-    (response.status(), response.into_cow())
-}
-
 #[test]
 fn anonymous_user_unauthorized() {
-    let (app, anon) = TestApp::init().empty();
-    let request = anon.request_builder(Method::GET, URL);
+    let (_, anon) = TestApp::init().empty();
+    let response: Response<Body> = anon.get(URL);
 
-    let (status, body) = into_parts(call(&app, request));
-    assert_eq!(status, StatusCode::FORBIDDEN);
-    assert_eq!(body, MUST_LOGIN);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
+    assert_eq!(response.json().to_string().as_bytes(), MUST_LOGIN);
 }
 
 #[test]
 fn token_auth_cannot_find_token() {
-    let (app, anon) = TestApp::init().empty();
+    let (_, anon) = TestApp::init().empty();
     let mut request = anon.request_builder(Method::GET, URL);
     request.header(header::AUTHORIZATION, "cio1tkfake-token");
+    let response: Response<Body> = anon.run(request);
 
-    let (status, body) = into_parts(call(&app, request));
-    assert_eq!(status, StatusCode::FORBIDDEN);
-    assert_eq!(body, MUST_LOGIN);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
+    assert_eq!(response.json().to_string().as_bytes(), MUST_LOGIN);
 }
 
 // Ensure that an unexpected authentication error is available for logging.  The user would see
@@ -55,7 +42,6 @@ fn cookie_auth_cannot_find_user() {
     let mut request = anon.request_builder(Method::GET, URL);
     request.header(header::COOKIE, &cookie);
 
-    let response = call(&app, request);
-    let log_message = response.map(|_| ()).unwrap_err().to_string();
-    assert_eq!(log_message, INTERNAL_ERROR_NO_USER);
+    let error = anon.run_err(request);
+    assert_eq!(error.to_string(), INTERNAL_ERROR_NO_USER);
 }

src/tests/krate/dependencies.rs

@@ -1,5 +1,4 @@
 use crate::builders::{CrateBuilder, VersionBuilder};
-use crate::new_dependency;
 use crate::util::{RequestHelper, TestApp};
 use cargo_registry::views::EncodableDependency;
 use http::StatusCode;
@@ -16,9 +15,10 @@ fn dependencies() {
 
     app.db(|conn| {
         let c1 = CrateBuilder::new("foo_deps", user.id).expect_build(conn);
-        let v = VersionBuilder::new("1.0.0").expect_build(c1.id, user.id, conn);
         let c2 = CrateBuilder::new("bar_deps", user.id).expect_build(conn);
-        new_dependency(conn, &v, &c2);
+        VersionBuilder::new("1.0.0")
+            .dependency(&c2, None)
+            .expect_build(c1.id, user.id, conn);
     });
 
     let deps: Deps = anon

src/tests/krate/publish.rs

@@ -222,7 +222,7 @@ fn new_krate_with_broken_dependency_requirement() {
 
     // create a request body with `version_req: "broken"`
     let (json, tarball) = crate_to_publish.build();
-    let new_json = json.replace("\"version_req\":\"1.2.3\"", "\"version_req\":\"broken\"");
+    let new_json = json.replace(r#""version_req":"1.2.3""#, r#""version_req":"broken""#);
     assert_ne!(json, new_json);
     let body = PublishBuilder::create_publish_body(&new_json, &tarball);
 

src/tests/server.rs

@@ -1,8 +1,8 @@
-use conduit::{header, Method};
-
 use crate::builders::*;
 use crate::util::*;
 
+use conduit::{header, Method, StatusCode};
+
 #[test]
 fn user_agent_is_required() {
     let (_app, anon) = TestApp::init().empty();

src/tests/token.rs

@@ -1,8 +1,4 @@
-use crate::{
-    user::UserShowPrivateResponse,
-    util::{header, StatusCode},
-    RequestHelper, TestApp,
-};
+use crate::{user::UserShowPrivateResponse, RequestHelper, TestApp};
 use cargo_registry::{
     models::ApiToken,
     schema::api_tokens,
@@ -11,6 +7,7 @@ use cargo_registry::{
 };
 use std::collections::HashSet;
 
+use conduit::{header, StatusCode};
 use diesel::prelude::*;
 
 #[derive(Deserialize)]

src/tests/user.rs

@@ -1,7 +1,7 @@
 use crate::{
     builders::{CrateBuilder, VersionBuilder},
     new_user,
-    util::{MockCookieUser, RequestHelper, Response, StatusCode},
+    util::{MockCookieUser, RequestHelper, Response},
     OkBool, TestApp,
 };
 use cargo_registry::{
@@ -10,6 +10,7 @@ use cargo_registry::{
     views::{EncodablePrivateUser, EncodablePublicUser, EncodableVersion, OwnedCrate},
 };
 
+use conduit::StatusCode;
 use diesel::prelude::*;
 
 #[derive(Deserialize)]