From 7c9e7acdde788c455a85b2bed13d49d0dc81a7e2 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Sun, 17 Mar 2024 18:05:48 +0100 Subject: [PATCH] Better parallel testing (#503) * remove serial tests, connect to random databases and buckets * fix redis * remove unused deps * share postgres and redis * sleep between lock attempts * rename examples --- .github/workflows/rust.yml | 28 ++++++- Cargo.lock | 49 +----------- crates/kitsune-activitypub/Cargo.toml | 1 - .../tests/fetcher/basic.rs | 3 - .../tests/fetcher/filter.rs | 2 - .../tests/fetcher/infinite.rs | 1 - .../tests/fetcher/origin.rs | 2 - .../tests/fetcher/webfinger.rs | 2 - crates/kitsune-db/Cargo.toml | 1 - crates/kitsune-db/tests/unicode_collation.rs | 2 - crates/kitsune-s3/Cargo.toml | 1 - crates/kitsune-s3/src/lib.rs | 2 - crates/kitsune-service/Cargo.toml | 1 - crates/kitsune-service/src/attachment.rs | 1 - crates/kitsune-service/src/post/resolver.rs | 2 +- crates/kitsune-test/Cargo.toml | 6 +- crates/kitsune-test/src/lib.rs | 80 ++++++++++++------- crates/kitsune-test/src/redis.rs | 49 ++++++++++++ crates/kitsune-test/src/resource.rs | 40 +++++----- kitsune/Cargo.toml | 1 - .../src/http/handler/well_known/webfinger.rs | 2 - .../{simple.rs => simple_txt_verification.rs} | 0 .../{simple.rs => simple_id_convert.rs} | 0 23 files changed, 155 insertions(+), 121 deletions(-) create mode 100644 crates/kitsune-test/src/redis.rs rename lib/geomjeungja/examples/{simple.rs => simple_txt_verification.rs} (100%) rename lib/masto-id-convert/examples/{simple.rs => simple_id_convert.rs} (100%) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 4c8b67d1f..7de3d8aa8 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -38,6 +38,29 @@ jobs: test: name: Test runs-on: ubuntu-latest + services: + postgres: + image: postgres + ports: + - 5432:5432 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: test_db + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + redis: + image: redis + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 steps: - uses: actions/checkout@v4 - uses: DeterminateSystems/nix-installer-action@v4 @@ -45,4 +68,7 @@ jobs: - uses: Swatinem/rust-cache@v2 - uses: rui314/setup-mold@v1 - uses: taiki-e/install-action@nextest - - run: nix develop --impure --command bash -c "unset DATABASE_URL LD_LIBRARY_PATH REDIS_URL && cargo nextest run --all-features" + - run: nix develop --impure --command bash -c "unset LD_LIBRARY_PATH && cargo nextest run --all-features" + env: + DATABASE_URL: "postgres://postgres:postgres@localhost/test_db" + REDIS_URL: "redis://localhost" diff --git a/Cargo.lock b/Cargo.lock index c5fb89cc5..d8422e6cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1870,19 +1870,6 @@ dependencies = [ "syn 2.0.53", ] -[[package]] -name = "dashmap" -version = "5.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856" -dependencies = [ - "cfg-if", - "hashbrown 0.14.3", - "lock_api", - "once_cell", - "parking_lot_core 0.9.9", -] - [[package]] name = "data-encoding" version = "2.5.0" @@ -3564,7 +3551,6 @@ dependencies = [ "scoped-futures", "serde", "serde_urlencoded", - "serial_test", "simd-json", "simdutf8", "speedy-uuid", @@ -3623,7 +3609,6 @@ dependencies = [ "rsa", "scoped-futures", "serde", - "serial_test", "sha2", "simd-json", "speedy-uuid", @@ -3734,7 +3719,6 @@ dependencies = [ "rustls 0.22.2", "rustls-native-certs", "serde", - "serial_test", "simd-json", "speedy-uuid", "thiserror", @@ -4002,7 +3986,6 @@ dependencies = [ "quick-xml 0.31.0", "rusty-s3", "serde", - "serial_test", "tokio", "typed-builder", ] @@ -4093,7 +4076,6 @@ dependencies = [ "rusty-s3", "scoped-futures", "serde", - "serial_test", "simd-json", "smol_str", "speedy-uuid", @@ -4126,7 +4108,6 @@ name = "kitsune-test" version = "0.0.1-pre.5" dependencies = [ "bytes", - "diesel", "diesel-async", "futures-util", "http 1.1.0", @@ -4137,11 +4118,14 @@ dependencies = [ "kitsune-s3", "multiplex-pool", "pin-project-lite", + "rand 0.8.5", "redis", "rusty-s3", - "scoped-futures", "testcontainers", "testcontainers-modules", + "tokio", + "url", + "uuid", ] [[package]] @@ -6982,31 +6966,6 @@ dependencies = [ "syn 2.0.53", ] -[[package]] -name = "serial_test" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "953ad9342b3aaca7cb43c45c097dd008d4907070394bd0751a0aa8817e5a018d" -dependencies = [ - "dashmap", - "futures", - "lazy_static", - "log", - "parking_lot 0.12.1", - "serial_test_derive", -] - -[[package]] -name = "serial_test_derive" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b93fb4adc70021ac1b47f7d45e8cc4169baaa7ea58483bc5b721d19a26202212" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.53", -] - [[package]] name = "servo_arc" version = "0.1.1" diff --git a/crates/kitsune-activitypub/Cargo.toml b/crates/kitsune-activitypub/Cargo.toml index 0739ce16b..0c5a76a70 100644 --- a/crates/kitsune-activitypub/Cargo.toml +++ b/crates/kitsune-activitypub/Cargo.toml @@ -52,7 +52,6 @@ kitsune-config = { path = "../kitsune-config" } kitsune-test = { path = "../kitsune-test" } kitsune-webfinger = { path = "../kitsune-webfinger" } pretty_assertions = "1.4.0" -serial_test = "3.0.0" tokio = { version = "1.36.0", features = ["macros"] } tower = { version = "0.4.13", default-features = false, features = ["util"] } diff --git a/crates/kitsune-activitypub/tests/fetcher/basic.rs b/crates/kitsune-activitypub/tests/fetcher/basic.rs index 2319c2409..25489cd11 100644 --- a/crates/kitsune-activitypub/tests/fetcher/basic.rs +++ b/crates/kitsune-activitypub/tests/fetcher/basic.rs @@ -20,7 +20,6 @@ use std::sync::Arc; use tower::service_fn; #[tokio::test] -#[serial_test::serial] async fn fetch_actor() { database_test(|db_pool| async move { let client = Client::builder().service(service_fn(handle)); @@ -63,7 +62,6 @@ async fn fetch_actor() { } #[tokio::test] -#[serial_test::serial] async fn fetch_emoji() { database_test(|db_pool| async move { let client = Client::builder().service(service_fn(handle)); @@ -116,7 +114,6 @@ async fn fetch_emoji() { } #[tokio::test] -#[serial_test::serial] async fn fetch_note() { database_test(|db_pool| async move { let client = Client::builder().service(service_fn(handle)); diff --git a/crates/kitsune-activitypub/tests/fetcher/filter.rs b/crates/kitsune-activitypub/tests/fetcher/filter.rs index d2d23a3a1..1cd2cdf1d 100644 --- a/crates/kitsune-activitypub/tests/fetcher/filter.rs +++ b/crates/kitsune-activitypub/tests/fetcher/filter.rs @@ -14,7 +14,6 @@ use std::{convert::Infallible, sync::Arc}; use tower::service_fn; #[tokio::test] -#[serial_test::serial] async fn federation_allow() { database_test(|db_pool| async move { let builder = Fetcher::builder() @@ -89,7 +88,6 @@ async fn federation_allow() { } #[tokio::test] -#[serial_test::serial] async fn federation_deny() { database_test(|db_pool| async move { let client = service_fn( diff --git a/crates/kitsune-activitypub/tests/fetcher/infinite.rs b/crates/kitsune-activitypub/tests/fetcher/infinite.rs index 108f77a80..7a3bf5a3f 100644 --- a/crates/kitsune-activitypub/tests/fetcher/infinite.rs +++ b/crates/kitsune-activitypub/tests/fetcher/infinite.rs @@ -24,7 +24,6 @@ use std::{ use tower::service_fn; #[tokio::test] -#[serial_test::serial] async fn fetch_infinitely_long_reply_chain() { database_test(|db_pool| async move { let request_counter = Arc::new(AtomicU32::new(0)); diff --git a/crates/kitsune-activitypub/tests/fetcher/origin.rs b/crates/kitsune-activitypub/tests/fetcher/origin.rs index 39d6d3c06..2588cf281 100644 --- a/crates/kitsune-activitypub/tests/fetcher/origin.rs +++ b/crates/kitsune-activitypub/tests/fetcher/origin.rs @@ -14,7 +14,6 @@ use std::{convert::Infallible, sync::Arc}; use tower::service_fn; #[tokio::test] -#[serial_test::serial] async fn check_ap_id_authority() { database_test(|db_pool| async move { let builder = Fetcher::builder() @@ -80,7 +79,6 @@ async fn check_ap_id_authority() { } #[tokio::test] -#[serial_test::serial] async fn check_ap_content_type() { database_test(|db_pool| async move { let client = service_fn(|req: Request<_>| async { diff --git a/crates/kitsune-activitypub/tests/fetcher/webfinger.rs b/crates/kitsune-activitypub/tests/fetcher/webfinger.rs index ef4a0de9d..75777d22e 100644 --- a/crates/kitsune-activitypub/tests/fetcher/webfinger.rs +++ b/crates/kitsune-activitypub/tests/fetcher/webfinger.rs @@ -16,7 +16,6 @@ use std::{convert::Infallible, sync::Arc}; use tower::service_fn; #[tokio::test] -#[serial_test::serial] async fn fetch_actor_with_custom_acct() { database_test(|db_pool| async move { let mut jrd_base = include_bytes!("../../../../test-fixtures/0x0_jrd.json").to_owned(); @@ -80,7 +79,6 @@ async fn fetch_actor_with_custom_acct() { } #[tokio::test] -#[serial_test::serial] async fn ignore_fake_webfinger_acct() { database_test(|db_pool| async move { let link = Link { diff --git a/crates/kitsune-db/Cargo.toml b/crates/kitsune-db/Cargo.toml index d027b4abc..7e539d5c5 100644 --- a/crates/kitsune-db/Cargo.toml +++ b/crates/kitsune-db/Cargo.toml @@ -42,7 +42,6 @@ typed-builder = "0.18.1" [dev-dependencies] kitsune-test = { path = "../kitsune-test" } -serial_test = "3.0.0" tokio = { version = "1.36.0", features = ["macros"] } [lints] diff --git a/crates/kitsune-db/tests/unicode_collation.rs b/crates/kitsune-db/tests/unicode_collation.rs index 08aac0ffc..cf7ef860b 100644 --- a/crates/kitsune-db/tests/unicode_collation.rs +++ b/crates/kitsune-db/tests/unicode_collation.rs @@ -63,7 +63,6 @@ async fn create_user(conn: &mut AsyncPgConnection, username: &str) -> Result; - pub fn build_ap_response(body: B) -> http::Response> where Bytes: From, @@ -37,33 +37,35 @@ where Fut: Future, { let resource_handle = get_resource!("DATABASE_URL", self::container::postgres); + let mut url = Url::parse(&resource_handle.url()).unwrap(); + + // Create a new separate database for this test + let id = Uuid::new_v4().as_simple().to_string(); + let db_name = format!("kitsune_test_{id}"); + + let mut admin_conn = AsyncPgConnection::establish(url.as_str()).await.unwrap(); + + admin_conn + .batch_execute(&format!("CREATE DATABASE {db_name}")) + .await + .unwrap(); + + url.set_path(&db_name); + let pool = kitsune_db::connect(&DatabaseConfig { - url: resource_handle.url().into(), + url: url.as_str().into(), max_connections: 10, use_tls: false, }) .await .expect("Failed to connect to database"); - provide_resource(pool, func, |pool| async move { - pool.with_connection(|db_conn| { - async move { - diesel::sql_query("DROP SCHEMA public CASCADE") - .execute(db_conn) - .await - .expect("Failed to delete schema"); - - diesel::sql_query("CREATE SCHEMA public") - .execute(db_conn) - .await - .expect("Failed to create schema"); - - Ok::<_, BoxError>(()) - } - .scoped() - }) - .await - .expect("Failed to get connection"); + provide_resource(pool, func, |_pool| async move { + // Drop the newly created database. We don't need it anymore. + admin_conn + .batch_execute(&format!("DROP DATABASE {db_name}")) + .await + .unwrap(); }) .await } @@ -83,10 +85,13 @@ where { let resource_handle = get_resource!("MINIO_URL", self::container::minio); let endpoint = resource_handle.url().parse().unwrap(); + + // Create a new bucket with a random ID + let bucket_id = Uuid::new_v4().as_simple().to_string(); let bucket = rusty_s3::Bucket::new( endpoint, rusty_s3::UrlStyle::Path, - "test-bucket", + format!("test-bucket-{bucket_id}"), "us-east-1", ) .unwrap(); @@ -111,9 +116,21 @@ where Fut: Future, { let resource_handle = get_resource!("REDIS_URL", self::container::redis); - let client = redis::Client::open(resource_handle.url().as_ref()).unwrap(); + let client = ::redis::Client::open(resource_handle.url().as_ref()).unwrap(); + + // Connect to a random Redis database + let db_id = self::redis::find_unused_database(&client).await; let pool = multiplex_pool::Pool::from_producer( - || client.get_connection_manager(), + || async { + let mut conn = client.get_connection_manager().await?; + let (): () = ::redis::cmd("SELECT") + .arg(db_id) + .query_async(&mut conn) + .await + .unwrap(); + + Ok::<_, ::redis::RedisError>(conn) + }, 5, RoundRobinStrategy::default(), ) @@ -122,7 +139,10 @@ where provide_resource(pool, func, |pool| async move { let mut conn = pool.get(); - let (): () = redis::cmd("FLUSHALL").query_async(&mut conn).await.unwrap(); + let (): () = ::redis::cmd("FLUSHDB") + .query_async(&mut conn) + .await + .unwrap(); }) .await } diff --git a/crates/kitsune-test/src/redis.rs b/crates/kitsune-test/src/redis.rs new file mode 100644 index 000000000..9f70bca1a --- /dev/null +++ b/crates/kitsune-test/src/redis.rs @@ -0,0 +1,49 @@ +use rand::Rng; +use redis::{aio::MultiplexedConnection, RedisResult, Value}; +use std::{ops::RangeInclusive, time::Duration}; + +const DATABASE_RANGE: RangeInclusive = 1..=15; +const LOCK_KEY: &str = "_TEST_LOCK"; +const LOCK_VALUE: &str = "LOCKED"; +const SLEEP_DURATION: Duration = Duration::from_millis(100); + +async fn switch_and_try_lock(conn: &mut MultiplexedConnection, id: u8) -> bool { + let (): () = redis::cmd("SELECT") + .arg(id) + .query_async(conn) + .await + .unwrap(); + + try_lock(conn).await +} + +async fn try_lock(conn: &mut MultiplexedConnection) -> bool { + let result: RedisResult = redis::cmd("SET") + .arg(LOCK_KEY) + .arg(LOCK_VALUE) + .arg("NX") + .query_async(conn) + .await; + + matches!(result, Ok(Value::Okay)) +} + +/// Find and claim one of the 16 database slots on the Redis instance +pub async fn find_unused_database(client: &redis::Client) -> u8 { + let mut connection = client.get_multiplexed_async_connection().await.unwrap(); + + for i in DATABASE_RANGE { + if switch_and_try_lock(&mut connection, i).await { + return i; + } + } + + loop { + let db_id = rand::thread_rng().gen_range(DATABASE_RANGE); + if switch_and_try_lock(&mut connection, db_id).await { + break db_id; + } + + tokio::time::sleep(SLEEP_DURATION).await; + } +} diff --git a/crates/kitsune-test/src/resource.rs b/crates/kitsune-test/src/resource.rs index eb2b2995f..07193a196 100644 --- a/crates/kitsune-test/src/resource.rs +++ b/crates/kitsune-test/src/resource.rs @@ -20,6 +20,26 @@ macro_rules! get_resource { }; } +pub enum ResourceHandle +where + S: Service, +{ + Container(S), + Url(String), +} + +impl ResourceHandle +where + S: Service, +{ + pub fn url(&self) -> Cow<'_, str> { + match self { + Self::Container(container) => Cow::Owned(container.url()), + Self::Url(ref url) => Cow::Borrowed(url), + } + } +} + /// Provide a resource to the `run` closure, catch any panics that may occur while polling the future returned by `run`, /// then run the `cleanup` closure, and resume any panic unwinds that were caught pub async fn provide_resource( @@ -42,23 +62,3 @@ where Err(err) => panic::resume_unwind(err), } } - -pub enum ResourceHandle -where - S: Service, -{ - Container(S), - Url(String), -} - -impl ResourceHandle -where - S: Service, -{ - pub fn url(&self) -> Cow<'_, str> { - match self { - Self::Container(container) => Cow::Owned(container.url()), - Self::Url(ref url) => Cow::Borrowed(url), - } - } -} diff --git a/kitsune/Cargo.toml b/kitsune/Cargo.toml index f366ca7cf..d2aa06e21 100644 --- a/kitsune/Cargo.toml +++ b/kitsune/Cargo.toml @@ -141,7 +141,6 @@ redis = { version = "0.25.2", default-features = false, features = [ "connection-manager", "tokio-comp", ] } -serial_test = "3.0.0" tower = "0.4.13" [features] diff --git a/kitsune/src/http/handler/well_known/webfinger.rs b/kitsune/src/http/handler/well_known/webfinger.rs index c66bbd7f0..d3e9b410b 100644 --- a/kitsune/src/http/handler/well_known/webfinger.rs +++ b/kitsune/src/http/handler/well_known/webfinger.rs @@ -166,7 +166,6 @@ mod tests { } #[tokio::test] - #[serial_test::serial] async fn basic() { database_test(|db_pool| { redis_test(|redis_pool| async move { @@ -234,7 +233,6 @@ mod tests { } #[tokio::test] - #[serial_test::serial] async fn custom_domain() { database_test(|db_pool| { redis_test(|redis_pool| async move { diff --git a/lib/geomjeungja/examples/simple.rs b/lib/geomjeungja/examples/simple_txt_verification.rs similarity index 100% rename from lib/geomjeungja/examples/simple.rs rename to lib/geomjeungja/examples/simple_txt_verification.rs diff --git a/lib/masto-id-convert/examples/simple.rs b/lib/masto-id-convert/examples/simple_id_convert.rs similarity index 100% rename from lib/masto-id-convert/examples/simple.rs rename to lib/masto-id-convert/examples/simple_id_convert.rs