Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: better integration test db instantiation #1206

Merged
merged 12 commits into from
Jan 10, 2025
1 change: 0 additions & 1 deletion docker/docker-compose.ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ services:
environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_DB: signer
POSTGRES_HOST_AUTH_METHOD: trust
ports:
- "5432:5432"
Expand Down
35 changes: 17 additions & 18 deletions docker/docker-compose.test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ services:
environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_DB: signer
POSTGRES_HOST_AUTH_METHOD: trust
ports:
- "5432:5432"
Expand All @@ -41,23 +40,6 @@ services:
timeout: 1s
retries: 5

flyway:
image: flyway/flyway:10.13.0
command: >-
-url=jdbc:postgresql://postgres:5432/signer
-user=postgres
-password=postgres
-sqlMigrationPrefix=""
-connectRetries=60
migrate
volumes:
- ../signer/migrations:/flyway/sql
depends_on:
postgres:
condition: service_healthy
profiles:
- manual_start

# DynamoDB Tables for the Emily API.
emily-dynamodb:
image: "amazon/dynamodb-local:latest"
Expand Down Expand Up @@ -94,3 +76,20 @@ services:
- PORT=3031
ports:
- "3031:3031"

flyway:
image: flyway/flyway:10.13.0
command: >-
-url=jdbc:postgresql://postgres:5432/postgres
-user=postgres
-password=postgres
-sqlMigrationPrefix=""
-connectRetries=60
migrate
volumes:
- ../signer/migrations:/flyway/sql
depends_on:
postgres:
condition: service_healthy
profiles:
- manual_start
5 changes: 1 addition & 4 deletions signer/src/stacks/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1418,7 +1418,6 @@ mod tests {
use crate::stacks::wallet::get_full_tx_size;
use crate::storage::in_memory::Store;
use crate::storage::DbWrite;
use crate::testing::storage::DATABASE_NUM;

use clarity::types::Address;
use clarity::vm::types::{
Expand All @@ -1432,7 +1431,6 @@ mod tests {

use super::*;
use std::io::Read;
use std::sync::atomic::Ordering;

fn generate_wallet(num_keys: u16, signatures_required: u16) -> SignerWallet {
let network_kind = NetworkKind::Regtest;
Expand All @@ -1448,8 +1446,7 @@ mod tests {
#[ignore = "This is an integration test that hasn't been setup for CI yet"]
#[test(tokio::test)]
async fn fetch_unknown_ancestors_works() {
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = crate::testing::storage::new_test_database(db_num, true).await;
let db = crate::testing::storage::new_test_database().await;

let settings = Settings::new_from_default_config().unwrap();
// This is an integration test that will read from the config, which provides
Expand Down
76 changes: 24 additions & 52 deletions signer/src/testing/storage.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Test utilities for the `storage` module
use std::sync::atomic::{AtomicBool, AtomicU16, Ordering};
use std::time::Duration;

use crate::storage::model::BitcoinBlockHash;
Expand All @@ -11,12 +10,7 @@ pub mod model;
pub mod postgres;

/// The postgres connection string to the test database.
pub const DATABASE_URL: &str = "postgres://postgres:postgres@localhost:5432/signer";

/// This is needed to make sure that each test has as many isolated
/// databases as it needs.
pub static DATABASE_NUM: AtomicU16 = AtomicU16::new(0);
static INITIAL_MIGRATIONS_APPLIED: AtomicBool = AtomicBool::new(false);
pub const DATABASE_URL_BASE: &str = "postgres://postgres:postgres@localhost:5432";

/// It's better to create a new pool for each test since there is some
/// weird bug in sqlx. The issue that can crop up with pool reuse is
Expand Down Expand Up @@ -44,66 +38,44 @@ fn get_connection_pool(url: &str) -> sqlx::PgPool {
/// 2. Do the above, but have each transaction connect to its own
/// database. This actually works, and it's not clear why.
/// 3. Have each test use a new pool to a new database. This works as well.
pub async fn new_test_database(db_num: u16, apply_migrations: bool) -> PgStore {
let db_name = format!("test_db_{db_num}");

pub async fn new_test_database() -> PgStore {
// We create a new connection to the default database each time this
// function is called, because we depend on all connections to this
// database being closed before it begins.
let pool = get_connection_pool(DATABASE_URL);
let postgres_url = format!("{}/postgres", DATABASE_URL_BASE);
let pool = get_connection_pool(&postgres_url);

// We only need to apply the initial migrations to the `signer` database
// once. This is because the `signer` database is the template for all
// other databases.
if !INITIAL_MIGRATIONS_APPLIED.load(Ordering::SeqCst) {
PgStore::connect(DATABASE_URL)
.await
.expect("failed to apply initial migrations")
.apply_migrations()
.await
.expect("failed to apply db migrations");
INITIAL_MIGRATIONS_APPLIED.store(true, Ordering::SeqCst);
}
sqlx::query("CREATE SEQUENCE IF NOT EXISTS db_num_seq;")
.execute(&pool)
.await
.unwrap();

// We need to manually check if it exists and drop it if it does.
let db_exists = sqlx::query_scalar::<_, bool>(
"SELECT EXISTS (SELECT TRUE FROM pg_database WHERE datname = $1);",
)
.bind(&db_name)
.fetch_one(&pool)
.await
.unwrap();
let db_num: i64 = sqlx::query_scalar("SELECT nextval('db_num_seq');")
.fetch_one(&pool)
.await
.unwrap();

if db_exists {
// FORCE closes all connections to the database if there are any
// and then drops the database.
let drop_db = format!("DROP DATABASE \"{db_name}\" WITH (FORCE);");
sqlx::query(&drop_db)
.execute(&pool)
.await
.expect("failed to create test database");
}
let create_db = format!("CREATE DATABASE \"{db_name}\" TEMPLATE signer;");
let db_name = format!("signer_test_{}", db_num);

let create_db = format!("CREATE DATABASE \"{db_name}\" WITH OWNER = 'postgres';");

sqlx::query(&create_db)
.execute(&pool)
.await
.expect("failed to create test database");

let test_db_url = DATABASE_URL.replace("signer", &db_name);
let test_db_url = format!("{}/{}", DATABASE_URL_BASE, db_name);
// In order to create a new database from another database, there
// cannot exist any other connections to that database. So we
// explicitly close this connection. See the notes section in the docs
// <https://www.postgresql.org/docs/16/sql-createdatabase.html>
pool.close().await;

let store = PgStore::connect(&test_db_url).await.unwrap();
if apply_migrations {
store
.apply_migrations()
.await
.expect("failed to apply db migrations");
}
store
.apply_migrations()
.await
.expect("failed to apply db migrations");
store
}

Expand All @@ -113,12 +85,12 @@ pub async fn new_test_database(db_num: u16, apply_migrations: bool) -> PgStore {
pub async fn drop_db(store: PgStore) {
if let Some(db_name) = store.pool().connect_options().get_database() {
// This is not a test database, we should not close it
if db_name == "signer" {
if db_name == "postgres" {
return;
}
// Might as well.
store.pool().close().await;
let pool = get_connection_pool(DATABASE_URL);

let postgres_url = format!("{}/postgres", DATABASE_URL_BASE);
let pool = get_connection_pool(&postgres_url);

// FORCE closes all connections to the database if there are any
// and then drops the database.
Expand Down
20 changes: 6 additions & 14 deletions signer/tests/integration/bitcoin_validation.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::collections::HashSet;
use std::ops::Deref;
use std::sync::atomic::Ordering;

use bitcoin::hashes::Hash as _;
use rand::rngs::OsRng;
Expand All @@ -25,7 +24,6 @@ use signer::testing::context::*;

use crate::setup::{backfill_bitcoin_blocks, TestSignerSet};
use crate::setup::{DepositAmounts, TestSweepSetup2};
use crate::DATABASE_NUM;

const TEST_FEE_RATE: f64 = 10.0;
const TEST_CONTEXT_WINDOW: u16 = 1000;
Expand Down Expand Up @@ -92,8 +90,7 @@ impl AssertConstantInvariants for Vec<BitcoinTxValidationData> {
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn one_tx_per_request_set() {
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut rng = rand::rngs::StdRng::seed_from_u64(51);
let (rpc, faucet) = regtest::initialize_blockchain();

Expand Down Expand Up @@ -188,8 +185,7 @@ async fn one_tx_per_request_set() {
async fn one_invalid_deposit_invalidates_tx() {
let low_fee = 10;

let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut rng = rand::rngs::StdRng::seed_from_u64(51);
let (rpc, faucet) = regtest::initialize_blockchain();

Expand Down Expand Up @@ -304,8 +300,7 @@ async fn one_invalid_deposit_invalidates_tx() {
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn one_withdrawal_errors_validation() {
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut rng = rand::rngs::StdRng::seed_from_u64(51);
let (rpc, faucet) = regtest::initialize_blockchain();

Expand Down Expand Up @@ -375,8 +370,7 @@ async fn one_withdrawal_errors_validation() {
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn cannot_sign_deposit_is_ok() {
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut rng = rand::rngs::StdRng::seed_from_u64(51);
let (rpc, faucet) = regtest::initialize_blockchain();

Expand Down Expand Up @@ -541,8 +535,7 @@ async fn cannot_sign_deposit_is_ok() {
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn sighashes_match_from_sbtc_requests_object() {
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut rng = rand::rngs::StdRng::seed_from_u64(51);
let (rpc, faucet) = regtest::initialize_blockchain();

Expand Down Expand Up @@ -674,8 +667,7 @@ async fn sighashes_match_from_sbtc_requests_object() {
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn outcome_is_independent_of_input_order() {
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut rng = OsRng;
let (rpc, faucet) = regtest::initialize_blockchain();

Expand Down
19 changes: 6 additions & 13 deletions signer/tests/integration/block_observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ use crate::setup::TestSweepSetup;
use crate::transaction_coordinator::mock_reqwests_status_code_error;
use crate::utxo_construction::make_deposit_request;
use crate::zmq::BITCOIN_CORE_ZMQ_ENDPOINT;
use crate::DATABASE_NUM;

pub const GET_POX_INFO_JSON: &str =
include_str!("../../tests/fixtures/stacksapi-get-pox-info-test-data.json");
Expand All @@ -77,8 +76,7 @@ async fn load_latest_deposit_requests_persists_requests_from_past(blocks_ago: u6
// database.
let mut rng = rand::rngs::StdRng::seed_from_u64(51);
let (rpc, faucet) = regtest::initialize_blockchain();
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut ctx = TestContext::builder()
.with_storage(db.clone())
.with_first_bitcoin_core_client()
Expand Down Expand Up @@ -250,8 +248,7 @@ async fn load_latest_deposit_requests_persists_requests_from_past(blocks_ago: u6
async fn link_blocks() {
setup_logging("info", true);

let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;

let stacks_client = StacksClient::new(Url::parse("http://localhost:20443").unwrap()).unwrap();

Expand Down Expand Up @@ -396,8 +393,7 @@ async fn block_observer_stores_donation_and_sbtc_utxos() {

// 1. Create a database, an associated context for the block observer.

let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut ctx = TestContext::builder()
.with_storage(db.clone())
.with_first_bitcoin_core_client()
Expand Down Expand Up @@ -673,8 +669,7 @@ async fn block_observer_handles_update_limits(deployed: bool, sbtc_limits: SbtcL
// with a real bitcoin core client and a real connection to our
// database.
let (_, faucet) = regtest::initialize_blockchain();
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let mut ctx = TestContext::builder()
.with_storage(db.clone())
.with_first_bitcoin_core_client()
Expand Down Expand Up @@ -825,8 +820,7 @@ async fn next_headers_to_process_gets_all_headers() {
const START_HEIGHT: u64 = 103;

let (_, faucet) = regtest::initialize_blockchain();
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;

let ctx = TestContext::builder()
.with_storage(db.clone())
Expand Down Expand Up @@ -885,8 +879,7 @@ async fn next_headers_to_process_ignores_known_headers() {
// with a real bitcoin core client and a real connection to our
// database.
let (rpc, _) = regtest::initialize_blockchain();
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let db = testing::storage::new_test_database(db_num, true).await;
let db = testing::storage::new_test_database().await;
let context = TestContext::builder()
.with_storage(db.clone())
.with_first_bitcoin_core_client()
Expand Down
Loading
Loading