Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .config/nextest.toml
Comment thread
TheSentinel454 marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
nextest-version = "0.9.136"
# The PostgreSQL lane uses a run-scoped desired-state template database and a
# per-test wrapper, both of which require nextest's script support.
experimental = ["setup-scripts", "wrapper-scripts"]

[scripts.setup.postgres-template]
# Bootstrap the desired-state source database once per nextest invocation.
command = { command-line = "scripts/postgres-test-setup.sh", relative-to = "workspace-root" }
slow-timeout = "60s"

[scripts.wrapper.postgres-isolation]
# Clone or create a unique database for each test process, then drop it on exit.
command = { command-line = "scripts/postgres-test-wrapper.sh", relative-to = "workspace-root" }

[profile.postgres-ci]
# This structural convention keeps new PostgreSQL-backed tests discoverable
# without maintaining an exact list of test names.
default-filter = """
(test(/postgres_tests::/) or binary(/^postgres_/))
and not test(/(^|::)external_infra[^:]*::/)
"""
fail-fast = false
# Eight workers was the fastest stable setting in the Blox benchmark while the
# wrapper retained one database per concurrently running test process.
test-threads = 8

[test-groups.postgres-cluster-global]
# These tests inspect cluster-wide activity or create least-privilege sessions,
# so database-per-test isolation alone cannot make them independent.
max-threads = 1

[[profile.postgres-ci.overrides]]
filter = "test(/cluster_global_/)"
test-group = "postgres-cluster-global"

[[profile.postgres-ci.scripts]]
# Script filters are separate from default-filter: they attach the setup and
# isolation wrapper to the same automatically discovered test set.
filter = "(test(/postgres_tests::/) or binary(/^postgres_/)) and not test(/(^|::)external_infra[^:]*::/)"
setup = "postgres-template"
run-wrapper = "postgres-isolation"
222 changes: 93 additions & 129 deletions .github/workflows/ci.yml

Large diffs are not rendered by default.

58 changes: 58 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,64 @@ connections, NIP-42 auth, event ingestion, search indexing, and workflow
execution. `just test` starts Docker services automatically if they're not
already running.

### PostgreSQL-backed tests
Comment thread
TheSentinel454 marked this conversation as resolved.

PostgreSQL-backed tests run in a dedicated nextest lane. Mark them ignored with
a PostgreSQL reason and place them in a module whose name ends in
`postgres_tests`. Standalone integration-test targets use a `postgres_`
filename prefix instead. Tests that also require infrastructure beyond
PostgreSQL and Redis live under an `external_infra*_tests` module and are
excluded without changing their descriptive function names.

See the [buzz-db testing guide](crates/buzz-db/TESTING.md) for the crate-level
checklist.

`scripts/test-postgres-test-discovery.sh` enforces the convention across every
Rust source file. It fails CI when an ignored PostgreSQL test would be omitted,
or when a Redis-only or hybrid test is accidentally included, so module or file
renames cannot silently change lane membership. The archive and runner derive
their Cargo package set from the same markers, so a database test in a new crate
does not require a separate package-list update.

The `postgres-ci` nextest profile creates one database per test process, so
destructive and concurrent tests must use the database URL supplied through
`BUZZ_TEST_DATABASE_URL`, `TEST_DATABASE_URL`, or `DATABASE_URL`; do not
hard-code the shared development database. Ordinary tests receive the committed
desired-state schema from `schema/schema.sql`. Tests under
`migration::postgres_tests` receive an empty database and own the embedded
migration lifecycle. A test outside that module whose behavior intentionally
depends on migration-created triggers or seed rows uses a
`migration_schema_` function-name prefix and also receives an empty database
with `BUZZ_TEST_SCHEMA_MODE=migration`. Test helpers that normally call the
migrator honor `BUZZ_TEST_SCHEMA_MODE=desired` so the desired-state contract is
not re-migrated.

Tests that inspect cluster-wide PostgreSQL state or open least-privilege
sessions use a `cluster_global_` function-name segment; migration-backed cases
use `migration_schema_cluster_global_`. Nextest serializes this small group
while the database-isolated remainder stays parallel.

