From 85c45cd56f7ec27272a3d56cbd3f91673138b554 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Mon, 27 Apr 2020 22:34:42 +0300 Subject: [PATCH] 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 | 17 +- private/dbutil/cockroachutil/driver_test.go | 9 +- .../dbutil/cockroachutil/openunique_test.go | 11 +- private/dbutil/pgtest/flag.go | 89 +++++++++ private/dbutil/pgutil/openunique_test.go | 9 +- private/dbutil/pgutil/pgtest/flag.go | 23 --- private/dbutil/pgutil/query_test.go | 180 ++++++++---------- private/migrate/create_test.go | 60 ++---- private/migrate/versions_test.go | 52 ++--- private/tagsql/db_test.go | 18 +- private/testplanet/planet.go | 5 - private/testplanet/run.go | 13 ++ satellite/satellitedb/migrate_test.go | 119 +++++------- .../satellitedb/satellitedbtest/pgschema.go | 18 -- satellite/satellitedb/satellitedbtest/run.go | 16 +- storage/cockroachkv/client_test.go | 10 +- storage/postgreskv/client_test.go | 8 +- storagenode/storagenodedb/schema.go | 1 - 18 files changed, 321 insertions(+), 337 deletions(-) create mode 100644 private/dbutil/pgtest/flag.go delete mode 100644 private/dbutil/pgutil/pgtest/flag.go delete mode 100644 satellite/satellitedb/satellitedbtest/pgschema.go diff --git a/Jenkinsfile.public b/Jenkinsfile.public index ebf173892..3257e031f 100644 --- a/Jenkinsfile.public +++ b/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;\'' } } diff --git a/private/dbutil/cockroachutil/driver_test.go b/private/dbutil/cockroachutil/driver_test.go index 09e478954..310a8ea0e 100644 --- a/private/dbutil/cockroachutil/driver_test.go +++ b/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 == "" { - t.Skip("CockroachDB flag missing") - } - testDB, err := OpenUnique(ctx, *pgtest.CrdbConnStr, "TestLibPqCompatibility") + testDB, err := OpenUnique(ctx, connstr, "TestLibPqCompatibility") require.NoError(t, err) defer ctx.Check(testDB.Close) diff --git a/private/dbutil/cockroachutil/openunique_test.go b/private/dbutil/cockroachutil/openunique_test.go index cf4b57b34..6a1f9f837 100644 --- a/private/dbutil/cockroachutil/openunique_test.go +++ b/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) diff --git a/private/dbutil/pgtest/flag.go b/private/dbutil/pgtest/flag.go new file mode 100644 index 000000000..529c6898f --- /dev/null +++ b/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))] +} diff --git a/private/dbutil/pgutil/openunique_test.go b/private/dbutil/pgutil/openunique_test.go index 15905afff..22fc5a270 100644 --- a/private/dbutil/pgutil/openunique_test.go +++ b/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 diff --git a/private/dbutil/pgutil/pgtest/flag.go b/private/dbutil/pgutil/pgtest/flag.go deleted file mode 100644 index 678f4c47b..000000000 --- a/private/dbutil/pgutil/pgtest/flag.go +++ /dev/null @@ -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" diff --git a/private/dbutil/pgutil/query_test.go b/private/dbutil/pgutil/query_test.go index f53834b10..108822251 100644 --- a/private/dbutil/pgutil/query_test.go +++ b/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 ( - // DefaultPostgresConn is a connstring that works with docker-compose - DefaultPostgresConn = "postgres://storj:storj-pass@test-postgres/teststorj?sslmode=disable" -) +func TestQuery(t *testing.T) { + pgtest.Run(t, func(ctx *testcontext.Context, t *testing.T, connstr string) { + db, err := tempdb.OpenUnique(ctx, connstr, "pgutil-query") + require.NoError(t, err) + defer ctx.Check(db.Close) -func TestQueryPostgres(t *testing.T) { - if *pgtest.ConnStr == "" { - t.Skip("Postgres flag missing, example: -postgres-test-db=" + DefaultPostgresConn) - } + emptySchema, err := pgutil.QuerySchema(ctx, db) + require.NoError(t, err) + 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) { - if *pgtest.CrdbConnStr == "" { - t.Skip("Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCrdbConnStr) - } + schema, err := pgutil.QuerySchema(ctx, db) + require.NoError(t, err) - doQueryTest(t, *pgtest.CrdbConnStr) -} - -func doQueryTest(t *testing.T, connStr string) { - ctx := testcontext.New(t) - defer ctx.Cleanup() - - db, err := tempdb.OpenUnique(ctx, connStr, "pgutil-query") - require.NoError(t, err) - defer ctx.Check(db.Close) - - emptySchema, err := pgutil.QuerySchema(ctx, db) - 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}, + 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{ + {"c"}, + }, }, - PrimaryKey: []string{"a"}, - Unique: [][]string{ - {"c"}, + { + Name: "names", + 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"}, + }, }, }, - { - Name: "names", - 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_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: ""}, }, - }, - 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 { - expected.Indexes = append(expected.Indexes, &dbschema.Index{ - Name: "names_auto_index_fk_users_a_ref_users", - Table: "names", - Columns: []string{"users_a"}, - }) - } + if db.Implementation == dbutil.Cockroach { + expected.Indexes = append(expected.Indexes, &dbschema.Index{ + Name: "names_auto_index_fk_users_a_ref_users", + Table: "names", + Columns: []string{"users_a"}, + }) + } - expected.Sort() - schema.Sort() - assert.Equal(t, expected, schema) + expected.Sort() + schema.Sort() + assert.Equal(t, expected, schema) + }) } diff --git a/private/migrate/create_test.go b/private/migrate/create_test.go index cdaef2555..75a3b81d7 100644 --- a/private/migrate/create_test.go +++ b/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) { - if *pgtest.ConnStr == "" { - t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr) - } +func TestCreate(t *testing.T) { + pgtest.Run(t, func(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()) }() - ctx := testcontext.New(t) - defer ctx.Cleanup() - testCreateGeneric(ctx, t, *pgtest.ConnStr) -} + // should create table + err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text)"}) + require.NoError(t, err) -func TestCreate_Cockroach(t *testing.T) { - if *pgtest.CrdbConnStr == "" { - t.Skip("Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCrdbConnStr) - } + // shouldn't create a new table + err = migrate.Create(ctx, "example", &postgresDB{db.DB, "CREATE TABLE example_table (id text)"}) + require.NoError(t, err) - ctx := testcontext.New(t) - defer ctx.Cleanup() - testCreateGeneric(ctx, t, *pgtest.CrdbConnStr) -} + // 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) -func testCreateGeneric(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()) }() - - // 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) + // 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 { diff --git a/private/migrate/versions_test.go b/private/migrate/versions_test.go index 77d3c8226..dc01f8f4b 100644 --- a/private/migrate/versions_test.go +++ b/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) { - if *pgtest.ConnStr == "" { - t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr) - } - ctx := testcontext.New(t) - defer ctx.Cleanup() +func TestBasicMigration(t *testing.T) { + pgtest.Run(t, func(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()) }() - testBasicMigrationGeneric(ctx, t, *pgtest.ConnStr) -} - -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}) + 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 == "" { - t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr) - } + connstr := pgtest.PickPostgres(t) - 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 == "" { - t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr) - } + connstr := pgtest.PickPostgres(t) - db, err := tagsql.Open("postgres", *pgtest.ConnStr) + db, err := tagsql.Open("postgres", connstr) require.NoError(t, err) defer func() { assert.NoError(t, db.Close()) }() diff --git a/private/tagsql/db_test.go b/private/tagsql/db_test.go index 01ba00ed6..c5d8b7236 100644 --- a/private/tagsql/db_test.go +++ b/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 == "" { - t.Skipf("postgresql flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr) - } - - db, err := pgutil.OpenUnique(ctx, *pgtest.ConnStr, "detect") + db, err := pgutil.OpenUnique(ctx, 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 == "" { - t.Skipf("postgresql flag missing, example:\n-cockroach-test-db=%s", pgtest.DefaultCrdbConnStr) - } - - db, err := cockroachutil.OpenUnique(ctx, *pgtest.CrdbConnStr, "detect") + db, err := cockroachutil.OpenUnique(ctx, connstr, "detect") require.NoError(t, err) defer ctx.Check(db.Close) diff --git a/private/testplanet/planet.go b/private/testplanet/planet.go index 4fc97adf1..ea195dbb8 100644 --- a/private/testplanet/planet.go +++ b/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 diff --git a/private/testplanet/run.go b/private/testplanet/run.go index 7f175e84d..da692386f 100644 --- a/private/testplanet/run.go +++ b/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() diff --git a/satellite/satellitedb/migrate_test.go b/satellite/satellitedb/migrate_test.go index 6a9bec6ba..f7895ebf1 100644 --- a/satellite/satellitedb/migrate_test.go +++ b/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) { - if *pgtest.CrdbConnStr == "" { - 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) - } - }() - } -} +func TestMigratePostgres(t *testing.T) { migrateTest(t, pgtest.PickPostgres(t)) } +func TestMigrateCockroach(t *testing.T) { migrateTest(t, pgtest.PickCockroach(t)) } // 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) + } + }() + } +} diff --git a/satellite/satellitedb/satellitedbtest/pgschema.go b/satellite/satellitedb/satellitedbtest/pgschema.go deleted file mode 100644 index 5bd02c0b7..000000000 --- a/satellite/satellitedb/satellitedbtest/pgschema.go +++ /dev/null @@ -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 -} diff --git a/satellite/satellitedb/satellitedbtest/run.go b/satellite/satellitedb/satellitedbtest/run.go index f44fe4aa0..6167ada3e 100644 --- a/satellite/satellitedb/satellitedbtest/run.go +++ b/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."}, - PointerDB: Database{"Postgres", *pgtest.ConnStr, ""}, + MasterDB: Database{"Postgres", postgresConnStr, "Postgres flag missing, example: -postgres-test-db=" + pgtest.DefaultPostgres + " or use STORJ_POSTGRES_TEST environment variable."}, + 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."}, - PointerDB: Database{"Cockroach", *pgtest.CrdbConnStr, ""}, + MasterDB: Database{"Cockroach", cockroachConnStr, "Cockroach flag missing, example: -cockroach-test-db=" + pgtest.DefaultCockroach + " or use STORJ_COCKROACH_TEST environment variable."}, + PointerDB: Database{"Cockroach", cockroachConnStr, ""}, }, } } diff --git a/storage/cockroachkv/client_test.go b/storage/cockroachkv/client_test.go index 8fdf05ec9..e666ccd86 100644 --- a/storage/cockroachkv/client_test.go +++ b/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 == "" { - t.Skipf("cockroach flag missing, example:\n-cockroach-test-db=%s", pgtest.DefaultCrdbConnStr) - } + connstr := pgtest.PickCockroach(t) - 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) } diff --git a/storage/postgreskv/client_test.go b/storage/postgreskv/client_test.go index f40972b7c..2beaf94b5 100644 --- a/storage/postgreskv/client_test.go +++ b/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 == "" { - t.Skipf("postgres flag missing, example:\n-postgres-test-db=%s", pgtest.DefaultConnStr) - } + connstr := pgtest.PickPostgres(t) - pgdb, err := New(*pgtest.ConnStr) + pgdb, err := New(connstr) if err != nil { t.Fatalf("init: %v", err) } diff --git a/storagenode/storagenodedb/schema.go b/storagenode/storagenodedb/schema.go index 9d18064dc..20aed3c16 100644 --- a/storagenode/storagenodedb/schema.go +++ b/storagenode/storagenodedb/schema.go @@ -675,4 +675,3 @@ func Schema() map[string]*dbschema.Schema { }, } } -