From 375a54ae6c15b3213ed4d0e210ef27040bdf2af5 Mon Sep 17 00:00:00 2001 From: Bretton Date: Wed, 29 Jul 2026 07:31:49 -0700 Subject: [PATCH] =?UTF-8?q?test:=20earn=20parallelism=20=E2=80=94=20343=20?= =?UTF-8?q?t.Parallel,=20both-dimension=20budget,=20-p=201=20for=20the=20s?= =?UTF-8?q?tream?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 task 9. Global-state audit: nothing needed conversion; four sites (63 calls) stay deliberately serial (env-parsing t.Setenv, slog capture). Nine internal straggler files migrate off the shared DB (goose is now extinct in test code). Three concurrency bugs surfaced that -p 1 had masked: a template-destruction race in testkit's own recovery tests (fixed via private templates with their own sweepable family); the legacy firehose loops' 30-second promise was worth exactly one 5-second deadline (gorilla corrupts on expiry; counter machinery deleted, one honest deadline, non-timeout errors terminate); and Jetstream account/identity events bypass wantedCollections filtering, so parallel signup storms starve every subscriber — measured 2/4 failures at -p 2 — which is the new, documented reason -p stays 1 until the legacy subscribers die. ConcurrencyBudget computes both -p and -parallel against max_connections with a nested-clone term and errors on undersized servers; the Makefile splice is proven fail-closed. Two-stream reviewed; 9 fixes applied. make ci green twice at 117s/128s — the clone tax is repaid and the gate beats its pre-clone baseline. -race and -shuffle clean; 3401 tests, 0 skips; peak 27/200 connections; 0 leaked clones or templates. Co-Authored-By: Claude Fable 5 --- Makefile | 40 ++- docker-compose.ci.yml | 2 +- docker-compose.dev.yml | 2 +- .../atproto/jetstream/bridged_stats_test.go | 150 ++------ .../jetstream/duplicate_delivery_test.go | 28 +- .../error_taxonomy_transient_test.go | 14 +- internal/atproto/jetstream/harness_test.go | 19 + .../atproto/jetstream/redrive_recency_test.go | 22 +- internal/atproto/jetstream/rev_gate_test.go | 81 ++--- .../atproto/jetstream/state_store_test.go | 54 +-- internal/atproto/oauth/harness_test.go | 19 + internal/atproto/oauth/store_test.go | 99 ++---- internal/core/unfurl/circuit_breaker_test.go | 7 + internal/core/unfurl/kagi_test.go | 8 + internal/core/unfurl/opengraph_test.go | 13 + internal/core/unfurl/providers_test.go | 1 + internal/db/postgres/harness_test.go | 19 + internal/db/postgres/user_repo_test.go | 219 +++--------- internal/db/postgres/vote_repo_test.go | 81 ++--- loop_state.md | 25 +- scripts/ci-runner.sh | 27 +- scripts/test-db-prepare.sh | 11 +- tests/integration/aggregator_e2e_test.go | 1 + .../aggregator_registration_test.go | 21 +- tests/integration/aggregator_test.go | 9 + .../author_avatar_hydration_test.go | 1 + tests/integration/author_posts_e2e_test.go | 15 +- tests/integration/blob_upload_e2e_test.go | 4 + .../block_handle_resolution_test.go | 2 + tests/integration/bluesky_post_test.go | 1 + tests/integration/comment_consumer_test.go | 9 + tests/integration/comment_e2e_test.go | 83 +++-- tests/integration/comment_query_test.go | 13 + tests/integration/comment_vote_test.go | 2 + tests/integration/comment_write_test.go | 45 +-- .../integration/community_avatar_e2e_test.go | 89 ++--- tests/integration/community_blocking_test.go | 4 + tests/integration/community_consumer_test.go | 4 + .../integration/community_credentials_test.go | 3 + tests/integration/community_e2e_test.go | 35 +- .../community_get_viewer_state_test.go | 1 + .../community_hostedby_security_test.go | 3 + .../community_identifier_resolution_test.go | 5 + .../community_list_viewer_state_test.go | 1 + .../community_provisioning_test.go | 16 + tests/integration/community_repo_test.go | 6 + .../community_service_integration_test.go | 3 + .../community_suggestion_e2e_test.go | 3 + .../integration/community_update_e2e_test.go | 47 +-- .../community_v2_validation_test.go | 2 + .../integration/concurrent_scenarios_test.go | 4 + tests/integration/discover_test.go | 10 + tests/integration/feed_test.go | 12 + tests/integration/helpers.go | 21 ++ tests/integration/identity_resolution_test.go | 3 + tests/integration/image_proxy_e2e_test.go | 7 + tests/integration/jetstream_consumer_test.go | 2 + tests/integration/oauth_e2e_test.go | 9 + .../oauth_session_fixation_test.go | 1 + .../oauth_session_handle_sync_test.go | 53 +-- .../oauth_token_verification_test.go | 1 + tests/integration/post_consumer_test.go | 1 + tests/integration/post_creation_test.go | 2 + tests/integration/post_delete_test.go | 59 ++-- tests/integration/post_e2e_test.go | 35 +- tests/integration/post_handler_test.go | 3 + .../integration/post_thumb_validation_test.go | 2 + tests/integration/post_unfurl_test.go | 4 + .../integration/subscription_indexing_test.go | 3 + tests/integration/timeline_test.go | 7 + tests/integration/token_refresh_test.go | 3 + tests/integration/user_journey_e2e_test.go | 43 ++- .../user_profile_avatar_e2e_test.go | 107 +++--- tests/integration/user_test.go | 11 + tests/integration/userblock_e2e_test.go | 2 + .../integration/userblock_enforcement_test.go | 5 + tests/integration/userblock_handler_test.go | 9 + tests/integration/userblock_indexing_test.go | 4 + tests/integration/userblock_repo_test.go | 8 + tests/integration/vote_e2e_test.go | 57 ++- tests/testkit/cmd/testdbprepare/main.go | 37 +- tests/testkit/db.go | 324 ++++++++++++------ tests/testkit/db_test.go | 143 +++++++- tests/testkit/harness_support_test.go | 65 ++++ 84 files changed, 1381 insertions(+), 1045 deletions(-) create mode 100644 internal/atproto/jetstream/harness_test.go create mode 100644 internal/atproto/oauth/harness_test.go create mode 100644 internal/db/postgres/harness_test.go diff --git a/Makefile b/Makefile index 3eb5345..7466147 100644 --- a/Makefile +++ b/Makefile @@ -153,9 +153,8 @@ test-integration: ## T1 integration tier - needs Postgres; starts postgres-test (echo "$(RED)✗ Container coves-test-postgres is not running, and 'compose up' did not start it.$(RESET)" && \ echo "$(RED) Rebuild it with 'make test-db-reset'.$(RESET)" && exit 1) @echo "Waiting for test database to accept connections..." - @# Provisions the template database that testkit.DB clones per test, migrates - @# the shared database the not-yet-migrated tests use, and sweeps clones - @# orphaned by killed runs. + @# Provisions the template database that testkit.DB clones per test and + @# sweeps clones orphaned by killed runs. @# @# This is also the readiness gate, and deliberately so: it waits by opening @# a real connection to POSTGRES_TEST_HOST:PORT — the same host endpoint the @@ -170,15 +169,23 @@ test-integration: ## T1 integration tier - needs Postgres; starts postgres-test @# The tag set is additive: an `integration` build contains the untagged @# unit files too, so this compiles and runs T0+T1 in one pass. @# - @# -p 1 runs packages sequentially: the legacy tests/integration setup wipes - @# shared test-DB tables (unscoped DELETEs), so package-parallel runs race - @# and randomly kill other packages' fixtures (jetstream DB tests above all). + @# Both concurrency flags come from the server's max_connections, because + @# they multiply: -p test binaries each running -parallel tests hold that + @# many clone pools at once. testkit.ConcurrencyBudget does the arithmetic; + @# hardcoding either number is how a suite discovers its ceiling by hitting + @# it, as a "too many clients" failure in whichever test was unlucky. @# - @# -parallel comes from the server's max_connections, because every test - @# under t.Parallel() holds its own clone pool. Inert until phase 3 enables - @# parallelism; wired now so the ceiling is never discovered by hitting it. - @go test -tags integration -p 1 -parallel $$(./scripts/test-db-prepare.sh --print-parallel) \ - ./cmd/... ./internal/... ./tests/... + @# Captured into a variable and checked, NOT spliced inline. A failed + @# $$(...) inside the go test line contributes an empty string and does not + @# fail the recipe, so go test would silently run at its DEFAULT -p + @# (GOMAXPROCS) — discarding the measured budget precisely when the thing + @# that measures it is broken. Fail-open on a safety limit is worse than + @# not having one, because it looks like it worked. + @set -e; \ + FLAGS=$$(./scripts/test-db-prepare.sh --print-flags) || exit 1; \ + [ -n "$$FLAGS" ] || { echo "$(RED)✗ test-db-prepare printed no concurrency flags$(RESET)"; exit 1; }; \ + echo " concurrency budget: $$FLAGS"; \ + go test -tags integration $$FLAGS ./cmd/... ./internal/... ./tests/... @echo "" @echo "$(YELLOW)Note: tests/integration needs a PDS and Jetstream as well as Postgres,$(RESET)" @echo "$(YELLOW)and its TestMain now says so up front — without 'make dev-up' the$(RESET)" @@ -212,8 +219,17 @@ test-db-reset: ## Reset test database @docker volume rm coves-test-postgres-data || true @docker-compose -f docker-compose.dev.yml --env-file .env.dev --profile test up -d postgres-test @echo "Waiting for PostgreSQL to be ready..." + @# Rebuilds the template testkit.DB clones. NOT `goose up` against + @# POSTGRES_TEST_DB: no test reads that database's schema any more — it is + @# only the maintenance database testkit connects to in order to CREATE and + @# DROP the others. Migrating it produced tables nothing queried and hid the + @# fact that the template, which every test actually clones, had not been + @# rebuilt at all. + @# + @# This also waits for the server, so the fixed sleep above is only for the + @# container process, not for Postgres accepting connections. @sleep 3 - @goose -dir internal/db/migrations postgres "postgresql://$(POSTGRES_TEST_USER):$(POSTGRES_TEST_PASSWORD)@localhost:$(POSTGRES_TEST_PORT)/$(POSTGRES_TEST_DB)?sslmode=disable" up || true + @./scripts/test-db-prepare.sh --force @echo "$(GREEN)✓ Test database reset$(RESET)" test-db-prepare: ## Create or refresh the template database that testkit.DB clones per test diff --git a/docker-compose.ci.yml b/docker-compose.ci.yml index 533cff8..5040580 100644 --- a/docker-compose.ci.yml +++ b/docker-compose.ci.yml @@ -91,7 +91,7 @@ services: # max_connections is raised from the default 100 because tests/testkit # clones a database per test and each clone opens its own small pool. The # ceiling on `go test -parallel` is derived from this number - # (testkit.ParallelBudget), so the two move together instead of the suite + # (testkit.ConcurrencyBudget), so the two move together instead of the suite # discovering the limit as "sorry, too many clients already" in whichever # test happened to be unlucky. command: ["-p", "5434", "-c", "max_connections=200"] diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 646c87a..1ecb68c 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -39,7 +39,7 @@ services: image: postgres:15 container_name: coves-test-postgres # Matches docker-compose.ci.yml: tests/testkit clones a database per test, - # each with its own pool, and testkit.ParallelBudget derives the safe + # each with its own pool, and testkit.ConcurrencyBudget derives the safe # `go test -parallel` value from this number. See the CI file for the full # reasoning. command: ["-c", "max_connections=200"] diff --git a/internal/atproto/jetstream/bridged_stats_test.go b/internal/atproto/jetstream/bridged_stats_test.go index d95aa51..935a5f3 100644 --- a/internal/atproto/jetstream/bridged_stats_test.go +++ b/internal/atproto/jetstream/bridged_stats_test.go @@ -5,25 +5,22 @@ package jetstream import ( "context" "database/sql" - "net/url" - "os" "testing" "time" "Coves/internal/atproto/identity" "Coves/internal/core/users" "Coves/internal/db/postgres" + "Coves/tests/testkit" _ "github.com/lib/pq" - "github.com/pressly/goose/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// These tests exercise the bridged-vote-stats support end-to-end against a local -// Postgres test database (the same container the rest of the postgres package tests -// use, port 5434 by default / TEST_DATABASE_URL). They are strictly local-only: no -// public PLC/relay/PDS/image hosts are contacted. +// These tests exercise the bridged-vote-stats support end-to-end against a +// private testkit clone of the migrated template. They are strictly local-only: +// no public PLC/relay/PDS/image hosts are contacted. // bridgeTrustForTests trusts only the bridge PDS host, so records from repos hosted // there may assert bridgedStats while every other repo is default-denied. @@ -31,49 +28,6 @@ func bridgeTrustForTests() *BridgeTrust { return NewBridgeTrust([]string{bridgedTestPDS}) } -// redactedDSN strips the password from a Postgres URL so a failure message can -// name the server it could not reach without copying the credential into the CI -// log. The test credentials are throwaway, but a log is the wrong place to -// practise leaking them. -func redactedDSN(dsn string) string { - u, err := url.Parse(dsn) - if err != nil { - return "(unparseable DSN)" - } - return u.Redacted() -} - -// setupBridgedTestDB connects to the local test database and runs migrations. -func setupBridgedTestDB(t *testing.T) *sql.DB { - t.Helper() - dsn := os.Getenv("TEST_DATABASE_URL") - if dsn == "" { - dsn = "postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" - } - db, err := sql.Open("postgres", dsn) - require.NoError(t, err, "Failed to connect to test database") - // Registered before the first thing that can fail, so the handle is closed - // even when Ping or the migration below calls FailNow. Callers still defer - // their own Close; database/sql tolerates the double close. - t.Cleanup(func() { _ = db.Close() }) - // Reaching this file at all means `-tags integration` was passed, which is - // a request for Postgres. An absent database is a failed run, not a - // shrunken one. - require.NoError(t, db.Ping(), - "test database not reachable at %s; bring it up with `make test-db-reset`", redactedDSN(dsn)) - require.NoError(t, goose.Up(db, "../../db/migrations"), "Failed to run migrations") - return db -} - -func cleanupBridgedTestData(t *testing.T, db *sql.DB) { - t.Helper() - _, _ = db.Exec("DELETE FROM votes WHERE voter_did LIKE $1", bridgedTestPrefix+"%") - _, _ = db.Exec("DELETE FROM comments WHERE commenter_did LIKE $1 OR root_uri LIKE $2", bridgedTestPrefix+"%", "at://"+bridgedTestPrefix+"%") - _, _ = db.Exec("DELETE FROM posts WHERE community_did LIKE $1", bridgedTestPrefix+"%") - _, _ = db.Exec("DELETE FROM communities WHERE did LIKE $1", bridgedTestPrefix+"%") - _, _ = db.Exec("DELETE FROM users WHERE did LIKE $1", bridgedTestPrefix+"%") -} - // insertBridgedUser inserts a user hosted on the trusted bridge PDS. func insertBridgedUser(t *testing.T, db *sql.DB, did, handle string) { t.Helper() @@ -148,10 +102,8 @@ func readPostRow(t *testing.T, db *sql.DB, uri string) (up, down, bridgedUp, bri } func TestPostConsumer_Create_WithBridgedStats(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -184,10 +136,8 @@ func TestPostConsumer_Create_WithBridgedStats(t *testing.T) { } func TestPostConsumer_CreateBeforeAuthorProfile_IndexesTrustedBridgeAuthor(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) // The community is already indexed, but the post author's profile event // has not arrived yet. This is the cross-repo ordering BigSky permits. @@ -223,10 +173,8 @@ func TestPostConsumer_CreateBeforeAuthorProfile_IndexesTrustedBridgeAuthor(t *te } func TestPostConsumer_Update_BridgedStats_NewerAsOfApplied(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -253,10 +201,8 @@ func TestPostConsumer_Update_BridgedStats_NewerAsOfApplied(t *testing.T) { } func TestPostConsumer_Update_StrictlyOlderIgnored_EqualApplied(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -287,10 +233,8 @@ func TestPostConsumer_Update_StrictlyOlderIgnored_EqualApplied(t *testing.T) { } func TestPostConsumer_Update_ReassignmentRejected(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedUser(t, db, bridgedTestOther, "brother.test") @@ -315,10 +259,8 @@ func TestPostConsumer_Update_ReassignmentRejected(t *testing.T) { } func TestPostConsumer_Update_SoftDeletedSkipped(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -342,10 +284,8 @@ func TestPostConsumer_Update_SoftDeletedSkipped(t *testing.T) { } func TestPostConsumer_Update_NonExistentSkipped(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -363,10 +303,8 @@ func TestPostConsumer_Update_NonExistentSkipped(t *testing.T) { } func TestPostConsumer_InclusiveScore_NativeVotesStackOnBridged(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -461,10 +399,8 @@ func setupCommentThread(t *testing.T, db *sql.DB) (postURI, postCID string) { } func TestCommentConsumer_Create_WithBridgedStats(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupCommentThread(t, db) cc := newCommentConsumer(db) @@ -493,10 +429,8 @@ func TestCommentConsumer_Create_WithBridgedStats(t *testing.T) { } func TestCommentConsumer_Update_AsOfGuard_AndInclusiveScore(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupCommentThread(t, db) cc := newCommentConsumer(db) @@ -559,10 +493,8 @@ func TestCommentConsumer_Update_AsOfGuard_AndInclusiveScore(t *testing.T) { // --- edited_at churn (fix 4) --- func TestPostConsumer_Update_StatsOnly_EditedAtUnchanged(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -593,10 +525,8 @@ func TestPostConsumer_Update_StatsOnly_EditedAtUnchanged(t *testing.T) { // --- provenance gate (fix 1a): untrusted repos cannot self-assert bridgedStats --- func TestPostConsumer_Create_UntrustedCommunity_BridgedStatsIgnored(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") // Community hosted on a NON-bridge PDS -> provenance gate denies bridgedStats. @@ -619,10 +549,8 @@ func TestPostConsumer_Create_UntrustedCommunity_BridgedStatsIgnored(t *testing.T } func TestCommentConsumer_Create_UntrustedCommenter_BridgedStatsIgnored(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupCommentThread(t, db) // Override the commenter to a non-bridge PDS -> provenance gate denies bridgedStats. @@ -646,10 +574,8 @@ func TestCommentConsumer_Create_UntrustedCommenter_BridgedStatsIgnored(t *testin // --- input hygiene (fix 1b): negative / over-cap aggregates are ignored whole --- func TestPostConsumer_Create_BridgedStatsHygiene(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, bridgedTestAuthor, "brauthor.test") insertBridgedCommunity(t, db, bridgedTestCommunity, "brcommunity.test", bridgedTestAuthor) @@ -686,10 +612,8 @@ func TestPostConsumer_Create_BridgedStatsHygiene(t *testing.T) { // --- comment update skips soft-deleted rows (fix 6) --- func TestCommentConsumer_Update_SoftDeletedSkipped(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupCommentThread(t, db) cc := newCommentConsumer(db) @@ -718,10 +642,8 @@ func TestCommentConsumer_Update_SoftDeletedSkipped(t *testing.T) { // --- comment resurrection score invariant (fix 5) --- func TestCommentConsumer_Resurrection_ScoreIncludesSurvivingNativeVotes(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupBridgedTestData(t, db) - cleanupBridgedTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupCommentThread(t, db) cc := newCommentConsumer(db) diff --git a/internal/atproto/jetstream/duplicate_delivery_test.go b/internal/atproto/jetstream/duplicate_delivery_test.go index f4dfcc6..b6bd32b 100644 --- a/internal/atproto/jetstream/duplicate_delivery_test.go +++ b/internal/atproto/jetstream/duplicate_delivery_test.go @@ -10,6 +10,7 @@ import ( "Coves/internal/core/users" "Coves/internal/db/postgres" + "Coves/tests/testkit" _ "github.com/lib/pq" "github.com/stretchr/testify/assert" @@ -32,15 +33,6 @@ const ( dupTestCommenter = dupTestPrefix + "commenter" ) -func cleanupDupTestData(t *testing.T, db *sql.DB) { - t.Helper() - _, _ = db.Exec("DELETE FROM votes WHERE voter_did LIKE $1", dupTestPrefix+"%") - _, _ = db.Exec("DELETE FROM comments WHERE commenter_did LIKE $1 OR root_uri LIKE $2", dupTestPrefix+"%", "at://"+dupTestPrefix+"%") - _, _ = db.Exec("DELETE FROM posts WHERE community_did LIKE $1", dupTestPrefix+"%") - _, _ = db.Exec("DELETE FROM communities WHERE did LIKE $1", dupTestPrefix+"%") - _, _ = db.Exec("DELETE FROM users WHERE did LIKE $1", dupTestPrefix+"%") -} - // setupDupFixtures indexes a user, community and one post, returning the post URI/CID. func setupDupFixtures(t *testing.T, db *sql.DB) (postURI, postCID string) { t.Helper() @@ -108,10 +100,8 @@ func readDupPostCounts(t *testing.T, db *sql.DB, uri string) (upvotes, downvotes } func TestVoteConsumer_DuplicateCreate_IncrementsExactlyOnce(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupDupTestData(t, db) - cleanupDupTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupDupFixtures(t, db) vc := NewVoteEventConsumer(postgres.NewVoteRepository(db), newMockUserService(), db) @@ -135,10 +125,8 @@ func TestVoteConsumer_DuplicateCreate_IncrementsExactlyOnce(t *testing.T) { } func TestVoteConsumer_DuplicateDelete_DecrementsExactlyOnce(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupDupTestData(t, db) - cleanupDupTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupDupFixtures(t, db) vc := NewVoteEventConsumer(postgres.NewVoteRepository(db), newMockUserService(), db) @@ -164,10 +152,8 @@ func TestVoteConsumer_DuplicateDelete_DecrementsExactlyOnce(t *testing.T) { } func TestCommentConsumer_DuplicateCreate_CountsExactlyOnce(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupDupTestData(t, db) - cleanupDupTestData(t, db) + t.Parallel() + db := testkit.DB(t) postURI, postCID := setupDupFixtures(t, db) cc := NewCommentEventConsumer(postgres.NewCommentRepository(db), db) diff --git a/internal/atproto/jetstream/error_taxonomy_transient_test.go b/internal/atproto/jetstream/error_taxonomy_transient_test.go index 7d93e06..311e602 100644 --- a/internal/atproto/jetstream/error_taxonomy_transient_test.go +++ b/internal/atproto/jetstream/error_taxonomy_transient_test.go @@ -7,6 +7,7 @@ import ( "testing" "Coves/internal/db/postgres" + "Coves/tests/testkit" _ "github.com/lib/pq" "github.com/stretchr/testify/assert" @@ -20,12 +21,11 @@ import ( // which fail before any repository access — stay in the unit tier. func TestPostConsumer_CommunityNotFound_IsTransient(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) + // The clone starts empty, so the community is absent by construction. const ghostCommunity = "did:plc:jstaxghostcommunity" - // Ensure the community really is absent. - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", ghostCommunity) c := NewPostEventConsumer(postgres.NewPostRepository(db), postgres.NewCommunityRepository(db), newMockUserService(), db) err := c.HandleEvent(context.Background(), taxonomyEvent( @@ -44,15 +44,13 @@ func TestPostConsumer_CommunityNotFound_IsTransient(t *testing.T) { } func TestCommunityConsumer_SubscriptionCommunityNotFound_IsTransient(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) const ( ghostCommunity = "did:plc:jstaxghostsubcomm" subscriber = "did:plc:jstaxsubscriber" ) - _, _ = db.Exec("DELETE FROM community_subscriptions WHERE user_did = $1", subscriber) - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", ghostCommunity) c := NewCommunityEventConsumer(postgres.NewCommunityRepository(db), "did:web:test.local", true, nil) err := c.HandleEvent(context.Background(), taxonomyEvent( diff --git a/internal/atproto/jetstream/harness_test.go b/internal/atproto/jetstream/harness_test.go new file mode 100644 index 0000000..8f9fd6b --- /dev/null +++ b/internal/atproto/jetstream/harness_test.go @@ -0,0 +1,19 @@ +//go:build integration + +package jetstream + +import ( + "os" + "testing" + + "Coves/tests/testkit" +) + +// TestMain sets the infrastructure floor for this package's integration build. +// +// It lives in a tagged file because a TestMain applies to the whole test +// binary: the untagged unit build of this package needs nothing out of +// process, and must not be made to probe Postgres before it can run. +func TestMain(m *testing.M) { + os.Exit(testkit.Main(m, testkit.RequirePostgres)) +} diff --git a/internal/atproto/jetstream/redrive_recency_test.go b/internal/atproto/jetstream/redrive_recency_test.go index 8421a48..6cdf763 100644 --- a/internal/atproto/jetstream/redrive_recency_test.go +++ b/internal/atproto/jetstream/redrive_recency_test.go @@ -10,6 +10,7 @@ import ( "Coves/internal/core/users" "Coves/internal/db/postgres" + "Coves/tests/testkit" _ "github.com/lib/pq" "github.com/stretchr/testify/assert" @@ -32,14 +33,6 @@ const ( recencyTestCommenter = recencyTestPrefix + "commenter" ) -func cleanupRecencyTestData(t *testing.T, db *sql.DB) { - t.Helper() - _, _ = db.Exec("DELETE FROM comments WHERE commenter_did LIKE $1 OR root_uri LIKE $2", recencyTestPrefix+"%", "at://"+recencyTestPrefix+"%") - _, _ = db.Exec("DELETE FROM posts WHERE community_did LIKE $1", recencyTestPrefix+"%") - _, _ = db.Exec("DELETE FROM communities WHERE did LIKE $1", recencyTestPrefix+"%") - _, _ = db.Exec("DELETE FROM users WHERE did LIKE $1", recencyTestPrefix+"%") -} - // recencyPostEvent builds a post commit event with an explicit Jetstream time_us. func recencyPostEvent(op, rkey, cid, title, content string, timeUS int64) *JetstreamEvent { var record map[string]interface{} @@ -107,10 +100,8 @@ func setupRecencyFixtures(t *testing.T, db *sql.DB) *PostEventConsumer { } func TestPostConsumer_StaleRedrivenUpdate_CannotRevertNewerContent(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRecencyTestData(t, db) - cleanupRecencyTestData(t, db) + t.Parallel() + db := testkit.DB(t) pc := setupRecencyFixtures(t, db) ctx := context.Background() @@ -159,10 +150,8 @@ func TestPostConsumer_StaleRedrivenUpdate_CannotRevertNewerContent(t *testing.T) } func TestCommentConsumer_StaleRedrivenUpdate_CannotRevertNewerContent(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRecencyTestData(t, db) - cleanupRecencyTestData(t, db) + t.Parallel() + db := testkit.DB(t) pc := setupRecencyFixtures(t, db) cc := NewCommentEventConsumer(postgres.NewCommentRepository(db), db) @@ -209,6 +198,7 @@ func TestCommentConsumer_StaleRedrivenUpdate_CannotRevertNewerContent(t *testing // guard on user profile events: an event older than the user row's last // successful write (users.updated_at) is skipped as success. func TestUserConsumer_StaleRedrivenProfileUpdate_Skipped(t *testing.T) { + t.Parallel() mockService := newMockUserService() lastWrite := time.Now() mockService.users["did:plc:rcyprofile"] = &users.User{ diff --git a/internal/atproto/jetstream/rev_gate_test.go b/internal/atproto/jetstream/rev_gate_test.go index 17ed3a9..fd94e26 100644 --- a/internal/atproto/jetstream/rev_gate_test.go +++ b/internal/atproto/jetstream/rev_gate_test.go @@ -10,6 +10,7 @@ import ( "Coves/internal/core/users" "Coves/internal/db/postgres" + "Coves/tests/testkit" _ "github.com/lib/pq" "github.com/stretchr/testify/assert" @@ -40,19 +41,6 @@ const ( revC = "3lrevtestaa2c" ) -func cleanupRevTestData(t *testing.T, db *sql.DB) { - t.Helper() - // Gate rows persist across runs BY DESIGN (they are tombstones); tests - // must clear their own or a re-run would reject its fixture creates. - _, _ = db.Exec("DELETE FROM jetstream_record_revs WHERE record_uri LIKE $1", "at://"+revTestPrefix+"%") - _, _ = db.Exec("DELETE FROM votes WHERE voter_did LIKE $1", revTestPrefix+"%") - _, _ = db.Exec("DELETE FROM comments WHERE commenter_did LIKE $1 OR root_uri LIKE $2", revTestPrefix+"%", "at://"+revTestPrefix+"%") - _, _ = db.Exec("DELETE FROM posts WHERE community_did LIKE $1", revTestPrefix+"%") - _, _ = db.Exec("DELETE FROM community_subscriptions WHERE user_did LIKE $1", revTestPrefix+"%") - _, _ = db.Exec("DELETE FROM communities WHERE did LIKE $1", revTestPrefix+"%") - _, _ = db.Exec("DELETE FROM users WHERE did LIKE $1", revTestPrefix+"%") -} - // revCommitEvent builds a commit event carrying a rev, the one field the // duplicate-delivery helpers omit. func revCommitEvent(did, collection, op, rkey, rev, cid string, timeUS int64, record map[string]interface{}) *JetstreamEvent { @@ -114,10 +102,8 @@ func revCommentRecord(content, rootURI, rootCID, parentURI, parentCID string) ma } func TestRevGate_AdvanceAndStalenessSemantics(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) ctx := context.Background() uri := "at://" + revTestPrefix + "gate/social.coves.feed.vote/g1" @@ -172,10 +158,8 @@ func TestRevGate_AdvanceAndStalenessSemantics(t *testing.T) { // NEWER time_us. Without the gate, the resurrection branch restores the // deleted comment permanently. func TestCommentConsumer_StaleCreateReplayAfterDelete_DoesNotResurrect(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) _, postURI, postCID := setupRevFixtures(t, db) cc := NewCommentEventConsumer(postgres.NewCommentRepository(db), db) @@ -212,10 +196,8 @@ func TestCommentConsumer_StaleCreateReplayAfterDelete_DoesNotResurrect(t *testin // still pass the gate and resurrect the row — proving the gate rejects only // stale copies, not the legitimate atProto recreate-same-rkey flow. func TestCommentConsumer_GenuineRecreateSameRKey_StillResurrects(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) _, postURI, postCID := setupRevFixtures(t, db) cc := NewCommentEventConsumer(postgres.NewCommentRepository(db), db) @@ -245,10 +227,8 @@ func TestCommentConsumer_GenuineRecreateSameRKey_StillResurrects(t *testing.T) { // time_us. The time-based recency guard passes it; only the rev gate rejects // it. Without the gate the content regresses until the next organic edit. func TestPostConsumer_StaleUpdateReplay_DoesNotClobberContent(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) pc, postURI, _ := setupRevFixtures(t, db) ctx := context.Background() @@ -280,10 +260,8 @@ func TestPostConsumer_StaleUpdateReplay_DoesNotClobberContent(t *testing.T) { // Same interleaving for comments. func TestCommentConsumer_StaleUpdateReplay_DoesNotClobberContent(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) _, postURI, postCID := setupRevFixtures(t, db) cc := NewCommentEventConsumer(postgres.NewCommentRepository(db), db) @@ -318,10 +296,8 @@ func TestCommentConsumer_StaleUpdateReplay_DoesNotClobberContent(t *testing.T) { // lagging feed replays the vote's create. Without the gate the vote row is // re-indexed and the count re-incremented, permanently. func TestVoteConsumer_StaleCreateReplayAfterDelete_NoPhantomVote(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) _, postURI, postCID := setupRevFixtures(t, db) vc := NewVoteEventConsumer(postgres.NewVoteRepository(db), newMockUserService(), db) @@ -358,10 +334,8 @@ func TestVoteConsumer_StaleCreateReplayAfterDelete_NoPhantomVote(t *testing.T) { // rev, so the create's late copy cannot index a vote whose record no longer // exists on the PDS. func TestVoteConsumer_DeleteBeforeCreate_TombstoneRejectsLateCreate(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) _, postURI, postCID := setupRevFixtures(t, db) vc := NewVoteEventConsumer(postgres.NewVoteRepository(db), newMockUserService(), db) @@ -396,10 +370,8 @@ func TestVoteConsumer_DeleteBeforeCreate_TombstoneRejectsLateCreate(t *testing.T // record's rev, so the create's late copy cannot index a post whose record no // longer exists on the PDS. func TestPostConsumer_DeleteBeforeCreate_TombstoneRejectsLateCreate(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) pc, _, _ := setupRevFixtures(t, db) ctx := context.Background() @@ -433,10 +405,8 @@ func TestPostConsumer_DeleteBeforeCreate_TombstoneRejectsLateCreate(t *testing.T // fast feed, then the lagging feed replays the original create with an older // rev but a NEWER time_us. The tombstoned delete rev must keep the post dead. func TestPostConsumer_StaleCreateReplayAfterDelete_DoesNotResurrect(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) // setupRevFixtures indexes revpost1 with revA / CID bafrevpost1. pc, postURI, _ := setupRevFixtures(t, db) @@ -473,12 +443,9 @@ func TestPostConsumer_StaleCreateReplayAfterDelete_DoesNotResurrect(t *testing.T // cross-feed profile replay that the wall-clock recency guard cannot: the // replay carries an OLDER rev but a NEWER time_us. func TestUserConsumer_StaleProfileUpdateReplay_DoesNotRegressProfile(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) - // The did shares revTestPrefix so cleanupRevTestData clears the gate row. const did = revTestPrefix + "profileuser" profileURI := "at://" + did + "/social.coves.actor.profile/self" @@ -514,10 +481,8 @@ func TestUserConsumer_StaleProfileUpdateReplay_DoesNotRegressProfile(t *testing. // only the gate's surviving row can reject the stale subscribe replay — this // is the case a per-table rev column could never cover. func TestCommunityConsumer_StaleSubscribeReplayAfterUnsubscribe_DoesNotResubscribe(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupRevTestData(t, db) - cleanupRevTestData(t, db) + t.Parallel() + db := testkit.DB(t) insertBridgedUser(t, db, revTestVoter, "revsubscriber.test") insertBridgedUser(t, db, revTestAuthor, "revauthor.test") diff --git a/internal/atproto/jetstream/state_store_test.go b/internal/atproto/jetstream/state_store_test.go index 1bc41e8..d1c8373 100644 --- a/internal/atproto/jetstream/state_store_test.go +++ b/internal/atproto/jetstream/state_store_test.go @@ -4,47 +4,22 @@ package jetstream import ( "context" - "database/sql" - "os" "testing" + "Coves/tests/testkit" + _ "github.com/lib/pq" - "github.com/pressly/goose/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// These tests exercise PostgresStateStore against the local test database -// (same container as the rest of the package's DB tests, port 5434 by -// default / TEST_DATABASE_URL). - -func setupStateStoreTestDB(t *testing.T) *sql.DB { - t.Helper() - dsn := os.Getenv("TEST_DATABASE_URL") - if dsn == "" { - dsn = "postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" - } - db, err := sql.Open("postgres", dsn) - require.NoError(t, err, "Failed to connect to test database") - // Registered before the first thing that can fail, so the handle is closed - // even when Ping or the migration below calls FailNow. - t.Cleanup(func() { - _, _ = db.Exec("DELETE FROM jetstream_cursors WHERE consumer_name LIKE 'statestore-test%'") - _, _ = db.Exec("DELETE FROM jetstream_dead_letters WHERE consumer_name LIKE 'statestore-test%'") - _ = db.Close() - }) - // Reaching this file at all means `-tags integration` was passed, which is - // a request for Postgres. An absent database is a failed run, not a - // shrunken one. - require.NoError(t, db.Ping(), - "test database not reachable at %s; bring it up with `make test-db-reset`", redactedDSN(dsn)) - require.NoError(t, goose.Up(db, "../../db/migrations"), "Failed to run migrations") - - return db -} +// These tests exercise PostgresStateStore against a private testkit clone of +// the migrated template, so each test starts from an empty schema and nothing +// it writes is visible to any other test. func TestPostgresStateStore_CursorLifecycle(t *testing.T) { - db := setupStateStoreTestDB(t) + t.Parallel() + db := testkit.DB(t) store := NewPostgresStateStore(db) ctx := context.Background() const consumer = "statestore-test-cursor" @@ -74,7 +49,8 @@ func TestPostgresStateStore_CursorLifecycle(t *testing.T) { } func TestPostgresStateStore_DeadLetterLifecycle(t *testing.T) { - db := setupStateStoreTestDB(t) + t.Parallel() + db := testkit.DB(t) store := NewPostgresStateStore(db) ctx := context.Background() const consumer = "statestore-test-dlq" @@ -124,7 +100,8 @@ func TestPostgresStateStore_DeadLetterLifecycle(t *testing.T) { // otherwise the failed dead-letter write tears down the connection without // advancing the cursor and the consumer replays the same frame forever. func TestPostgresStateStore_DeadLetterBinaryPayload(t *testing.T) { - db := setupStateStoreTestDB(t) + t.Parallel() + db := testkit.DB(t) store := NewPostgresStateStore(db) ctx := context.Background() const consumer = "statestore-test-binary" @@ -143,7 +120,8 @@ func TestPostgresStateStore_DeadLetterBinaryPayload(t *testing.T) { // that never advances the cursor) must not insert a fresh row — and a fresh // redrive budget — on every reconnect. func TestPostgresStateStore_DeadLetterDedup(t *testing.T) { - db := setupStateStoreTestDB(t) + t.Parallel() + db := testkit.DB(t) store := NewPostgresStateStore(db) ctx := context.Background() const consumer = "statestore-test-dedup" @@ -163,7 +141,8 @@ func TestPostgresStateStore_DeadLetterDedup(t *testing.T) { // Permanent failures are inserted with their redrive budget already // exhausted so the redriver never touches them. func TestPostgresStateStore_PermanentDeadLetterInsertedExhausted(t *testing.T) { - db := setupStateStoreTestDB(t) + t.Parallel() + db := testkit.DB(t) store := NewPostgresStateStore(db) ctx := context.Background() const consumer = "statestore-test-permanent" @@ -182,7 +161,8 @@ func TestPostgresStateStore_PermanentDeadLetterInsertedExhausted(t *testing.T) { // RetireDeadLetter exhausts a row in one step (used for unparseable payloads // discovered during redrive) while keeping it for forensics. func TestPostgresStateStore_RetireDeadLetter(t *testing.T) { - db := setupStateStoreTestDB(t) + t.Parallel() + db := testkit.DB(t) store := NewPostgresStateStore(db) ctx := context.Background() const consumer = "statestore-test-retire" diff --git a/internal/atproto/oauth/harness_test.go b/internal/atproto/oauth/harness_test.go new file mode 100644 index 0000000..6454a43 --- /dev/null +++ b/internal/atproto/oauth/harness_test.go @@ -0,0 +1,19 @@ +//go:build integration + +package oauth + +import ( + "os" + "testing" + + "Coves/tests/testkit" +) + +// TestMain sets the infrastructure floor for this package's integration build. +// +// It lives in a tagged file because a TestMain applies to the whole test +// binary: the untagged unit build of this package needs nothing out of +// process, and must not be made to probe Postgres before it can run. +func TestMain(m *testing.M) { + os.Exit(testkit.Main(m, testkit.RequirePostgres)) +} diff --git a/internal/atproto/oauth/store_test.go b/internal/atproto/oauth/store_test.go index c2aa153..306b543 100644 --- a/internal/atproto/oauth/store_test.go +++ b/internal/atproto/oauth/store_test.go @@ -4,47 +4,20 @@ package oauth import ( "context" - "database/sql" - "os" "testing" + "Coves/tests/testkit" + "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/bluesky-social/indigo/atproto/syntax" _ "github.com/lib/pq" - "github.com/pressly/goose/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// setupTestDB creates a test database connection and runs migrations -func setupTestDB(t *testing.T) *sql.DB { - dsn := os.Getenv("TEST_DATABASE_URL") - if dsn == "" { - dsn = "postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" - } - - db, err := sql.Open("postgres", dsn) - require.NoError(t, err, "Failed to connect to test database") - - // Run migrations - require.NoError(t, goose.Up(db, "../../db/migrations"), "Failed to run migrations") - - return db -} - -// cleanupOAuth removes all test OAuth data from the database -func cleanupOAuth(t *testing.T, db *sql.DB) { - _, err := db.Exec("DELETE FROM oauth_sessions WHERE did LIKE 'did:plc:test%'") - require.NoError(t, err, "Failed to cleanup oauth_sessions") - - _, err = db.Exec("DELETE FROM oauth_requests WHERE state LIKE 'test%'") - require.NoError(t, err, "Failed to cleanup oauth_requests") -} - func TestPostgresOAuthStore_SaveAndGetSession(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -89,9 +62,8 @@ func TestPostgresOAuthStore_SaveAndGetSession(t *testing.T) { } func TestPostgresOAuthStore_SaveSession_Upsert(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -142,8 +114,8 @@ func TestPostgresOAuthStore_SaveSession_Upsert(t *testing.T) { } func TestPostgresOAuthStore_GetSession_NotFound(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -156,9 +128,8 @@ func TestPostgresOAuthStore_GetSession_NotFound(t *testing.T) { } func TestPostgresOAuthStore_DeleteSession(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -192,8 +163,8 @@ func TestPostgresOAuthStore_DeleteSession(t *testing.T) { } func TestPostgresOAuthStore_DeleteSession_NotFound(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -206,9 +177,8 @@ func TestPostgresOAuthStore_DeleteSession_NotFound(t *testing.T) { } func TestPostgresOAuthStore_SaveAndGetAuthRequestInfo(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -250,9 +220,8 @@ func TestPostgresOAuthStore_SaveAndGetAuthRequestInfo(t *testing.T) { } func TestPostgresOAuthStore_SaveAuthRequestInfo_NoDID(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -281,8 +250,8 @@ func TestPostgresOAuthStore_SaveAuthRequestInfo_NoDID(t *testing.T) { } func TestPostgresOAuthStore_GetAuthRequestInfo_NotFound(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -292,9 +261,8 @@ func TestPostgresOAuthStore_GetAuthRequestInfo_NotFound(t *testing.T) { } func TestPostgresOAuthStore_DeleteAuthRequestInfo(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -324,8 +292,8 @@ func TestPostgresOAuthStore_DeleteAuthRequestInfo(t *testing.T) { } func TestPostgresOAuthStore_DeleteAuthRequestInfo_NotFound(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() @@ -335,21 +303,16 @@ func TestPostgresOAuthStore_DeleteAuthRequestInfo_NotFound(t *testing.T) { } func TestPostgresOAuthStore_CleanupExpiredSessions(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) storeInterface := NewPostgresOAuthStore(db, 0) // Use default TTL store, ok := storeInterface.(*PostgresOAuthStore) require.True(t, ok, "store should be *PostgresOAuthStore") ctx := context.Background() - // Pre-cleanup any expired sessions (from any source) to ensure the count - // assertion below returns exactly 1. cleanupOAuth only handles did:plc:test% - // records, but CleanupExpiredSessions operates on all expired sessions. - _, err := store.CleanupExpiredSessions(ctx) - require.NoError(t, err, "Failed to cleanup pre-existing expired sessions") - + // The clone starts empty, so the count assertion below sees only the + // sessions this test inserts. did1, err := syntax.ParseDID("did:plc:testexpired1") require.NoError(t, err) var did2 syntax.DID @@ -407,9 +370,8 @@ func TestPostgresOAuthStore_CleanupExpiredSessions(t *testing.T) { } func TestPostgresOAuthStore_CleanupExpiredAuthRequests(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) storeInterface := NewPostgresOAuthStore(db, 0) pgStore, ok := storeInterface.(*PostgresOAuthStore) @@ -467,9 +429,8 @@ func TestPostgresOAuthStore_CleanupExpiredAuthRequests(t *testing.T) { } func TestPostgresOAuthStore_MultipleSessions(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupOAuth(t, db) + t.Parallel() + db := testkit.DB(t) store := NewPostgresOAuthStore(db, 0) // Use default TTL ctx := context.Background() diff --git a/internal/core/unfurl/circuit_breaker_test.go b/internal/core/unfurl/circuit_breaker_test.go index 34cf01b..bbd4783 100644 --- a/internal/core/unfurl/circuit_breaker_test.go +++ b/internal/core/unfurl/circuit_breaker_test.go @@ -7,6 +7,7 @@ import ( ) func TestCircuitBreaker_Basic(t *testing.T) { + t.Parallel() cb := newCircuitBreaker() provider := "test-provider" @@ -26,6 +27,7 @@ func TestCircuitBreaker_Basic(t *testing.T) { } func TestCircuitBreaker_OpensAfterFailures(t *testing.T) { + t.Parallel() cb := newCircuitBreaker() provider := "failing-provider" @@ -45,6 +47,7 @@ func TestCircuitBreaker_OpensAfterFailures(t *testing.T) { } func TestCircuitBreaker_RecoveryAfterSuccess(t *testing.T) { + t.Parallel() cb := newCircuitBreaker() provider := "recovery-provider" @@ -68,6 +71,7 @@ func TestCircuitBreaker_RecoveryAfterSuccess(t *testing.T) { } func TestCircuitBreaker_HalfOpenTransition(t *testing.T) { + t.Parallel() cb := newCircuitBreaker() cb.openDuration = 100 * time.Millisecond // Short duration for testing provider := "half-open-provider" @@ -103,6 +107,7 @@ func TestCircuitBreaker_HalfOpenTransition(t *testing.T) { } func TestCircuitBreaker_MultipleProviders(t *testing.T) { + t.Parallel() cb := newCircuitBreaker() // Open circuit for provider A @@ -124,6 +129,7 @@ func TestCircuitBreaker_MultipleProviders(t *testing.T) { } func TestCircuitBreaker_GetStats(t *testing.T) { + t.Parallel() cb := newCircuitBreaker() // Record some activity @@ -150,6 +156,7 @@ func TestCircuitBreaker_GetStats(t *testing.T) { } func TestCircuitBreaker_FailureThresholdExact(t *testing.T) { + t.Parallel() cb := newCircuitBreaker() provider := "exact-threshold-provider" diff --git a/internal/core/unfurl/kagi_test.go b/internal/core/unfurl/kagi_test.go index f5e79a1..8e70bbb 100644 --- a/internal/core/unfurl/kagi_test.go +++ b/internal/core/unfurl/kagi_test.go @@ -12,6 +12,7 @@ import ( ) func TestFetchKagiKite_Success(t *testing.T) { + t.Parallel() // Mock Kagi HTML response mockHTML := ` @@ -46,6 +47,7 @@ func TestFetchKagiKite_Success(t *testing.T) { } func TestFetchKagiKite_NoImage(t *testing.T) { + t.Parallel() mockHTML := ` Test Story @@ -69,6 +71,7 @@ func TestFetchKagiKite_NoImage(t *testing.T) { } func TestFetchKagiKite_FallbackToTitle(t *testing.T) { + t.Parallel() mockHTML := ` Fallback Title @@ -94,6 +97,7 @@ func TestFetchKagiKite_FallbackToTitle(t *testing.T) { } func TestFetchKagiKite_ImageWithAltText(t *testing.T) { + t.Parallel() mockHTML := ` News Story @@ -120,6 +124,7 @@ func TestFetchKagiKite_ImageWithAltText(t *testing.T) { } func TestFetchKagiKite_HTTPError(t *testing.T) { + t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) })) @@ -135,6 +140,7 @@ func TestFetchKagiKite_HTTPError(t *testing.T) { } func TestFetchKagiKite_Timeout(t *testing.T) { + t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { time.Sleep(2 * time.Second) w.WriteHeader(http.StatusOK) @@ -150,6 +156,7 @@ func TestFetchKagiKite_Timeout(t *testing.T) { } func TestFetchKagiKite_MultipleImages_PicksSecond(t *testing.T) { + t.Parallel() mockHTML := ` Story with multiple images @@ -177,6 +184,7 @@ func TestFetchKagiKite_MultipleImages_PicksSecond(t *testing.T) { } func TestFetchKagiKite_OnlyNonKagiImages_NoMatch(t *testing.T) { + t.Parallel() mockHTML := ` Story with non-Kagi images diff --git a/internal/core/unfurl/opengraph_test.go b/internal/core/unfurl/opengraph_test.go index 2d51b91..d3b8a5f 100644 --- a/internal/core/unfurl/opengraph_test.go +++ b/internal/core/unfurl/opengraph_test.go @@ -12,6 +12,7 @@ import ( ) func TestParseOpenGraph_ValidTags(t *testing.T) { + t.Parallel() html := ` @@ -37,6 +38,7 @@ func TestParseOpenGraph_ValidTags(t *testing.T) { } func TestParseOpenGraph_MissingImage(t *testing.T) { + t.Parallel() html := ` @@ -57,6 +59,7 @@ func TestParseOpenGraph_MissingImage(t *testing.T) { } func TestParseOpenGraph_FallbackToTitle(t *testing.T) { + t.Parallel() html := ` @@ -76,6 +79,7 @@ func TestParseOpenGraph_FallbackToTitle(t *testing.T) { } func TestParseOpenGraph_PreferOpenGraphOverFallback(t *testing.T) { + t.Parallel() html := ` @@ -97,6 +101,7 @@ func TestParseOpenGraph_PreferOpenGraphOverFallback(t *testing.T) { } func TestParseOpenGraph_MalformedHTML(t *testing.T) { + t.Parallel() html := ` @@ -117,6 +122,7 @@ func TestParseOpenGraph_MalformedHTML(t *testing.T) { } func TestParseOpenGraph_Empty(t *testing.T) { + t.Parallel() html := ` @@ -134,6 +140,7 @@ func TestParseOpenGraph_Empty(t *testing.T) { } func TestFetchOpenGraph_Success(t *testing.T) { + t.Parallel() // Create test server with OpenGraph metadata server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Contains(t, r.Header.Get("User-Agent"), "CovesBot") @@ -169,6 +176,7 @@ func TestFetchOpenGraph_Success(t *testing.T) { } func TestFetchOpenGraph_HTTPError(t *testing.T) { + t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) })) @@ -182,6 +190,7 @@ func TestFetchOpenGraph_HTTPError(t *testing.T) { } func TestFetchOpenGraph_Timeout(t *testing.T) { + t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { time.Sleep(2 * time.Second) w.WriteHeader(http.StatusOK) @@ -195,6 +204,7 @@ func TestFetchOpenGraph_Timeout(t *testing.T) { } func TestFetchOpenGraph_NoMetadata(t *testing.T) { + t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { html := `

