Fix migrations in github pipeline

This commit is contained in:
Jonathon Leight
2026-08-09 17:20:02 -04:00
parent 8dddc6ceb0
commit 76f7b6af30
2 changed files with 27 additions and 2 deletions
+9 -2
View File
@@ -21,6 +21,13 @@ import (
// both tests below "migrate" an already-migrated database, which proves nothing:
// Provider.Up returns early when nothing is pending and never touches the lock. (That is
// exactly why an earlier version of these tests passed with the locker removed.)
// Every testdb.Fresh call in a package MUST pass the same migrate callback: testdb
// builds its template once per process, so the first caller decides the schema every
// other test in that package clones. These tests want a bare database, and it is
// tempting to ask for one by passing a no-op callback — that used to be exactly what
// they did, and when one of them won the race the whole package cloned an EMPTY
// template and failed with "relation ... does not exist" all over. So they take the
// migrated template like everyone else and empty it here instead.
func emptySchema(t *testing.T, st *Store) {
t.Helper()
ctx := context.Background()
@@ -68,7 +75,7 @@ func TestMigrateConcurrentlyFromManyConnections(t *testing.T) {
t.Parallel()
ctx := context.Background()
dsn := testdb.Fresh(t, func(string) error { return nil })
dsn := testdb.Fresh(t, migrateTemplate) // emptied below; see the note on emptySchema
const replicas = 4
stores := make([]*Store, replicas)
@@ -145,7 +152,7 @@ func TestMigrateConcurrentlyFromManyConnections(t *testing.T) {
func TestMigrateWaitsForTheAdvisoryLock(t *testing.T) {
t.Parallel()
ctx := context.Background()
dsn := testdb.Fresh(t, func(string) error { return nil })
dsn := testdb.Fresh(t, migrateTemplate) // emptied below; see the note on emptySchema
st, err := New(ctx, dsn)
if err != nil {
+18
View File
@@ -24,6 +24,7 @@ import (
"fmt"
"net/url"
"os"
"reflect"
"sync"
"sync/atomic"
"testing"
@@ -45,6 +46,15 @@ var (
templateOnce sync.Once
templateErr error
templateName string
// templateMigrate records which migrate callback built the template, so a second
// caller passing a different one is reported instead of silently ignored. The
// template is built once per process: without this check, a test that asks for a
// DIFFERENT schema gets whatever the first caller created, and if it wins the race
// instead, every other test in the package clones ITS schema. That failure lands
// nowhere near its cause — an empty template surfaces as "relation ... does not
// exist" in unrelated tests, only sometimes, because which parallel test calls
// Fresh first is not deterministic.
templateMigrate uintptr
createMu sync.Mutex // serializes per-test CREATE DATABASE within this process
dbCounter atomic.Int64
@@ -90,6 +100,7 @@ func ensureServer(ctx context.Context) error {
// requires that no sessions are connected to the template).
func ensureTemplate(ctx context.Context, migrate func(dsn string) error) error {
templateOnce.Do(func() {
templateMigrate = reflect.ValueOf(migrate).Pointer()
templateName = fmt.Sprintf("mt_tmpl_%d", pid)
conn, err := pgx.Connect(ctx, adminDSN)
if err != nil {
@@ -131,6 +142,13 @@ func ensureTemplate(ctx context.Context, migrate func(dsn string) error) error {
templateErr = fmt.Errorf("migrate template: %w", err)
}
})
if templateErr == nil && reflect.ValueOf(migrate).Pointer() != templateMigrate {
return fmt.Errorf("the template was already built by a different migrate " +
"callback. One template is built per process, so every Fresh call in a package must " +
"pass the same one — otherwise the schema a test gets depends on which test ran " +
"first. If a test needs a different schema, take the shared template and adjust it " +
"inside the test")
}
return templateErr
}