The setup process requires a PostgreSQL role that can create and drop databases
and owns the databases it creates; the harness itself does not require
superuser access. The complete inventory includes privilege-boundary tests that
create temporary roles and inspect all sessions, so grant that role
`CREATEROLE` and membership in `pg_read_all_stats` (or use an ephemeral
superuser, as CI does).
Set `BUZZ_POSTGRES_ADMIN_URL` to that role's maintenance database, and set
`PGHOST`, `PGPORT`, `PGUSER`, and `PGPASSWORD` for the desired-state
schema bootstrap. PostgreSQL client tools are resolved from `PATH` unless
`PG_BIN_DIR` is set. Tests that use Redis read `REDIS_URL`.

With native PostgreSQL and Redis running, the complete lane is below. The
runner bounds compilation to the packages discovered from the current source
tree and removes the run-scoped desired-state source database on exit.
Per-test and source-database cleanup retries transient PostgreSQL disconnect
races and emits a warning if all five attempts fail.

```bash
./scripts/postgres-test-run.sh
```

### End-to-End Tests

End-to-end tests live in `crates/buzz-test-client/tests/`:
Expand Down
2 changes: 1 addition & 1 deletion crates/buzz-audit/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ fn row_to_audit_entry(row: &sqlx::postgres::PgRow) -> Result<AuditEntry, AuditEr
}

