private/dbutil/pgtest: support multiple databases for testing

Currently Cockroach isn't performant for concurrent database setup and
tear-down. Instead of a single instance allow setting multiple potential
connection strings and let the tests pick one connection string
randomly.

This improves test duration by ~10 minutes.

While we are at significantly changing how pgtest works, introduce
helper PickPostgres and PickCockroach for selecting the database to
reduce code duplications in multiple places.

Change-Id: I8ad171d5c4c8a4fc081ec2ae9bdd0cc948a80619

Jenkinsfile.public

@@ -30,11 +30,13 @@ pipeline {
                 sh 'make -j4 build-packages'
                 sh 'make install-sim'
 
-                sh 'cockroach start-single-node --insecure --store=type=mem,size=2GiB --listen-addr=localhost:26257 --http-addr=localhost:8080 --cache 512MiB --max-sql-memory 512MiB --background'
+                sh 'cockroach start-single-node --insecure --store=type=mem,size=2GiB --listen-addr=localhost:26256 --http-addr=localhost:8086 --cache 512MiB --max-sql-memory 512MiB --background'
+                sh 'cockroach start-single-node --insecure --store=type=mem,size=2GiB --listen-addr=localhost:26257 --http-addr=localhost:8087 --cache 512MiB --max-sql-memory 512MiB --background'
+                sh 'cockroach start-single-node --insecure --store=type=mem,size=2GiB --listen-addr=localhost:26258 --http-addr=localhost:8088 --cache 512MiB --max-sql-memory 512MiB --background'
+                sh 'cockroach start-single-node --insecure --store=type=mem,size=2GiB --listen-addr=localhost:26259 --http-addr=localhost:8089 --cache 512MiB --max-sql-memory 512MiB --background'
             }
         }
 
