From d2dedcbb00bbaa3948ee559d34805bc9bb435a01 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 31 May 2026 23:15:40 +0200 Subject: [PATCH] fix(integration-test): ensure integration tests are runned on a separate DB to avoid polution --- .github/workflows/ci.yml | 30 ++------- justfile | 15 ++++- .../services/subject_group_service.rs | 7 +- .../pg/subject_group_pg_repository.rs | 8 +-- src/integration_test_support.rs | 67 +++++++++++++++++++ src/lib.rs | 5 ++ tests/common/init-test-schema.sh | 45 +++++++++++++ 7 files changed, 143 insertions(+), 34 deletions(-) create mode 100644 src/integration_test_support.rs create mode 100644 tests/common/init-test-schema.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3a4545d5..8713edb0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -125,37 +125,21 @@ jobs: - uses: Swatinem/rust-cache@v2 - name: Initialize test database - # Apply every migration in lexical order so the schema matches what - # the running server would set up (the app applies sqlx migrations on - # startup, but `cargo test` runs without going through main()). - run: | - set -e - for f in migrations/*.sql; do - echo "Applying $f" - psql -h localhost -U postgres -d oxicloud_test -v ON_ERROR_STOP=1 -f "$f" - done + # Applies every migration + seeds the integration-test admin row. + # Same script used by `just test-integration` locally. + run: bash tests/common/init-test-schema.sh env: + PGHOST: localhost + PGPORT: "5432" + PGUSER: postgres PGPASSWORD: postgres + PGDATABASE: oxicloud_test - name: Run tests run: cargo test --all-features --workspace env: DATABASE_URL: "postgres://postgres:postgres@localhost/oxicloud_test" - # ── Integration tests (PG-backed, gated on `--cfg integration_tests`) ── - # These tests connect to the live postgres service above to exercise - # CITEXT, XOR, recursive-CTE cycle/depth checks, and transactional - # grant-cleanup. They depend on at least one row in `auth.users` so - # the `first_admin()` helper resolves to a real UUID. - - name: Seed admin row for integration tests - run: | - psql -h localhost -U postgres -d oxicloud_test -v ON_ERROR_STOP=1 -c " - INSERT INTO auth.users (username, email, password_hash, role) - VALUES ('ci-admin', 'ci-admin@example.test', 'placeholder-not-validated', 'admin') - ON CONFLICT (username) DO NOTHING;" - env: - PGPASSWORD: postgres - - name: Run integration tests run: cargo test --all-features --workspace --tests env: diff --git a/justfile b/justfile index 6d33b655..9e77e0b1 100644 --- a/justfile +++ b/justfile @@ -25,10 +25,21 @@ test-mocks: # DB-dependent integration tests gated on `--cfg integration_tests`. # Spins up the test postgres on port 5433 first. Requires one row in -# auth.users (start the server against the test DB once to seed). +# auth.users on the test DB (start the server against it once to seed). +# +# IMPORTANT: DATABASE_URL is pinned to the test container on port 5433 +# so a stray DATABASE_URL in `.env` (which `set dotenv-load` at the top +# of this file would otherwise leak in) cannot point the tests at the +# real dev DB. The test pool helpers also refuse non-`oxicloud_test` +# URLs as defence in depth. test-integration: bash tests/common/spawn-db.sh - RUSTFLAGS='--cfg integration_tests' cargo test --workspace --tests + PGHOST=localhost PGPORT=5433 PGUSER=oxicloud_test PGPASSWORD=oxicloud_test \ + PGDATABASE=oxicloud_test \ + bash tests/common/init-test-schema.sh + DATABASE_URL='postgres://oxicloud_test:oxicloud_test@localhost:5433/oxicloud_test' \ + RUSTFLAGS='--cfg integration_tests' \ + cargo test --workspace --tests test-one name: cargo test {{name}} diff --git a/src/application/services/subject_group_service.rs b/src/application/services/subject_group_service.rs index bf0f01f9..12684b0f 100644 --- a/src/application/services/subject_group_service.rs +++ b/src/application/services/subject_group_service.rs @@ -398,16 +398,15 @@ mod integration_tests { use sqlx::Row; use sqlx::postgres::PgPoolOptions; - const DEFAULT_TEST_DB: &str = - "postgres://oxicloud_test:oxicloud_test@localhost:5433/oxicloud_test"; + use crate::integration_test_support::{ensure_clean_test_db, test_db_url}; async fn make_service() -> SubjectGroupService { - let url = std::env::var("DATABASE_URL").unwrap_or_else(|_| DEFAULT_TEST_DB.to_string()); let pool = PgPoolOptions::new() .max_connections(2) - .connect(&url) + .connect(&test_db_url()) .await .expect("connect to test DB — run tests/common/spawn-db.sh first"); + ensure_clean_test_db(&pool).await; let pool = Arc::new(pool); let repo = Arc::new(SubjectGroupPgRepository::new(pool.clone())); SubjectGroupService::new(repo, pool) diff --git a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs index 4a9c6442..39bdc61c 100644 --- a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs +++ b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs @@ -596,18 +596,16 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { #[allow(dead_code)] mod integration_tests { use super::*; + use crate::integration_test_support::{ensure_clean_test_db, test_db_url}; use sqlx::postgres::PgPoolOptions; - const DEFAULT_TEST_DB: &str = - "postgres://oxicloud_test:oxicloud_test@localhost:5433/oxicloud_test"; - async fn test_pool() -> Arc { - let url = std::env::var("DATABASE_URL").unwrap_or_else(|_| DEFAULT_TEST_DB.to_string()); let pool = PgPoolOptions::new() .max_connections(2) - .connect(&url) + .connect(&test_db_url()) .await .expect("connect to test DB — run tests/common/spawn-db.sh first"); + ensure_clean_test_db(&pool).await; Arc::new(pool) } diff --git a/src/integration_test_support.rs b/src/integration_test_support.rs new file mode 100644 index 00000000..8e2e2e3a --- /dev/null +++ b/src/integration_test_support.rs @@ -0,0 +1,67 @@ +//! Shared helpers for `#[cfg(integration_tests)]` test modules. +//! +//! Compiled only when the build is invoked with `--cfg integration_tests`. +//! The module exists so the OnceCell that guards pre-suite cleanup is +//! singleton across the whole lib test binary — without it, each test +//! file would have its own cell and module-B's first test could nuke +//! module-A's in-flight rows. + +use sqlx::PgPool; + +/// Substring the DATABASE_URL must contain for integration tests to +/// run. Both the local docker-compose-test database (`oxicloud_test` +/// on port 5433) and the GitHub Actions postgres service (`oxicloud_test` +/// on the default port) use this name — checking the substring is +/// portable across both. Port-based guards would break CI. +pub const TEST_DB_DISCRIMINATOR: &str = "oxicloud_test"; + +pub const DEFAULT_TEST_DB: &str = + "postgres://oxicloud_test:oxicloud_test@localhost:5433/oxicloud_test"; + +/// Resolve the test DB URL, panicking if it doesn't recognisably point +/// at a test database. Without this guard, a `DATABASE_URL` in `.env` +/// (loaded by `set dotenv-load` in the justfile) would leak into the +/// test run and mutate the real dev DB. +pub fn test_db_url() -> String { + let url = std::env::var("DATABASE_URL").unwrap_or_else(|_| DEFAULT_TEST_DB.to_string()); + assert!( + url.contains(TEST_DB_DISCRIMINATOR), + "DATABASE_URL ({url}) does not point to a test database \ + (expected substring '{TEST_DB_DISCRIMINATOR}'). Refusing to \ + run integration tests — they would mutate the real DB. Run \ + via `just test-integration`, or unset DATABASE_URL to use \ + the default test pool at port 5433." + ); + url +} + +/// Once-per-process cleanup of stale test rows from prior runs. +/// +/// Naming convention (`rust-test--`) keeps this LIKE scan +/// safe — no real group can match. Runs synchronously inside the +/// OnceCell so concurrent test threads block until the first caller +/// finishes; subsequent calls are zero-cost. +/// +/// Order matters: `storage.access_grants` rows go first because there's +/// no FK from there to `auth.subject_groups` (the service's `delete` +/// path does this transactionally; here we bypass the service). +static CLEANUP_ONCE: tokio::sync::OnceCell<()> = tokio::sync::OnceCell::const_new(); + +pub async fn ensure_clean_test_db(pool: &PgPool) { + CLEANUP_ONCE + .get_or_init(|| async { + let _ = sqlx::query( + "DELETE FROM storage.access_grants + WHERE subject_type = 'group' + AND subject_id IN ( + SELECT id FROM auth.subject_groups WHERE name LIKE 'rust-test-%' + )", + ) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM auth.subject_groups WHERE name LIKE 'rust-test-%'") + .execute(pool) + .await; + }) + .await; +} diff --git a/src/lib.rs b/src/lib.rs index 49996529..301ea930 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,6 +7,11 @@ pub mod domain; pub mod infrastructure; pub mod interfaces; +// Test-only helpers for #[cfg(integration_tests)] modules across the +// crate (shared pool URL guard + pre-suite cleanup OnceCell). +#[cfg(integration_tests)] +pub mod integration_test_support; + // Common public re-exports pub use application::services::folder_service::FolderService; pub use application::services::i18n_application_service::I18nApplicationService; diff --git a/tests/common/init-test-schema.sh b/tests/common/init-test-schema.sh new file mode 100644 index 00000000..434604fe --- /dev/null +++ b/tests/common/init-test-schema.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash +# Apply every migration in lexical order to a test database, then seed +# the minimum `auth.users` row that integration tests need. +# +# Connection parameters come from the libpq env vars (PGHOST, PGPORT, +# PGUSER, PGPASSWORD, PGDATABASE) so the same script works against: +# +# - the local docker-compose-test postgres on port 5433 +# (PGHOST=localhost PGPORT=5433 PGUSER=oxicloud_test +# PGPASSWORD=oxicloud_test PGDATABASE=oxicloud_test) +# +# - the CI postgres service on port 5432 +# (PGHOST=localhost PGPORT=5432 PGUSER=postgres +# PGPASSWORD=postgres PGDATABASE=oxicloud_test) +# +# The seed user is purely a placeholder so `first_admin()` in the Rust +# integration tests has a UUID to attach `added_by` to. The password +# hash is not a real argon2 hash — these tests never log in as this +# user, only reference its id. + +set -euo pipefail + +: "${PGHOST:?PGHOST must be set}" +: "${PGPORT:?PGPORT must be set}" +: "${PGUSER:?PGUSER must be set}" +: "${PGPASSWORD:?PGPASSWORD must be set}" +: "${PGDATABASE:?PGDATABASE must be set}" +export PGHOST PGPORT PGUSER PGPASSWORD PGDATABASE + +REPO_ROOT="$(cd "$(dirname "$0")/../.." && pwd)" + +echo "[init-schema] applying migrations to ${PGUSER}@${PGHOST}:${PGPORT}/${PGDATABASE}" +for f in "$REPO_ROOT"/migrations/*.sql; do + echo "[init-schema] $(basename "$f")" + psql -v ON_ERROR_STOP=1 -f "$f" >/dev/null +done + +echo "[init-schema] seeding ci-admin row (idempotent)" +psql -v ON_ERROR_STOP=1 -c " + INSERT INTO auth.users (username, email, password_hash, role) + VALUES ('ci-admin', 'ci-admin@example.test', 'placeholder-not-validated', 'admin') + ON CONFLICT (username) DO NOTHING; +" >/dev/null + +echo "[init-schema] done"