Add database namespacing for unit tests (#2340)
* Add database namespacing for unit tests Background: Running `go test ./...` will run tests in different packages concurrently. This can be stopped or limited by using `-p 1` (no concurrency). We want concurrency, but this causes problems when running Postgres DBs in CI. The problem is that, in CI, we have 1x postgres server exposing 1x postgres DB, which we wipe clean at the end of each test via `defer close()`. When tests run concurrently, calls to `close()` will delete data/tables which other tests are currently using, causing havoc. Fix this by: - Creating a database per package. - Namespacing the database name by a hash of the current working directory (the directory containing those `_test.go` files) This is exactly what SQLite does, quite unintentionally, via the use of `file:dendrite_test.db`, which dumps the file into the current working directory which is the package running the tests, hence deleting the file is safe when running concurrently. * Linting * Don't create the database in a txn * dupe db is not an error
This commit is contained in:
parent
69f2ff7c82
commit
ea92f80c12
2
.github/workflows/dendrite.yml
vendored
2
.github/workflows/dendrite.yml
vendored
|
@ -111,7 +111,7 @@ jobs:
|
||||||
key: ${{ runner.os }}-go${{ matrix.go }}-test-${{ hashFiles('**/go.sum') }}
|
key: ${{ runner.os }}-go${{ matrix.go }}-test-${{ hashFiles('**/go.sum') }}
|
||||||
restore-keys: |
|
restore-keys: |
|
||||||
${{ runner.os }}-go${{ matrix.go }}-test-
|
${{ runner.os }}-go${{ matrix.go }}-test-
|
||||||
- run: go test -p 1 ./...
|
- run: go test ./...
|
||||||
env:
|
env:
|
||||||
POSTGRES_HOST: localhost
|
POSTGRES_HOST: localhost
|
||||||
POSTGRES_USER: postgres
|
POSTGRES_USER: postgres
|
||||||
|
|
58
test/db.go
58
test/db.go
|
@ -15,12 +15,16 @@
|
||||||
package test
|
package test
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"crypto/sha256"
|
||||||
"database/sql"
|
"database/sql"
|
||||||
|
"encoding/hex"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"os/user"
|
"os/user"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/lib/pq"
|
||||||
)
|
)
|
||||||
|
|
||||||
type DBType int
|
type DBType int
|
||||||
|
@ -30,7 +34,7 @@ var DBTypePostgres DBType = 2
|
||||||
|
|
||||||
var Quiet = false
|
var Quiet = false
|
||||||
|
|
||||||
func createLocalDB(dbName string) string {
|
func createLocalDB(dbName string) {
|
||||||
if !Quiet {
|
if !Quiet {
|
||||||
fmt.Println("Note: tests require a postgres install accessible to the current user")
|
fmt.Println("Note: tests require a postgres install accessible to the current user")
|
||||||
}
|
}
|
||||||
|
@ -43,7 +47,29 @@ func createLocalDB(dbName string) string {
|
||||||
if err != nil && !Quiet {
|
if err != nil && !Quiet {
|
||||||
fmt.Println("createLocalDB returned error:", err)
|
fmt.Println("createLocalDB returned error:", err)
|
||||||
}
|
}
|
||||||
return dbName
|
}
|
||||||
|
|
||||||
|
func createRemoteDB(t *testing.T, dbName, user, connStr string) {
|
||||||
|
db, err := sql.Open("postgres", connStr+" dbname=postgres")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to open postgres conn with connstr=%s : %s", connStr, err)
|
||||||
|
}
|
||||||
|
_, err = db.Exec(fmt.Sprintf(`CREATE DATABASE %s;`, dbName))
|
||||||
|
if err != nil {
|
||||||
|
pqErr, ok := err.(*pq.Error)
|
||||||
|
if !ok {
|
||||||
|
t.Fatalf("failed to CREATE DATABASE: %s", err)
|
||||||
|
}
|
||||||
|
// we ignore duplicate database error as we expect this
|
||||||
|
if pqErr.Code != "42P04" {
|
||||||
|
t.Fatalf("failed to CREATE DATABASE with code=%s msg=%s", pqErr.Code, pqErr.Message)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_, err = db.Exec(fmt.Sprintf(`GRANT ALL PRIVILEGES ON DATABASE %s TO %s`, dbName, user))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to GRANT: %s", err)
|
||||||
|
}
|
||||||
|
_ = db.Close()
|
||||||
}
|
}
|
||||||
|
|
||||||
func currentUser() string {
|
func currentUser() string {
|
||||||
|
@ -64,6 +90,7 @@ func currentUser() string {
|
||||||
// TODO: namespace for concurrent package tests
|
// TODO: namespace for concurrent package tests
|
||||||
func PrepareDBConnectionString(t *testing.T, dbType DBType) (connStr string, close func()) {
|
func PrepareDBConnectionString(t *testing.T, dbType DBType) (connStr string, close func()) {
|
||||||
if dbType == DBTypeSQLite {
|
if dbType == DBTypeSQLite {
|
||||||
|
// this will be made in the current working directory which namespaces concurrent package runs correctly
|
||||||
dbname := "dendrite_test.db"
|
dbname := "dendrite_test.db"
|
||||||
return fmt.Sprintf("file:%s", dbname), func() {
|
return fmt.Sprintf("file:%s", dbname), func() {
|
||||||
err := os.Remove(dbname)
|
err := os.Remove(dbname)
|
||||||
|
@ -79,13 +106,9 @@ func PrepareDBConnectionString(t *testing.T, dbType DBType) (connStr string, clo
|
||||||
if user == "" {
|
if user == "" {
|
||||||
user = currentUser()
|
user = currentUser()
|
||||||
}
|
}
|
||||||
dbName := os.Getenv("POSTGRES_DB")
|
|
||||||
if dbName == "" {
|
|
||||||
dbName = createLocalDB("dendrite_test")
|
|
||||||
}
|
|
||||||
connStr = fmt.Sprintf(
|
connStr = fmt.Sprintf(
|
||||||
"user=%s dbname=%s sslmode=disable",
|
"user=%s sslmode=disable",
|
||||||
user, dbName,
|
user,
|
||||||
)
|
)
|
||||||
// optional vars, used in CI
|
// optional vars, used in CI
|
||||||
password := os.Getenv("POSTGRES_PASSWORD")
|
password := os.Getenv("POSTGRES_PASSWORD")
|
||||||
|
@ -97,6 +120,25 @@ func PrepareDBConnectionString(t *testing.T, dbType DBType) (connStr string, clo
|
||||||
connStr += fmt.Sprintf(" host=%s", host)
|
connStr += fmt.Sprintf(" host=%s", host)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// superuser database
|
||||||
|
postgresDB := os.Getenv("POSTGRES_DB")
|
||||||
|
// we cannot use 'dendrite_test' here else 2x concurrently running packages will try to use the same db.
|
||||||
|
// instead, hash the current working directory, snaffle the first 16 bytes and append that to dendrite_test
|
||||||
|
// and use that as the unique db name. We do this because packages are per-directory hence by hashing the
|
||||||
|
// working (test) directory we ensure we get a consistent hash and don't hash against concurrent packages.
|
||||||
|
wd, err := os.Getwd()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("cannot get working directory: %s", err)
|
||||||
|
}
|
||||||
|
hash := sha256.Sum256([]byte(wd))
|
||||||
|
dbName := fmt.Sprintf("dendrite_test_%s", hex.EncodeToString(hash[:16]))
|
||||||
|
if postgresDB == "" { // local server, use createdb
|
||||||
|
createLocalDB(dbName)
|
||||||
|
} else { // remote server, shell into the postgres user and CREATE DATABASE
|
||||||
|
createRemoteDB(t, dbName, user, connStr)
|
||||||
|
}
|
||||||
|
connStr += fmt.Sprintf(" dbname=%s", dbName)
|
||||||
|
|
||||||
return connStr, func() {
|
return connStr, func() {
|
||||||
// Drop all tables on the database to get a fresh instance
|
// Drop all tables on the database to get a fresh instance
|
||||||
db, err := sql.Open("postgres", connStr)
|
db, err := sql.Open("postgres", connStr)
|
||||||
|
|
Loading…
Reference in a new issue