Repository navigation
fix: v0.3.1 — timestamp-version panic in collectApplied (BMW P0) #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
fa9ecbb
test(golangmigrate): add timestamp-version regression test (pre-fix: …
intel352 fec39f4
fix(golangmigrate): replace collectApplied+integer loops with version…
intel352 e784a17
ci: add migrations-ci.yml — ephemeral Postgres regression gate on eve…
intel352 42fdf01
chore: add .golangci.yml + nolint annotations for pre-existing patterns
intel352 2cd24b7
fix(golangmigrate): address 5 Copilot review findings on PR #5
intel352 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) ✓" | ||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.