diff --git a/.github/workflows/migrations-ci.yml b/.github/workflows/migrations-ci.yml new file mode 100644 index 0000000..c84b1a6 --- /dev/null +++ b/.github/workflows/migrations-ci.yml @@ -0,0 +1,188 @@ +name: Migrations CI + +# Runs workflow-migrate against an ephemeral Postgres on every PR that touches +# migration driver code. This is the regression gate that catches panics like +# the v0.3.0 collectApplied() overflow before merge. + +permissions: + contents: read + +on: + pull_request: + paths: + - 'internal/golangmigrate/**' + - 'internal/goose/**' + - 'internal/atlas/**' + - 'cmd/workflow-migrate/**' + - 'pkg/**' + - 'go.mod' + - 'go.sum' + - '.github/workflows/migrations-ci.yml' + +jobs: + ephemeral-migrate: + name: workflow-migrate up/down (golang-migrate, timestamp versions) + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: ci + POSTGRES_PASSWORD: ci + POSTGRES_DB: ci_test + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U ci" + --health-interval 2s + --health-timeout 5s + --health-retries 20 + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Build workflow-migrate + run: | + CGO_ENABLED=0 go build -o /tmp/workflow-migrate ./cmd/workflow-migrate + + - name: Prepare fixture migrations (timestamp-based — the v0.3.0 regression scenario) + run: | + mkdir -p /tmp/fixture-migrations + + cat > /tmp/fixture-migrations/20240101000001_init.up.sql <<'SQL' + CREATE TABLE users (id SERIAL PRIMARY KEY, email TEXT UNIQUE NOT NULL); + SQL + cat > /tmp/fixture-migrations/20240101000001_init.down.sql <<'SQL' + DROP TABLE IF EXISTS users; + SQL + + cat > /tmp/fixture-migrations/20240201000001_posts.up.sql <<'SQL' + CREATE TABLE posts (id SERIAL PRIMARY KEY, user_id INT); + SQL + cat > /tmp/fixture-migrations/20240201000001_posts.down.sql <<'SQL' + DROP TABLE IF EXISTS posts; + SQL + + cat > /tmp/fixture-migrations/20240301000001_comments.up.sql <<'SQL' + CREATE TABLE comments (id SERIAL PRIMARY KEY, post_id INT, body TEXT); + SQL + cat > /tmp/fixture-migrations/20240301000001_comments.down.sql <<'SQL' + DROP TABLE IF EXISTS comments; + SQL + + - name: Run workflow-migrate up — timestamp-based versions (the v0.3.0 panic scenario) + env: + DATABASE_URL: postgres://ci:ci@localhost:5432/ci_test?sslmode=disable + run: | + /tmp/workflow-migrate up --source-dir /tmp/fixture-migrations + + - name: Verify tables created + env: + PGPASSWORD: ci + run: | + PSQL_OUTPUT=$(psql -h localhost -U ci -d ci_test -tAc '\dt') + echo "$PSQL_OUTPUT" + echo "$PSQL_OUTPUT" | grep -q 'users' || { echo "FAIL: users table missing after up"; exit 1; } + echo "$PSQL_OUTPUT" | grep -q 'posts' || { echo "FAIL: posts table missing after up"; exit 1; } + echo "$PSQL_OUTPUT" | grep -q 'comments' || { echo "FAIL: comments table missing after up"; exit 1; } + echo "All 3 tables present after up ✓" + + - name: Run workflow-migrate down — rollback smoke test + env: + DATABASE_URL: postgres://ci:ci@localhost:5432/ci_test?sslmode=disable + PGPASSWORD: ci + run: | + /tmp/workflow-migrate down --source-dir /tmp/fixture-migrations + # One migration rolled back — comments should be gone. + REMAINING=$(psql -h localhost -U ci -d ci_test -tAc \ + "SELECT count(*) FROM pg_tables WHERE schemaname='public' AND tablename != 'schema_migrations'") + REMAINING=$(echo "$REMAINING" | tr -d '[:space:]') + test "$REMAINING" = "2" || { echo "FAIL: expected 2 tables after down(1), got $REMAINING"; exit 1; } + echo "Correct table count ($REMAINING) after down(1) ✓" + + - name: Run workflow-migrate status + env: + DATABASE_URL: postgres://ci:ci@localhost:5432/ci_test?sslmode=disable + run: | + /tmp/workflow-migrate status --source-dir /tmp/fixture-migrations + + ephemeral-migrate-goose: + name: workflow-migrate up/down (goose driver, sequential versions) + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: ci + POSTGRES_PASSWORD: ci + POSTGRES_DB: ci_goose + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U ci" + --health-interval 2s + --health-timeout 5s + --health-retries 20 + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Build workflow-migrate + run: | + CGO_ENABLED=0 go build -o /tmp/workflow-migrate ./cmd/workflow-migrate + + - name: Prepare goose fixture migrations + run: | + mkdir -p /tmp/goose-migrations + + cat > /tmp/goose-migrations/00001_users.sql <<'SQL' + -- +goose Up + CREATE TABLE users (id SERIAL PRIMARY KEY, email TEXT UNIQUE NOT NULL); + -- +goose Down + DROP TABLE IF EXISTS users; + SQL + + cat > /tmp/goose-migrations/00002_posts.sql <<'SQL' + -- +goose Up + CREATE TABLE posts (id SERIAL PRIMARY KEY, user_id INT); + -- +goose Down + DROP TABLE IF EXISTS posts; + SQL + + - name: Run workflow-migrate up (goose driver) + env: + DATABASE_URL: postgres://ci:ci@localhost:5432/ci_goose?sslmode=disable + run: | + /tmp/workflow-migrate up --driver goose --source-dir /tmp/goose-migrations + + - name: Verify goose tables created + env: + PGPASSWORD: ci + run: | + PSQL_OUTPUT=$(psql -h localhost -U ci -d ci_goose -tAc '\dt') + echo "$PSQL_OUTPUT" + echo "$PSQL_OUTPUT" | grep -q 'users' || { echo "FAIL: users missing"; exit 1; } + echo "$PSQL_OUTPUT" | grep -q 'posts' || { echo "FAIL: posts missing"; exit 1; } + echo "Goose: both tables present after up ✓" + + - name: Run workflow-migrate down (goose driver) + env: + DATABASE_URL: postgres://ci:ci@localhost:5432/ci_goose?sslmode=disable + PGPASSWORD: ci + run: | + /tmp/workflow-migrate down --driver goose --source-dir /tmp/goose-migrations + REMAINING=$(psql -h localhost -U ci -d ci_goose -tAc \ + "SELECT count(*) FROM pg_tables WHERE schemaname='public' AND tablename != 'goose_db_version'") + REMAINING=$(echo "$REMAINING" | tr -d '[:space:]') + test "$REMAINING" = "1" || { echo "FAIL: expected 1 table after goose down(1), got $REMAINING"; exit 1; } + echo "Goose: correct table count ($REMAINING) after down(1) ✓" diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..623e8fb --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,26 @@ +version: "2" + +run: + timeout: 5m + +linters: + default: none + enable: + - errcheck + - govet + - staticcheck + - unused + +issues: + exclusions: + rules: + # defer Close() calls — these are cleanup paths where errors are + # unactionable and the pattern is idiomatic Go (pre-existing). + - linters: [errcheck] + text: "Error return value of.*\\.Close" + # fmt.Fprintln to stdout in CLI test helper (pre-existing). + - linters: [errcheck] + text: "Error return value of `fmt\\.Fprintln`" + # goreleaser-injected version vars set via -ldflags at build time. + - linters: [unused] + text: "var version is unused" diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e7ca7d..626a9c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.3.1] - 2026-04-24 + +### Fixed + +- **P0: golang-migrate driver panics on timestamp-based migration versions** — `collectApplied(before, after uint)` pre-allocated `make([]string, 0, after-before)`, assuming sequential version numbers. BMW uses timestamp-based versions (e.g. `20240101000001`), so `after-before = 2×10¹³` → `makeslice: cap out of range` panic. **`v0.3.0` is defective and should not be used with timestamp-based migrations.** The fix replaces the integer-range loops in `Up()` and `Down()` with file-system source walking (the same pattern already used by `listPendingVersions`). New shared helper: `versionsInRange(dir, lo, hi, loNil)`. + +### Added + +- **`migrations-ci.yml` GHA workflow** — runs `workflow-migrate up` + `down` against ephemeral Postgres:16 on every PR touching migration driver code, using timestamp-based fixture migrations. This is the prevention mechanism that would have caught v0.3.0's panic before release. +- **`.golangci.yml`** — adds golangci-lint v2 config (previously absent) with errcheck exclusions for idiomatic `defer .Close()` patterns. + ## [0.3.0] - 2026-04-24 ### Added diff --git a/cmd/workflow-plugin-atlas-migrate/main.go b/cmd/workflow-plugin-atlas-migrate/main.go index 05692c6..fe62154 100644 --- a/cmd/workflow-plugin-atlas-migrate/main.go +++ b/cmd/workflow-plugin-atlas-migrate/main.go @@ -10,7 +10,7 @@ import ( ) // version is set at build time via -X main.version=. -var version string +var version string //nolint:unused func main() { sdk.Serve(atlasplugin.NewPlugin()) diff --git a/cmd/workflow-plugin-migrations/main.go b/cmd/workflow-plugin-migrations/main.go index 29f00db..9f65988 100644 --- a/cmd/workflow-plugin-migrations/main.go +++ b/cmd/workflow-plugin-migrations/main.go @@ -14,7 +14,7 @@ import ( ) // version is set at build time via -X main.version=. -var version string +var version string //nolint:unused func main() { sdk.ServePluginFull(internal.NewPlugin(), cli.NewCLIProvider(), nil) diff --git a/internal/atlas/revisions.go b/internal/atlas/revisions.go index a713fa3..b8a272f 100644 --- a/internal/atlas/revisions.go +++ b/internal/atlas/revisions.go @@ -64,7 +64,7 @@ func (r *sqlRevisionRW) ReadRevisions(ctx context.Context) ([]*atlmigrate.Revisi if err != nil { return nil, err } - defer rows.Close() + defer rows.Close() //nolint:errcheck return scanRevisions(rows) } diff --git a/internal/golangmigrate/driver.go b/internal/golangmigrate/driver.go index fa9e8e7..e9c9ad7 100644 --- a/internal/golangmigrate/driver.go +++ b/internal/golangmigrate/driver.go @@ -5,6 +5,8 @@ import ( "context" "errors" "fmt" + "log" + "os" "strings" "time" @@ -34,17 +36,33 @@ func (d *Driver) Up(ctx context.Context, req interfaces.MigrationRequest) (inter if err != nil { return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate: %w", err) } - defer m.Close() + defer m.Close() //nolint:errcheck - // Capture the current version before applying. - before, _, _ := m.Version() + // Capture the version before applying; fail fast if the DB is unavailable. + before, _, beforeErr := m.Version() + if beforeErr != nil && !errors.Is(beforeErr, migrate.ErrNilVersion) { + return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate: version before up: %w", beforeErr) + } + atNilBefore := errors.Is(beforeErr, migrate.ErrNilVersion) if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) { return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate up: %w", err) } - after, _, _ := m.Version() - applied := collectApplied(before, after) + after, _, afterErr := m.Version() + if afterErr != nil && !errors.Is(afterErr, migrate.ErrNilVersion) { + return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate: version after up: %w", afterErr) + } + + // Walk the source directory to enumerate applied versions; this is safe + // for timestamp-based version numbers where after-before can be 10^13. + applied, err := versionsInRange(req.Source.Dir, before, after, atNilBefore) + if err != nil { + // Migration succeeded but we can't enumerate applied versions — log and + // return partial info rather than hiding the applied state entirely. + log.Printf("warn: golang-migrate: applied version enumeration failed: %v", err) + applied = nil + } return interfaces.MigrationResult{ Applied: applied, @@ -62,31 +80,41 @@ func (d *Driver) Down(ctx context.Context, req interfaces.MigrationRequest) (int if err != nil { return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate: %w", err) } - defer m.Close() + defer m.Close() //nolint:errcheck steps := req.Options.Steps if steps <= 0 { steps = 1 } - before, _, _ := m.Version() + before, _, beforeErr := m.Version() + if beforeErr != nil && !errors.Is(beforeErr, migrate.ErrNilVersion) { + return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate: version before down: %w", beforeErr) + } if err := m.Steps(-steps); err != nil && !errors.Is(err, migrate.ErrNoChange) { return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate down: %w", err) } after, _, afterErr := m.Version() + if afterErr != nil && !errors.Is(afterErr, migrate.ErrNilVersion) { + return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate: version after down: %w", afterErr) + } // Build list of rolled-back version strings (highest to lowest). - // If we're back at nil version (ErrNilVersion), treat after as 0. + // Walk the source directory instead of integer-range loops — safe for + // timestamp-based version numbers where before-after can be 10^13. var rolledBack []string - if errors.Is(afterErr, migrate.ErrNilVersion) { - for v := uint(1); v <= before; v++ { - rolledBack = append(rolledBack, fmt.Sprintf("%d", v)) + afterNil := errors.Is(afterErr, migrate.ErrNilVersion) + if afterNil || after < before { + // versionsInRange returns versions in (after, before] ascending order; + // we reverse to produce highest-first (the order rolled back). + ascending, err := versionsInRange(req.Source.Dir, after, before, afterNil) + if err != nil { + log.Printf("warn: golang-migrate: rolled-back version enumeration failed: %v", err) } - } else if after < before { - for v := after + 1; v <= before; v++ { - rolledBack = append(rolledBack, fmt.Sprintf("%d", v)) + for i := len(ascending) - 1; i >= 0; i-- { + rolledBack = append(rolledBack, ascending[i]) } } // If after >= before, nothing was rolled back — return empty slice. @@ -106,7 +134,7 @@ func (d *Driver) Status(_ context.Context, req interfaces.MigrationRequest) (int if err != nil { return interfaces.MigrationStatus{}, fmt.Errorf("golang-migrate: %w", err) } - defer m.Close() + defer m.Close() //nolint:errcheck version, dirty, err := m.Version() if err != nil && !errors.Is(err, migrate.ErrNilVersion) { @@ -139,7 +167,7 @@ func (d *Driver) Goto(_ context.Context, req interfaces.MigrationRequest, target if err != nil { return interfaces.MigrationResult{}, fmt.Errorf("golang-migrate: %w", err) } - defer m.Close() + defer m.Close() //nolint:errcheck var version uint if _, err := fmt.Sscanf(target, "%d", &version); err != nil { @@ -173,17 +201,37 @@ func newMigrate(req interfaces.MigrationRequest) (*migrate.Migrate, error) { return migrate.New(sourceURL, dsn) } -// collectApplied returns a list of version strings between before (exclusive) -// and after (inclusive). If the versions are equal, returns empty. -func collectApplied(before, after uint) []string { - if after <= before { - return nil +// versionsInRange opens the file source and returns version strings v where +// lo < v <= hi (exclusive lo, inclusive hi), in ascending order. +// When loNil is true all versions v <= hi qualify (DB had no prior state). +// This replaces the deleted collectApplied() which assumed sequential version +// numbers and would panic on timestamp-based versions (e.g. 20240101000001). +// +// End-of-stream is signalled by os.ErrNotExist per the golang-migrate source.Driver +// contract; any other error from Next() is returned to the caller. +func versionsInRange(dir string, lo, hi uint, loNil bool) ([]string, error) { + src := &migratefile.File{} + s, err := src.Open("file://" + dir) + if err != nil { + return nil, fmt.Errorf("golang-migrate: open source for version range: %w", err) } - applied := make([]string, 0, after-before) - for v := before + 1; v <= after; v++ { - applied = append(applied, fmt.Sprintf("%d", v)) + defer s.Close() //nolint:errcheck + + var result []string + v, err := s.First() + for { + if err != nil { + if errors.Is(err, os.ErrNotExist) { + break // normal end of stream + } + return result, fmt.Errorf("golang-migrate: iterate source: %w", err) + } + if (loNil || v > lo) && v <= hi { + result = append(result, fmt.Sprintf("%d", v)) + } + v, err = s.Next(v) } - return applied + return result, nil } // listPendingVersions opens the file source and returns the version strings of @@ -195,11 +243,17 @@ func listPendingVersions(dir string, current uint, atNil bool) ([]string, error) if err != nil { return nil, fmt.Errorf("golang-migrate: open source for pending list: %w", err) } - defer s.Close() + defer s.Close() //nolint:errcheck var pending []string v, err := s.First() - for err == nil { + for { + if err != nil { + if errors.Is(err, os.ErrNotExist) { + break // normal end of stream + } + return pending, fmt.Errorf("golang-migrate: iterate source for pending: %w", err) + } if atNil || v > current { pending = append(pending, fmt.Sprintf("%d", v)) } diff --git a/internal/golangmigrate/driver_timestamp_test.go b/internal/golangmigrate/driver_timestamp_test.go new file mode 100644 index 0000000..3117f60 --- /dev/null +++ b/internal/golangmigrate/driver_timestamp_test.go @@ -0,0 +1,212 @@ +package golangmigrate_test + +// TestDriver_Up_TimestampVersions and TestDriver_Down_TimestampVersions are +// regression tests for the v0.3.0 panic in collectApplied() when migration +// versions are timestamp-based (e.g. 20240101000001) rather than sequential +// (1, 2, 3). +// +// Root cause: collectApplied(before=0, after=20240101000001) called +// make([]string, 0, after-before) with cap = 2×10¹³ +// which panics: "makeslice: cap out of range". +// +// The Down() path had an equivalent bug with integer-range loops. +// +// These tests MUST PANIC on the pre-fix code and PASS after the fix. + +import ( + "context" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" + + "github.com/GoCodeAlone/workflow-plugin-migrations/internal/golangmigrate" + "github.com/GoCodeAlone/workflow-plugin-migrations/pkg/testharness" +) + +// timestampMigrations writes three pairs of up/down SQL files with +// BMW-style timestamp-based version numbers. +func timestampMigrations(t *testing.T) string { + t.Helper() + dir := t.TempDir() + + writeSQL(t, dir, "20240101000001_init.up.sql", + "CREATE TABLE users (id SERIAL PRIMARY KEY, email TEXT UNIQUE NOT NULL);") + writeSQL(t, dir, "20240101000001_init.down.sql", + "DROP TABLE IF EXISTS users;") + + writeSQL(t, dir, "20240201000001_posts.up.sql", + "CREATE TABLE posts (id SERIAL PRIMARY KEY, user_id INT);") + writeSQL(t, dir, "20240201000001_posts.down.sql", + "DROP TABLE IF EXISTS posts;") + + writeSQL(t, dir, "20240301000001_comments.up.sql", + "CREATE TABLE comments (id SERIAL PRIMARY KEY, post_id INT, body TEXT);") + writeSQL(t, dir, "20240301000001_comments.down.sql", + "DROP TABLE IF EXISTS comments;") + + return dir +} + +// TestDriver_Up_TimestampVersions exercises Up() with timestamp-based migration +// versions — the exact scenario that triggered the v0.3.0 production panic. +// +// Pre-fix: panics with "makeslice: cap out of range" in collectApplied(). +// Post-fix: applies 3 migrations, returns 3 version strings. +func TestDriver_Up_TimestampVersions(t *testing.T) { + if testing.Short() { + t.Skip("skipping: requires postgres (set up with testharness)") + } + h, err := testharness.New() + if err != nil { + t.Skipf("skipping: no postgres available: %v", err) + } + defer h.Close(t) + + dir := timestampMigrations(t) + + ctx := context.Background() + d := golangmigrate.New() + req := interfaces.MigrationRequest{ + DSN: h.DSN(), + Source: interfaces.MigrationSource{Dir: dir}, + } + + // Up: apply all 3 timestamp-versioned migrations. + // PRE-FIX: panics with "makeslice: cap out of range". + // POST-FIX: returns 3 applied version strings. + result, err := d.Up(ctx, req) + if err != nil { + t.Fatalf("Up() error: %v", err) + } + + // Verify we got back meaningful applied versions (not an empty list). + if len(result.Applied) == 0 { + t.Error("Up(): expected applied migrations, got none") + } + + // Each applied version string should be a timestamp-format number. + for _, v := range result.Applied { + if len(v) < 10 { + t.Errorf("Up(): applied version %q looks too short — expected timestamp format like 20240101000001", v) + } + } + + // Status should reflect the highest version applied. + st, err := d.Status(ctx, req) + if err != nil { + t.Fatalf("Status() error: %v", err) + } + if st.Current == "" { + t.Error("Status().Current is empty after Up()") + } + if st.Current != "20240301000001" { + t.Errorf("Status().Current = %q; want %q", st.Current, "20240301000001") + } + if len(st.Pending) != 0 { + t.Errorf("Status().Pending = %v; want empty after full Up()", st.Pending) + } + + t.Logf("Up() applied: %v", result.Applied) + t.Logf("Status after Up: current=%s pending=%v", st.Current, st.Pending) +} + +// TestDriver_Down_TimestampVersions exercises Down() with timestamp-based migration +// versions — the rollback equivalent of the v0.3.0 panic. +// +// Pre-fix: the integer-range loops at Down() lines 83-91 would iterate +// 2×10¹³ times, effectively hanging or OOM'ing before the makeslice panic. +// Post-fix: rolls back 1 migration, returns the rolled-back version string. +func TestDriver_Down_TimestampVersions(t *testing.T) { + if testing.Short() { + t.Skip("skipping: requires postgres (set up with testharness)") + } + h, err := testharness.New() + if err != nil { + t.Skipf("skipping: no postgres available: %v", err) + } + defer h.Close(t) + + dir := timestampMigrations(t) + + ctx := context.Background() + d := golangmigrate.New() + req := interfaces.MigrationRequest{ + DSN: h.DSN(), + Source: interfaces.MigrationSource{Dir: dir}, + } + + // Apply all migrations first (Up path must work for Down to be testable). + if _, err := d.Up(ctx, req); err != nil { + t.Fatalf("Up() prerequisite error: %v", err) + } + + // Down: roll back 1 step. + // PRE-FIX: hangs or panics with integer-range overflow. + // POST-FIX: returns rolled-back version string. + req.Options.Steps = 1 + result, err := d.Down(ctx, req) + if err != nil { + t.Fatalf("Down() error: %v", err) + } + + if len(result.Applied) == 0 { + t.Error("Down(): expected at least one rolled-back version, got none") + } + + // The rolled-back version should be the highest one (20240301000001). + if result.Applied[0] != "20240301000001" { + t.Errorf("Down(): rolled-back version = %q; want %q", result.Applied[0], "20240301000001") + } + + // Status should now show 20240201000001 as current. + st, err := d.Status(ctx, req) + if err != nil { + t.Fatalf("Status() after Down() error: %v", err) + } + if st.Current != "20240201000001" { + t.Errorf("Status().Current after Down() = %q; want %q", st.Current, "20240201000001") + } + + t.Logf("Down() rolled back: %v", result.Applied) + t.Logf("Status after Down: current=%s pending=%v", st.Current, st.Pending) +} + +// TestDriver_Down_TimestampVersions_ToNil exercises Down() rolling back all +// migrations from a timestamp-versioned DB back to nil (clean state). +func TestDriver_Down_TimestampVersions_ToNil(t *testing.T) { + if testing.Short() { + t.Skip("skipping: requires postgres (set up with testharness)") + } + h, err := testharness.New() + if err != nil { + t.Skipf("skipping: no postgres available: %v", err) + } + defer h.Close(t) + + dir := timestampMigrations(t) + + ctx := context.Background() + d := golangmigrate.New() + req := interfaces.MigrationRequest{ + DSN: h.DSN(), + Source: interfaces.MigrationSource{Dir: dir}, + } + + // Apply all. + if _, err := d.Up(ctx, req); err != nil { + t.Fatalf("Up() prerequisite error: %v", err) + } + + // Roll back all 3 migrations. + req.Options.Steps = 3 + result, err := d.Down(ctx, req) + if err != nil { + t.Fatalf("Down(steps=3) error: %v", err) + } + + if len(result.Applied) == 0 { + t.Error("Down(steps=3): expected rolled-back versions, got none") + } + + t.Logf("Down(steps=3) rolled back: %v", result.Applied) +} diff --git a/internal/goose/driver.go b/internal/goose/driver.go index 647bba0..e8df905 100644 --- a/internal/goose/driver.go +++ b/internal/goose/driver.go @@ -34,7 +34,7 @@ func (d *Driver) Up(ctx context.Context, req interfaces.MigrationRequest) (inter if err != nil { return interfaces.MigrationResult{}, err } - defer db.Close() + defer db.Close() //nolint:errcheck provider, err := newProvider(db, req) if err != nil { @@ -74,7 +74,7 @@ func (d *Driver) Down(ctx context.Context, req interfaces.MigrationRequest) (int if err != nil { return interfaces.MigrationResult{}, err } - defer db.Close() + defer db.Close() //nolint:errcheck provider, err := newProvider(db, req) if err != nil { @@ -108,7 +108,7 @@ func (d *Driver) Status(ctx context.Context, req interfaces.MigrationRequest) (i if err != nil { return interfaces.MigrationStatus{}, err } - defer db.Close() + defer db.Close() //nolint:errcheck provider, err := newProvider(db, req) if err != nil { @@ -153,7 +153,7 @@ func (d *Driver) Goto(ctx context.Context, req interfaces.MigrationRequest, targ if err != nil { return interfaces.MigrationResult{}, err } - defer db.Close() + defer db.Close() //nolint:errcheck provider, err := newProvider(db, req) if err != nil { diff --git a/pkg/cli/tenant_ensure.go b/pkg/cli/tenant_ensure.go index 2b86c62..71271a6 100644 --- a/pkg/cli/tenant_ensure.go +++ b/pkg/cli/tenant_ensure.go @@ -36,7 +36,7 @@ has its own schema.`, if err != nil { return fmt.Errorf("tenant-ensure: open db: %w", err) } - defer db.Close() + defer db.Close() //nolint:errcheck if _, err = db.ExecContext(context.Background(), fmt.Sprintf(`CREATE SCHEMA IF NOT EXISTS %q`, schema)); err != nil { diff --git a/pkg/cli/test.go b/pkg/cli/test.go index 4e50fe5..fe1fc33 100644 --- a/pkg/cli/test.go +++ b/pkg/cli/test.go @@ -35,7 +35,7 @@ integration test setup (the calling process then uses that DB).`, if err := runner.Setup(ctx); err != nil { return fmt.Errorf("migrate test --keep-alive: %w", err) } - fmt.Fprintln(os.Stdout, req.DSN) + fmt.Fprintln(os.Stdout, req.DSN) //nolint:errcheck return nil }