From af3ff825a1aef7630b9dbab306f536d3c5f8b788 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Mon, 27 Oct 2025 14:31:32 +0400 Subject: [PATCH] test: track postgres database creation by package and test name (#20492) Adds columns to track package and test name to test_databases table, and populates them as databases are created using the Broker. In order to seamlessly work with existing `coder_database` databases with the old schema, the SQL that creates the table and columns is additive and idempotent, so we run it every time we initialize the Broker (once per test binary execution). We include a transaction level advisorly lock to prevent deadlocks before attempting to alter the schema. I was seeing deadlocks without this. --- Makefile | 5 ++ coderd/database/dbtestutil/broker.go | 57 +++++++++++++++++-- .../dbtestutil/broker_internal_test.go | 13 +++++ coderd/database/dbtestutil/coder_testing.sql | 10 ++++ 4 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 coderd/database/dbtestutil/broker_internal_test.go diff --git a/Makefile b/Makefile index 93b0245a8a..7f21f1fa6d 100644 --- a/Makefile +++ b/Makefile @@ -1182,3 +1182,8 @@ endif dogfood/coder/nix.hash: flake.nix flake.lock sha256sum flake.nix flake.lock >./dogfood/coder/nix.hash + +# Count the number of test databases created per test package. +count-test-databases: + PGPASSWORD=postgres psql -h localhost -U postgres -d coder_testing -P pager=off -c 'SELECT test_package, count(*) as count from test_databases GROUP BY test_package ORDER BY count DESC' +.PHONY: count-test-databases diff --git a/coderd/database/dbtestutil/broker.go b/coderd/database/dbtestutil/broker.go index 8a0d47b7b1..158a44cbc9 100644 --- a/coderd/database/dbtestutil/broker.go +++ b/coderd/database/dbtestutil/broker.go @@ -6,6 +6,8 @@ import ( _ "embed" "fmt" "os" + "runtime" + "strings" "sync" "time" @@ -45,6 +47,8 @@ func (b *Broker) Create(t TBSubset, opts ...OpenOption) (ConnectionParams, error host = defaultConnectionParams.Host port = defaultConnectionParams.Port ) + packageName := getTestPackageName(t) + testName := t.Name() // Use a time-based prefix to make it easier to find the database // when debugging. @@ -55,9 +59,9 @@ func (b *Broker) Create(t TBSubset, opts ...OpenOption) (ConnectionParams, error } dbName := now + "_" + dbSuffix - // TODO: add package and test name _, err = b.coderTestingDB.Exec( - "INSERT INTO test_databases (name, process_uuid) VALUES ($1, $2)", dbName, b.uuid) + "INSERT INTO test_databases (name, process_uuid, test_package, test_name) VALUES ($1, $2, $3, $4)", + dbName, b.uuid, packageName, testName) if err != nil { return ConnectionParams{}, xerrors.Errorf("insert test_database row: %w", err) } @@ -104,10 +108,10 @@ func (b *Broker) clean(t TBSubset, dbName string) func() { func (b *Broker) init(t TBSubset) error { b.Lock() defer b.Unlock() - b.refCount++ - t.Cleanup(b.decRef) if b.coderTestingDB != nil { // already initialized + b.refCount++ + t.Cleanup(b.decRef) return nil } @@ -124,8 +128,8 @@ func (b *Broker) init(t TBSubset) error { return xerrors.Errorf("open postgres connection: %w", err) } - // creating the db can succeed even if the database doesn't exist. Ping it to find out. - err = coderTestingDB.Ping() + // coderTestingSQLInit is idempotent, so we can run it every time. + _, err = coderTestingDB.Exec(coderTestingSQLInit) var pqErr *pq.Error if xerrors.As(err, &pqErr) && pqErr.Code == "3D000" { // database does not exist. @@ -145,6 +149,8 @@ func (b *Broker) init(t TBSubset) error { return xerrors.Errorf("ping '%s' database: %w", CoderTestingDBName, err) } b.coderTestingDB = coderTestingDB + b.refCount++ + t.Cleanup(b.decRef) if b.uuid == uuid.Nil { b.uuid = uuid.New() @@ -186,3 +192,42 @@ func (b *Broker) decRef() { b.coderTestingDB = nil } } + +// getTestPackageName returns the package name of the test that called it. +func getTestPackageName(t TBSubset) string { + packageName := "unknown" + // Ask runtime.Callers for up to 100 program counters, including runtime.Callers itself. + pc := make([]uintptr, 100) + n := runtime.Callers(0, pc) + if n == 0 { + // No PCs available. This can happen if the first argument to + // runtime.Callers is large. + // + // Return now to avoid processing the zero Frame that would + // otherwise be returned by frames.Next below. + t.Logf("could not determine test package name: no PCs available") + return packageName + } + + pc = pc[:n] // pass only valid pcs to runtime.CallersFrames + frames := runtime.CallersFrames(pc) + + // Loop to get frames. + // A fixed number of PCs can expand to an indefinite number of Frames. + for { + frame, more := frames.Next() + + if strings.HasPrefix(frame.Function, "github.com/coder/coder/v2/") { + packageName = strings.SplitN(strings.TrimPrefix(frame.Function, "github.com/coder/coder/v2/"), ".", 2)[0] + } + if strings.HasPrefix(frame.Function, "testing") { + break + } + + // Check whether there are more frames to process after this one. + if !more { + break + } + } + return packageName +} diff --git a/coderd/database/dbtestutil/broker_internal_test.go b/coderd/database/dbtestutil/broker_internal_test.go new file mode 100644 index 0000000000..944ae2a477 --- /dev/null +++ b/coderd/database/dbtestutil/broker_internal_test.go @@ -0,0 +1,13 @@ +package dbtestutil + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGetTestPackageName(t *testing.T) { + t.Parallel() + packageName := getTestPackageName(t) + require.Equal(t, "coderd/database/dbtestutil", packageName) +} diff --git a/coderd/database/dbtestutil/coder_testing.sql b/coderd/database/dbtestutil/coder_testing.sql index edaab486c8..453b38d2d4 100644 --- a/coderd/database/dbtestutil/coder_testing.sql +++ b/coderd/database/dbtestutil/coder_testing.sql @@ -1,3 +1,6 @@ +BEGIN TRANSACTION; +SELECT pg_advisory_xact_lock(7283699); + CREATE TABLE IF NOT EXISTS test_databases ( name text PRIMARY KEY, created_at timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP, @@ -6,3 +9,10 @@ CREATE TABLE IF NOT EXISTS test_databases ( ); CREATE INDEX IF NOT EXISTS test_databases_process_uuid ON test_databases (process_uuid, dropped_at); + +ALTER TABLE test_databases ADD COLUMN IF NOT EXISTS test_name text; +COMMENT ON COLUMN test_databases.test_name IS 'Name of the test that created the database'; +ALTER TABLE test_databases ADD COLUMN IF NOT EXISTS test_package text; +COMMENT ON COLUMN test_databases.test_package IS 'Package of the test that created the database'; + +COMMIT;