-
         stage('Verification') {
             parallel {
                 stage('Lint') {
@@ -55,12 +57,19 @@ pipeline {
 
                 stage('Tests') {
                     environment {
-                        STORJ_COCKROACH_TEST = 'cockroach://root@localhost:26257/testcockroach?sslmode=disable'
+                        STORJ_COCKROACH_TEST = 'cockroach://root@localhost:26256/testcockroach?sslmode=disable;' +
+                            'cockroach://root@localhost:26257/testcockroach?sslmode=disable;' +
+                            'cockroach://root@localhost:26258/testcockroach?sslmode=disable;' +
+                            'cockroach://root@localhost:26259/testcockroach?sslmode=disable'
                         STORJ_POSTGRES_TEST = 'postgres://postgres@localhost/teststorj?sslmode=disable'
                         COVERFLAGS = "${ env.BRANCH_NAME != 'master' ? '' : '-coverprofile=.build/coverprofile -coverpkg=storj.io/storj/private/...,storj.io/storj/lib/...,storj.io/storj/pkg/...,storj.io/storj/satellite/...,storj.io/storj/storage/...,storj.io/storj/storagenode/...,storj.io/storj/versioncontrol/...'}"
                     }
                     steps {
+                        sh 'cockroach sql --insecure --host=localhost:26256 -e \'create database testcockroach;\''
                         sh 'cockroach sql --insecure --host=localhost:26257 -e \'create database testcockroach;\''
+                        sh 'cockroach sql --insecure --host=localhost:26258 -e \'create database testcockroach;\''
+                        sh 'cockroach sql --insecure --host=localhost:26259 -e \'create database testcockroach;\''
+
                         sh 'psql -U postgres -c \'create database teststorj;\''
                         sh 'use-ports -from 1024 -to 10000 &'
                         sh 'go test -parallel 4 -p 6 -vet=off $COVERFLAGS -timeout 20m -json -race ./... 2>&1 | tee .build/tests.json | xunit -out .build/tests.xml'
@@ -112,6 +121,7 @@ pipeline {
                     steps {
                         sh 'cockroach sql --insecure --host=localhost:26257 -e \'create database testcockroach4;\''
                         sh 'make test-sim'
+                        sh 'cockroach sql --insecure --host=localhost:26257 -e \'drop database testcockroach4;\''
                     }
                 }
 
@@ -141,6 +151,7 @@ pipeline {
                     steps {
                         sh 'cockroach sql --insecure --host=localhost:26257 -e \'create database testcockroach5;\''
                         sh 'make test-sim-backwards-compatible'
+                        sh 'cockroach sql --insecure --host=localhost:26257 -e \'drop database testcockroach5;\''
                     }
                 }
 

private/dbutil/cockroachutil/driver_test.go

@@ -11,18 +11,17 @@ import (
 	"github.com/zeebo/errs"
 
 	"storj.io/common/testcontext"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/tagsql"
 )
 
 func TestLibPqCompatibility(t *testing.T) {
+	connstr := pgtest.PickCockroach(t)
+
 	ctx := testcontext.New(t)
 	defer ctx.Cleanup()
 
-	if *pgtest.CrdbConnStr == "" {
+	testDB, err := OpenUnique(ctx, connstr, "TestLibPqCompatibility")
-		t.Skip("CockroachDB flag missing")
-	}
-	testDB, err := OpenUnique(ctx, *pgtest.CrdbConnStr, "TestLibPqCompatibility")
 	require.NoError(t, err)
 	defer ctx.Check(testDB.Close)
 

private/dbutil/cockroachutil/openunique_test.go

@@ -12,20 +12,19 @@ import (
 	"storj.io/common/testcontext"
 	"storj.io/storj/private/dbutil"
 	"storj.io/storj/private/dbutil/cockroachutil"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/tempdb"
 	"storj.io/storj/private/tagsql"
 )
 
 func TestTempCockroachDB(t *testing.T) {
+	connstr := pgtest.PickCockroach(t)
+
 	ctx := testcontext.New(t)
 	defer ctx.Cleanup()
 
-	if *pgtest.CrdbConnStr == "" {
-		t.Skip("CockroachDB flag missing")
-	}
 	prefix := "name#spaced/Test/DB"
-	testDB, err := tempdb.OpenUnique(ctx, *pgtest.CrdbConnStr, prefix)
+	testDB, err := tempdb.OpenUnique(ctx, connstr, prefix)
 	require.NoError(t, err)
 
 	require.Equal(t, "cockroach", testDB.Driver)
@@ -62,7 +61,7 @@ func TestTempCockroachDB(t *testing.T) {
 
 	// make a new connection back to the master connstr just to check that the our temp db
 	// really was dropped
-	plainDBConn, err := tagsql.Open("cockroach", *pgtest.CrdbConnStr)
+	plainDBConn, err := tagsql.Open("cockroach", connstr)
 	require.NoError(t, err)
 	defer ctx.Check(plainDBConn.Close)
 

private/dbutil/pgtest/flag.go

@@ -0,0 +1,89 @@
+// Copyright (C) 2019 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package pgtest
+
+import (
+	"flag"
+	"math/rand"
+	"os"
+	"strings"
+	"testing"
+
+	"storj.io/common/testcontext"
+)
+
+// We need to define this in a separate package due to https://golang.org/issue/23910.
+
+// postgres is the test database connection string.
+var postgres = flag.String("postgres-test-db", os.Getenv("STORJ_POSTGRES_TEST"), "PostgreSQL test database connection string (semicolon delimited for multiple)")
+
+// cockroach is the test database connection string for CockroachDB
+var cockroach = flag.String("cockroach-test-db", os.Getenv("STORJ_COCKROACH_TEST"), "CockroachDB test database connection string (semicolon delimited for multiple)")
+
+// DefaultPostgres is expected to work under the storj-test docker-compose instance
+const DefaultPostgres = "postgres://storj:storj-pass@test-postgres/teststorj?sslmode=disable"
+
+// DefaultCockroach is expected to work when a local cockroachDB instance is running
+const DefaultCockroach = "cockroach://root@localhost:26257/master?sslmode=disable"
+
+// Database defines a postgres compatible database.
+type Database struct {
+	Name string
+	// Pick picks a connection string for the database and skips when it's missing.
+	Pick func(t TB) string
+}
+
+// TB defines minimal interface required for Pick.
+type TB interface {
+	Skip(...interface{})
+}
+
+// Databases returns list of postgres compatible databases.
+func Databases() []Database {
+	return []Database{
+		{Name: "Postgres", Pick: PickPostgres},
+		{Name: "Cockroach", Pick: PickCockroach},
+	}
+}
+
+// Run runs tests with all postgres compatible databases.
+func Run(t *testing.T, test func(ctx *testcontext.Context, t *testing.T, connstr string)) {
+	for _, db := range Databases() {
+		db := db
+		t.Run(db.Name, func(t *testing.T) {
+			connstr := db.Pick(t)
+
+			t.Parallel()
+
+			ctx := testcontext.New(t)
+			defer ctx.Cleanup()
+
+			test(ctx, t, connstr)
+		})
+	}
+}
+
+// PickPostgres picks a random postgres database from flag.
+func PickPostgres(t TB) string {
+	if *postgres == "" {
+		t.Skip("Postgres flag missing, example: -postgres-test-db=" + DefaultPostgres)
+	}
+	return pickRandom(*postgres)
+}
+
+// PickCockroach picks a random cockroach database from flag.
+func PickCockroach(t TB) string {
+	if *cockroach == "" {
+		t.Skip("Cockroach flag missing, example: -cockroach-test-db=" + DefaultCockroach)
+	}
+	return pickRandom(*cockroach)
+}
+
+func pickRandom(dbstr string) string {
+	values := strings.Split(dbstr, ";")
+	if len(values) <= 1 {
+		return dbstr
+	}
+	return values[rand.Intn(len(values))]
+}

private/dbutil/pgutil/openunique_test.go

@@ -10,20 +10,19 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"storj.io/common/testcontext"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/tempdb"
 	"storj.io/storj/private/tagsql"
 )
 
 func TestTempPostgresDB(t *testing.T) {
+	connstr := pgtest.PickPostgres(t)
+
 	ctx := testcontext.New(t)
 	defer ctx.Cleanup()
 
-	if *pgtest.ConnStr == "" {
-		t.Skip("PostgreSQL flag missing")
-	}
 	prefix := "name#spaced/Test/DB"
-	testDB, err := tempdb.OpenUnique(ctx, *pgtest.ConnStr, prefix)
+	testDB, err := tempdb.OpenUnique(ctx, connstr, prefix)
 	require.NoError(t, err)
 
 	// assert new test db exists and can be connected to again

private/dbutil/pgutil/pgtest/flag.go

@@ -1,23 +0,0 @@
-// Copyright (C) 2019 Storj Labs, Inc.
-// See LICENSE for copying information.
-
-package pgtest
-
-import (
-	"flag"
-	"os"
-)
-
-// We need to define this in a separate package due to https://golang.org/issue/23910.
-
-// ConnStr is the test database connection string.
-var ConnStr = flag.String("postgres-test-db", os.Getenv("STORJ_POSTGRES_TEST"), "PostgreSQL test database connection string")
-
-// CrdbConnStr is the test database connection string for CockroachDB
-var CrdbConnStr = flag.String("cockroach-test-db", os.Getenv("STORJ_COCKROACH_TEST"), "CockroachDB test database connection string")
-
-// DefaultConnStr is expected to work under the storj-test docker-compose instance
-const DefaultConnStr = "postgres://storj:storj-pass@test-postgres/teststorj?sslmode=disable"
-
-// DefaultCrdbConnStr is expected to work when a local cockroachDB instance is running
-const DefaultCrdbConnStr = "cockroach://root@localhost:26257/master?sslmode=disable"

private/dbutil/pgutil/query_test.go

@@ -13,119 +13,97 @@ import (
 	"storj.io/common/testcontext"
 	"storj.io/storj/private/dbutil"
 	"storj.io/storj/private/dbutil/dbschema"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/pgutil"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
 	"storj.io/storj/private/dbutil/tempdb"
 )
 
-const (
+func TestQuery(t *testing.T) {
-	// DefaultPostgresConn is a connstring that works with docker-compose
+	pgtest.Run(t, func(ctx *testcontext.Context, t *testing.T, connstr string) {
-	DefaultPostgresConn = "postgres://storj:storj-pass@test-postgres/teststorj?sslmode=disable"
+		db, err := tempdb.OpenUnique(ctx, connstr, "pgutil-query")
-)
+		require.NoError(t, err)
+		defer ctx.Check(db.Close)
 
-func TestQueryPostgres(t *testing.T) {
+		emptySchema, err := pgutil.QuerySchema(ctx, db)
-	if *pgtest.ConnStr == "" {
+		require.NoError(t, err)
-		t.Skip("Postgres flag missing, example: -postgres-test-db=" + DefaultPostgresConn)
+		assert.Equal(t, &dbschema.Schema{}, emptySchema)
-	}
 
-	doQueryTest(t, *pgtest.ConnStr)
+		_, err = db.ExecContext(ctx, `
-}
+			CREATE TABLE users (
+				a bigint NOT NULL,
+				b bigint NOT NULL,
+				c text,
+				UNIQUE (c),
+				PRIMARY KEY (a)
+			);
+			CREATE TABLE names (
+				users_a bigint REFERENCES users( a ) ON DELETE CASCADE,
+				a text NOT NULL,
+				x text,
+				b text,
+				PRIMARY KEY (a, x),
+				UNIQUE ( x ),
+				UNIQUE ( a, b )
+			);
+		`)
+		require.NoError(t, err)
 
-func TestQueryCockroach(t *testing.T) {
+		schema, err := pgutil.QuerySchema(ctx, db)
-	if *pgtest.CrdbConnStr == "" {
+		require.NoError(t, err)
-		t.Skip("Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCrdbConnStr)
-	}
 
-	doQueryTest(t, *pgtest.CrdbConnStr)
+		expected := &dbschema.Schema{
-}
+			Tables: []*dbschema.Table{
-
+				{
-func doQueryTest(t *testing.T, connStr string) {
+					Name: "users",
-	ctx := testcontext.New(t)
+					Columns: []*dbschema.Column{
-	defer ctx.Cleanup()
+						{Name: "a", Type: "bigint", IsNullable: false, Reference: nil},
-
+						{Name: "b", Type: "bigint", IsNullable: false, Reference: nil},
-	db, err := tempdb.OpenUnique(ctx, connStr, "pgutil-query")
+						{Name: "c", Type: "text", IsNullable: true, Reference: nil},
-	require.NoError(t, err)
+					},
-	defer ctx.Check(db.Close)
+					PrimaryKey: []string{"a"},
-
+					Unique: [][]string{
-	emptySchema, err := pgutil.QuerySchema(ctx, db)
+						{"c"},
-	require.NoError(t, err)
+					},
-	assert.Equal(t, &dbschema.Schema{}, emptySchema)
-
-	_, err = db.ExecContext(ctx, `
-		CREATE TABLE users (
-			a bigint NOT NULL,
-			b bigint NOT NULL,
-			c text,
-			UNIQUE (c),
-			PRIMARY KEY (a)
-		);
-		CREATE TABLE names (
-			users_a bigint REFERENCES users( a ) ON DELETE CASCADE,
-			a text NOT NULL,
-			x text,
-			b text,
-			PRIMARY KEY (a, x),
-			UNIQUE ( x ),
-			UNIQUE ( a, b )
-		);
-	`)
-	require.NoError(t, err)
-
-	schema, err := pgutil.QuerySchema(ctx, db)
-	require.NoError(t, err)
-
-	expected := &dbschema.Schema{
-		Tables: []*dbschema.Table{
-			{
-				Name: "users",
-				Columns: []*dbschema.Column{
-					{Name: "a", Type: "bigint", IsNullable: false, Reference: nil},
-					{Name: "b", Type: "bigint", IsNullable: false, Reference: nil},
-					{Name: "c", Type: "text", IsNullable: true, Reference: nil},
 				},
-				PrimaryKey: []string{"a"},
+				{
-				Unique: [][]string{
+					Name: "names",
-					{"c"},
+					Columns: []*dbschema.Column{
+						{Name: "users_a", Type: "bigint", IsNullable: true,
+							Reference: &dbschema.Reference{
+								Table:    "users",
+								Column:   "a",
+								OnDelete: "CASCADE",
+							}},
+						{Name: "a", Type: "text", IsNullable: false, Reference: nil},
+						{Name: "x", Type: "text", IsNullable: false, Reference: nil}, // not null, because primary key
+						{Name: "b", Type: "text", IsNullable: true, Reference: nil},
+					},
+					PrimaryKey: []string{"a", "x"},
+					Unique: [][]string{
+						{"a", "b"},
+						{"x"},
+					},
 				},
 			},
-			{
+			Indexes: []*dbschema.Index{
-				Name: "names",
+				{Name: "names_a_b_key", Table: "names", Columns: []string{"a", "b"}, Unique: true, Partial: ""},
-				Columns: []*dbschema.Column{
+				{Name: "names_pkey", Table: "names", Columns: []string{"a", "x"}, Unique: true, Partial: ""},
-					{Name: "users_a", Type: "bigint", IsNullable: true,
+				{Name: "names_x_key", Table: "names", Columns: []string{"x"}, Unique: true, Partial: ""},
-						Reference: &dbschema.Reference{
+				{Name: "users_c_key", Table: "users", Columns: []string{"c"}, Unique: true, Partial: ""},
-							Table:    "users",
+				{Name: "users_pkey", Table: "users", Columns: []string{"a"}, Unique: true, Partial: ""},
-							Column:   "a",
-							OnDelete: "CASCADE",
-						}},
-					{Name: "a", Type: "text", IsNullable: false, Reference: nil},
-					{Name: "x", Type: "text", IsNullable: false, Reference: nil}, // not null, because primary key
-					{Name: "b", Type: "text", IsNullable: true, Reference: nil},
-				},
-				PrimaryKey: []string{"a", "x"},
-				Unique: [][]string{
-					{"a", "b"},
-					{"x"},
-				},
 			},
-		},
+		}
-		Indexes: []*dbschema.Index{
-			{Name: "names_a_b_key", Table: "names", Columns: []string{"a", "b"}, Unique: true, Partial: ""},
-			{Name: "names_pkey", Table: "names", Columns: []string{"a", "x"}, Unique: true, Partial: ""},
-			{Name: "names_x_key", Table: "names", Columns: []string{"x"}, Unique: true, Partial: ""},
-			{Name: "users_c_key", Table: "users", Columns: []string{"c"}, Unique: true, Partial: ""},
-			{Name: "users_pkey", Table: "users", Columns: []string{"a"}, Unique: true, Partial: ""},
-		},
-	}
 
-	if db.Implementation == dbutil.Cockroach {
+		if db.Implementation == dbutil.Cockroach {
-		expected.Indexes = append(expected.Indexes, &dbschema.Index{
+			expected.Indexes = append(expected.Indexes, &dbschema.Index{
-			Name:    "names_auto_index_fk_users_a_ref_users",
+				Name:    "names_auto_index_fk_users_a_ref_users",
-			Table:   "names",
+				Table:   "names",
-			Columns: []string{"users_a"},
+				Columns: []string{"users_a"},
-		})
+			})
-	}
+		}
 
-	expected.Sort()
+		expected.Sort()
-	schema.Sort()
+		schema.Sort()
-	assert.Equal(t, expected, schema)
+		assert.Equal(t, expected, schema)
+	})
 }

private/migrate/create_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"storj.io/common/testcontext"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/tempdb"
 	"storj.io/storj/private/migrate"
 	"storj.io/storj/private/tagsql"
@@ -46,48 +46,30 @@ func TestCreate_Sqlite(t *testing.T) {
 	require.Error(t, err)
 }
 
-func TestCreate_Postgres(t *testing.T) {
+func TestCreate(t *testing.T) {
-	if *pgtest.ConnStr == "" {
+	pgtest.Run(t, func(ctx *testcontext.Context, t *testing.T, connstr string) {
-		t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr)
+		db, err := tempdb.OpenUnique(ctx, connstr, "create-")
-	}
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer func() { assert.NoError(t, db.Close()) }()
 
-	ctx := testcontext.New(t)
+		// should create table
-	defer ctx.Cleanup()
+		err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text)"})
-	testCreateGeneric(ctx, t, *pgtest.ConnStr)
+		require.NoError(t, err)
-}
 
-func TestCreate_Cockroach(t *testing.T) {
+		// shouldn't create a new table
-	if *pgtest.CrdbConnStr == "" {
+		err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text)"})
-		t.Skip("Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCrdbConnStr)
+		require.NoError(t, err)
-	}
 
-	ctx := testcontext.New(t)
+		// should fail, because schema changed
-	defer ctx.Cleanup()
+		err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text, version integer)"})
-	testCreateGeneric(ctx, t, *pgtest.CrdbConnStr)
+		require.Error(t, err)
-}
 
-func testCreateGeneric(ctx *testcontext.Context, t *testing.T, connStr string) {
+		// should fail, because of trying to CREATE TABLE with same name
-	db, err := tempdb.OpenUnique(ctx, connStr, "create-")
+		err = migrate.Create(ctx, "conflict", &postgresDB{db.DB, "CREATE TABLE example_table (id text, version integer)"})
-	if err != nil {
+		require.Error(t, err)
-		t.Fatal(err)
+	})
-	}
-	defer func() { assert.NoError(t, db.Close()) }()
-
-	// should create table
-	err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text)"})
-	require.NoError(t, err)
-
-	// shouldn't create a new table
-	err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text)"})
-	require.NoError(t, err)
-
-	// should fail, because schema changed
-	err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text, version integer)"})
-	require.Error(t, err)
-
-	// should fail, because of trying to CREATE TABLE with same name
-	err = migrate.Create(ctx, "conflict", &postgresDB{db.DB, "CREATE TABLE example_table (id text, version integer)"})
-	require.Error(t, err)
 }
 
 type sqliteDB struct {

private/migrate/versions_test.go

@@ -18,7 +18,7 @@ import (
 	"go.uber.org/zap"
 
 	"storj.io/common/testcontext"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/tempdb"
 	"storj.io/storj/private/migrate"
 	"storj.io/storj/private/tagsql"
@@ -46,38 +46,20 @@ func TestBasicMigrationSqlite(t *testing.T) {
 	basicMigration(ctx, t, db, &sqliteDB{DB: db})
 }
 
-func TestBasicMigrationPostgres(t *testing.T) {
+func TestBasicMigration(t *testing.T) {
-	if *pgtest.ConnStr == "" {
+	pgtest.Run(t, func(ctx *testcontext.Context, t *testing.T, connstr string) {
-		t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr)
+		db, err := tempdb.OpenUnique(ctx, connstr, "create-")
-	}
+		if err != nil {
-	ctx := testcontext.New(t)
+			t.Fatal(err)
-	defer ctx.Cleanup()
+		}
+		defer func() { assert.NoError(t, db.Close()) }()
 
-	testBasicMigrationGeneric(ctx, t, *pgtest.ConnStr)
+		basicMigration(ctx, t, db.DB, &postgresDB{DB: db.DB})
-}
+	})
-
-func TestBasicMigrationCockroach(t *testing.T) {
-	if *pgtest.CrdbConnStr == "" {
-		t.Skipf("cockroach flag missing, example:\n-cockroach-test-db=%s", pgtest.DefaultCrdbConnStr)
-	}
-	ctx := testcontext.New(t)
-	defer ctx.Cleanup()
-
-	testBasicMigrationGeneric(ctx, t, *pgtest.CrdbConnStr)
-}
-
-func testBasicMigrationGeneric(ctx *testcontext.Context, t *testing.T, connStr string) {
-	db, err := tempdb.OpenUnique(ctx, connStr, "create-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer func() { assert.NoError(t, db.Close()) }()
-
-	basicMigration(ctx, t, db.DB, &postgresDB{DB: db.DB})
 }
 
 func basicMigration(ctx *testcontext.Context, t *testing.T, db tagsql.DB, testDB tagsql.DB) {
-	dbName := strings.ToLower(`versions_` + t.Name())
+	dbName := strings.ToLower(`versions_` + strings.Replace(t.Name(), "/", "_", -1))
 	defer func() { assert.NoError(t, dropTables(ctx, db, dbName, "users")) }()
 
 	err := ioutil.WriteFile(ctx.File("alpha.txt"), []byte("test"), 0644)
@@ -160,11 +142,9 @@ func TestMultipleMigrationSqlite(t *testing.T) {
 }
 
 func TestMultipleMigrationPostgres(t *testing.T) {
-	if *pgtest.ConnStr == "" {
+	connstr := pgtest.PickPostgres(t)
-		t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr)
-	}
 
-	db, err := tagsql.Open("postgres", *pgtest.ConnStr)
+	db, err := tagsql.Open("postgres", connstr)
 	require.NoError(t, err)
 	defer func() { assert.NoError(t, db.Close()) }()
 
@@ -236,11 +216,9 @@ func TestFailedMigrationSqlite(t *testing.T) {
 }
 
 func TestFailedMigrationPostgres(t *testing.T) {
-	if *pgtest.ConnStr == "" {
+	connstr := pgtest.PickPostgres(t)
-		t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr)
-	}
 
-	db, err := tagsql.Open("postgres", *pgtest.ConnStr)
+	db, err := tagsql.Open("postgres", connstr)
 	require.NoError(t, err)
 	defer func() { assert.NoError(t, db.Close()) }()
 

private/tagsql/db_test.go

@@ -12,8 +12,8 @@ import (
 
 	"storj.io/common/testcontext"
 	"storj.io/storj/private/dbutil/cockroachutil"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/pgutil"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
 	"storj.io/storj/private/tagsql"
 )
 
@@ -34,14 +34,12 @@ func run(t *testing.T, fn func(*testcontext.Context, *testing.T, tagsql.DB, tags
 	})
 
 	t.Run("lib-pq-postgres", func(t *testing.T) {
+		connstr := pgtest.PickPostgres(t)
+
 		ctx := testcontext.New(t)
 		defer ctx.Cleanup()
 
-		if *pgtest.ConnStr == "" {
+		db, err := pgutil.OpenUnique(ctx, connstr, "detect")
-			t.Skipf("postgresql flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr)
-		}
-
-		db, err := pgutil.OpenUnique(ctx, *pgtest.ConnStr, "detect")
 		require.NoError(t, err)
 		defer ctx.Check(db.Close)
 
@@ -49,14 +47,12 @@ func run(t *testing.T, fn func(*testcontext.Context, *testing.T, tagsql.DB, tags
 	})
 
 	t.Run("lib-pq-cockroach", func(t *testing.T) {
+		connstr := pgtest.PickCockroach(t)
+
 		ctx := testcontext.New(t)
 		defer ctx.Cleanup()
 
-		if *pgtest.CrdbConnStr == "" {
+		db, err := cockroachutil.OpenUnique(ctx, connstr, "detect")
-			t.Skipf("postgresql flag missing, example:\n-cockroach-test-db=%s", pgtest.DefaultCrdbConnStr)
-		}
-
-		db, err := cockroachutil.OpenUnique(ctx, *pgtest.CrdbConnStr, "detect")
 		require.NoError(t, err)
 		defer ctx.Check(db.Close)
 

private/testplanet/planet.go

@@ -121,11 +121,6 @@ func (peer *closablePeer) Close() error {
 
 // NewCustom creates a new full system with the specified configuration.
 func NewCustom(log *zap.Logger, config Config, satelliteDatabases satellitedbtest.SatelliteDatabases) (*Planet, error) {
-	// Clear error in the beginning to avoid issues down the line.
-	if err := satellitedbtest.PostgresDefined(); err != nil {
-		return nil, err
-	}
-
 	if config.IdentityVersion == nil {
 		version := storj.LatestIDVersion()
 		config.IdentityVersion = &version

private/testplanet/run.go

@@ -9,11 +9,23 @@ import (
 	"go.uber.org/zap/zaptest"
 
 	"storj.io/common/testcontext"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/satellite/satellitedb/satellitedbtest"
 )
 
 // Run runs testplanet in multiple configurations.
 func Run(t *testing.T, config Config, test func(t *testing.T, ctx *testcontext.Context, planet *Planet)) {
+	databases := satellitedbtest.Databases()
+	hasDatabase := false
+	for _, db := range databases {
+		hasDatabase = hasDatabase || db.MasterDB.URL != ""
+	}
+	if !hasDatabase {
+		t.Fatal("Databases flag missing, set at least one:\n" +
+			"-postgres-test-db=" + pgtest.DefaultPostgres + "\n" +
+			"-cockroach-test-db=" + pgtest.DefaultCockroach)
+	}
+
 	for _, satelliteDB := range satellitedbtest.Databases() {
 		satelliteDB := satelliteDB
 		t.Run(satelliteDB.Name, func(t *testing.T) {
@@ -28,6 +40,7 @@ func Run(t *testing.T, config Config, test func(t *testing.T, ctx *testcontext.C
 			if satelliteDB.MasterDB.URL == "" {
 				t.Skipf("Database %s connection string not provided. %s", satelliteDB.MasterDB.Name, satelliteDB.MasterDB.Message)
 			}
+
 			planetConfig := config
 			if planetConfig.Name == "" {
 				planetConfig.Name = t.Name()

satellite/satellitedb/migrate_test.go

@@ -22,8 +22,8 @@ import (
 
 	"storj.io/common/testcontext"
 	"storj.io/storj/private/dbutil/dbschema"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/pgutil"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
 	"storj.io/storj/private/dbutil/tempdb"
 	"storj.io/storj/private/migrate"
 	"storj.io/storj/satellite/satellitedb"
@@ -132,72 +132,8 @@ func loadSchemaFromSQL(ctx context.Context, connstr, script string) (_ *dbschema
 	return pgutil.QuerySchema(ctx, db)
 }
 
-func TestMigrateCockroach(t *testing.T) {
+func TestMigratePostgres(t *testing.T)  { migrateTest(t, pgtest.PickPostgres(t)) }
-	if *pgtest.CrdbConnStr == "" {
+func TestMigrateCockroach(t *testing.T) { migrateTest(t, pgtest.PickCockroach(t)) }
-		t.Skip("Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCrdbConnStr)
-	}
-	t.Parallel()
-	migrateTest(t, *pgtest.CrdbConnStr)
-}
-
-func TestMigratePostgres(t *testing.T) {
-	if *pgtest.ConnStr == "" {
-		t.Skip("Postgres flag missing, example: -postgres-test-db=" + pgtest.DefaultConnStr)
-	}
-	t.Parallel()
-	migrateTest(t, *pgtest.ConnStr)
-}
-
-func BenchmarkSetup_Postgres(b *testing.B) {
-	if *pgtest.ConnStr == "" {
-		b.Skip("Postgres flag missing, example: -postgres-test-db=" + pgtest.DefaultConnStr)
-	}
-	b.Run("merged", func(b *testing.B) {
-		benchmarkSetup(b, *pgtest.ConnStr, true)
-	})
-	b.Run("separate", func(b *testing.B) {
-		benchmarkSetup(b, *pgtest.ConnStr, false)
-	})
-}
-
-func BenchmarkSetup_Cockroach(b *testing.B) {
-	if *pgtest.CrdbConnStr == "" {
-		b.Skip("Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCrdbConnStr)
-	}
-	b.Run("merged", func(b *testing.B) {
-		benchmarkSetup(b, *pgtest.CrdbConnStr, true)
-	})
-	b.Run("separate", func(b *testing.B) {
-		benchmarkSetup(b, *pgtest.CrdbConnStr, false)
-	})
-}
-
-func benchmarkSetup(b *testing.B, connStr string, merged bool) {
-	for i := 0; i < b.N; i++ {
-		func() {
-			ctx := context.Background()
-			log := zap.NewNop()
-
-			// create tempDB
-			tempDB, err := tempdb.OpenUnique(ctx, connStr, "migrate")
-			require.NoError(b, err)
-			defer func() { require.NoError(b, tempDB.Close()) }()
-
-			// create a new satellitedb connection
-			db, err := satellitedb.New(log, tempDB.ConnStr, satellitedb.Options{})
-			require.NoError(b, err)
-			defer func() { require.NoError(b, db.Close()) }()
-
-			if merged {
-				err = db.TestingCreateTables(ctx)
-				require.NoError(b, err)
-			} else {
-				err = db.CreateTables(ctx)
-				require.NoError(b, err)
-			}
-		}()
-	}
-}
 
 // satelliteDB provides access to certain methods on a *satellitedb.satelliteDB
 // instance, since that type is not exported.
@@ -207,6 +143,8 @@ type satelliteDB interface {
 }
 
 func migrateTest(t *testing.T, connStr string) {
+	t.Parallel()
+
 	ctx := testcontext.NewWithTimeout(t, 8*time.Minute)
 	defer ctx.Cleanup()
 
@@ -271,3 +209,50 @@ func migrateTest(t *testing.T, connStr string) {
 	// verify that we also match the dbx version
 	require.Equal(t, dbxschema, finalSchema, "result of all migration scripts did not match dbx schema")
 }
+
+func BenchmarkSetup_Postgres(b *testing.B) {
+	connstr := pgtest.PickPostgres(b)
+	b.Run("merged", func(b *testing.B) {
+		benchmarkSetup(b, connstr, true)
+	})
+	b.Run("separate", func(b *testing.B) {
+		benchmarkSetup(b, connstr, false)
+	})
+}
+
+func BenchmarkSetup_Cockroach(b *testing.B) {
+	connstr := pgtest.PickCockroach(b)
+	b.Run("merged", func(b *testing.B) {
+		benchmarkSetup(b, connstr, true)
+	})
+	b.Run("separate", func(b *testing.B) {
+		benchmarkSetup(b, connstr, false)
+	})
+}
+
+func benchmarkSetup(b *testing.B, connStr string, merged bool) {
+	for i := 0; i < b.N; i++ {
+		func() {
+			ctx := context.Background()
+			log := zap.NewNop()
+
+			// create tempDB
+			tempDB, err := tempdb.OpenUnique(ctx, connStr, "migrate")
+			require.NoError(b, err)
+			defer func() { require.NoError(b, tempDB.Close()) }()
+
+			// create a new satellitedb connection
+			db, err := satellitedb.New(log, tempDB.ConnStr, satellitedb.Options{})
+			require.NoError(b, err)
+			defer func() { require.NoError(b, db.Close()) }()
+
+			if merged {
+				err = db.TestingCreateTables(ctx)
+				require.NoError(b, err)
+			} else {
+				err = db.CreateTables(ctx)
+				require.NoError(b, err)
+			}
+		}()
+	}
+}

satellite/satellitedb/satellitedbtest/pgschema.go

@@ -1,18 +0,0 @@
-// Copyright (C) 2019 Storj Labs, Inc.
-// See LICENSE for copying information.
-
-package satellitedbtest
-
-import (
-	"github.com/zeebo/errs"
-
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
-)
-
-// PostgresDefined returns an error when the --postgres-test-db or STORJ_POSTGRES_TEST is not set for tests.
-func PostgresDefined() error {
-	if *pgtest.ConnStr == "" {
-		return errs.New("flag --postgres-test-db or environment variable STORJ_POSTGRES_TEST not defined for PostgreSQL test database")
-	}
-	return nil
-}

satellite/satellitedb/satellitedbtest/run.go

@@ -18,8 +18,8 @@ import (
 
 	"storj.io/common/testcontext"
 	"storj.io/storj/private/dbutil"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/pgutil"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
 	"storj.io/storj/private/dbutil/tempdb"
 	"storj.io/storj/satellite"
 	"storj.io/storj/satellite/metainfo"
@@ -41,18 +41,24 @@ type Database struct {
 	Message string
 }
 
+type ignoreSkip struct{}
+
+func (ignoreSkip) Skip(...interface{}) {}
+
 // Databases returns default databases.
 func Databases() []SatelliteDatabases {
+	cockroachConnStr := pgtest.PickCockroach(ignoreSkip{})
+	postgresConnStr := pgtest.PickPostgres(ignoreSkip{})
 	return []SatelliteDatabases{
 		{
 			Name:      "Postgres",
-			MasterDB:  Database{"Postgres", *pgtest.ConnStr, "Postgres flag missing, example: -postgres-test-db=" + pgtest.DefaultConnStr + " or use STORJ_POSTGRES_TEST environment variable."},
+			MasterDB:  Database{"Postgres", postgresConnStr, "Postgres flag missing, example: -postgres-test-db=" + pgtest.DefaultPostgres + " or use STORJ_POSTGRES_TEST environment variable."},
-			PointerDB: Database{"Postgres", *pgtest.ConnStr, ""},
+			PointerDB: Database{"Postgres", postgresConnStr, ""},
 		},
 		{
 			Name:      "Cockroach",
-			MasterDB:  Database{"Cockroach", *pgtest.CrdbConnStr, "Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCrdbConnStr + " or use STORJ_COCKROACH_TEST environment variable."},
+			MasterDB:  Database{"Cockroach", cockroachConnStr, "Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCockroach + " or use STORJ_COCKROACH_TEST environment variable."},
-			PointerDB: Database{"Cockroach", *pgtest.CrdbConnStr, ""},
+			PointerDB: Database{"Cockroach", cockroachConnStr, ""},
 		},
 	}
 }

storage/cockroachkv/client_test.go

@@ -11,21 +11,19 @@ import (
 
 	"storj.io/common/testcontext"
 	"storj.io/storj/private/dbutil/cockroachutil"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/storage/testsuite"
 )
 
 func newTestCockroachDB(ctx context.Context, t testing.TB) (store *Client, cleanup func()) {
-	if *pgtest.CrdbConnStr == "" {
+	connstr := pgtest.PickCockroach(t)
-		t.Skipf("cockroach flag missing, example:\n-cockroach-test-db=%s", pgtest.DefaultCrdbConnStr)
-	}
 
-	tdb, err := cockroachutil.OpenUnique(ctx, *pgtest.CrdbConnStr, "test-schema")
+	tdb, err := cockroachutil.OpenUnique(ctx, connstr, "test-schema")
 	if err != nil {
 		t.Fatalf("init: %+v", err)
 	}
 
-	return NewWith(tdb.DB, *pgtest.CrdbConnStr), func() {
+	return NewWith(tdb.DB, connstr), func() {
 		if err := tdb.Close(); err != nil {
 			t.Fatalf("failed to close db: %v", err)
 		}

storage/postgreskv/client_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/zeebo/errs"
 
 	"storj.io/common/testcontext"
-	"storj.io/storj/private/dbutil/pgutil/pgtest"
+	"storj.io/storj/private/dbutil/pgtest"
 	"storj.io/storj/private/dbutil/txutil"
 	"storj.io/storj/private/tagsql"
 	"storj.io/storj/storage"
@@ -21,11 +21,9 @@ import (
 )
 
 func newTestPostgres(t testing.TB) (store *Client, cleanup func()) {
-	if *pgtest.ConnStr == "" {
+	connstr := pgtest.PickPostgres(t)
-		t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr)
-	}
 
-	pgdb, err := New(*pgtest.ConnStr)
+	pgdb, err := New(connstr)
 	if err != nil {
 		t.Fatalf("init: %v", err)
 	}

storagenode/storagenodedb/schema.go

@@ -675,4 +675,3 @@ func Schema() map[string]*dbschema.Schema {
 		},
 	}
 }
-