From 76f7b6af30263a2a56610dd3db5ab8b026bf32f4 Mon Sep 17 00:00:00 2001
From: Jonathon Leight <jonathon.leight@jleight.com>
Date: Sun, 9 Aug 2026 17:20:02 -0400
Subject: [PATCH] Fix migrations in github pipeline

---
 internal/store/migrate_test.go | 11 +++++++++--
 internal/testdb/testdb.go      | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go
index 6c7a2de..0bfe664 100644
--- a/internal/store/migrate_test.go
+++ b/internal/store/migrate_test.go
@@ -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 {
diff --git a/internal/testdb/testdb.go b/internal/testdb/testdb.go
index 6767f22..28b5d58 100644
--- a/internal/testdb/testdb.go
+++ b/internal/testdb/testdb.go
@@ -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
 }