forked from molecule-ai/molecule-core
debe29c889
The migration-replay step globbed only *.up.sql, silently skipping the older flat-naming migrations (001_workspaces.sql, 009_activity_logs.sql, etc.). Fine while no integration test depended on those tables; broke when the #149 cross-table atomicity test came in needing both workspaces (FK target for activity_logs) and activity_logs themselves. Switch to globbing *.sql + sorted lex-order, excluding *.down.sql so up/down pairs don't undo themselves mid-run. Add a sanity check for workspaces + activity_logs + pending_uploads alongside the existing delegations gate so a future migration drift fails loud instead of silently skipping the regressed test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
172 lines
7.2 KiB
YAML
172 lines
7.2 KiB
YAML
name: Handlers Postgres Integration
|
|
|
|
# Real-Postgres integration tests for workspace-server/internal/handlers/.
|
|
# Triggered on every PR/push that touches the handlers package.
|
|
#
|
|
# Why this workflow exists
|
|
# ------------------------
|
|
# Strict-sqlmock unit tests pin which SQL statements fire — they're fast
|
|
# and let us iterate without a DB. But sqlmock CANNOT detect bugs that
|
|
# depend on the row state AFTER the SQL runs. The result_preview-lost
|
|
# bug shipped to staging in PR #2854 because every unit test was
|
|
# satisfied with "an UPDATE statement fired" — none verified the row's
|
|
# preview field actually landed. The local-postgres E2E that retrofit
|
|
# self-review caught it took 2 minutes to set up and would have caught
|
|
# the bug at PR-time.
|
|
#
|
|
# This job spins a Postgres service container, applies the migration,
|
|
# and runs `go test -tags=integration` against a live DB. Required
|
|
# check on staging branch protection — backend handler PRs cannot
|
|
# merge without a real-DB regression gate.
|
|
#
|
|
# Cost: ~30s job (postgres pull from GH cache + go build + 4 tests).
|
|
|
|
on:
|
|
push:
|
|
branches: [main, staging]
|
|
pull_request:
|
|
branches: [main, staging]
|
|
merge_group:
|
|
types: [checks_requested]
|
|
workflow_dispatch:
|
|
|
|
concurrency:
|
|
group: handlers-pg-integ-${{ github.event.pull_request.head.sha || github.sha }}
|
|
cancel-in-progress: false
|
|
|
|
jobs:
|
|
detect-changes:
|
|
name: detect-changes
|
|
runs-on: ubuntu-latest
|
|
outputs:
|
|
handlers: ${{ steps.filter.outputs.handlers }}
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
|
- uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1
|
|
id: filter
|
|
with:
|
|
filters: |
|
|
handlers:
|
|
- 'workspace-server/internal/handlers/**'
|
|
- 'workspace-server/internal/wsauth/**'
|
|
- 'workspace-server/migrations/**'
|
|
- '.github/workflows/handlers-postgres-integration.yml'
|
|
|
|
# Single-job-with-per-step-if pattern: always runs to satisfy the
|
|
# required-check name on branch protection; real work gates on the
|
|
# paths filter. See ci.yml's Platform (Go) for the same shape.
|
|
integration:
|
|
name: Handlers Postgres Integration
|
|
needs: detect-changes
|
|
runs-on: ubuntu-latest
|
|
services:
|
|
postgres:
|
|
image: postgres:15-alpine
|
|
env:
|
|
POSTGRES_PASSWORD: test
|
|
POSTGRES_DB: molecule
|
|
ports:
|
|
- 5432:5432
|
|
# GHA spins this with --health-cmd built in for postgres images.
|
|
options: >-
|
|
--health-cmd pg_isready
|
|
--health-interval 5s
|
|
--health-timeout 5s
|
|
--health-retries 10
|
|
defaults:
|
|
run:
|
|
working-directory: workspace-server
|
|
steps:
|
|
- if: needs.detect-changes.outputs.handlers != 'true'
|
|
working-directory: .
|
|
run: echo "No handlers/migrations changes — skipping; this job always runs to satisfy the required-check name."
|
|
|
|
- if: needs.detect-changes.outputs.handlers == 'true'
|
|
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
|
|
|
- if: needs.detect-changes.outputs.handlers == 'true'
|
|
uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5
|
|
with:
|
|
go-version: 'stable'
|
|
|
|
- if: needs.detect-changes.outputs.handlers == 'true'
|
|
name: Apply migrations to Postgres service
|
|
env:
|
|
PGPASSWORD: test
|
|
run: |
|
|
# Wait for postgres to actually accept connections (the
|
|
# GHA --health-cmd is best-effort but psql can still race).
|
|
for i in {1..15}; do
|
|
if pg_isready -h localhost -p 5432 -U postgres -q; then break; fi
|
|
echo "waiting for postgres..."; sleep 2
|
|
done
|
|
|
|
# Apply every .up.sql in lexicographic order with
|
|
# ON_ERROR_STOP=0 — failing migrations are SKIPPED rather than
|
|
# blocking the suite. This handles the current schema state
|
|
# where a few historical migrations (e.g. 017_memories_fts_*)
|
|
# depend on tables that were later renamed/dropped and so
|
|
# cannot replay from scratch. The migrations that DO succeed
|
|
# land their tables, which is sufficient for the integration
|
|
# tests in handlers/.
|
|
#
|
|
# Why not maintain a curated allowlist: every new migration
|
|
# touching a handlers/-tested table would have to update this
|
|
# workflow. With apply-all-or-skip, a future migration that
|
|
# adds a column to delegations runs automatically (its base
|
|
# table 049_delegations.up.sql already succeeded above it in
|
|
# the order). Operators only need to revisit this if the
|
|
# migration chain becomes legitimately replayable end-to-end.
|
|
#
|
|
# Per-migration result is logged so a failed migration that
|
|
# SHOULD have been replayable surfaces in the CI log instead
|
|
# of silently failing.
|
|
# Apply both *.sql (legacy, lives next to its module) and
|
|
# *.up.sql (newer up/down convention) in a single
|
|
# lexicographically-sorted pass. Excluding *.down.sql so the
|
|
# newest-naming-convention pairs don't undo themselves mid-run.
|
|
# Pre-#149-followup this loop only globbed *.up.sql, which
|
|
# silently skipped 001_workspaces.sql + 009_activity_logs.sql
|
|
# — fine while no integration test depended on those tables,
|
|
# not fine once a cross-table atomicity test came in.
|
|
set +e
|
|
for migration in $(ls migrations/*.sql 2>/dev/null | grep -v '\.down\.sql$' | sort); do
|
|
if psql -h localhost -U postgres -d molecule -v ON_ERROR_STOP=1 \
|
|
-f "$migration" >/dev/null 2>&1; then
|
|
echo "✓ $(basename "$migration")"
|
|
else
|
|
echo "⊘ $(basename "$migration") (skipped — see comment in workflow)"
|
|
fi
|
|
done
|
|
set -e
|
|
|
|
# Sanity: the delegations + workspaces + activity_logs tables
|
|
# MUST exist for the integration tests to be meaningful. Hard-
|
|
# fail if any didn't land — that would be a real regression we
|
|
# want loud.
|
|
for tbl in delegations workspaces activity_logs pending_uploads; do
|
|
if ! psql -h localhost -U postgres -d molecule -tA \
|
|
-c "SELECT 1 FROM information_schema.tables WHERE table_name = '$tbl'" \
|
|
| grep -q 1; then
|
|
echo "::error::$tbl table missing after migration replay — handler integration tests would be meaningless"
|
|
exit 1
|
|
fi
|
|
echo "✓ $tbl table present"
|
|
done
|
|
|
|
- if: needs.detect-changes.outputs.handlers == 'true'
|
|
name: Run integration tests
|
|
env:
|
|
INTEGRATION_DB_URL: postgres://postgres:test@localhost:5432/molecule?sslmode=disable
|
|
run: |
|
|
go test -tags=integration -timeout 5m -v ./internal/handlers/ -run "^TestIntegration_"
|
|
|
|
- if: needs.detect-changes.outputs.handlers == 'true' && failure()
|
|
name: Diagnostic dump on failure
|
|
env:
|
|
PGPASSWORD: test
|
|
run: |
|
|
echo "::group::delegations table state"
|
|
psql -h localhost -U postgres -d molecule -c "SELECT * FROM delegations LIMIT 50;" || true
|
|
echo "::endgroup::"
|