Merge #1562

1562: Convert more tests r=carols10cents a=jtgeibel

These commits contain minor test cleanups and convert some additional tests to the new infrastructure.

Co-authored-by: Justin Geibel <jtgeibel@gmail.com>

Author: bors-voyager[bot]

src/bin/update-downloads.rs

@@ -144,7 +144,7 @@ mod test {
             &HashMap::new(),
             None,
             None,
-            None,
+            0,
         ).unwrap();
         let version = version.save(&conn, &[]).unwrap();
         (krate, version)

src/controllers/krate/publish.rs

@@ -135,7 +135,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {
             license_file,
             // Downcast is okay because the file length must be less than the max upload size
             // to get here, and max upload sizes are way less than i32 max
-            Some(file_length as i32),
+            file_length as i32,
         )?.save(&conn, &new_crate.authors)?;
 
         // Link this new version to all dependencies

src/models/version.rs

@@ -120,7 +120,7 @@ impl NewVersion {
         features: &HashMap<String, Vec<String>>,
         license: Option<String>,
         license_file: Option<&str>,
-        crate_size: Option<i32>,
+        crate_size: i32,
     ) -> CargoResult<Self> {
         let features = serde_json::to_value(features)?;
 
@@ -129,7 +129,7 @@ impl NewVersion {
             num: num.to_string(),
             features,
             license,
-            crate_size,
+            crate_size: Some(crate_size),
         };
 
         new_version.validate_license(license_file)?;

src/tests/all.rs

@@ -47,7 +47,7 @@ use cargo_registry::{models, schema, views};
 use util::{Bad, RequestHelper, TestApp};
 
 use models::{Crate, CrateOwner, Dependency, Team, User, Version};
-use models::{NewCategory, NewTeam, NewUser, NewVersion};
+use models::{NewCategory, NewTeam, NewUser};
 use schema::*;
 use views::krate_publish as u;
 use views::{EncodableCrate, EncodableKeyword, EncodableVersion};
@@ -267,11 +267,6 @@ fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) ->
 
 use cargo_registry::util::CargoResult;
 
-fn new_version(crate_id: i32, num: &str, crate_size: Option<i32>) -> NewVersion {
-    let num = semver::Version::parse(num).unwrap();
-    NewVersion::new(crate_id, &num, &HashMap::new(), None, None, crate_size).unwrap()
-}
-
 fn krate(name: &str) -> Crate {
     static NEXT_CRATE_ID: AtomicUsize = ATOMIC_USIZE_INIT;
 

src/tests/builders.rs

@@ -26,6 +26,7 @@ pub struct VersionBuilder<'a> {
     features: HashMap<String, Vec<String>>,
     dependencies: Vec<(i32, Option<&'static str>)>,
     yanked: bool,
+    size: i32,
 }
 
 impl<'a> VersionBuilder<'a> {
@@ -46,6 +47,7 @@ impl<'a> VersionBuilder<'a> {
             features: HashMap::new(),
             dependencies: Vec::new(),
             yanked: false,
+            size: 0,
         }
     }
 
@@ -66,6 +68,12 @@ impl<'a> VersionBuilder<'a> {
         Self { yanked, ..self }
     }
 
+    /// Sets the version's size.
+    pub fn size(mut self, size: i32) -> Self {
+        self.size = size;
+        self
+    }
+
     fn build(self, crate_id: i32, connection: &PgConnection) -> CargoResult<Version> {
         use diesel::{insert_into, update};
 
@@ -80,7 +88,7 @@ impl<'a> VersionBuilder<'a> {
             &self.features,
             license,
             self.license_file,
-            None,
+            self.size,
         )?.save(connection, &[])?;
 
         if self.yanked {
@@ -109,6 +117,17 @@ impl<'a> VersionBuilder<'a> {
 
         Ok(vers)
     }
+
+    /// Consumes the builder and creates the version record in the database.
+    ///
+    /// # Panics
+    ///
+    /// Panics (and fails the test) if any part of inserting the version record fails.
+    pub fn expect_build(self, crate_id: i32, connection: &PgConnection) -> Version {
+        self.build(crate_id, connection).unwrap_or_else(|e| {
+            panic!("Unable to create version: {:?}", e);
+        })
+    }
 }
 
 impl<'a> From<&'a str> for VersionBuilder<'a> {

src/tests/krate.rs

@@ -34,8 +34,8 @@ use {
     app, krate, new_category, new_crate, new_crate_to_body, new_crate_to_body_with_io,
     new_crate_to_body_with_tarball, new_dependency, new_req, new_req_body_version_2, new_req_full,
     new_req_with_badges, new_req_with_categories, new_req_with_documentation,
-    new_req_with_keywords, new_user, new_version, req, sign_in, sign_in_as, Bad, CrateMeta,
-    CrateResponse, GoodCrate, OkBool, RequestHelper, TestApp,
+    new_req_with_keywords, new_user, req, sign_in, sign_in_as, Bad, CrateMeta, CrateResponse,
+    GoodCrate, OkBool, RequestHelper, TestApp,
 };
 
 #[derive(Deserialize)]
@@ -1144,7 +1144,7 @@ fn dependencies() {
         let conn = app.diesel_database.get().unwrap();
         let user = new_user("foo").create_or_update(&conn).unwrap();
         let c1 = CrateBuilder::new("foo_deps", user.id).expect_build(&conn);
-        let v = new_version(c1.id, "1.0.0", None).save(&conn, &[]).unwrap();
+        let v = VersionBuilder::new("1.0.0").expect_build(c1.id, &conn);
         let c2 = CrateBuilder::new("bar_deps", user.id).expect_build(&conn);
         new_dependency(&conn, &v, &c2);
     }

src/tests/schema_details.rs

@@ -1,7 +1,7 @@
 use diesel::prelude::*;
 use diesel::sql_types::Text;
 
-use app;
+use TestApp;
 
 #[test]
 fn all_columns_called_crate_id_have_a_cascading_foreign_key() {
@@ -64,11 +64,12 @@ struct TableNameAndConstraint {
 fn get_fk_constraint_definitions(column_name: &str) -> Vec<TableNameAndConstraint> {
     use diesel::sql_query;
 
-    let (_r, app, _) = app();
-    let conn = app.diesel_database.get().unwrap();
+    let (app, _) = TestApp::init().empty();
 
-    sql_query(include_str!("load_foreign_key_constraints.sql"))
-        .bind::<Text, _>(column_name)
-        .load(&*conn)
-        .unwrap()
+    app.db(|conn| {
+        sql_query(include_str!("load_foreign_key_constraints.sql"))
+            .bind::<Text, _>(column_name)
+            .load(conn)
+            .unwrap()
+    })
 }