No metadata

` w.Header().Set("Content-Type", "text/html") @@ -215,6 +225,7 @@ func TestFetchOpenGraph_NoMetadata(t *testing.T) { } func TestIsOEmbedProvider(t *testing.T) { + t.Parallel() tests := []struct { url string expected bool @@ -238,6 +249,7 @@ func TestIsOEmbedProvider(t *testing.T) { } func TestIsSupported(t *testing.T) { + t.Parallel() tests := []struct { url string expected bool @@ -262,6 +274,7 @@ func TestIsSupported(t *testing.T) { } func TestGetAttr(t *testing.T) { + t.Parallel() html := `` doc, err := parseOpenGraph(html) require.NoError(t, err) diff --git a/internal/core/unfurl/providers_test.go b/internal/core/unfurl/providers_test.go index 6545fcd..cad3e56 100644 --- a/internal/core/unfurl/providers_test.go +++ b/internal/core/unfurl/providers_test.go @@ -7,6 +7,7 @@ import ( ) func TestNormalizeURL(t *testing.T) { + t.Parallel() tests := []struct { name string input string diff --git a/internal/db/postgres/harness_test.go b/internal/db/postgres/harness_test.go new file mode 100644 index 0000000..43b23e8 --- /dev/null +++ b/internal/db/postgres/harness_test.go @@ -0,0 +1,19 @@ +//go:build integration + +package postgres + +import ( + "os" + "testing" + + "Coves/tests/testkit" +) + +// TestMain sets the infrastructure floor for this package's integration build. +// +// It lives in a tagged file because a TestMain applies to the whole test +// binary: the untagged unit build of this package needs nothing out of +// process, and must not be made to probe Postgres before it can run. +func TestMain(m *testing.M) { + os.Exit(testkit.Main(m, testkit.RequirePostgres)) +} diff --git a/internal/db/postgres/user_repo_test.go b/internal/db/postgres/user_repo_test.go index 8d88fc5..93ac027 100644 --- a/internal/db/postgres/user_repo_test.go +++ b/internal/db/postgres/user_repo_test.go @@ -4,64 +4,18 @@ package postgres import ( "Coves/internal/core/users" + "Coves/tests/testkit" "context" "database/sql" "fmt" - "os" "testing" "time" _ "github.com/lib/pq" - "github.com/pressly/goose/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// setupUserTestDB creates a test database connection and runs migrations -func setupUserTestDB(t *testing.T) *sql.DB { - dsn := os.Getenv("TEST_DATABASE_URL") - if dsn == "" { - dsn = "postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" - } - - db, err := sql.Open("postgres", dsn) - require.NoError(t, err, "Failed to connect to test database") - - // Run migrations - require.NoError(t, goose.Up(db, "../../db/migrations"), "Failed to run migrations") - - return db -} - -// cleanupUserData removes all test data related to users -func cleanupUserData(t *testing.T, db *sql.DB, did string) { - // Clean up in reverse order of foreign key dependencies - _, err := db.Exec("DELETE FROM votes WHERE voter_did = $1", did) - require.NoError(t, err) - - _, err = db.Exec("DELETE FROM comments WHERE commenter_did = $1", did) - require.NoError(t, err) - - _, err = db.Exec("DELETE FROM community_blocks WHERE user_did = $1", did) - require.NoError(t, err) - - _, err = db.Exec("DELETE FROM community_memberships WHERE user_did = $1", did) - require.NoError(t, err) - - _, err = db.Exec("DELETE FROM community_subscriptions WHERE user_did = $1", did) - require.NoError(t, err) - - _, err = db.Exec("DELETE FROM oauth_requests WHERE did = $1", did) - require.NoError(t, err) - - _, err = db.Exec("DELETE FROM oauth_sessions WHERE did = $1", did) - require.NoError(t, err) - - // Posts are deleted by CASCADE when user is deleted - _, err = db.Exec("DELETE FROM users WHERE did = $1", did) - require.NoError(t, err) -} - // createTestCommunity creates a minimal test community for foreign key constraints func createTestCommunity(t *testing.T, db *sql.DB, did, handle, ownerDID string) { query := ` @@ -74,19 +28,13 @@ func createTestCommunity(t *testing.T, db *sql.DB, did, handle, ownerDID string) } func TestUserRepo_Delete_Success(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testdeleteuser123" testHandle := "testdeleteuser123.test" communityDID := "did:plc:testdeletecommunity" - defer cleanupUserData(t, db, testDID) - defer func() { - // Cleanup community - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", communityDID) - }() - repo := NewUserRepository(db) ctx := context.Background() @@ -181,8 +129,8 @@ func TestUserRepo_Delete_Success(t *testing.T) { } func TestUserRepo_Delete_NonExistentUser(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) repo := NewUserRepository(db) ctx := context.Background() @@ -193,8 +141,8 @@ func TestUserRepo_Delete_NonExistentUser(t *testing.T) { } func TestUserRepo_Delete_InvalidDID(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) repo := NewUserRepository(db) ctx := context.Background() @@ -206,14 +154,12 @@ func TestUserRepo_Delete_InvalidDID(t *testing.T) { } func TestUserRepo_Delete_Idempotent(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testdeletetwice" testHandle := "testdeletetwice.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -236,20 +182,13 @@ func TestUserRepo_Delete_Idempotent(t *testing.T) { } func TestUserRepo_Delete_WithPosts_CascadeDeletes(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testdeletewithposts" testHandle := "testdeletewithposts.test" communityDID := "did:plc:testpostcommunity" - defer cleanupUserData(t, db, testDID) - defer func() { - // Cleanup posts and community - _, _ = db.Exec("DELETE FROM posts WHERE author_did = $1", testDID) - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", communityDID) - }() - repo := NewUserRepository(db) ctx := context.Background() @@ -293,18 +232,16 @@ func TestUserRepo_Delete_WithPosts_CascadeDeletes(t *testing.T) { } func TestUserRepo_Delete_TransactionRollback(t *testing.T) { + t.Parallel() // This test verifies that if any part of the deletion fails, // the entire transaction is rolled back and no partial deletions occur. // We can't easily simulate a failure in the middle of the transaction, // but we verify that the function properly handles the transaction. - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + db := testkit.DB(t) testDID := "did:plc:testtransaction" testHandle := "testtransaction.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -331,14 +268,12 @@ func TestUserRepo_Delete_TransactionRollback(t *testing.T) { } func TestUserRepo_Create(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testcreateuser" testHandle := "testcreateuser.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -356,14 +291,12 @@ func TestUserRepo_Create(t *testing.T) { } func TestUserRepo_Create_DuplicateDID(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testduplicatedid" testHandle := "testduplicatedid.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -390,14 +323,12 @@ func TestUserRepo_Create_DuplicateDID(t *testing.T) { } func TestUserRepo_GetByDID(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testgetbydid" testHandle := "testgetbydid.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -418,8 +349,8 @@ func TestUserRepo_GetByDID(t *testing.T) { } func TestUserRepo_GetByDID_NotFound(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) repo := NewUserRepository(db) ctx := context.Background() @@ -429,14 +360,12 @@ func TestUserRepo_GetByDID_NotFound(t *testing.T) { } func TestUserRepo_GetByHandle(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testgetbyhandle" testHandle := "testgetbyhandle.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -457,15 +386,13 @@ func TestUserRepo_GetByHandle(t *testing.T) { } func TestUserRepo_UpdateHandle(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testupdatehandle" oldHandle := "testupdatehandle.test" newHandle := "newhandle.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -490,18 +417,13 @@ func TestUserRepo_UpdateHandle(t *testing.T) { } func TestUserRepo_GetProfileStats(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testprofilestats" testHandle := "testprofilestats.test" communityDID := "did:plc:teststatscommunity" - defer cleanupUserData(t, db, testDID) - defer func() { - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", communityDID) - }() - repo := NewUserRepository(db) ctx := context.Background() @@ -556,14 +478,12 @@ func TestUserRepo_GetProfileStats(t *testing.T) { } func TestUserRepo_Delete_WithOAuthRequests(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testoauthrequests" testHandle := "testoauthrequests.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -595,18 +515,13 @@ func TestUserRepo_Delete_WithOAuthRequests(t *testing.T) { } func TestUserRepo_Delete_WithCommunityBlocks(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testcommunityblocks" testHandle := "testcommunityblocks.test" communityDID := "did:plc:testblockcommunity" - defer cleanupUserData(t, db, testDID) - defer func() { - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", communityDID) - }() - repo := NewUserRepository(db) ctx := context.Background() @@ -641,24 +556,15 @@ func TestUserRepo_Delete_WithCommunityBlocks(t *testing.T) { } func TestUserRepo_Delete_TimingPerformance(t *testing.T) { + t.Parallel() // This test ensures deletion completes in a reasonable time // even with multiple related records - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + db := testkit.DB(t) testDID := "did:plc:testperformance" testHandle := "testperformance.test" communityDID := "did:plc:testperfcommunity" - // Clean up any leftover data from previous test runs - cleanupUserData(t, db, testDID) - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", communityDID) - - defer cleanupUserData(t, db, testDID) - defer func() { - _, _ = db.Exec("DELETE FROM communities WHERE did = $1", communityDID) - }() - repo := NewUserRepository(db) ctx := context.Background() @@ -693,13 +599,17 @@ func TestUserRepo_Delete_TimingPerformance(t *testing.T) { require.NoError(t, err) } - // Time the deletion + // No wall-clock assertion. This test now runs alongside dozens of parallel + // peers competing for the same Postgres, so elapsed time here measures the + // machine's load, not the query — and the failure it would produce is a + // flake that reads like a performance regression. What the test proves is + // that a cascade delete over this much related data completes correctly; + // the duration is logged for a human, not asserted. start := time.Now() err = repo.Delete(ctx, testDID) elapsed := time.Since(start) assert.NoError(t, err) - assert.Less(t, elapsed, 5*time.Second, "Deletion should complete in under 5 seconds") t.Logf("Deletion of user with %d comments and %d votes took %v", 10, 10, elapsed) } @@ -714,14 +624,12 @@ func stringPtr(s string) *string { } func TestUserRepo_UpdateProfile(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testupdateprofile" testHandle := "testupdateprofile.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -760,14 +668,12 @@ func TestUserRepo_UpdateProfile(t *testing.T) { } func TestUserRepo_UpdateProfile_PartialUpdate(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testpartialupdate" testHandle := "testpartialupdate.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -809,14 +715,12 @@ func TestUserRepo_UpdateProfile_PartialUpdate(t *testing.T) { } func TestUserRepo_UpdateProfile_ReturnsUpdatedUser(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testreturnsupdated" testHandle := "testreturnsupdated.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -851,8 +755,8 @@ func TestUserRepo_UpdateProfile_ReturnsUpdatedUser(t *testing.T) { } func TestUserRepo_UpdateProfile_UserNotFound(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) repo := NewUserRepository(db) ctx := context.Background() @@ -867,14 +771,12 @@ func TestUserRepo_UpdateProfile_UserNotFound(t *testing.T) { } func TestUserRepo_UpdateProfile_ClearFields(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testclearfields" testHandle := "testclearfields.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -916,14 +818,12 @@ func TestUserRepo_UpdateProfile_ClearFields(t *testing.T) { } func TestUserRepo_GetByDID_ReturnsNewFields(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testgetbydidnewfields" testHandle := "testgetbydidnewfields.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -965,14 +865,12 @@ func TestUserRepo_GetByDID_ReturnsNewFields(t *testing.T) { } func TestUserRepo_GetByHandle_ReturnsNewFields(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID := "did:plc:testgetbyhandlenewfields" testHandle := "testgetbyhandlenewfields.test" - defer cleanupUserData(t, db, testDID) - repo := NewUserRepository(db) ctx := context.Background() @@ -1014,8 +912,8 @@ func TestUserRepo_GetByHandle_ReturnsNewFields(t *testing.T) { } func TestUpdateProfile_InvalidDID(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) repo := NewUserRepository(db) ctx := context.Background() @@ -1032,17 +930,14 @@ func TestUpdateProfile_InvalidDID(t *testing.T) { } func TestUserRepo_GetByDIDs_ReturnsNewFields(t *testing.T) { - db := setupUserTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) testDID1 := "did:plc:testgetbydidsbatch1" testHandle1 := "testgetbydidsbatch1.test" testDID2 := "did:plc:testgetbydidsbatch2" testHandle2 := "testgetbydidsbatch2.test" - defer cleanupUserData(t, db, testDID1) - defer cleanupUserData(t, db, testDID2) - repo := NewUserRepository(db) ctx := context.Background() diff --git a/internal/db/postgres/vote_repo_test.go b/internal/db/postgres/vote_repo_test.go index 6a00518..26eaab7 100644 --- a/internal/db/postgres/vote_repo_test.go +++ b/internal/db/postgres/vote_repo_test.go @@ -4,43 +4,17 @@ package postgres import ( "Coves/internal/core/votes" + "Coves/tests/testkit" "context" "database/sql" - "os" "testing" "time" _ "github.com/lib/pq" - "github.com/pressly/goose/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// setupTestDB creates a test database connection and runs migrations -func setupTestDB(t *testing.T) *sql.DB { - dsn := os.Getenv("TEST_DATABASE_URL") - if dsn == "" { - dsn = "postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" - } - - db, err := sql.Open("postgres", dsn) - require.NoError(t, err, "Failed to connect to test database") - - // Run migrations - require.NoError(t, goose.Up(db, "../../db/migrations"), "Failed to run migrations") - - return db -} - -// cleanupVotes removes all test votes and users from the database -func cleanupVotes(t *testing.T, db *sql.DB) { - _, err := db.Exec("DELETE FROM votes WHERE voter_did LIKE 'did:plc:test%' OR voter_did LIKE 'did:plc:nonexistent%'") - require.NoError(t, err, "Failed to cleanup votes") - - _, err = db.Exec("DELETE FROM users WHERE did LIKE 'did:plc:test%'") - require.NoError(t, err, "Failed to cleanup test users") -} - // createTestUser creates a minimal test user for foreign key constraints func createTestUser(t *testing.T, db *sql.DB, handle, did string) { query := ` @@ -53,9 +27,8 @@ func createTestUser(t *testing.T, db *sql.DB, handle, did string) { } func TestVoteRepo_Create(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -82,9 +55,8 @@ func TestVoteRepo_Create(t *testing.T) { } func TestVoteRepo_Create_Idempotent(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -124,9 +96,8 @@ func TestVoteRepo_Create_Idempotent(t *testing.T) { } func TestVoteRepo_Create_VoterNotFound(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -154,9 +125,8 @@ func TestVoteRepo_Create_VoterNotFound(t *testing.T) { } func TestVoteRepo_GetByURI(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -188,8 +158,8 @@ func TestVoteRepo_GetByURI(t *testing.T) { } func TestVoteRepo_GetByURI_NotFound(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -199,9 +169,8 @@ func TestVoteRepo_GetByURI_NotFound(t *testing.T) { } func TestVoteRepo_GetByVoterAndSubject(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -234,8 +203,8 @@ func TestVoteRepo_GetByVoterAndSubject(t *testing.T) { } func TestVoteRepo_GetByVoterAndSubject_NotFound(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -245,9 +214,8 @@ func TestVoteRepo_GetByVoterAndSubject_NotFound(t *testing.T) { } func TestVoteRepo_Delete(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -284,9 +252,8 @@ func TestVoteRepo_Delete(t *testing.T) { } func TestVoteRepo_Delete_Idempotent(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -317,9 +284,8 @@ func TestVoteRepo_Delete_Idempotent(t *testing.T) { } func TestVoteRepo_ListBySubject(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() @@ -363,9 +329,8 @@ func TestVoteRepo_ListBySubject(t *testing.T) { } func TestVoteRepo_ListByVoter(t *testing.T) { - db := setupTestDB(t) - defer func() { _ = db.Close() }() - defer cleanupVotes(t, db) + t.Parallel() + db := testkit.DB(t) repo := NewVoteRepository(db) ctx := context.Background() diff --git a/loop_state.md b/loop_state.md index 9fb5486..06b0d0c 100644 --- a/loop_state.md +++ b/loop_state.md @@ -45,7 +45,7 @@ Stop the loop when every task is done, or on any blocked task. | 6 | Split multi-tier files by test func (manifest in commit msg); add build tags in place; retarget Makefile to tags; delete -short/testing.Short(); delete test-all | 2 ⛩ | S | done | (see git log) | PHASE 2 COMPLETE. 76 files `integration`, 3 `e2e`; 2 jetstream files split; 161 Short guards deleted (162nd was a doc comment); test-all + 4 dead targets gone. Honesty test: untagged suite green under --network none FIRST TRY (36 pkgs). make test = 11s no-Docker. Review (Codex needs-work / Opus safe-as-is): 8 fixes — GATE INTEGRITY closed (exit codes captured + mismatch rule; OOM-137-with-green-report now fails — was a silent pass since the harness was born; proved via truth table), -parallel 1 pinned on e2e (serial T2), readiness probe now hits the HOST endpoint tests dial, shared-DB migrate restored via testkit.MigrateSharedDatabase (advisory-locked, in testdbprepare), DSN redacted via url.Redacted, pure testkit files untagged (TestMain split into tagged harness_test.go + untagged harness_support_test.go), T0 socket-free (failingTransport). make ci GREEN 3399/0 skips 2m4s; audit 573 | | 7 | Migrate setupTestDB call sites → testkit.DB(t), batch 1 (~25 files) + delete their DELETE FROMs/cleanups | 3 | M | done | (see git log) | 29 files (aggregator_e2e..concurrent_scenarios incl. 4 hand-rolled setup clones), 115 sites → testkit.DB(t) (2 needed NO db at all — migration doubles as unused-DB detector), 19 DELETE FROMs + 1 cleanup fn deleted, diff +142/−828. Isolation PROVEN: concurrent -count=2 on a hardcoded-PK pair green; 0 leaked clones. make ci GREEN 3399/0 @142s (+18s vs baseline: ~150ms/test = FORCE-drop + 2 lock RTs — task 9 pays it back). Audit 573→561. NO order-dependency failures surfaced | | 8 | Migrate remaining call sites; delete all 3 setupTestDB defs + per-file cleanup fns | 3 | M | done | (see git log) | DB MIGRATION COMPLETE: 128 sites (31 files incl. live+e2e), all 4 defs + 4 cleanup fns + 18 goose pairs + 63 wipes deleted, +189/−1182. grep setupTestDB|goose in tests/ = EMPTY. e2e shared-DB hazard was HYPOTHETICAL (user_signup setupTestDB had ZERO callers; error_recovery all in-process) — SharedDB not needed. TestMain → testkit.Main(RequirePostgres, RequirePDS, RequireJetstream): make test-integration now FAILS without dev stack instead of skip-green (spec-honest, kept). FULL -shuffle=on INTEGRATION RUN GREEN — wipes were dead weight. make ci GREEN 3399/0 @2:39 (+17s ≈ 133ms/clone, consistent) | -| 9 | Global-state audit (t.Setenv/os.Setenv/logger/http-default → testkit injection); enable t.Parallel on proven-safe; connection budgets; `-race` clean; drop -p 1 | 3 ⛩ | S | pending | | wall-clock vs task-1 baseline recorded here | +| 9 | Global-state audit (t.Setenv/os.Setenv/logger/http-default → testkit injection); enable t.Parallel on proven-safe; connection budgets; `-race` clean; drop -p 1 | 3 ⛩ | S | done | (see git log) | PHASE 3 COMPLETE. 343 t.Parallel; audit: 0 convert / 4 sites deliberately-serial / rest safe. 9 internal straggler files migrated (goose now EXTINCT in test code; MigrateSharedDatabase deleted). THREE concurrency bugs -p 1 was masking: [A] template-destruction race (fixed: usePrivateTemplate) [B] legacy firehose 5s-behind-30s-promise, quantified (patched: jetstreamReadBudget, counter machinery deleted, non-timeout errors terminate) [C] Jetstream account/identity events BYPASS wantedCollections → parallel signup storms starve subscribers (measured 2/4 fail at -p 2; -p STAYS 1 with new documented reason). ConcurrencyBudget models both dims + nestedClonePools; -p 1 -parallel 26. Review: Codex good + Opus 3-high (binary-abort class, all fixed incl. fail-open Makefile splice PROVEN closed). make ci GREEN ×2 117/128s (clone tax repaid, beats 124s pre-clone); -race + -shuffle clean; peak 27/200 conns; 3401 tests/0 skips; audit 532 | | 10 | Contract-manifest CI check (WantedCollections ↔ //coves:ingestion-contract markers) + T2 skeleton (serial runner via compose runner; make test-e2e; test-e2e-dev escape hatch) | 4 ⛩ | S | pending | | build BEFORE first contract so every contract lands against it | | 11 | Contracts: community (community.profile ingestion + API) — strangler: behavior inventory of community_e2e_test.go (1820 LOC) → down-tier T1s → contract → delete old | 4 | S | pending | | template for tasks 12-16; sync-indexing trap per spec §3.4 | | 12 | Contracts: post (community.post) + post_delete + decompose post god-files | 4 | S | pending | | | @@ -198,3 +198,26 @@ Stop the loop when every task is done, or on any blocked task. Cumulative clone cost at 3399 tests: ~35s over baseline (~133ms/clone) — task 9's parallelism must beat that. No AppView-written row is asserted from Go anywhere (clean T2 boundary for tasks 10-16). +- **From task 9 (THE SHARED-RESOURCE LADDER — governs phase 4+)**: DB + (tasks 7-8) → template (fixed: private templates, sweepable + tktmpl_test_ family) → JETSTREAM STREAM (unfixed: account/identity + events bypass wantedCollections; signup storms visible to every + subscriber). -p stays 1 for the STREAM, not the DB — flip + packageParallelism (tests/testkit/db.go) only after phase 4 deletes + tests/integration's 9 serial firehose files, then fix the GOMAXPROCS + caveat (read in the PREPARE process, not the test process — documented + at the read site). Budget rule: model the WORST case one test can + create (nestedClonePools term); a test holding 3 clones would silently + re-break the ceiling — cheap contract check = grep testkit.DB( counts + per test func. Effective-vs-advertised timeouts are the tree smell: + phase-4 contracts use ONE named budget constant. Serial firehose block + = 25.8s of the 48.9s tier — phase 4's wall-clock prize. Dev Jetstream + accumulates (29MB) and replays thousands of pre-cursor events — + docker restart coves-dev-jetstream when firehose tests slow; testkit's + discard counter is the diagnostic. Local postgres-test CAN take + max_connections=200 from a worktree: docker-compose -p coves (project + name, not checkout, was task-3's trap). TestOAuthSessionHandleSync_ + LiveJetstream is assertion-free (3 t.Logs) — phase 4 rebuilds or + deletes. Reviewer calibration: Opus caught all 3 binary-abort highs + this round (incl. fail-open Makefile splice); Codex strongest on + budget arithmetic — both earning their seats. diff --git a/scripts/ci-runner.sh b/scripts/ci-runner.sh index 4e24589..56f310c 100755 --- a/scripts/ci-runner.sh +++ b/scripts/ci-runner.sh @@ -105,13 +105,13 @@ bash /src/scripts/test-audit.sh || true echo "▶ Preparing the test template database..." go run ./tests/testkit/cmd/testdbprepare -# The connection budget, derived from the server's max_connections rather than -# guessed: every test running under t.Parallel() holds its own clone pool. -# Nothing uses t.Parallel() outside tests/testkit yet, so this is inert today — -# it is wired now so that the phase enabling parallelism does not also have to -# discover the ceiling by exhausting it. -TEST_PARALLEL=$(bash /src/scripts/test-db-prepare.sh --print-parallel) -echo " ✓ connection budget allows -parallel $TEST_PARALLEL" +# The concurrency budget, derived from the server's max_connections rather than +# guessed: every test running under t.Parallel() holds its own clone pool, and +# -p multiplies that by the number of test binaries running at once. Both flags +# come from testkit.ConcurrencyBudget, which also documents why the package +# dimension is currently 1 (the shared Jetstream, not the old shared database). +TEST_FLAGS=$(bash /src/scripts/test-db-prepare.sh --print-flags) +echo " ✓ connection budget allows $TEST_FLAGS" echo # --------------------------------------------------------------------------- @@ -130,15 +130,11 @@ echo # runs second so the pipeline contracts are graded last, against a stack the # earlier tier has already exercised. # -# -p 1 serialises packages. The legacy tests/integration setup issues unscoped -# DELETEs against shared tables in the test database, so packages running -# concurrently delete each other's fixtures. -# # -count=1 defeats the test result cache. The toolchain hashes inputs it knows # about, and it does not know about PostgreSQL, the PDS, or the firehose — so a # cached PASS can survive an infrastructure change that would have failed. A # gate must actually execute. -echo "▶ Running the full suite (-p 1 -count=1, timeout $TEST_TIMEOUT)..." +echo "▶ Running the full suite ($TEST_FLAGS -count=1, timeout $TEST_TIMEOUT)..." echo # The capture is deliberately NOT a pipeline. @@ -179,12 +175,15 @@ trap 'kill "$progress_pid" 2>/dev/null || true' EXIT # reads as green. The statuses are the out-of-band evidence that the run # actually finished, and they are cross-checked against the report below. # -# T2 is pinned to -parallel 1 rather than the computed budget: the pipeline +# T2 is pinned to -p 1 -parallel 1 rather than the computed budget: the pipeline # contracts share one AppView, one PDS and one firehose cursor space, so they # are serial by design (docs/TEST_ARCHITECTURE.md §3.4). The budget applies to # the integration tier, where per-test database clones are the constraint. +# +# TEST_FLAGS is deliberately unquoted: it carries two flags and two values. set +e -go test -json -tags integration -p 1 -parallel "$TEST_PARALLEL" -count=1 -timeout "$TEST_TIMEOUT" \ +# shellcheck disable=SC2086 +go test -json -tags integration $TEST_FLAGS -count=1 -timeout "$TEST_TIMEOUT" \ ./cmd/... ./internal/... ./tests/... \ >>"$RAW" 2>&1 integration_status=$? diff --git a/scripts/test-db-prepare.sh b/scripts/test-db-prepare.sh index f85bed1..a1e1250 100755 --- a/scripts/test-db-prepare.sh +++ b/scripts/test-db-prepare.sh @@ -34,16 +34,17 @@ # --force rebuild even if the stamp matches # --sweep-age 30m change the orphan age cutoff (0 disables sweeping) # --wait 90s how long to wait for Postgres to accept connections -# --print-parallel print ONLY the safe `go test -parallel` value and exit, -# for $(...) capture by the Makefile and the CI runner +# --print-flags print ONLY the safe `go test` concurrency flags and exit +# ("-p N -parallel M"), for $(...) splicing by the Makefile +# and the CI runner set -euo pipefail REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) cd "$REPO_ROOT" -# --print-parallel is consumed by a shell substitution, so it must emit the -# number and nothing else. -if [[ ${1:-} != "--print-parallel" ]]; then +# --print-flags is consumed by a shell substitution, so it must emit the flags +# and nothing else. +if [[ ${1:-} != "--print-flags" ]]; then echo "▶ Preparing the test template database" fi diff --git a/tests/integration/aggregator_e2e_test.go b/tests/integration/aggregator_e2e_test.go index af8d52f..5adee3d 100644 --- a/tests/integration/aggregator_e2e_test.go +++ b/tests/integration/aggregator_e2e_test.go @@ -41,6 +41,7 @@ import ( // // NOTE: Requires PDS running at http://localhost:3001 func TestAggregator_E2E_WithJetstream(t *testing.T) { + t.Parallel() // Check if PDS is available pdsURL := "http://localhost:3001" resp, err := http.Get(pdsURL + "/xrpc/_health") diff --git a/tests/integration/aggregator_registration_test.go b/tests/integration/aggregator_registration_test.go index f05dd36..791764a 100644 --- a/tests/integration/aggregator_registration_test.go +++ b/tests/integration/aggregator_registration_test.go @@ -66,6 +66,7 @@ func (m *mockAggregatorIdentityResolver) Purge(ctx context.Context, identifier s } func TestAggregatorRegistration_Success(t *testing.T) { + t.Parallel() // Setup test database db := testkit.DB(t) @@ -153,6 +154,7 @@ func TestAggregatorRegistration_Success(t *testing.T) { } func TestAggregatorRegistration_DomainVerificationFailed(t *testing.T) { + t.Parallel() // Setup test database db := testkit.DB(t) @@ -216,6 +218,7 @@ func TestAggregatorRegistration_DomainVerificationFailed(t *testing.T) { } func TestAggregatorRegistration_InvalidDID(t *testing.T) { + t.Parallel() db := testkit.DB(t) tests := []struct { @@ -273,6 +276,7 @@ func TestAggregatorRegistration_InvalidDID(t *testing.T) { } func TestAggregatorRegistration_AlreadyRegistered(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Pre-create user with same DID @@ -352,6 +356,7 @@ func TestAggregatorRegistration_AlreadyRegistered(t *testing.T) { } func TestAggregatorRegistration_WellKnownNotAccessible(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup test server that returns 404 for .well-known @@ -409,6 +414,7 @@ func TestAggregatorRegistration_WellKnownNotAccessible(t *testing.T) { } func TestAggregatorRegistration_WellKnownTooLarge(t *testing.T) { + t.Parallel() db := testkit.DB(t) testDID := "did:plc:toolarge" @@ -469,6 +475,7 @@ func TestAggregatorRegistration_WellKnownTooLarge(t *testing.T) { } func TestAggregatorRegistration_DIDResolutionFailed(t *testing.T) { + t.Parallel() db := testkit.DB(t) testDID := "did:plc:nonexistent" @@ -540,6 +547,7 @@ func TestAggregatorRegistration_DIDResolutionFailed(t *testing.T) { } func TestAggregatorRegistration_LargeWellKnownResponse(t *testing.T) { + t.Parallel() db := testkit.DB(t) testDID := "did:plc:largedos123" @@ -602,11 +610,11 @@ func TestAggregatorRegistration_LargeWellKnownResponse(t *testing.T) { // Call handler - should fail gracefully, not hang or DoS handler.HandleRegister(rr, req) - elapsed := time.Since(startTime) - - // Assert the handler completed quickly (not trying to read 10MB) - // Should complete in well under 1 second. Using 5 seconds as generous upper bound. - assert.Less(t, elapsed, 5*time.Second, "Handler should complete quickly even with large response") + // No wall-clock assertion: under -parallel this measures contention with + // dozens of peers rather than whether the handler read 10MB. The real + // evidence that it did not is the status code below, which it could only + // reach by rejecting the response rather than consuming it. + t.Logf("handler returned in %v", time.Since(startTime)) // Should fail with domain verification error (DID mismatch: got "AAAA..." instead of expected DID) assert.Equal(t, http.StatusUnauthorized, rr.Code, "Should reject due to DID mismatch") @@ -621,10 +629,11 @@ func TestAggregatorRegistration_LargeWellKnownResponse(t *testing.T) { // Verify user was NOT created assertUserDoesNotExist(t, db, testDID) - t.Logf("✓ DoS protection test completed in %v (prevented reading 10MB payload)", elapsed) + t.Logf("✓ DoS protection test completed (prevented reading 10MB payload)") } func TestAggregatorRegistration_E2E_WithRealInfrastructure(t *testing.T) { + t.Parallel() // This test requires docker-compose infrastructure to be running: // docker-compose -f docker-compose.dev.yml --profile test up postgres-test // diff --git a/tests/integration/aggregator_test.go b/tests/integration/aggregator_test.go index 9d2f6d2..e985c8b 100644 --- a/tests/integration/aggregator_test.go +++ b/tests/integration/aggregator_test.go @@ -16,6 +16,7 @@ import ( // TestAggregatorRepository_Create tests basic aggregator creation func TestAggregatorRepository_Create(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewAggregatorRepository(db) @@ -117,6 +118,7 @@ func TestAggregatorRepository_Create(t *testing.T) { // TestAggregatorRepository_IsAggregator tests the fast existence check func TestAggregatorRepository_IsAggregator(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewAggregatorRepository(db) @@ -163,6 +165,7 @@ func TestAggregatorRepository_IsAggregator(t *testing.T) { // TestAggregatorAuthorization_Create tests authorization creation func TestAggregatorAuthorization_Create(t *testing.T) { + t.Parallel() db := testkit.DB(t) aggRepo := postgres.NewAggregatorRepository(db) @@ -317,6 +320,7 @@ func TestAggregatorAuthorization_Create(t *testing.T) { // TestAggregatorAuthorization_IsAuthorized tests fast authorization check func TestAggregatorAuthorization_IsAuthorized(t *testing.T) { + t.Parallel() db := testkit.DB(t) aggRepo := postgres.NewAggregatorRepository(db) @@ -450,6 +454,7 @@ func TestAggregatorAuthorization_IsAuthorized(t *testing.T) { // TestAggregatorService_PostCreationIntegration tests the full post creation flow with aggregators func TestAggregatorService_PostCreationIntegration(t *testing.T) { + t.Parallel() db := testkit.DB(t) aggRepo := postgres.NewAggregatorRepository(db) @@ -544,6 +549,7 @@ func TestAggregatorService_PostCreationIntegration(t *testing.T) { // TestAggregatorService_RateLimiting tests rate limit enforcement func TestAggregatorService_RateLimiting(t *testing.T) { + t.Parallel() db := testkit.DB(t) aggRepo := postgres.NewAggregatorRepository(db) @@ -630,6 +636,7 @@ func TestAggregatorService_RateLimiting(t *testing.T) { // TestAggregatorPostService_Integration tests the posts service integration func TestAggregatorPostService_Integration(t *testing.T) { + t.Parallel() db := testkit.DB(t) aggRepo := postgres.NewAggregatorRepository(db) @@ -676,6 +683,7 @@ func TestAggregatorPostService_Integration(t *testing.T) { // TestAggregatorTriggers tests database triggers for auto-updating stats func TestAggregatorTriggers(t *testing.T) { + t.Parallel() db := testkit.DB(t) aggRepo := postgres.NewAggregatorRepository(db) @@ -785,6 +793,7 @@ func TestAggregatorTriggers(t *testing.T) { // TestAggregatorAuthorization_DisabledAtField tests that disabledAt is properly stored and retrieved func TestAggregatorAuthorization_DisabledAtField(t *testing.T) { + t.Parallel() db := testkit.DB(t) aggRepo := postgres.NewAggregatorRepository(db) diff --git a/tests/integration/author_avatar_hydration_test.go b/tests/integration/author_avatar_hydration_test.go index b5b9f33..f6c970e 100644 --- a/tests/integration/author_avatar_hydration_test.go +++ b/tests/integration/author_avatar_hydration_test.go @@ -25,6 +25,7 @@ import ( // Regression test for the bug where feeds and post views only hydrated the // community avatar and author cards were always bare even for fully indexed users. func TestAuthorProfileHydration(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/author_posts_e2e_test.go b/tests/integration/author_posts_e2e_test.go index e885ab0..100c0b3 100644 --- a/tests/integration/author_posts_e2e_test.go +++ b/tests/integration/author_posts_e2e_test.go @@ -22,6 +22,8 @@ import ( "time" "github.com/go-chi/chi/v5" + + "github.com/stretchr/testify/require" ) // getPostTitleFromView extracts title from PostView.Record. @@ -45,14 +47,13 @@ func getPostTitleFromView(t *testing.T, pv *posts.PostView) string { // TestGetAuthorPosts_E2E_Success tests the full author posts flow with real PDS // Flow: Create user on PDS → Create posts → Query via XRPC → Verify response func TestGetAuthorPosts_E2E_Success(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Check if PDS is running pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v", pdsURL, err) - } + require.NoError(t, err, "PDS health check at %s (TestMain's RequirePDS should have caught this)", pdsURL) _ = healthResp.Body.Close() ctx := context.Background() @@ -268,6 +269,7 @@ func TestGetAuthorPosts_E2E_Success(t *testing.T) { // TestGetAuthorPosts_FilterLogic tests the different filter options func TestGetAuthorPosts_FilterLogic(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -402,6 +404,7 @@ func TestGetAuthorPosts_FilterLogic(t *testing.T) { // TestGetAuthorPosts_ServiceErrors tests error handling in the service layer func TestGetAuthorPosts_ServiceErrors(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -515,6 +518,7 @@ func TestGetAuthorPosts_ServiceErrors(t *testing.T) { // TestGetAuthorPosts_WithJetstreamIndexing tests the full flow including Jetstream indexing func TestGetAuthorPosts_WithJetstreamIndexing(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -540,9 +544,7 @@ func TestGetAuthorPosts_WithJetstreamIndexing(t *testing.T) { testUserPassword := "test-password-123" _, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Index user in AppView _ = createTestUser(t, db, testUserHandle, userDID) @@ -623,6 +625,7 @@ func TestGetAuthorPosts_WithJetstreamIndexing(t *testing.T) { // TestGetAuthorPosts_CommunityFilter tests filtering posts by community func TestGetAuthorPosts_CommunityFilter(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/blob_upload_e2e_test.go b/tests/integration/blob_upload_e2e_test.go index a618e4f..a824712 100644 --- a/tests/integration/blob_upload_e2e_test.go +++ b/tests/integration/blob_upload_e2e_test.go @@ -41,6 +41,7 @@ import ( // - Blob references in atProto records // - URL transformation in AppView responses func TestBlobUpload_E2E_PostWithImages(t *testing.T) { + t.Parallel() // Check if PDS is available before running E2E test pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -351,6 +352,7 @@ func TestBlobUpload_E2E_PostWithImages(t *testing.T) { // TestBlobUpload_E2E_CommentWithImage tests image upload in comments func TestBlobUpload_E2E_CommentWithImage(t *testing.T) { + t.Parallel() // Check if PDS is available before running E2E test pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -456,6 +458,7 @@ func TestBlobUpload_E2E_CommentWithImage(t *testing.T) { // TestBlobUpload_PDS_MockServer tests blob upload with a mock PDS server // This allows testing without a live PDS instance func TestBlobUpload_PDS_MockServer(t *testing.T) { + t.Parallel() // Create mock PDS server mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Verify request @@ -510,6 +513,7 @@ func TestBlobUpload_PDS_MockServer(t *testing.T) { // TestBlobUpload_Validation tests blob upload validation func TestBlobUpload_Validation(t *testing.T) { + t.Parallel() db := testkit.DB(t) communityRepo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/block_handle_resolution_test.go b/tests/integration/block_handle_resolution_test.go index 1c69cf8..9c84020 100644 --- a/tests/integration/block_handle_resolution_test.go +++ b/tests/integration/block_handle_resolution_test.go @@ -35,6 +35,7 @@ func createTestOAuthSessionForBlock(did string) *oauth.ClientSessionData { // TestBlockHandler_HandleResolution tests that the block handler accepts handles // in addition to DIDs and resolves them correctly func TestBlockHandler_HandleResolution(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -268,6 +269,7 @@ func TestBlockHandler_HandleResolution(t *testing.T) { // TestUnblockHandler_HandleResolution tests that the unblock handler accepts handles func TestUnblockHandler_HandleResolution(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/bluesky_post_test.go b/tests/integration/bluesky_post_test.go index 73afd7f..387da47 100644 --- a/tests/integration/bluesky_post_test.go +++ b/tests/integration/bluesky_post_test.go @@ -16,6 +16,7 @@ import ( // TestBlueskyPostCrossPosting_E2E_LivePDS tests writing posts with Bluesky URLs to a real PDS // This catches lexicon validation errors like invalid strongRef CIDs func TestBlueskyPostCrossPosting_E2E_LivePDS(t *testing.T) { + t.Parallel() // Check if PDS is running pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") diff --git a/tests/integration/comment_consumer_test.go b/tests/integration/comment_consumer_test.go index 867ddff..902f97f 100644 --- a/tests/integration/comment_consumer_test.go +++ b/tests/integration/comment_consumer_test.go @@ -14,6 +14,7 @@ import ( ) func TestCommentConsumer_CreateComment(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -167,6 +168,7 @@ func TestCommentConsumer_CreateComment(t *testing.T) { } func TestCommentConsumer_Threading(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -318,6 +320,7 @@ func TestCommentConsumer_Threading(t *testing.T) { } func TestCommentConsumer_UpdateComment(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -439,6 +442,7 @@ func TestCommentConsumer_UpdateComment(t *testing.T) { } func TestCommentConsumer_DeleteComment(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -612,6 +616,7 @@ func TestCommentConsumer_DeleteComment(t *testing.T) { } func TestCommentConsumer_SecurityValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -860,6 +865,7 @@ func TestCommentConsumer_SecurityValidation(t *testing.T) { } func TestCommentRepository_Queries(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -1018,6 +1024,7 @@ func TestCommentRepository_Queries(t *testing.T) { // TestCommentConsumer_OutOfOrderReconciliation tests that parent counts are // correctly reconciled when child comments arrive before their parent func TestCommentConsumer_OutOfOrderReconciliation(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -1271,6 +1278,7 @@ func TestCommentConsumer_OutOfOrderReconciliation(t *testing.T) { // TestCommentConsumer_Resurrection tests that soft-deleted comments can be recreated // In atProto, deleted records' rkeys become available for reuse func TestCommentConsumer_Resurrection(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -1571,6 +1579,7 @@ func TestCommentConsumer_Resurrection(t *testing.T) { // TestCommentConsumer_ThreadingImmutability tests that UPDATE events cannot change threading refs func TestCommentConsumer_ThreadingImmutability(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/comment_e2e_test.go b/tests/integration/comment_e2e_test.go index 72dbb8f..40ca214 100644 --- a/tests/integration/comment_e2e_test.go +++ b/tests/integration/comment_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/atproto/jetstream" "Coves/internal/atproto/pds" @@ -630,10 +641,10 @@ func subscribeToJetstreamForComment( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) for { select { @@ -642,7 +653,7 @@ func subscribeToJetstreamForComment( case <-ctx.Done(): return ctx.Err() default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -650,21 +661,17 @@ func subscribeToJetstreamForComment( err := conn.ReadJSON(&event) if err != nil { if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - // Check if this is a comment create event for the target DID if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.comment" && @@ -700,10 +707,10 @@ func subscribeToJetstreamForCommentUpdate( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) for { select { @@ -712,7 +719,7 @@ func subscribeToJetstreamForCommentUpdate( case <-ctx.Done(): return ctx.Err() default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -720,21 +727,17 @@ func subscribeToJetstreamForCommentUpdate( err := conn.ReadJSON(&event) if err != nil { if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.comment" && event.Commit.Operation == "update" { @@ -769,10 +772,10 @@ func subscribeToJetstreamForCommentDelete( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) for { select { @@ -781,7 +784,7 @@ func subscribeToJetstreamForCommentDelete( case <-ctx.Done(): return ctx.Err() default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -789,21 +792,17 @@ func subscribeToJetstreamForCommentDelete( err := conn.ReadJSON(&event) if err != nil { if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.comment" && event.Commit.Operation == "delete" { diff --git a/tests/integration/comment_query_test.go b/tests/integration/comment_query_test.go index 9182aad..a2bcb0d 100644 --- a/tests/integration/comment_query_test.go +++ b/tests/integration/comment_query_test.go @@ -24,6 +24,7 @@ import ( // TestCommentQuery_BasicFetch tests fetching top-level comments with default params func TestCommentQuery_BasicFetch(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -79,6 +80,7 @@ func TestCommentQuery_BasicFetch(t *testing.T) { // TestCommentQuery_NestedReplies tests fetching comments with nested reply structure func TestCommentQuery_NestedReplies(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -164,6 +166,7 @@ func TestCommentQuery_NestedReplies(t *testing.T) { // TestCommentQuery_DepthLimit tests depth limiting works correctly func TestCommentQuery_DepthLimit(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -243,6 +246,7 @@ func TestCommentQuery_DepthLimit(t *testing.T) { // TestCommentQuery_HotSorting tests hot sorting with Lemmy algorithm func TestCommentQuery_HotSorting(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -289,6 +293,7 @@ func TestCommentQuery_HotSorting(t *testing.T) { // TestCommentQuery_TopSorting tests top sorting with score-based ordering func TestCommentQuery_TopSorting(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -347,6 +352,7 @@ func TestCommentQuery_TopSorting(t *testing.T) { // TestCommentQuery_NewSorting tests chronological sorting func TestCommentQuery_NewSorting(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -381,6 +387,7 @@ func TestCommentQuery_NewSorting(t *testing.T) { // TestCommentQuery_Pagination tests cursor-based pagination func TestCommentQuery_Pagination(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -450,6 +457,7 @@ func TestCommentQuery_Pagination(t *testing.T) { // TestCommentQuery_EmptyThread tests fetching comments from a post with no comments func TestCommentQuery_EmptyThread(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -481,6 +489,7 @@ func TestCommentQuery_EmptyThread(t *testing.T) { // TestCommentQuery_DeletedComments tests that soft-deleted comments are excluded func TestCommentQuery_DeletedComments(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -601,6 +610,7 @@ func TestCommentQuery_DeletedComments(t *testing.T) { // TestCommentQuery_InvalidInputs tests error handling for invalid inputs func TestCommentQuery_InvalidInputs(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -696,6 +706,7 @@ func TestCommentQuery_InvalidInputs(t *testing.T) { // TestCommentQuery_HTTPHandler tests the HTTP handler end-to-end func TestCommentQuery_HTTPHandler(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -750,6 +761,7 @@ func TestCommentQuery_HTTPHandler(t *testing.T) { // TestCommentQuery_ParentRkeySubtree tests fetching a comment subtree via parentRkey // Backs the comment-permalink page and "continue this thread" for deep threads func TestCommentQuery_ParentRkeySubtree(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -1145,6 +1157,7 @@ func TestCommentQuery_ParentRkeySubtree(t *testing.T) { // TestCommentQuery_ParentRkeyHTTPHandler tests the real XRPC handler with parentRkey func TestCommentQuery_ParentRkeyHTTPHandler(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/comment_vote_test.go b/tests/integration/comment_vote_test.go index 2054ffd..44cc442 100644 --- a/tests/integration/comment_vote_test.go +++ b/tests/integration/comment_vote_test.go @@ -16,6 +16,7 @@ import ( // TestCommentVote_CreateAndUpdate tests voting on comments and vote count updates func TestCommentVote_CreateAndUpdate(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -321,6 +322,7 @@ func TestCommentVote_CreateAndUpdate(t *testing.T) { // TestCommentVote_ViewerState tests viewer vote state in comment query responses func TestCommentVote_ViewerState(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/comment_write_test.go b/tests/integration/comment_write_test.go index 333c1fc..2c114e8 100644 --- a/tests/integration/comment_write_test.go +++ b/tests/integration/comment_write_test.go @@ -21,19 +21,20 @@ import ( oauthlib "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/bluesky-social/indigo/atproto/syntax" + + "github.com/stretchr/testify/require" ) // TestCommentWrite_CreateTopLevelComment tests creating a comment on a post via E2E flow func TestCommentWrite_CreateTopLevelComment(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Check if PDS is running pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v", pdsURL, err) - } + require.NoError(t, err, "PDS health check at %s (TestMain's RequirePDS should have caught this)", pdsURL) func() { if closeErr := healthResp.Body.Close(); closeErr != nil { t.Logf("Failed to close health response: %v", closeErr) @@ -254,6 +255,7 @@ func TestCommentWrite_CreateTopLevelComment(t *testing.T) { // TestCommentWrite_CreateNestedReply tests creating a reply to another comment func TestCommentWrite_CreateNestedReply(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -291,9 +293,7 @@ func TestCommentWrite_CreateNestedReply(t *testing.T) { testUserPassword := "test-password-123" pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") testUser := createTestUser(t, db, testUserHandle, userDID) @@ -400,6 +400,7 @@ func TestCommentWrite_CreateNestedReply(t *testing.T) { // TestCommentWrite_UpdateComment tests updating an existing comment func TestCommentWrite_UpdateComment(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -436,9 +437,7 @@ func TestCommentWrite_UpdateComment(t *testing.T) { testUserPassword := "test-password-123" pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Setup OAuth mockStore := NewMockOAuthStore() @@ -516,6 +515,7 @@ func TestCommentWrite_UpdateComment(t *testing.T) { // TestCommentWrite_DeleteComment tests deleting a comment func TestCommentWrite_DeleteComment(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -552,9 +552,7 @@ func TestCommentWrite_DeleteComment(t *testing.T) { testUserPassword := "test-password-123" pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Setup OAuth mockStore := NewMockOAuthStore() @@ -620,6 +618,7 @@ func TestCommentWrite_DeleteComment(t *testing.T) { // TestCommentWrite_CannotUpdateOthersComment tests authorization for updates func TestCommentWrite_CannotUpdateOthersComment(t *testing.T) { + t.Parallel() ctx := context.Background() pdsURL := getTestPDSURL() @@ -650,18 +649,14 @@ func TestCommentWrite_CannotUpdateOthersComment(t *testing.T) { ownerHandle := fmt.Sprintf("own%s.local.coves.dev", ownerID) ownerEmail := fmt.Sprintf("owner-%s@test.local", ownerID) _, ownerDID, err := createPDSAccount(pdsURL, ownerHandle, ownerEmail, "password123") - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Create second user (attacker) attackerID := uniqueTestID() attackerHandle := fmt.Sprintf("atk%s.local.coves.dev", attackerID) attackerEmail := fmt.Sprintf("attacker-%s@test.local", attackerID) attackerToken, attackerDID, err := createPDSAccount(pdsURL, attackerHandle, attackerEmail, "password123") - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Setup OAuth for attacker mockStore := NewMockOAuthStore() @@ -693,6 +688,7 @@ func TestCommentWrite_CannotUpdateOthersComment(t *testing.T) { // TestCommentWrite_CannotDeleteOthersComment tests authorization for deletes func TestCommentWrite_CannotDeleteOthersComment(t *testing.T) { + t.Parallel() ctx := context.Background() pdsURL := getTestPDSURL() @@ -723,18 +719,14 @@ func TestCommentWrite_CannotDeleteOthersComment(t *testing.T) { ownerHandle := fmt.Sprintf("own%s.local.coves.dev", ownerID) ownerEmail := fmt.Sprintf("owner-%s@test.local", ownerID) _, ownerDID, err := createPDSAccount(pdsURL, ownerHandle, ownerEmail, "password123") - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Create second user (attacker) attackerID := uniqueTestID() attackerHandle := fmt.Sprintf("atk%s.local.coves.dev", attackerID) attackerEmail := fmt.Sprintf("attacker-%s@test.local", attackerID) attackerToken, attackerDID, err := createPDSAccount(pdsURL, attackerHandle, attackerEmail, "password123") - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Setup OAuth for attacker mockStore := NewMockOAuthStore() @@ -772,6 +764,7 @@ func parseTestDID(did string) (syntax.DID, error) { // CID validation correctly detects concurrent modifications. // This verifies the optimistic locking mechanism that prevents lost updates. func TestCommentWrite_ConcurrentModificationDetection(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -806,9 +799,7 @@ func TestCommentWrite_ConcurrentModificationDetection(t *testing.T) { testUserPassword := "test-password-123" pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") // Setup OAuth mockStore := NewMockOAuthStore() diff --git a/tests/integration/community_avatar_e2e_test.go b/tests/integration/community_avatar_e2e_test.go index dbf356f..976445d 100644 --- a/tests/integration/community_avatar_e2e_test.go +++ b/tests/integration/community_avatar_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/atproto/identity" "Coves/internal/atproto/jetstream" @@ -11,12 +22,10 @@ import ( "Coves/tests/testkit" "bytes" "context" - "errors" "fmt" "image" "image/color" "image/png" - "net" "net/http" "os" "strings" @@ -126,7 +135,11 @@ func TestCommunityAvatarE2E_CreateWithAvatar(t *testing.T) { } defer func() { _ = conn.Close() }() - consecutiveTimeouts := 0 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + for { select { case <-done: @@ -134,22 +147,18 @@ func TestCommunityAvatarE2E_CreateWithAvatar(t *testing.T) { case <-subscribeCtx.Done(): return default: - if deadlineErr := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); deadlineErr != nil { + if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { return } var event jetstream.JetstreamEvent if readErr := conn.ReadJSON(&event); readErr != nil { - var netErr net.Error - if errors.As(readErr, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= 10 { - return // Connection stale, exit to prevent panic - } - } - continue + // Any read error ends this subscription. A gorilla connection is + // corrupt once its read deadline has expired, and looping on it + // is what reaches the panic that aborts the whole test binary. + // The caller's own timeout reports the missing event. + return } - consecutiveTimeouts = 0 // Only process community profile create events if event.Kind == "commit" && event.Commit != nil && @@ -187,7 +196,7 @@ func TestCommunityAvatarE2E_CreateWithAvatar(t *testing.T) { // Wait for REAL Jetstream event t.Logf("\n⏳ Waiting for create event from Jetstream...") var realEvent *jetstream.JetstreamEvent - timeout := time.After(15 * time.Second) + timeout := time.After(jetstreamReadBudget) eventLoop: for { @@ -345,7 +354,11 @@ func TestCommunityAvatarE2E_UpdateWithAvatar(t *testing.T) { } defer func() { _ = conn.Close() }() - consecutiveTimeouts := 0 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + for { select { case <-done: @@ -353,22 +366,18 @@ func TestCommunityAvatarE2E_UpdateWithAvatar(t *testing.T) { case <-subscribeCtx.Done(): return default: - if deadlineErr := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); deadlineErr != nil { + if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { return } var event jetstream.JetstreamEvent if readErr := conn.ReadJSON(&event); readErr != nil { - var netErr net.Error - if errors.As(readErr, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= 10 { - return // Connection stale, exit to prevent panic - } - } - continue + // Any read error ends this subscription. A gorilla connection is + // corrupt once its read deadline has expired, and looping on it + // is what reaches the panic that aborts the whole test binary. + // The caller's own timeout reports the missing event. + return } - consecutiveTimeouts = 0 if event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.profile" && @@ -427,7 +436,7 @@ func TestCommunityAvatarE2E_UpdateWithAvatar(t *testing.T) { // Start listening for Jetstream event eventReceived := make(chan *jetstream.JetstreamEvent, 1) go func() { - event := waitForUpdateEvent(t, community.DID, 15*time.Second) + event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) eventReceived <- event }() time.Sleep(500 * time.Millisecond) // Give subscriber time to connect @@ -545,7 +554,7 @@ func TestCommunityAvatarE2E_UpdateWithAvatar(t *testing.T) { // Start listening for Jetstream event eventReceived := make(chan *jetstream.JetstreamEvent, 1) go func() { - event := waitForUpdateEvent(t, community.DID, 15*time.Second) + event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) eventReceived <- event }() time.Sleep(500 * time.Millisecond) @@ -701,7 +710,11 @@ func TestCommunityAvatarE2E_UpdateWithBanner(t *testing.T) { } defer func() { _ = conn.Close() }() - consecutiveTimeouts := 0 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + for { select { case <-done: @@ -709,22 +722,18 @@ func TestCommunityAvatarE2E_UpdateWithBanner(t *testing.T) { case <-subscribeCtx.Done(): return default: - if deadlineErr := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); deadlineErr != nil { + if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { return } var event jetstream.JetstreamEvent if readErr := conn.ReadJSON(&event); readErr != nil { - var netErr net.Error - if errors.As(readErr, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= 10 { - return // Connection stale, exit to prevent panic - } - } - continue + // Any read error ends this subscription. A gorilla connection is + // corrupt once its read deadline has expired, and looping on it + // is what reaches the panic that aborts the whole test binary. + // The caller's own timeout reports the missing event. + return } - consecutiveTimeouts = 0 if event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.profile" && @@ -783,7 +792,7 @@ func TestCommunityAvatarE2E_UpdateWithBanner(t *testing.T) { // Start listening for Jetstream event eventReceived := make(chan *jetstream.JetstreamEvent, 1) go func() { - event := waitForUpdateEvent(t, community.DID, 15*time.Second) + event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) eventReceived <- event }() time.Sleep(500 * time.Millisecond) // Give subscriber time to connect @@ -901,7 +910,7 @@ func TestCommunityAvatarE2E_UpdateWithBanner(t *testing.T) { // Start listening for Jetstream event eventReceived := make(chan *jetstream.JetstreamEvent, 1) go func() { - event := waitForUpdateEvent(t, community.DID, 15*time.Second) + event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) eventReceived <- event }() time.Sleep(500 * time.Millisecond) diff --git a/tests/integration/community_blocking_test.go b/tests/integration/community_blocking_test.go index a976fdf..3b87151 100644 --- a/tests/integration/community_blocking_test.go +++ b/tests/integration/community_blocking_test.go @@ -17,6 +17,7 @@ import ( // TestCommunityBlocking_Indexing tests Jetstream indexing of block events func TestCommunityBlocking_Indexing(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -204,6 +205,7 @@ func TestCommunityBlocking_Indexing(t *testing.T) { // TestCommunityBlocking_ListBlocked tests listing blocked communities func TestCommunityBlocking_ListBlocked(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -280,6 +282,7 @@ func TestCommunityBlocking_ListBlocked(t *testing.T) { // TestCommunityBlocking_IsBlocked tests the fast block check func TestCommunityBlocking_IsBlocked(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -343,6 +346,7 @@ func TestCommunityBlocking_IsBlocked(t *testing.T) { // TestCommunityBlocking_GetBlock tests block retrieval func TestCommunityBlocking_GetBlock(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) diff --git a/tests/integration/community_consumer_test.go b/tests/integration/community_consumer_test.go index c03e2ff..647f5b2 100644 --- a/tests/integration/community_consumer_test.go +++ b/tests/integration/community_consumer_test.go @@ -16,6 +16,7 @@ import ( ) func TestCommunityConsumer_HandleCommunityProfile(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -222,6 +223,7 @@ func TestCommunityConsumer_HandleCommunityProfile(t *testing.T) { } func TestCommunityConsumer_HandleSubscription(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -307,6 +309,7 @@ func TestCommunityConsumer_HandleSubscription(t *testing.T) { } func TestCommunityConsumer_IgnoresNonCommunityEvents(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -392,6 +395,7 @@ func (m *mockIdentityResolver) Resolve(ctx context.Context, did string) (*identi } func TestCommunityConsumer_PLCHandleResolution(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/community_credentials_test.go b/tests/integration/community_credentials_test.go index 7ab56bc..ba2f0db 100644 --- a/tests/integration/community_credentials_test.go +++ b/tests/integration/community_credentials_test.go @@ -14,6 +14,7 @@ import ( // TestCommunityRepository_CredentialPersistence tests that PDS credentials are properly persisted func TestCommunityRepository_CredentialPersistence(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -120,6 +121,7 @@ func TestCommunityRepository_CredentialPersistence(t *testing.T) { // TestCommunityRepository_EncryptedCredentials tests encryption at rest func TestCommunityRepository_EncryptedCredentials(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -232,6 +234,7 @@ func TestCommunityRepository_EncryptedCredentials(t *testing.T) { // TestCommunityRepository_V2OwnershipModel tests that communities are self-owned func TestCommunityRepository_V2OwnershipModel(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/community_e2e_test.go b/tests/integration/community_e2e_test.go index 09558b9..d62d295 100644 --- a/tests/integration/community_e2e_test.go +++ b/tests/integration/community_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/api/routes" "Coves/internal/atproto/identity" @@ -1721,10 +1732,10 @@ func subscribeToJetstream( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) // Read messages until we find our event or receive done signal for { @@ -1735,7 +1746,7 @@ func subscribeToJetstream( return ctx.Err() default: // Set read deadline to avoid blocking forever - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -1744,22 +1755,18 @@ func subscribeToJetstream( if err != nil { // Check if it's a timeout (expected) if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue // Timeout is expected, keep listening + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } // For other errors, don't retry reading from a broken connection return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - // Check if this is the event we're looking for if event.Did == targetDID && event.Kind == "commit" { // Process the event through the consumer diff --git a/tests/integration/community_get_viewer_state_test.go b/tests/integration/community_get_viewer_state_test.go index 9538cb7..ffe8f93 100644 --- a/tests/integration/community_get_viewer_state_test.go +++ b/tests/integration/community_get_viewer_state_test.go @@ -34,6 +34,7 @@ func (m *getViewerMockService) GetCommunity(ctx context.Context, identifier stri // social.coves.community.get lexicon promise ("viewer state will be // included if authenticated"). func TestCommunityGet_ViewerState(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/community_hostedby_security_test.go b/tests/integration/community_hostedby_security_test.go index ba4d63b..f4144c3 100644 --- a/tests/integration/community_hostedby_security_test.go +++ b/tests/integration/community_hostedby_security_test.go @@ -17,6 +17,7 @@ import ( // TestHostedByVerification_DomainMatching tests that hostedBy domain must match handle domain func TestHostedByVerification_DomainMatching(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -237,6 +238,7 @@ func TestHostedByVerification_DomainMatching(t *testing.T) { // TestBidirectionalDIDVerification tests the full bidirectional verification with mock HTTP server // This test verifies that the DID document must claim the handle in alsoKnownAs field func TestBidirectionalDIDVerification(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -385,6 +387,7 @@ func TestBidirectionalDIDVerification(t *testing.T) { // TestExtractDomainFromHandle tests the domain extraction logic for various handle formats func TestExtractDomainFromHandle(t *testing.T) { + t.Parallel() // This is an internal function test - we'll test it through the consumer db := testkit.DB(t) diff --git a/tests/integration/community_identifier_resolution_test.go b/tests/integration/community_identifier_resolution_test.go index 2b98734..2b1d950 100644 --- a/tests/integration/community_identifier_resolution_test.go +++ b/tests/integration/community_identifier_resolution_test.go @@ -18,6 +18,7 @@ import ( // TestCommunityIdentifierResolution tests all formats accepted by ResolveCommunityIdentifier func TestCommunityIdentifierResolution(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -209,6 +210,7 @@ func TestCommunityIdentifierResolution(t *testing.T) { // TestResolveScopedIdentifier_InputValidation tests input sanitization func TestResolveScopedIdentifier_InputValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -311,6 +313,7 @@ func TestResolveScopedIdentifier_InputValidation(t *testing.T) { // TestGetDisplayHandle tests the GetDisplayHandle method func TestGetDisplayHandle(t *testing.T) { + t.Parallel() tests := []struct { name string handle string @@ -379,6 +382,7 @@ func TestGetDisplayHandle(t *testing.T) { // TestIdentifierResolution_ErrorContext verifies error messages include identifier context func TestIdentifierResolution_ErrorContext(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -437,6 +441,7 @@ func TestIdentifierResolution_ErrorContext(t *testing.T) { // TestGetCommunity_IdentifierResolution tests all formats accepted by GetCommunity // This is distinct from ResolveCommunityIdentifier - GetCommunity returns the full Community object func TestGetCommunity_IdentifierResolution(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/community_list_viewer_state_test.go b/tests/integration/community_list_viewer_state_test.go index 27ac840..679f959 100644 --- a/tests/integration/community_list_viewer_state_test.go +++ b/tests/integration/community_list_viewer_state_test.go @@ -23,6 +23,7 @@ import ( // TestCommunityList_ViewerState tests that the list communities endpoint // correctly populates viewer.subscribed field for authenticated users func TestCommunityList_ViewerState(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/community_provisioning_test.go b/tests/integration/community_provisioning_test.go index dc817f7..c795792 100644 --- a/tests/integration/community_provisioning_test.go +++ b/tests/integration/community_provisioning_test.go @@ -16,6 +16,7 @@ import ( // TestCommunityRepository_PasswordEncryption verifies P0 fix: // Password must be encrypted (not hashed) so we can recover it for session renewal func TestCommunityRepository_PasswordEncryption(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -135,6 +136,7 @@ func TestCommunityRepository_PasswordEncryption(t *testing.T) { // TestCommunityService_NameValidation verifies P1 fix: // Community names must respect DNS label limits (63 chars max) func TestCommunityService_NameValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -298,6 +300,7 @@ func TestCommunityService_NameValidation(t *testing.T) { // TestPasswordSecurity verifies password generation security properties // Critical for P0: Passwords must be unpredictable and have sufficient entropy func TestPasswordSecurity(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -427,6 +430,7 @@ func TestPasswordSecurity(t *testing.T) { // TestConcurrentProvisioning verifies thread-safety during community creation // Critical: Prevents race conditions that could create duplicate communities func TestConcurrentProvisioning(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -564,9 +568,15 @@ func TestConcurrentProvisioning(t *testing.T) { // TestPDSNetworkFailures verifies graceful handling of PDS network issues // Critical: Ensures service doesn't crash or leak resources on PDS failures func TestPDSNetworkFailures(t *testing.T) { + t.Parallel() + // The subtests are parallel because each one waits out the same retry + // ladder against a dead endpoint (4 attempts, 1s+2s+4s of backoff). They + // share nothing — no database, no fixtures, a fresh provisioner each — + // so running them serially only adds their backoffs together. ctx := context.Background() t.Run("handles invalid PDS URL", func(t *testing.T) { + t.Parallel() // Invalid URL should fail gracefully invalidURLs := []string{ "not-a-url", @@ -594,6 +604,7 @@ func TestPDSNetworkFailures(t *testing.T) { }) t.Run("handles unreachable PDS server", func(t *testing.T) { + t.Parallel() // Use a port that's guaranteed to be unreachable unreachablePDS := "http://localhost:9999" provisioner := communities.NewPDSAccountProvisioner("test.local", unreachablePDS) @@ -613,6 +624,7 @@ func TestPDSNetworkFailures(t *testing.T) { }) t.Run("handles timeout scenarios", func(t *testing.T) { + t.Parallel() // Create a context with a very short timeout timeoutCtx, cancel := context.WithTimeout(ctx, 1) defer cancel() @@ -629,6 +641,7 @@ func TestPDSNetworkFailures(t *testing.T) { }) t.Run("FetchPDSDID handles invalid URLs", func(t *testing.T) { + t.Parallel() invalidURLs := []string{ "not-a-url", "http://", @@ -647,6 +660,7 @@ func TestPDSNetworkFailures(t *testing.T) { }) t.Run("FetchPDSDID handles unreachable server", func(t *testing.T) { + t.Parallel() unreachablePDS := "http://localhost:9998" _, err := communities.FetchPDSDID(ctx, unreachablePDS) @@ -662,6 +676,7 @@ func TestPDSNetworkFailures(t *testing.T) { }) t.Run("FetchPDSDID handles timeout", func(t *testing.T) { + t.Parallel() timeoutCtx, cancel := context.WithTimeout(ctx, 1) defer cancel() @@ -679,6 +694,7 @@ func TestPDSNetworkFailures(t *testing.T) { // TestTokenValidation verifies that PDS-returned tokens meet requirements // Critical for P0: Tokens must be valid JWTs that can be used for authentication func TestTokenValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/community_repo_test.go b/tests/integration/community_repo_test.go index 7e116b6..46307cd 100644 --- a/tests/integration/community_repo_test.go +++ b/tests/integration/community_repo_test.go @@ -13,6 +13,7 @@ import ( ) func TestCommunityRepository_Create(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -119,6 +120,7 @@ func TestCommunityRepository_Create(t *testing.T) { } func TestCommunityRepository_GetByDID(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -172,6 +174,7 @@ func TestCommunityRepository_GetByDID(t *testing.T) { } func TestCommunityRepository_GetByHandle(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -213,6 +216,7 @@ func TestCommunityRepository_GetByHandle(t *testing.T) { } func TestCommunityRepository_Subscriptions(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -302,6 +306,7 @@ func TestCommunityRepository_Subscriptions(t *testing.T) { } func TestCommunityRepository_List(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -388,6 +393,7 @@ func TestCommunityRepository_List(t *testing.T) { } func TestCommunityRepository_GetSubscribedCommunityDIDs(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/community_service_integration_test.go b/tests/integration/community_service_integration_test.go index 7d340e9..6b6d8c8 100644 --- a/tests/integration/community_service_integration_test.go +++ b/tests/integration/community_service_integration_test.go @@ -29,6 +29,7 @@ import ( // - Unit tests (direct DB writes, bypass PDS) // - E2E tests (full HTTP + Jetstream flow) func TestCommunityService_CreateWithRealPDS(t *testing.T) { + t.Parallel() // Check if PDS is running pdsURL := "http://localhost:3001" healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -274,6 +275,7 @@ func TestCommunityService_CreateWithRealPDS(t *testing.T) { // - Authorization checks (only creator can update) // - Record rkey is always "self" for V2 func TestCommunityService_UpdateWithRealPDS(t *testing.T) { + t.Parallel() // Check if PDS is running pdsURL := "http://localhost:3001" healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -460,6 +462,7 @@ func TestCommunityService_UpdateWithRealPDS(t *testing.T) { // TestPasswordAuthentication verifies that generated passwords work for PDS authentication // This is CRITICAL for P0: passwords must be recoverable for session renewal func TestPasswordAuthentication(t *testing.T) { + t.Parallel() // Check if PDS is running pdsURL := "http://localhost:3001" healthResp, err := http.Get(pdsURL + "/xrpc/_health") diff --git a/tests/integration/community_suggestion_e2e_test.go b/tests/integration/community_suggestion_e2e_test.go index 05552e7..103f187 100644 --- a/tests/integration/community_suggestion_e2e_test.go +++ b/tests/integration/community_suggestion_e2e_test.go @@ -225,6 +225,7 @@ func setupSuggestionTestRouter(t *testing.T, adminDIDs []string) (http.Handler, // Community Suggestions & Voting feature. It tests the full stack: // HTTP handlers -> service -> repository -> PostgreSQL. func TestCommunitySuggestionE2E(t *testing.T) { + t.Parallel() adminDID := "did:plc:testadmin" userDID := "did:plc:testuser1" user2DID := "did:plc:testuser2" @@ -1112,6 +1113,7 @@ func TestCommunitySuggestionE2E(t *testing.T) { // TestCommunitySuggestionE2E_ViewerStateOnGet tests that the get endpoint properly // populates viewer state for authenticated users. func TestCommunitySuggestionE2E_ViewerStateOnGet(t *testing.T) { + t.Parallel() adminDID := "did:plc:testadmin" userDID := "did:plc:vieweruser1" user2DID := "did:plc:vieweruser2" @@ -1187,6 +1189,7 @@ func TestCommunitySuggestionE2E_ViewerStateOnGet(t *testing.T) { // TestCommunitySuggestionE2E_DownvoteFlow tests the full downvote lifecycle: // downvote, toggle off, then upvote. func TestCommunitySuggestionE2E_DownvoteFlow(t *testing.T) { + t.Parallel() adminDID := "did:plc:testadmin" userDID := "did:plc:downvoteuser1" voterDID := "did:plc:downvotevoter1" diff --git a/tests/integration/community_update_e2e_test.go b/tests/integration/community_update_e2e_test.go index 49f0ec8..bf59c5c 100644 --- a/tests/integration/community_update_e2e_test.go +++ b/tests/integration/community_update_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/atproto/identity" "Coves/internal/atproto/jetstream" @@ -300,10 +311,12 @@ func subscribeToJetstreamForCommunityEvent( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + // The gorilla/websocket library panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 for { select { @@ -312,7 +325,7 @@ func subscribeToJetstreamForCommunityEvent( case <-ctx.Done(): return ctx.Err() default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -326,32 +339,26 @@ func subscribeToJetstreamForCommunityEvent( default: } if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived: %w", err) } - // Handle EOF - connection was closed by server if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) { - return nil + return fmt.Errorf("Jetstream hung up before the event arrived: %w", err) } - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - // If we get too many consecutive timeouts, the connection may be in a bad state - // Exit to avoid the gorilla/websocket panic on repeated reads to failed connections - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue + if netErr, ok := err.(net.Error); ok && netErr.Timeout() { + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } - // Check for connection closed errors (happens during shutdown) + // Still nil: this one only happens when the socket was closed + // underneath us during shutdown, which the done check above + // has already established is not a missing event. if strings.Contains(err.Error(), "use of closed network connection") { return nil } return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - // Check if this is the event we're looking for if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.profile" && diff --git a/tests/integration/community_v2_validation_test.go b/tests/integration/community_v2_validation_test.go index cb09eac..35143b9 100644 --- a/tests/integration/community_v2_validation_test.go +++ b/tests/integration/community_v2_validation_test.go @@ -16,6 +16,7 @@ import ( // TestCommunityConsumer_V2RKeyValidation tests that only V2 communities (rkey="self") are accepted func TestCommunityConsumer_V2RKeyValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) @@ -246,6 +247,7 @@ func TestCommunityConsumer_V2RKeyValidation(t *testing.T) { // TestCommunityConsumer_HandleField tests the V2 handle field func TestCommunityConsumer_HandleField(t *testing.T) { + t.Parallel() db := testkit.DB(t) repo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/concurrent_scenarios_test.go b/tests/integration/concurrent_scenarios_test.go index 6c3709c..565587a 100644 --- a/tests/integration/concurrent_scenarios_test.go +++ b/tests/integration/concurrent_scenarios_test.go @@ -19,6 +19,7 @@ import ( // TestConcurrentVoting_MultipleUsersOnSamePost tests race conditions when multiple users // vote on the same post simultaneously func TestConcurrentVoting_MultipleUsersOnSamePost(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -321,6 +322,7 @@ func TestConcurrentVoting_MultipleUsersOnSamePost(t *testing.T) { // TestConcurrentCommenting_MultipleUsersOnSamePost tests race conditions when multiple users // comment on the same post simultaneously func TestConcurrentCommenting_MultipleUsersOnSamePost(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -574,6 +576,7 @@ func TestConcurrentCommenting_MultipleUsersOnSamePost(t *testing.T) { // TestConcurrentCommunityCreation tests race conditions when multiple goroutines // try to create communities with the same handle func TestConcurrentCommunityCreation_DuplicateHandle(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -704,6 +707,7 @@ func TestConcurrentCommunityCreation_DuplicateHandle(t *testing.T) { // TestConcurrentSubscription tests race conditions when multiple users subscribe // to the same community simultaneously func TestConcurrentSubscription_RaceConditions(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/discover_test.go b/tests/integration/discover_test.go index 2b418ef..3f26527 100644 --- a/tests/integration/discover_test.go +++ b/tests/integration/discover_test.go @@ -73,6 +73,7 @@ func (m *mockVoteService) GetViewerVotesForSubjects(userDID string, subjectURIs // TestGetDiscover_ShowsAllCommunities tests discover feed shows posts from ALL communities func TestGetDiscover_ShowsAllCommunities(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -139,6 +140,7 @@ func TestGetDiscover_ShowsAllCommunities(t *testing.T) { // TestGetDiscover_NoAuthRequired tests discover feed works without authentication func TestGetDiscover_NoAuthRequired(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -181,6 +183,7 @@ func TestGetDiscover_NoAuthRequired(t *testing.T) { // TestGetDiscover_HotSort tests hot sorting across all communities func TestGetDiscover_HotSort(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -233,6 +236,7 @@ func TestGetDiscover_HotSort(t *testing.T) { // a day-old genuinely popular post should outrank a six-hour-old post nobody // voted on. func TestGetDiscover_HotSort_LogDampedRanking(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -288,6 +292,7 @@ func TestGetDiscover_HotSort_LogDampedRanking(t *testing.T) { // exactly once, in rank order, with no skips or duplicates. A divergence // between the live and cursor formulas fails this test. func TestGetDiscover_HotSort_PaginationCoversNegativeScores(t *testing.T) { + t.Parallel() db := testkit.DB(t) discoverRepo := postgres.NewDiscoverRepository(db, "test-cursor-secret") @@ -344,6 +349,7 @@ func TestGetDiscover_HotSort_PaginationCoversNegativeScores(t *testing.T) { // future-dated post ranks like a brand-new 0-vote post — it must not error // the query (negative POWER base) and must not outrank a post with real votes. func TestGetDiscover_HotSort_FutureDatedPost(t *testing.T) { + t.Parallel() db := testkit.DB(t) discoverRepo := postgres.NewDiscoverRepository(db, "test-cursor-secret") @@ -385,6 +391,7 @@ func TestGetDiscover_HotSort_FutureDatedPost(t *testing.T) { // TestGetDiscover_Pagination tests cursor-based pagination func TestGetDiscover_Pagination(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -437,6 +444,7 @@ func TestGetDiscover_Pagination(t *testing.T) { // TestGetDiscover_LimitValidation tests limit parameter validation func TestGetDiscover_LimitValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -462,6 +470,7 @@ func TestGetDiscover_LimitValidation(t *testing.T) { // TestGetDiscover_ViewerVoteState tests that authenticated users see their vote state on posts func TestGetDiscover_ViewerVoteState(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -545,6 +554,7 @@ func TestGetDiscover_ViewerVoteState(t *testing.T) { // TestGetDiscover_NoViewerStateWithoutAuth tests that unauthenticated users don't get viewer state func TestGetDiscover_NoViewerStateWithoutAuth(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/feed_test.go b/tests/integration/feed_test.go index 21ff808..cd144ec 100644 --- a/tests/integration/feed_test.go +++ b/tests/integration/feed_test.go @@ -41,6 +41,7 @@ func getPostTitle(t *testing.T, pv *posts.PostView) string { // TestGetCommunityFeed_Hot tests hot feed sorting algorithm func TestGetCommunityFeed_Hot(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -115,6 +116,7 @@ func TestGetCommunityFeed_Hot(t *testing.T) { // TestGetCommunityFeed_Top_WithTimeframe tests top sorting with time filters func TestGetCommunityFeed_Top_WithTimeframe(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -188,6 +190,7 @@ func TestGetCommunityFeed_Top_WithTimeframe(t *testing.T) { // TestGetCommunityFeed_New tests chronological sorting func TestGetCommunityFeed_New(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -241,6 +244,7 @@ func TestGetCommunityFeed_New(t *testing.T) { // TestGetCommunityFeed_Pagination tests cursor-based pagination func TestGetCommunityFeed_Pagination(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -329,6 +333,7 @@ func TestGetCommunityFeed_Pagination(t *testing.T) { // TestGetCommunityFeed_InvalidCommunity tests error handling for invalid community func TestGetCommunityFeed_InvalidCommunity(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -362,6 +367,7 @@ func TestGetCommunityFeed_InvalidCommunity(t *testing.T) { // TestGetCommunityFeed_InvalidCursor tests cursor validation func TestGetCommunityFeed_InvalidCursor(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -415,6 +421,7 @@ func TestGetCommunityFeed_InvalidCursor(t *testing.T) { // TestGetCommunityFeed_EmptyFeed tests handling of empty communities func TestGetCommunityFeed_EmptyFeed(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -456,6 +463,7 @@ func TestGetCommunityFeed_EmptyFeed(t *testing.T) { // TestGetCommunityFeed_LimitValidation tests limit parameter validation func TestGetCommunityFeed_LimitValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -506,6 +514,7 @@ func TestGetCommunityFeed_LimitValidation(t *testing.T) { // TestGetCommunityFeed_HotPaginationBug tests the critical hot pagination bug fix // Verifies that posts with higher raw scores but lower hot ranks don't get dropped during pagination func TestGetCommunityFeed_HotPaginationBug(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -604,6 +613,7 @@ func TestGetCommunityFeed_HotPaginationBug(t *testing.T) { // TestGetCommunityFeed_HotCursorPrecision tests that hot rank cursor preserves full float precision // Regression test for precision bug where posts with hot ranks differing by <1e-6 were dropped func TestGetCommunityFeed_HotCursorPrecision(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -703,6 +713,7 @@ func TestGetCommunityFeed_HotCursorPrecision(t *testing.T) { // Fix: Store the cursor creation timestamp in the cursor and use it for subsequent comparisons, // ensuring stable hot_rank computation across pagination requests. func TestGetCommunityFeed_HotCursorTimeDrift(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -801,6 +812,7 @@ func TestGetCommunityFeed_HotCursorTimeDrift(t *testing.T) { // TestGetCommunityFeed_BlobURLTransformation tests that blob refs are transformed to URLs func TestGetCommunityFeed_BlobURLTransformation(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services diff --git a/tests/integration/helpers.go b/tests/integration/helpers.go index 54b0594..1a955c9 100644 --- a/tests/integration/helpers.go +++ b/tests/integration/helpers.go @@ -124,6 +124,27 @@ func authenticateWithPDS(pdsURL, handle, password string) (string, string, error return sessionResp.AccessJwt, sessionResp.DID, nil } +// jetstreamReadBudget is how long the hand-rolled subscribeToJetstream* helpers +// in this package wait, in total, for the event they are looking for. +// +// It is set once as an absolute read deadline for the whole subscription, and +// its expiry is terminal. That shape is forced by gorilla/websocket: a +// connection is CORRUPT once its read deadline expires, so every read after +// the first timeout fails instantly. The loops used to count those failures +// and give up after ten, which bought nothing — one real expiry plus nine +// phantom ones in microseconds — while advertising "max 30 seconds" over a +// budget that was really five. Worse, the copies in the two avatar files +// retried on NON-timeout errors without counting at all, so a Jetstream that +// hung up sent them into a tight loop straight into gorilla's +// panic("repeated read on failed websocket connection"), which takes the whole +// test binary with it. +// +// So: one deadline, and any read error ends the subscription. testkit.Firehose +// re-dials instead, which is the right answer; these copies are deleted with +// the rest of tests/integration in Phase 4 of docs/TEST_ARCHITECTURE.md rather +// than being rebuilt here. +const jetstreamReadBudget = 30 * time.Second + // tidCounter is used to ensure unique TIDs even when generateTID is called rapidly. var tidCounter atomic.Uint64 diff --git a/tests/integration/identity_resolution_test.go b/tests/integration/identity_resolution_test.go index bf75d55..3b89d01 100644 --- a/tests/integration/identity_resolution_test.go +++ b/tests/integration/identity_resolution_test.go @@ -18,6 +18,7 @@ func uniqueID() string { // TestIdentityCache tests the PostgreSQL identity cache operations func TestIdentityCache(t *testing.T) { + t.Parallel() db := testkit.DB(t) cache := identity.NewPostgresCache(db, 5*time.Minute) @@ -211,6 +212,7 @@ func TestIdentityCache(t *testing.T) { // TestIdentityCacheTTL tests that expired cache entries are not returned func TestIdentityCacheTTL(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Create cache with very short TTL (reduced from 1s to 100ms for faster, less flaky tests) @@ -252,6 +254,7 @@ func TestIdentityCacheTTL(t *testing.T) { // TestIdentityResolverWithCache tests the caching resolver behavior func TestIdentityResolverWithCache(t *testing.T) { + t.Parallel() db := testkit.DB(t) cache := identity.NewPostgresCache(db, 5*time.Minute) diff --git a/tests/integration/image_proxy_e2e_test.go b/tests/integration/image_proxy_e2e_test.go index 69461dd..32ecb1d 100644 --- a/tests/integration/image_proxy_e2e_test.go +++ b/tests/integration/image_proxy_e2e_test.go @@ -39,6 +39,7 @@ import ( // - Testing ETag-based caching (304 responses) // - Error handling for invalid presets and missing blobs func TestImageProxy_E2E(t *testing.T) { + t.Parallel() // Check if PDS is running pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -369,6 +370,7 @@ func TestImageProxy_E2E(t *testing.T) { // TestImageProxy_CacheHit tests that cache hits are faster than cache misses func TestImageProxy_CacheHit(t *testing.T) { + t.Parallel() // Check if PDS is running pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -522,6 +524,7 @@ func createImageProxyTestServerWithCache(t *testing.T, pdsURL string, identityRe // TestImageProxy_MockPDS tests the image proxy with a mock PDS server // This allows testing image proxy behavior without a real PDS func TestImageProxy_MockPDS(t *testing.T) { + t.Parallel() // Create test image testImage := createTestImageForProxy(t, 100, 100, color.RGBA{R: 255, G: 128, B: 64, A: 255}) testCID := "bafybeimockimagetest123" @@ -643,6 +646,7 @@ func (m *mockIdentityResolverForImageProxy) Purge(ctx context.Context, identifie // TestImageProxy_ErrorHandling tests various error conditions func TestImageProxy_ErrorHandling(t *testing.T) { + t.Parallel() // Create mock identity resolver mockResolver := &mockIdentityResolverForImageProxy{ pdsURL: "http://localhost:9999", // Non-existent server @@ -727,6 +731,7 @@ func (m *errorMockResolver) Purge(ctx context.Context, identifier string) error // TestImageProxy_UnsupportedFormat tests behavior with unsupported image formats func TestImageProxy_UnsupportedFormat(t *testing.T) { + t.Parallel() // Create mock PDS that returns invalid image data mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(r.URL.Path, "/xrpc/com.atproto.sync.getBlob") { @@ -805,6 +810,7 @@ func TestImageProxy_UnsupportedFormat(t *testing.T) { // TestImageProxy_LargeImage tests behavior with large images func TestImageProxy_LargeImage(t *testing.T) { + t.Parallel() // Create a large test image (1000x1000) largeImage := createTestImageForProxy(t, 1000, 1000, color.RGBA{R: 200, G: 100, B: 50, A: 255}) testCID := "bafylargeimagecid" @@ -892,6 +898,7 @@ func TestImageProxy_LargeImage(t *testing.T) { // TestImageProxy_ResponseJSON verifies no JSON is returned (should be plain text or image) func TestImageProxy_ResponseJSON(t *testing.T) { + t.Parallel() mockResolver := &mockIdentityResolverForImageProxy{ pdsURL: "http://localhost:9999", } diff --git a/tests/integration/jetstream_consumer_test.go b/tests/integration/jetstream_consumer_test.go index d68cdd2..bba5003 100644 --- a/tests/integration/jetstream_consumer_test.go +++ b/tests/integration/jetstream_consumer_test.go @@ -14,6 +14,7 @@ import ( ) func TestUserIndexingFromJetstream(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Wire up dependencies @@ -307,6 +308,7 @@ func TestUserIndexingFromJetstream(t *testing.T) { } func TestUserServiceIdempotency(t *testing.T) { + t.Parallel() db := testkit.DB(t) userRepo := postgres.NewUserRepository(db) diff --git a/tests/integration/oauth_e2e_test.go b/tests/integration/oauth_e2e_test.go index 3b666c2..cb7350a 100644 --- a/tests/integration/oauth_e2e_test.go +++ b/tests/integration/oauth_e2e_test.go @@ -32,6 +32,7 @@ import ( // The OAuth redirect flow is handled by indigo's library and enforces OAuth 2.0 spec // (HTTPS required for authorization servers and redirect URIs). func TestOAuth_Components(t *testing.T) { + t.Parallel() // Setup test database db := testkit.DB(t) @@ -135,6 +136,7 @@ func testOAuthComponentsWithMockedSession(t *testing.T, ctx context.Context, _ i // TestOAuthE2E_TokenExpiration tests that expired sealed tokens are rejected func TestOAuthE2E_TokenExpiration(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -192,6 +194,7 @@ func TestOAuthE2E_TokenExpiration(t *testing.T) { // TestOAuthE2E_InvalidToken tests that invalid/tampered tokens are rejected func TestOAuthE2E_InvalidToken(t *testing.T) { + t.Parallel() db := testkit.DB(t) t.Log("🔒 Testing OAuth invalid token rejection...") @@ -248,6 +251,7 @@ func TestOAuthE2E_InvalidToken(t *testing.T) { // TestOAuthE2E_SessionNotFound tests behavior when session doesn't exist in DB func TestOAuthE2E_SessionNotFound(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -275,6 +279,7 @@ func TestOAuthE2E_SessionNotFound(t *testing.T) { // TestOAuthE2E_MultipleSessionsPerUser tests that a user can have multiple active sessions func TestOAuthE2E_MultipleSessionsPerUser(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -355,6 +360,7 @@ func TestOAuthE2E_MultipleSessionsPerUser(t *testing.T) { // TestOAuthE2E_AuthRequestStorage tests OAuth auth request storage and retrieval func TestOAuthE2E_AuthRequestStorage(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -456,6 +462,7 @@ func TestOAuthE2E_AuthRequestStorage(t *testing.T) { // TestOAuthE2E_TokenRefresh tests the refresh token flow func TestOAuthE2E_TokenRefresh(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -703,6 +710,7 @@ func TestOAuthE2E_TokenRefresh(t *testing.T) { // TestOAuthE2E_SessionUpdate tests that refresh updates the session in database func TestOAuthE2E_SessionUpdate(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -781,6 +789,7 @@ func TestOAuthE2E_SessionUpdate(t *testing.T) { // TestOAuthE2E_RefreshTokenRotation tests refresh token rotation behavior func TestOAuthE2E_RefreshTokenRotation(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/oauth_session_fixation_test.go b/tests/integration/oauth_session_fixation_test.go index f44a0ad..d5bc40f 100644 --- a/tests/integration/oauth_session_fixation_test.go +++ b/tests/integration/oauth_session_fixation_test.go @@ -34,6 +34,7 @@ import ( // 5. WITHOUT THE FIX: Callback sends sealed token, DID, session_id to attacker's deep link // 6. WITH THE FIX: Binding mismatch is detected, mobile cookies cleared, user gets web session func TestOAuth_SessionFixationAttackPrevention(t *testing.T) { + t.Parallel() // Setup test database db := testkit.DB(t) diff --git a/tests/integration/oauth_session_handle_sync_test.go b/tests/integration/oauth_session_handle_sync_test.go index 9c53cc3..8fa2f6b 100644 --- a/tests/integration/oauth_session_handle_sync_test.go +++ b/tests/integration/oauth_session_handle_sync_test.go @@ -4,6 +4,7 @@ package integration import ( "context" + "errors" "fmt" "net/http" "testing" @@ -27,15 +28,11 @@ import ( // This ensures mobile/web apps display the correct handle after a user // changes their handle on their PDS. // -// Prerequisites: -// - Test database on localhost:5434 +// Run with `make test-integration`, or against an already-running dev stack: // -// Run with: -// -// docker-compose --profile test up -d postgres-test -// TEST_DATABASE_URL="postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" \ -// go test -v ./tests/integration/ -run "TestOAuthSessionHandleSync" +// go test -tags integration ./tests/integration/ -run TestOAuthSessionHandleSync func TestOAuthSessionHandleSync(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -278,16 +275,14 @@ func TestOAuthSessionHandleSync(t *testing.T) { // TestOAuthSessionHandleSync_LiveJetstream tests the full flow with real Jetstream // This requires the dev infrastructure to be running. // -// Prerequisites: -// - PDS running on localhost:3001 -// - Jetstream running on localhost:6008 -// - Test database on localhost:5434 +// Run with `make test-integration` (which brings the stack up), or directly +// against a running one: // -// Run with: +// go test -tags integration ./tests/integration/ -run TestOAuthSessionHandleSync_LiveJetstream // -// docker-compose --profile test --profile jetstream up -d -// TEST_DATABASE_URL="postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" \ -// go test -v ./tests/integration/ -run "TestOAuthSessionHandleSync_LiveJetstream" +// SERIAL BY DESIGN — no t.Parallel(): this is the one test in the file that +// runs a connector against the shared live stream rather than feeding a +// consumer in process. func TestOAuthSessionHandleSync_LiveJetstream(t *testing.T) { // Check if Jetstream is available if !isServiceAvailable("http://localhost:6008") { @@ -319,15 +314,29 @@ func TestOAuthSessionHandleSync_LiveJetstream(t *testing.T) { ) connector := jetstream.NewConnector("users-test", "ws://localhost:6008/subscribe", consumer) - // Start consumer in background + // Start consumer in background, and JOIN IT before returning. + // + // Without the join this goroutine outlives the test, and its t.Logf then + // panics the whole binary with "Log in goroutine after test has completed". + // That is not theoretical here: the clone this consumer writes to is + // dropped WITH (FORCE) in testkit.DB's cleanup, so Start returns a database + // error rather than context.Canceled, sails past the guard, and logs. consumerCtx, consumerCancel := context.WithCancel(ctx) - defer consumerCancel() - - go func() { - if err := connector.Start(consumerCtx); err != nil && err != context.Canceled { - t.Logf("Consumer stopped: %v", err) + consumerStopped := make(chan error, 1) + go func() { consumerStopped <- connector.Start(consumerCtx) }() + t.Cleanup(func() { + consumerCancel() + select { + case err := <-consumerStopped: + // Reported after the join, so it is the test's own goroutine + // logging, and only when it is not the shutdown we asked for. + if err != nil && !errors.Is(err, context.Canceled) { + t.Logf("Consumer stopped: %v", err) + } + case <-time.After(10 * time.Second): + t.Errorf("consumer goroutine did not stop within 10s of cancellation") } - }() + }) // Give consumer time to connect time.Sleep(500 * time.Millisecond) diff --git a/tests/integration/oauth_token_verification_test.go b/tests/integration/oauth_token_verification_test.go index 284ad6c..ed8b6b2 100644 --- a/tests/integration/oauth_token_verification_test.go +++ b/tests/integration/oauth_token_verification_test.go @@ -25,6 +25,7 @@ import ( // for testing purposes. Real OAuth tokens from PDS would be sealed using the // OAuth client's seal secret. func TestOAuthTokenVerification(t *testing.T) { + t.Parallel() pdsURL := os.Getenv("PDS_URL") if pdsURL == "" { diff --git a/tests/integration/post_consumer_test.go b/tests/integration/post_consumer_test.go index 4286b5b..0803982 100644 --- a/tests/integration/post_consumer_test.go +++ b/tests/integration/post_consumer_test.go @@ -20,6 +20,7 @@ import ( // comment suggests reconciliation is not implemented. This test verifies that // the reconciliation logic in post_consumer.go:210-226 works correctly. func TestPostConsumer_CommentCountReconciliation(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/post_creation_test.go b/tests/integration/post_creation_test.go index aa221df..a4f85f4 100644 --- a/tests/integration/post_creation_test.go +++ b/tests/integration/post_creation_test.go @@ -20,6 +20,7 @@ import ( ) func TestPostCreation_Basic(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup: Initialize services @@ -280,6 +281,7 @@ func TestPostCreation_Basic(t *testing.T) { // TestPostRepository_Create tests the repository layer func TestPostRepository_Create(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup: Create test user and community diff --git a/tests/integration/post_delete_test.go b/tests/integration/post_delete_test.go index 42c1a82..ea3c17b 100644 --- a/tests/integration/post_delete_test.go +++ b/tests/integration/post_delete_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/api/middleware" "Coves/internal/atproto/identity" @@ -714,10 +725,10 @@ func subscribeToJetstreamForPostCreate( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) for { select { @@ -726,7 +737,7 @@ func subscribeToJetstreamForPostCreate( case <-ctx.Done(): return ctx.Err() default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -734,21 +745,17 @@ func subscribeToJetstreamForPostCreate( err := conn.ReadJSON(&event) if err != nil { if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.post" && event.Commit.Operation == "create" { @@ -783,10 +790,10 @@ func subscribeToJetstreamForPostDelete( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) for { select { @@ -795,7 +802,7 @@ func subscribeToJetstreamForPostDelete( case <-ctx.Done(): return ctx.Err() default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -803,21 +810,17 @@ func subscribeToJetstreamForPostDelete( err := conn.ReadJSON(&event) if err != nil { if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.post" && event.Commit.Operation == "delete" { diff --git a/tests/integration/post_e2e_test.go b/tests/integration/post_e2e_test.go index 614285f..22cf0ba 100644 --- a/tests/integration/post_e2e_test.go +++ b/tests/integration/post_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/api/handlers/post" "Coves/internal/atproto/identity" @@ -595,10 +606,10 @@ func subscribeToJetstreamForPost( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) // Read messages until we find our event or receive done signal for { @@ -609,7 +620,7 @@ func subscribeToJetstreamForPost( return ctx.Err() default: // Set read deadline to avoid blocking forever - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -618,22 +629,18 @@ func subscribeToJetstreamForPost( if err != nil { // Check if it's a timeout (expected) if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue // Timeout is expected, keep listening + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } // For other errors, don't retry reading from a broken connection return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - // Check if this is a post event for the target DID if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.post" { diff --git a/tests/integration/post_handler_test.go b/tests/integration/post_handler_test.go index f7cf5c5..735ba31 100644 --- a/tests/integration/post_handler_test.go +++ b/tests/integration/post_handler_test.go @@ -22,6 +22,7 @@ import ( // TestPostHandler_SecurityValidation tests HTTP handler-level security checks func TestPostHandler_SecurityValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -383,6 +384,7 @@ func TestPostHandler_SecurityValidation(t *testing.T) { // TestPostHandler_SpecialCharacters tests content with special characters func TestPostHandler_SpecialCharacters(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -460,6 +462,7 @@ func TestPostHandler_SpecialCharacters(t *testing.T) { // TestPostService_DIDValidationSecurity tests service-layer DID validation (defense-in-depth) func TestPostService_DIDValidationSecurity(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services diff --git a/tests/integration/post_thumb_validation_test.go b/tests/integration/post_thumb_validation_test.go index 51ec3f2..0ab4af7 100644 --- a/tests/integration/post_thumb_validation_test.go +++ b/tests/integration/post_thumb_validation_test.go @@ -45,6 +45,7 @@ func createTestCommunityWithCredentials(t *testing.T, repo communities.Repositor // TestPostHandler_ThumbValidation tests strict validation of thumb field in external embeds func TestPostHandler_ThumbValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -292,6 +293,7 @@ func TestPostHandler_ThumbValidation(t *testing.T) { // tests still pass — guarding against regression of the silent-corruption bug // the validation exists to prevent. func TestPostHandler_EmbedValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) communityRepo := postgres.NewCommunityRepository(db) diff --git a/tests/integration/post_unfurl_test.go b/tests/integration/post_unfurl_test.go index b2d669b..b2292e2 100644 --- a/tests/integration/post_unfurl_test.go +++ b/tests/integration/post_unfurl_test.go @@ -27,6 +27,7 @@ import ( // TestPostUnfurl_UnsupportedURL tests that posts with unsupported URLs still succeed func TestPostUnfurl_UnsupportedURL(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -117,6 +118,7 @@ func TestPostUnfurl_UnsupportedURL(t *testing.T) { // TestPostUnfurl_MissingEmbedType tests posts without external embed type don't trigger unfurling func TestPostUnfurl_MissingEmbedType(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -242,6 +244,7 @@ func TestPostUnfurl_MissingEmbedType(t *testing.T) { // The kagi-news trusted aggregator already supplies authoritative metadata from // the Kagi JSON feed, so the unfurl path for Kite URLs is intentionally disabled. func TestPostUnfurl_KagiKiteExcluded(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -278,6 +281,7 @@ func TestPostUnfurl_KagiKiteExcluded(t *testing.T) { // TestPostUnfurl_E2E_WithJetstream tests the full unfurl flow with Jetstream consumer // This simulates: Create post → unfurl → write to PDS → Jetstream event → index in AppView func TestPostUnfurl_E2E_WithJetstream(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() diff --git a/tests/integration/subscription_indexing_test.go b/tests/integration/subscription_indexing_test.go index c808820..520af31 100644 --- a/tests/integration/subscription_indexing_test.go +++ b/tests/integration/subscription_indexing_test.go @@ -18,6 +18,7 @@ import ( // TestSubscriptionIndexing_ContentVisibility tests that contentVisibility is properly indexed // from Jetstream events and stored in the AppView database func TestSubscriptionIndexing_ContentVisibility(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -238,6 +239,7 @@ func TestSubscriptionIndexing_ContentVisibility(t *testing.T) { // TestSubscriptionIndexing_DeleteOperations tests unsubscribe (DELETE) event handling func TestSubscriptionIndexing_DeleteOperations(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -349,6 +351,7 @@ func TestSubscriptionIndexing_DeleteOperations(t *testing.T) { // TestSubscriptionIndexing_SubscriberCount tests that subscriber counts are updated atomically func TestSubscriptionIndexing_SubscriberCount(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) diff --git a/tests/integration/timeline_test.go b/tests/integration/timeline_test.go index 3f502b4..cb56786 100644 --- a/tests/integration/timeline_test.go +++ b/tests/integration/timeline_test.go @@ -23,6 +23,7 @@ import ( // TestGetTimeline_Basic tests timeline feed shows posts from subscribed communities func TestGetTimeline_Basic(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -107,6 +108,7 @@ func TestGetTimeline_Basic(t *testing.T) { // TestGetTimeline_HotSort tests hot sorting across multiple communities func TestGetTimeline_HotSort(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -173,6 +175,7 @@ func TestGetTimeline_HotSort(t *testing.T) { // TestGetTimeline_Pagination tests cursor-based pagination func TestGetTimeline_Pagination(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -244,6 +247,7 @@ func TestGetTimeline_Pagination(t *testing.T) { // TestGetTimeline_EmptyWhenNoSubscriptions tests timeline is empty when user has no subscriptions func TestGetTimeline_EmptyWhenNoSubscriptions(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -281,6 +285,7 @@ func TestGetTimeline_EmptyWhenNoSubscriptions(t *testing.T) { // TestGetTimeline_Unauthorized tests timeline requires authentication func TestGetTimeline_Unauthorized(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -305,6 +310,7 @@ func TestGetTimeline_Unauthorized(t *testing.T) { // TestGetTimeline_LimitValidation tests limit parameter validation func TestGetTimeline_LimitValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services @@ -351,6 +357,7 @@ func TestGetTimeline_LimitValidation(t *testing.T) { // - Tests all sorting modes (hot, top, new) across communities // - Ensures proper aggregation and no cross-contamination func TestGetTimeline_MultiCommunity_E2E(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Setup services diff --git a/tests/integration/token_refresh_test.go b/tests/integration/token_refresh_test.go index e4db735..0bf0eb4 100644 --- a/tests/integration/token_refresh_test.go +++ b/tests/integration/token_refresh_test.go @@ -16,6 +16,7 @@ import ( // TestTokenRefresh_ExpirationDetection tests the NeedsRefresh function with various token states func TestTokenRefresh_ExpirationDetection(t *testing.T) { + t.Parallel() tests := []struct { name string token string @@ -96,6 +97,7 @@ func TestTokenRefresh_ExpirationDetection(t *testing.T) { // TestTokenRefresh_UpdateCredentials tests the repository UpdateCredentials method func TestTokenRefresh_UpdateCredentials(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -157,6 +159,7 @@ func TestTokenRefresh_UpdateCredentials(t *testing.T) { // TestTokenRefresh_E2E_UpdateAfterTokenRefresh tests end-to-end token refresh during community update func TestTokenRefresh_E2E_UpdateAfterTokenRefresh(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) diff --git a/tests/integration/user_journey_e2e_test.go b/tests/integration/user_journey_e2e_test.go index d49ea0c..885bf21 100644 --- a/tests/integration/user_journey_e2e_test.go +++ b/tests/integration/user_journey_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/api/routes" "Coves/internal/atproto/identity" @@ -851,10 +862,12 @@ func subscribeToJetstreamForCommunity( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + // The gorilla/websocket library panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 for { select { @@ -863,7 +876,7 @@ func subscribeToJetstreamForCommunity( case <-ctx.Done(): return ctx.Err() default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -872,24 +885,19 @@ func subscribeToJetstreamForCommunity( if err != nil { // Handle close errors - connection is done if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived: %w", err) } - // Handle EOF - connection was closed by server if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) { - return nil + return fmt.Errorf("Jetstream hung up before the event arrived: %w", err) } // Handle timeout errors using errors.As for wrapped errors - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - // If we get too many consecutive timeouts, the connection may be in a bad state - // Exit to avoid the gorilla/websocket panic on repeated reads to failed connections - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue + if netErr, ok := err.(net.Error); ok && netErr.Timeout() { + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } // For any other error, return immediately to avoid re-reading from failed connection @@ -897,9 +905,6 @@ func subscribeToJetstreamForCommunity( return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - if event.Did == targetDID && event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.community.profile" { if err := consumer.HandleEvent(ctx, &event); err != nil { diff --git a/tests/integration/user_profile_avatar_e2e_test.go b/tests/integration/user_profile_avatar_e2e_test.go index 3520fa4..4dad196 100644 --- a/tests/integration/user_profile_avatar_e2e_test.go +++ b/tests/integration/user_profile_avatar_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/api/handlers/user" "Coves/internal/api/routes" @@ -13,12 +24,10 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "image" "image/color" "image/png" - "net" "net/http" "net/http/httptest" "net/url" @@ -160,7 +169,11 @@ func TestUserProfileAvatarE2E_UpdateWithAvatar(t *testing.T) { } defer func() { _ = conn.Close() }() - consecutiveTimeouts := 0 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + for { select { case <-done: @@ -168,22 +181,18 @@ func TestUserProfileAvatarE2E_UpdateWithAvatar(t *testing.T) { case <-subscribeCtx.Done(): return default: - if deadlineErr := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); deadlineErr != nil { + if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { return } var event jetstream.JetstreamEvent if readErr := conn.ReadJSON(&event); readErr != nil { - var netErr net.Error - if errors.As(readErr, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= 10 { - return // Connection stale, exit to prevent panic - } - } - continue + // Any read error ends this subscription. A gorilla connection is + // corrupt once its read deadline has expired, and looping on it + // is what reaches the panic that aborts the whole test binary. + // The caller's own timeout reports the missing event. + return } - consecutiveTimeouts = 0 // Only process profile update events for our user if event.Kind == "commit" && event.Commit != nil && @@ -229,7 +238,7 @@ func TestUserProfileAvatarE2E_UpdateWithAvatar(t *testing.T) { // Wait for REAL Jetstream event t.Logf("\n Waiting for profile update event from Jetstream...") var realEvent *jetstream.JetstreamEvent - timeout := time.After(15 * time.Second) + timeout := time.After(jetstreamReadBudget) eventLoop: for { @@ -432,7 +441,11 @@ func TestUserProfileAvatarE2E_UpdateWithBanner(t *testing.T) { } defer func() { _ = conn.Close() }() - consecutiveTimeouts := 0 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + for { select { case <-done: @@ -440,22 +453,18 @@ func TestUserProfileAvatarE2E_UpdateWithBanner(t *testing.T) { case <-subscribeCtx.Done(): return default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return } var event jetstream.JetstreamEvent if err := conn.ReadJSON(&event); err != nil { - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= 10 { - return // Connection stale, exit to prevent panic - } - } - continue + // Any read error ends this subscription. A gorilla connection is + // corrupt once its read deadline has expired, and looping on it + // is what reaches the panic that aborts the whole test binary. + // The caller's own timeout reports the missing event. + return } - consecutiveTimeouts = 0 if event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.actor.profile" && @@ -496,7 +505,7 @@ func TestUserProfileAvatarE2E_UpdateWithBanner(t *testing.T) { // Wait for Jetstream event t.Logf("\n Waiting for profile update event from Jetstream...") var realEvent *jetstream.JetstreamEvent - timeout := time.After(15 * time.Second) + timeout := time.After(jetstreamReadBudget) eventLoop: for { @@ -646,7 +655,11 @@ func TestUserProfileAvatarE2E_UpdateDisplayNameAndBio(t *testing.T) { } defer func() { _ = conn.Close() }() - consecutiveTimeouts := 0 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) + for { select { case <-done: @@ -654,22 +667,18 @@ func TestUserProfileAvatarE2E_UpdateDisplayNameAndBio(t *testing.T) { case <-subscribeCtx.Done(): return default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return } var event jetstream.JetstreamEvent if err := conn.ReadJSON(&event); err != nil { - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= 10 { - return // Connection stale, exit to prevent panic - } - } - continue + // Any read error ends this subscription. A gorilla connection is + // corrupt once its read deadline has expired, and looping on it + // is what reaches the panic that aborts the whole test binary. + // The caller's own timeout reports the missing event. + return } - consecutiveTimeouts = 0 if event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.actor.profile" && @@ -704,7 +713,7 @@ func TestUserProfileAvatarE2E_UpdateDisplayNameAndBio(t *testing.T) { // Wait for Jetstream event var realEvent *jetstream.JetstreamEvent - timeout := time.After(15 * time.Second) + timeout := time.After(jetstreamReadBudget) eventLoop: for { @@ -828,9 +837,13 @@ func TestUserProfileAvatarE2E_ReplaceAvatar(t *testing.T) { return } defer func() { _ = conn.Close() }() + + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) close(ready) // socket dialed; safe for the caller to write - consecutiveTimeouts := 0 for { select { case <-done: @@ -838,22 +851,18 @@ func TestUserProfileAvatarE2E_ReplaceAvatar(t *testing.T) { case <-subscribeCtx.Done(): return default: - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return } var event jetstream.JetstreamEvent if err := conn.ReadJSON(&event); err != nil { - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= 10 { - return // Connection stale, exit to prevent panic - } - } - continue + // Any read error ends this subscription. A gorilla connection is + // corrupt once its read deadline has expired, and looping on it + // is what reaches the panic that aborts the whole test binary. + // The caller's own timeout reports the missing event. + return } - consecutiveTimeouts = 0 if event.Kind == "commit" && event.Commit != nil && event.Commit.Collection == "social.coves.actor.profile" && diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 195e1ec..d5faaf5 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -51,6 +51,7 @@ func generateTestDID(suffix string) string { } func TestUserCreationAndRetrieval(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Wire up dependencies @@ -116,6 +117,7 @@ func TestUserCreationAndRetrieval(t *testing.T) { } func TestGetProfileEndpoint(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Wire up dependencies @@ -215,6 +217,7 @@ func TestGetProfileEndpoint(t *testing.T) { // TestDuplicateCreation tests that duplicate DID/handle creation fails properly func TestDuplicateCreation(t *testing.T) { + t.Parallel() db := testkit.DB(t) userRepo := postgres.NewUserRepository(db) @@ -270,6 +273,7 @@ func TestDuplicateCreation(t *testing.T) { // TestUserRepository_GetByDIDs tests the batch user retrieval functionality func TestUserRepository_GetByDIDs(t *testing.T) { + t.Parallel() db := testkit.DB(t) userRepo := postgres.NewUserRepository(db) @@ -429,6 +433,7 @@ func TestUserRepository_GetByDIDs(t *testing.T) { // TestProfileStats tests that profile stats are returned correctly func TestProfileStats(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Use unique test DID to avoid conflicts with other test runs @@ -572,6 +577,7 @@ func TestProfileStats(t *testing.T) { // TestProfileStats_CommentCount tests that comment counting works correctly func TestProfileStats_CommentCount(t *testing.T) { + t.Parallel() db := testkit.DB(t) uniqueSuffix := time.Now().UnixNano() @@ -662,6 +668,7 @@ func TestProfileStats_CommentCount(t *testing.T) { // TestProfileStats_CommunityCount tests that subscription counting works correctly func TestProfileStats_CommunityCount(t *testing.T) { + t.Parallel() db := testkit.DB(t) uniqueSuffix := time.Now().UnixNano() @@ -721,6 +728,7 @@ func TestProfileStats_CommunityCount(t *testing.T) { // TestGetProfile_NonExistentDID tests that GetProfile returns appropriate error for non-existent DID func TestGetProfile_NonExistentDID(t *testing.T) { + t.Parallel() db := testkit.DB(t) userRepo := postgres.NewUserRepository(db) @@ -767,6 +775,7 @@ func TestGetProfile_NonExistentDID(t *testing.T) { // TestProfileStatsEndpoint tests the HTTP endpoint returns stats correctly func TestProfileStatsEndpoint(t *testing.T) { + t.Parallel() db := testkit.DB(t) // Wire up dependencies @@ -859,6 +868,7 @@ func TestProfileStatsEndpoint(t *testing.T) { // TestHandleValidation tests atProto handle validation rules func TestHandleValidation(t *testing.T) { + t.Parallel() db := testkit.DB(t) userRepo := postgres.NewUserRepository(db) @@ -979,6 +989,7 @@ func TestHandleValidation(t *testing.T) { // TestAccountDeletion_Integration tests the complete account deletion flow // from handler → service → repository with a real database func TestAccountDeletion_Integration(t *testing.T) { + t.Parallel() db := testkit.DB(t) uniqueSuffix := time.Now().UnixNano() diff --git a/tests/integration/userblock_e2e_test.go b/tests/integration/userblock_e2e_test.go index 6e23420..568a2fa 100644 --- a/tests/integration/userblock_e2e_test.go +++ b/tests/integration/userblock_e2e_test.go @@ -26,6 +26,7 @@ import ( // Flow: Client -> XRPC -> PDS Write -> Verify on PDS -> Jetstream -> Consumer -> AppView // Then: Client -> XRPC Unblock -> PDS Delete -> Jetstream -> Consumer -> AppView removal func TestUserBlockE2E_BlockAndUnblock(t *testing.T) { + t.Parallel() db := testkit.DB(t) ctx := context.Background() @@ -363,6 +364,7 @@ func TestUserBlockE2E_BlockAndUnblock(t *testing.T) { // TestUserBlockE2E_SelfBlockPrevented tests that a user cannot block themselves. // This validates the self-block guard in the service layer with a real PDS. func TestUserBlockE2E_SelfBlockPrevented(t *testing.T) { + t.Parallel() db := testkit.DB(t) pdsURL := getTestPDSURL() diff --git a/tests/integration/userblock_enforcement_test.go b/tests/integration/userblock_enforcement_test.go index 17dc273..9987c05 100644 --- a/tests/integration/userblock_enforcement_test.go +++ b/tests/integration/userblock_enforcement_test.go @@ -21,6 +21,7 @@ import ( // blocked users' posts from community feeds when a viewer is authenticated, // but still shows them to unauthenticated viewers. func TestUserBlock_CommunityFeedFiltering(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -173,6 +174,7 @@ func TestUserBlock_CommunityFeedFiltering(t *testing.T) { // blocked users' posts from the discover feed when a viewer is authenticated, // but still shows them to unauthenticated viewers. func TestUserBlock_DiscoverFeedFiltering(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -297,6 +299,7 @@ func TestUserBlock_DiscoverFeedFiltering(t *testing.T) { // TestUserBlock_ProfileViewerState verifies that the user block repository correctly // returns block records, confirming that GetBlock returns a RecordURI when a block exists. func TestUserBlock_ProfileViewerState(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -359,6 +362,7 @@ func TestUserBlock_ProfileViewerState(t *testing.T) { // TestUserBlock_CommentFiltering verifies that comments from blocked users are // filtered out when querying with a viewerDID. func TestUserBlock_CommentFiltering(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -466,6 +470,7 @@ func TestUserBlock_CommentFiltering(t *testing.T) { // TestUserBlock_TimelineFeedFiltering verifies that user block enforcement filters // blocked users' posts from the authenticated user's timeline feed. func TestUserBlock_TimelineFeedFiltering(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) diff --git a/tests/integration/userblock_handler_test.go b/tests/integration/userblock_handler_test.go index 95cbf9d..afafb97 100644 --- a/tests/integration/userblock_handler_test.go +++ b/tests/integration/userblock_handler_test.go @@ -206,6 +206,7 @@ func getXRPC(t *testing.T, serverURL, path, token string) *http.Response { // TestUserBlockHandler_BlockUser tests the block user endpoint func TestUserBlockHandler_BlockUser(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) blockerDID := "did:plc:blocker123" @@ -271,6 +272,7 @@ func TestUserBlockHandler_BlockUser(t *testing.T) { // TestUserBlockHandler_BlockUser_MissingSubject tests blocking with missing subject func TestUserBlockHandler_BlockUser_MissingSubject(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) blockerDID := "did:plc:blocker789" @@ -302,6 +304,7 @@ func TestUserBlockHandler_BlockUser_MissingSubject(t *testing.T) { // TestUserBlockHandler_BlockUser_Unauthenticated tests blocking without auth func TestUserBlockHandler_BlockUser_Unauthenticated(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) // No token — unauthenticated request @@ -318,6 +321,7 @@ func TestUserBlockHandler_BlockUser_Unauthenticated(t *testing.T) { // TestUserBlockHandler_BlockUser_SelfBlock tests that self-blocking returns 400 func TestUserBlockHandler_BlockUser_SelfBlock(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) selfDID := "did:plc:selfblock" @@ -348,6 +352,7 @@ func TestUserBlockHandler_BlockUser_SelfBlock(t *testing.T) { // TestUserBlockHandler_UnblockUser tests the unblock user endpoint func TestUserBlockHandler_UnblockUser(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) blockerDID := "did:plc:unblocker1" @@ -442,6 +447,7 @@ func TestUserBlockHandler_UnblockUser(t *testing.T) { // TestUserBlockHandler_GetBlockedUsers tests listing blocked users func TestUserBlockHandler_GetBlockedUsers(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) blockerDID := "did:plc:lister1" @@ -519,6 +525,7 @@ func TestUserBlockHandler_GetBlockedUsers(t *testing.T) { // TestUserBlockHandler_GetBlockedUsers_Unauthenticated tests that getBlockedUsers requires auth func TestUserBlockHandler_GetBlockedUsers_Unauthenticated(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) // No token — unauthenticated request @@ -534,6 +541,7 @@ func TestUserBlockHandler_GetBlockedUsers_Unauthenticated(t *testing.T) { // TestUserBlockHandler_BlockUser_DuplicateConflict tests that blocking a user who is // already blocked on PDS returns the existing block (via repo lookup) or 409 Conflict. func TestUserBlockHandler_BlockUser_DuplicateConflict(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) blockerDID := "did:plc:conflict-blocker" @@ -602,6 +610,7 @@ func TestUserBlockHandler_BlockUser_DuplicateConflict(t *testing.T) { // TestUserBlockHandler_BlockUser_DuplicateConflict_NotIndexed tests the 409 path when // PDS returns conflict but the block hasn't been indexed in AppView yet. func TestUserBlockHandler_BlockUser_DuplicateConflict_NotIndexed(t *testing.T) { + t.Parallel() env := setupUserBlockTestServer(t) blockerDID := "did:plc:conflict-noindex-blocker" diff --git a/tests/integration/userblock_indexing_test.go b/tests/integration/userblock_indexing_test.go index 2896239..8bc4943 100644 --- a/tests/integration/userblock_indexing_test.go +++ b/tests/integration/userblock_indexing_test.go @@ -17,6 +17,7 @@ import ( // TestUserBlockIndexing_CreateEvent tests that a Jetstream CREATE event for // social.coves.actor.block is properly indexed in the AppView. func TestUserBlockIndexing_CreateEvent(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -86,6 +87,7 @@ func TestUserBlockIndexing_CreateEvent(t *testing.T) { // TestUserBlockIndexing_DeleteEvent tests that a Jetstream DELETE event // properly removes a previously indexed block from the AppView. func TestUserBlockIndexing_DeleteEvent(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -157,6 +159,7 @@ func TestUserBlockIndexing_DeleteEvent(t *testing.T) { // TestUserBlockIndexing_Idempotent tests that processing the same CREATE event // twice results in only 1 block (idempotent via ON CONFLICT DO UPDATE). func TestUserBlockIndexing_Idempotent(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -209,6 +212,7 @@ func TestUserBlockIndexing_Idempotent(t *testing.T) { // TestUserBlockIndexing_DeleteNonExistent tests that a DELETE event for a // non-existent block does not error (graceful/idempotent). func TestUserBlockIndexing_DeleteNonExistent(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) diff --git a/tests/integration/userblock_repo_test.go b/tests/integration/userblock_repo_test.go index 680842f..58fc006 100644 --- a/tests/integration/userblock_repo_test.go +++ b/tests/integration/userblock_repo_test.go @@ -15,6 +15,7 @@ import ( // TestUserBlockRepo_BlockUser tests creating user blocks func TestUserBlockRepo_BlockUser(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -118,6 +119,7 @@ func TestUserBlockRepo_BlockUser(t *testing.T) { // TestUserBlockRepo_UnblockUser tests removing user blocks func TestUserBlockRepo_UnblockUser(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -163,6 +165,7 @@ func TestUserBlockRepo_UnblockUser(t *testing.T) { // TestUserBlockRepo_GetBlock tests block retrieval by blocker + blocked DID func TestUserBlockRepo_GetBlock(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -211,6 +214,7 @@ func TestUserBlockRepo_GetBlock(t *testing.T) { // TestUserBlockRepo_GetBlockByURI tests block retrieval by record URI func TestUserBlockRepo_GetBlockByURI(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -256,6 +260,7 @@ func TestUserBlockRepo_GetBlockByURI(t *testing.T) { // TestUserBlockRepo_ListBlockedUsers tests listing blocked users with pagination func TestUserBlockRepo_ListBlockedUsers(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -351,6 +356,7 @@ func TestUserBlockRepo_ListBlockedUsers(t *testing.T) { // TestUserBlockRepo_IsBlocked tests the fast block check func TestUserBlockRepo_IsBlocked(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -426,6 +432,7 @@ func TestUserBlockRepo_IsBlocked(t *testing.T) { // TestUserBlockRepo_AreBlocked tests the batch block check func TestUserBlockRepo_AreBlocked(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) @@ -491,6 +498,7 @@ func TestUserBlockRepo_AreBlocked(t *testing.T) { // by record URI and then deleting it — the path used by the Jetstream consumer // when processing DELETE operations (which only carry the record URI, not DID pairs). func TestUserBlockRepo_UnblockByRecordURI(t *testing.T) { + t.Parallel() ctx := context.Background() db := testkit.DB(t) diff --git a/tests/integration/vote_e2e_test.go b/tests/integration/vote_e2e_test.go index f6192d5..3b24f67 100644 --- a/tests/integration/vote_e2e_test.go +++ b/tests/integration/vote_e2e_test.go @@ -2,6 +2,17 @@ package integration +// SERIAL BY DESIGN — do not add t.Parallel() to this file. +// +// Its tests drive the Jetstream firehose through the hand-rolled +// subscribeToJetstream* helpers below rather than testkit's cursor-gated +// subscriber. Those helpers subscribe to one shared stream and match on the +// first event of a collection, so a concurrent test writing the same +// collection is delivered to them too and either steals the match or trips +// their timeout. Per-test database clones do not isolate a shared websocket. +// +// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). + import ( "Coves/internal/api/routes" "Coves/internal/atproto/jetstream" @@ -24,6 +35,8 @@ import ( "github.com/go-chi/chi/v5" "github.com/gorilla/websocket" + + "github.com/stretchr/testify/require" ) // TestVoteE2E_CreateUpvote tests the full vote creation flow with a real local PDS @@ -39,9 +52,7 @@ func TestVoteE2E_CreateUpvote(t *testing.T) { } healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v", pdsURL, err) - } + require.NoError(t, err, "PDS health check at %s (TestMain's RequirePDS should have caught this)", pdsURL) func() { if closeErr := healthResp.Body.Close(); closeErr != nil { t.Logf("Failed to close health response: %v", closeErr) @@ -283,9 +294,7 @@ func TestVoteE2E_ToggleSameDirection(t *testing.T) { testUserPassword := "test-password-123" pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") testUser := createTestUser(t, db, testUserHandle, userDID) @@ -448,9 +457,7 @@ func TestVoteE2E_ToggleDifferentDirection(t *testing.T) { testUserPassword := "test-password-123" pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") testUser := createTestUser(t, db, testUserHandle, userDID) @@ -669,9 +676,7 @@ func TestVoteE2E_DeleteVote(t *testing.T) { testUserPassword := "test-password-123" pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") testUser := createTestUser(t, db, testUserHandle, userDID) @@ -852,9 +857,7 @@ func TestVoteE2E_JetstreamIndexing(t *testing.T) { testUserPassword := "test-password-123" accessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Skipf("PDS not available: %v", err) - } + require.NoError(t, err, "creating the test account on the PDS") testUser := createTestUser(t, db, testUserHandle, userDID) @@ -977,10 +980,10 @@ func subscribeToJetstreamForVote( } defer func() { _ = conn.Close() }() - // Track consecutive timeouts to detect stale connections - // gorilla/websocket panics after 1000 repeated reads on a failed connection - consecutiveTimeouts := 0 - const maxConsecutiveTimeouts = 10 + // ONE deadline for the whole subscription, not one per read: the + // budget is what the caller is willing to wait in total, and a + // per-read deadline would let a busy stream extend it indefinitely. + readDeadline := time.Now().Add(jetstreamReadBudget) // Read messages until we find our event or receive done signal for { @@ -991,7 +994,7 @@ func subscribeToJetstreamForVote( return ctx.Err() default: // Set read deadline to avoid blocking forever - if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + if err := conn.SetReadDeadline(readDeadline); err != nil { return fmt.Errorf("failed to set read deadline: %w", err) } @@ -1000,21 +1003,17 @@ func subscribeToJetstreamForVote( if err != nil { // Check if it's a timeout (expected) if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return nil + return fmt.Errorf("Jetstream closed the subscription before the event arrived") } if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - consecutiveTimeouts++ - if consecutiveTimeouts >= maxConsecutiveTimeouts { - return fmt.Errorf("connection appears stale after %d consecutive timeouts", consecutiveTimeouts) - } - continue // Timeout is expected, keep listening + // The deadline is the whole budget, so its expiry is the answer: + // no matching event arrived. Reading on would be reading a + // connection gorilla has already marked failed. + return fmt.Errorf("no matching event within %s", jetstreamReadBudget) } return fmt.Errorf("failed to read Jetstream message: %w", err) } - // Reset timeout counter on successful read - consecutiveTimeouts = 0 - // Check if this is the event we're looking for if event.Did == targetDID && event.Kind == "commit" && event.Commit.Collection == "social.coves.feed.vote" { // Process the event through the consumer diff --git a/tests/testkit/cmd/testdbprepare/main.go b/tests/testkit/cmd/testdbprepare/main.go index 8ec34fa..2c5cb51 100644 --- a/tests/testkit/cmd/testdbprepare/main.go +++ b/tests/testkit/cmd/testdbprepare/main.go @@ -1,6 +1,6 @@ // Command testdbprepare provisions the template database that testkit.DB -// clones, and sweeps clones left behind by processes that died before their -// cleanup ran. +// clones, and sweeps the clones and private templates left behind by processes +// that died before their cleanup ran. // // It exists as a Go program rather than as SQL in a shell script for one // reason: the CI runner image has no psql and no goose binary, and adding them @@ -28,18 +28,18 @@ func main() { force := flag.Bool("force", false, "rebuild the template even if its stamp already matches the migrations") sweepAge := flag.Duration("sweep-age", time.Hour, - "drop idle leftover clone databases older than this (0 disables the sweep)") + "drop idle leftover clone and private-template databases older than this (0 disables the sweep)") wait := flag.Duration("wait", 60*time.Second, "how long to wait for Postgres to accept connections before giving up") - printParallel := flag.Bool("print-parallel", false, - "print only the safe `go test -parallel` value for this server, and exit") + printFlags := flag.Bool("print-flags", false, + "print only the safe `go test` concurrency flags for this server, and exit") flag.Parse() ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - if *printParallel { - if err := reportParallelBudget(ctx, *wait); err != nil { + if *printFlags { + if err := reportConcurrencyBudget(ctx, *wait); err != nil { fmt.Fprintf(os.Stderr, "test-db-prepare: %v\n", err) os.Exit(1) } @@ -52,17 +52,17 @@ func main() { } } -// reportParallelBudget prints the -parallel value and nothing else, so a shell -// can capture it with $(...). -func reportParallelBudget(ctx context.Context, wait time.Duration) error { +// reportConcurrencyBudget prints the concurrency flags and nothing else, so a +// shell can splice them into a `go test` command line with $(...). +func reportConcurrencyBudget(ctx context.Context, wait time.Duration) error { if err := testkit.WaitForPostgres(ctx, wait); err != nil { return err } - budget, err := testkit.ParallelBudget(ctx) + budget, err := testkit.ConcurrencyBudget(ctx) if err != nil { return err } - fmt.Println(budget) + fmt.Println(budget.Flags()) return nil } @@ -81,15 +81,6 @@ func run(ctx context.Context, force bool, sweepAge, wait time.Duration) error { return err } - // The shared database the not-yet-migrated tests write to directly. Not - // testkit's concern, but this is the only step that runs before every test - // binary, so it is the only place the legacy path can be prepared once - // rather than by whichever package happens to run first. - if err := testkit.MigrateSharedDatabase(ctx); err != nil { - return err - } - fmt.Printf(" shared: %s migrated\n", pg.Redacted(pg.Database)) - // One call, one lock acquisition. Asking whether the template is current, // releasing the lock, and then acting on the answer would act on a fact // another process may have invalidated in between — and would need a @@ -110,10 +101,10 @@ func run(ctx context.Context, force bool, sweepAge, wait time.Duration) error { if sweepAge > 0 { result, err := testkit.SweepOrphanClones(ctx, sweepAge) if err != nil { - return fmt.Errorf("sweeping orphaned clones: %w", err) + return fmt.Errorf("sweeping orphaned testkit databases: %w", err) } if len(result.Dropped) > 0 { - fmt.Printf(" swept %d orphaned clone(s) older than %s\n", len(result.Dropped), sweepAge) + fmt.Printf(" swept %d orphaned database(s) older than %s\n", len(result.Dropped), sweepAge) for _, name := range result.Dropped { fmt.Printf(" - %s\n", name) } diff --git a/tests/testkit/db.go b/tests/testkit/db.go index 1d1a1c8..31d420c 100644 --- a/tests/testkit/db.go +++ b/tests/testkit/db.go @@ -12,6 +12,7 @@ import ( "io" "io/fs" "regexp" + "runtime" "slices" "strconv" "strings" @@ -70,6 +71,17 @@ const ( // an acceptable rule. ClonePrefix = "tkclone_" + // PrivateTemplatePrefix marks a throwaway template database created by + // testkit's own tests, which need to drop and rebuild a template without + // pulling the shared one out from under another test binary. + // + // It is a SEPARATE prefix rather than a clone name because the two are + // dropped under different rules — and it is a sweepable one because a + // killed run would otherwise leave a fully migrated database behind + // forever: nothing else names it, and the clone sweep would not look at it. + // It contains "test" so that validateTemplateName accepts it. + PrivateTemplatePrefix = "tktmpl_test_" + // Advisory lock coordinates. The class id spells "COVE" in ASCII; object // ids are allocated here as testkit grows more cluster-wide critical // sections. Both halves are needed because Postgres advisory locks are @@ -90,7 +102,7 @@ const ( // clonePoolMaxOpen bounds the connections a single test's pool may open. // // This is the connection budget from the spec: with a clone per test and - // tests running in parallel, pools multiply. ParallelBudget derives the + // tests running in parallel, pools multiply. ConcurrencyBudget derives the // safe -parallel value from this and the server's max_connections, and the // compose files raise max_connections to match. clonePoolMaxOpen = 3 @@ -115,6 +127,19 @@ const ( // issues checks its target against this first. var cloneNamePattern = regexp.MustCompile(`^` + ClonePrefix + `[a-z0-9_]+$`) +var privateTemplateNamePattern = regexp.MustCompile(`^` + PrivateTemplatePrefix + `[a-z0-9_]+$`) + +// sweepableFamilies are the database name families testkit creates and may drop +// unattended. Both share the layout newSweepableName writes, so both can be +// aged by name. +var sweepableFamilies = []struct { + prefix string + pattern *regexp.Regexp +}{ + {ClonePrefix, cloneNamePattern}, + {PrivateTemplatePrefix, privateTemplateNamePattern}, +} + // templateNamePattern constrains what may be used as a template database name. var templateNamePattern = regexp.MustCompile(`^[a-z][a-z0-9_]*$`) @@ -490,46 +515,6 @@ func EnsureTemplate(ctx context.Context) error { return nil } -// MigrateSharedDatabase brings the shared test database — the one named by -// POSTGRES_TEST_DB, which testkit otherwise uses only for administration — up -// to the current migration set. -// -// Nothing testkit owns needs this. It exists for the not-yet-migrated tests, -// which connect to that database directly and expect its tables to be there. -// Before this, the Makefile ran `goose up` against it and `make ci` relied on -// whichever test binary happened to call goose.Up first — an ordering -// coincidence that a -run filter or a new package would break, and whose -// failure mode is a wall of "relation does not exist". Doing it here means the -// one place that prepares databases prepares all of them, for every caller. -// -// Delete this along with tests/integration. -func MigrateSharedDatabase(ctx context.Context) error { - shared := Endpoints().Postgres.Database - - // Same exclusive lock the template provisioning uses. Two concurrent - // preparers running goose against one database is a deadlock or a partial - // migration, and the lock is already the thing that serialises them. - return withAdvisoryLock(ctx, false, func(ctx context.Context, _ *sql.Conn) error { - db, err := openDatabase(shared, 1) - if err != nil { - return err - } - defer func() { _ = db.Close() }() - - // goose.NewProvider, not the package-level goose.Up, for the reason in - // migrateAndStamp: the package API keeps its dialect and filesystem in - // globals that the legacy path also writes to. - provider, err := goose.NewProvider(goose.DialectPostgres, db, migrations.FS) - if err != nil { - return fmt.Errorf("configuring goose for %s: %w", Endpoints().Postgres.Redacted(shared), err) - } - if _, err := provider.Up(ctx); err != nil { - return fmt.Errorf("migrating %s: %w", Endpoints().Postgres.Redacted(shared), err) - } - return nil - }) -} - // ProvisionTemplate creates or rebuilds the template database and reports what // it did. With force, the template is rebuilt even if its stamp already matches. // @@ -726,65 +711,170 @@ func openDatabase(name string, maxOpen int) (*sql.DB, error) { // Connection budget // --------------------------------------------------------------------------- -// appviewConnectionHeadroom is the number of connections reserved for everything -// that is not a test clone: the AppView's own pool, the admin pool, psql -// sessions, Postgres' superuser reserve. -const appviewConnectionHeadroom = 40 +// connectionHeadroom is the number of connections the budget refuses to spend +// on tests: Postgres' superuser reserve, a developer's psql, a concurrent +// test-db-prepare, and slack for the burst between a clone's pool opening and +// the previous test's pool closing. +// +// It does NOT cover the AppView, which talks to a different server (coves_dev +// on :5435) in every configuration this repository ships. +const connectionHeadroom = 40 -// ParallelBudget returns the largest `go test -parallel` value the server's +// packageParallelism is the `go test -p` value the budget is divided across. +// +// -p and -parallel multiply: N test binaries each running M parallel tests hold +// N × M clone pools at once. The server's max_connections therefore buys a +// fixed number of concurrent test databases, and these two flags only decide +// how that total is split between the dimensions. +// +// It is 1, and the reason is NOT the one that used to be written on `-p 1` in +// the Makefile and the CI runner. That reason — tests/integration wiping shared +// tables — is gone: every test owns a private clone and packages cannot corrupt +// each other's data any more. +// +// The reason now is the ONE resource that stayed shared, and that no amount of +// database isolation can partition: the Jetstream firehose. Running two test +// binaries at once means tests/testkit's firehose tests subscribe while +// tests/integration is creating PDS accounts 25 at a time — and Jetstream's +// wantedCollections filter does not apply to account/identity events, so those +// tests get a stream that is almost entirely other tests' account churn and +// time out waiting for their own commit. Measured, not assumed: on a dev stack +// with an accumulated Jetstream store, the full tagged run failed 2 of 4 times +// at -p 2 and 0 of 4 times at -p 1, with the same -parallel. +// +// So the budget goes to within-package parallelism, which is where this tree's +// wall clock lives anyway — tests/integration alone is most of it. Raise this +// when Phase 4 of docs/TEST_ARCHITECTURE.md deletes tests/integration and its +// ten hand-rolled subscribers, leaving testkit the only firehose consumer. +const packageParallelism = 1 + +// maxParallelPerPackage keeps a very large max_connections from producing a +// -parallel value far past what the machine can usefully run. +const maxParallelPerPackage = 64 + +// nestedClonePools is the most testkit.DB clones a SINGLE test may hold open at +// the same time, and therefore the multiplier the budget has to apply to every +// parallel slot. +// +// One is the norm, but a test that calls testkit.DB and then calls it again in +// a subtest holds two pools at once — the outer clone outlives the inner one. +// TestPasswordSecurity in tests/integration does exactly that today. Budgeting +// one pool per slot made the model understate the true peak, so the formula +// could hand out a -parallel whose worst case sat above the ceiling it had just +// computed. +// +// Two costs nothing here, which is why it is the conservative choice rather +// than a tuned one: measured on this tree, -parallel 26 and -parallel 52 finish +// the tagged run within noise of each other (91-95s vs 89-94s), because the +// wall clock is dominated by the serial firehose block, not by how many clones +// run at once. Raise it only if some test starts holding three. +const nestedClonePools = 2 + +// A Budget is the pair of `go test` concurrency flags a server can support. +type Budget struct { + Packages int // -p: test binaries running at once + Parallel int // -parallel: t.Parallel() tests at once within each binary +} + +// Flags renders the budget as the flags themselves, so a caller can splice it +// into a command line without knowing which flag is which. +func (b Budget) Flags() string { + return fmt.Sprintf("-p %d -parallel %d", b.Packages, b.Parallel) +} + +// ConcurrencyBudget returns the largest `go test` concurrency the server's // max_connections can support, given that every parallel test holds its own -// clone pool. +// clone pool and every test binary holds an admin pool. // // Without this the connection budget is an unwritten assumption, and the way it // surfaces is a run that fails with "sorry, too many clients already" in // whichever test happened to be unlucky — a failure that reads like a bug in // that test. -func ParallelBudget(ctx context.Context) (int, error) { +func ConcurrencyBudget(ctx context.Context) (Budget, error) { db, err := adminDB() if err != nil { - return 0, err + return Budget{}, err } var raw string if err := db.QueryRowContext(ctx, "SHOW max_connections").Scan(&raw); err != nil { - return 0, fmt.Errorf("reading max_connections: %w", err) + return Budget{}, fmt.Errorf("reading max_connections: %w", err) } maxConns, err := strconv.Atoi(strings.TrimSpace(raw)) if err != nil { - return 0, fmt.Errorf("parsing max_connections %q: %w", raw, err) + return Budget{}, fmt.Errorf("parsing max_connections %q: %w", raw, err) + } + return budgetFor(maxConns) +} + +// budgetFor is the arithmetic on its own, so it can be tested at the sizes that +// matter — including the ones no development server is configured with. +func budgetFor(maxConns int) (Budget, error) { + // More test binaries than the machine has processors would divide the + // budget without buying any overlap, so GOMAXPROCS is the ceiling whatever + // packageParallelism asks for. + // + // CAVEAT, latent while packageParallelism is 1: this GOMAXPROCS is the + // PREPARE process's, and the flags it prints are consumed by a separate + // `go test` process. They agree today because both run in the same + // container with the same cgroup limits, and because a ceiling of 1 makes + // GOMAXPROCS irrelevant. The day packageParallelism rises, a prepare step + // running somewhere narrower than the test run (or wider) would size the + // budget for the wrong machine. Read it in the test process — or pass it in + // — before raising the constant. + packages := max(1, min(packageParallelism, runtime.GOMAXPROCS(0))) + available := maxConns - connectionHeadroom - packages*adminPoolMaxOpen + parallel := available / (packages * nestedClonePools * clonePoolMaxOpen) + + // No floor of 1 here. Clamping an impossible budget up to one would report + // a limit this server cannot honour and then fail later as "sorry, too many + // clients already" somewhere unrelated — the exact failure this function + // exists to prevent. A server too small to run one test says so. + if parallel < 1 { + return Budget{}, fmt.Errorf( + "max_connections=%d cannot support even one test: %d packages need %d admin connections "+ + "plus %d for a single test's clones, and %d are reserved as headroom; "+ + "raise max_connections (the compose files set 200)", + maxConns, packages, packages*adminPoolMaxOpen, + nestedClonePools*clonePoolMaxOpen, connectionHeadroom) } - budget := (maxConns - appviewConnectionHeadroom) / clonePoolMaxOpen - // Floor of 1 (never zero, which go test would reject) and a ceiling that - // keeps a huge max_connections from producing a -parallel value far past - // the machine's ability to run tests anyway. - return max(1, min(budget, 64)), nil + + return Budget{ + Packages: packages, + Parallel: min(parallel, maxParallelPerPackage), + }, nil } // --------------------------------------------------------------------------- // Per-test clones // --------------------------------------------------------------------------- -var cloneCounter = newCounter() +var sweepableCounter = newCounter() -// newCloneName builds a collision-free, sweepable clone name. +// newSweepableName builds a collision-free, sweepable database name. // -// Shape: tkclone___. +// Shape: __. // The run prefix is per-process random, so two concurrent `go test` processes // (or two developers against one Postgres) cannot collide. The timestamp is // there for the orphan sweep: pg_database records no creation time, so age // has to be written into the name. -func newCloneName() string { +func newSweepableName(prefix string) string { return fmt.Sprintf("%s%s_%s_%s", - ClonePrefix, + prefix, strconv.FormatInt(time.Now().Unix(), 36), RunPrefix(), - strconv.FormatUint(cloneCounter.next(), 36), + strconv.FormatUint(sweepableCounter.next(), 36), ) } +func newCloneName() string { return newSweepableName(ClonePrefix) } + // DB returns a private, fully migrated Postgres database for this test. // // The database is a clone of the migrated template, so it starts with the -// production schema and no rows. It is dropped when the test finishes; nothing +// production schema and exactly the rows the migrations seed — which is not +// none: 006 and 025 insert encryption keys, and code that decrypts community +// or aggregator credentials depends on them being there. "Empty" means no rows +// from any other test. It is dropped when the test finishes; nothing // the test writes is visible to any other test, and no cleanup code is needed // (or wanted — an explicit DELETE in a test body is a sign the test is still // assuming a shared database). @@ -939,8 +1029,14 @@ type SweepResult struct { Failed map[string]error } -// SweepOrphanClones drops testkit clone databases that are older than maxAge -// and have no sessions connected. +// SweepOrphanClones drops testkit-created databases that are older than maxAge +// and have no sessions connected: per-test clones AND the private templates +// testkit's own tests build. +// +// Both families are swept because both are created by a process that intends +// to drop them and may not survive to do it. A leaked private template is the +// worse leak of the two — it is a fully migrated database that nothing else +// names, so without this it would sit there until someone noticed by hand. // // Orphans come from processes that died between CREATE DATABASE and their // cleanup: a panicking test binary, a killed `go test`, a docker stop. @@ -963,55 +1059,62 @@ func SweepOrphanClones(ctx context.Context, maxAge time.Duration) (SweepResult, if err != nil { return result, err } + cutoff := time.Now().Add(-maxAge) + for _, family := range sweepableFamilies { + names, err := databaseNamesWithPrefix(ctx, db, family.prefix) + if err != nil { + return result, err + } + for _, name := range names { + created, ok := sweepableCreatedAt(family.prefix, family.pattern, name) + if !ok { + // A name that matches the prefix but not the layout is not + // something testkit created, so it is not something testkit + // drops. + continue + } + if created.After(cutoff) { + continue + } + + busy, err := databaseHasSessions(ctx, db, name) + if err != nil { + result.Failed[name] = err + continue + } + if busy { + result.Skipped = append(result.Skipped, name) + continue + } + + if err := dropDatabaseIfIdle(ctx, db, family.pattern, name); err != nil { + result.Failed[name] = err + continue + } + result.Dropped = append(result.Dropped, name) + } + } + return result, nil +} + +func databaseNamesWithPrefix(ctx context.Context, db *sql.DB, prefix string) ([]string, error) { rows, err := db.QueryContext(ctx, `SELECT datname FROM pg_database WHERE datname LIKE $1 ORDER BY datname`, - ClonePrefix+"%") + prefix+"%") if err != nil { - return result, fmt.Errorf("listing clone databases: %w", err) + return nil, fmt.Errorf("listing %s databases: %w", prefix, err) } defer func() { _ = rows.Close() }() - var candidates []string + var names []string for rows.Next() { var name string if err := rows.Scan(&name); err != nil { - return result, err + return nil, err } - candidates = append(candidates, name) - } - if err := rows.Err(); err != nil { - return result, err + names = append(names, name) } - - cutoff := time.Now().Add(-maxAge) - for _, name := range candidates { - created, ok := cloneCreatedAt(name) - if !ok { - // A name that matches the prefix but not the layout is not - // something testkit created, so it is not something testkit drops. - continue - } - if created.After(cutoff) { - continue - } - - busy, err := databaseHasSessions(ctx, db, name) - if err != nil { - result.Failed[name] = err - continue - } - if busy { - result.Skipped = append(result.Skipped, name) - continue - } - - if err := dropDatabaseIfIdle(ctx, db, name); err != nil { - result.Failed[name] = err - continue - } - result.Dropped = append(result.Dropped, name) - } - return result, nil + return names, rows.Err() } func databaseHasSessions(ctx context.Context, db *sql.DB, name string) (bool, error) { @@ -1025,9 +1128,9 @@ func databaseHasSessions(ctx context.Context, db *sql.DB, name string) (bool, er // dropDatabaseIfIdle drops a clone WITHOUT FORCE, so a session that connected // between the idle check and here makes the drop fail rather than be killed. -func dropDatabaseIfIdle(ctx context.Context, db *sql.DB, name string) error { - if !cloneNamePattern.MatchString(name) { - return fmt.Errorf("refusing to drop %q: not a testkit clone name", name) +func dropDatabaseIfIdle(ctx context.Context, db *sql.DB, pattern *regexp.Regexp, name string) error { + if !pattern.MatchString(name) { + return fmt.Errorf("refusing to drop %q: not a sweepable testkit database name", name) } dropCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() @@ -1038,12 +1141,13 @@ func dropDatabaseIfIdle(ctx context.Context, db *sql.DB, name string) error { return nil } -// cloneCreatedAt recovers the creation time newCloneName encoded into the name. -func cloneCreatedAt(name string) (time.Time, bool) { - if !cloneNamePattern.MatchString(name) { +// sweepableCreatedAt recovers the creation time newSweepableName encoded into +// the name. +func sweepableCreatedAt(prefix string, pattern *regexp.Regexp, name string) (time.Time, bool) { + if !pattern.MatchString(name) { return time.Time{}, false } - parts := strings.Split(strings.TrimPrefix(name, ClonePrefix), "_") + parts := strings.Split(strings.TrimPrefix(name, prefix), "_") if len(parts) != 3 { return time.Time{}, false } diff --git a/tests/testkit/db_test.go b/tests/testkit/db_test.go index d9fc171..9acbd47 100644 --- a/tests/testkit/db_test.go +++ b/tests/testkit/db_test.go @@ -156,6 +156,7 @@ func TestDB_CleanupIsNotBlockedByAQueryStillRunning(t *testing.T) { func TestDB_ProvisionsTheTemplateWhenItIsMissing(t *testing.T) { // The ad-hoc path: someone runs `go test ./internal/foo` without the // Makefile having prepared anything. testkit has to notice and provision. + usePrivateTemplate(t) dropTemplate(t) resetTemplateVerification() require.False(t, databaseExists(t, TemplateName())) @@ -168,6 +169,7 @@ func TestDB_ProvisionsTheTemplateWhenItIsMissing(t *testing.T) { } func TestDB_RebuildsTheTemplateWhenTheMigrationsChange(t *testing.T) { + usePrivateTemplate(t) require.NoError(t, EnsureTemplate(context.Background())) // Stand in for "a migration was added since this template was built", and @@ -198,6 +200,7 @@ func TestDB_RebuildsTheTemplateWhenTheMigrationsChange(t *testing.T) { } func TestTemplateStatus(t *testing.T) { + usePrivateTemplate(t) _, err := ProvisionTemplate(context.Background(), false) require.NoError(t, err) @@ -328,6 +331,17 @@ func TestSweepOrphanClones(t *testing.T) { createDatabase(t, orphan) t.Cleanup(func() { _ = dropDatabase(ctx, orphan) }) + // The same shape in the OTHER sweepable family: a private template left + // behind by a killed testkit run. This is the leak that matters most — + // nothing else names it, so if the sweep does not reap it, nothing ever + // will. + orphanTemplate := fmt.Sprintf("%s%s_%s_orphan", + PrivateTemplatePrefix, + strconv.FormatInt(time.Now().Add(-2*time.Hour).Unix(), 36), + RunPrefix()) + createDatabase(t, orphanTemplate) + t.Cleanup(func() { _ = dropSweepable(ctx, orphanTemplate) }) + // A clone from a run that is still going: same prefix, current timestamp. // Dropping this one out from under a live test is the failure the age // bound exists to prevent. @@ -335,13 +349,34 @@ func TestSweepOrphanClones(t *testing.T) { createDatabase(t, live) t.Cleanup(func() { _ = dropDatabase(ctx, live) }) + // And a private template from a run still going, for the same reason. + liveTemplate := newSweepableName(PrivateTemplatePrefix) + createDatabase(t, liveTemplate) + t.Cleanup(func() { _ = dropSweepable(ctx, liveTemplate) }) + result, err := SweepOrphanClones(ctx, time.Hour) require.NoError(t, err) assert.Contains(t, result.Dropped, orphan) + assert.Contains(t, result.Dropped, orphanTemplate, + "a leaked private template is a fully migrated database nothing else names") assert.NotContains(t, result.Dropped, live) + assert.NotContains(t, result.Dropped, liveTemplate) assert.False(t, databaseExists(t, orphan)) + assert.False(t, databaseExists(t, orphanTemplate)) assert.True(t, databaseExists(t, live), "a clone younger than the cutoff must survive") + assert.True(t, databaseExists(t, liveTemplate), "a private template younger than the cutoff must survive") +} + +// dropSweepable removes either sweepable family, for test cleanup only: +// dropDatabase deliberately refuses anything that is not a clone. +func dropSweepable(ctx context.Context, name string) error { + db, err := adminDB() + if err != nil { + return err + } + _, err = db.ExecContext(ctx, "DROP DATABASE IF EXISTS "+pq.QuoteIdentifier(name)+" WITH (FORCE)") + return err } func TestSweepOrphanClones_SkipsAnOldCloneThatIsStillInUse(t *testing.T) { @@ -433,14 +468,68 @@ func TestValidateTemplateName(t *testing.T) { }) } -func TestParallelBudget(t *testing.T) { - budget, err := ParallelBudget(context.Background()) +// The budget exists to keep `go test` inside max_connections, so that — not a +// range check on the numbers — is what this asserts. -p and -parallel multiply: +// overcommitting surfaces as "sorry, too many clients already" in whichever +// test happened to be unlucky, which reads like a bug in that test. +func TestConcurrencyBudget_PeakFitsInsideMaxConnections(t *testing.T) { + ctx := context.Background() + + budget, err := ConcurrencyBudget(ctx) require.NoError(t, err) + require.GreaterOrEqual(t, budget.Packages, 1, "go test rejects -p 0") + require.GreaterOrEqual(t, budget.Parallel, 1, "go test rejects -parallel 0") - // The compose files set max_connections=200, so the budget should be - // comfortably above one but never unbounded. - assert.GreaterOrEqual(t, budget, 1) - assert.LessOrEqual(t, budget, 64) + db, err := adminDB() + require.NoError(t, err) + var raw string + require.NoError(t, db.QueryRowContext(ctx, "SHOW max_connections").Scan(&raw)) + maxConns, err := strconv.Atoi(strings.TrimSpace(raw)) + require.NoError(t, err) + + // Every test binary holds an admin pool; every parallel test inside it + // holds up to nestedClonePools clone pools (a test that calls testkit.DB + // again inside a subtest holds two at once). + peak := budget.Packages * (adminPoolMaxOpen + budget.Parallel*nestedClonePools*clonePoolMaxOpen) + assert.LessOrEqual(t, peak, maxConns-connectionHeadroom, + "%s peaks at %d connections, which does not fit in max_connections=%d less %d of headroom", + budget.Flags(), peak, maxConns, connectionHeadroom) + + assert.Equal(t, + fmt.Sprintf("-p %d -parallel %d", budget.Packages, budget.Parallel), + budget.Flags(), + "Flags is spliced into a go test command line by scripts/test-db-prepare.sh") +} + +// A server too small for one test must SAY so. Clamping the budget up to a +// -parallel of 1 it cannot honour would move the failure to an unrelated test, +// as "sorry, too many clients already", which is what the budget exists to +// prevent. +func TestConcurrencyBudget_RefusesAServerTooSmallToRunOneTest(t *testing.T) { + // Just under what one package plus one test plus the headroom needs. + tooSmall := connectionHeadroom + adminPoolMaxOpen + nestedClonePools*clonePoolMaxOpen - 1 + + _, err := budgetFor(tooSmall) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot support even one test") + assert.Contains(t, err.Error(), "raise max_connections") + + // One more connection is exactly enough, and must not error. + budget, err := budgetFor(tooSmall + 1) + require.NoError(t, err) + assert.Equal(t, 1, budget.Parallel, "the smallest workable server gets the smallest workable budget") +} + +// The invariant across every plausible server size, not just this one: the +// budget's own worst case must fit in the connections it was derived from. +func TestConcurrencyBudget_NeverOvercommitsAtAnySize(t *testing.T) { + for _, maxConns := range []int{60, 100, 200, 500, 5000} { + budget, err := budgetFor(maxConns) + require.NoError(t, err, "max_connections=%d", maxConns) + peak := budget.Packages * (adminPoolMaxOpen + budget.Parallel*nestedClonePools*clonePoolMaxOpen) + assert.LessOrEqual(t, peak, maxConns-connectionHeadroom, + "at max_connections=%d, %s peaks at %d", maxConns, budget.Flags(), peak) + } } func TestDescribeLockHolders_NamesTheBlockingSession(t *testing.T) { @@ -496,19 +585,33 @@ func TestDropDatabase_RefusesAnythingButAClone(t *testing.T) { } } -func TestCloneCreatedAt(t *testing.T) { - name := newCloneName() - created, ok := cloneCreatedAt(name) - require.True(t, ok, "newCloneName must produce a sweepable name: %q", name) - assert.WithinDuration(t, time.Now(), created, 5*time.Second) +// Both sweepable families must be ageable by name, because that is the only +// creation time Postgres records for them — pg_database has no such column. +func TestSweepableCreatedAt(t *testing.T) { + for _, family := range sweepableFamilies { + name := newSweepableName(family.prefix) + created, ok := sweepableCreatedAt(family.prefix, family.pattern, name) + require.True(t, ok, "newSweepableName must produce a sweepable name: %q", name) + assert.WithinDuration(t, time.Now(), created, 5*time.Second) + + for _, bogus := range []string{ + "coves_dev", + family.prefix + "notatimestamp", // too few segments + family.prefix + "zzz_" + RunPrefix() + "_1_2", // too many segments + } { + _, ok := sweepableCreatedAt(family.prefix, family.pattern, bogus) + assert.False(t, ok, "%q must not be treated as sweepable", bogus) + } - for _, bogus := range []string{ - "coves_dev", - "tkclone_notatimestamp", // too few segments - "tkclone_zzz_" + RunPrefix() + "_1_2", // too many segments - } { - _, ok := cloneCreatedAt(bogus) - assert.False(t, ok, "%q must not be treated as a sweepable clone", bogus) + // A name from the OTHER family must never be aged by this one, or the + // sweep would drop a database under the wrong rules. + for _, other := range sweepableFamilies { + if other.prefix == family.prefix { + continue + } + _, ok := sweepableCreatedAt(family.prefix, family.pattern, newSweepableName(other.prefix)) + assert.False(t, ok, "%s names must not decode as %s names", other.prefix, family.prefix) + } } } @@ -581,7 +684,9 @@ func databaseExists(t *testing.T, name string) bool { func createDatabase(t *testing.T, name string) { t.Helper() - require.Regexp(t, cloneNamePattern, name, "tests only create clone-shaped databases") + require.True(t, + cloneNamePattern.MatchString(name) || privateTemplateNamePattern.MatchString(name), + "tests only create sweepable-shaped databases, got %q", name) db, err := adminDB() require.NoError(t, err) _, err = db.Exec("CREATE DATABASE " + pq.QuoteIdentifier(name)) diff --git a/tests/testkit/harness_support_test.go b/tests/testkit/harness_support_test.go index d0059a7..46a1296 100644 --- a/tests/testkit/harness_support_test.go +++ b/tests/testkit/harness_support_test.go @@ -1,12 +1,17 @@ package testkit import ( + "context" "database/sql" "fmt" "runtime" "strings" "sync" + "sync/atomic" "testing" + + "github.com/lib/pq" + "github.com/stretchr/testify/require" ) // Test support shared by every tier of testkit's own suite, and therefore @@ -161,3 +166,63 @@ func swapSingletons(endpoints func() EndpointSet, admin func() (*sql.DB, error)) templateMu.Unlock() } } + +// privateTemplateInUse guards the singleton rebinding below. It is a real +// concurrency check rather than a comment because the failure it prevents — +// two tests holding different template names at once — surfaces somewhere else +// entirely, as another test cloning a template that has just been dropped. +var privateTemplateInUse atomic.Bool + +// usePrivateTemplate points this test at a template database of its own, and +// drops it afterwards. +// +// Tests that drop or corrupt the template are proving how provisioning +// recovers, and the recovery window — between the drop and the rebuild — is +// precisely when another test binary's testkit.DB call finds nothing to clone +// and fails with `template database "coves_test_template" does not exist`. +// +// The advisory lock cannot close that window: during it the template is +// legitimately absent rather than half-written, so a clone that waits its turn +// still finds nothing. Under `go test -p 1` no other binary was running and +// pointing these tests at the shared template was safe by accident. Dropping +// -p 1 ends that, and this is the fix: the destructive tests exercise the same +// code paths against a template nobody else clones. +// +// The name carries PrivateTemplatePrefix and a timestamp, so a run killed +// before cleanup leaves something the orphan sweep will reap rather than a +// migrated database nothing names. +// +// The CALLER MUST NOT BE PARALLEL: this rebinds a process singleton. +func usePrivateTemplate(t *testing.T) { + t.Helper() + require.True(t, privateTemplateInUse.CompareAndSwap(false, true), + "usePrivateTemplate rebinds a process-wide singleton, so two tests cannot "+ + "hold one at the same time — is one of them calling t.Parallel()?") + + name := newSweepableName(PrivateTemplatePrefix) + require.NoError(t, validateTemplateName(name, Endpoints().Postgres.Database), + "the private template name must clear the same rail as a configured one") + + singletonMu.Lock() + saved := templateNameOnce + templateNameOnce = sync.OnceValues(func() (string, error) { return name, nil }) + singletonMu.Unlock() + resetTemplateVerification() + + t.Cleanup(func() { + // Under the exclusive lock, for the same reason dropTemplate takes it. + if err := withAdvisoryLock(context.Background(), false, + func(ctx context.Context, conn *sql.Conn) error { + _, err := conn.ExecContext(ctx, + "DROP DATABASE IF EXISTS "+pq.QuoteIdentifier(name)+" WITH (FORCE)") + return err + }); err != nil { + t.Errorf("leaked private template %q: %v", name, err) + } + singletonMu.Lock() + templateNameOnce = saved + singletonMu.Unlock() + resetTemplateVerification() + privateTemplateInUse.Store(false) + }) +} -- 2.51.2