Files
buzz/scripts/postgres-test-wrapper.sh
Luke Tornquist bd73490418 ci: run PostgreSQL tests in isolated lane (#6730)
## Why

PR #6660 introduced four PostgreSQL-backed persistence tests and a
focused CI selector, but an exact test-name list does not automatically
cover future database tests. Broad ignored-test execution also exposed
shared-schema races and ambiguity between desired-state and
migration-applied schema expectations.

## What

- Establish a discoverable convention: PostgreSQL unit modules use
`postgres_tests`, PostgreSQL integration binaries use a `postgres_`
prefix, and unrelated external-infrastructure tests use an
`external_infra_` prefix.
- Add a dedicated nextest PostgreSQL profile and archive-backed CI job
covering the relevant crates without enumerating test names.
- Create a run-scoped desired-state source database and a unique
PostgreSQL database per test process for parallel-safe isolation, with
cleanup on success, failure, or interruption.
- Route destructive migration and migration-parity tests to clean
`template0` databases while desired-state tests clone the desired-state
database.
- Document discovery, schema modes, required role privileges, and a
bounded portable local runner.
- Keep the existing infrastructure-free unit-test jobs unchanged.

This is intentionally limited to test and CI harness behavior. It does
not move production database code, change persistence semantics, or
implement issue #20's broader shared test-utility refactor.

## Risk

Low production risk because all changes are confined to tests, CI
configuration, documentation, and test harness scripts.

Remaining operational risks:

- Cleanup retries `dropdb --force` five times across roughly four
seconds. Exhaustion warns with the database name but deliberately does
not mask the test result; individual PostgreSQL diagnostics are
suppressed.
- The seven-package boundary is duplicated between the runner and CI
archive and must remain synchronized if PostgreSQL tests move to a new
crate.
- Portable SHA fallback branches were exercised on Linux; no macOS Blox
workstation was needed for this Linux CI artifact.
- GitHub's unchanged generic Unit Tests job currently fails while
cold-linking `buzz-voice` because `sherpa-onnx-c-api` is absent. The
branch changes no voice/build/toolchain inputs; the exact parent
previously passed that job, and Blox passes the same infrastructure-free
suite (1,450/1,450). A prior-head one-job retry and the final-head run
both reproduced the hosted-runner failure.

## Verification

Author Blox workstation `2020088`:

- `cargo fmt --all -- --check`
- `just clippy`
- `just test-unit`: 1,450 passed; PostgreSQL tests remain skipped in
fast jobs
- PR #6660 focused tests: 4/4 passed through the final runner
- Full lane: 282/282 passed in 10.087s with a non-superuser role limited
to `CREATEDB`, `CREATEROLE`, and `pg_read_all_stats`
  - 272 desired-state tests
  - 10 migration-applied tests
  - 6 explicitly filtered external-infrastructure tests
- Cleanup fault injection: a deliberately underprivileged 282-test run
produced the expected 277 passes and 5 failures, and the post-run
catalog audit found zero lane databases after retry cleanup. The
restored successful run also left zero lane databases.

GitHub exact final head `92c231e48f299ea2af23817763f5ae80ac013d68`:

- [PostgreSQL
Tests](https://github.com/block/buzz/actions/runs/32789030203/job/97630827823):
282/282 passed across 10 binaries in 98.886s; 1,240 skipped, including 6
via the profile filter
- The shared relay/PostgreSQL archive predecessor completed successfully
- [Unit
Tests](https://github.com/block/buzz/actions/runs/32789030203/job/97626930420):
unrelated `sherpa-onnx-c-api` native-link failure described above
- Independent exact-head review on separate Blox workstation `2022762`:
no substantive findings after all initial findings were addressed. The
reviewer independently verified cleanup retry behavior, exit-status
preservation, shell portability, syntax, and a clean exact-head
worktree.

## References

- Stacked on #6660 at exact head
`561de54be4d9c2b622b7c2aa5b61bc3068f47e2a`
- https://github.com/TheSentinel454/buzz/issues/20
-
https://github.com/TheSentinel454/buzz/issues/20#issuecomment-5398526792

Generated with Codex.

## Update — August 24, 2026 review follow-up

- Centralized the repeated PostgreSQL test URL resolution in
crate-local, test-only helpers without introducing the broader shared
utility refactor from issue #20.
- Restored descriptive hybrid/Redis test function names. Structural
`external_infra_*_tests` modules now own exclusion, and the nextest
filter only recognizes module path segments.
- Added a three-second source guard that scans every Rust file and fails
CI when an ignored PostgreSQL test would be omitted or an
external-infrastructure test would be included. Fixture tests cover
accepted modules/binaries and both failure modes.
- Removed the two implementation-plan documents.

Performance profiling on Blox workstation `2027352` (same prebuilt
archive and cargo-nextest 0.9.143 for every comparison):

- Current per-test database model, 8 workers: 10.15–10.40s across three
runs. The 283 `createdb` calls and 284 `dropdb` calls consumed
28.85–29.46 aggregate seconds, about 39–40% of aggregate test-process
duration.
- Reusing one database per worker without cleanup: 6.55–6.84s, about 35%
faster, but all three runs failed because global matcher-queue tests
inherited a quiesced community from an earlier test; the failing test
varied with scheduling.
- Reusing workers with `TRUNCATE … RESTART IDENTITY CASCADE`: all 282
tests passed, but total time regressed to 10.47–10.67s. The 265 truncate
calls consumed 24.34–25.45 aggregate seconds, so truncation merely
replaced most clone/drop cost.
- Raising concurrency to 16 workers reduced one isolated-database run to
8.79s, but 24 workers exposed a cluster-global `pg_stat_activity` race.
The profile remains at the proven-safe 8 workers.

Decision: retain unique per-test databases. Harness cleanup uses `dropdb
--if-exists --force`; it drops the database rather than truncating
tables or deleting rows.

Verification at local branch head
`60086c1f6ee34a193c342255a5e8e6293b60e988`:

- Discovery inventory: exactly 282 intended tests; all 6 hybrid/Redis
tests present and structurally excluded.
- PR #6660 focused persistence tests: 4/4 passed.
- Full PostgreSQL lane: 282/282 passed three times; 1,240 skipped each
run; wall time 9.72–9.96s without profiling shims.
- Non-ignored tests with a desired-state database: 1,234/1,234 passed;
one mesh-demo timeout from the first run passed immediately in
isolation.
- `cargo fmt --all -- --check` and all-target/all-feature clippy for
`buzz-db`, `buzz-deletion`, and `buzz-relay` passed.

Final independent re-review at exact head
`42097c0136ab3fa8f0efe5720ae829c04575b9b6` on separate Blox workstation
`2028455`: no substantive residual findings. The reviewer independently
exercised ordinary strings, zero/one/three-hash raw strings, line/block
comments, and string-contained lookalikes, then reran the full 353-file
scan, shell syntax, Python AST parsing, and diff check.

Independent review found that the source guard's initial regular
expression could miss raw-string ignore reasons and treat a commented
attribute as real. Final head `42097c0136ab3fa8f0efe5720ae829c04575b9b6`
parses valid ordinary/raw Rust string literals only at attributes found
in comment-sanitized source; regression fixtures cover both cases. The
fixture suite, Python compilation, shell syntax, diff check, and full
353-file repository scan pass on Blox.

## Update — August 25, 2026 restack

PR #6660 merged, so this follow-up was rebased from its former exact
parent onto current `origin/main`
(`8d2d0ff5ad42733e9949442c4b6358d0ba87f9a8`). The final candidate head
is `05dcc2ab28948e3ab79bb44839a69f2ba44648a2`; the PR no longer carries
#6660's pre-squash history.

Fresh exact-head verification on Blox workstation `2028572`:

- Discovery guard passed across 353 Rust files; inventory remained
exactly 282 intended PostgreSQL tests.
- PR #6660 focused persistence tests: 4/4 passed.
- Full PostgreSQL lane: 282/282 passed three times; 1,243 skipped each
run; wall time 9.70–9.87s.
- `cargo fmt --all -- --check` and all-target/all-feature clippy for
`buzz-db`, `buzz-deletion`, and `buzz-relay` passed.

## Update — August 25, 2026 final hosted verification

- [GitHub PostgreSQL
Tests](https://github.com/block/buzz/actions/runs/32801679226/job/97667835080):
282/282 passed in 53.787s; 1,243 skipped.
- The final guard scanned all 353 Rust files successfully before the
hosted lane ran.
- The shared relay/archive prerequisite and infrastructure-free Unit
Tests job both passed at exact final head
`42097c0136ab3fa8f0efe5720ae829c04575b9b6`.

---------

Signed-off-by: Luke Tornquist <tornquist@squareup.com>
Signed-off-by: tornquist <tornquist@squareup.com>
2026-09-01 12:07:28 -04:00

93 lines
2.8 KiB
Bash
Executable File

#!/usr/bin/env bash
# Give each nextest process its own PostgreSQL database. The test binary and
# arguments supplied by nextest must always be executed by this wrapper.
set -euo pipefail
: "${NEXTEST_RUN_ID:?nextest must provide NEXTEST_RUN_ID}"
: "${NEXTEST_BINARY_ID:?nextest must provide NEXTEST_BINARY_ID}"
: "${NEXTEST_TEST_NAME:?nextest must provide NEXTEST_TEST_NAME}"
: "${NEXTEST_ATTEMPT_ID:?nextest must provide NEXTEST_ATTEMPT_ID}"
: "${BUZZ_POSTGRES_ADMIN_URL:?setup must provide BUZZ_POSTGRES_ADMIN_URL}"
: "${BUZZ_POSTGRES_DESIRED_TEMPLATE:?setup must provide BUZZ_POSTGRES_DESIRED_TEMPLATE}"
resolve_pg_command() {
local name="$1"
local candidate
if [[ -n "${PG_BIN_DIR:-}" ]]; then
candidate="${PG_BIN_DIR}/${name}"
else
candidate="$(command -v "$name" || true)"
fi
if [[ -z "$candidate" || ! -x "$candidate" ]]; then
echo "required PostgreSQL client is not executable: ${candidate:-$name}" >&2
exit 1
fi
printf '%s\n' "$candidate"
}
sha256_hex() {
if command -v sha256sum >/dev/null 2>&1; then
sha256sum | awk '{print $1}'
elif command -v shasum >/dev/null 2>&1; then
shasum -a 256 | awk '{print $1}'
elif command -v openssl >/dev/null 2>&1; then
openssl dgst -sha256 | awk '{print $NF}'
else
echo "a SHA-256 utility (sha256sum, shasum, or openssl) is required" >&2
return 1
fi
}
createdb="$(resolve_pg_command createdb)"
dropdb="$(resolve_pg_command dropdb)"
identity="${NEXTEST_RUN_ID}:${NEXTEST_BINARY_ID}:${NEXTEST_TEST_NAME}:${NEXTEST_ATTEMPT_ID}"
database_hash="$(printf '%s' "$identity" | sha256_hex)"
database_hash="${database_hash:0:24}"
database="buzz_nt_${database_hash}"
schema_mode="desired"
source_database="$BUZZ_POSTGRES_DESIRED_TEMPLATE"
# A leading separator lets the same patterns cover root and nested test paths.
qualified_test_name="::$NEXTEST_TEST_NAME"
# These tests own the migration lifecycle and intentionally begin empty.
case "$qualified_test_name" in
*::migration::postgres_tests::* | *::migration_schema_*)
schema_mode="migration"
source_database="template0"
;;
esac
cleanup() {
local attempt
for attempt in 1 2 3 4 5; do
if "$dropdb" --if-exists --force \
--maintenance-db="$BUZZ_POSTGRES_ADMIN_URL" \
"$database" >/dev/null 2>&1; then
return 0
fi
if [[ "$attempt" -lt 5 ]]; then
sleep 1
fi
done
echo "warning: failed to remove isolated PostgreSQL test database after 5 attempts: $database" >&2
return 0
}
trap cleanup EXIT
trap 'exit 129' HUP
trap 'exit 130' INT
trap 'exit 143' TERM
"$createdb" \
--maintenance-db="$BUZZ_POSTGRES_ADMIN_URL" \
--template="$source_database" \
"$database"
database_url="${BUZZ_POSTGRES_ADMIN_URL%/*}/$database"
export DATABASE_URL="$database_url"
export TEST_DATABASE_URL="$database_url"
export BUZZ_TEST_DATABASE_URL="$database_url"
export BUZZ_TEST_SCHEMA_MODE="$schema_mode"
"$@"