Merge pull request #2154 from josephschorr/db-flaky-tests

Changes to address flaky DB tests

internal/datastore/mysql/datastore_test.go

@@ -92,13 +92,11 @@ func createDatastoreTest(b testdatastore.RunningEngineForTest, tf datastoreTestF
 }
 
 func TestMySQLDatastoreDSNWithoutParseTime(t *testing.T) {
-	t.Parallel()
 	_, err := NewMySQLDatastore(context.Background(), "root:password@(localhost:1234)/mysql")
 	require.ErrorContains(t, err, "https://spicedb.dev/d/parse-time-mysql")
 }
 
 func TestMySQL8Datastore(t *testing.T) {
-	t.Parallel()
 	b := testdatastore.RunMySQLForTestingWithOptions(t, testdatastore.MySQLTesterOptions{MigrateForNewDatastore: true}, "")
 	dst := datastoreTester{b: b, t: t}
 	test.AllWithExceptions(t, test.DatastoreTesterFunc(dst.createDatastore), test.WithCategories(test.WatchSchemaCategory, test.WatchCheckpointsCategory), true)
@@ -665,7 +663,6 @@ func TransactionTimestampsTest(t *testing.T, ds datastore.Datastore) {
 }
 
 func TestMySQLMigrations(t *testing.T) {
-	t.Parallel()
 	req := require.New(t)
 
 	db := datastoreDB(t, false)
@@ -687,7 +684,6 @@ func TestMySQLMigrations(t *testing.T) {
 }
 
 func TestMySQLMigrationsWithPrefix(t *testing.T) {
-	t.Parallel()
 	req := require.New(t)
 
 	prefix := "spicedb_"

internal/datastore/postgres/postgres_test.go

@@ -16,14 +16,10 @@ import (
 )
 
 func TestPostgresDatastore(t *testing.T) {
-	t.Parallel()
-
 	testPostgresDatastore(t, postgresConfigs)
 }
 
 func TestPostgresDatastoreWithoutCommitTimestamps(t *testing.T) {
-	t.Parallel()
-
 	testPostgresDatastoreWithoutCommitTimestamps(t, postgresConfigs)
 }
 

internal/testserver/datastore/crdb.go

@@ -52,6 +52,7 @@ func RunCRDBForTesting(t testing.TB, bridgeNetworkName string) RunningEngineForT
 		creds:    "root:fake",
 	}
 	t.Cleanup(func() {
+		require.NoError(t, builder.conn.Close(context.Background()))
 		require.NoError(t, pool.Purge(resource))
 	})
 

internal/testserver/datastore/mysql.go

@@ -72,6 +72,7 @@ func RunMySQLForTestingWithOptions(t testing.TB, options MySQLTesterOptions, bri
 		options: options,
 	}
 	t.Cleanup(func() {
+		require.NoError(t, builder.db.Close())
 		require.NoError(t, pool.Purge(resource))
 	})
 

internal/testserver/datastore/postgres.go

@@ -83,9 +83,6 @@ func RunPostgresForTestingWithCommitTimestamps(t testing.TB, bridgeNetworkName s
 		Cmd:          cmd,
 	})
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		require.NoError(t, pool.Purge(postgres))
-	})
 
 	builder := &postgresTester{
 		container: container{
@@ -99,6 +96,11 @@ func RunPostgresForTestingWithCommitTimestamps(t testing.TB, bridgeNetworkName s
 		useContainerHostname: bridgeSupplied,
 	}
 
+	t.Cleanup(func() {
+		require.NoError(t, builder.hostConn.Close(context.Background()))
+		require.NoError(t, pool.Purge(postgres))
+	})
+
 	if enablePgbouncer {
 		// if we are running with pgbouncer enabled then set it up
 		builder.runPgbouncerForTesting(t, pool, bridgeNetworkName)
@@ -118,6 +120,12 @@ func (b *postgresTester) NewDatabase(t testing.TB) string {
 	_, err = b.hostConn.Exec(context.Background(), "CREATE DATABASE "+newDBName)
 	require.NoError(t, err)
 
+	row := b.hostConn.QueryRow(context.Background(), "SELECT datname FROM pg_catalog.pg_database WHERE datname = $1", newDBName)
+	var dbName string
+	err = row.Scan(&dbName)
+	require.NoError(t, err)
+	require.Equal(t, newDBName, dbName)
+
 	hostname, port := b.getHostnameAndPort()
 	return fmt.Sprintf(
 		"postgres://%s@%s:%s/%s?sslmode=disable",
@@ -146,6 +154,8 @@ func (b *postgresTester) NewDatastore(t testing.TB, initFunc InitFunc) datastore
 
 		if i == retryCount {
 			require.NoError(t, err, "got error when trying to create migration driver")
+		} else {
+			t.Logf("failed to create migration driver: %v, retrying... %d", err, i)
 		}
 
 		time.Sleep(time.Duration(i) * timeBetweenRetries)