From 93381ba5cba91151db7e0f1b16b49d8972c247d6 Mon Sep 17 00:00:00 2001 From: Xevion Date: Wed, 7 Jan 2026 00:00:29 -0600 Subject: [PATCH] test(server): gate PostgreSQL tests behind postgres-tests feature Move testcontainers to optional dependency and create feature flag for database integration tests. Tests without the feature use a dummy pool and skip database operations. --- pacman-server/Cargo.toml | 6 +- pacman-server/src/data/pool.rs | 19 +++- pacman-server/src/data/user.rs | 11 ++- pacman-server/tests/common/mod.rs | 154 ++++++++++++++++++------------ pacman-server/tests/health.rs | 6 +- pacman-server/tests/oauth.rs | 38 +++++--- pacman-server/tests/sessions.rs | 12 ++- pacman/Justfile | 5 +- 8 files changed, 159 insertions(+), 92 deletions(-) diff --git a/pacman-server/Cargo.toml b/pacman-server/Cargo.toml index bdf54c8..fa46126 100644 --- a/pacman-server/Cargo.toml +++ b/pacman-server/Cargo.toml @@ -52,8 +52,13 @@ rustls = { version = "0.23", features = ["ring"] } image = { version = "0.25", features = ["png", "jpeg"] } sha2 = "0.10" mockall = "0.14.0" +testcontainers = { version = "0.25.0", optional = true } # validator = { version = "0.16", features = ["derive"] } +[features] +default = [] +postgres-tests = ["dep:testcontainers"] + [dev-dependencies] tokio = { version = "1", features = ["full"] } http = "1" @@ -63,7 +68,6 @@ bytes = "1" anyhow = "1" axum-test = "18.1.0" pretty_assertions = "1.4.1" -testcontainers = "0.25.0" bon = "3.7.2" cookie = "0.18.1" diff --git a/pacman-server/src/data/pool.rs b/pacman-server/src/data/pool.rs index 6bd068c..afe2322 100644 --- a/pacman-server/src/data/pool.rs +++ b/pacman-server/src/data/pool.rs @@ -10,7 +10,7 @@ pub type PgPool = Pool; /// - `database_url`: The database connection URL. /// - `max_connections`: Maximum number of connections in the pool. pub async fn create_pool(immediate: bool, database_url: &str, max_connections: u32) -> PgPool { - info!(immediate, "Connecting to PostgreSQL"); + info!(immediate, url = %redact_url(database_url), "Connecting to PostgreSQL"); let options = PgPoolOptions::new().max_connections(max_connections); @@ -26,13 +26,24 @@ pub async fn create_pool(immediate: bool, database_url: &str, max_connections: u } } -/// Create a dummy pool that will fail on any actual database operation. +/// Create a dummy PostgreSQL pool that will fail on any actual database operation. /// Used when database is not configured but the app still needs to start. pub fn create_dummy_pool() -> PgPool { - // This creates a pool with an invalid URL that will fail on actual use - // The pool itself can be created (lazy), but any operation will fail PgPoolOptions::new() .max_connections(1) .connect_lazy("postgres://invalid:invalid@localhost:5432/invalid") .expect("Failed to create dummy pool") } + +/// Redact password from database URL for logging. +fn redact_url(url: &str) -> String { + if let Some(at_pos) = url.find('@') { + if let Some(colon_pos) = url[..at_pos].rfind(':') { + let scheme_end = url.find("://").map(|p| p + 3).unwrap_or(0); + if colon_pos > scheme_end { + return format!("{}:***{}", &url[..colon_pos], &url[at_pos..]); + } + } + } + url.to_string() +} diff --git a/pacman-server/src/data/user.rs b/pacman-server/src/data/user.rs index 22aaf22..357bcd7 100644 --- a/pacman-server/src/data/user.rs +++ b/pacman-server/src/data/user.rs @@ -1,3 +1,4 @@ +use chrono::{DateTime, Utc}; use serde::Serialize; use sqlx::FromRow; @@ -7,8 +8,8 @@ use super::pool::PgPool; pub struct User { pub id: i64, pub email: Option, - pub created_at: chrono::DateTime, - pub updated_at: chrono::DateTime, + pub created_at: DateTime, + pub updated_at: DateTime, } #[derive(Debug, Clone, Serialize, FromRow)] @@ -21,8 +22,8 @@ pub struct OAuthAccount { pub username: Option, pub display_name: Option, pub avatar_url: Option, - pub created_at: chrono::DateTime, - pub updated_at: chrono::DateTime, + pub created_at: DateTime, + pub updated_at: DateTime, } pub async fn find_user_by_email(pool: &PgPool, email: &str) -> Result, sqlx::Error> { @@ -53,7 +54,7 @@ pub async fn link_oauth_account( INSERT INTO oauth_accounts (user_id, provider, provider_user_id, email, username, display_name, avatar_url) VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (provider, provider_user_id) - DO UPDATE SET email = EXCLUDED.email, username = EXCLUDED.username, display_name = EXCLUDED.display_name, avatar_url = EXCLUDED.avatar_url, user_id = EXCLUDED.user_id, updated_at = NOW() + DO UPDATE SET email = EXCLUDED.email, username = EXCLUDED.username, display_name = EXCLUDED.display_name, avatar_url = EXCLUDED.avatar_url, user_id = EXCLUDED.user_id, updated_at = CURRENT_TIMESTAMP RETURNING id, user_id, provider, provider_user_id, email, username, display_name, avatar_url, created_at, updated_at "#, ) diff --git a/pacman-server/tests/common/mod.rs b/pacman-server/tests/common/mod.rs index 176dc2b..0741f9c 100644 --- a/pacman-server/tests/common/mod.rs +++ b/pacman-server/tests/common/mod.rs @@ -3,34 +3,89 @@ use bon::builder; use pacman_server::{ app::{create_router, AppState}, auth::AuthRegistry, - config::{Config, DatabaseConfig, DiscordConfig, GithubConfig}, - data::pool::{create_dummy_pool, create_pool}, + config::{Config, DiscordConfig, GithubConfig}, + data::pool::create_dummy_pool, }; use std::sync::{Arc, Once}; +use tokio::sync::Notify; + +#[cfg(feature = "postgres-tests")] +use pacman_server::{config::DatabaseConfig, data::pool::create_pool}; +#[cfg(feature = "postgres-tests")] use testcontainers::{ core::{IntoContainerPort, WaitFor}, runners::AsyncRunner, ContainerAsync, GenericImage, ImageExt, }; -use tokio::sync::Notify; -use tracing::{debug, debug_span, Instrument}; +#[cfg(feature = "postgres-tests")] +use tracing::debug; -static CRYPTO_INIT: Once = Once::new(); +#[allow(dead_code)] +static INIT: Once = Once::new(); -/// Test configuration for integration tests -/// Do not destructure this struct if you need the database, it will be dropped implicitly, which will kill the database container prematurely. +/// Test configuration for integration tests. +/// Do not destructure this struct if you need the database container - it will be dropped +/// implicitly, which will kill the database container prematurely. #[allow(dead_code)] pub struct TestContext { pub config: Config, pub server: TestServer, pub app_state: AppState, - // Optional database container (only for Postgres tests) + /// Container handle (only present for PostgreSQL tests with postgres-tests feature) + #[cfg(feature = "postgres-tests")] pub container: Option>, } +#[allow(dead_code)] +fn init_once() { + INIT.call_once(|| { + rustls::crypto::ring::default_provider() + .install_default() + .expect("Failed to install default crypto provider"); + }); +} + +/// Create a PostgreSQL test database via testcontainers. +#[cfg(feature = "postgres-tests")] +async fn create_postgres_test_pool() -> (pacman_server::data::pool::PgPool, ContainerAsync) { + let db = "testdb"; + let user = "testuser"; + let password = "testpass"; + + let container_request = GenericImage::new("postgres", "15") + .with_exposed_port(5432.tcp()) + .with_wait_for(WaitFor::message_on_stderr( + "database system is ready to accept connections", + )) + .with_env_var("POSTGRES_DB", db) + .with_env_var("POSTGRES_USER", user) + .with_env_var("POSTGRES_PASSWORD", password); + + tracing::debug!(request_image = ?container_request.image(), "Acquiring postgres testcontainer"); + let start = std::time::Instant::now(); + let container = container_request.start().await.unwrap(); + let duration = start.elapsed(); + let host = container.get_host().await.unwrap(); + let port = container.get_host_port_ipv4(5432).await.unwrap(); + + tracing::debug!(host = %host, port = %port, duration = ?duration, "Test database ready"); + let url = format!("postgresql://{user}:{password}@{host}:{port}/{db}?sslmode=disable"); + + let pool = create_pool(false, &url, 5).await; + + // Run migrations for Postgres + sqlx::migrate!("./migrations") + .run(&pool) + .await + .expect("Failed to run database migrations"); + debug!("Database migrations ran successfully"); + + (pool, container) +} + #[builder] pub async fn test_context( - /// Whether to use a real PostgreSQL database via testcontainers (default: false) + /// Use real PostgreSQL via testcontainers (requires `postgres-tests` feature, default: false) #[builder(default = false)] use_database: bool, /// Optional custom AuthRegistry (otherwise built from config) @@ -42,41 +97,32 @@ pub async fn test_context( #[builder(default = true)] with_github: bool, ) -> TestContext { - CRYPTO_INIT.call_once(|| { - rustls::crypto::ring::default_provider() - .install_default() - .expect("Failed to install default crypto provider"); - }); + init_once(); // Set up logging std::env::set_var("RUST_LOG", "debug,sqlx=info"); pacman_server::logging::setup_logging(); - let (database_config, container) = if use_database { - let db = "testdb"; - let user = "testuser"; - let password = "testpass"; - - // Create container request - let container_request = GenericImage::new("postgres", "15") - .with_exposed_port(5432.tcp()) - .with_wait_for(WaitFor::message_on_stderr("database system is ready to accept connections")) - .with_env_var("POSTGRES_DB", db) - .with_env_var("POSTGRES_USER", user) - .with_env_var("POSTGRES_PASSWORD", password); - - tracing::debug!(request_image = ?container_request.image(), "Acquiring postgres testcontainer"); - let start = std::time::Instant::now(); - let container = container_request.start().await.unwrap(); - let duration: std::time::Duration = start.elapsed(); - let host = container.get_host().await.unwrap(); - let port = container.get_host_port_ipv4(5432).await.unwrap(); - - tracing::debug!(host = %host, port = %port, duration = ?duration, "Test database ready"); - let url = format!("postgresql://{user}:{password}@{host}:{port}/{db}?sslmode=disable"); - (Some(DatabaseConfig { url }), Some(container)) + // Create database pool based on configuration + #[cfg(feature = "postgres-tests")] + let (db, container, database_config, database_configured) = if use_database { + let (pool, container) = create_postgres_test_pool().await; + (pool, Some(container), Some(DatabaseConfig { url: "postgres://test".to_string() }), true) } else { - (None, None) + let pool = create_dummy_pool(); + (pool, None, None, false) + }; + + #[cfg(not(feature = "postgres-tests"))] + let (db, database_config, database_configured) = { + if use_database { + panic!( + "Database tests require the `postgres-tests` feature. \ + Run with: cargo test --features postgres-tests" + ); + } + let pool = create_dummy_pool(); + (pool, None, false) }; // Build OAuth configs if requested @@ -102,43 +148,26 @@ pub async fn test_context( database: database_config, discord, github, - s3: None, // Tests don't need S3 - port: 0, // Will be set by test server + s3: None, + port: 0, host: "127.0.0.1".parse().unwrap(), shutdown_timeout_seconds: 5, public_base_url: "http://localhost:3000".to_string(), jwt_secret: "test_jwt_secret_key_for_testing_only".to_string(), }; - // Create database pool - let db = if let Some(ref db_config) = config.database { - let pool = create_pool(false, &db_config.url, 5).await; - - // Run migrations for Postgres - sqlx::migrate!("./migrations") - .run(&pool) - .instrument(debug_span!("running_migrations")) - .await - .expect("Failed to run database migrations"); - debug!("Database migrations ran successfully"); - - pool - } else { - // Create dummy pool for tests that don't need database - create_dummy_pool() - }; - // Create auth registry - let auth = auth_registry.unwrap_or_else(|| AuthRegistry::new(&config).expect("Failed to create auth registry")); + let auth = + auth_registry.unwrap_or_else(|| AuthRegistry::new(&config).expect("Failed to create auth registry")); // Create app state let notify = Arc::new(Notify::new()); - let app_state = AppState::new_with_options(config.clone(), auth, db, notify, use_database).await; + let app_state = AppState::new_with_options(config.clone(), auth, db, notify, database_configured).await; - // Set health status + // Set health status based on database configuration { let mut health = app_state.health.write().await; - if use_database { + if database_configured { health.set_migrations(true); health.set_database(true); } @@ -152,6 +181,7 @@ pub async fn test_context( server, app_state, config, + #[cfg(feature = "postgres-tests")] container, } } diff --git a/pacman-server/tests/health.rs b/pacman-server/tests/health.rs index e0a789b..5fb52ed 100644 --- a/pacman-server/tests/health.rs +++ b/pacman-server/tests/health.rs @@ -1,11 +1,11 @@ mod common; -use pretty_assertions::assert_eq; - +#[cfg(feature = "postgres-tests")] use crate::common::{test_context, TestContext}; -/// Test health endpoint functionality with real database connectivity +/// Test health endpoint with PostgreSQL (requires postgres-tests feature) #[tokio::test] +#[cfg(feature = "postgres-tests")] async fn test_health_endpoint() { let TestContext { server, container, .. } = test_context().use_database(true).call().await; diff --git a/pacman-server/tests/oauth.rs b/pacman-server/tests/oauth.rs index 18c71bf..2246b25 100644 --- a/pacman-server/tests/oauth.rs +++ b/pacman-server/tests/oauth.rs @@ -2,19 +2,22 @@ use std::{collections::HashMap, sync::Arc}; use pacman_server::{ auth::{ - provider::{AuthUser, MockOAuthProvider, OAuthProvider}, + provider::{MockOAuthProvider, OAuthProvider}, AuthRegistry, }, - data::user as user_repo, session, }; use pretty_assertions::assert_eq; + +#[cfg(feature = "postgres-tests")] +use pacman_server::auth::provider::AuthUser; +#[cfg(feature = "postgres-tests")] use time::Duration; mod common; use crate::common::{test_context, TestContext}; -/// Test the basic authorization redirect flow +/// Test the basic authorization redirect flow (no database needed) #[tokio::test] async fn test_oauth_authorization_redirect() { let mut mock = MockOAuthProvider::new(); @@ -41,9 +44,12 @@ async fn test_oauth_authorization_redirect() { assert!(session::is_pkce_session(&claims), "A PKCE session should be set"); } -/// Test new user registration via OAuth callback +/// Test new user registration via OAuth callback (requires postgres-tests feature) #[tokio::test] +#[cfg(feature = "postgres-tests")] async fn test_new_user_registration() { + use pacman_server::data::user as user_repo; + let mut mock = MockOAuthProvider::new(); mock.expect_handle_callback().returning(|_, _, _, _| { Ok(AuthUser { @@ -80,9 +86,12 @@ async fn test_new_user_registration() { assert_eq!(providers[0].provider_user_id, "newuser123"); } -/// Test sign-in for an existing user with an already-linked provider +/// Test sign-in for an existing user with an already-linked provider (requires postgres-tests feature) #[tokio::test] +#[cfg(feature = "postgres-tests")] async fn test_existing_user_signin() { + use pacman_server::data::user as user_repo; + let mut mock = MockOAuthProvider::new(); mock.expect_handle_callback().returning(|_, _, _, _| { Ok(AuthUser { @@ -124,16 +133,19 @@ async fn test_existing_user_signin() { assert_eq!(response.headers().get("location").unwrap(), "/api/profile"); // Verify no new user was created - let users = sqlx::query("SELECT * FROM users") + let users: Vec<_> = sqlx::query("SELECT * FROM users") .fetch_all(&context.app_state.db) .await .unwrap(); assert_eq!(users.len(), 1, "No new user should be created"); } -/// Test implicit account linking via a shared verified email +/// Test implicit account linking via a shared verified email (requires postgres-tests feature) #[tokio::test] +#[cfg(feature = "postgres-tests")] async fn test_implicit_account_linking() { + use pacman_server::data::user as user_repo; + // 1. User signs in with 'provider-a' let mut mock_a = MockOAuthProvider::new(); mock_a.expect_handle_callback().returning(|_, _, _, _| { @@ -185,7 +197,7 @@ async fn test_implicit_account_linking() { assert_eq!(response2.status_code(), 302); // Assertions: No new user, but a new provider link - let users = sqlx::query("SELECT * FROM users") + let users: Vec<_> = sqlx::query("SELECT * FROM users") .fetch_all(&context.app_state.db) .await .unwrap(); @@ -197,9 +209,12 @@ async fn test_implicit_account_linking() { assert!(providers2.iter().any(|p| p.provider == "provider-b")); } -/// Test that an unverified email does NOT link accounts +/// Test that an unverified email does NOT link accounts (requires postgres-tests feature) #[tokio::test] +#[cfg(feature = "postgres-tests")] async fn test_unverified_email_creates_new_account() { + use pacman_server::data::user as user_repo; + let mut mock = MockOAuthProvider::new(); mock.expect_handle_callback().returning(|_, _, _, _| { Ok(AuthUser { @@ -228,15 +243,16 @@ async fn test_unverified_email_creates_new_account() { assert_eq!(response.status_code(), 302); // Should create a second user because the email wasn't trusted for linking - let users = sqlx::query("SELECT * FROM users") + let users: Vec<_> = sqlx::query("SELECT * FROM users") .fetch_all(&context.app_state.db) .await .unwrap(); assert_eq!(users.len(), 2, "A new user should be created for the unverified email"); } -/// Test logout functionality +/// Test logout functionality (requires postgres-tests feature) #[tokio::test] +#[cfg(feature = "postgres-tests")] async fn test_logout_functionality() { let mut mock = MockOAuthProvider::new(); mock.expect_handle_callback().returning(|_, _, _, _| { diff --git a/pacman-server/tests/sessions.rs b/pacman-server/tests/sessions.rs index cf6de61..1925c59 100644 --- a/pacman-server/tests/sessions.rs +++ b/pacman-server/tests/sessions.rs @@ -1,12 +1,14 @@ mod common; -use crate::common::test_context; -use cookie::Cookie; -use pacman_server::{data::user as user_repo, session}; - -use pretty_assertions::assert_eq; +/// Test session management (requires postgres-tests feature) #[tokio::test] +#[cfg(feature = "postgres-tests")] async fn test_session_management() { + use crate::common::test_context; + use cookie::Cookie; + use pacman_server::{data::user as user_repo, session}; + use pretty_assertions::assert_eq; + let context = test_context().use_database(true).call().await; // 1. Create a user and link a provider account diff --git a/pacman/Justfile b/pacman/Justfile index acdcfb2..29f8392 100644 --- a/pacman/Justfile +++ b/pacman/Justfile @@ -2,6 +2,9 @@ set shell := ["bash", "-c"] binary_extension := if os() == "windows" { ".exe" } else { "" } +default: + just --list + # Run cargo-vcpkg build for SDL2 dependencies vcpkg: cargo vcpkg build @@ -22,7 +25,7 @@ check: # Run tests with nextest for pacman package test: - cargo nextest run -p pacman --no-fail-fast + cargo nextest run --all --no-fail-fast # Auto-format code for pacman package format: