diff --git a/.agents/checks/review.md b/.agents/checks/review.md new file mode 100644 index 0000000..4635d39 --- /dev/null +++ b/.agents/checks/review.md @@ -0,0 +1,48 @@ +# Review Checks + +Checks for AI review agents. The authoritative rules live in [SAFETY.md](../../SAFETY.md), +[AGENTS.md](../../AGENTS.md), and [docs/tcb-model.md](../../docs/tcb-model.md) — this file is +the reviewer's distillation. + +- Look up every touched `pkg/` package in the SAFETY.md partition table first — the review bar + differs between the safety-critical core and the periphery. Flag core changes with 🌶️ and + state the blast radius (data corruption, lost writes, wrong-table swap, stranded slot). +- Core packages: every loop, queue, retry, and wait must be bounded. An unbounded anything in + a core package is a review-blocking defect. +- Dangerous APIs accept proof types (`statement.Classified`, `PreflightedTable`, + `VerifiedShadow`, `CleanWatermark`, `TableLock`) with package-private constructors — never a + raw string or bool that a caller could fabricate. Core code re-verifies its own + preconditions; it never trusts that the planner or CLI checked. +- Invariant enforcement points carry a `// INV: ` comment matching + [docs/invariants.md](../../docs/invariants.md); violations use `ErrInvariantViolation` + naming the ID and abort fail-closed — never a warning, never retried. +- New dependencies inside a core package require a recorded decision (core dependency list: + `pgx/v5`, `pglogrepl`, stdlib). `github.com/block/spirit` must never be imported as a + module — ideas are ported with citations, not code. +- Connections go through `pkg/dbconn` (bounded `lock_timeout` / `statement_timeout`) — flag + raw `pgx` pools in production code. +- SQL parsing goes through `pg_query_go`; flag `strings.Split(";")` or any hand-parsing. A + parse failure is an error surfaced to the caller. +- Generated SQL quotes every user-supplied or introspected identifier + (`pgx.Identifier{...}.Sanitize()` / `quote_ident()`) — flag raw interpolation of names into + SQL. Connection strings are parsed and re-serialized (`pgx.ParseConfig`), never + string-manipulated. +- Terminology: "schema change", not "migration", in code, CLI output, error messages, and new + docs — flag new occurrences except citations of external sources. +- Errors: wrapped with context and identifiers; no log-and-continue, no silent branch cases, + no discarded `Close()` errors, no `nolint`, no `--no-verify`. No panics in library code — + invariant violations return `ErrInvariantViolation` fail-closed. Postgres errors are matched + by SQLSTATE (`errors.As` → `*pgconn.PgError`, `.Code`), never by message text. +- Goroutines: every goroutine has an owner, a bounded lifetime, and a stop path — flag + fire-and-forget `go func()`. Core logic takes time from an injected clock — flag inline + `time.Now()`/`time.Sleep` in core packages. +- Comments describe *what* and *why*, never history — flag bug/PR references, + "previously X" notes, and counts or thresholds that will go stale. Log messages state what + *will* happen, not what *might*. No internal company details (cluster names, hostnames, + org names) in code, comments, commits, or PRs. +- Tests: real PostgreSQL for core logic (no mocked-DB tests), testify, `t.Context()` + (cleanups use `context.WithoutCancel(t.Context())`), named polling deadlines — flag bare + `time.Sleep` readiness waits and any timeout increase that masks a flake instead of fixing + the root cause. +- CI coverage: behavior that varies by PostgreSQL major must be exercised across the + supported matrix (14–18), not just the default version. diff --git a/.cursorrules b/.cursorrules new file mode 120000 index 0000000..47dc3e3 --- /dev/null +++ b/.cursorrules @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..22b1492 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,48 @@ +#!/bin/sh +set -e + +# Pre-commit: fast local feedback on staged Go files. Docs-only commits +# (no staged .go files) skip the Go checks entirely so Markdown edits stay +# instant. CI remains the authoritative gate. +# +# Enable with: make setup (sets core.hooksPath .githooks) + +# Ensure we're at the worktree root (not the git common dir). Unset GIT_DIR +# first — git sets it during hooks and it makes subprocess git commands +# resolve the wrong repo root. +unset GIT_DIR +cd "$(git rev-parse --show-toplevel)" + +staged_go=$(git diff --cached --name-only --diff-filter=d -- '*.go') +if [ -z "$staged_go" ]; then + echo "pre-commit: no staged Go files; skipping Go checks." + exit 0 +fi + +# Ensure GOROOT matches the Go version in go.mod. External tools (hermit, +# asdf) may set GOROOT to a different version, causing "compile: version +# does not match go tool version" errors in golangci-lint and go build. +GO_VERSION=$(sed -n 's/^go //p' go.mod | head -1) +if [ -n "$GO_VERSION" ]; then + for candidate in \ + "/opt/homebrew/opt/go/libexec" \ + "/usr/local/go" \ + "$HOME/sdk/go${GO_VERSION}" \ + "$HOME/Library/Caches/hermit/pkg/go-${GO_VERSION}"; do + if [ -x "$candidate/bin/go" ]; then + CANDIDATE_VERSION=$("$candidate/bin/go" version 2>/dev/null | grep -o "go${GO_VERSION}" || true) + if [ -n "$CANDIDATE_VERSION" ]; then + export GOROOT="$candidate" + export PATH="$GOROOT/bin:$PATH" + break + fi + fi + done +fi +export PATH="$PATH:$HOME/go/bin:/usr/local/go/bin:/opt/homebrew/bin" + +# Everything staged must still build. +go build ./... + +# gofmt + golangci-lint on staged files: auto-fix, re-stage, verify. +scripts/lint-fix.sh diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 0000000..3e7c8bd --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,115 @@ +#!/bin/sh +set -e + +# Pre-push: run unit tests (go test -race, integration skipped) for the +# packages touched by the commits being pushed. Docs-only pushes skip +# entirely. CI remains the authoritative gate and runs the full suite +# including the PostgreSQL matrix — this hook does not replace it. +# +# Reads the standard pre-push stdin records: +# + +unset GIT_DIR +cd "$(git rev-parse --show-toplevel)" + +remote_name="$1" + +# Ensure GOROOT matches the Go version in go.mod (see pre-commit). +GO_VERSION=$(sed -n 's/^go //p' go.mod | head -1) +if [ -n "$GO_VERSION" ]; then + for candidate in \ + "/opt/homebrew/opt/go/libexec" \ + "/usr/local/go" \ + "$HOME/sdk/go${GO_VERSION}" \ + "$HOME/Library/Caches/hermit/pkg/go-${GO_VERSION}"; do + if [ -x "$candidate/bin/go" ]; then + CANDIDATE_VERSION=$("$candidate/bin/go" version 2>/dev/null | grep -o "go${GO_VERSION}" || true) + if [ -n "$CANDIDATE_VERSION" ]; then + export GOROOT="$candidate" + export PATH="$GOROOT/bin:$PATH" + break + fi + fi + done +fi +export PATH="$PATH:$HOME/go/bin:/usr/local/go/bin:/opt/homebrew/bin" + +# Resolve a base ref for new-branch pushes (no remote sha yet): prefer the +# push remote's default branch, then origin/main, then main. +base_ref="" +for cand in "refs/remotes/$remote_name/HEAD" "refs/remotes/$remote_name/main" "refs/remotes/origin/main" "main"; do + if git rev-parse --verify --quiet "$cand" >/dev/null 2>&1; then + base_ref="$cand" + break + fi +done + +# is_zero_sha returns success when the sha is all zeros (branch create/delete +# sentinel), independent of SHA-1 vs SHA-256 length. +is_zero_sha() { + case "$1" in + *[!0]*) return 1 ;; + *) return 0 ;; + esac +} + +changed_go_files="" +while read -r local_ref local_sha remote_ref remote_sha; do + # Skip branch deletions — nothing to test. + if is_zero_sha "$local_sha"; then + continue + fi + + if is_zero_sha "$remote_sha"; then + # New branch on the remote: test what this branch introduces relative + # to the base. Fall back to the single tip commit if no base exists. + if [ -n "$base_ref" ]; then + range_base=$(git merge-base "$base_ref" "$local_sha" 2>/dev/null || true) + else + range_base="" + fi + if [ -z "$range_base" ]; then + range_base="${local_sha}~1" + fi + else + range_base="$remote_sha" + fi + + files=$(git diff --name-only --diff-filter=d "$range_base" "$local_sha" -- '*.go' 2>/dev/null || true) + if [ -n "$files" ]; then + if [ -z "$changed_go_files" ]; then + changed_go_files="$files" + else + changed_go_files="$changed_go_files +$files" + fi + fi +done + +changed_go_files=$(printf '%s\n' "$changed_go_files" | sed '/^$/d' | sort -u) +if [ -z "$changed_go_files" ]; then + echo "pre-push: no Go changes in pushed commits; skipping unit tests." + exit 0 +fi + +# Map changed files to package directories that still exist and contain Go +# source for the default build. +pkgs="" +for dir in $(printf '%s\n' "$changed_go_files" | xargs -n1 dirname | sort -u); do + testable_pkg=$(go list -e -f '{{if or (or .GoFiles .TestGoFiles) .XTestGoFiles}}{{.ImportPath}}{{end}}' "./$dir" 2>/dev/null || true) + if [ -d "$dir" ] && [ -n "$testable_pkg" ]; then + pkgs="$pkgs ./$dir" + fi +done + +pkgs=$(printf '%s' "$pkgs" | sed 's/^ *//') +if [ -z "$pkgs" ]; then + echo "pre-push: changed Go files map to no testable packages; skipping unit tests." + exit 0 +fi + +echo "pre-push: running unit tests (race, integration skipped) for changed packages:" +for p in $pkgs; do echo " $p"; done + +# shellcheck disable=SC2086 +SKIP_INTEGRATION=1 go test -race -count=1 $pkgs diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 120000 index 0000000..be77ac8 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1 @@ +../AGENTS.md \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7546aec..0bbc1c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,17 +5,70 @@ on: branches: [main] pull_request: +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: + # Detect whether the change touches anything other than *.md / docs/**. + # PRs that touch only docs skip the heavy jobs below; pushes to main + # always run. Fails open: if detection fails, treat as a code change. + changes: + runs-on: ubuntu-latest + permissions: + pull-requests: read + outputs: + code: ${{ steps.out.outputs.code }} + steps: + - name: Detect non-docs changes + if: github.event_name == 'pull_request' + id: filter + continue-on-error: true + uses: dorny/paths-filter@d1c1ffe0248fe513906c8e24db8ea791d46f8590 # v3.0.3 + with: + filters: | + code: + - '**' + - '!**/*.md' + - '!docs/**' + - name: Resolve code-change flag + id: out + shell: bash + env: + EVENT_NAME: ${{ github.event_name }} + FILTER_OUTCOME: ${{ steps.filter.outcome }} + FILTER_CODE: ${{ steps.filter.outputs.code }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" != "pull_request" ]; then + code=true + elif [ "$FILTER_OUTCOME" != "success" ]; then + echo "::warning::paths-filter failed; running CI instead of skipping" + code=true + else + code="$FILTER_CODE" + fi + echo "code=$code" >> "$GITHUB_OUTPUT" + echo "event=$EVENT_NAME code=$code" + lint: + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: go-version-file: go.mod - - uses: golangci/golangci-lint-action@v6 + # Pin the same golangci-lint major used locally (v2 config format); + # the action's default binary lags and cannot load a v2 config. + - uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 + with: + version: v2.12.2 build: + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -25,9 +78,13 @@ jobs: - run: make build # The integration suite runs against every Aurora-supported PostgreSQL - # major (see the version-support research doc): the version floor is a - # promise CI enforces, not documentation. + # major (see docs/postgresql-version-support.md): the version floor is a + # promise CI enforces, not documentation. These are vanilla PostgreSQL + # images — real Aurora engine-version validation is a separate gate that + # cannot run in public CI. test: + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest strategy: fail-fast: false @@ -41,3 +98,20 @@ jobs: with: go-version-file: go.mod - run: make test + + # Single required status for branch protection. Succeeds when nothing + # failed — including docs-only PRs where the heavy jobs were skipped. + ci-ok: + if: always() + needs: [changes, lint, build, test] + runs-on: ubuntu-latest + steps: + - name: Check job results + env: + RESULTS: ${{ toJSON(needs) }} + run: | + echo "$RESULTS" + if echo "$RESULTS" | grep -Eq '"result": *"(failure|cancelled)"'; then + echo "a required job failed or was cancelled" + exit 1 + fi diff --git a/.golangci.yml b/.golangci.yml index 31c51ae..dc76b27 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -2,6 +2,48 @@ version: "2" linters: enable: - - bodyclose - - misspell - - nolintlint + - errcheck # unchecked errors + - govet # suspicious constructs + - staticcheck # comprehensive static analysis + - ineffassign # useless assignments + - unused # unused code + - gocritic # opinionated but catches real bugs + - errorlint # proper error wrapping/comparison + - modernize # suggest modern Go idioms (wg.Go, etc.) + - noctx # requests without context + - bodyclose # unclosed HTTP response bodies + - usetesting # enforce t.Context() over context.Background() in tests + - unparam # unused/constant function parameters and results + - misspell # commonly misspelled words + - nolintlint # ill-formed or unexplained nolint directives + - revive # doc comments on exported symbols (see settings) + - gochecknoinits # no init() functions + - gochecknoglobals # no package-level mutable state (error sentinels exempt) + - containedctx # no context.Context stored in struct fields + settings: + usetesting: + context-background: true + context-todo: true + revive: + # Only the doc-comment rules; everything else revive offers is either + # covered by other linters or a judgment call that lives in AGENTS.md. + rules: + - name: exported + - name: package-comments + exclusions: + rules: + # Test helpers intentionally keep uniform signatures and fixed arguments + # for readability, so unused/constant params there are not worth churn. + - path: _test\.go + linters: + - unparam + # Test files and test-only support code may use fixtures and shared + # state; the no-globals bar applies to production code. + - path: (_test\.go|^internal/testutil/) + linters: + - gochecknoglobals + +formatters: + enable: + - gofmt + - goimports diff --git a/.goosehints b/.goosehints new file mode 120000 index 0000000..47dc3e3 --- /dev/null +++ b/.goosehints @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index efcd234..9bb974d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,6 +3,10 @@ Guidance for AI coding agents working on pg-sprite — an online schema-change engine for Aurora PostgreSQL. Deliberately short: don't restate what you can infer from the code. +This file is canonical. `CLAUDE.md`, `GEMINI.md`, `.cursorrules`, `.goosehints`, and +`.github/copilot-instructions.md` are symlinks to it — edit only this file. Review-agent +checks live in [.agents/checks/review.md](.agents/checks/review.md). + ## Read SAFETY.md first This codebase is partitioned into a **safety-critical core** and a periphery. @@ -14,28 +18,45 @@ SAFETY.md — the review bar and the AI-assistance posture differ by side. ## Build and test ```sh +make setup # one-time: configure git hooks (core.hooksPath .githooks) make build # build ./... and bin/pg-sprite make test # full suite; integration tests need Docker make test-unit # SKIP_INTEGRATION=1, no Docker +make test-db # suite against the compose DB (make db-up first); PG_DSN +make test-supported-postgres # full suite on every major 14 -> 18 make lint # golangci-lint ``` +- **Coverage invariant:** no behavior lands without a test that would fail without it; bug + fixes land with a regression test; the full suite (unit, integration, TLS, version matrix) + is a merge gate. Full statement: [docs/testing.md](docs/testing.md#the-coverage-invariant); + test-methodology rules (lifecycle fixtures, two-oracle SQL tests, real fault injection, + convergence oracle) are the `TM-*` registry in the same doc. - Always run the full `make test` when the scope of a change is unclear. - Never assume a test failure is unrelated to your change; investigate it. -- Never increase timeouts to fix flakes; find the root cause. +- Never increase timeouts to fix flakes; find the root cause — then prove the fix holds with + `scripts/test-flaky.sh [iterations] [package]` before declaring it fixed. - Integration tests run against real PostgreSQL (testcontainers); `PG_VERSION` selects the major (default 16), CI runs the matrix 14 → 18. Core logic is validated against a real database — no mocked-DB tests for core logic. ## Conventions +- Say **"schema change"**, not "migration", in code, CLI output, error messages, and new docs — + pg-sprite strings surface through orchestrators that ban "migration". Use "migration" only + when citing external sources (Spirit's `pkg/migration`, peer tools, PostgreSQL docs). - Use `pkg/dbconn` for connections — never raw `pgx` pools in production code (tests excepted). Every session runs under bounded `lock_timeout` / `statement_timeout`. +- Never build SQL by interpolating raw identifiers: any user-supplied or introspected name in + generated SQL goes through `pgx.Identifier{...}.Sanitize()` (or `quote_ident()` server-side). +- Never string-manipulate connection strings/DSNs — parse (`pgx.ParseConfig`), modify fields, + re-serialize; string ops break on passwords containing `/`, `@`, or `%`. - All SQL parsing goes through `pg_query_go` (once `pkg/statement` exists). No `strings.Split(";")`, no hand-parsing; a parse failure is an error surfaced to the caller. -- Tests use testify (`require` for setup, `assert` for verification), `t.Context()` (except in - cleanups, which run after the context is cancelled), and named polling deadlines — no bare - `time.Sleep` readiness waits. +- Tests use testify (`require` for setup, `assert` for verification), `t.Context()` (in + cleanups, which run after the context is cancelled, use + `context.WithoutCancel(t.Context())`), and named polling deadlines — no bare `time.Sleep` + readiness waits. - Errors: wrap with context and identifiers (`fmt.Errorf("create slot %s: %w", name, err)`); never log-and-continue; no silent branch cases; no `nolint`; no `--no-verify`. @@ -64,6 +85,45 @@ make lint # golangci-lint redundant safety closer on a handle someone else owns discards its guaranteed already-closed error). - State comparisons use typed constants and helpers, never raw string matching. +- **Never panic in library code.** Invariant violations return `ErrInvariantViolation` + fail-closed (see [SAFETY.md](SAFETY.md)); panics are reserved for provable programmer error + at startup. +- **Match Postgres errors by SQLSTATE** (`errors.As` → `*pgconn.PgError`, branch on `.Code`), + never by message text — error text varies by server version and locale. Sentinel/typed + errors are compared with `errors.Is`/`As` at boundaries. +- **Every goroutine has an owner, a bounded lifetime, and a stop path** — no fire-and-forget + `go func()`. +- **Core logic takes time from an injected clock**, not inline `time.Now()`/`time.Sleep` — + deterministic tests depend on it. +- Comments describe *what* and *why*, never history — no bug/PR references, no + "previously X" notes, no counts or thresholds that go stale; move comments with the code + they explain. +- Tests assert specific values, not just existence; no negative regression tests for removed + behavior. Log messages state what *will* happen, not what *might* ("will block", not + "may be blocked"). +- Never reference internal company details (cluster names, hostnames, org names) in code, + comments, commits, or PRs — this is a public repo. + +Mechanical style rules (doc comments on exported symbols, no `init()`, no package-level +mutable state, no `context.Context` in structs) are enforced by `.golangci.yml`, not prose. + +## Git and PRs + +- Do not create PRs automatically — pushing a branch is fine; opening the PR is the author's + decision. When asked, create PRs as drafts (`gh pr create --draft`); the author marks ready. +- Never squash or rewrite history after a human has reviewed (comments or approval) — add + commits so reviewers can see increments. Squash freely before review. +- Agent disclosure lines (agent name + model) go at the *bottom* of PR bodies and issue + bodies, after the content. +- Never reply to, post on, or resolve *human* review threads without the author's explicit + approval — agents do not speak for the author. Automated reviewer (e.g. Copilot) comments + may be replied to and resolved without separate approval, provided each reply describes the + fix with a commit link (or a reasoned rejection), is prefixed 🤖, and carries the agent + disclosure — resolve only after the reply is posted. +- After pushing new commits, refresh the PR title/summary to match — unless a human has + edited it. +- Upstream large branches with the leaf approach: map the dependency graph, peel off leaf + changes as small independent PRs first, in topological order. Design docs live in [docs/](docs/) — start at [docs/README.md](docs/README.md); the invariant registry is [docs/invariants.md](docs/invariants.md). diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 0000000..47dc3e3 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/GEMINI.md b/GEMINI.md new file mode 120000 index 0000000..47dc3e3 --- /dev/null +++ b/GEMINI.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/Makefile b/Makefile index 6fe553f..f8a23fc 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,17 @@ GO ?= go +COMPOSE ?= docker compose -.PHONY: build test test-unit lint clean +# PostgreSQL major under test (see docs/postgresql-version-support.md). +PG_VERSION ?= 16 +PG_PORT ?= 5432 +# Local dev database credentials (compose/compose.yml); test-only defaults. +PG_USER ?= pgsprite +PG_PASSWORD ?= pgsprite +PG_DATABASE ?= pgsprite +# Localhost-only test credentials, parameterized above — not a real secret. +PG_DSN_LOCAL = postgres://$(PG_USER):$(PG_PASSWORD)@localhost:$(PG_PORT)/$(PG_DATABASE)?sslmode=disable# sadscan:disable np.postgres.1 + +.PHONY: build test test-unit test-db test-supported-postgres lint setup db-up db-down clean build: $(GO) build -o bin/pg-sprite ./cmd/pg-sprite @@ -13,8 +24,34 @@ test: test-unit: SKIP_INTEGRATION=1 $(GO) test -race ./... +# Integration suite against the long-lived compose database (no per-test +# containers). Start it first: make db-up [PG_VERSION=14] +test-db: + PG_DSN="$(PG_DSN_LOCAL)" $(GO) test -race -count=1 ./... + +# Full suite against every supported PostgreSQL major (14 -> 18) via +# testcontainers — the local mirror of the CI matrix. +test-supported-postgres: + @for v in 14 15 16 17 18; do \ + echo "=== PostgreSQL $$v ==="; \ + PG_VERSION=$$v $(GO) test -race -count=1 ./... || exit 1; \ + done + lint: golangci-lint run +# Configure git hooks (relative path so worktrees work too). +setup: + git config core.hooksPath .githooks + +# Start / stop the local development database (compose/compose.yml). +COMPOSE_ENV = PG_VERSION=$(PG_VERSION) PG_PORT=$(PG_PORT) PG_USER=$(PG_USER) PG_PASSWORD=$(PG_PASSWORD) PG_DATABASE=$(PG_DATABASE) + +db-up: + $(COMPOSE_ENV) $(COMPOSE) -f compose/compose.yml up --wait -d + +db-down: + $(COMPOSE_ENV) $(COMPOSE) -f compose/compose.yml down -v + clean: rm -rf bin diff --git a/README.md b/README.md index 8dc2a51..e1c9562 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ rules that apply inside the core. Read it before changing anything under ## Development ```sh +make setup # one-time: configure git hooks (.githooks) make build # build ./... and the bin/pg-sprite binary make test # full suite; integration tests need Docker make test-unit # unit tests only (SKIP_INTEGRATION=1) @@ -36,7 +37,19 @@ make lint # golangci-lint ``` Integration tests run against a real PostgreSQL via testcontainers. `PG_VERSION` -selects the major (default 16); CI runs the matrix 14 → 18. +selects the major (default 16); CI runs the matrix 14 → 18. To iterate against a +long-lived local database instead of per-test containers: + +```sh +make db-up PG_VERSION=14 # start PostgreSQL 14 on localhost via compose +make test-db # run the suite against it (PG_DSN) +make db-down # stop and discard it +``` + +`make test-supported-postgres` runs the full suite against every supported +major (14 → 18) — the local mirror of the CI matrix. See +[docs/testing.md](docs/testing.md) for the test-suite layout, what each build +phase owes, and the vanilla-PostgreSQL-vs-real-Aurora validation boundary. ## Contributing diff --git a/cmd/pg-sprite/main.go b/cmd/pg-sprite/main.go index 462f3ba..0f1ad85 100644 --- a/cmd/pg-sprite/main.go +++ b/cmd/pg-sprite/main.go @@ -1,3 +1,4 @@ +// Command pg-sprite is an online schema-change engine for Aurora PostgreSQL. package main import ( diff --git a/compose/compose.yml b/compose/compose.yml new file mode 100644 index 0000000..3a9046f --- /dev/null +++ b/compose/compose.yml @@ -0,0 +1,21 @@ +# Local development database — the long-lived analogue of the testcontainers +# harness. `make db-up PG_VERSION=14` starts one PostgreSQL major on +# localhost; `make test-db` points the integration suite at it via PG_DSN so +# no per-test containers are started. This is a test database: durability is +# deliberately relaxed and all state is discarded on `make db-down`. +services: + postgres: + image: postgres:${PG_VERSION:-16} + environment: + POSTGRES_USER: ${PG_USER:-pgsprite} + POSTGRES_PASSWORD: ${PG_PASSWORD:-pgsprite} + POSTGRES_DB: ${PG_DATABASE:-pgsprite} + ports: + - "${PG_PORT:-5432}:5432" + # Test-only speed settings; never carry these to a real deployment. + command: ["postgres", "-c", "fsync=off", "-c", "full_page_writes=off"] + healthcheck: + test: ["CMD-SHELL", "pg_isready -U $${POSTGRES_USER} -d $${POSTGRES_DB}"] + interval: 1s + timeout: 3s + retries: 60 diff --git a/docs/README.md b/docs/README.md index 5a44e96..17dbd2e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1,6 +1,6 @@ # Online schema change engine for Aurora PostgreSQL -Research and design notes for building an online schema migration engine targeting +Research and design notes for building an online schema-change engine targeting **Aurora PostgreSQL**, by deriving and combining the best practices from established tools — [Spirit](https://github.com/block/spirit) (Aurora MySQL), [pg_osc](https://github.com/shayonj/pg-osc), [pg_repack](https://github.com/reorg/pg_repack), @@ -44,6 +44,7 @@ checkpoint/resume, tuned for Aurora. That is the gap this engine targets. | [change-capture-tradeoff.md](change-capture-tradeoff.md) | The canonical **triggers vs logical-decoding** trade-off for copy-and-swap — overhead, failover survival, WAL risk, and whether either lets us drop the checksum/checkpoint (answer: keep the checksum; triggers simplify but don't remove the checkpoint). Any doc proposing logical decoding as the default points here. | | [invariants.md](invariants.md) | The canonical **invariant registry** — testable runtime MUST-statements (correctness, locking, state/resume, refusals, orchestration), each with its enforcement point and source. Mined from this doc set plus [Spirit](https://github.com/block/spirit)'s stated safety invariants and [SchemaBot](https://github.com/block/schemabot)'s control-plane discipline; the build plan's phases carry per-invariant test obligations. | | [tcb-model.md](tcb-model.md) | The **TCB model** — the trusted-computing-base partition of the engine: which components are the small trusted core that enforces the invariant registry vs the untrusted periphery, the never-trust-callers rule, domain types that make illegal states unrepresentable, the in-TCB engineering rules (from TigerBeetle TIGER_STYLE, s2n-tls, qmail, bitcoin-core), the verification ladder, and the per-side AI-assisted development policy. | +| [testing.md](testing.md) | The **test-suite guide** — how to run the suite (unit, per-major, all supported majors, compose database), what Phase 0 covers today, the per-phase deferred test obligations, and the vanilla-PostgreSQL-matrix vs real-Aurora validation boundary. | | [schemabot-integration.md](schemabot-integration.md) | The **single home for orchestrator integration** — how SchemaBot (the reference orchestrator) drives the engine: the pluggable-engine overview, the verb mappings, the concrete adapter contract, and the design constraints (OC-* invariants) the integration imposes on the core. | ## TL;DR recommendation diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000..91fb224 --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,206 @@ +# Testing + +How the test suite is organized, what it covers today, and what each build +phase is obligated to add. Core logic is validated against a **real +PostgreSQL** — no mocked-DB tests for core logic (see +[design-principles.md](design-principles.md)). + +## The coverage invariant + +The suite is part of the safety argument, not a formality. The standing rule, +binding on every merge: + +> **No behavior lands without a test that would fail without it. Core logic +> is proven against real PostgreSQL on every supported major. CI runs the +> whole suite — unit, integration, TLS, and the 14 → 18 version matrix — as +> a merge gate on every PR. Coverage only ratchets up.** + +Concretely: + +- **Same-PR tests.** New behavior and the tests proving it land in the same + PR — code never merges ahead of its tests, and an invariant from + [invariants.md](invariants.md) lands with the test named for it (see the + build-phase mapping there). +- **Regression-first bug fixes.** A bug fix lands with a test that reproduces + the bug and fails on the pre-fix code. +- **Real database, race-enabled.** Unit tests run with `-race`; core (`pkg/`) + logic is never validated against mocks — integration tests run against + real PostgreSQL, across every supported major in CI. +- **The matrix is a gate, not advisory.** The `ci-ok` sentinel requires the + full version matrix; docs-only changes are the only path that skips it. +- **Coverage never regresses.** Deleting or skipping a test to get green is + forbidden (same rule as the hooks: no `--no-verify`, no `nolint`). A + numeric coverage ratchet on `pkg/` packages is wired into CI once Phase 1 + lands the first core package — until then this clause is enforced in + review. + +## Test-methodology invariants (TM) + +How tests are *built*, mined from the peer suites (pgroll, pg_repack, +pg-delta — the topology survey below covers *what environments* they test; +this covers *how*). Each rule binds from the phase noted. + +### TM-1 — Lifecycle fixture, not happy-path tests + +Every executor/schema-change integration test drives the **full lifecycle +through one shared fixture** — start → assert → abort → assert → restart → +complete → assert — so interrupted-and-retried is the default tested path, +not a special case. Once checkpointing exists, kill → resume joins the +lifecycle. *Binds:* Phase 2 (native executor) onward. *Source:* pgroll +`ExecuteTests` (`pkg/migrations/op_common_test.go`). + +### TM-2 — Two oracles for safety-encoding SQL + +Generated SQL whose exact shape carries a safety property (chunk +continuation predicates, `ON CONFLICT` arbiters, timeout preludes, +fallback-mode trigger bodies) is **frozen by exact-string test AND proven +behaviorally against a real database** — never just one of the two. +*Binds:* Phase 2 onward. *Source:* pgroll trigger/backfill template tests; +pg-delta's snapshot + roundtrip pairing. + +### TM-3 — Fault injection is real, and asserts durable state + +Contention tests hold a real `ACCESS EXCLUSIVE` lock from a second +connection; cancellation tests use context deadlines. After any injected +failure the test asserts the **durable state** (no wedged schema-change record, +no leaked shadow objects/slots/triggers) and the ability to proceed — not +merely the returned error type. *Binds:* Phase 2 onward; full +phase-boundary kill/resume matrix at Phases 4–8. *Source:* pgroll's +lock-holder pattern — and pg_repack's absence of it, the gap peers left +that our copy-and-swap phases must fill. + +### TM-4 — The adversarial schema corpus only grows + +Integration fixtures include the shapes that break naive engines: quoted +and whitespace identifiers, dropped-column tuple layouts, TOASTed values, +expression/partial indexes, non-default reloptions, generated and identity +columns, partitioned parents, tablespaces (including quoted names). The +corpus is shared across phases and **never shrinks to make a phase land**. +*Binds:* Phase 1 onward. *Source:* pg_repack `regress/sql/repack-setup.sql`. + +### TM-5 — Convergence is the diff oracle + +Every declarative-diff test proves, against two real databases: the derived +plan applies cleanly; re-introspect + re-diff yields **empty**; a second +derivation emits nothing (idempotency). Comparison is **semantic catalog +state** (normalized), with SQL snapshots as the secondary oracle; failures +print the residual diff and the original plan. *Binds:* Phase 3. +*Source:* pg-delta `tests/integration/roundtrip.ts`. + +### TM-6 — Every mutation direction per property + +For each object property the diff handles: absent → present, present → +changed, present → absent — plus replacement where PostgreSQL cannot ALTER +in place. *Binds:* Phase 3. *Source:* pg-delta operation suites. + +### TM-7 — Benchmarks carry correctness assertions + +Performance tests (copy throughput at multiple row scales; fallback-mode +trigger write amplification) verify post-benchmark data correctness and tag +results with commit SHA + PG version. A fast wrong answer is a failure. +*Binds:* Phase 4 onward. *Source:* pgroll `internal/benchmarks`. + +### TM-8 — A compiled-binary e2e path exists in CI + +Separate from Go package tests, CI runs the **built `pg-sprite` binary** +against a real database with checked-in example inputs as the acceptance +corpus — exit codes, output, and resulting database state asserted. +*Binds:* Phase 2 (first executing command). *Source:* pgroll `make +examples` CI job; pg_repack driving its CLI through `pg_regress`. + +### TM-9 — The operation must outlive the observer + +Any test that observes or interrupts an operation **in flight** (progress +polling, kill/resume mid-copy, injected faults between phases) seeds enough +rows that the operation demonstrably spans the observation or injection +point — otherwise the operation can finish before the fault lands and the +test passes without testing anything. Vacuous runs are a failure: the test +asserts the interruption actually hit mid-operation (e.g. the checkpoint +shows partial progress), not just the final state. *Binds:* Phase 4 +(copy-and-swap kill/resume) onward. *Source:* SchemaBot's in-flight +progress tests, which seed large row counts so an operation spans a poll +interval. + +**Beyond the peers:** none of the three does generative testing. From +Phase 3 we add **seeded schema/DDL generation** (generate desired state → +plan → apply → re-diff must be empty), printing the seed on failure and +promoting failing seeds to fixed regression cases. This is deliberately a +capability no peer suite has. + +## How to run + +| Command | What it does | +| --- | --- | +| `make test-unit` | Race-enabled unit tests, no Docker (`SKIP_INTEGRATION=1`). | +| `make test` | Full suite; integration tests start disposable PostgreSQL containers (testcontainers). `PG_VERSION` selects the major (default 16). | +| `make test-supported-postgres` | Full suite against every supported major, 14 → 18 — the local mirror of the CI matrix. | +| `make db-up` / `make test-db` / `make db-down` | Long-lived compose database on localhost; the suite connects to it via `PG_DSN` instead of starting per-test containers. Fastest loop for repeated integration runs. | + +The harness is [internal/testutil](../internal/testutil/postgres.go): +`StartPostgres` returns a connection URL (container, or `PG_DSN` when set) +and `NewSchema` gives each test a throwaway schema so parallel tests never +collide — which also means every integration test runs against a +**non-`public` schema**, an axis some peer tools (pgroll) treat as a separate +matrix dimension. `StartPostgresTLS` starts a TLS-only server with a +generated CA for verify-full tests. The harness has its own tests proving +the version selected by `PG_VERSION` is the version actually running, and +that throwaway schemas are isolated. + +## Version matrix vs real Aurora + +CI runs the matrix against **vanilla PostgreSQL 14 → 18 images** — the floor +promised in [postgresql-version-support.md](postgresql-version-support.md) is +enforced by CI, not just documented. Vanilla PostgreSQL is *not* Aurora: +storage internals, replication, and failover behavior differ, and some +Aurora-specific behavior (e.g. `rds.logical_replication`, failover slot +loss) cannot be exercised in public CI. Validation against real Aurora +engine versions is a separate, environment-specific gate that lives outside +this repository's CI. + +## Current coverage (Phase 0) + +| Area | Tests | +| --- | --- | +| CLI grammar / config | [internal/cli](../internal/cli/cli_test.go) | +| Pool config, bounded session timeouts | [pkg/dbconn](../pkg/dbconn/pool_config_test.go), [integration](../pkg/dbconn/dbconn_integration_test.go) | +| Retry classification and behavior | [pkg/dbconn/retry_test.go](../pkg/dbconn/retry_test.go) | +| RDS/Aurora TLS (unit) | [pkg/dbconn/rds_test.go](../pkg/dbconn/rds_test.go) | +| Verify-full TLS against a live TLS-only server | [pkg/dbconn/tls_integration_test.go](../pkg/dbconn/tls_integration_test.go) | +| Targeted blocker termination | [pkg/dbconn/dbconn_integration_test.go](../pkg/dbconn/dbconn_integration_test.go) | +| Test harness self-checks | [internal/testutil](../internal/testutil/postgres_test.go) | + +## Deferred test obligations (Phases 1–3) + +These are owed when the corresponding implementation lands — they are not +written speculatively against unimplemented behavior. The authoritative +per-phase test lists live in the build plan; the invariant registry +([invariants.md](invariants.md)) carries the per-invariant enforcement +points. + +| Phase | Test obligations (summary) | +| --- | --- | +| 1 — statement/classifier | Parse-based classification per DDL form; refusal (`not-native-safe`) contract; every classification decision tested against the reference table in [postgres-online-ddl-reference.md](postgres-online-ddl-reference.md). | +| 2 — native executor | Each native idiom (`CONCURRENTLY`, `NOT VALID` + `VALIDATE`, fast default, `USING INDEX`) exercised against all supported majors; bounded lock behavior under contention; invalid-index cleanup. | +| 3 — declarative diff | Desired-state → `ALTER` derivation correctness; diff idempotency (no-op on converged schema); refusal propagation through the diff path. | + +Copy-and-swap (shadow table, CDC, checksum gate, cutover, resume) is +Phases 4–7 and carries its own obligations, including checksum-gate and +checkpoint/resume fault-injection tests. + +## Topology obligations from peer-tool CIs + +A survey of the CI setups of pgroll, Reshape, pg-osc, pg_repack, +pg-schema-diff, pg-delta, migra, Atlas, SchemaHero, and Bytebase found no +peer testing physical replicas, poolers as live intermediaries, failover, or +cloud-managed PostgreSQL — those remain environment-gate territory (see +above). The patterns worth carrying, tied to the phase whose implementation +makes them meaningful: + +| Pattern (peer precedent) | Where it lands here | +| --- | --- | +| TLS-required server, verify-full + untrusted-CA rejection (pg-delta) | **Done** — `StartPostgresTLS` + [tls_integration_test.go](../pkg/dbconn/tls_integration_test.go). | +| Non-`public` schema placement (pgroll matrix dimension) | Structural — every test already runs in a throwaway non-`public` schema; Phase 1 classifier tests must keep qualifying objects. | +| Partitioned tables (pg_repack regression, pg-schema-diff acceptance) | Phases 1–2 — classifier and native-executor cases for partitioned parents/partitions (`DETACH PARTITION CONCURRENTLY` is PG 14+). | +| Tablespaces, including quoted names (pg_repack) | Phases 4–7 — shadow-table placement must preserve tablespace. | +| `wal_level=logical` server + publication interaction (pg_repack, pg-delta) | Phases 4–7 — CDC tests run against logical-decoding-enabled servers; add `wal_level=logical` to the harness/compose when Phase 4 starts. | +| Pinned minor versions in the matrix (pgroll, SchemaHero) vs floating major tags | Deliberate choice: we track floating major tags (`postgres:14` … `postgres:18`) so CI follows each major's latest minor automatically. Revisit if a minor-specific regression ever matters. | diff --git a/internal/testutil/postgres.go b/internal/testutil/postgres.go index 8742f39..1535f61 100644 --- a/internal/testutil/postgres.go +++ b/internal/testutil/postgres.go @@ -29,17 +29,24 @@ func PGVersion() string { return DefaultPGVersion } -// StartPostgres starts a disposable PostgreSQL container for the test and -// returns its connection URL. The container is terminated when the test ends. -// Set SKIP_INTEGRATION=1 to skip tests that need Docker. +// StartPostgres returns a PostgreSQL connection URL for the test. +// +// By default it starts a disposable container (terminated when the test +// ends). When PG_DSN is set, that external server is used instead and no +// container is started — the compose/ workflow and CI variants that run a +// long-lived server use this. Set SKIP_INTEGRATION=1 to skip tests that need +// a database entirely. func StartPostgres(t *testing.T) string { t.Helper() if os.Getenv("SKIP_INTEGRATION") != "" { - t.Skip("SKIP_INTEGRATION set; skipping test that needs Docker") + t.Skip("SKIP_INTEGRATION set; skipping test that needs a database") } - // The container must outlive t.Context (which is cancelled before - // cleanups run), so use Background and terminate via t.Cleanup. - ctx := context.Background() + if dsn := os.Getenv("PG_DSN"); dsn != "" { + return dsn + } + // t.Context only governs the start request; the running container is + // not tied to it and is terminated via t.Cleanup below. + ctx := t.Context() ctr, err := tcpostgres.Run(ctx, "postgres:"+PGVersion(), tcpostgres.BasicWaitStrategies()) require.NoError(t, err, "start postgres container") t.Cleanup(func() { @@ -63,8 +70,8 @@ func NewSchema(t *testing.T, pool *pgxpool.Pool) string { _, err := pool.Exec(t.Context(), fmt.Sprintf("CREATE SCHEMA %s", name)) require.NoError(t, err, "create throwaway schema") t.Cleanup(func() { - // t.Context is done by cleanup time; use a fresh context. - _, err := pool.Exec(context.Background(), fmt.Sprintf("DROP SCHEMA IF EXISTS %s CASCADE", name)) + // t.Context is cancelled by cleanup time; strip the cancellation. + _, err := pool.Exec(context.WithoutCancel(t.Context()), fmt.Sprintf("DROP SCHEMA IF EXISTS %s CASCADE", name)) if err != nil { t.Logf("drop throwaway schema %s: %v", name, err) } diff --git a/internal/testutil/postgres_test.go b/internal/testutil/postgres_test.go new file mode 100644 index 0000000..7cf29d9 --- /dev/null +++ b/internal/testutil/postgres_test.go @@ -0,0 +1,59 @@ +package testutil_test + +import ( + "os" + "strings" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" +) + +// TestServerMajorMatchesRequestedVersion proves the harness tests what it +// claims: the server the suite connects to actually runs the PG_VERSION +// major the CI matrix selected. Without this, a matrix entry that silently +// fell back to a default image would still pass every test. +func TestServerMajorMatchesRequestedVersion(t *testing.T) { + if os.Getenv("PG_DSN") != "" { + t.Skip("PG_DSN points at an external server; version is not harness-selected") + } + url := testutil.StartPostgres(t) + + pool, err := pgxpool.New(t.Context(), url) + require.NoError(t, err) + t.Cleanup(pool.Close) + + var major string + require.NoError(t, pool.QueryRow(t.Context(), + "SELECT (current_setting('server_version_num')::int / 10000)::text").Scan(&major)) + assert.Equal(t, testutil.PGVersion(), major, + "server major must match the requested PG_VERSION") +} + +// TestNewSchemaIsolation proves the per-test schema isolation the whole +// suite relies on: two schemas from NewSchema never collide, and objects +// created in one are invisible to the other. +func TestNewSchemaIsolation(t *testing.T) { + url := testutil.StartPostgres(t) + + pool, err := pgxpool.New(t.Context(), url) + require.NoError(t, err) + t.Cleanup(pool.Close) + + s1 := testutil.NewSchema(t, pool) + s2 := testutil.NewSchema(t, pool) + require.NotEqual(t, s1, s2) + require.True(t, strings.HasPrefix(s1, "t_")) + + _, err = pool.Exec(t.Context(), "CREATE TABLE "+s1+".only_here (id int)") + require.NoError(t, err) + + var exists bool + require.NoError(t, pool.QueryRow(t.Context(), + "SELECT EXISTS (SELECT 1 FROM information_schema.tables WHERE table_schema = $1 AND table_name = 'only_here')", + s2).Scan(&exists)) + assert.False(t, exists, "object in one throwaway schema must not appear in another") +} diff --git a/internal/testutil/postgres_tls.go b/internal/testutil/postgres_tls.go new file mode 100644 index 0000000..3890f76 --- /dev/null +++ b/internal/testutil/postgres_tls.go @@ -0,0 +1,174 @@ +package testutil + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go" + tcpostgres "github.com/testcontainers/testcontainers-go/modules/postgres" +) + +// TLSPostgres describes a TLS-only PostgreSQL started by StartPostgresTLS. +type TLSPostgres struct { + // URL is the connection URL without an sslmode parameter, so the + // caller's TLS configuration decides the handshake. + URL string + // CACertPath is the PEM CA certificate that signed the server + // certificate — the trust anchor for verify-full connections. + CACertPath string + // UntrustedCACertPath is a valid CA certificate that did NOT sign the + // server certificate, for negative verification tests. + UntrustedCACertPath string +} + +// tlsInitScript runs as the postgres user during initdb: it installs the +// server certificate and restricts pg_hba to TLS-only TCP connections, so +// every network connection in the test must complete a TLS handshake. +const tlsInitScript = `#!/bin/sh +set -e +cp /tls/server.crt /tls/server.key "$PGDATA"/ +chmod 0600 "$PGDATA"/server.key +cat >> "$PGDATA"/postgresql.conf < "$PGDATA"/pg_hba.conf </dev/null; then + echo "Auto-fixed: $file" + git add "$file" + fi + done +} + +# Use local golangci-lint if available, otherwise Docker. +# Check common Go binary paths since git hooks may not inherit the full user PATH. +LINT_CMD="" +for candidate in golangci-lint "$HOME/go/bin/golangci-lint" "$GOPATH/bin/golangci-lint" "$GOBIN/golangci-lint"; do + if command -v "$candidate" >/dev/null 2>&1; then + LINT_CMD="$candidate" + break + fi +done +if [ -z "$LINT_CMD" ]; then + LINT_CMD="docker run --rm -v $(pwd):/app -w /app golangci/golangci-lint:latest golangci-lint" +fi + +# Detect the merge-base so we only flag issues introduced by this branch. +# If merge-base equals HEAD (e.g., after git reset --soft for squashing), +# skip --new-from-rev to avoid treating every changed line as "new". +NEW_FROM_REV="" +for base_branch in origin/main origin/master; do + if git rev-parse --verify "$base_branch" >/dev/null 2>&1; then + MERGE_BASE=$(git merge-base HEAD "$base_branch" 2>/dev/null || true) + if [ -n "$MERGE_BASE" ] && [ "$MERGE_BASE" != "$(git rev-parse HEAD)" ]; then + NEW_FROM_REV="$MERGE_BASE" + fi + break + fi +done + +new_flag="" +if [ -n "$NEW_FROM_REV" ]; then + new_flag="--new-from-rev=$NEW_FROM_REV" +fi + +# Lint the packages containing staged files: auto-fix, re-stage, then verify. +PACKAGES=$(echo "$STAGED_GO_FILES" | xargs -n1 dirname | sort -u | sed 's|^|./|' | sed 's|$|/...|') + +echo "Running golangci-lint --fix..." +# shellcheck disable=SC2086 +$LINT_CMD run --fix --timeout=5m $PACKAGES || true + +restage_fixed_files + +# shellcheck disable=SC2086 +if ! $LINT_CMD run --timeout=5m $new_flag $PACKAGES; then + echo "" + echo "golangci-lint found issues that cannot be auto-fixed." + echo "Please fix them manually before committing." + exit 1 +fi + +echo "All lint checks passed!" diff --git a/scripts/test-flaky.sh b/scripts/test-flaky.sh new file mode 100755 index 0000000..7443440 --- /dev/null +++ b/scripts/test-flaky.sh @@ -0,0 +1,27 @@ +#!/bin/bash +# +# Proves a flake fix holds by running one test repeatedly. +# +# Usage: scripts/test-flaky.sh [iterations] [package] +# +# scripts/test-flaky.sh TestBuildIndexConcurrently +# scripts/test-flaky.sh TestBuildIndexConcurrently 20 ./pkg/executor/... +# +# Fails fast on the first failing iteration. Environment variables +# (PG_VERSION, PG_DSN, SKIP_INTEGRATION) pass through to `go test`. + +set -euo pipefail + +TEST_NAME="${1:?usage: scripts/test-flaky.sh [iterations] [package]}" +ITERATIONS="${2:-10}" +PACKAGE="${3:-./...}" + +for i in $(seq 1 "$ITERATIONS"); do + echo "=== iteration $i/$ITERATIONS: $TEST_NAME ===" + if ! go test -race -count=1 -run "^${TEST_NAME}$" "$PACKAGE"; then + echo "FAILED on iteration $i/$ITERATIONS" + exit 1 + fi +done + +echo "PASSED all $ITERATIONS iterations"