From e3f055d9501ebd0cb703fae17366629436d3072c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 7 Feb 2022 15:00:00 -0800 Subject: [PATCH 1/4] Use a pool of databases to speed up integration tests Also, use env_logger consistently in the tests for each crate. Only initiallize the logger at all if some RUST_LOG env var is set. Co-Authored-By: Nathan Sobo --- crates/editor/src/test.rs | 5 +- crates/gpui/src/test.rs | 6 +- crates/language/src/tests.rs | 5 +- crates/server/src/db.rs | 116 ++++++++++++++++++++++++----------- crates/server/src/rpc.rs | 5 +- crates/text/src/tests.rs | 5 +- crates/zed/src/test.rs | 4 +- 7 files changed, 99 insertions(+), 47 deletions(-) diff --git a/crates/editor/src/test.rs b/crates/editor/src/test.rs index f4622d1f6e..67622db83f 100644 --- a/crates/editor/src/test.rs +++ b/crates/editor/src/test.rs @@ -1,6 +1,7 @@ #[cfg(test)] #[ctor::ctor] fn init_logger() { - // std::env::set_var("RUST_LOG", "info"); - env_logger::init(); + if std::env::var("RUST_LOG").is_ok() { + env_logger::init(); + } } diff --git a/crates/gpui/src/test.rs b/crates/gpui/src/test.rs index b4b4e621ac..af6430d36c 100644 --- a/crates/gpui/src/test.rs +++ b/crates/gpui/src/test.rs @@ -18,9 +18,9 @@ use crate::{ #[cfg(test)] #[ctor::ctor] fn init_logger() { - env_logger::builder() - .filter_level(log::LevelFilter::Info) - .init(); + if std::env::var("RUST_LOG").is_ok() { + env_logger::init(); + } } pub fn run_test( diff --git a/crates/language/src/tests.rs b/crates/language/src/tests.rs index 1fe3c7a917..40dfd06e94 100644 --- a/crates/language/src/tests.rs +++ b/crates/language/src/tests.rs @@ -17,8 +17,9 @@ use util::test::Network; #[cfg(test)] #[ctor::ctor] fn init_logger() { - // std::env::set_var("RUST_LOG", "info"); - env_logger::init(); + if std::env::var("RUST_LOG").is_ok() { + env_logger::init(); + } } #[test] diff --git a/crates/server/src/db.rs b/crates/server/src/db.rs index f71f40efd0..b50d7daf46 100644 --- a/crates/server/src/db.rs +++ b/crates/server/src/db.rs @@ -526,54 +526,88 @@ pub struct ChannelMessage { #[cfg(test)] pub mod tests { use super::*; + use lazy_static::lazy_static; + use parking_lot::Mutex; use rand::prelude::*; use sqlx::{ migrate::{MigrateDatabase, Migrator}, Postgres, }; - use std::path::Path; + use std::{mem, path::Path}; pub struct TestDb { - pub db: Db, + pub db: Option, pub name: String, pub url: String, } - impl TestDb { - pub fn new() -> Self { - // Enable tests to run in parallel by serializing the creation of each test database. - lazy_static::lazy_static! { - static ref DB_CREATION: std::sync::Mutex<()> = std::sync::Mutex::new(()); - } + lazy_static! { + static ref POOL: Mutex> = Default::default(); + } - let mut rng = StdRng::from_entropy(); - let name = format!("zed-test-{}", rng.gen::()); - let url = format!("postgres://postgres@localhost/{}", name); - let migrations_path = Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/migrations")); - let db = block_on(async { - { - let _lock = DB_CREATION.lock(); - Postgres::create_database(&url) - .await - .expect("failed to create test db"); - } - let mut db = Db::new(&url, 5).await.unwrap(); - db.test_mode = true; - let migrator = Migrator::new(migrations_path).await.unwrap(); - migrator.run(&db.pool).await.unwrap(); - db - }); - - Self { db, name, url } - } - - pub fn db(&self) -> &Db { - &self.db + #[ctor::dtor] + fn clear_pool() { + for db in POOL.lock().drain(..) { + db.teardown(); } } - impl Drop for TestDb { - fn drop(&mut self) { + impl TestDb { + pub fn new() -> Self { + let mut pool = POOL.lock(); + if let Some(db) = pool.pop() { + db.truncate(); + db + } else { + let mut rng = StdRng::from_entropy(); + let name = format!("zed-test-{}", rng.gen::()); + let url = format!("postgres://postgres@localhost/{}", name); + let migrations_path = Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/migrations")); + let db = block_on(async { + Postgres::create_database(&url) + .await + .expect("failed to create test db"); + let mut db = Db::new(&url, 5).await.unwrap(); + db.test_mode = true; + let migrator = Migrator::new(migrations_path).await.unwrap(); + migrator.run(&db.pool).await.unwrap(); + db + }); + + Self { + db: Some(db), + name, + url, + } + } + } + + pub fn db(&self) -> &Db { + self.db.as_ref().unwrap() + } + + fn truncate(&self) { + block_on(async { + let query = " + SELECT tablename FROM pg_tables + WHERE schemaname = 'public'; + "; + let table_names = sqlx::query_scalar::<_, String>(query) + .fetch_all(&self.db().pool) + .await + .unwrap(); + sqlx::query(&format!( + "TRUNCATE TABLE {} RESTART IDENTITY", + table_names.join(", ") + )) + .execute(&self.db().pool) + .await + .unwrap(); + }) + } + + fn teardown(mut self) { + let db = self.db.take().unwrap(); block_on(async { let query = " SELECT pg_terminate_backend(pg_stat_activity.pid) @@ -582,15 +616,27 @@ pub mod tests { "; sqlx::query(query) .bind(&self.name) - .execute(&self.db.pool) + .execute(&db.pool) .await .unwrap(); - self.db.pool.close().await; + db.pool.close().await; Postgres::drop_database(&self.url).await.unwrap(); }); } } + impl Drop for TestDb { + fn drop(&mut self) { + if let Some(db) = self.db.take() { + POOL.lock().push(TestDb { + db: Some(db), + name: mem::take(&mut self.name), + url: mem::take(&mut self.url), + }); + } + } + } + #[gpui::test] async fn test_get_users_by_ids() { let test_db = TestDb::new(); diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 85b29540f5..bcf222f853 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -1196,8 +1196,9 @@ mod tests { #[cfg(test)] #[ctor::ctor] fn init_logger() { - // std::env::set_var("RUST_LOG", "info"); - env_logger::init(); + if std::env::var("RUST_LOG").is_ok() { + env_logger::init(); + } } #[gpui::test] diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index b4ac7d46d4..de735051ae 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -12,8 +12,9 @@ use util::test::Network; #[cfg(test)] #[ctor::ctor] fn init_logger() { - // std::env::set_var("RUST_LOG", "info"); - env_logger::init(); + if std::env::var("RUST_LOG").is_ok() { + env_logger::init(); + } } #[test] diff --git a/crates/zed/src/test.rs b/crates/zed/src/test.rs index a365fdc6f4..5bf43b555f 100644 --- a/crates/zed/src/test.rs +++ b/crates/zed/src/test.rs @@ -12,7 +12,9 @@ use workspace::Settings; #[cfg(test)] #[ctor::ctor] fn init_logger() { - env_logger::init(); + if std::env::var("RUST_LOG").is_ok() { + env_logger::init(); + } } pub fn test_app_state(cx: &mut MutableAppContext) -> Arc { From b0ed58add3cc2b437a2b2dac82ab0f0ca2ceb7bd Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 7 Feb 2022 15:11:41 -0800 Subject: [PATCH 2/4] Run multiple iterations of all integration tests Co-Authored-By: Nathan Sobo --- crates/lsp/src/lsp.rs | 2 +- crates/server/src/rpc.rs | 32 ++++++++++++++++---------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index 5cc1fee8aa..567bc435ea 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -547,7 +547,7 @@ impl FakeLanguageServer { request.params, ); } else { - println!( + log::info!( "skipping message in fake language server {:?}", std::str::from_utf8(&self.buffer) ); diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index bcf222f853..bc1ef781f4 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -1201,7 +1201,7 @@ mod tests { } } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_share_project(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { let (window_b, _) = cx_b.add_window(|_| EmptyView); let lang_registry = Arc::new(LanguageRegistry::new()); @@ -1340,7 +1340,7 @@ mod tests { .await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_unshare_project(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { let lang_registry = Arc::new(LanguageRegistry::new()); let fs = Arc::new(FakeFs::new(cx_a.background())); @@ -1437,7 +1437,7 @@ mod tests { .unwrap(); } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_propagate_saves_and_fs_changes( mut cx_a: TestAppContext, mut cx_b: TestAppContext, @@ -1623,7 +1623,7 @@ mod tests { .await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_buffer_conflict_after_save(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { cx_a.foreground().forbid_parking(); let lang_registry = Arc::new(LanguageRegistry::new()); @@ -1716,7 +1716,7 @@ mod tests { }); } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_buffer_reloading(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { cx_a.foreground().forbid_parking(); let lang_registry = Arc::new(LanguageRegistry::new()); @@ -1878,7 +1878,7 @@ mod tests { buffer_b.condition(&cx_b, |buf, _| buf.text() == text).await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_leaving_worktree_while_opening_buffer( mut cx_a: TestAppContext, mut cx_b: TestAppContext, @@ -1956,7 +1956,7 @@ mod tests { .await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_peer_disconnection(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { cx_a.foreground().forbid_parking(); let lang_registry = Arc::new(LanguageRegistry::new()); @@ -2027,7 +2027,7 @@ mod tests { .await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_collaborating_with_diagnostics( mut cx_a: TestAppContext, mut cx_b: TestAppContext, @@ -2251,7 +2251,7 @@ mod tests { }); } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_collaborating_with_completion( mut cx_a: TestAppContext, mut cx_b: TestAppContext, @@ -2476,7 +2476,7 @@ mod tests { ); } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_formatting_buffer(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { cx_a.foreground().forbid_parking(); let mut lang_registry = Arc::new(LanguageRegistry::new()); @@ -2580,7 +2580,7 @@ mod tests { ); } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_definition(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { cx_a.foreground().forbid_parking(); let mut lang_registry = Arc::new(LanguageRegistry::new()); @@ -2737,7 +2737,7 @@ mod tests { .await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_open_buffer_while_getting_definition_pointing_to_it( mut cx_a: TestAppContext, mut cx_b: TestAppContext, @@ -2851,7 +2851,7 @@ mod tests { assert_eq!(definitions[0].target_buffer, buffer_b2); } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_basic_chat(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { cx_a.foreground().forbid_parking(); @@ -2991,7 +2991,7 @@ mod tests { .await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_chat_message_validation(mut cx_a: TestAppContext) { cx_a.foreground().forbid_parking(); @@ -3051,7 +3051,7 @@ mod tests { ); } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_chat_reconnection(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { cx_a.foreground().forbid_parking(); @@ -3263,7 +3263,7 @@ mod tests { .await; } - #[gpui::test] + #[gpui::test(iterations = 10)] async fn test_contacts( mut cx_a: TestAppContext, mut cx_b: TestAppContext, From c245356b42870192eca57b96c3facac83c6a1c83 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 7 Feb 2022 15:23:35 -0800 Subject: [PATCH 3/4] Try another hacky approach for tearing down DBs after all tests Co-Authored-By: Nathan Sobo --- Cargo.lock | 1 + crates/server/Cargo.toml | 1 + crates/server/src/db.rs | 47 +++++++++++++++++++++++++++------------- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 11c9d229c8..1e9d43b2c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5850,6 +5850,7 @@ dependencies = [ "tide-compress", "time 0.2.25", "toml", + "util", "zed", ] diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index 2ce5a6342d..a4357415da 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -61,6 +61,7 @@ gpui = { path = "../gpui" } zed = { path = "../zed", features = ["test-support"] } ctor = "0.1" env_logger = "0.8" +util = { path = "../util" } lazy_static = "1.4" serde_json = { version = "1.0.64", features = ["preserve_order"] } diff --git a/crates/server/src/db.rs b/crates/server/src/db.rs index b50d7daf46..5ef070bba2 100644 --- a/crates/server/src/db.rs +++ b/crates/server/src/db.rs @@ -534,6 +534,7 @@ pub mod tests { Postgres, }; use std::{mem, path::Path}; + use util::ResultExt as _; pub struct TestDb { pub db: Option, @@ -545,13 +546,31 @@ pub mod tests { static ref POOL: Mutex> = Default::default(); } - #[ctor::dtor] - fn clear_pool() { - for db in POOL.lock().drain(..) { - db.teardown(); + use std::os::raw::c_int; + + extern "C" { + fn atexit(callback: extern "C" fn()) -> c_int; + } + + #[ctor::ctor] + fn init() { + unsafe { + atexit(teardown_db_pool); } } + extern "C" fn teardown_db_pool() { + std::thread::spawn(|| { + block_on(async move { + for db in POOL.lock().drain(..) { + db.teardown().await.log_err(); + } + }); + }) + .join() + .log_err(); + } + impl TestDb { pub fn new() -> Self { let mut pool = POOL.lock(); @@ -606,22 +625,20 @@ pub mod tests { }) } - fn teardown(mut self) { + async fn teardown(mut self) -> Result<()> { let db = self.db.take().unwrap(); - block_on(async { - let query = " + let query = " SELECT pg_terminate_backend(pg_stat_activity.pid) FROM pg_stat_activity WHERE pg_stat_activity.datname = '{}' AND pid <> pg_backend_pid(); "; - sqlx::query(query) - .bind(&self.name) - .execute(&db.pool) - .await - .unwrap(); - db.pool.close().await; - Postgres::drop_database(&self.url).await.unwrap(); - }); + sqlx::query(query) + .bind(&self.name) + .execute(&db.pool) + .await?; + db.pool.close().await; + Postgres::drop_database(&self.url).await?; + Ok(()) } } From 050f95149e85053c3279db9678fce5724c8c8d37 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 7 Feb 2022 18:30:09 -0800 Subject: [PATCH 4/4] Clear test db pool whenever no dbs are in use --- crates/server/src/db.rs | 55 +++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/crates/server/src/db.rs b/crates/server/src/db.rs index 5ef070bba2..a48673f30f 100644 --- a/crates/server/src/db.rs +++ b/crates/server/src/db.rs @@ -533,7 +533,11 @@ pub mod tests { migrate::{MigrateDatabase, Migrator}, Postgres, }; - use std::{mem, path::Path}; + use std::{ + mem, + path::Path, + sync::atomic::{AtomicUsize, Ordering::SeqCst}, + }; use util::ResultExt as _; pub struct TestDb { @@ -543,37 +547,14 @@ pub mod tests { } lazy_static! { - static ref POOL: Mutex> = Default::default(); - } - - use std::os::raw::c_int; - - extern "C" { - fn atexit(callback: extern "C" fn()) -> c_int; - } - - #[ctor::ctor] - fn init() { - unsafe { - atexit(teardown_db_pool); - } - } - - extern "C" fn teardown_db_pool() { - std::thread::spawn(|| { - block_on(async move { - for db in POOL.lock().drain(..) { - db.teardown().await.log_err(); - } - }); - }) - .join() - .log_err(); + static ref DB_POOL: Mutex> = Default::default(); + static ref DB_COUNT: AtomicUsize = Default::default(); } impl TestDb { pub fn new() -> Self { - let mut pool = POOL.lock(); + DB_COUNT.fetch_add(1, SeqCst); + let mut pool = DB_POOL.lock(); if let Some(db) = pool.pop() { db.truncate(); db @@ -628,10 +609,10 @@ pub mod tests { async fn teardown(mut self) -> Result<()> { let db = self.db.take().unwrap(); let query = " - SELECT pg_terminate_backend(pg_stat_activity.pid) - FROM pg_stat_activity - WHERE pg_stat_activity.datname = '{}' AND pid <> pg_backend_pid(); - "; + SELECT pg_terminate_backend(pg_stat_activity.pid) + FROM pg_stat_activity + WHERE pg_stat_activity.datname = '{}' AND pid <> pg_backend_pid(); + "; sqlx::query(query) .bind(&self.name) .execute(&db.pool) @@ -645,11 +626,19 @@ pub mod tests { impl Drop for TestDb { fn drop(&mut self) { if let Some(db) = self.db.take() { - POOL.lock().push(TestDb { + DB_POOL.lock().push(TestDb { db: Some(db), name: mem::take(&mut self.name), url: mem::take(&mut self.url), }); + if DB_COUNT.fetch_sub(1, SeqCst) == 1 { + block_on(async move { + let mut pool = DB_POOL.lock(); + for db in pool.drain(..) { + db.teardown().await.log_err(); + } + }); + } } } }