From de2a4485a763b739ac328921f4b5fcb37c1d73e0 Mon Sep 17 00:00:00 2001 From: Bretton Date: Wed, 29 Jul 2026 04:35:09 -0700 Subject: [PATCH] =?UTF-8?q?test:=20mechanical=20tiers=20=E2=80=94=20build?= =?UTF-8?q?=20tags,=20tag-aware=20gate,=20honest=20Makefile=20surface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 task 6. Every test file now carries its tier as a build tag (integration: 76 files; e2e: 3; live unchanged; everything else T0), with two jetstream files split by function so each file is single-tier. All 161 testing.Short() guards are deleted — the tag decides inclusion. make test is an 11s no-Docker inner loop (untagged suite proven green under --network none); test-integration probes the published Postgres port and migrates the shared DB via an advisory-locked testkit step; test-all and four dead targets are deleted. ci-runner runs -tags integration then a serial (-parallel 1) -tags e2e pass into one JSON stream, vets all four tag sets, and now enforces exit/stream integrity: a crash-class go test exit (OOM 137, build 2) or a nonzero exit with a green ci-report fails the gate — closing a false-green hole as old as the harness. Two-stream reviewed (Codex + Opus); 8 fixes applied incl. DSN redaction in failure messages. make ci green: 3399/3399, 0 skips. Co-Authored-By: Claude Fable 5 --- .env.ci | 11 +- Makefile | 185 ++++++++---------- cmd/ci-report/main.go | 5 +- docs/COMMENT_SYSTEM_IMPLEMENTATION.md | 4 +- .../jetstream/bridged_stats_fixtures_test.go | 98 ++++++++++ .../atproto/jetstream/bridged_stats_test.go | 110 +++-------- .../jetstream/duplicate_delivery_test.go | 2 + .../atproto/jetstream/error_taxonomy_test.go | 50 ----- .../error_taxonomy_transient_test.go | 68 +++++++ .../atproto/jetstream/redrive_recency_test.go | 2 + internal/atproto/jetstream/rev_gate_test.go | 2 + .../atproto/jetstream/state_store_test.go | 17 +- internal/atproto/oauth/store_test.go | 2 + internal/core/users/service_test.go | 14 +- internal/core/users/turnstile_test.go | 27 ++- internal/db/postgres/user_repo_test.go | 2 + internal/db/postgres/vote_repo_test.go | 2 + loop_state.md | 16 +- scripts/ci-bootstrap.sh | 4 +- scripts/ci-runner.sh | 123 ++++++++++-- scripts/ci.sh | 14 +- tests/e2e/error_recovery_test.go | 6 +- tests/e2e/user_signup_test.go | 8 +- tests/e2e/user_signup_token_test.go | 7 +- tests/integration/aggregator_e2e_test.go | 2 + .../aggregator_registration_test.go | 38 +--- tests/integration/aggregator_test.go | 2 + .../author_avatar_hydration_test.go | 6 +- tests/integration/author_posts_e2e_test.go | 22 +-- tests/integration/blob_upload_e2e_test.go | 10 +- .../block_handle_resolution_test.go | 2 + tests/integration/bluesky_post_test.go | 6 +- tests/integration/comment_consumer_test.go | 2 + tests/integration/comment_e2e_test.go | 22 +-- tests/integration/comment_query_test.go | 2 + tests/integration/comment_vote_test.go | 2 + tests/integration/comment_write_test.go | 30 +-- .../integration/community_avatar_e2e_test.go | 14 +- tests/integration/community_blocking_test.go | 18 +- tests/integration/community_consumer_test.go | 2 + .../integration/community_credentials_test.go | 2 + tests/integration/community_e2e_test.go | 6 +- .../community_get_viewer_state_test.go | 2 + .../community_hostedby_security_test.go | 2 + .../community_identifier_resolution_test.go | 18 +- .../community_list_viewer_state_test.go | 2 + .../community_provisioning_test.go | 2 + tests/integration/community_repo_test.go | 2 + .../community_service_integration_test.go | 14 +- .../community_suggestion_e2e_test.go | 14 +- .../integration/community_update_e2e_test.go | 6 +- .../community_v2_validation_test.go | 2 + .../integration/concurrent_scenarios_test.go | 18 +- tests/integration/discover_test.go | 42 +--- tests/integration/feed_test.go | 50 +---- tests/integration/helpers.go | 2 + tests/integration/identity_resolution_test.go | 2 + tests/integration/image_proxy_e2e_test.go | 10 +- tests/integration/jetstream_consumer_test.go | 2 + tests/integration/oauth_e2e_test.go | 38 +--- tests/integration/oauth_helpers.go | 2 + .../oauth_session_fixation_test.go | 6 +- .../oauth_session_handle_sync_test.go | 6 +- .../oauth_token_verification_test.go | 6 +- tests/integration/post_consumer_test.go | 2 + tests/integration/post_creation_test.go | 10 +- tests/integration/post_delete_test.go | 14 +- tests/integration/post_e2e_test.go | 6 +- tests/integration/post_handler_test.go | 14 +- .../integration/post_thumb_validation_test.go | 10 +- tests/integration/post_unfurl_test.go | 18 +- .../integration/subscription_indexing_test.go | 14 +- tests/integration/timeline_test.go | 30 +-- tests/integration/token_refresh_test.go | 10 +- tests/integration/user_journey_e2e_test.go | 6 +- .../user_profile_avatar_e2e_test.go | 18 +- tests/integration/user_test.go | 4 +- tests/integration/userblock_e2e_test.go | 10 +- .../integration/userblock_enforcement_test.go | 22 +-- tests/integration/userblock_handler_test.go | 2 + tests/integration/userblock_indexing_test.go | 18 +- tests/integration/userblock_repo_test.go | 34 +--- tests/integration/vote_e2e_test.go | 24 +-- tests/lexicon_validation_test.go | 2 +- tests/live/bluesky_post_test.go | 20 -- tests/live/post_unfurl_test.go | 32 --- tests/testkit/cmd/testdbprepare/main.go | 9 + tests/testkit/db.go | 40 ++++ tests/testkit/db_test.go | 2 + tests/testkit/firehose_test.go | 2 + tests/testkit/harness_support_test.go | 163 +++++++++++++++ tests/testkit/harness_test.go | 161 +-------------- tests/testkit/pds_test.go | 2 + 93 files changed, 822 insertions(+), 1092 deletions(-) create mode 100644 internal/atproto/jetstream/bridged_stats_fixtures_test.go create mode 100644 internal/atproto/jetstream/error_taxonomy_transient_test.go create mode 100644 tests/testkit/harness_support_test.go diff --git a/.env.ci b/.env.ci index f7c5064..2cd4cd9 100644 --- a/.env.ci +++ b/.env.ci @@ -40,8 +40,8 @@ POSTGRES_TEST_PASSWORD=test_password POSTGRES_TEST_PORT=5434 # CI: set explicitly. The 21 read sites all fall back to this exact URL when -# unset — including during `make test-all`, which never defines it — so making -# it explicit removes a silent dependency on that fallback. +# unset — which is what happens on a developer's machine — so making it +# explicit here removes a silent dependency on that fallback. TEST_DATABASE_URL=postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable # ============================================================================= @@ -128,10 +128,9 @@ AUTH_SKIP_VERIFY=true # ============================================================================= # Logging # ============================================================================= -# CI: quiet by default so the go test -json stream stays readable, matching what -# `make test-all` does with LOG_ENABLED=false. docker-compose.ci.yml re-enables -# logging for the AppView container specifically, where the logs are the only -# way to diagnose a failing E2E test. +# CI: quiet by default so the go test -json stream stays readable. +# docker-compose.ci.yml re-enables logging for the AppView container +# specifically, where the logs are the only way to diagnose a failing E2E test. LOG_LEVEL=debug LOG_ENABLED=false diff --git a/Makefile b/Makefile index c363ecf..3500623 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help dev-up dev-up-otel dev-down dev-logs dev-status dev-reset test test-all test-db-prepare test-audit ci ci-clean e2e-test clean verify-stack create-test-account mobile-full-setup +.PHONY: help dev-up dev-up-otel dev-down dev-logs dev-status dev-reset test test-integration test-e2e test-live test-db-prepare test-audit ci ci-clean clean mobile-full-setup # Default target - show help .DEFAULT_GOAL := help @@ -21,7 +21,7 @@ help: ## Show this help message @echo "$(CYAN)Coves Development Commands$(RESET)" @echo "" @awk 'BEGIN {FS = ":.*##"; printf "Usage: make $(CYAN)$(RESET)\n"} \ - /^[a-zA-Z_-]+:.*?##/ { printf " $(CYAN)%-15s$(RESET) %s\n", $$1, $$2 } \ + /^[a-zA-Z0-9_-]+:.*?##/ { printf " $(CYAN)%-18s$(RESET) %s\n", $$1, $$2 } \ /^##@/ { printf "\n$(YELLOW)%s$(RESET)\n", substr($$0, 5) }' $(MAKEFILE_LIST) @echo "" @@ -120,61 +120,92 @@ db-reset: ## Reset database (delete all data and re-run migrations) ##@ Testing -test: ## Run fast unit/integration tests (skips slow E2E tests) +test: ## T0 unit tier - no Docker, no database, no network. The inner loop. + @# Untagged `go test` is the unit tier by construction: every test that + @# needs something out of process carries a build tag (integration/e2e/live) + @# and is therefore not in this build at all. That is what lets this target + @# start no containers and wait for nothing — and it is checked, not hoped + @# for: the tier is verified by running this selection with the network + @# switched off entirely. + @echo "$(GREEN)Running the unit tier (untagged)...$(RESET)" + @go test ./cmd/... ./internal/... ./tests/... + @echo "$(GREEN)✓ Unit tier complete$(RESET)" + +test-integration: ## T1 integration tier - needs Postgres; starts postgres-test itself @echo "$(GREEN)Starting test database...$(RESET)" - @docker-compose -f docker-compose.dev.yml --env-file .env.dev --profile test up -d postgres-test - @echo "Waiting for test database to be ready..." - @sleep 3 - @echo "$(GREEN)Running migrations on test database...$(RESET)" - @goose -dir internal/db/migrations postgres "postgresql://$(POSTGRES_TEST_USER):$(POSTGRES_TEST_PASSWORD)@localhost:$(POSTGRES_TEST_PORT)/$(POSTGRES_TEST_DB)?sslmode=disable" up || true - @# Provisions the template database that testkit.DB clones per test, and - @# sweeps clones orphaned by killed runs. Additive: the goose line above - @# still migrates the shared database the not-yet-migrated tests use. - @./scripts/test-db-prepare.sh - @echo "$(GREEN)Running fast tests (use 'make e2e-test' for E2E tests)...$(RESET)" - @# -p 1 runs packages sequentially: the integration suite's setup wipes + @# Best-effort, deliberately: `compose up` fails when the container already + @# exists under a different Compose project name, which is the normal case in + @# a git worktree (the project name comes from the directory, the container + @# name is pinned). An already-running database is success, not an error. + @# + @# This is not a swallowed failure, because the readiness poll below is the + @# actual assertion — if `up` failed AND no database is listening, that loop + @# exits non-zero with a message naming the fix. + @docker-compose -f docker-compose.dev.yml --env-file .env.dev --profile test up -d postgres-test 2>/dev/null \ + || echo "$(YELLOW) compose up declined (already running under another project?) - checking readiness anyway$(RESET)" + @# Diagnose the two failures that are NOT "the database is still starting", + @# because both otherwise surface as a readiness timeout that blames the + @# wrong thing. + @docker info >/dev/null 2>&1 || \ + (echo "$(RED)✗ Docker is not responding. Start Docker Desktop (or the daemon) and retry.$(RESET)" && exit 1) + @docker ps --filter name=^/coves-test-postgres$$ --filter status=running --format '{{.Names}}' \ + | grep -q coves-test-postgres || \ + (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. + @# + @# 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 + @# tests dial. An in-container `pg_isready` would prove only that Postgres is + @# up on its own loopback, so a wrong or already-claimed published port would + @# sail through the check and then fail as a wall of "connection refused". + @./scripts/test-db-prepare.sh || \ + (echo "$(RED)✗ Could not reach Postgres at localhost:$(POSTGRES_TEST_PORT) even though the container is running.$(RESET)" && \ + echo "$(RED) Check POSTGRES_TEST_PORT in .env.dev against the port the container actually publishes:$(RESET)" && \ + echo "$(RED) docker port coves-test-postgres$(RESET)" && exit 1) + @echo "$(GREEN)Running the integration tier (-tags integration)...$(RESET)" + @# 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). @# @# -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 -p 1 -parallel $$(./scripts/test-db-prepare.sh --print-parallel) \ - ./cmd/... ./internal/... ./tests/... -short -v - @echo "$(GREEN)✓ Tests complete$(RESET)" - -e2e-test: ## Run automated E2E tests (requires: make dev-up + make run in another terminal) - @echo "$(CYAN)========================================$(RESET)" - @echo "$(CYAN) E2E Test: Full User Signup Flow $(RESET)" - @echo "$(CYAN)========================================$(RESET)" - @echo "" - @echo "$(CYAN)Prerequisites:$(RESET)" - @echo " 1. Run 'make dev-up' (starts PDS + Jetstream)" - @echo " 2. Run 'make run' in another terminal (AppView must be running)" - @echo "" - @echo "$(GREEN)Running E2E tests...$(RESET)" - @go test ./tests/e2e -run TestE2E_UserSignup -v - @echo "" - @echo "$(GREEN)✓ E2E tests complete!$(RESET)" - -e2e-vote-test: ## Run vote E2E tests (requires: make dev-up) - @echo "$(CYAN)========================================$(RESET)" - @echo "$(CYAN) E2E Test: Vote System $(RESET)" - @echo "$(CYAN)========================================$(RESET)" - @echo "" - @echo "$(CYAN)Prerequisites:$(RESET)" - @echo " 1. Run 'make dev-up' (starts PDS + Jetstream + PostgreSQL)" - @echo " 2. Test database will be used (port 5434)" - @echo "" - @echo "$(GREEN)Running vote E2E tests...$(RESET)" - @echo "" - @echo "$(CYAN)Running simulated E2E test (fast)...$(RESET)" - @go test ./tests/integration -run TestVote_E2E_WithJetstream -v - @echo "" - @echo "$(CYAN)Running live PDS E2E test (requires PDS + Jetstream)...$(RESET)" - @go test ./tests/integration -run TestVote_E2E_LivePDS -v || echo "$(YELLOW)Live PDS test skipped (run 'make dev-up' first)$(RESET)" + @go test -tags integration -p 1 -parallel $$(./scripts/test-db-prepare.sh --print-parallel) \ + ./cmd/... ./internal/... ./tests/... + @echo "" + @echo "$(YELLOW)Note: the not-yet-migrated files under tests/integration also want$(RESET)" + @echo "$(YELLOW)a PDS and Jetstream, and they still SKIP themselves when those are$(RESET)" + @echo "$(YELLOW)missing — so a green run here does not mean the suite ran in full.$(RESET)" + @echo "$(YELLOW)Run 'make dev-up' first for the fuller local picture, and 'make ci'$(RESET)" + @echo "$(YELLOW)for the gate that refuses to count a skip as a pass.$(RESET)" + +test-e2e: ## T2 pipeline tier - needs the dev stack AND a running AppView + @# TRANSITIONAL. docs/TEST_ARCHITECTURE.md §3.5 puts this tier inside the + @# hermetic stack's network namespace via the compose runner, which is the + @# only way to reach a stack that publishes no host ports. Building that + @# runner path is task 10; until then this target grades whatever the dev + @# stack and `make run` happen to be serving, and asserts they are at least + @# reachable rather than letting the tests skip themselves. + @echo "$(CYAN)Checking the dev stack is reachable...$(RESET)" + @curl -sf http://127.0.0.1:3001/xrpc/_health >/dev/null 2>&1 || \ + (echo "$(RED) ✗ PDS not reachable on :3001. Run 'make dev-up'.$(RESET)" && exit 1) + @echo " $(GREEN)✓ PDS (:3001)$(RESET)" + @curl -sf http://127.0.0.1:8081/xrpc/_health >/dev/null 2>&1 || \ + (echo "$(RED) ✗ AppView not reachable on :8081. Run 'make run' in another terminal.$(RESET)" && exit 1) + @echo " $(GREEN)✓ AppView (:8081)$(RESET)" @echo "" - @echo "$(GREEN)✓ Vote E2E tests complete!$(RESET)" + @echo "$(GREEN)Running the pipeline tier (-tags e2e)...$(RESET)" + @# Serial, like the same tier in scripts/ci-runner.sh: the contracts share + @# one AppView, one PDS and one firehose cursor space (§3.4). + @go test -tags e2e -p 1 -parallel 1 -count=1 ./tests/e2e/... + @echo "$(GREEN)✓ Pipeline tier complete$(RESET)" test-db-reset: ## Reset test database @echo "$(GREEN)Resetting test database...$(RESET)" @@ -196,54 +227,6 @@ test-db-stop: ## Stop test database @docker-compose -f docker-compose.dev.yml --env-file .env.dev --profile test stop postgres-test @echo "$(GREEN)✓ Test database stopped$(RESET)" -test-all: ## Run ALL tests with live infrastructure (required before merge) - @echo "" - @echo "$(CYAN)═══════════════════════════════════════════════════════════════$(RESET)" - @echo "$(CYAN) FULL TEST SUITE - All tests with live infrastructure $(RESET)" - @echo "$(CYAN)═══════════════════════════════════════════════════════════════$(RESET)" - @echo "" - @echo "$(YELLOW)▶ Checking infrastructure...$(RESET)" - @echo "" - @# Check dev stack is running - @echo " Checking dev stack (PDS, Jetstream, PLC)..." - @docker-compose -f docker-compose.dev.yml --env-file .env.dev ps 2>/dev/null | grep -q "Up" || \ - (echo "$(RED) ✗ Dev stack not running. Run 'make dev-up' first.$(RESET)" && exit 1) - @echo " $(GREEN)✓ Dev stack is running$(RESET)" - @# Check AppView is running - @echo " Checking AppView (port 8081)..." - @curl -sf http://127.0.0.1:8081/xrpc/_health >/dev/null 2>&1 || \ - curl -sf http://127.0.0.1:8081/ >/dev/null 2>&1 || \ - (echo "$(RED) ✗ AppView not running. Run 'make run' in another terminal.$(RESET)" && exit 1) - @echo " $(GREEN)✓ AppView is running$(RESET)" - @# Check test database - @echo " Checking test database (port 5434)..." - @docker-compose -f docker-compose.dev.yml --env-file .env.dev ps postgres-test 2>/dev/null | grep -q "Up" || \ - (echo "$(YELLOW) ⚠ Test database not running, starting it...$(RESET)" && \ - docker-compose -f docker-compose.dev.yml --env-file .env.dev --profile test up -d postgres-test && \ - 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) - @echo " $(GREEN)✓ Test database is running$(RESET)" - @echo "" - @echo "$(GREEN)▶ [1/3] Unit & Package Tests (./cmd/... ./internal/...)$(RESET)" - @echo "$(CYAN)───────────────────────────────────────────────────────────────$(RESET)" - @LOG_ENABLED=false go test ./cmd/... ./internal/... -timeout 120s - @echo "" - @echo "$(GREEN)▶ [2/3] Integration Tests (./tests/integration/...)$(RESET)" - @echo "$(CYAN)───────────────────────────────────────────────────────────────$(RESET)" - @LOG_ENABLED=false go test ./tests/integration/... -timeout 600s - @echo "" - @echo "$(GREEN)▶ [3/3] E2E Tests (./tests/e2e/...)$(RESET)" - @echo "$(CYAN)───────────────────────────────────────────────────────────────$(RESET)" - @LOG_ENABLED=false go test ./tests/e2e/... -timeout 180s - @echo "" - @echo "$(GREEN)═══════════════════════════════════════════════════════════════$(RESET)" - @echo "$(GREEN) ✓ No test failures $(RESET)" - @echo "$(YELLOW) Note: this counts skips as passes. A test whose infrastructure$(RESET)" - @echo "$(YELLOW) is missing skips itself, so a partial stack still prints this.$(RESET)" - @echo "$(YELLOW) Run 'make ci' for the gate that enforces coverage.$(RESET)" - @echo "$(GREEN)═══════════════════════════════════════════════════════════════$(RESET)" - @echo "" - test-live: ## Run the opt-in tests that deliberately hit the public internet (NOT part of the merge gate) @echo "$(CYAN)═══════════════════════════════════════════════════════════════$(RESET)" @echo "$(CYAN) LIVE TIER - real Bluesky, real PLC, real third-party unfurls $(RESET)" @@ -354,13 +337,7 @@ mobile-reset: ## Remove all Android port forwarding @adb reverse --remove-all || echo "$(YELLOW)No device connected$(RESET)" @echo "$(GREEN)✓ Port forwarding removed$(RESET)" -verify-stack: ## Verify local development stack (PLC, PDS, configs) - @./scripts/verify-local-stack.sh - -create-test-account: ## Create a test account on local PDS for OAuth testing - @./scripts/create-test-account.sh - -mobile-full-setup: verify-stack create-test-account mobile-setup ## Full mobile setup: verify stack, create account, setup ports +mobile-full-setup: mobile-setup ## Full mobile setup: setup ports @echo "" @echo "$(GREEN)═══════════════════════════════════════════════════════════$(RESET)" @echo "$(GREEN) Mobile development environment ready! $(RESET)" diff --git a/cmd/ci-report/main.go b/cmd/ci-report/main.go index 1375786..e6c2fd8 100644 --- a/cmd/ci-report/main.go +++ b/cmd/ci-report/main.go @@ -15,8 +15,9 @@ // // Those guards are good developer ergonomics — running one package against a // partial stack should not drown you in failures. But they make a *gate* -// meaningless: stop the PDS and `make test-all` still prints "ALL TESTS PASSED", -// having silently skipped every real PDS write and every firehose round-trip. +// meaningless: stop the PDS and a plain `go test` still prints "ok" for every +// package, having silently skipped every real PDS write and every firehose +// round-trip. // // So this tool inverts the default. A skip is a failure unless it appears in an // allowlist committed to the repository with a stated reason. The allowlist diff --git a/docs/COMMENT_SYSTEM_IMPLEMENTATION.md b/docs/COMMENT_SYSTEM_IMPLEMENTATION.md index 1fe9344..18459a5 100644 --- a/docs/COMMENT_SYSTEM_IMPLEMENTATION.md +++ b/docs/COMMENT_SYSTEM_IMPLEMENTATION.md @@ -1412,7 +1412,7 @@ TEST_DATABASE_URL="postgres://test_user:test_password@localhost:5434/coves_test? **Unit Tests (Service Layer):** ```bash # Run all unit tests -go test -v ./internal/core/comments/... -short +go test -v ./internal/core/comments/... # Run with coverage report go test -cover ./internal/core/comments/... @@ -1437,7 +1437,7 @@ TEST_DATABASE_URL="postgres://test_user:test_password@localhost:5434/coves_test? -timeout 120s # Unit tests (no database) -go test -v ./internal/core/comments/... -short +go test -v ./internal/core/comments/... ``` ### Apply Migration diff --git a/internal/atproto/jetstream/bridged_stats_fixtures_test.go b/internal/atproto/jetstream/bridged_stats_fixtures_test.go new file mode 100644 index 0000000..808f002 --- /dev/null +++ b/internal/atproto/jetstream/bridged_stats_fixtures_test.go @@ -0,0 +1,98 @@ +package jetstream + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Record fixtures and the parser assertions for bridged vote stats. Building a +// record and parsing it needs nothing out of process, so this half of the +// bridged-stats coverage stays in the unit tier; the consumer behaviour those +// records drive lives in bridged_stats_test.go behind the integration tag. +// +// The shared identifiers live here rather than beside the database tests so a +// tagless build can still compile the fixtures. + +const ( + bridgedTestPrefix = "did:plc:brtest" + bridgedTestCommunity = "did:plc:brtestcommunity" + bridgedTestAuthor = "did:plc:brtestauthor" + bridgedTestOther = "did:plc:brtestotherauthor" + bridgedTestVoter = "did:plc:brtestvoter" + bridgedTestCommenter = bridgedTestPrefix + "commenter" + + // bridgedTestPDS is the trusted bridge PDS host used across these tests. Test + // users/communities are created with this pds_url and the consumers are constructed + // trusting it, so the provenance gate lets their bridgedStats through. Tests that + // exercise the default-deny path override the repo's pds_url instead. + bridgedTestPDS = "https://bridge.test" + bridgedTestNativePDS = "https://native.pds.test" + + asOfEarly = "2026-01-01T00:00:00Z" + asOfLate = "2026-06-01T00:00:00Z" +) + +func bridgedStatsRecord(up, down int, asOf string) map[string]interface{} { + return map[string]interface{}{ + "upvotes": up, + "downvotes": down, + "asOf": asOf, + } +} + +func postRecord(title, content string, bridged map[string]interface{}) map[string]interface{} { + rec := map[string]interface{}{ + "$type": "social.coves.community.post", + "community": bridgedTestCommunity, + "author": bridgedTestAuthor, + "title": title, + "content": content, + "createdAt": "2026-01-01T00:00:00Z", + } + if bridged != nil { + rec["bridgedStats"] = bridged + } + return rec +} + +func commentRecord(content, rootURI, rootCID, parentURI, parentCID string, bridged map[string]interface{}) map[string]interface{} { + rec := map[string]interface{}{ + "$type": "social.coves.community.comment", + "content": content, + "reply": map[string]interface{}{ + "root": map[string]interface{}{"uri": rootURI, "cid": rootCID}, + "parent": map[string]interface{}{"uri": parentURI, "cid": parentCID}, + }, + "createdAt": "2026-01-02T00:00:00Z", + } + if bridged != nil { + rec["bridgedStats"] = bridged + } + return rec +} + +// TestParseRecord_BridgedStats verifies the record parsers tolerate presence/absence +// of bridgedStats without a database. +func TestParseRecord_BridgedStats(t *testing.T) { + withStats, err := parsePostRecord(postRecord("t", "c", bridgedStatsRecord(7, 2, asOfEarly))) + require.NoError(t, err) + require.NotNil(t, withStats.BridgedStats) + assert.Equal(t, 7, withStats.BridgedStats.Upvotes) + assert.Equal(t, 2, withStats.BridgedStats.Downvotes) + assert.Equal(t, asOfEarly, withStats.BridgedStats.AsOf) + + without, err := parsePostRecord(postRecord("t", "c", nil)) + require.NoError(t, err) + assert.Nil(t, without.BridgedStats, "absent bridgedStats parses as nil") + + cWith, err := parseCommentRecord(commentRecord("hi", "at://x/c/1", "cid", "at://x/c/1", "cid", bridgedStatsRecord(3, 1, asOfLate))) + require.NoError(t, err) + require.NotNil(t, cWith.BridgedStats) + assert.Equal(t, 3, cWith.BridgedStats.Upvotes) + + cWithout, err := parseCommentRecord(commentRecord("hi", "at://x/c/1", "cid", "at://x/c/1", "cid", nil)) + require.NoError(t, err) + assert.Nil(t, cWithout.BridgedStats) +} diff --git a/internal/atproto/jetstream/bridged_stats_test.go b/internal/atproto/jetstream/bridged_stats_test.go index d7da06a..d95aa51 100644 --- a/internal/atproto/jetstream/bridged_stats_test.go +++ b/internal/atproto/jetstream/bridged_stats_test.go @@ -1,8 +1,11 @@ +//go:build integration + package jetstream import ( "context" "database/sql" + "net/url" "os" "testing" "time" @@ -22,31 +25,24 @@ import ( // use, port 5434 by default / TEST_DATABASE_URL). They are strictly local-only: no // public PLC/relay/PDS/image hosts are contacted. -const ( - bridgedTestPrefix = "did:plc:brtest" - bridgedTestCommunity = "did:plc:brtestcommunity" - bridgedTestAuthor = "did:plc:brtestauthor" - bridgedTestOther = "did:plc:brtestotherauthor" - bridgedTestVoter = "did:plc:brtestvoter" - bridgedTestCommenter = bridgedTestPrefix + "commenter" - - // bridgedTestPDS is the trusted bridge PDS host used across these tests. Test - // users/communities are created with this pds_url and the consumers are constructed - // trusting it, so the provenance gate lets their bridgedStats through. Tests that - // exercise the default-deny path override the repo's pds_url instead. - bridgedTestPDS = "https://bridge.test" - bridgedTestNativePDS = "https://native.pds.test" - - asOfEarly = "2026-01-01T00:00:00Z" - asOfLate = "2026-06-01T00:00:00Z" -) - // bridgeTrustForTests trusts only the bridge PDS host, so records from repos hosted // there may assert bridgedStats while every other repo is default-denied. 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() @@ -56,10 +52,15 @@ func setupBridgedTestDB(t *testing.T) *sql.DB { } db, err := sql.Open("postgres", dsn) require.NoError(t, err, "Failed to connect to test database") - if pingErr := db.Ping(); pingErr != nil { - _ = db.Close() - t.Skipf("test database not reachable (%v); start it with `make test-db-reset`", pingErr) - } + // 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 } @@ -122,14 +123,6 @@ func newVoteConsumer(db *sql.DB) *VoteEventConsumer { return NewVoteEventConsumer(postgres.NewVoteRepository(db), newMockUserService(), db) } -func bridgedStatsRecord(up, down int, asOf string) map[string]interface{} { - return map[string]interface{}{ - "upvotes": up, - "downvotes": down, - "asOf": asOf, - } -} - func postCommitEvent(op, rkey, cid string, record map[string]interface{}) *JetstreamEvent { return &JetstreamEvent{ Kind: "commit", @@ -144,21 +137,6 @@ func postCommitEvent(op, rkey, cid string, record map[string]interface{}) *Jetst } } -func postRecord(title, content string, bridged map[string]interface{}) map[string]interface{} { - rec := map[string]interface{}{ - "$type": "social.coves.community.post", - "community": bridgedTestCommunity, - "author": bridgedTestAuthor, - "title": title, - "content": content, - "createdAt": "2026-01-01T00:00:00Z", - } - if bridged != nil { - rec["bridgedStats"] = bridged - } - return rec -} - // readPostRow returns the stored native/bridged columns for assertions. func readPostRow(t *testing.T, db *sql.DB, uri string) (up, down, bridgedUp, bridgedDown, score int, asOf *time.Time, deletedAt *time.Time, title string) { t.Helper() @@ -444,22 +422,6 @@ func TestPostConsumer_InclusiveScore_NativeVotesStackOnBridged(t *testing.T) { // --- Comment consumer --- -func commentRecord(content, rootURI, rootCID, parentURI, parentCID string, bridged map[string]interface{}) map[string]interface{} { - rec := map[string]interface{}{ - "$type": "social.coves.community.comment", - "content": content, - "reply": map[string]interface{}{ - "root": map[string]interface{}{"uri": rootURI, "cid": rootCID}, - "parent": map[string]interface{}{"uri": parentURI, "cid": parentCID}, - }, - "createdAt": "2026-01-02T00:00:00Z", - } - if bridged != nil { - rec["bridgedStats"] = bridged - } - return rec -} - func commentCommitEvent(op, rkey, cid string, record map[string]interface{}) *JetstreamEvent { return &JetstreamEvent{ Kind: "commit", @@ -594,30 +556,6 @@ func TestCommentConsumer_Update_AsOfGuard_AndInclusiveScore(t *testing.T) { assert.Equal(t, 19, score5, "score = (1+20)-(0+2)") } -// TestParseRecord_BridgedStats verifies the record parsers tolerate presence/absence -// of bridgedStats without a database. -func TestParseRecord_BridgedStats(t *testing.T) { - withStats, err := parsePostRecord(postRecord("t", "c", bridgedStatsRecord(7, 2, asOfEarly))) - require.NoError(t, err) - require.NotNil(t, withStats.BridgedStats) - assert.Equal(t, 7, withStats.BridgedStats.Upvotes) - assert.Equal(t, 2, withStats.BridgedStats.Downvotes) - assert.Equal(t, asOfEarly, withStats.BridgedStats.AsOf) - - without, err := parsePostRecord(postRecord("t", "c", nil)) - require.NoError(t, err) - assert.Nil(t, without.BridgedStats, "absent bridgedStats parses as nil") - - cWith, err := parseCommentRecord(commentRecord("hi", "at://x/c/1", "cid", "at://x/c/1", "cid", bridgedStatsRecord(3, 1, asOfLate))) - require.NoError(t, err) - require.NotNil(t, cWith.BridgedStats) - assert.Equal(t, 3, cWith.BridgedStats.Upvotes) - - cWithout, err := parseCommentRecord(commentRecord("hi", "at://x/c/1", "cid", "at://x/c/1", "cid", nil)) - require.NoError(t, err) - assert.Nil(t, cWithout.BridgedStats) -} - // --- edited_at churn (fix 4) --- func TestPostConsumer_Update_StatsOnly_EditedAtUnchanged(t *testing.T) { diff --git a/internal/atproto/jetstream/duplicate_delivery_test.go b/internal/atproto/jetstream/duplicate_delivery_test.go index 400ddeb..f4dfcc6 100644 --- a/internal/atproto/jetstream/duplicate_delivery_test.go +++ b/internal/atproto/jetstream/duplicate_delivery_test.go @@ -1,3 +1,5 @@ +//go:build integration + package jetstream import ( diff --git a/internal/atproto/jetstream/error_taxonomy_test.go b/internal/atproto/jetstream/error_taxonomy_test.go index afb9308..103aea7 100644 --- a/internal/atproto/jetstream/error_taxonomy_test.go +++ b/internal/atproto/jetstream/error_taxonomy_test.go @@ -6,9 +6,7 @@ import ( "time" "Coves/internal/core/userblocks" - "Coves/internal/db/postgres" - _ "github.com/lib/pq" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -67,30 +65,6 @@ func TestPostConsumer_MissingRequiredField_IsPermanent(t *testing.T) { assert.ErrorIs(t, err, ErrPermanentEvent, "record missing required fields is permanently invalid") } -func TestPostConsumer_CommunityNotFound_IsTransient(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - - 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( - ghostCommunity, "social.coves.community.post", "create", "p1", - map[string]interface{}{ - "$type": "social.coves.community.post", - "community": ghostCommunity, - "author": "did:plc:someauthor", - "createdAt": "2026-01-01T00:00:00Z", - }, - )) - require.Error(t, err, "post for a not-yet-indexed community must fail") - assert.NotErrorIs(t, err, ErrPermanentEvent, - "community-not-found is an ORDERING failure and must stay transient so the redrive can succeed") - assert.Contains(t, err.Error(), "community not found") -} - func TestCommentConsumer_ValidationRejections_ArePermanent(t *testing.T) { // Validation runs before any repository/DB access. c := NewCommentEventConsumer(nil, nil) @@ -191,30 +165,6 @@ func TestCommunityConsumer_SubscriptionMissingSubject_IsPermanent(t *testing.T) assert.ErrorIs(t, err, ErrPermanentEvent, "subscription record without subject is permanently invalid") } -func TestCommunityConsumer_SubscriptionCommunityNotFound_IsTransient(t *testing.T) { - db := setupBridgedTestDB(t) - defer func() { _ = db.Close() }() - - 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( - subscriber, "social.coves.community.subscription", "create", "s1", - map[string]interface{}{ - "subject": ghostCommunity, - "createdAt": "2026-01-01T00:00:00Z", - }, - )) - require.Error(t, err, "subscription to a not-yet-indexed community must fail") - assert.NotErrorIs(t, err, ErrPermanentEvent, - "subscription community-not-found is an ORDERING failure and must stay transient so the redrive can succeed") -} - func TestAggregatorConsumer_ValidationRejections_ArePermanent(t *testing.T) { // Both checks fire before any repository access. c := NewAggregatorEventConsumer(nil) diff --git a/internal/atproto/jetstream/error_taxonomy_transient_test.go b/internal/atproto/jetstream/error_taxonomy_transient_test.go new file mode 100644 index 0000000..7d93e06 --- /dev/null +++ b/internal/atproto/jetstream/error_taxonomy_transient_test.go @@ -0,0 +1,68 @@ +//go:build integration + +package jetstream + +import ( + "context" + "testing" + + "Coves/internal/db/postgres" + + _ "github.com/lib/pq" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The transient half of the error taxonomy pinned in error_taxonomy_test.go. +// "Not found" here means "not indexed YET": proving these stay retryable +// requires a real database in which the dependency is genuinely absent, so +// they carry the integration tag while their permanent-rejection siblings — +// 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() }() + + 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( + ghostCommunity, "social.coves.community.post", "create", "p1", + map[string]interface{}{ + "$type": "social.coves.community.post", + "community": ghostCommunity, + "author": "did:plc:someauthor", + "createdAt": "2026-01-01T00:00:00Z", + }, + )) + require.Error(t, err, "post for a not-yet-indexed community must fail") + assert.NotErrorIs(t, err, ErrPermanentEvent, + "community-not-found is an ORDERING failure and must stay transient so the redrive can succeed") + assert.Contains(t, err.Error(), "community not found") +} + +func TestCommunityConsumer_SubscriptionCommunityNotFound_IsTransient(t *testing.T) { + db := setupBridgedTestDB(t) + defer func() { _ = db.Close() }() + + 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( + subscriber, "social.coves.community.subscription", "create", "s1", + map[string]interface{}{ + "subject": ghostCommunity, + "createdAt": "2026-01-01T00:00:00Z", + }, + )) + require.Error(t, err, "subscription to a not-yet-indexed community must fail") + assert.NotErrorIs(t, err, ErrPermanentEvent, + "subscription community-not-found is an ORDERING failure and must stay transient so the redrive can succeed") +} diff --git a/internal/atproto/jetstream/redrive_recency_test.go b/internal/atproto/jetstream/redrive_recency_test.go index e005189..8421a48 100644 --- a/internal/atproto/jetstream/redrive_recency_test.go +++ b/internal/atproto/jetstream/redrive_recency_test.go @@ -1,3 +1,5 @@ +//go:build integration + package jetstream import ( diff --git a/internal/atproto/jetstream/rev_gate_test.go b/internal/atproto/jetstream/rev_gate_test.go index 4aaad08..17ed3a9 100644 --- a/internal/atproto/jetstream/rev_gate_test.go +++ b/internal/atproto/jetstream/rev_gate_test.go @@ -1,3 +1,5 @@ +//go:build integration + package jetstream import ( diff --git a/internal/atproto/jetstream/state_store_test.go b/internal/atproto/jetstream/state_store_test.go index 948edf3..1bc41e8 100644 --- a/internal/atproto/jetstream/state_store_test.go +++ b/internal/atproto/jetstream/state_store_test.go @@ -1,3 +1,5 @@ +//go:build integration + package jetstream import ( @@ -24,17 +26,20 @@ func setupStateStoreTestDB(t *testing.T) *sql.DB { } db, err := sql.Open("postgres", dsn) require.NoError(t, err, "Failed to connect to test database") - if pingErr := db.Ping(); pingErr != nil { - _ = db.Close() - t.Skipf("test database not reachable (%v); start it with `make test-db-reset`", pingErr) - } - require.NoError(t, goose.Up(db, "../../db/migrations"), "Failed to run migrations") - + // 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 } diff --git a/internal/atproto/oauth/store_test.go b/internal/atproto/oauth/store_test.go index f3ff432..c2aa153 100644 --- a/internal/atproto/oauth/store_test.go +++ b/internal/atproto/oauth/store_test.go @@ -1,3 +1,5 @@ +//go:build integration + package oauth import ( diff --git a/internal/core/users/service_test.go b/internal/core/users/service_test.go index 3e9a506..f80dcf3 100644 --- a/internal/core/users/service_test.go +++ b/internal/core/users/service_test.go @@ -1057,14 +1057,24 @@ func TestRequestSignupToken_PDSAdminReturnsEmptyCode(t *testing.T) { assert.Contains(t, mintErr.Body(), "empty code") } +// withPDSAdminClient replaces the client the service uses for PDS admin calls. +// Unexported and test-only: it exists so the transport-failure path can be +// exercised deterministically, not as a production knob. +func withPDSAdminClient(c *http.Client) UserServiceOption { + return func(s *userService) { s.pdsAdminClient = c } +} + // Transport failure (PDS unreachable) must wrap ErrPDSAdminUnavailable so the // handler maps to 503, not a bare 500. func TestRequestSignupToken_PDSAdminTransportFailure(t *testing.T) { mockRepo := new(MockUserRepository) turnstile := &mockTurnstile{} - // 127.0.0.1:0 → guaranteed unreachable. http.Client.Do returns a transport error. - service := NewUserService(mockRepo, nil, "http://127.0.0.1:0", turnstile, "admin-pw") + // The admin call fails in the transport, without a socket: see + // failingTransport in turnstile_test.go for why this is injected rather than + // dialled at an unreachable address. + service := NewUserService(mockRepo, nil, "http://pds.invalid", turnstile, "admin-pw", + withPDSAdminClient(unreachableClient())) _, err := service.RequestSignupToken(context.Background(), RequestSignupTokenRequest{ TurnstileToken: "tok", diff --git a/internal/core/users/turnstile_test.go b/internal/core/users/turnstile_test.go index 77e8126..b784296 100644 --- a/internal/core/users/turnstile_test.go +++ b/internal/core/users/turnstile_test.go @@ -17,6 +17,25 @@ import ( "github.com/stretchr/testify/require" ) +// failingTransport fails every request with a deterministic transport error, +// standing in for an unreachable siteverify host. +// +// The alternative — pointing the client at 127.0.0.1:0 and letting the dial +// fail — makes a unit test's result depend on the machine's network stack, and +// costs whatever the client's timeout is. This costs nothing and cannot be +// affected by the sandbox the test happens to run in. +type failingTransport struct{ err error } + +func (f failingTransport) RoundTrip(*http.Request) (*http.Response, error) { + return nil, f.err +} + +// unreachableClient returns an http.Client whose every request fails in the +// transport, without opening a socket. +func unreachableClient() *http.Client { + return &http.Client{Transport: failingTransport{err: errors.New("dial tcp: simulated connection refused")}} +} + func newTestTurnstile(t *testing.T, handler http.HandlerFunc) (*cloudflareTurnstile, *httptest.Server) { t.Helper() server := httptest.NewServer(handler) @@ -140,8 +159,8 @@ func TestTurnstile_Verify_DecodeErrorIsUnavailable(t *testing.T) { func TestTurnstile_Verify_UnreachableIsUnavailable(t *testing.T) { v := &cloudflareTurnstile{ secret: "test", - siteverifyURL: "http://127.0.0.1:0", // guaranteed unreachable - httpClient: &http.Client{Timeout: 500 * time.Millisecond}, + siteverifyURL: "http://siteverify.invalid", + httpClient: unreachableClient(), } err := v.Verify(context.Background(), "tok", "") @@ -316,8 +335,8 @@ func TestTurnstile_Verify_TransportFailureLogsClientIPNotToken(t *testing.T) { v := &cloudflareTurnstile{ secret: "s", - siteverifyURL: "http://127.0.0.1:0", - httpClient: &http.Client{Timeout: 200 * time.Millisecond}, + siteverifyURL: "http://siteverify.invalid", + httpClient: unreachableClient(), } _ = v.Verify(context.Background(), tokenSentinel, ipSentinel) diff --git a/internal/db/postgres/user_repo_test.go b/internal/db/postgres/user_repo_test.go index 4bf4dad..8d88fc5 100644 --- a/internal/db/postgres/user_repo_test.go +++ b/internal/db/postgres/user_repo_test.go @@ -1,3 +1,5 @@ +//go:build integration + package postgres import ( diff --git a/internal/db/postgres/vote_repo_test.go b/internal/db/postgres/vote_repo_test.go index 61491da..6a00518 100644 --- a/internal/db/postgres/vote_repo_test.go +++ b/internal/db/postgres/vote_repo_test.go @@ -1,3 +1,5 @@ +//go:build integration + package postgres import ( diff --git a/loop_state.md b/loop_state.md index 9621ed5..fdb374e 100644 --- a/loop_state.md +++ b/loop_state.md @@ -42,7 +42,7 @@ Stop the loop when every task is done, or on any blocked task. | 3 | testkit core: db.go (template-clone, advisory lock), wait.go (WaitFor/Holds, terminal errs), fixtures.go (UniqueID run-prefix), scripts/test-db-prepare.sh, scripts/test-audit.sh (warn mode) + testkit's own tests | 1 | S | done | (see git log) | 47 testkit tests, -race -shuffle clean; clone ~30ms (cheaper than spec est). Review: Codex needs-work (5 high) + Opus not-safe-as-is (2 high) → 15-item batch applied: template-drop name rail (found a panic-in-error-path bug), drop-before-close teardown, 55006 retry, sweep de-FORCEd + error-accumulating, wait-primitive deadline contract, max_connections=200 + ParallelBudget (CI: -parallel 53, inert until ph.3), bounded lock wait w/ holder diagnostics, grep -H audit fix. ADJUDICATION: Codex RIGHT / Opus WRONG on advisory-lock leak (async cancel can grant lock after Go sees error; fixed via Conn.Raw(ErrBadConn) eviction). make ci GREEN 3347 tests / 14 allowlisted skips | | 4 | testkit pds.go (absorb 4 factories, createPDSAccount×2, XRPC clients), firehose.go (generic cursor-gated), appview.go; fix 5 handle-collision sites | 1 ⛩ | S | done | (see git log) | PHASE 1 COMPLETE. Worker found ALL 10 legacy subscribeToJetstream copies broken (gorilla corrupt-after-deadline → every "30s wait" gave up at ~5s — explains historical flakes). 5 factories not 4 (comments missed by helpers.go); old generateTID never emitted valid TIDs (now wraps indigo TIDClock); dep rule is TRANSITIVE (atproto/pds imports core/blobs → reimplemented). FULL PANEL (Codex+CR+SFH+TA+security): security CLEAN; ~28-item batch applied — same-time_us dedupe set, deadline-bounded dials, all-read-errors-recover, discard counting (clock-skew diagnosis), overflow=failure, lock-free blocking I/O, Event.Raw()/Into() (unblocks consumer migrations), XRPC-shaped 404 classification, PendingIfUnavailable, ConsumerHealth + WithConsumerHealth, option-pattern unification, testkit.Main(m, Require*...). CR false-positive on ParallelBudget wiring (discarded). testkit 117 tests -race -shuffle green; make ci GREEN 3429/14 | | 5 | Kill the lies: delete 6 debt tests; lexicon validator stops generating defs-only subtests (retire 8 allowlist entries); move 2 ratelimit files to internal/api/middleware (T0); fold tests/unit into internal/core/communities | 2 | M | done | (see git log) | ALLOWLIST → 0 ENTRIES; make ci GREEN 3399/3399, 0 skips. Lexicon fix is a coverage GAIN (43 defs-only fragment resolutions previously asserted nothing + two-way naming consistency). tests/unit was 100% FAKE (servers never dialed, literals asserted against themselves, t.Log theater) — deleted wholesale, nothing to port; communities now at honest zero (task 17). 3 tautology "tests" deleted rather than moved. Audit 911→897 | -| 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 | pending | | identity_resolution, bluesky_post (what's left post-task-2), post_unfurl | +| 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 | pending | | mechanical; gates are the reviewer | | 8 | Migrate remaining call sites; delete all 3 setupTestDB defs + per-file cleanup fns | 3 | M | pending | | | | 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 | @@ -159,3 +159,17 @@ Stop the loop when every task is done, or on any blocked task. aggregator_e2e_test.go Part 4 (the deleted tautologies never were). allowed_skips.txt is deliberately empty with the rationale in its header — new entries must argue for themselves. +- **From task 6 (MIGRATION RULE for tasks 7-8)**: a package's TestMain sets + the floor for its whole test binary — mixed-tier packages keep the + infra-gated TestMain in a TAGGED file and shared test support (fakes, + helpers) in an untagged sibling; testkit is the worked example + (harness_test.go tagged / harness_support_test.go untagged). Tag sets are + additive: tagged halves may use untagged helpers, never the reverse. + Fully-tagged directories are silently skipped by ./... (not an error). + ci-runner now enforces exit/stream integrity: go test status >1 fails the + gate regardless of ci-report, nonzero-status+ok-report = truncated + capture = fail. T0 is socket-free (failingTransport pattern for + transport-error paths; withPDSAdminClient test-only option in users). + make test-integration's readiness gate is test-db-prepare's real + connection to the published port — compose-up conflicts are tolerated by + design because the connection probe is the decider. diff --git a/scripts/ci-bootstrap.sh b/scripts/ci-bootstrap.sh index fb65475..37fb237 100755 --- a/scripts/ci-bootstrap.sh +++ b/scripts/ci-bootstrap.sh @@ -6,8 +6,8 @@ # The AppView writes community records to the PDS as PDS_INSTANCE_HANDLE (see # PDSConfig.HasInstanceCredentials in internal/config). The dev stack's PDS # volume persists, so that account was created once — by hand, or by a -# scripts/create-test-account.sh that is referenced by the Makefile but is no -# longer in the tree — and has been there ever since. +# scripts/create-test-account.sh that no longer exists in the tree — and has +# been there ever since. # # CI starts from an empty PDS on every run, so the account has to be created # each time, and it has to exist before the AppView boots. That ordering is why diff --git a/scripts/ci-runner.sh b/scripts/ci-runner.sh index 0d45a21..4e24589 100755 --- a/scripts/ci-runner.sh +++ b/scripts/ci-runner.sh @@ -61,17 +61,26 @@ wait_for "appview (:8081)" "http://localhost:8081/xrpc/_health" 90 echo # --------------------------------------------------------------------------- -# 1b. Type-check the live tier +# 1b. Type-check every tag set # --------------------------------------------------------------------------- -# tests/live is excluded from every build the gate runs, so nothing here would -# ever notice it stopped compiling — a rename in internal/ would break it -# silently and the breakage would surface weeks later, to whoever next ran -# `make test-live`. Vet type-checks it without executing anything, so it needs -# no network: the live tier stays buildable on the merge path even though it -# never runs there. -echo "▶ Type-checking the live tier (-tags live, not executed)..." +# Build tags make tiers invisible to builds that did not ask for them, which +# also makes their rot invisible: a rename in internal/ can break a tier for +# weeks, until whoever next runs that tier discovers it. Vet type-checks each +# selection without executing anything, so none of this needs infrastructure. +# +# The untagged and integration passes are belt-and-braces — the suite below +# compiles both anyway — but they fail here with a clear message instead of +# inside a 20-minute test run. The live pass is the load-bearing one: tests/live +# never executes on the merge path at all. +echo "▶ Type-checking every tag set (nothing is executed)..." +go vet ./... +echo " ✓ untagged (unit tier)" +go vet -tags integration ./... +echo " ✓ -tags integration" +go vet -tags e2e ./tests/e2e/... +echo " ✓ -tags e2e" go vet -tags live ./tests/live/... -echo " ✓ tests/live compiles" +echo " ✓ -tags live" echo # --------------------------------------------------------------------------- @@ -109,11 +118,21 @@ echo # 2. Run the suite # --------------------------------------------------------------------------- -# -p 1 serialises packages. The integration suite's setup issues unscoped +# The gate runs two selections, because tiers are build tags and a tag set is a +# compilation, not a filter: +# +# -tags integration ./cmd/... ./internal/... ./tests/... T0 + T1 +# -tags e2e ./tests/e2e/... T2 +# +# Tags are additive, so the first selection compiles the untagged unit files in +# alongside the integration ones and covers both tiers in a single pass. T2 is a +# separate compilation because `e2e` and `integration` are disjoint sets, and it +# 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 — the same reason `make test` -# already passes -p 1. `make test-all` does not pass it for ./cmd/... and -# ./internal/..., which is a latent race there. +# 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 @@ -149,10 +168,30 @@ progress_pid=$! # cannot leave a stray tail holding the container open. trap 'kill "$progress_pid" 2>/dev/null || true' EXIT +# Both runs APPEND to the same raw stream, and neither is piped, for the reason +# above: ci-report consumes one -json stream covering every tier, and a +# downstream reader must never be able to affect whether a run completes. +# +# Their exit statuses are KEPT, not discarded. ci-report is the verdict on what +# the stream says, but it can only judge events that reached the file: if a run +# is killed (OOM, SIGKILL, a panic that takes the harness down) the stream is +# truncated *without* fail events, and a report built from the surviving prefix +# 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 +# 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. set +e -go test -json -p 1 -parallel "$TEST_PARALLEL" -count=1 -timeout "$TEST_TIMEOUT" \ +go test -json -tags integration -p 1 -parallel "$TEST_PARALLEL" -count=1 -timeout "$TEST_TIMEOUT" \ ./cmd/... ./internal/... ./tests/... \ >>"$RAW" 2>&1 +integration_status=$? +go test -json -tags e2e -p 1 -parallel 1 -count=1 -timeout "$TEST_TIMEOUT" \ + ./tests/e2e/... \ + >>"$RAW" 2>&1 +e2e_status=$? set -e # Let the reader drain the tail of the file before its output is interleaved @@ -165,15 +204,63 @@ wait "$progress_pid" 2>/dev/null || true # 3. Judge # --------------------------------------------------------------------------- -# go test's own exit code is deliberately ignored: it reports a skipped suite as -# success, which is the whole reason ci-report exists. ci-report reads the same -# stream and applies the stricter rules. +# ci-report is the primary verdict. `go test`'s exit code alone is not a gate: +# it reports a suite that skipped itself into nothing as success, which is the +# whole reason this tool exists. ci-report reads the stream and applies the +# stricter rules. # # Built rather than `go run`, so the exit code is ci-report's own and the # toolchain does not print its own "exit status 1" line over the report. go build -o /tmp/ci-report ./cmd/ci-report -exec /tmp/ci-report \ +set +e +/tmp/ci-report \ -allowlist "$ALLOWLIST" \ -summary "$SUMMARY" \ -allow-stale "${COVES_CI_ALLOW_STALE:-false}" \ <"$RAW" +report_status=$? +set -e + +# --------------------------------------------------------------------------- +# 3b. Cross-check the report against the runs that produced it +# --------------------------------------------------------------------------- +# +# ci-report can only judge what reached the stream, so it is blind to a run that +# died without writing failure events. Two rules close that hole, and both are +# about the STREAM's integrity rather than about any individual test: +# +# * status > 1 is not a test failure. `go test` exits 1 when tests fail and 2 +# when it could not run them (bad flags, a package that would not build); +# a signal death (OOM killer, SIGKILL) surfaces as 128+signo. None of those +# are guaranteed to leave fail events behind, so they fail the gate outright +# regardless of what the report says. +# +# * status != 0 while ci-report says ok is a contradiction. `go test` saw +# something wrong that the stream does not contain — the definition of a +# truncated capture. Trusting the report here is exactly the false-green +# this check exists to prevent. +# +# An ordinary failing test satisfies neither rule (status 1, report not ok), so +# it is still reported by ci-report in ci-report's own words. +gate_status=$report_status + +for tier_status in "integration:$integration_status" "e2e:$e2e_status"; do + tier=${tier_status%%:*} + status=${tier_status##*:} + + if [ "$status" -gt 1 ]; then + echo + echo "✗ the $tier run exited $status — that is a crashed or unrunnable" + echo " suite, not a test failure, and its -json stream may be truncated." + echo " Failing the gate on the exit status rather than on the report." + gate_status=1 + elif [ "$status" -ne 0 ] && [ "$report_status" -eq 0 ]; then + echo + echo "✗ the $tier run exited $status but the report says every test passed." + echo " go test saw a failure that never reached $RAW, so the captured" + echo " stream is incomplete and the report cannot be trusted." + gate_status=1 + fi +done + +exit "$gate_status" diff --git a/scripts/ci.sh b/scripts/ci.sh index 00a7a93..3856752 100755 --- a/scripts/ci.sh +++ b/scripts/ci.sh @@ -1,17 +1,15 @@ #!/usr/bin/env bash # The hermetic merge gate. Driven by `make ci`. # -# WHAT THIS DOES THAT `make test-all` DOES NOT +# WHAT THIS DOES THAT THE PER-TIER TARGETS DO NOT # # * Creates its own infrastructure instead of asserting that a human already -# started it. test-all checks `docker-compose ps | grep -q "Up"` — which -# passes if *any one* service is up — and then exits telling you to run -# `make dev-up`. +# started it. `make test-integration` and `make test-e2e` grade whatever the +# dev stack happens to be serving; this builds the stack it grades against. # -# * Tests a binary built from the current working tree. test-all tests -# whatever long-running `make run` started in another terminal, which may be -# many edits stale. A gate that grades the wrong binary is worse than no -# gate. +# * Tests a binary built from the current working tree, rather than whatever +# long-running `make run` started in another terminal, which may be many +# edits stale. A gate that grades the wrong binary is worse than no gate. # # * Starts from empty state every run: fresh PDS, fresh PLC registry, fresh # databases. No accumulated accounts, no handle collisions, no fixtures from diff --git a/tests/e2e/error_recovery_test.go b/tests/e2e/error_recovery_test.go index b923cc4..50f95db 100644 --- a/tests/e2e/error_recovery_test.go +++ b/tests/e2e/error_recovery_test.go @@ -1,3 +1,5 @@ +//go:build e2e + package e2e import ( @@ -28,10 +30,6 @@ import ( // - Malformed events // - Out-of-order events func TestE2E_ErrorRecovery(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E error recovery test in short mode") - } - t.Run("Jetstream reconnection after disconnect", testJetstreamReconnection) t.Run("Malformed Jetstream events", testMalformedJetstreamEvents) t.Run("Database connection recovery", testDatabaseConnectionRecovery) diff --git a/tests/e2e/user_signup_test.go b/tests/e2e/user_signup_test.go index a755607..5fbf3b0 100644 --- a/tests/e2e/user_signup_test.go +++ b/tests/e2e/user_signup_test.go @@ -1,3 +1,5 @@ +//go:build e2e + package e2e import ( @@ -21,7 +23,7 @@ import ( // TestMain controls test setup for the e2e package. // Set LOG_ENABLED=false to suppress application log output during tests. func TestMain(m *testing.M) { - // Silence logs when LOG_ENABLED=false (used by make test-all) + // Silence logs when LOG_ENABLED=false (what .env.ci sets for the gate) if os.Getenv("LOG_ENABLED") == "false" { log.SetOutput(io.Discard) } @@ -47,10 +49,6 @@ func TestMain(m *testing.M) { // go run ./cmd/server & # Start AppView // go test ./tests/e2e -run TestE2E_UserSignup -v func TestE2E_UserSignup(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Check if AppView is available if !isAppViewAvailable(t) { t.Skip("AppView not available at localhost:8081 - run 'go run ./cmd/server' first") diff --git a/tests/e2e/user_signup_token_test.go b/tests/e2e/user_signup_token_test.go index e20c221..954156f 100644 --- a/tests/e2e/user_signup_token_test.go +++ b/tests/e2e/user_signup_token_test.go @@ -1,3 +1,5 @@ +//go:build e2e + package e2e import ( @@ -33,14 +35,11 @@ import ( // go run ./cmd/server & // go test ./tests/e2e -run TestE2E_UserSignupToken -v func TestE2E_UserSignupToken(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } if !isAppViewAvailable(t) { t.Skip("AppView not available at localhost:8081 - run 'go run ./cmd/server' first") } if !isPDSAvailable(t) { - t.Skip("PDS not available at localhost:3001 - run 'make e2e-up' first") + t.Skip("PDS not available at localhost:3001 - run 'make dev-up' first") } t.Run("Happy path: mint invite and sign up end-to-end", func(t *testing.T) { diff --git a/tests/integration/aggregator_e2e_test.go b/tests/integration/aggregator_e2e_test.go index 2805a13..b838c84 100644 --- a/tests/integration/aggregator_e2e_test.go +++ b/tests/integration/aggregator_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/aggregator_registration_test.go b/tests/integration/aggregator_registration_test.go index 0b3c672..e81579d 100644 --- a/tests/integration/aggregator_registration_test.go +++ b/tests/integration/aggregator_registration_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -63,10 +65,6 @@ func (m *mockAggregatorIdentityResolver) Purge(ctx context.Context, identifier s } func TestAggregatorRegistration_Success(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - // Setup test database db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -155,10 +153,6 @@ func TestAggregatorRegistration_Success(t *testing.T) { } func TestAggregatorRegistration_DomainVerificationFailed(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - // Setup test database db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -223,10 +217,6 @@ func TestAggregatorRegistration_DomainVerificationFailed(t *testing.T) { } func TestAggregatorRegistration_InvalidDID(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -285,10 +275,6 @@ func TestAggregatorRegistration_InvalidDID(t *testing.T) { } func TestAggregatorRegistration_AlreadyRegistered(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -369,10 +355,6 @@ func TestAggregatorRegistration_AlreadyRegistered(t *testing.T) { } func TestAggregatorRegistration_WellKnownNotAccessible(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -431,10 +413,6 @@ func TestAggregatorRegistration_WellKnownNotAccessible(t *testing.T) { } func TestAggregatorRegistration_WellKnownTooLarge(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -496,10 +474,6 @@ func TestAggregatorRegistration_WellKnownTooLarge(t *testing.T) { } func TestAggregatorRegistration_DIDResolutionFailed(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -572,10 +546,6 @@ func TestAggregatorRegistration_DIDResolutionFailed(t *testing.T) { } func TestAggregatorRegistration_LargeWellKnownResponse(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -662,10 +632,6 @@ func TestAggregatorRegistration_LargeWellKnownResponse(t *testing.T) { } func TestAggregatorRegistration_E2E_WithRealInfrastructure(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // 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 ea0af30..7ebc1a3 100644 --- a/tests/integration/aggregator_test.go +++ b/tests/integration/aggregator_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/author_avatar_hydration_test.go b/tests/integration/author_avatar_hydration_test.go index 664fa47..ced9076 100644 --- a/tests/integration/author_avatar_hydration_test.go +++ b/tests/integration/author_avatar_hydration_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -22,10 +24,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) diff --git a/tests/integration/author_posts_e2e_test.go b/tests/integration/author_posts_e2e_test.go index 58796f8..f88887d 100644 --- a/tests/integration/author_posts_e2e_test.go +++ b/tests/integration/author_posts_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -46,10 +48,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -291,10 +289,6 @@ func TestGetAuthorPosts_E2E_Success(t *testing.T) { // TestGetAuthorPosts_FilterLogic tests the different filter options func TestGetAuthorPosts_FilterLogic(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -430,10 +424,6 @@ func TestGetAuthorPosts_FilterLogic(t *testing.T) { // TestGetAuthorPosts_ServiceErrors tests error handling in the service layer func TestGetAuthorPosts_ServiceErrors(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -548,10 +538,6 @@ func TestGetAuthorPosts_ServiceErrors(t *testing.T) { // TestGetAuthorPosts_WithJetstreamIndexing tests the full flow including Jetstream indexing func TestGetAuthorPosts_WithJetstreamIndexing(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -661,10 +647,6 @@ func TestGetAuthorPosts_WithJetstreamIndexing(t *testing.T) { // TestGetAuthorPosts_CommunityFilter tests filtering posts by community func TestGetAuthorPosts_CommunityFilter(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() diff --git a/tests/integration/blob_upload_e2e_test.go b/tests/integration/blob_upload_e2e_test.go index fb5f7fd..91f2870 100644 --- a/tests/integration/blob_upload_e2e_test.go +++ b/tests/integration/blob_upload_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -38,10 +40,6 @@ import ( // - Blob references in atProto records // - URL transformation in AppView responses func TestBlobUpload_E2E_PostWithImages(t *testing.T) { - if testing.Short() { - t.Skip("Skipping blob upload E2E test in short mode") - } - // Check if PDS is available before running E2E test pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -357,10 +355,6 @@ func TestBlobUpload_E2E_PostWithImages(t *testing.T) { // TestBlobUpload_E2E_CommentWithImage tests image upload in comments func TestBlobUpload_E2E_CommentWithImage(t *testing.T) { - if testing.Short() { - t.Skip("Skipping comment image E2E test in short mode") - } - // Check if PDS is available before running E2E test pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") diff --git a/tests/integration/block_handle_resolution_test.go b/tests/integration/block_handle_resolution_test.go index b393213..7cf8094 100644 --- a/tests/integration/block_handle_resolution_test.go +++ b/tests/integration/block_handle_resolution_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/bluesky_post_test.go b/tests/integration/bluesky_post_test.go index ca622b1..d3c9c46 100644 --- a/tests/integration/bluesky_post_test.go +++ b/tests/integration/bluesky_post_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -13,10 +15,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping live PDS E2E test in short mode") - } - // 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 754929c..8305312 100644 --- a/tests/integration/comment_consumer_test.go +++ b/tests/integration/comment_consumer_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/comment_e2e_test.go b/tests/integration/comment_e2e_test.go index 3ba1779..b750741 100644 --- a/tests/integration/comment_e2e_test.go +++ b/tests/integration/comment_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -28,10 +30,6 @@ import ( // TestCommentE2E_CreateWithJetstream tests the full comment creation flow with real Jetstream // Flow: Client → Service → PDS Write → Jetstream Firehose → Consumer → AppView func TestCommentE2E_CreateWithJetstream(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -244,10 +242,6 @@ func TestCommentE2E_CreateWithJetstream(t *testing.T) { // TestCommentE2E_UpdateWithJetstream tests comment update with real Jetstream indexing func TestCommentE2E_UpdateWithJetstream(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -478,10 +472,6 @@ func TestCommentE2E_UpdateWithJetstream(t *testing.T) { // TestCommentE2E_DeleteWithJetstream tests comment deletion with real Jetstream indexing func TestCommentE2E_DeleteWithJetstream(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -891,10 +881,6 @@ func subscribeToJetstreamForCommentDelete( // TestCommentE2E_Authorization tests that users cannot modify other users' comments func TestCommentE2E_Authorization(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -1083,10 +1069,6 @@ func TestCommentE2E_Authorization(t *testing.T) { // TestCommentE2E_ValidationErrors tests that validation errors are properly returned func TestCommentE2E_ValidationErrors(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { diff --git a/tests/integration/comment_query_test.go b/tests/integration/comment_query_test.go index d0cebb4..ea09a82 100644 --- a/tests/integration/comment_query_test.go +++ b/tests/integration/comment_query_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/comment_vote_test.go b/tests/integration/comment_vote_test.go index 2f01fce..bffe46d 100644 --- a/tests/integration/comment_vote_test.go +++ b/tests/integration/comment_vote_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/comment_write_test.go b/tests/integration/comment_write_test.go index 1cca94d..5a88cb4 100644 --- a/tests/integration/comment_write_test.go +++ b/tests/integration/comment_write_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -26,10 +28,6 @@ import ( // TestCommentWrite_CreateTopLevelComment tests creating a comment on a post via E2E flow func TestCommentWrite_CreateTopLevelComment(t *testing.T) { - // Skip in short mode since this requires real PDS - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") @@ -281,10 +279,6 @@ func TestCommentWrite_CreateTopLevelComment(t *testing.T) { // TestCommentWrite_CreateNestedReply tests creating a reply to another comment func TestCommentWrite_CreateNestedReply(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -432,10 +426,6 @@ func TestCommentWrite_CreateNestedReply(t *testing.T) { // TestCommentWrite_UpdateComment tests updating an existing comment func TestCommentWrite_UpdateComment(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -553,10 +543,6 @@ func TestCommentWrite_UpdateComment(t *testing.T) { // TestCommentWrite_DeleteComment tests deleting a comment func TestCommentWrite_DeleteComment(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -662,10 +648,6 @@ func TestCommentWrite_DeleteComment(t *testing.T) { // TestCommentWrite_CannotUpdateOthersComment tests authorization for updates func TestCommentWrite_CannotUpdateOthersComment(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -742,10 +724,6 @@ func TestCommentWrite_CannotUpdateOthersComment(t *testing.T) { // TestCommentWrite_CannotDeleteOthersComment tests authorization for deletes func TestCommentWrite_CannotDeleteOthersComment(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -828,10 +806,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() diff --git a/tests/integration/community_avatar_e2e_test.go b/tests/integration/community_avatar_e2e_test.go index 239e40f..11327a9 100644 --- a/tests/integration/community_avatar_e2e_test.go +++ b/tests/integration/community_avatar_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -46,10 +48,6 @@ func createTestPNGImage(width, height int, c color.Color) []byte { // TestCommunityAvatarE2E_CreateWithAvatar tests creating a community with an avatar // Flow: CreateCommunity(avatar) → PDS uploadBlob + putRecord → Jetstream → Consumer → AppView func TestCommunityAvatarE2E_CreateWithAvatar(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -296,10 +294,6 @@ func TestCommunityAvatarE2E_CreateWithAvatar(t *testing.T) { // TestCommunityAvatarE2E_UpdateWithAvatar tests updating a community's avatar // Flow: UpdateCommunity(avatar) → PDS uploadBlob + putRecord → Jetstream → Consumer → AppView func TestCommunityAvatarE2E_UpdateWithAvatar(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -674,10 +668,6 @@ func TestCommunityAvatarE2E_UpdateWithAvatar(t *testing.T) { // TestCommunityAvatarE2E_UpdateWithBanner tests updating a community's banner // Flow: UpdateCommunity(banner) → PDS uploadBlob + putRecord → Jetstream → Consumer → AppView func TestCommunityAvatarE2E_UpdateWithBanner(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { diff --git a/tests/integration/community_blocking_test.go b/tests/integration/community_blocking_test.go index ea2e188..9deb5c2 100644 --- a/tests/integration/community_blocking_test.go +++ b/tests/integration/community_blocking_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -14,10 +16,6 @@ import ( // TestCommunityBlocking_Indexing tests Jetstream indexing of block events func TestCommunityBlocking_Indexing(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupBlockingTestDB(t, db) @@ -206,10 +204,6 @@ func TestCommunityBlocking_Indexing(t *testing.T) { // TestCommunityBlocking_ListBlocked tests listing blocked communities func TestCommunityBlocking_ListBlocked(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupBlockingTestDB(t, db) @@ -287,10 +281,6 @@ func TestCommunityBlocking_ListBlocked(t *testing.T) { // TestCommunityBlocking_IsBlocked tests the fast block check func TestCommunityBlocking_IsBlocked(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupBlockingTestDB(t, db) @@ -355,10 +345,6 @@ func TestCommunityBlocking_IsBlocked(t *testing.T) { // TestCommunityBlocking_GetBlock tests block retrieval func TestCommunityBlocking_GetBlock(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupBlockingTestDB(t, db) diff --git a/tests/integration/community_consumer_test.go b/tests/integration/community_consumer_test.go index d203d0b..4f17d4f 100644 --- a/tests/integration/community_consumer_test.go +++ b/tests/integration/community_consumer_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/community_credentials_test.go b/tests/integration/community_credentials_test.go index fa39c03..6f11466 100644 --- a/tests/integration/community_credentials_test.go +++ b/tests/integration/community_credentials_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/community_e2e_test.go b/tests/integration/community_e2e_test.go index cdf947e..d250297 100644 --- a/tests/integration/community_e2e_test.go +++ b/tests/integration/community_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -42,10 +44,6 @@ import ( // - Real Jetstream firehose subscription and event consumption // - Complete data flow from HTTP write to HTTP read via real infrastructure func TestCommunity_E2E(t *testing.T) { - // Skip in short mode since this requires real PDS - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") diff --git a/tests/integration/community_get_viewer_state_test.go b/tests/integration/community_get_viewer_state_test.go index 94481ed..3eb3910 100644 --- a/tests/integration/community_get_viewer_state_test.go +++ b/tests/integration/community_get_viewer_state_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/community_hostedby_security_test.go b/tests/integration/community_hostedby_security_test.go index 2ba66f3..b786e96 100644 --- a/tests/integration/community_hostedby_security_test.go +++ b/tests/integration/community_hostedby_security_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/community_identifier_resolution_test.go b/tests/integration/community_identifier_resolution_test.go index 9f55925..6c0cce6 100644 --- a/tests/integration/community_identifier_resolution_test.go +++ b/tests/integration/community_identifier_resolution_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -15,10 +17,6 @@ import ( // TestCommunityIdentifierResolution tests all formats accepted by ResolveCommunityIdentifier func TestCommunityIdentifierResolution(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -215,10 +213,6 @@ func TestCommunityIdentifierResolution(t *testing.T) { // TestResolveScopedIdentifier_InputValidation tests input sanitization func TestResolveScopedIdentifier_InputValidation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -394,10 +388,6 @@ func TestGetDisplayHandle(t *testing.T) { // TestIdentifierResolution_ErrorContext verifies error messages include identifier context func TestIdentifierResolution_ErrorContext(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -461,10 +451,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { diff --git a/tests/integration/community_list_viewer_state_test.go b/tests/integration/community_list_viewer_state_test.go index 827d717..1db9d61 100644 --- a/tests/integration/community_list_viewer_state_test.go +++ b/tests/integration/community_list_viewer_state_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/community_provisioning_test.go b/tests/integration/community_provisioning_test.go index 1cf8509..ffe48c5 100644 --- a/tests/integration/community_provisioning_test.go +++ b/tests/integration/community_provisioning_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/community_repo_test.go b/tests/integration/community_repo_test.go index b3ef9e7..3fc2793 100644 --- a/tests/integration/community_repo_test.go +++ b/tests/integration/community_repo_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/community_service_integration_test.go b/tests/integration/community_service_integration_test.go index 4454ba6..2ac4af2 100644 --- a/tests/integration/community_service_integration_test.go +++ b/tests/integration/community_service_integration_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -26,10 +28,6 @@ import ( // - Unit tests (direct DB writes, bypass PDS) // - E2E tests (full HTTP + Jetstream flow) func TestCommunityService_CreateWithRealPDS(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode - requires PDS") - } - // Check if PDS is running pdsURL := "http://localhost:3001" healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -280,10 +278,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode - requires PDS") - } - // Check if PDS is running pdsURL := "http://localhost:3001" healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -475,10 +469,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode - requires PDS") - } - // 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 77e66f9..ace7568 100644 --- a/tests/integration/community_suggestion_e2e_test.go +++ b/tests/integration/community_suggestion_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -238,10 +240,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - adminDID := "did:plc:testadmin" userDID := "did:plc:testuser1" user2DID := "did:plc:testuser2" @@ -1129,10 +1127,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - adminDID := "did:plc:testadmin" userDID := "did:plc:vieweruser1" user2DID := "did:plc:vieweruser2" @@ -1208,10 +1202,6 @@ func TestCommunitySuggestionE2E_ViewerStateOnGet(t *testing.T) { // TestCommunitySuggestionE2E_DownvoteFlow tests the full downvote lifecycle: // downvote, toggle off, then upvote. func TestCommunitySuggestionE2E_DownvoteFlow(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - 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 845af95..4bc8f36 100644 --- a/tests/integration/community_update_e2e_test.go +++ b/tests/integration/community_update_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -28,10 +30,6 @@ import ( // // This is a TRUE E2E test - no simulated Jetstream events! func TestCommunityUpdateE2E_WithJetstream(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { diff --git a/tests/integration/community_v2_validation_test.go b/tests/integration/community_v2_validation_test.go index 0ee2cc4..5f9248a 100644 --- a/tests/integration/community_v2_validation_test.go +++ b/tests/integration/community_v2_validation_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/concurrent_scenarios_test.go b/tests/integration/concurrent_scenarios_test.go index 9ae2638..82ede8d 100644 --- a/tests/integration/concurrent_scenarios_test.go +++ b/tests/integration/concurrent_scenarios_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -16,10 +18,6 @@ import ( // TestConcurrentVoting_MultipleUsersOnSamePost tests race conditions when multiple users // vote on the same post simultaneously func TestConcurrentVoting_MultipleUsersOnSamePost(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -327,10 +325,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -589,10 +583,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -728,10 +718,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { diff --git a/tests/integration/discover_test.go b/tests/integration/discover_test.go index 135a407..b6ab252 100644 --- a/tests/integration/discover_test.go +++ b/tests/integration/discover_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -70,10 +72,6 @@ func (m *mockVoteService) GetViewerVotesForSubjects(userDID string, subjectURIs // TestGetDiscover_ShowsAllCommunities tests discover feed shows posts from ALL communities func TestGetDiscover_ShowsAllCommunities(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -141,10 +139,6 @@ func TestGetDiscover_ShowsAllCommunities(t *testing.T) { // TestGetDiscover_NoAuthRequired tests discover feed works without authentication func TestGetDiscover_NoAuthRequired(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -188,10 +182,6 @@ func TestGetDiscover_NoAuthRequired(t *testing.T) { // TestGetDiscover_HotSort tests hot sorting across all communities func TestGetDiscover_HotSort(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -245,10 +235,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -305,10 +291,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -366,10 +348,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -412,10 +390,6 @@ func TestGetDiscover_HotSort_FutureDatedPost(t *testing.T) { // TestGetDiscover_Pagination tests cursor-based pagination func TestGetDiscover_Pagination(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -469,10 +443,6 @@ func TestGetDiscover_Pagination(t *testing.T) { // TestGetDiscover_LimitValidation tests limit parameter validation func TestGetDiscover_LimitValidation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -499,10 +469,6 @@ func TestGetDiscover_LimitValidation(t *testing.T) { // TestGetDiscover_ViewerVoteState tests that authenticated users see their vote state on posts func TestGetDiscover_ViewerVoteState(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -587,10 +553,6 @@ func TestGetDiscover_ViewerVoteState(t *testing.T) { // TestGetDiscover_NoViewerStateWithoutAuth tests that unauthenticated users don't get viewer state func TestGetDiscover_NoViewerStateWithoutAuth(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) diff --git a/tests/integration/feed_test.go b/tests/integration/feed_test.go index de9c493..c5a06be 100644 --- a/tests/integration/feed_test.go +++ b/tests/integration/feed_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -38,10 +40,6 @@ func getPostTitle(t *testing.T, pv *posts.PostView) string { // TestGetCommunityFeed_Hot tests hot feed sorting algorithm func TestGetCommunityFeed_Hot(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -117,10 +115,6 @@ func TestGetCommunityFeed_Hot(t *testing.T) { // TestGetCommunityFeed_Top_WithTimeframe tests top sorting with time filters func TestGetCommunityFeed_Top_WithTimeframe(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -195,10 +189,6 @@ func TestGetCommunityFeed_Top_WithTimeframe(t *testing.T) { // TestGetCommunityFeed_New tests chronological sorting func TestGetCommunityFeed_New(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -253,10 +243,6 @@ func TestGetCommunityFeed_New(t *testing.T) { // TestGetCommunityFeed_Pagination tests cursor-based pagination func TestGetCommunityFeed_Pagination(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -346,10 +332,6 @@ func TestGetCommunityFeed_Pagination(t *testing.T) { // TestGetCommunityFeed_InvalidCommunity tests error handling for invalid community func TestGetCommunityFeed_InvalidCommunity(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -384,10 +366,6 @@ func TestGetCommunityFeed_InvalidCommunity(t *testing.T) { // TestGetCommunityFeed_InvalidCursor tests cursor validation func TestGetCommunityFeed_InvalidCursor(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -442,10 +420,6 @@ func TestGetCommunityFeed_InvalidCursor(t *testing.T) { // TestGetCommunityFeed_EmptyFeed tests handling of empty communities func TestGetCommunityFeed_EmptyFeed(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -488,10 +462,6 @@ func TestGetCommunityFeed_EmptyFeed(t *testing.T) { // TestGetCommunityFeed_LimitValidation tests limit parameter validation func TestGetCommunityFeed_LimitValidation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -543,10 +513,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -646,10 +612,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -750,10 +712,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -853,10 +811,6 @@ func TestGetCommunityFeed_HotCursorTimeDrift(t *testing.T) { // TestGetCommunityFeed_BlobURLTransformation tests that blob refs are transformed to URLs func TestGetCommunityFeed_BlobURLTransformation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) diff --git a/tests/integration/helpers.go b/tests/integration/helpers.go index 72e1119..54b0594 100644 --- a/tests/integration/helpers.go +++ b/tests/integration/helpers.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/identity_resolution_test.go b/tests/integration/identity_resolution_test.go index 87816f9..62d0fdf 100644 --- a/tests/integration/identity_resolution_test.go +++ b/tests/integration/identity_resolution_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/image_proxy_e2e_test.go b/tests/integration/image_proxy_e2e_test.go index 66a858e..25901ea 100644 --- a/tests/integration/image_proxy_e2e_test.go +++ b/tests/integration/image_proxy_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -36,10 +38,6 @@ import ( // - Testing ETag-based caching (304 responses) // - Error handling for invalid presets and missing blobs func TestImageProxy_E2E(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E integration test in short mode") - } - // Check if PDS is running pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") @@ -371,10 +369,6 @@ func TestImageProxy_E2E(t *testing.T) { // TestImageProxy_CacheHit tests that cache hits are faster than cache misses func TestImageProxy_CacheHit(t *testing.T) { - if testing.Short() { - t.Skip("Skipping cache test in short mode") - } - // Check if PDS is running pdsURL := getTestPDSURL() healthResp, err := http.Get(pdsURL + "/xrpc/_health") diff --git a/tests/integration/jetstream_consumer_test.go b/tests/integration/jetstream_consumer_test.go index f9b8df8..d65719e 100644 --- a/tests/integration/jetstream_consumer_test.go +++ b/tests/integration/jetstream_consumer_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/oauth_e2e_test.go b/tests/integration/oauth_e2e_test.go index b330d48..2c6b9db 100644 --- a/tests/integration/oauth_e2e_test.go +++ b/tests/integration/oauth_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -31,10 +33,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping OAuth component test in short mode") - } - // Setup test database db := setupTestDB(t) defer func() { @@ -147,10 +145,6 @@ func testOAuthComponentsWithMockedSession(t *testing.T, ctx context.Context, _ i // TestOAuthE2E_TokenExpiration tests that expired sealed tokens are rejected func TestOAuthE2E_TokenExpiration(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth token expiration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -213,10 +207,6 @@ func TestOAuthE2E_TokenExpiration(t *testing.T) { // TestOAuthE2E_InvalidToken tests that invalid/tampered tokens are rejected func TestOAuthE2E_InvalidToken(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth invalid token test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -278,10 +268,6 @@ func TestOAuthE2E_InvalidToken(t *testing.T) { // TestOAuthE2E_SessionNotFound tests behavior when session doesn't exist in DB func TestOAuthE2E_SessionNotFound(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth session not found test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -314,10 +300,6 @@ func TestOAuthE2E_SessionNotFound(t *testing.T) { // TestOAuthE2E_MultipleSessionsPerUser tests that a user can have multiple active sessions func TestOAuthE2E_MultipleSessionsPerUser(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth multiple sessions test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -403,10 +385,6 @@ func TestOAuthE2E_MultipleSessionsPerUser(t *testing.T) { // TestOAuthE2E_AuthRequestStorage tests OAuth auth request storage and retrieval func TestOAuthE2E_AuthRequestStorage(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth auth request storage test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -513,10 +491,6 @@ func TestOAuthE2E_AuthRequestStorage(t *testing.T) { // TestOAuthE2E_TokenRefresh tests the refresh token flow func TestOAuthE2E_TokenRefresh(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth token refresh test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -769,10 +743,6 @@ func TestOAuthE2E_TokenRefresh(t *testing.T) { // TestOAuthE2E_SessionUpdate tests that refresh updates the session in database func TestOAuthE2E_SessionUpdate(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth session update test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -856,10 +826,6 @@ func TestOAuthE2E_SessionUpdate(t *testing.T) { // TestOAuthE2E_RefreshTokenRotation tests refresh token rotation behavior func TestOAuthE2E_RefreshTokenRotation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping OAuth refresh token rotation test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() diff --git a/tests/integration/oauth_helpers.go b/tests/integration/oauth_helpers.go index cb92bb9..ec1d1db 100644 --- a/tests/integration/oauth_helpers.go +++ b/tests/integration/oauth_helpers.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/oauth_session_fixation_test.go b/tests/integration/oauth_session_fixation_test.go index c42374b..4624128 100644 --- a/tests/integration/oauth_session_fixation_test.go +++ b/tests/integration/oauth_session_fixation_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -32,10 +34,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping OAuth session fixation test in short mode") - } - // Setup test database db := setupTestDB(t) defer func() { diff --git a/tests/integration/oauth_session_handle_sync_test.go b/tests/integration/oauth_session_handle_sync_test.go index 23fd14a..750c607 100644 --- a/tests/integration/oauth_session_handle_sync_test.go +++ b/tests/integration/oauth_session_handle_sync_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -291,10 +293,6 @@ func TestOAuthSessionHandleSync(t *testing.T) { // TEST_DATABASE_URL="postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" \ // go test -v ./tests/integration/ -run "TestOAuthSessionHandleSync_LiveJetstream" func TestOAuthSessionHandleSync_LiveJetstream(t *testing.T) { - if testing.Short() { - t.Skip("Skipping live Jetstream test in short mode") - } - // Check if Jetstream is available if !isServiceAvailable("http://localhost:6008") { t.Skip("Jetstream not available at localhost:6008 - run 'docker-compose --profile jetstream up -d' first") diff --git a/tests/integration/oauth_token_verification_test.go b/tests/integration/oauth_token_verification_test.go index 693584b..284ad6c 100644 --- a/tests/integration/oauth_token_verification_test.go +++ b/tests/integration/oauth_token_verification_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -23,10 +25,6 @@ import ( // for testing purposes. Real OAuth tokens from PDS would be sealed using the // OAuth client's seal secret. func TestOAuthTokenVerification(t *testing.T) { - // Skip in short mode since this requires real PDS - if testing.Short() { - t.Skip("Skipping OAuth token verification test in short mode") - } pdsURL := os.Getenv("PDS_URL") if pdsURL == "" { diff --git a/tests/integration/post_consumer_test.go b/tests/integration/post_consumer_test.go index bed51f2..6aff2f9 100644 --- a/tests/integration/post_consumer_test.go +++ b/tests/integration/post_consumer_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/post_creation_test.go b/tests/integration/post_creation_test.go index 659a911..2a1c08c 100644 --- a/tests/integration/post_creation_test.go +++ b/tests/integration/post_creation_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -17,10 +19,6 @@ import ( ) func TestPostCreation_Basic(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -291,10 +289,6 @@ func TestPostCreation_Basic(t *testing.T) { // TestPostRepository_Create tests the repository layer func TestPostRepository_Create(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { diff --git a/tests/integration/post_delete_test.go b/tests/integration/post_delete_test.go index e38847d..bafa4c5 100644 --- a/tests/integration/post_delete_test.go +++ b/tests/integration/post_delete_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -225,10 +227,6 @@ func TestPostDeletion_JetstreamConsumer(t *testing.T) { // TestPostDeletion_Authorization tests that only the post author can delete their posts func TestPostDeletion_Authorization(t *testing.T) { - if testing.Short() { - t.Skip("Skipping authorization test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -347,10 +345,6 @@ func TestPostDeletion_Authorization(t *testing.T) { // TestPostDeletion_ServiceAuthorization tests the author verification logic in the service layer // This test requires a live PDS to fully test the authorization flow func TestPostDeletion_ServiceAuthorization_LivePDS(t *testing.T) { - if testing.Short() { - t.Skip("Skipping live PDS test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -493,10 +487,6 @@ func TestPostDeletion_ServiceAuthorization_LivePDS(t *testing.T) { // 5. Receive delete event from Jetstream // 6. Verify post is soft-deleted in AppView DB func TestPostE2E_DeleteWithJetstream(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { diff --git a/tests/integration/post_e2e_test.go b/tests/integration/post_e2e_test.go index 3e7a5a4..f8a6142 100644 --- a/tests/integration/post_e2e_test.go +++ b/tests/integration/post_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -323,10 +325,6 @@ func TestPostCreation_E2E_WithJetstream(t *testing.T) { // - Live Jetstream running at the local dev Jetstream (JETSTREAM_FEEDS default: self=ws://localhost:6008) // - Test database running func TestPostCreation_E2E_LivePDS(t *testing.T) { - if testing.Short() { - t.Skip("Skipping live PDS E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { diff --git a/tests/integration/post_handler_test.go b/tests/integration/post_handler_test.go index c568f83..f151809 100644 --- a/tests/integration/post_handler_test.go +++ b/tests/integration/post_handler_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -19,10 +21,6 @@ import ( // TestPostHandler_SecurityValidation tests HTTP handler-level security checks func TestPostHandler_SecurityValidation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -389,10 +387,6 @@ func TestPostHandler_SecurityValidation(t *testing.T) { // TestPostHandler_SpecialCharacters tests content with special characters func TestPostHandler_SpecialCharacters(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -475,10 +469,6 @@ func TestPostHandler_SpecialCharacters(t *testing.T) { // TestPostService_DIDValidationSecurity tests service-layer DID validation (defense-in-depth) func TestPostService_DIDValidationSecurity(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { diff --git a/tests/integration/post_thumb_validation_test.go b/tests/integration/post_thumb_validation_test.go index 414c880..9eccaff 100644 --- a/tests/integration/post_thumb_validation_test.go +++ b/tests/integration/post_thumb_validation_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -42,10 +44,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -307,10 +305,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { diff --git a/tests/integration/post_unfurl_test.go b/tests/integration/post_unfurl_test.go index 96c62cb..f787310 100644 --- a/tests/integration/post_unfurl_test.go +++ b/tests/integration/post_unfurl_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -24,10 +26,6 @@ import ( // TestPostUnfurl_UnsupportedURL tests that posts with unsupported URLs still succeed func TestPostUnfurl_UnsupportedURL(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -128,10 +126,6 @@ func TestPostUnfurl_UnsupportedURL(t *testing.T) { // TestPostUnfurl_MissingEmbedType tests posts without external embed type don't trigger unfurling func TestPostUnfurl_MissingEmbedType(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -267,10 +261,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -312,10 +302,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { diff --git a/tests/integration/subscription_indexing_test.go b/tests/integration/subscription_indexing_test.go index 532a592..211e316 100644 --- a/tests/integration/subscription_indexing_test.go +++ b/tests/integration/subscription_indexing_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -15,10 +17,6 @@ import ( // TestSubscriptionIndexing_ContentVisibility tests that contentVisibility is properly indexed // from Jetstream events and stored in the AppView database func TestSubscriptionIndexing_ContentVisibility(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupTestDB(t, db) @@ -240,10 +238,6 @@ func TestSubscriptionIndexing_ContentVisibility(t *testing.T) { // TestSubscriptionIndexing_DeleteOperations tests unsubscribe (DELETE) event handling func TestSubscriptionIndexing_DeleteOperations(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupTestDB(t, db) @@ -356,10 +350,6 @@ func TestSubscriptionIndexing_DeleteOperations(t *testing.T) { // TestSubscriptionIndexing_SubscriberCount tests that subscriber counts are updated atomically func TestSubscriptionIndexing_SubscriberCount(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupTestDB(t, db) diff --git a/tests/integration/timeline_test.go b/tests/integration/timeline_test.go index c8b982f..ced0d7f 100644 --- a/tests/integration/timeline_test.go +++ b/tests/integration/timeline_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -20,10 +22,6 @@ import ( // TestGetTimeline_Basic tests timeline feed shows posts from subscribed communities func TestGetTimeline_Basic(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -109,10 +107,6 @@ func TestGetTimeline_Basic(t *testing.T) { // TestGetTimeline_HotSort tests hot sorting across multiple communities func TestGetTimeline_HotSort(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -180,10 +174,6 @@ func TestGetTimeline_HotSort(t *testing.T) { // TestGetTimeline_Pagination tests cursor-based pagination func TestGetTimeline_Pagination(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -256,10 +246,6 @@ func TestGetTimeline_Pagination(t *testing.T) { // TestGetTimeline_EmptyWhenNoSubscriptions tests timeline is empty when user has no subscriptions func TestGetTimeline_EmptyWhenNoSubscriptions(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -298,10 +284,6 @@ func TestGetTimeline_EmptyWhenNoSubscriptions(t *testing.T) { // TestGetTimeline_Unauthorized tests timeline requires authentication func TestGetTimeline_Unauthorized(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -327,10 +309,6 @@ func TestGetTimeline_Unauthorized(t *testing.T) { // TestGetTimeline_LimitValidation tests limit parameter validation func TestGetTimeline_LimitValidation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) @@ -378,10 +356,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) t.Cleanup(func() { _ = db.Close() }) diff --git a/tests/integration/token_refresh_test.go b/tests/integration/token_refresh_test.go index 86936a8..44e1ffa 100644 --- a/tests/integration/token_refresh_test.go +++ b/tests/integration/token_refresh_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -93,10 +95,6 @@ func TestTokenRefresh_ExpirationDetection(t *testing.T) { // TestTokenRefresh_UpdateCredentials tests the repository UpdateCredentials method func TestTokenRefresh_UpdateCredentials(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer func() { @@ -163,10 +161,6 @@ 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) { - if testing.Short() { - t.Skip("skipping E2E test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer func() { diff --git a/tests/integration/user_journey_e2e_test.go b/tests/integration/user_journey_e2e_test.go index cacfb6d..d3789c2 100644 --- a/tests/integration/user_journey_e2e_test.go +++ b/tests/integration/user_journey_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -50,10 +52,6 @@ import ( // - Multi-user interactions and data consistency // - Timeline aggregation and feed generation func TestFullUserJourney_E2E(t *testing.T) { - // Skip in short mode since this requires real PDS and Jetstream - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") diff --git a/tests/integration/user_profile_avatar_e2e_test.go b/tests/integration/user_profile_avatar_e2e_test.go index e8c899f..1aeec0d 100644 --- a/tests/integration/user_profile_avatar_e2e_test.go +++ b/tests/integration/user_profile_avatar_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -58,10 +60,6 @@ func createTestAvatarPNG(width, height int, c color.Color) []byte { // 3. Jetstream consumer receives and processes the event // 4. GetProfile returns the correct avatar URL func TestUserProfileAvatarE2E_UpdateWithAvatar(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -356,10 +354,6 @@ func TestUserProfileAvatarE2E_UpdateWithAvatar(t *testing.T) { // TestUserProfileAvatarE2E_UpdateWithBanner tests the full flow of updating a user profile with a banner func TestUserProfileAvatarE2E_UpdateWithBanner(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -600,10 +594,6 @@ func TestUserProfileAvatarE2E_UpdateWithBanner(t *testing.T) { // TestUserProfileAvatarE2E_UpdateDisplayNameAndBio tests updating non-blob profile fields func TestUserProfileAvatarE2E_UpdateDisplayNameAndBio(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { @@ -804,10 +794,6 @@ func TestUserProfileAvatarE2E_UpdateDisplayNameAndBio(t *testing.T) { // TestUserProfileAvatarE2E_ReplaceAvatar tests replacing an existing avatar with a new one func TestUserProfileAvatarE2E_ReplaceAvatar(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") if dbURL == "" { diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 37d2966..3e757e2 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -39,7 +41,7 @@ func testUserRouteOptions() *routes.UserRouteOptions { // TestMain controls test setup for the integration package. // Set LOG_ENABLED=false to suppress application log output during tests. func TestMain(m *testing.M) { - // Silence logs when LOG_ENABLED=false (used by make test-all) + // Silence logs when LOG_ENABLED=false (what .env.ci sets for the gate) if os.Getenv("LOG_ENABLED") == "false" { log.SetOutput(io.Discard) } diff --git a/tests/integration/userblock_e2e_test.go b/tests/integration/userblock_e2e_test.go index 97baf6f..9caea9d 100644 --- a/tests/integration/userblock_e2e_test.go +++ b/tests/integration/userblock_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -24,10 +26,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -371,10 +369,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() diff --git a/tests/integration/userblock_enforcement_test.go b/tests/integration/userblock_enforcement_test.go index 6a84602..7888e5a 100644 --- a/tests/integration/userblock_enforcement_test.go +++ b/tests/integration/userblock_enforcement_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -19,10 +21,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) @@ -185,10 +183,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) @@ -321,10 +315,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) @@ -394,10 +384,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) @@ -514,10 +500,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) diff --git a/tests/integration/userblock_handler_test.go b/tests/integration/userblock_handler_test.go index b110720..e599009 100644 --- a/tests/integration/userblock_handler_test.go +++ b/tests/integration/userblock_handler_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( diff --git a/tests/integration/userblock_indexing_test.go b/tests/integration/userblock_indexing_test.go index 0d27cfe..7bc490b 100644 --- a/tests/integration/userblock_indexing_test.go +++ b/tests/integration/userblock_indexing_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -14,10 +16,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -88,10 +86,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -164,10 +158,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -221,10 +211,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) diff --git a/tests/integration/userblock_repo_test.go b/tests/integration/userblock_repo_test.go index f8de703..d388e4b 100644 --- a/tests/integration/userblock_repo_test.go +++ b/tests/integration/userblock_repo_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -13,10 +15,6 @@ import ( // TestUserBlockRepo_BlockUser tests creating user blocks func TestUserBlockRepo_BlockUser(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -121,10 +119,6 @@ func TestUserBlockRepo_BlockUser(t *testing.T) { // TestUserBlockRepo_UnblockUser tests removing user blocks func TestUserBlockRepo_UnblockUser(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -171,10 +165,6 @@ func TestUserBlockRepo_UnblockUser(t *testing.T) { // TestUserBlockRepo_GetBlock tests block retrieval by blocker + blocked DID func TestUserBlockRepo_GetBlock(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -224,10 +214,6 @@ func TestUserBlockRepo_GetBlock(t *testing.T) { // TestUserBlockRepo_GetBlockByURI tests block retrieval by record URI func TestUserBlockRepo_GetBlockByURI(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -274,10 +260,6 @@ func TestUserBlockRepo_GetBlockByURI(t *testing.T) { // TestUserBlockRepo_ListBlockedUsers tests listing blocked users with pagination func TestUserBlockRepo_ListBlockedUsers(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -374,10 +356,6 @@ func TestUserBlockRepo_ListBlockedUsers(t *testing.T) { // TestUserBlockRepo_IsBlocked tests the fast block check func TestUserBlockRepo_IsBlocked(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -454,10 +432,6 @@ func TestUserBlockRepo_IsBlocked(t *testing.T) { // TestUserBlockRepo_AreBlocked tests the batch block check func TestUserBlockRepo_AreBlocked(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) @@ -524,10 +498,6 @@ 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) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - ctx := context.Background() db := setupTestDB(t) defer cleanupUserBlockTestDB(t, db) diff --git a/tests/integration/vote_e2e_test.go b/tests/integration/vote_e2e_test.go index cac6c91..843257e 100644 --- a/tests/integration/vote_e2e_test.go +++ b/tests/integration/vote_e2e_test.go @@ -1,3 +1,5 @@ +//go:build integration + package integration import ( @@ -29,10 +31,6 @@ import ( // TestVoteE2E_CreateUpvote tests the full vote creation flow with a real local PDS // Flow: Client → XRPC → PDS Write → Jetstream → Consumer → AppView func TestVoteE2E_CreateUpvote(t *testing.T) { - // Skip in short mode since this requires real PDS - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } // Setup test database dbURL := os.Getenv("TEST_DATABASE_URL") @@ -291,10 +289,6 @@ func TestVoteE2E_CreateUpvote(t *testing.T) { // TestVoteE2E_ToggleSameDirection tests voting twice in same direction (toggle off) func TestVoteE2E_ToggleSameDirection(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -461,10 +455,6 @@ func TestVoteE2E_ToggleSameDirection(t *testing.T) { // TestVoteE2E_ToggleDifferentDirection tests changing vote direction func TestVoteE2E_ToggleDifferentDirection(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -687,10 +677,6 @@ func TestVoteE2E_ToggleDifferentDirection(t *testing.T) { // TestVoteE2E_DeleteVote tests explicit vote deletion func TestVoteE2E_DeleteVote(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -878,10 +864,6 @@ func TestVoteE2E_DeleteVote(t *testing.T) { // TestVoteE2E_JetstreamIndexing tests real Jetstream firehose consumption func TestVoteE2E_JetstreamIndexing(t *testing.T) { - if testing.Short() { - t.Skip("Skipping E2E test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -913,7 +895,7 @@ func TestVoteE2E_JetstreamIndexing(t *testing.T) { // connects AFTER the write, so it must use a cursor from before the write // to replay the event. A live-tail subscription races event propagation // (PDS → relay → Jetstream) and loses whenever the pipeline is warm — - // this was a reliable failure under `make test-all` and a pass in + // this was a reliable failure in a full-suite run and a pass in // isolation before the cursor was added. subscribeCursorUS := time.Now().Add(-2 * time.Second).UnixMicro() diff --git a/tests/lexicon_validation_test.go b/tests/lexicon_validation_test.go index 588d6a6..f82a05f 100644 --- a/tests/lexicon_validation_test.go +++ b/tests/lexicon_validation_test.go @@ -19,7 +19,7 @@ const lexiconDir = "../internal/atproto/lexicon" // TestMain controls test setup for the tests package. // Set LOG_ENABLED=false to suppress application log output during tests. func TestMain(m *testing.M) { - // Silence logs when LOG_ENABLED=false (used by make test-all) + // Silence logs when LOG_ENABLED=false (what .env.ci sets for the gate) if os.Getenv("LOG_ENABLED") == "false" { log.SetOutput(io.Discard) } diff --git a/tests/live/bluesky_post_test.go b/tests/live/bluesky_post_test.go index b192847..76fe4fd 100644 --- a/tests/live/bluesky_post_test.go +++ b/tests/live/bluesky_post_test.go @@ -32,10 +32,6 @@ func productionPLCIdentityResolver(db *sql.DB) identity.Resolver { // TestBlueskyPostCrossPosting_URLParsing tests URL detection and parsing func TestBlueskyPostCrossPosting_URLParsing(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -111,10 +107,6 @@ func TestBlueskyPostCrossPosting_URLParsing(t *testing.T) { // TestBlueskyPostCrossPosting_LiveAPI tests fetching real posts from Bluesky func TestBlueskyPostCrossPosting_LiveAPI(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -390,10 +382,6 @@ func TestBlueskyPostCrossPosting_LiveAPI(t *testing.T) { // TestBlueskyPostCrossPosting_CircuitBreaker tests circuit breaker behavior func TestBlueskyPostCrossPosting_CircuitBreaker(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - // This test verifies the circuit breaker pattern works correctly. // We don't actually want to trip the circuit breaker against production, // so this is more of a unit-level integration test. @@ -435,10 +423,6 @@ func TestBlueskyPostCrossPosting_CircuitBreaker(t *testing.T) { // TestBlueskyPostCrossPosting_E2E_PostCreation tests the full flow of creating a post with a Bluesky embed func TestBlueskyPostCrossPosting_E2E_PostCreation(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() @@ -506,10 +490,6 @@ func TestBlueskyPostCrossPosting_E2E_PostCreation(t *testing.T) { // TestBlueskyPostCrossPosting_EmbedConversion tests that Bluesky URLs in external embeds // are converted to social.coves.embed.post with proper strongRef (uri + cid) func TestBlueskyPostCrossPosting_EmbedConversion(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { _ = db.Close() }() diff --git a/tests/live/post_unfurl_test.go b/tests/live/post_unfurl_test.go index 64b518a..52cf7de 100644 --- a/tests/live/post_unfurl_test.go +++ b/tests/live/post_unfurl_test.go @@ -20,10 +20,6 @@ import ( // TestPostUnfurl_Streamable tests that a post with a Streamable URL gets unfurled func TestPostUnfurl_Streamable(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -168,10 +164,6 @@ func TestPostUnfurl_Streamable(t *testing.T) { // TestPostUnfurl_YouTube tests that a post with a YouTube URL gets unfurled func TestPostUnfurl_YouTube(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -215,10 +207,6 @@ func TestPostUnfurl_YouTube(t *testing.T) { // TestPostUnfurl_Reddit tests that a post with a Reddit URL gets unfurled func TestPostUnfurl_Reddit(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -261,10 +249,6 @@ func TestPostUnfurl_Reddit(t *testing.T) { // TestPostUnfurl_CacheHit tests that the second post with the same URL uses cache func TestPostUnfurl_CacheHit(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -327,10 +311,6 @@ func TestPostUnfurl_CacheHit(t *testing.T) { // TestPostUnfurl_UserProvidedMetadata tests that user-provided metadata is preserved func TestPostUnfurl_UserProvidedMetadata(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -442,10 +422,6 @@ func TestPostUnfurl_UserProvidedMetadata(t *testing.T) { // TestPostUnfurl_OpenGraph tests that OpenGraph URLs get unfurled func TestPostUnfurl_OpenGraph(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -497,10 +473,6 @@ func TestPostUnfurl_OpenGraph(t *testing.T) { // TestPostUnfurl_SmartRouting tests that oEmbed still works while OpenGraph handles others func TestPostUnfurl_SmartRouting(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { @@ -557,10 +529,6 @@ func TestPostUnfurl_SmartRouting(t *testing.T) { // TestPostUnfurl_KagiKite tests that Kagi Kite URLs get unfurled with story images func TestPostUnfurl_KagiKite(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - db := setupTestDB(t) defer func() { if err := db.Close(); err != nil { diff --git a/tests/testkit/cmd/testdbprepare/main.go b/tests/testkit/cmd/testdbprepare/main.go index d31f09e..8ec34fa 100644 --- a/tests/testkit/cmd/testdbprepare/main.go +++ b/tests/testkit/cmd/testdbprepare/main.go @@ -81,6 +81,15 @@ 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 diff --git a/tests/testkit/db.go b/tests/testkit/db.go index 9acc91e..1d1a1c8 100644 --- a/tests/testkit/db.go +++ b/tests/testkit/db.go @@ -490,6 +490,46 @@ 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. // diff --git a/tests/testkit/db_test.go b/tests/testkit/db_test.go index 08b4f7f..d9fc171 100644 --- a/tests/testkit/db_test.go +++ b/tests/testkit/db_test.go @@ -1,3 +1,5 @@ +//go:build integration + package testkit import ( diff --git a/tests/testkit/firehose_test.go b/tests/testkit/firehose_test.go index c1b983a..6ccc255 100644 --- a/tests/testkit/firehose_test.go +++ b/tests/testkit/firehose_test.go @@ -1,3 +1,5 @@ +//go:build integration + package testkit import ( diff --git a/tests/testkit/harness_support_test.go b/tests/testkit/harness_support_test.go new file mode 100644 index 0000000..d0059a7 --- /dev/null +++ b/tests/testkit/harness_support_test.go @@ -0,0 +1,163 @@ +package testkit + +import ( + "database/sql" + "fmt" + "runtime" + "strings" + "sync" + "testing" +) + +// Test support shared by every tier of testkit's own suite, and therefore +// untagged: a tagged build includes untagged files, but not the reverse, so +// anything both tiers use has to live here. TestMain, which genuinely needs +// infrastructure, is in harness_test.go behind the integration tag. + +// fakeT records what a testkit helper did to the test it was handed, so the +// failure paths can be asserted on. +// +// This is why TestingT exists as an interface: testing.TB cannot be implemented +// outside the standard library, so without it the only way to test "WaitFor +// fails with the last observation attached" would be to compile and run a +// throwaway test binary in a subprocess. +type fakeT struct { + mu sync.Mutex + failures []string + logs []string + cleanups []func() + fatal bool +} + +func (f *fakeT) Helper() {} +func (f *fakeT) Name() string { return "fakeT" } + +func (f *fakeT) Cleanup(fn func()) { + f.mu.Lock() + defer f.mu.Unlock() + f.cleanups = append(f.cleanups, fn) +} + +func (f *fakeT) Logf(format string, args ...any) { + f.mu.Lock() + defer f.mu.Unlock() + f.logs = append(f.logs, fmt.Sprintf(format, args...)) +} + +func (f *fakeT) Errorf(format string, args ...any) { + f.mu.Lock() + defer f.mu.Unlock() + f.failures = append(f.failures, fmt.Sprintf(format, args...)) +} + +// Fatalf mirrors *testing.T: it records the failure and does not return. +// Helpers under test rely on that — they call Fatalf and then `return`, and a +// fake that simply returned would let them run on with invalid state. +func (f *fakeT) Fatalf(format string, args ...any) { + f.mu.Lock() + f.failures = append(f.failures, fmt.Sprintf(format, args...)) + f.fatal = true + f.mu.Unlock() + runtime.Goexit() +} + +func (f *fakeT) failed() bool { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.failures) > 0 +} + +func (f *fakeT) message() string { + f.mu.Lock() + defer f.mu.Unlock() + return strings.Join(f.failures, "\n") +} + +func (f *fakeT) runCleanups() { + f.mu.Lock() + cleanups := f.cleanups + f.cleanups = nil + f.mu.Unlock() + // Reverse order, as testing.T runs them. + for i := len(cleanups) - 1; i >= 0; i-- { + cleanups[i]() + } +} + +// runIsolated runs fn on its own goroutine, so a fakeT.Fatalf inside it exits +// that goroutine instead of the test's. +func runIsolated(fn func()) { + done := make(chan struct{}) + go func() { + defer close(done) + fn() + }() + <-done +} + +// resetTemplateVerification forgets that this process already checked the +// template, forcing the next EnsureTemplate to re-verify against Postgres. +func resetTemplateVerification() { + templateMu.Lock() + defer templateMu.Unlock() + templateVerified = false +} + +// swapEndpoints rebinds only the endpoint singleton, re-reading it from the +// current environment, and registers the restore as a cleanup. +// +// Assigning endpointsOnce directly is a data race the moment `-shuffle=on` +// reorders tests or anything runs in parallel — the accessors read it under +// singletonMu, so writers must take that lock too. Every endpoint test needs to +// re-read the environment after t.Setenv, so the locking lives here once rather +// than being retyped (and forgotten) at each site. +func swapEndpoints(t *testing.T) { + t.Helper() + singletonMu.Lock() + savedEndpoints, savedTemplateName := endpointsOnce, templateNameOnce + endpointsOnce = sync.OnceValue(loadEndpoints) + // The template name is validated against the maintenance database, so it + // has to be re-derived whenever the endpoints change. + templateNameOnce = sync.OnceValues(loadTemplateName) + singletonMu.Unlock() + + t.Cleanup(func() { + singletonMu.Lock() + endpointsOnce, templateNameOnce = savedEndpoints, savedTemplateName + singletonMu.Unlock() + }) +} + +// swapSingletons rebinds the memoised process singletons and returns a function +// that restores them. +// +// Every write goes through singletonMu, the same lock the accessors read under. +// Assigning these directly from a test — as an earlier version did — is a data +// race the moment any test runs in parallel or `-shuffle=on` reorders things, +// and it would surface as an unrelated test failing under -race. +func swapSingletons(endpoints func() EndpointSet, admin func() (*sql.DB, error)) func() { + singletonMu.Lock() + savedEndpoints, savedAdmin := endpointsOnce, adminDBOnce + savedTemplateName := templateNameOnce + endpointsOnce, adminDBOnce = endpoints, admin + // The template name is validated against the maintenance database, so it + // has to be re-derived whenever the endpoints change. + templateNameOnce = sync.OnceValues(loadTemplateName) + singletonMu.Unlock() + + templateMu.Lock() + savedVerified := templateVerified + templateVerified = false + templateMu.Unlock() + + return func() { + singletonMu.Lock() + endpointsOnce, adminDBOnce = savedEndpoints, savedAdmin + templateNameOnce = savedTemplateName + singletonMu.Unlock() + + templateMu.Lock() + templateVerified = savedVerified + templateMu.Unlock() + } +} diff --git a/tests/testkit/harness_test.go b/tests/testkit/harness_test.go index 5985841..bb3e9ff 100644 --- a/tests/testkit/harness_test.go +++ b/tests/testkit/harness_test.go @@ -1,166 +1,21 @@ +//go:build integration + package testkit import ( - "database/sql" - "fmt" "os" - "runtime" - "strings" - "sync" "testing" ) // TestMain is also the worked example of the TestMain every migrating package // gets: testkit's own tests exercise Postgres, the PDS and Jetstream, so all // three are probed once here rather than failing test by test. +// +// It is tagged, and the support code it used to share a file with is not, +// because testkit's own suite spans two tiers: the pure tests (wait, fixtures, +// endpoints, the XRPC client against httptest) run in the tagless unit tier and +// must not be gated on infrastructure that they never touch. A TestMain governs +// the whole binary, so leaving this one untagged would have gated them anyway. func TestMain(m *testing.M) { os.Exit(Main(m, RequirePostgres, RequirePDS, RequireJetstream)) } - -// fakeT records what a testkit helper did to the test it was handed, so the -// failure paths can be asserted on. -// -// This is why TestingT exists as an interface: testing.TB cannot be implemented -// outside the standard library, so without it the only way to test "WaitFor -// fails with the last observation attached" would be to compile and run a -// throwaway test binary in a subprocess. -type fakeT struct { - mu sync.Mutex - failures []string - logs []string - cleanups []func() - fatal bool -} - -func (f *fakeT) Helper() {} -func (f *fakeT) Name() string { return "fakeT" } - -func (f *fakeT) Cleanup(fn func()) { - f.mu.Lock() - defer f.mu.Unlock() - f.cleanups = append(f.cleanups, fn) -} - -func (f *fakeT) Logf(format string, args ...any) { - f.mu.Lock() - defer f.mu.Unlock() - f.logs = append(f.logs, fmt.Sprintf(format, args...)) -} - -func (f *fakeT) Errorf(format string, args ...any) { - f.mu.Lock() - defer f.mu.Unlock() - f.failures = append(f.failures, fmt.Sprintf(format, args...)) -} - -// Fatalf mirrors *testing.T: it records the failure and does not return. -// Helpers under test rely on that — they call Fatalf and then `return`, and a -// fake that simply returned would let them run on with invalid state. -func (f *fakeT) Fatalf(format string, args ...any) { - f.mu.Lock() - f.failures = append(f.failures, fmt.Sprintf(format, args...)) - f.fatal = true - f.mu.Unlock() - runtime.Goexit() -} - -func (f *fakeT) failed() bool { - f.mu.Lock() - defer f.mu.Unlock() - return len(f.failures) > 0 -} - -func (f *fakeT) message() string { - f.mu.Lock() - defer f.mu.Unlock() - return strings.Join(f.failures, "\n") -} - -func (f *fakeT) runCleanups() { - f.mu.Lock() - cleanups := f.cleanups - f.cleanups = nil - f.mu.Unlock() - // Reverse order, as testing.T runs them. - for i := len(cleanups) - 1; i >= 0; i-- { - cleanups[i]() - } -} - -// runIsolated runs fn on its own goroutine, so a fakeT.Fatalf inside it exits -// that goroutine instead of the test's. -func runIsolated(fn func()) { - done := make(chan struct{}) - go func() { - defer close(done) - fn() - }() - <-done -} - -// resetTemplateVerification forgets that this process already checked the -// template, forcing the next EnsureTemplate to re-verify against Postgres. -func resetTemplateVerification() { - templateMu.Lock() - defer templateMu.Unlock() - templateVerified = false -} - -// swapEndpoints rebinds only the endpoint singleton, re-reading it from the -// current environment, and registers the restore as a cleanup. -// -// Assigning endpointsOnce directly is a data race the moment `-shuffle=on` -// reorders tests or anything runs in parallel — the accessors read it under -// singletonMu, so writers must take that lock too. Every endpoint test needs to -// re-read the environment after t.Setenv, so the locking lives here once rather -// than being retyped (and forgotten) at each site. -func swapEndpoints(t *testing.T) { - t.Helper() - singletonMu.Lock() - savedEndpoints, savedTemplateName := endpointsOnce, templateNameOnce - endpointsOnce = sync.OnceValue(loadEndpoints) - // The template name is validated against the maintenance database, so it - // has to be re-derived whenever the endpoints change. - templateNameOnce = sync.OnceValues(loadTemplateName) - singletonMu.Unlock() - - t.Cleanup(func() { - singletonMu.Lock() - endpointsOnce, templateNameOnce = savedEndpoints, savedTemplateName - singletonMu.Unlock() - }) -} - -// swapSingletons rebinds the memoised process singletons and returns a function -// that restores them. -// -// Every write goes through singletonMu, the same lock the accessors read under. -// Assigning these directly from a test — as an earlier version did — is a data -// race the moment any test runs in parallel or `-shuffle=on` reorders things, -// and it would surface as an unrelated test failing under -race. -func swapSingletons(endpoints func() EndpointSet, admin func() (*sql.DB, error)) func() { - singletonMu.Lock() - savedEndpoints, savedAdmin := endpointsOnce, adminDBOnce - savedTemplateName := templateNameOnce - endpointsOnce, adminDBOnce = endpoints, admin - // The template name is validated against the maintenance database, so it - // has to be re-derived whenever the endpoints change. - templateNameOnce = sync.OnceValues(loadTemplateName) - singletonMu.Unlock() - - templateMu.Lock() - savedVerified := templateVerified - templateVerified = false - templateMu.Unlock() - - return func() { - singletonMu.Lock() - endpointsOnce, adminDBOnce = savedEndpoints, savedAdmin - templateNameOnce = savedTemplateName - singletonMu.Unlock() - - templateMu.Lock() - templateVerified = savedVerified - templateMu.Unlock() - } -} diff --git a/tests/testkit/pds_test.go b/tests/testkit/pds_test.go index 3c85927..189b012 100644 --- a/tests/testkit/pds_test.go +++ b/tests/testkit/pds_test.go @@ -1,3 +1,5 @@ +//go:build integration + package testkit import ( -- 2.51.2