Repository: lightningnetwork/lnd
Pull request #5550 (2021-07-21 10:35:50)

kvdb: run channeldb test suite on postgres
Extend test coverage by running the existing channeldb test suite on the postgres backend. This also includes a modest refactoring of the postgres test fixture to allow the same postgres test instance to be reused, each time with a fresh database. The channeldb tests concurrently instantiate countless databases and it wouldn't be feasible to spin up a new postgres instance on a unique port for each test.

Roasbeef: Can be rebased now!
bhandras: My only real comment is the Start/Stop Oliver mentioned other than that PR looks good.

Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 6f35225ab..81331b740 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -206,7 +206,8 @@ jobs:
       matrix:
         unit_type:
           - btcd unit-cover
-          - unit tags="kvdb_etcd kvdb_postgres"
+          - unit tags="kvdb_etcd"
+          - unit tags="kvdb_postgres"
           - travis-race
     steps:
       - name: git checkout
diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md
index eb013ee94..4e782686e 100644
--- a/docs/release-notes/release-notes-0.14.0.md
+++ b/docs/release-notes/release-notes-0.14.0.md
@@ -460,6 +460,8 @@ messages directly. There is no routing/path finding involved.
 
 * [Fixed flake that occurred with the switch dust forwarding test under the data race unit build.](https://github.com/lightningnetwork/lnd/pull/5828)
 
+* [Run channeldb tests on postgres](https://github.com/lightningnetwork/lnd/pull/5550)
+
 ## Database
 
 * [Ensure single writer for legacy
diff --git a/kvdb/backend.go b/kvdb/backend.go
index 014260b2f..7d4cbc36f 100644
--- a/kvdb/backend.go
+++ b/kvdb/backend.go
@@ -5,7 +5,9 @@ package kvdb
 
 import (
 	"context"
+	"crypto/sha256"
 	"encoding/binary"
+	"encoding/hex"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -245,7 +247,20 @@ func updateLastCompactionDate(dbFile string) error {
 func GetTestBackend(path, name string) (Backend, func(), error) {
 	empty := func() {}
 
-	if TestBackend == BoltBackendName {
+	switch {
+	case PostgresBackend:
+		key := filepath.Join(path, name)
+		keyHash := sha256.Sum256([]byte(key))
+
+		f, err := NewPostgresFixture("test_" + hex.EncodeToString(keyHash[:]))
+		if err != nil {
+			return nil, func() {}, err
+		}
+		return f.DB(), func() {
+			_ = f.DB().Close()
+		}, nil
+
+	case TestBackend == BoltBackendName:
 		db, err := GetBoltBackend(&BoltBackendConfig{
 			DBPath:         path,
 			DBFileName:     name,
@@ -256,7 +271,8 @@ func GetTestBackend(path, name string) (Backend, func(), error) {
 			return nil, nil, err
 		}
 		return db, empty, nil
-	} else if TestBackend == EtcdBackendName {
+
+	case TestBackend == EtcdBackendName:
 		etcdConfig, cancel, err := StartEtcdTestBackend(path, 0, 0, "")
 		if err != nil {
 			return nil, empty, err
@@ -265,6 +281,7 @@ func GetTestBackend(path, name string) (Backend, func(), error) {
 			EtcdBackendName, context.TODO(), etcdConfig,
 		)
 		return backend, cancel, err
+
 	}
 
 	return nil, nil, fmt.Errorf("unknown backend")
diff --git a/kvdb/kvdb_no_postgres.go b/kvdb/kvdb_no_postgres.go
new file mode 100644
index 000000000..d581c2b55
--- /dev/null
+++ b/kvdb/kvdb_no_postgres.go
@@ -0,0 +1,19 @@
+// +build !kvdb_postgres
+
+package kvdb
+
+import (
+	"errors"
+
+	"github.com/lightningnetwork/lnd/kvdb/postgres"
+)
+
+const PostgresBackend = false
+
+func NewPostgresFixture(dbName string) (postgres.Fixture, error) {
+	return nil, errors.New("postgres backend not available")
+}
+
+func StartEmbeddedPostgres() (func() error, error) {
+	return nil, errors.New("postgres backend not available")
+}
diff --git a/kvdb/kvdb_postgres.go b/kvdb/kvdb_postgres.go
new file mode 100644
index 000000000..023a51840
--- /dev/null
+++ b/kvdb/kvdb_postgres.go
@@ -0,0 +1,15 @@
+// +build kvdb_postgres
+
+package kvdb
+
+import "github.com/lightningnetwork/lnd/kvdb/postgres"
+
+const PostgresBackend = true
+
+func NewPostgresFixture(dbName string) (postgres.Fixture, error) {
+	return postgres.NewFixture(dbName)
+}
+
+func StartEmbeddedPostgres() (func() error, error) {
+	return postgres.StartEmbeddedPostgres()
+}
diff --git a/kvdb/postgres/db.go b/kvdb/postgres/db.go
index 0db46b6df..3276fb557 100644
--- a/kvdb/postgres/db.go
+++ b/kvdb/postgres/db.go
@@ -151,8 +151,15 @@ func (db *db) getPrefixedTableName(table string) string {
 func catchPanic(f func() error) (err error) {
 	defer func() {
 		if r := recover(); r != nil {
-			err = r.(error)
-			log.Criticalf("Caught unhandled error: %v", err)
+			log.Criticalf("Caught unhandled error: %v", r)
+
+			switch data := r.(type) {
+			case error:
+				err = data
+
+			default:
+				err = errors.New(fmt.Sprintf("%v", data))
+			}
 		}
 	}()
 
diff --git a/kvdb/postgres/fixture.go b/kvdb/postgres/fixture.go
index 8b3ecae73..9257f49aa 100644
--- a/kvdb/postgres/fixture.go
+++ b/kvdb/postgres/fixture.go
@@ -4,87 +4,122 @@ package postgres
 
 import (
 	"context"
+	"crypto/rand"
 	"database/sql"
-	"testing"
+	"encoding/hex"
+	"fmt"
+	"strings"
 
 	"github.com/btcsuite/btcwallet/walletdb"
 	embeddedpostgres "github.com/fergusstrange/embedded-postgres"
-	"github.com/stretchr/testify/require"
 )
 
 const (
-	testDsn = "postgres://postgres:postgres@localhost:9876/postgres?sslmode=disable"
-	prefix  = "test"
+	testDsnTemplate = "postgres://postgres:postgres@localhost:9876/%v?sslmode=disable"
+	prefix          = "test"
 )
 
-func clearTestDb(t *testing.T) {
-	dbConn, err := sql.Open("pgx", testDsn)
-	require.NoError(t, err)
+func getTestDsn(dbName string) string {
+	return fmt.Sprintf(testDsnTemplate, dbName)
+}
+
+var testPostgres *embeddedpostgres.EmbeddedPostgres
+
+// StartEmbeddedPostgres starts an embedded postgres instance. This only needs
+// to be done once, because NewFixture will create random new databases on every
+// call. It returns a stop closure that stops the database if called.
+func StartEmbeddedPostgres() (func() error, error) {
+	postgres := embeddedpostgres.NewDatabase(
+		embeddedpostgres.DefaultConfig().
+			Port(9876))
+
+	err := postgres.Start()
+	if err != nil {
+		return nil, err
+	}
+
+	testPostgres = postgres
 
-	_, err = dbConn.ExecContext(context.Background(), "DROP SCHEMA IF EXISTS public CASCADE;")
-	require.NoError(t, err)
+	return testPostgres.Stop, nil
 }
 
-func openTestDb(t *testing.T) *db {
-	clearTestDb(t)
+// NewFixture returns a new postgres test database. The database name is
+// randomly generated.
+func NewFixture(dbName string) (*fixture, error) {
+	if dbName == "" {
+		// Create random database name.
+		randBytes := make([]byte, 8)
+		_, err := rand.Read(randBytes)
+		if err != nil {
+			return nil, err
+		}
+
+		dbName = "test_" + hex.EncodeToString(randBytes)
+	}
+
+	// Create database if it doesn't exist yet.
+	dbConn, err := sql.Open("pgx", getTestDsn("postgres"))
+	if err != nil {
+		return nil, err
+	}
+	defer dbConn.Close()
 
+	_, err = dbConn.ExecContext(
+		context.Background(), "CREATE DATABASE "+dbName,
+	)
+	if err != nil && !strings.Contains(err.Error(), "already exists") {
+		return nil, err
+	}
+
+	// Open database
+	dsn := getTestDsn(dbName)
 	db, err := newPostgresBackend(
 		context.Background(),
 		&Config{
-			Dsn: testDsn,
+			Dsn: dsn,
 		},
 		prefix,
 	)
-	require.NoError(t, err)
-
-	return db
-}
-
-type fixture struct {
-	t        *testing.T
-	tempDir  string
-	postgres *embeddedpostgres.EmbeddedPostgres
-}
-
-func NewFixture(t *testing.T) *fixture {
-	postgres := embeddedpostgres.NewDatabase(
-		embeddedpostgres.DefaultConfig().
-			Port(9876))
-
-	err := postgres.Start()
-	require.NoError(t, err)
+	if err != nil {
+		return nil, err
+	}
 
 	return &fixture{
-		t:        t,
-		postgres: postgres,
-	}
+		Dsn: dsn,
+		Db:  db,
+	}, nil
 }
 
-func (b *fixture) Cleanup() {
-	b.postgres.Stop()
+type fixture struct {
+	Dsn string
+	Db  walletdb.DB
 }
 
-func (b *fixture) NewBackend() walletdb.DB {
-	clearTestDb(b.t)
-	db := openTestDb(b.t)
-
-	return db
+func (b *fixture) DB() walletdb.DB {
+	return b.Db
 }
 
-func (b *fixture) Dump() map[string]interface{} {
-	dbConn, err := sql.Open("pgx", testDsn)
-	require.NoError(b.t, err)
+// Dump returns the raw contents of the database.
+func (b *fixture) Dump() (map[string]interface{}, error) {
+	dbConn, err := sql.Open("pgx", b.Dsn)
+	if err != nil {
+		return nil, err
+	}
 
 	rows, err := dbConn.Query(
 		"SELECT tablename FROM pg_catalog.pg_tables WHERE schemaname='public'",
 	)
-	require.NoError(b.t, err)
+	if err != nil {
+		return nil, err
+	}
 
 	var tables []string
 	for rows.Next() {
 		var table string
 		err := rows.Scan(&table)
-		require.NoError(b.t, err)
+		if err != nil {
+			return nil, err
+		}
 
 		tables = append(tables, table)
 	}
@@ -93,10 +128,14 @@ func (b *fixture) Dump() map[string]interface{} {
 
 	for _, table := range tables {
 		rows, err := dbConn.Query("SELECT * FROM " + table)
-		require.NoError(b.t, err)
+		if err != nil {
+			return nil, err
+		}
 
 		cols, err := rows.Columns()
-		require.NoError(b.t, err)
+		if err != nil {
+			return nil, err
+		}
 		colCount := len(cols)
 
 		var tableRows []map[string]interface{}
@@ -108,7 +147,9 @@ func (b *fixture) Dump() map[string]interface{} {
 			}
 
 			err := rows.Scan(valuePtrs...)
-			require.NoError(b.t, err)
+			if err != nil {
+				return nil, err
+			}
 
 			tableData := make(map[string]interface{})
 			for i, v := range values {
@@ -127,5 +168,5 @@ func (b *fixture) Dump() map[string]interface{} {
 		result[table] = tableRows
 	}
 
-	return result
+	return result, nil
 }
diff --git a/kvdb/postgres/fixture_interface.go b/kvdb/postgres/fixture_interface.go
new file mode 100644
index 000000000..7a1f8b9e9
--- /dev/null
+++ b/kvdb/postgres/fixture_interface.go
@@ -0,0 +1,8 @@
+package postgres
+
+import "github.com/btcsuite/btcwallet/walletdb"
+
+type Fixture interface {
+	DB() walletdb.DB
+	Dump() (map[string]interface{}, error)
+}


Test patch:
diff --git a/channeldb/db_test.go b/channeldb/db_test.go
index 5731c03a8..c0b9bd79c 100644
--- a/channeldb/db_test.go
+++ b/channeldb/db_test.go
@@ -25,6 +25,11 @@ import (
 func TestOpenWithCreate(t *testing.T) {
 	t.Parallel()
 
+	// Checking for db file existence is not possible with postgres.
+	if kvdb.PostgresBackend {
+		t.Skip()
+	}
+
 	// First, create a temporary directory to be used for the duration of
 	// this test.
 	tempDirName, err := ioutil.TempDir("", "channeldb")
diff --git a/channeldb/setup_test.go b/channeldb/setup_test.go
new file mode 100644
index 000000000..91d3bfb47
--- /dev/null
+++ b/channeldb/setup_test.go
@@ -0,0 +1,11 @@
+package channeldb
+
+import (
+	"testing"
+
+	"github.com/lightningnetwork/lnd/kvdb"
+)
+
+func TestMain(m *testing.M) {
+	kvdb.RunTests(m)
+}
diff --git a/contractcourt/setup_test.go b/contractcourt/setup_test.go
new file mode 100644
index 000000000..f9925ce04
--- /dev/null
+++ b/contractcourt/setup_test.go
@@ -0,0 +1,11 @@
+package contractcourt
+
+import (
+	"testing"
+
+	"github.com/lightningnetwork/lnd/kvdb"
+)
+
+func TestMain(m *testing.M) {
+	kvdb.RunTests(m)
+}
diff --git a/kvdb/postgres/db_test.go b/kvdb/postgres/db_test.go
index faf853dd2..dab6bf447 100644
--- a/kvdb/postgres/db_test.go
+++ b/kvdb/postgres/db_test.go
@@ -14,15 +14,19 @@ import (
 
 // TestInterface performs all interfaces tests for this database driver.
 func TestInterface(t *testing.T) {
-	f := NewFixture(t)
-	defer f.Cleanup()
+	stop, err := StartEmbeddedPostgres()
+	require.NoError(t, err)
+	defer stop()
+
+	f, err := NewFixture("")
+	require.NoError(t, err)
 
 	// dbType is the database type name for this driver.
 	const dbType = "postgres"
 
 	ctx := context.Background()
 	cfg := &Config{
-		Dsn: testDsn,
+		Dsn: f.Dsn,
 	}
 
 	walletdbtest.TestInterface(t, dbType, ctx, cfg, prefix)
@@ -30,17 +34,19 @@ func TestInterface(t *testing.T) {
 
 // TestPanic tests recovery from panic conditions.
 func TestPanic(t *testing.T) {
-	f := NewFixture(t)
-	defer f.Cleanup()
+	stop, err := StartEmbeddedPostgres()
+	require.NoError(t, err)
+	defer stop()
 
-	d := f.NewBackend()
+	f, err := NewFixture("")
+	require.NoError(t, err)
 
-	err := d.(*db).Update(func(tx walletdb.ReadWriteTx) error {
+	err = f.Db.(*db).Update(func(tx walletdb.ReadWriteTx) error {
 		bucket, err := tx.CreateTopLevelBucket([]byte("test"))
 		require.NoError(t, err)
 
 		// Stop database server.
-		f.Cleanup()
+		stop()
 
 		// Keep trying to get data until Get panics because the
 		// connection is lost.
diff --git a/kvdb/postgres_test.go b/kvdb/postgres_test.go
index a880ceea7..445f71b73 100644
--- a/kvdb/postgres_test.go
+++ b/kvdb/postgres_test.go
@@ -13,8 +13,9 @@ import (
 type m = map[string]interface{}
 
 func TestPostgres(t *testing.T) {
-	f := postgres.NewFixture(t)
-	defer f.Cleanup()
+	stop, err := postgres.StartEmbeddedPostgres()
+	require.NoError(t, err)
+	defer stop()
 
 	tests := []struct {
 		name       string
@@ -173,10 +174,14 @@ func TestPostgres(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			test.test(t, f.NewBackend())
+			f, err := postgres.NewFixture("")
+			require.NoError(t, err)
+
+			test.test(t, f.Db)
 
 			if test.expectedDb != nil {
-				dump := f.Dump()
+				dump, err := f.Dump()
+				require.NoError(t, err)
 				require.Equal(t, test.expectedDb, dump)
 			}
 		})
diff --git a/kvdb/test_utils.go b/kvdb/test_utils.go
new file mode 100644
index 000000000..adc1156e7
--- /dev/null
+++ b/kvdb/test_utils.go
@@ -0,0 +1,34 @@
+package kvdb
+
+import (
+	"fmt"
+	"os"
+	"testing"
+)
+
+// RunTests is a helper function to run the tests in a package with
+// initialization and tear-down of a test kvdb backend.
+func RunTests(m *testing.M) {
+	var close func() error
+	if PostgresBackend {
+		var err error
+		close, err = StartEmbeddedPostgres()
+		if err != nil {
+			fmt.Printf("Error: %v\n", err)
+			os.Exit(1)
+		}
+	}
+
+	// os.Exit() does not respect defer statements
+	code := m.Run()
+
+	if close != nil {
+		err := close()
+		if err != nil {
+			fmt.Printf("Error: %v\n", err)
+		}
+	}
+
+	os.Exit(code)
+
+}
diff --git a/routing/setup_test.go b/routing/setup_test.go
new file mode 100644
index 000000000..544fc3885
--- /dev/null
+++ b/routing/setup_test.go
@@ -0,0 +1,11 @@
+package routing
+
+import (
+	"testing"
+
+	"github.com/lightningnetwork/lnd/kvdb"
+)
+
+func TestMain(m *testing.M) {
+	kvdb.RunTests(m)
+}
diff --git a/sweep/setup_test.go b/sweep/setup_test.go
new file mode 100644
index 000000000..e2d22ce3b
--- /dev/null
+++ b/sweep/setup_test.go
@@ -0,0 +1,11 @@
+package sweep
+
+import (
+	"testing"
+
+	"github.com/lightningnetwork/lnd/kvdb"
+)
+
+func TestMain(m *testing.M) {
+	kvdb.RunTests(m)
+}