#[cfg(test)]
mod tests {
mod postgres_tests {
use super::*;
use crate::action::AuditAction;
use crate::entry::NewAuditEntry;
Expand Down
63 changes: 63 additions & 0 deletions crates/buzz-db/TESTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# PostgreSQL-backed tests in buzz-db

The dedicated PostgreSQL CI lane discovers tests and Cargo packages by
structure rather than by exact lists. Follow this checklist so a new database
test is run automatically and remains safe under parallel execution.

## Adding a test

1. Put the test in a module whose name ends in `postgres_tests`.
2. Mark it `#[ignore = "requires Postgres"]` so infrastructure-free unit-test
jobs stay fast.
3. Connect through `crate::test_support::database_url()`. The CI wrapper sets
this helper's environment to a unique database for each test process; never
hard-code the shared development database.
4. Keep tests that need infrastructure beyond PostgreSQL and Redis in an
`external_infra*_tests` module. The PostgreSQL lane excludes those tests.
5. Run `scripts/test-postgres-test-discovery.sh` after adding or moving the
test. The same guard runs in CI immediately after changed-path detection.

The wrapper isolates destructive tests by dropping the entire per-test
database after the process exits. It does not `DELETE` rows or `TRUNCATE`
shared tables, so tests may run concurrently without coordinating cleanup.

## Choose the schema intentionally

Most tests use the committed desired-state schema from `schema/schema.sql`.
That is the default and is appropriate for data-access behavior.

Tests in `migration::postgres_tests` receive an empty database and own the
embedded migration lifecycle. A test outside that module that intentionally
depends on migration-created triggers or seed rows must prefix its function
name with `migration_schema_`; it also receives an empty database with
`BUZZ_TEST_SCHEMA_MODE=migration`.

Helpers that normally run migrations honor `BUZZ_TEST_SCHEMA_MODE=desired` in
the default lane. Do not rerun migrations against a desired-state database.
When behavior should match in both schema paths, add explicit desired-state and
migration-applied coverage rather than making the bootstrap implicit.

Tests that inspect cluster-wide PostgreSQL state or open least-privilege
sessions include `cluster_global_` in the function name. Migration-backed cases
use `migration_schema_cluster_global_`. Nextest serializes this small group
because separate databases still share `pg_stat_activity` and roles.

## Run the lane locally

Start native PostgreSQL and Redis, activate Hermit, and run:

```bash
. ./bin/activate-hermit
scripts/test-postgres-test-discovery.sh
scripts/postgres-test-run.sh
```

Set `BUZZ_POSTGRES_ADMIN_URL` to a PostgreSQL maintenance database owned by a
role that can create and drop databases. Set `PGHOST`, `PGPORT`, `PGUSER`, and
`PGPASSWORD` for desired-state bootstrap, plus `REDIS_URL` for Redis-backed
tests. The complete privilege-boundary inventory also needs `CREATEROLE` and
membership in `pg_read_all_stats`, or an ephemeral superuser as CI uses.

The runner creates one desired-state source database per invocation and clones
it for ordinary tests. Migration-mode tests start empty. Cleanup retries
transient disconnect races before reporting a warning.
3 changes: 3 additions & 0 deletions crates/buzz-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ mod store;
/// Database error types.
pub mod error;

#[cfg(test)]
mod test_support;

pub use runtime::{
insert_mentions, migration, replica_fence, Db, DbConfig, DbPoolStats, ReadSession,
};
Expand Down
2 changes: 1 addition & 1 deletion crates/buzz-db/src/runtime/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ async fn reject_legacy_nip_rs_cardinality_ambiguity(conn: &mut PgConnection) ->
}

#[cfg(test)]
mod tests {
mod postgres_tests {
use super::*;
use std::{
collections::BTreeSet,
Expand Down
3 changes: 2 additions & 1 deletion crates/buzz-db/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1041,4 +1041,5 @@ impl Db {
}

#[cfg(test)]
mod tests;
#[path = "tests.rs"]
mod postgres_tests;
18 changes: 14 additions & 4 deletions crates/buzz-db/src/runtime/observability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,6 @@ mod tests {
}
}

#[tokio::test(flavor = "current_thread")]
#[ignore = "requires Postgres"]
async fn pool_acquire_records_success_timeout_and_error_with_wait_time() {
let database_url = std::env::var("TEST_DATABASE_URL")
.unwrap_or_else(|_| "postgres://buzz:buzz_dev@localhost:5432/buzz".to_owned()); // sadscan:disable np.postgres.1 -- local test-only credentials
Expand Down Expand Up @@ -490,8 +488,6 @@ mod tests {
);
}

#[tokio::test(flavor = "current_thread")]
#[ignore = "requires Postgres"]
async fn advisory_lock_records_success_contention_timeout_and_error() {
let database_url = std::env::var("TEST_DATABASE_URL")
.unwrap_or_else(|_| "postgres://buzz:buzz_dev@localhost:5432/buzz".to_owned()); // sadscan:disable np.postgres.1 -- local test-only credentials
Expand Down Expand Up @@ -633,4 +629,18 @@ mod tests {
"lock timer must include the holder wait: {contention:?}"
);
}

mod postgres_tests {
#[tokio::test(flavor = "current_thread")]
#[ignore = "requires Postgres"]
async fn pool_acquire_records_success_timeout_and_error_with_wait_time() {
super::pool_acquire_records_success_timeout_and_error_with_wait_time().await;
}

#[tokio::test(flavor = "current_thread")]
#[ignore = "requires Postgres"]
async fn advisory_lock_records_success_contention_timeout_and_error() {
super::advisory_lock_records_success_contention_timeout_and_error().await;
}
}
}
50 changes: 30 additions & 20 deletions crates/buzz-db/src/runtime/replica_fence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,28 +789,22 @@ pub async fn run_probe(writer: PgPool, fence: Arc<ReplicaFence>) {
}

#[cfg(test)]
mod tests {
mod postgres_tests {
use super::*;

const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1

fn test_db_url() -> String {
std::env::var("TEST_DATABASE_URL").unwrap_or_else(|_| TEST_DB_URL.into())
}

/// A private scratch database with migrations applied: the probe tests
/// mutate the singleton heartbeat row (rewind/rotate), which must never
/// race the shared dev database or each other.
async fn scratch_db() -> (PgPool, PgPool, String) {
let admin = PgPool::connect(&test_db_url())
let admin = PgPool::connect(&crate::test_support::database_url())
.await
.expect("connect admin");
let name = format!("fence_probe_{}", uuid::Uuid::new_v4().simple());
sqlx::query(sqlx::AssertSqlSafe(format!("CREATE DATABASE {name}")))
.execute(&admin)
.await
.expect("create scratch db");
let base = test_db_url();
let base = crate::test_support::database_url();
let idx = base.rfind('/').expect("db url has a path segment");
let pool = PgPool::connect(&format!("{}/{}", &base[..idx], name))
.await
Expand Down Expand Up @@ -973,17 +967,27 @@ mod tests {
/// sessions, per the agreed classification.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn sample_writer_sees_open_transactions_and_ignores_idle() {
let pool = PgPool::connect(&test_db_url()).await.expect("connect");
async fn migration_schema_cluster_global_sample_writer_sees_open_transactions_and_ignores_idle()
{
let pool = PgPool::connect(&crate::test_support::database_url())
.await
.expect("connect");
crate::migration::run_migrations(&pool)
.await
.expect("apply migration schema");

// A plain idle session: pinned connection, no transaction.
let idle_pool = PgPool::connect(&test_db_url()).await.expect("connect idle");
let idle_pool = PgPool::connect(&crate::test_support::database_url())
.await
.expect("connect idle");
let _idle_conn = idle_pool.acquire().await.expect("idle conn");

let before = sample_writer(&pool).await.expect("sample without tx");

// Now hold a transaction open on a second connection.
let tx_pool = PgPool::connect(&test_db_url()).await.expect("connect tx");
let tx_pool = PgPool::connect(&crate::test_support::database_url())
.await
.expect("connect tx");
let mut tx = tx_pool.begin().await.expect("begin");
sqlx::query("SELECT 1")
.execute(&mut *tx)
Expand Down Expand Up @@ -1016,12 +1020,16 @@ mod tests {
/// never silently `MIN()` the hidden row away.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn sample_writer_fails_closed_when_activity_is_masked() {
let admin = PgPool::connect(&test_db_url()).await.expect("connect");
async fn cluster_global_sample_writer_fails_closed_when_activity_is_masked() {
let admin = PgPool::connect(&crate::test_support::database_url())
.await
.expect("connect");

// Hold a transaction open as the privileged user: this is the row
// the unprivileged probe must notice it cannot classify.
let tx_pool = PgPool::connect(&test_db_url()).await.expect("connect tx");
let tx_pool = PgPool::connect(&crate::test_support::database_url())
.await
.expect("connect tx");
let mut tx = tx_pool.begin().await.expect("begin");
sqlx::query("SELECT 1")
.execute(&mut *tx)
Expand All @@ -1038,7 +1046,7 @@ mod tests {
.await
.expect("create unprivileged role");

let base = test_db_url();
let base = crate::test_support::database_url();
let unpriv_url = {
let rest = base.strip_prefix("postgres://").expect("pg url");
let at = rest.rfind('@').expect("credentials in url");
Expand Down Expand Up @@ -1079,7 +1087,9 @@ mod tests {
#[tokio::test]
#[ignore = "requires Postgres"]
async fn aurora_identity_probe_reports_false_on_plain_postgres() {
let pool = PgPool::connect(&test_db_url()).await.expect("connect");
let pool = PgPool::connect(&crate::test_support::database_url())
.await
.expect("connect");
let mut conn = pool.acquire().await.expect("conn");
assert!(
!reader_supports_aurora_identity(&mut conn)
Expand All @@ -1100,7 +1110,7 @@ mod tests {
/// same database observes a token/epoch that resolves that entry.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn probe_commits_tokens_and_sessions_prove_coverage() {
async fn cluster_global_probe_commits_tokens_and_sessions_prove_coverage() {
let (admin, pool, name) = scratch_db().await;
let fence = ReplicaFence::new();

Expand Down Expand Up @@ -1155,7 +1165,7 @@ mod tests {
/// epoch — fails the epoch check instead of proving stale coverage.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn probe_rotates_epoch_on_same_epoch_token_regression() {
async fn cluster_global_probe_rotates_epoch_on_same_epoch_token_regression() {
let (admin, pool, name) = scratch_db().await;
let fence = ReplicaFence::new();

Expand Down
7 changes: 6 additions & 1 deletion crates/buzz-db/src/runtime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ async fn setup_db() -> Db {
let pool = PgPool::connect(&database_url)
.await
.expect("connect to test DB");
if std::env::var("BUZZ_TEST_SCHEMA_MODE").as_deref() == Ok("migration") {
crate::migration::run_migrations(&pool)
.await
.expect("apply migration schema");
}
Db::from_pool(pool)
}

Expand All @@ -28,7 +33,7 @@ async fn make_community(pool: &PgPool) -> Uuid {

#[tokio::test]
#[ignore = "requires Postgres"]
async fn database_guard_covers_legacy_writer_and_nip09_deletion() {
async fn migration_schema_database_guard_covers_legacy_writer_and_nip09_deletion() {
use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp};

let db = setup_db().await;
Expand Down
2 changes: 1 addition & 1 deletion crates/buzz-db/src/store/admin_moderation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ impl Db {
}

#[cfg(test)]
mod tests {
mod postgres_tests {
use super::*;

const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1
Expand Down
Loading
Loading