Inline test_db_connection() fn (#9976)

Now that we are more and more using async connections, the `test_db_connection()` fn isn't all that useful anymore since it always returns a sync connection. We can call these two statements individually instead to make our tests easier to migrate to async connections.

Author: Turbo87

src/lib.rs

@@ -53,7 +53,6 @@ pub mod sqs;
 pub mod ssh;
 pub mod storage;
 pub mod tasks;
-mod test_util;
 #[cfg(test)]
 pub mod tests;
 pub mod typosquat;

src/models/default_versions.rs

@@ -130,7 +130,7 @@ fn calculate_default_version(crate_id: i32, conn: &mut impl Conn) -> QueryResult
 mod tests {
     use super::*;
     use crate::schema::crates;
-    use crate::test_util::test_db_connection;
+    use crates_io_test_db::TestDatabase;
     use insta::assert_snapshot;
     use std::fmt::Write;
 
@@ -275,7 +275,8 @@ mod tests {
 
     #[test]
     fn test_update_default_version() {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
 
         let crate_id = create_crate("foo", conn);
         create_version(crate_id, "1.0.0", conn);

src/models/keyword.rs

@@ -97,13 +97,15 @@ impl Keyword {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_util::test_db_connection;
+    use crates_io_test_db::TestDatabase;
 
     #[test]
     fn dont_associate_with_non_lowercased_keywords() {
         use diesel::RunQueryDsl;
 
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
+
         // The code should be preventing lowercased keywords from existing,
         // but if one happens to sneak in there, don't associate crates with it.
 

src/rate_limiter.rs

@@ -182,11 +182,12 @@ struct Bucket {
 mod tests {
     use super::*;
     use crate::schema::users;
-    use crate::test_util::*;
+    use crates_io_test_db::TestDatabase;
 
     #[test]
     fn default_rate_limits() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         // Set the defaults as if no env vars have been set in production
@@ -254,7 +255,8 @@ mod tests {
 
     #[test]
     fn take_token_with_no_bucket_creates_new_one() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -301,7 +303,8 @@ mod tests {
 
     #[test]
     fn take_token_with_existing_bucket_modifies_existing_bucket() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -324,7 +327,8 @@ mod tests {
 
     #[test]
     fn take_token_after_delay_refills() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -348,7 +352,8 @@ mod tests {
 
     #[test]
     fn refill_subsecond_rate() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         // Subsecond rates have floating point rounding issues, so use a known
         // timestamp that rounds fine
         let now =
@@ -376,7 +381,8 @@ mod tests {
 
     #[test]
     fn last_refill_always_advanced_by_multiple_of_rate() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -405,7 +411,8 @@ mod tests {
 
     #[test]
     fn zero_tokens_returned_when_user_has_no_tokens_left() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -431,7 +438,8 @@ mod tests {
 
     #[test]
     fn a_user_with_no_tokens_gets_a_token_after_exactly_rate() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -456,7 +464,8 @@ mod tests {
 
     #[test]
     fn tokens_never_refill_past_burst() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -481,7 +490,8 @@ mod tests {
 
     #[test]
     fn two_actions_dont_interfere_with_each_other() -> QueryResult<()> {
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let mut config = HashMap::new();
@@ -526,7 +536,8 @@ mod tests {
     fn override_is_used_instead_of_global_burst_if_present() -> QueryResult<()> {
         use diesel::RunQueryDsl;
 
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -558,7 +569,8 @@ mod tests {
     fn overrides_can_expire() -> QueryResult<()> {
         use diesel::RunQueryDsl;
 
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
 
         let rate = SampleRateLimiter {
@@ -607,7 +619,8 @@ mod tests {
     fn override_is_different_for_each_action() -> QueryResult<()> {
         use diesel::RunQueryDsl;
 
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let now = now();
         let user_id = new_user(conn, "user")?;
 

src/test_util.rs

@@ -163,14 +163,15 @@ impl From<crate::models::Owner> for Owner {
 
 #[cfg(test)]
 mod tests {
-    use crate::{test_util::test_db_connection, typosquat::test_util::faker};
-    use thiserror::Error;
-
     use super::*;
+    use crate::typosquat::test_util::faker;
+    use crates_io_test_db::TestDatabase;
+    use thiserror::Error;
 
     #[test]
     fn top_crates() -> Result<(), Error> {
-        let (_test_db, mut conn) = test_db_connection();
+        let test_db = TestDatabase::new();
+        let mut conn = test_db.connect();
 
         // Set up two users.
         let user_a = faker::user(&mut conn, "a")?;

src/typosquat/database.rs

@@ -107,7 +107,7 @@ mod tests {
     use super::*;
     use crate::models::{Crate, NewCrate, NewUser, NewVersion, User, Version};
     use crate::schema::{crate_downloads, crates, users, versions};
-    use crate::test_util::test_db_connection;
+    use crates_io_test_db::TestDatabase;
 
     fn user(conn: &mut impl Conn) -> User {
         let user = NewUser::new(2, "login", None, None, "access_token");
@@ -138,7 +138,8 @@ mod tests {
     fn increment() {
         use diesel::dsl::*;
 
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let user = user(conn);
         let (krate, version) = crate_and_version(conn, user.id);
         insert_into(version_downloads::table)
@@ -181,7 +182,8 @@ mod tests {
     fn set_processed_true() {
         use diesel::dsl::*;
 
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let user = user(conn);
         let (_, version) = crate_and_version(conn, user.id);
         insert_into(version_downloads::table)
@@ -205,7 +207,8 @@ mod tests {
     #[test]
     fn dont_process_recent_row() {
         use diesel::dsl::*;
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let user = user(conn);
         let (_, version) = crate_and_version(conn, user.id);
         insert_into(version_downloads::table)
@@ -231,7 +234,8 @@ mod tests {
         use diesel::dsl::*;
         use diesel::update;
 
-        let (_test_db, conn) = &mut test_db_connection();
+        let test_db = TestDatabase::new();
+        let conn = &mut test_db.connect();
         let user = user(conn);
         let (krate, version) = crate_and_version(conn, user.id);
         update(versions::table)
@@ -296,7 +300,8 @@ mod tests {
         use diesel::dsl::*;
         use diesel::update;
 
-        let (_test_db, mut conn) = test_db_connection();
+        let test_db = TestDatabase::new();
+        let mut conn = test_db.connect();
 
         // This test is using a transaction to ensure `now` is the same for all
         // queries within this test.

src/worker/jobs/downloads/update_metadata.rs

@@ -122,15 +122,16 @@ Specific squat checks that triggered:
 
 #[cfg(test)]
 mod tests {
-    use crate::{test_util::test_db_connection, typosquat::test_util::faker};
-    use lettre::Address;
-
     use super::*;
+    use crate::typosquat::test_util::faker;
+    use crates_io_test_db::TestDatabase;
+    use lettre::Address;
 
     #[tokio::test]
     async fn integration() -> anyhow::Result<()> {
         let emails = Emails::new_in_memory();
-        let (_test_db, mut conn) = test_db_connection();
+        let test_db = TestDatabase::new();
+        let mut conn = test_db.connect();
 
         // Set up a user and a popular crate to match against.
         let user = faker::user(&mut conn, "a")?;