From 88486cb2c3ff0806a95e33ef02da6503401fa3e6 Mon Sep 17 00:00:00 2001 From: Adrian Macneil Date: Sun, 21 Apr 2024 11:29:07 -0700 Subject: [PATCH] Clean up test helper functions (#544) Reduce duplication in tests and consistent messages for skipped integration tests. --- pkg/dbmate/db_test.go | 53 +++++++++---------- pkg/dbtest/dbtest.go | 36 +++++++++++++ pkg/dbutil/dbutil.go | 15 ------ pkg/dbutil/dbutil_test.go | 5 +- pkg/driver/bigquery/bigquery_test.go | 17 ++---- pkg/driver/clickhouse/clickhouse.go | 9 +++- .../clickhouse/clickhouse_cluster_test.go | 36 +++---------- pkg/driver/clickhouse/clickhouse_test.go | 3 +- .../clickhouse/clickhouse_testutils_test.go | 25 +++++---- .../clickhouse/cluster_parameters_test.go | 12 ++--- pkg/driver/mysql/mysql_test.go | 17 +++--- pkg/driver/postgres/postgres.go | 7 ++- pkg/driver/postgres/postgres_test.go | 18 ++----- pkg/driver/sqlite/sqlite_test.go | 37 ++++++------- 14 files changed, 139 insertions(+), 151 deletions(-) create mode 100644 pkg/dbtest/dbtest.go diff --git a/pkg/dbmate/db_test.go b/pkg/dbmate/db_test.go index 5b24269d..7be16a6f 100644 --- a/pkg/dbmate/db_test.go +++ b/pkg/dbmate/db_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/amacneil/dbmate/v2/pkg/dbmate" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" _ "github.com/amacneil/dbmate/v2/pkg/driver/mysql" _ "github.com/amacneil/dbmate/v2/pkg/driver/postgres" @@ -21,12 +22,12 @@ import ( var rootDir string -func sqliteTestURL() *url.URL { - return dbutil.MustParseURL("sqlite:dbmate_test.sqlite3") +func sqliteTestURL(t *testing.T) *url.URL { + return dbtest.MustParseURL(t, "sqlite:dbmate_test.sqlite3") } -func sqliteBrokenTestURL() *url.URL { - return dbutil.MustParseURL("sqlite:/doesnotexist/dbmate_test.sqlite3") +func sqliteBrokenTestURL(t *testing.T) *url.URL { + return dbtest.MustParseURL(t, "sqlite:/doesnotexist/dbmate_test.sqlite3") } func newTestDB(t *testing.T, u *url.URL) *dbmate.DB { @@ -48,7 +49,7 @@ func newTestDB(t *testing.T, u *url.URL) *dbmate.DB { } func TestNew(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("foo:test")) + db := dbmate.New(dbtest.MustParseURL(t, "foo:test")) require.True(t, db.AutoDumpSchema) require.Equal(t, "foo:test", db.DatabaseURL.String()) require.Equal(t, []string{"./db/migrations"}, db.MigrationsDir) @@ -68,14 +69,14 @@ func TestGetDriver(t *testing.T) { }) t.Run("missing schema", func(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("//hi")) + db := dbmate.New(dbtest.MustParseURL(t, "//hi")) drv, err := db.Driver() require.Nil(t, drv) require.EqualError(t, err, "invalid url, have you set your --url flag or DATABASE_URL environment variable?") }) t.Run("invalid driver", func(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("foo://bar")) + db := dbmate.New(dbtest.MustParseURL(t, "foo://bar")) drv, err := db.Driver() require.EqualError(t, err, "unsupported driver: foo") require.Nil(t, drv) @@ -83,7 +84,7 @@ func TestGetDriver(t *testing.T) { } func TestWait(t *testing.T) { - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) // speed up retry loop for testing db.WaitInterval = time.Millisecond @@ -95,7 +96,7 @@ func TestWait(t *testing.T) { }) t.Run("invalid connection", func(t *testing.T) { - db.DatabaseURL = sqliteBrokenTestURL() + db.DatabaseURL = sqliteBrokenTestURL(t) err := db.Wait() require.Error(t, err) @@ -104,7 +105,7 @@ func TestWait(t *testing.T) { } func TestDumpSchema(t *testing.T) { - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) // create custom schema file directory dir, err := os.MkdirTemp("", "dbmate") @@ -137,7 +138,7 @@ func TestDumpSchema(t *testing.T) { } func TestAutoDumpSchema(t *testing.T) { - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) db.AutoDumpSchema = true // create custom schema file directory @@ -180,7 +181,7 @@ func TestAutoDumpSchema(t *testing.T) { } func TestLoadSchema(t *testing.T) { - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) drv, err := db.Driver() require.NoError(t, err) @@ -245,7 +246,7 @@ func TestLoadSchema(t *testing.T) { func checkWaitCalled(t *testing.T, db *dbmate.DB, command func() error) { oldDatabaseURL := db.DatabaseURL - db.DatabaseURL = sqliteBrokenTestURL() + db.DatabaseURL = sqliteBrokenTestURL(t) err := command() require.Error(t, err) @@ -255,7 +256,7 @@ func checkWaitCalled(t *testing.T, db *dbmate.DB, command func() error) { } func testWaitBefore(t *testing.T, verbose bool) { - u := sqliteTestURL() + u := sqliteTestURL(t) db := newTestDB(t, u) db.Verbose = verbose db.WaitBefore = true @@ -329,7 +330,7 @@ Rows affected: 0`) func testEachURL(t *testing.T, fn func(*testing.T, *url.URL)) { t.Run("sqlite", func(t *testing.T) { - fn(t, sqliteTestURL()) + fn(t, sqliteTestURL(t)) }) optionalTestURLs := []string{"MYSQL_TEST_URL", "POSTGRES_TEST_URL"} @@ -337,12 +338,8 @@ func testEachURL(t *testing.T, fn func(*testing.T, *url.URL)) { // split on underscore and take first part testname := strings.ToLower(strings.Split(varname, "_")[0]) t.Run(testname, func(t *testing.T) { - val := os.Getenv(varname) - if val == "" { - t.Skipf("no %s url provided", varname) - } else { - fn(t, dbutil.MustParseURL(val)) - } + u := dbtest.GetenvURLOrSkip(t, varname) + fn(t, u) }) } } @@ -543,7 +540,7 @@ func TestFindMigrations(t *testing.T) { func TestFindMigrationsAbsolute(t *testing.T) { t.Run("relative path", func(t *testing.T) { - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) db.MigrationsDir = []string{"db/migrations"} migrations, err := db.FindMigrations() @@ -562,7 +559,7 @@ func TestFindMigrationsAbsolute(t *testing.T) { require.NoError(t, err) defer file.Close() - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) db.MigrationsDir = []string{dir} require.Nil(t, db.FS) @@ -594,7 +591,7 @@ drop table users; "db/not_migrations/20151129054053_test_migration.sql": {}, } - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) db.FS = mapFS // drop and recreate database @@ -641,7 +638,7 @@ func TestFindMigrationsFSMultipleDirs(t *testing.T) { "db/migrations_c/006_test_migration_c.sql": {}, } - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) db.FS = mapFS db.MigrationsDir = []string{"./db/migrations_a", "./db/migrations_b", "./db/migrations_c"} @@ -667,7 +664,7 @@ func TestMigrateUnrestrictedOrder(t *testing.T) { emptyMigration := []byte("-- migrate:up\n-- migrate:down") // initialize database - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) err := db.Drop() require.NoError(t, err) @@ -698,7 +695,7 @@ func TestMigrateStrictOrder(t *testing.T) { emptyMigration := []byte("-- migrate:up\n-- migrate:down") // initialize database - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) db.Strict = true err := db.Drop() @@ -738,7 +735,7 @@ func TestMigrateStrictOrder(t *testing.T) { } func TestMigrateQueryErrorMessage(t *testing.T) { - db := newTestDB(t, sqliteTestURL()) + db := newTestDB(t, sqliteTestURL(t)) err := db.Drop() require.NoError(t, err) diff --git a/pkg/dbtest/dbtest.go b/pkg/dbtest/dbtest.go new file mode 100644 index 00000000..4c21feba --- /dev/null +++ b/pkg/dbtest/dbtest.go @@ -0,0 +1,36 @@ +// Helper package that should only be used in test files +package dbtest + +import ( + "net/url" + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +// MustParseURL parses a URL from string, and fails the test if the URL is invalid. +func MustParseURL(t *testing.T, s string) *url.URL { + require.NotEmpty(t, s) + + u, err := url.Parse(s) + require.NoError(t, err) + + return u +} + +// GetenvOrSkip gets an environment variable, and skips the test if it is empty. +func GetenvOrSkip(t *testing.T, key string) string { + value := os.Getenv(key) + if value == "" { + t.Skipf("no %s provided", key) + } + + return value +} + +// GetenvURLOrSkip gets an environment variable, parses it as a URL, +// fails the test if the URL is invalid, and skips the test if empty. +func GetenvURLOrSkip(t *testing.T, key string) *url.URL { + return MustParseURL(t, GetenvOrSkip(t, key)) +} diff --git a/pkg/dbutil/dbutil.go b/pkg/dbutil/dbutil.go index ac15ee0f..311ef882 100644 --- a/pkg/dbutil/dbutil.go +++ b/pkg/dbutil/dbutil.go @@ -137,21 +137,6 @@ func QueryValue(db Transaction, query string, args ...interface{}) (string, erro return result.String, nil } -// MustParseURL parses a URL from string, and panics if it fails. -// It is used during testing and in cases where we are parsing a generated URL. -func MustParseURL(s string) *url.URL { - if s == "" { - panic("missing url") - } - - u, err := url.Parse(s) - if err != nil { - panic(err) - } - - return u -} - // MustUnescapePath unescapes a URL path, and panics if it fails. // It is used during in cases where we are parsing a generated path. func MustUnescapePath(s string) string { diff --git a/pkg/dbutil/dbutil_test.go b/pkg/dbutil/dbutil_test.go index 57ceed5b..79fa3169 100644 --- a/pkg/dbutil/dbutil_test.go +++ b/pkg/dbutil/dbutil_test.go @@ -4,6 +4,7 @@ import ( "database/sql" "testing" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" _ "github.com/mattn/go-sqlite3" // database/sql driver @@ -12,13 +13,13 @@ import ( func TestDatabaseName(t *testing.T) { t.Run("valid", func(t *testing.T) { - u := dbutil.MustParseURL("foo://host/dbname?query") + u := dbtest.MustParseURL(t, "foo://host/dbname?query") name := dbutil.DatabaseName(u) require.Equal(t, "dbname", name) }) t.Run("empty", func(t *testing.T) { - u := dbutil.MustParseURL("foo://host") + u := dbtest.MustParseURL(t, "foo://host") name := dbutil.DatabaseName(u) require.Equal(t, "", name) }) diff --git a/pkg/driver/bigquery/bigquery_test.go b/pkg/driver/bigquery/bigquery_test.go index e16379e8..03025a85 100644 --- a/pkg/driver/bigquery/bigquery_test.go +++ b/pkg/driver/bigquery/bigquery_test.go @@ -4,22 +4,17 @@ import ( "database/sql" "fmt" "net/url" - "os" "testing" "github.com/stretchr/testify/require" "github.com/amacneil/dbmate/v2/pkg/dbmate" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" ) func testBigQueryDriver(t *testing.T) *Driver { - url := os.Getenv("BIGQUERY_TEST_URL") - if url == "" { - t.Skip("no BIGQUERY_TEST_URL provided") - } - - u := dbutil.MustParseURL(url) + u := dbtest.GetenvURLOrSkip(t, "BIGQUERY_TEST_URL") drv, err := dbmate.New(u).Driver() require.NoError(t, err) @@ -27,11 +22,7 @@ func testBigQueryDriver(t *testing.T) *Driver { } func testGoogleBigQueryDriver(t *testing.T) *Driver { - testURL := os.Getenv("GOOGLE_BIGQUERY_TEST_URL") - if testURL == "" { - t.Skip("no GOOGLE_BIGQUERY_TEST_URL provided") - } - u := dbutil.MustParseURL(testURL) + u := dbtest.GetenvURLOrSkip(t, "GOOGLE_BIGQUERY_TEST_URL") endpoint := u.Query().Get("endpoint") if endpoint != "" { @@ -86,7 +77,7 @@ func prepTestGoogleBigQueryDB(t *testing.T) *sql.DB { } func TestGetDriver(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("bigquery://")) + db := dbmate.New(dbtest.MustParseURL(t, "bigquery://")) drvInterface, err := db.Driver() require.NoError(t, err) diff --git a/pkg/driver/clickhouse/clickhouse.go b/pkg/driver/clickhouse/clickhouse.go index f712eb12..1878f5bd 100644 --- a/pkg/driver/clickhouse/clickhouse.go +++ b/pkg/driver/clickhouse/clickhouse.go @@ -40,7 +40,7 @@ func NewDriver(config dbmate.DriverConfig) dbmate.Driver { func connectionString(initialURL *url.URL) string { // clone url - u := dbutil.MustParseURL(initialURL.String()) + u, _ := url.Parse(initialURL.String()) host := u.Host if u.Port() == "" { @@ -109,7 +109,12 @@ func (drv *Driver) onClusterClause() string { } func (drv *Driver) databaseName() string { - name := strings.TrimLeft(dbutil.MustParseURL(connectionString(drv.databaseURL)).Path, "/") + u, err := url.Parse(connectionString(drv.databaseURL)) + if err != nil { + panic(err) + } + + name := strings.TrimLeft(u.Path, "/") if name == "" { name = "default" } diff --git a/pkg/driver/clickhouse/clickhouse_cluster_test.go b/pkg/driver/clickhouse/clickhouse_cluster_test.go index 5920dee2..b956200c 100644 --- a/pkg/driver/clickhouse/clickhouse_cluster_test.go +++ b/pkg/driver/clickhouse/clickhouse_cluster_test.go @@ -2,36 +2,15 @@ package clickhouse import ( "database/sql" - "fmt" - "os" "testing" "github.com/amacneil/dbmate/v2/pkg/dbmate" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" "github.com/stretchr/testify/require" ) -func testClickHouseDriverCluster01(t *testing.T) *Driver { - url := os.Getenv("CLICKHOUSE_CLUSTER_01_TEST_URL") - if url == "" { - t.Skip("no CLICKHOUSE_CLUSTER_01_TEST_URL provided") - } - - u := fmt.Sprintf("%s?on_cluster", url) - return testClickHouseDriverURL(t, u) -} - -func testClickHouseDriverCluster02(t *testing.T) *Driver { - url := os.Getenv("CLICKHOUSE_CLUSTER_02_TEST_URL") - if url == "" { - t.Skip("no CLICKHOUSE_CLUSTER_02_TEST_URL provided") - } - - u := fmt.Sprintf("%s?on_cluster", url) - return testClickHouseDriverURL(t, u) -} - func assertDatabaseExists(t *testing.T, drv *Driver, shouldExist bool) { db, err := sql.Open("clickhouse", drv.databaseURL.String()) require.NoError(t, err) @@ -47,15 +26,13 @@ func assertDatabaseExists(t *testing.T, drv *Driver, shouldExist bool) { // Makes sure driver creatinon is atomic func TestDriverCreationSanity(t *testing.T) { - if os.Getenv("CLICKHOUSE_CLUSTER_01_TEST_URL") == "" { - t.Skip("no CLICKHOUSE_CLUSTER_01_TEST_URL provided") - } - - url := fmt.Sprintf("%s?on_cluster", os.Getenv("CLICKHOUSE_CLUSTER_01_TEST_URL")) - u := dbutil.MustParseURL(url) + u := dbtest.GetenvURLOrSkip(t, "CLICKHOUSE_CLUSTER_01_TEST_URL") + u.RawQuery = "on_cluster" dbm := dbmate.New(u) + drv, err := dbm.Driver() require.NoError(t, err) + drvAgain, err := dbm.Driver() require.NoError(t, err) @@ -77,8 +54,7 @@ func TestOnClusterClause(t *testing.T) { for _, c := range cases { t.Run(c.input, func(t *testing.T) { - drv := testClickHouseDriverURL(t, c.input) - + drv := testClickHouseDriverURL(t, dbtest.MustParseURL(t, c.input)) actual := drv.onClusterClause() require.Equal(t, c.expected, actual) }) diff --git a/pkg/driver/clickhouse/clickhouse_test.go b/pkg/driver/clickhouse/clickhouse_test.go index ef1f521c..da9912e4 100644 --- a/pkg/driver/clickhouse/clickhouse_test.go +++ b/pkg/driver/clickhouse/clickhouse_test.go @@ -6,13 +6,14 @@ import ( "testing" "github.com/amacneil/dbmate/v2/pkg/dbmate" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" "github.com/stretchr/testify/require" ) func TestGetDriver(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("clickhouse://")) + db := dbmate.New(dbtest.MustParseURL(t, "clickhouse://")) drvInterface, err := db.Driver() require.NoError(t, err) diff --git a/pkg/driver/clickhouse/clickhouse_testutils_test.go b/pkg/driver/clickhouse/clickhouse_testutils_test.go index 0f7b562b..97786576 100644 --- a/pkg/driver/clickhouse/clickhouse_testutils_test.go +++ b/pkg/driver/clickhouse/clickhouse_testutils_test.go @@ -2,17 +2,16 @@ package clickhouse import ( "database/sql" - "os" + "net/url" "testing" "github.com/amacneil/dbmate/v2/pkg/dbmate" - "github.com/amacneil/dbmate/v2/pkg/dbutil" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/stretchr/testify/require" ) -func testClickHouseDriverURL(t *testing.T, url string) *Driver { - u := dbutil.MustParseURL(url) +func testClickHouseDriverURL(t *testing.T, u *url.URL) *Driver { drv, err := dbmate.New(u).Driver() require.NoError(t, err) @@ -20,12 +19,20 @@ func testClickHouseDriverURL(t *testing.T, url string) *Driver { } func testClickHouseDriver(t *testing.T) *Driver { - url := os.Getenv("CLICKHOUSE_TEST_URL") - if url == "" { - t.Skip("no CLICKHOUSE_TEST_URL provided") - } + u := dbtest.GetenvURLOrSkip(t, "CLICKHOUSE_TEST_URL") + return testClickHouseDriverURL(t, u) +} + +func testClickHouseDriverCluster01(t *testing.T) *Driver { + u := dbtest.GetenvURLOrSkip(t, "CLICKHOUSE_CLUSTER_01_TEST_URL") + u.RawQuery = "on_cluster" + return testClickHouseDriverURL(t, u) +} - return testClickHouseDriverURL(t, url) +func testClickHouseDriverCluster02(t *testing.T) *Driver { + u := dbtest.GetenvURLOrSkip(t, "CLICKHOUSE_CLUSTER_02_TEST_URL") + u.RawQuery = "on_cluster" + return testClickHouseDriverURL(t, u) } func prepTestClickHouseDB(t *testing.T, drv *Driver) *sql.DB { diff --git a/pkg/driver/clickhouse/cluster_parameters_test.go b/pkg/driver/clickhouse/cluster_parameters_test.go index fb8c29f7..2651acb1 100644 --- a/pkg/driver/clickhouse/cluster_parameters_test.go +++ b/pkg/driver/clickhouse/cluster_parameters_test.go @@ -3,9 +3,9 @@ package clickhouse import ( "testing" - "github.com/stretchr/testify/require" + "github.com/amacneil/dbmate/v2/pkg/dbtest" - "github.com/amacneil/dbmate/v2/pkg/dbutil" + "github.com/stretchr/testify/require" ) func TestOnCluster(t *testing.T) { @@ -25,7 +25,7 @@ func TestOnCluster(t *testing.T) { for _, c := range cases { t.Run(c.input, func(t *testing.T) { - u := dbutil.MustParseURL(c.input) + u := dbtest.MustParseURL(t, c.input) actual := extractOnCluster(u) require.Equal(t, c.expected, actual) @@ -46,7 +46,7 @@ func TestClusterMacro(t *testing.T) { for _, c := range cases { t.Run(c.input, func(t *testing.T) { - u := dbutil.MustParseURL(c.input) + u := dbtest.MustParseURL(t, c.input) actual := extractClusterMacro(u) require.Equal(t, c.expected, actual) @@ -67,7 +67,7 @@ func TestReplicaMacro(t *testing.T) { for _, c := range cases { t.Run(c.input, func(t *testing.T) { - u := dbutil.MustParseURL(c.input) + u := dbtest.MustParseURL(t, c.input) actual := extractReplicaMacro(u) require.Equal(t, c.expected, actual) @@ -88,7 +88,7 @@ func TestZookeeperPath(t *testing.T) { for _, c := range cases { t.Run(c.input, func(t *testing.T) { - u := dbutil.MustParseURL(c.input) + u := dbtest.MustParseURL(t, c.input) actual := extractZookeeperPath(u) require.Equal(t, c.expected, actual) diff --git a/pkg/driver/mysql/mysql_test.go b/pkg/driver/mysql/mysql_test.go index 3adef6a9..048b205f 100644 --- a/pkg/driver/mysql/mysql_test.go +++ b/pkg/driver/mysql/mysql_test.go @@ -3,22 +3,17 @@ package mysql import ( "database/sql" "net/url" - "os" "testing" "github.com/amacneil/dbmate/v2/pkg/dbmate" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" "github.com/stretchr/testify/require" ) func testMySQLDriver(t *testing.T) *Driver { - url := os.Getenv("MYSQL_TEST_URL") - if url == "" { - t.Skip("no MYSQL_TEST_URL provided") - } - - u := dbutil.MustParseURL(url) + u := dbtest.GetenvURLOrSkip(t, "MYSQL_TEST_URL") drv, err := dbmate.New(u).Driver() require.NoError(t, err) @@ -44,7 +39,7 @@ func prepTestMySQLDB(t *testing.T) *sql.DB { } func TestGetDriver(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("mysql://")) + db := dbmate.New(dbtest.MustParseURL(t, "mysql://")) drvInterface, err := db.Driver() require.NoError(t, err) @@ -152,7 +147,7 @@ func TestMySQLCreateDropDatabase(t *testing.T) { func TestMySQLDumpArgs(t *testing.T) { drv := testMySQLDriver(t) - drv.databaseURL = dbutil.MustParseURL("mysql://bob/mydb") + drv.databaseURL = dbtest.MustParseURL(t, "mysql://bob/mydb") require.Equal(t, []string{"--opt", "--routines", @@ -162,7 +157,7 @@ func TestMySQLDumpArgs(t *testing.T) { "--host=bob", "mydb"}, drv.mysqldumpArgs()) - drv.databaseURL = dbutil.MustParseURL("mysql://alice:pw@bob:5678/mydb") + drv.databaseURL = dbtest.MustParseURL(t, "mysql://alice:pw@bob:5678/mydb") require.Equal(t, []string{"--opt", "--routines", "--no-data", @@ -174,7 +169,7 @@ func TestMySQLDumpArgs(t *testing.T) { "--password=pw", "mydb"}, drv.mysqldumpArgs()) - drv.databaseURL = dbutil.MustParseURL("mysql://alice:pw@bob:5678/mydb?socket=/var/run/mysqld/mysqld.sock") + drv.databaseURL = dbtest.MustParseURL(t, "mysql://alice:pw@bob:5678/mydb?socket=/var/run/mysqld/mysqld.sock") require.Equal(t, []string{"--opt", "--routines", "--no-data", diff --git a/pkg/driver/postgres/postgres.go b/pkg/driver/postgres/postgres.go index d0240db8..8f91a408 100644 --- a/pkg/driver/postgres/postgres.go +++ b/pkg/driver/postgres/postgres.go @@ -91,8 +91,11 @@ func connectionString(u *url.URL) string { return out.String() } -func connectionArgsForDump(u *url.URL) []string { - u = dbutil.MustParseURL(connectionString(u)) +func connectionArgsForDump(conn *url.URL) []string { + u, err := url.Parse(connectionString(conn)) + if err != nil { + panic(err) + } // find schemas from search_path query := u.Query() diff --git a/pkg/driver/postgres/postgres_test.go b/pkg/driver/postgres/postgres_test.go index 605d905c..9b15e65f 100644 --- a/pkg/driver/postgres/postgres_test.go +++ b/pkg/driver/postgres/postgres_test.go @@ -4,23 +4,18 @@ import ( "database/sql" "fmt" "net/url" - "os" "runtime" "testing" "github.com/amacneil/dbmate/v2/pkg/dbmate" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" "github.com/stretchr/testify/require" ) func testPostgresDriver(t *testing.T) *Driver { - url := os.Getenv("POSTGRES_TEST_URL") - if url == "" { - t.Skip("no POSTGRES_TEST_URL provided") - } - - u := dbutil.MustParseURL(url) + u := dbtest.GetenvURLOrSkip(t, "POSTGRES_TEST_URL") drv, err := dbmate.New(u).Driver() require.NoError(t, err) @@ -28,12 +23,7 @@ func testPostgresDriver(t *testing.T) *Driver { } func testRedshiftDriver(t *testing.T) *Driver { - url := os.Getenv("REDSHIFT_TEST_URL") - if url == "" { - t.Skip("no REDSHIFT_TEST_URL provided") - } - - u := dbutil.MustParseURL(url) + u := dbtest.GetenvURLOrSkip(t, "REDSHIFT_TEST_URL") drv, err := dbmate.New(u).Driver() require.NoError(t, err) @@ -75,7 +65,7 @@ func prepRedshiftTestDB(t *testing.T, drv *Driver) *sql.DB { } func TestGetDriver(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("postgres://")) + db := dbmate.New(dbtest.MustParseURL(t, "postgres://")) drvInterface, err := db.Driver() require.NoError(t, err) diff --git a/pkg/driver/sqlite/sqlite_test.go b/pkg/driver/sqlite/sqlite_test.go index 9d1d1bc7..895ef994 100644 --- a/pkg/driver/sqlite/sqlite_test.go +++ b/pkg/driver/sqlite/sqlite_test.go @@ -9,13 +9,14 @@ import ( "testing" "github.com/amacneil/dbmate/v2/pkg/dbmate" + "github.com/amacneil/dbmate/v2/pkg/dbtest" "github.com/amacneil/dbmate/v2/pkg/dbutil" "github.com/stretchr/testify/require" ) func testSQLiteDriver(t *testing.T) *Driver { - u := dbutil.MustParseURL("sqlite:dbmate_test.sqlite3") + u := dbtest.MustParseURL(t, "sqlite:dbmate_test.sqlite3") drv, err := dbmate.New(u).Driver() require.NoError(t, err) @@ -41,7 +42,7 @@ func prepTestSQLiteDB(t *testing.T) *sql.DB { } func TestGetDriver(t *testing.T) { - db := dbmate.New(dbutil.MustParseURL("sqlite://")) + db := dbmate.New(dbtest.MustParseURL(t, "sqlite://")) drvInterface, err := db.Driver() require.NoError(t, err) @@ -54,86 +55,86 @@ func TestGetDriver(t *testing.T) { func TestConnectionString(t *testing.T) { t.Run("relative", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:foo/bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:foo/bar.sqlite3?mode=ro") require.Equal(t, "foo/bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("relative with dot", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:./foo/bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:./foo/bar.sqlite3?mode=ro") require.Equal(t, "./foo/bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("relative with double dot", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:../foo/bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:../foo/bar.sqlite3?mode=ro") require.Equal(t, "../foo/bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("absolute", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:/tmp/foo.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:/tmp/foo.sqlite3?mode=ro") require.Equal(t, "/tmp/foo.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("two slashes", func(t *testing.T) { // interpreted as absolute path - u := dbutil.MustParseURL("sqlite://tmp/foo.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite://tmp/foo.sqlite3?mode=ro") require.Equal(t, "/tmp/foo.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("three slashes", func(t *testing.T) { // interpreted as absolute path - u := dbutil.MustParseURL("sqlite:///tmp/foo.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:///tmp/foo.sqlite3?mode=ro") require.Equal(t, "/tmp/foo.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("four slashes", func(t *testing.T) { // interpreted as absolute path // supported for backwards compatibility - u := dbutil.MustParseURL("sqlite:////tmp/foo.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:////tmp/foo.sqlite3?mode=ro") require.Equal(t, "/tmp/foo.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("relative with space", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:foo bar.sqlite3?mode=ro") require.Equal(t, "foo bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("relative with space and dot", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:./foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:./foo bar.sqlite3?mode=ro") require.Equal(t, "./foo bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("relative with space and double dot", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:../foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:../foo bar.sqlite3?mode=ro") require.Equal(t, "../foo bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("absolute with space", func(t *testing.T) { - u := dbutil.MustParseURL("sqlite:/foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:/foo bar.sqlite3?mode=ro") require.Equal(t, "/foo bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("two slashes with space in path", func(t *testing.T) { // interpreted as absolute path - u := dbutil.MustParseURL("sqlite://tmp/foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite://tmp/foo bar.sqlite3?mode=ro") require.Equal(t, "/tmp/foo bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("three slashes with space in path", func(t *testing.T) { // interpreted as absolute path - u := dbutil.MustParseURL("sqlite:///tmp/foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:///tmp/foo bar.sqlite3?mode=ro") require.Equal(t, "/tmp/foo bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("three slashes with space in path (1st dir)", func(t *testing.T) { // interpreted as absolute path - u := dbutil.MustParseURL("sqlite:///tm p/foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:///tm p/foo bar.sqlite3?mode=ro") require.Equal(t, "/tm p/foo bar.sqlite3?mode=ro", ConnectionString(u)) }) t.Run("four slashes with space", func(t *testing.T) { // interpreted as absolute path // supported for backwards compatibility - u := dbutil.MustParseURL("sqlite:////tmp/foo bar.sqlite3?mode=ro") + u := dbtest.MustParseURL(t, "sqlite:////tmp/foo bar.sqlite3?mode=ro") require.Equal(t, "/tmp/foo bar.sqlite3?mode=ro", ConnectionString(u)) }) } @@ -198,7 +199,7 @@ func TestSQLiteDumpSchema(t *testing.T) { require.NotContains(t, string(schema), "sqlite_") // DumpSchema should return error if command fails - drv.databaseURL = dbutil.MustParseURL(".") + drv.databaseURL = dbtest.MustParseURL(t, ".") schema, err = drv.DumpSchema(db) require.Nil(t, schema) require.Error(t, err)