diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..2e75761 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,48 @@ +linters: + enable: + - errcheck + - gocritic + - goimports + - golint + - govet + - megacheck + + disable-all: true + +linters-settings: + # Enable-checks are based on the gocritic check list tagged as "stable" check here: + # https://github.com/go-critic/go-critic/blob/e92184cb98471e662585b9b49e9b133cab72e20a/checkers/checkers_test.go#L63 + gocritic: + enabled-checks: + - appendAssign + - appendCombine + - assignOp + - builtinShadow + - captLocal + - caseOrder + - defaultCaseOrder + - dupArg + - dupBranchBody + - dupCase + - elseif + - flagDeref + - ifElseChain + - importShadow + - indexAlloc + - paramTypeCombine + - rangeExprCopy + - rangeValCopy + - regexpMust + - singleCaseSwitch + - sloppyLen + - switchTrue + - typeSwitchVar + - typeUnparen + - underef + - unlambda + - unslice + - dupSubExpr + - hugeParam + +issues: + exclude-use-default: false diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..3c70016 --- /dev/null +++ b/Makefile @@ -0,0 +1,7 @@ +.PHONY: sanity-check +sanity-check: golangci-lint + +.PHONY: golangci-lint +golangci-lint: + @echo "Running golangci-lint..." + golangci-lint run ./... \ No newline at end of file diff --git a/README.md b/README.md index b0705e9..64e7824 100644 --- a/README.md +++ b/README.md @@ -207,5 +207,15 @@ Phoenix driver, which uses the scheme to determine if we should connect over `ht project is structured, it was also almost impossible to add support for embeddable migration files without major changes. +## Contributing +We run automatically some sanitizations to check code quality before merging it. For this purpose, +we use the [golangci-lint](https://github.com/golangci/golangci-lint) tool which is run via a [Makefile](Makefile) +target. + +It is a recommended practice to run and ensure passing all the checks locally before submitting a pull request. + +After [installing golangci-lint](https://github.com/golangci/golangci-lint) running a `make sanity-check` locally +would trigger this tests. + ## License This library is licensed under the Apache 2 License. \ No newline at end of file diff --git a/driver/golang/golang.go b/driver/golang/golang.go index 619b56d..861a79c 100644 --- a/driver/golang/golang.go +++ b/driver/golang/golang.go @@ -9,12 +9,13 @@ import ( m "github.com/Boostport/migration" ) +// Source implements migration.Source type Source struct { sync.Mutex migrations map[string]func() error } -// NewGolangSource creates a source for storing Go functions as migrations. +// NewSource creates a source for storing Go functions as migrations. func NewSource() *Source { return &Source{ migrations: map[string]func() error{}, @@ -73,17 +74,20 @@ func (s *Source) GetMigrationFile(file string) (io.Reader, error) { return strings.NewReader(""), nil } +// Driver is the golang migration.Driver implementation type Driver struct { source *Source updateVersion UpdateVersion applied AppliedVersions } +// UpdateVersion takes an id and a direction and returns an error if something fails type UpdateVersion func(id string, direction m.Direction) error +// AppliedVersions returns a list of applied versions and an error if something fails type AppliedVersions func() ([]string, error) -// NewGolang creates a new Go migration driver. It requires a source a function for saving the executed migration version, a function for deleting a version +// New creates a new Go migration driver. It requires a source a function for saving the executed migration version, a function for deleting a version // that was migrated downwards, a function for listing all applied migrations and optionally a configuration. func New(source *Source, updateVersion UpdateVersion, applied AppliedVersions) (m.Driver, error) { return &Driver{ @@ -93,12 +97,13 @@ func New(source *Source, updateVersion UpdateVersion, applied AppliedVersions) ( }, nil } +// Close is the migration.Driver implementation of io.Closer func (g *Driver) Close() error { return nil } +// Migrate executes a planned migration func (g *Driver) Migrate(migration *m.PlannedMigration) error { - file := migration.ID if migration.Direction == m.Up { @@ -124,7 +129,7 @@ func (g *Driver) Migrate(migration *m.PlannedMigration) error { return nil } -// Version returns all applied migration versions +// Versions returns all applied migration versions func (g *Driver) Versions() ([]string, error) { return g.applied() } diff --git a/driver/mysql/mysql.go b/driver/mysql/mysql.go index 78fa347..34c6e68 100644 --- a/driver/mysql/mysql.go +++ b/driver/mysql/mysql.go @@ -11,6 +11,7 @@ import ( "github.com/go-sql-driver/mysql" ) +// Driver is the mysql migration.Driver implementation type Driver struct { db *sql.DB } @@ -50,8 +51,8 @@ func New(dsn string) (m.Driver, error) { return d, nil } +// NewFromDB returns a mysql driver from a sql.DB func NewFromDB(db *sql.DB) (m.Driver, error) { - if _, ok := db.Driver().(*mysql.MySQLDriver); !ok { return nil, errors.New("database instance is not using the MySQL driver") } @@ -132,7 +133,9 @@ func (driver *Driver) Versions() ([]string, error) { return versions, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() for rows.Next() { var version string diff --git a/driver/mysql/mysql_test.go b/driver/mysql/mysql_test.go index 24328a1..d80cd3a 100644 --- a/driver/mysql/mysql_test.go +++ b/driver/mysql/mysql_test.go @@ -12,7 +12,6 @@ import ( ) func TestMySQLDriver(t *testing.T) { - mysqlHost := os.Getenv("MYSQL_HOST") database := "migrationtest" @@ -24,7 +23,12 @@ func TestMySQLDriver(t *testing.T) { t.Fatal(err) } - defer connection.Close() + defer func() { + err := connection.Close() + if err != nil { + t.Errorf("unexpected error while closing the mysql connection: %v", err) + } + }() _, err = connection.Exec("CREATE DATABASE IF NOT EXISTS " + database) @@ -41,13 +45,21 @@ func TestMySQLDriver(t *testing.T) { driver, err := New("root:@tcp(" + mysqlHost + ")/" + database + "?multiStatements=true") if err != nil { - t.Errorf("Unable to open connection to mysql server: %s", err) + t.Errorf("unable to open connection to mysql server: %s", err) } - defer driver.Close() + defer func() { + err := driver.Close() + if err != nil { + t.Errorf("unexpected error while closing the mysql driver: %v", err) + } + }() defer func() { - connection.Exec("DROP DATABASE IF EXISTS " + database) + _, err := connection.Exec("DROP DATABASE IF EXISTS " + database) + if err != nil { + t.Errorf("unexpected error while dropping the mysql database %s: %v", database, err) + } }() migrations := []*migration.PlannedMigration{ @@ -100,49 +112,49 @@ func TestMySQLDriver(t *testing.T) { err = driver.Migrate(migrations[0]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } _, err = connection.Exec("INSERT INTO test_table1 (id) values (1)") if err != nil { - t.Errorf("Unexpected error while testing if migration succeeded: %s", err) + t.Errorf("unexpected error while testing if migration succeeded: %s", err) } _, err = connection.Exec("INSERT INTO test_table2 (id) values (1)") if err != nil { - t.Errorf("Unexpected error while testing if migration succeeded: %s", err) + t.Errorf("unexpected error while testing if migration succeeded: %s", err) } err = driver.Migrate(migrations[1]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } if _, err = connection.Exec("INSERT INTO test_table2 (id) values (1)"); err != nil { if err.(*mysql.MySQLError).Number != 1146 { - t.Errorf("Received an error while inserting into a non-existent table, but it was not a table_undefined error: %s", err) + t.Errorf("received an error while inserting into a non-existent table, but it was not a table_undefined error: %s", err) } } else { - t.Error("Expected an error while inserting into non-existent table, but did not receive any.") + t.Error("expected an error while inserting into non-existent table, but did not receive any.") } err = driver.Migrate(migrations[2]) if err == nil { - t.Error("Expected an error while executing invalid statement, but did not receive any.") + t.Error("expected an error while executing invalid statement, but did not receive any.") } versions, err := driver.Versions() if err != nil { - t.Errorf("Unexpected error while retriving version information: %s", err) + t.Errorf("unexpected error while retriving version information: %s", err) } if len(versions) != 2 { - t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions)) + t.Errorf("expected %d versions to be applied, %d was actually applied.", 2, len(versions)) } migrations[1].Direction = migration.Down @@ -150,32 +162,31 @@ func TestMySQLDriver(t *testing.T) { err = driver.Migrate(migrations[1]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } versions, err = driver.Versions() if err != nil { - t.Errorf("Unexpected error while retriving version information: %s", err) + t.Errorf("unexpected error while retriving version information: %s", err) } if len(versions) != 1 { - t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions)) + t.Errorf("expected %d versions to be applied, %d was actually applied.", 2, len(versions)) } } func TestCreateDriverUsingInvalidDBInstance(t *testing.T) { - db, _, err := sqlmock.New() if err != nil { - t.Fatalf("Error opening stub database connection: %s", err) + t.Fatalf("error opening stub database connection: %s", err) } _, err = NewFromDB(db) if err == nil { - t.Error("Expected error when creating MySQL driver with a non-MySQL database instance, but there was no error") + t.Error("expected error when creating MySQL driver with a non-MySQL database instance, but there was no error") } } @@ -197,23 +208,36 @@ func TestCreateDriverUsingDBInstance(t *testing.T) { t.Fatal(err) } - defer connection.Close() + defer func() { + err := connection.Close() + if err != nil { + t.Errorf("unexpected error while closing the mysql connection: %v", err) + } + }() defer func() { - connection.Exec("DROP DATABASE IF EXISTS " + database) + _, err := connection.Exec("DROP DATABASE IF EXISTS " + database) + if err != nil { + t.Errorf("unexpected error while dropping the mysql database %s: %v", database, err) + } }() db, err := sql.Open("mysql", "root:@tcp("+mysqlHost+")/"+database+"?multiStatements=true") if err != nil { - t.Fatalf("Could not open MySQL connection: %s", err) + t.Fatalf("could not open MySQL connection: %s", err) } driver, err := NewFromDB(db) if err != nil { - t.Errorf("Unable to create MySQL driver: %s", err) + t.Errorf("unable to create MySQL driver: %s", err) } - defer driver.Close() + defer func() { + err := driver.Close() + if err != nil { + t.Errorf("unexpected error while closing the mysql driver: %v", err) + } + }() } diff --git a/driver/phoenix/phoenix.go b/driver/phoenix/phoenix.go index 04541db..119068a 100644 --- a/driver/phoenix/phoenix.go +++ b/driver/phoenix/phoenix.go @@ -8,9 +8,10 @@ import ( m "github.com/Boostport/migration" "github.com/Boostport/migration/parser" - "github.com/apache/calcite-avatica-go/v3" + avatica "github.com/apache/calcite-avatica-go/v3" ) +// Driver is the phoenix migration.Driver implementation type Driver struct { db *sql.DB } @@ -20,7 +21,6 @@ const phoenixTableName = "schema_migration" // New creates a new Apache Avatica Driver. // The DSN is documented here: https://calcite.apache.org/avatica/docs/go_client_reference.html#dsn-data-source-name func New(dsn string) (m.Driver, error) { - db, err := sql.Open("avatica", dsn) if err != nil { @@ -42,8 +42,8 @@ func New(dsn string) (m.Driver, error) { return p, nil } +// NewFromDB returns a phoenix driver from a sql.DB func NewFromDB(db *sql.DB) (m.Driver, error) { - if _, ok := db.Driver().(*avatica.Driver); !ok { return nil, errors.New("database instance is not using the avatica driver") } @@ -76,22 +76,17 @@ func (driver *Driver) ensureVersionTableExists() error { // Migrate runs a migration. func (driver *Driver) Migrate(migration *m.PlannedMigration) error { - // TODO: Driver does not support DDL statements in a transaction yet :( See PHOENIX-3358 var migrationStatements *parser.ParsedMigration if migration.Direction == m.Up { - migrationStatements = migration.Up - } else if migration.Direction == m.Down { - migrationStatements = migration.Down } for _, sqlStmt := range migrationStatements.Statements { - // Special case for Phoenix. We force a statement split here, because Phoenix SQL statements must not be terminated with ;. // In addition, this explicitly splits the SQL statements into its constituent statements. splitted := strings.Split(sqlStmt, ";") @@ -130,7 +125,9 @@ func (driver *Driver) Versions() ([]string, error) { return versions, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() for rows.Next() { var version string diff --git a/driver/phoenix/phoenix_test.go b/driver/phoenix/phoenix_test.go index 874bec4..188c2b8 100644 --- a/driver/phoenix/phoenix_test.go +++ b/driver/phoenix/phoenix_test.go @@ -12,7 +12,6 @@ import ( ) func TestPhoenixDriver(t *testing.T) { - phoenixHost := os.Getenv("PHOENIX_HOST") // prepare clean database @@ -22,7 +21,12 @@ func TestPhoenixDriver(t *testing.T) { t.Fatal(err) } - defer connection.Close() + defer func() { + err := connection.Close() + if err != nil { + t.Errorf("unexpected error while closing the phoenix connection: %v", err) + } + }() schema := "migrationtest" @@ -38,13 +42,30 @@ func TestPhoenixDriver(t *testing.T) { t.Errorf("Unable to open connection to phoenix server: %s", err) } - defer driver.Close() + defer func() { + err := driver.Close() + if err != nil { + t.Errorf("unexpected error while closing the phoenix driver: %v", err) + } + }() defer func() { - connection.Exec("DROP TABLE IF EXISTS test_table1") - connection.Exec("DROP TABLE IF EXISTS test_table2") - connection.Exec("DROP TABLE IF EXISTS schema_migration") - connection.Exec("DROP SCHEMA IF EXISTS " + schema) + _, err := connection.Exec("DROP TABLE IF EXISTS test_table1") + if err != nil { + t.Errorf("unexpected error while dropping the phoenix table: %v", err) + } + _, err = connection.Exec("DROP TABLE IF EXISTS test_table2") + if err != nil { + t.Errorf("unexpected error while dropping the phoenix table: %v", err) + } + _, err = connection.Exec("DROP TABLE IF EXISTS schema_migration") + if err != nil { + t.Errorf("unexpected error while dropping the phoenix table: %v", err) + } + _, err = connection.Exec("DROP SCHEMA IF EXISTS " + schema) + if err != nil { + t.Errorf("unexpected error while dropping the phoenix schema %s: %v", schema, err) + } }() migrations := []*migration.PlannedMigration{ @@ -186,32 +207,45 @@ func TestCreateDriverUsingDBInstance(t *testing.T) { t.Fatal(err) } - defer connection.Close() + defer func() { + err := connection.Close() + if err != nil { + t.Errorf("unexpected error while closing the phoenix connection: %v", err) + } + }() schema := "migrationtest" _, err = connection.Exec("CREATE SCHEMA IF NOT EXISTS " + schema) - if err != nil { t.Fatal(err) } defer func() { - connection.Exec("DROP TABLE IF EXISTS schema_migration") - connection.Exec("DROP SCHEMA IF EXISTS " + schema) + _, err := connection.Exec("DROP TABLE IF EXISTS schema_migration") + if err != nil { + t.Errorf("unexpected error while dropping the phoenix table: %v", err) + } + _, err = connection.Exec("DROP SCHEMA IF EXISTS " + schema) + if err != nil { + t.Errorf("unexpected error while dropping the phoenix schema %s: %v", schema, err) + } }() db, err := sql.Open("avatica", phoenixHost+"/") - if err != nil { t.Fatalf("Could not open avatica connection: %s", err) } driver, err := NewFromDB(db) - if err != nil { t.Errorf("Unable to create Avatica driver: %s", err) } - defer driver.Close() + defer func() { + err := driver.Close() + if err != nil { + t.Errorf("unexpected error while closing the phoenix driver: %v", err) + } + }() } diff --git a/driver/postgres/postgres.go b/driver/postgres/postgres.go index 649ab7e..93699b0 100644 --- a/driver/postgres/postgres.go +++ b/driver/postgres/postgres.go @@ -10,16 +10,16 @@ import ( "github.com/lib/pq" ) +// Driver is the postgres migration.Driver implementation type Driver struct { db *sql.DB } const postgresTableName = "schema_migration" -// NewPostgres creates a new Driver driver. +// New creates a new Driver driver. // The DSN is documented here: https://godoc.org/github.com/lib/pq#hdr-Connection_String_Parameters func New(dsn string) (m.Driver, error) { - db, err := sql.Open("postgres", dsn) if err != nil { @@ -41,8 +41,8 @@ func New(dsn string) (m.Driver, error) { return d, nil } +// NewFromDB returns a postgres driver from a sql.DB func NewFromDB(db *sql.DB) (m.Driver, error) { - if _, ok := db.Driver().(*pq.Driver); !ok { return nil, errors.New("database instance is not using the postgres driver") } @@ -103,7 +103,7 @@ func (driver *Driver) Migrate(migration *m.PlannedMigration) (err error) { defer func() { if err != nil { if errRb := tx.Rollback(); errRb != nil { - err = fmt.Errorf("Error rolling back: %s\n%s", errRb, err) + err = fmt.Errorf("error rolling back: %s\n%s", errRb, err) } return } @@ -112,24 +112,24 @@ func (driver *Driver) Migrate(migration *m.PlannedMigration) (err error) { for _, statement := range migrationStatements.Statements { if _, err = tx.Exec(statement); err != nil { - return fmt.Errorf("Error executing statement: %s\n%s", err, statement) + return fmt.Errorf("error executing statement: %s\n%s", err, statement) } } if _, err = tx.Exec(insertVersion, migration.ID); err != nil { - return fmt.Errorf("Error updating migration versions: %s", err) + return fmt.Errorf("error updating migration versions: %s", err) } } else { for _, statement := range migrationStatements.Statements { if _, err := driver.db.Exec(statement); err != nil { - return fmt.Errorf("Error executing statement: %s\n%s", err, statement) + return fmt.Errorf("error executing statement: %s\n%s", err, statement) } } if _, err = driver.db.Exec(insertVersion, migration.ID); err != nil { - return fmt.Errorf("Error updating migration versions: %s", err) + return fmt.Errorf("error updating migration versions: %s", err) } } @@ -146,7 +146,9 @@ func (driver *Driver) Versions() ([]string, error) { return versions, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() for rows.Next() { var version string diff --git a/driver/postgres/postgres_test.go b/driver/postgres/postgres_test.go index d1e67fa..f944430 100644 --- a/driver/postgres/postgres_test.go +++ b/driver/postgres/postgres_test.go @@ -24,7 +24,12 @@ func TestPostgresDriver(t *testing.T) { t.Fatal(err) } - defer connection.Close() + defer func() { + err := connection.Close() + if err != nil { + t.Errorf("unexpected error while closing the postgres connection: %v", err) + } + }() _, err = connection.Exec("CREATE DATABASE " + database) @@ -33,7 +38,10 @@ func TestPostgresDriver(t *testing.T) { } defer func() { - connection.Exec("DROP DATABASE IF EXISTS " + database) + _, err := connection.Exec("DROP DATABASE IF EXISTS " + database) + if err != nil { + t.Errorf("unexpected error while dropping the postgres database %s: %v", database, err) + } }() connection2, err := sql.Open("postgres", "postgres://postgres:@"+postgresHost+"/"+database+"?sslmode=disable") @@ -42,12 +50,17 @@ func TestPostgresDriver(t *testing.T) { t.Fatal(err) } - defer connection2.Close() + defer func() { + err := connection2.Close() + if err != nil { + t.Errorf("unexpected error while closing the postgres connection: %v", err) + } + }() driver, err := New("postgres://postgres:@" + postgresHost + "/" + database + "?sslmode=disable") if err != nil { - t.Errorf("Unable to open connection to postgres server: %s", err) + t.Errorf("unable to open connection to postgres server: %s", err) } migrations := []*migration.PlannedMigration{ @@ -100,49 +113,49 @@ func TestPostgresDriver(t *testing.T) { err = driver.Migrate(migrations[0]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } _, err = connection2.Exec("INSERT INTO test_table1 (id) values (1)") if err != nil { - t.Errorf("Unexpected error while testing if migration succeeded: %s", err) + t.Errorf("unexpected error while testing if migration succeeded: %s", err) } _, err = connection2.Exec("INSERT INTO test_table2 (id) values (1)") if err != nil { - t.Errorf("Unexpected error while testing if migration succeeded: %s", err) + t.Errorf("unexpected error while testing if migration succeeded: %s", err) } err = driver.Migrate(migrations[1]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } if _, err = connection2.Exec("INSERT INTO test_table2 (id) values (1)"); err != nil { if err.(*pq.Error).Code.Name() != "undefined_table" { - t.Errorf("Received an error while inserting into a non-existent table, but it was not a undefined_table error: %s", err) + t.Errorf("received an error while inserting into a non-existent table, but it was not a undefined_table error: %s", err) } } else { - t.Error("Expected an error while inserting into non-existent table, but did not receive any.") + t.Error("expected an error while inserting into non-existent table, but did not receive any.") } err = driver.Migrate(migrations[2]) if err == nil { - t.Error("Expected an error while executing invalid statement, but did not receive any.") + t.Error("expected an error while executing invalid statement, but did not receive any.") } versions, err := driver.Versions() if err != nil { - t.Errorf("Unexpected error while retriving version information: %s", err) + t.Errorf("unexpected error while retriving version information: %s", err) } if len(versions) != 2 { - t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions)) + t.Errorf("expected %d versions to be applied, %d was actually applied.", 2, len(versions)) } migrations[1].Direction = migration.Down @@ -150,39 +163,40 @@ func TestPostgresDriver(t *testing.T) { err = driver.Migrate(migrations[1]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } versions, err = driver.Versions() if err != nil { - t.Errorf("Unexpected error while retriving version information: %s", err) + t.Errorf("unexpected error while retriving version information: %s", err) } if len(versions) != 1 { - t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions)) + t.Errorf("expected %d versions to be applied, %d was actually applied.", 2, len(versions)) } - driver.Close() + err = driver.Close() + if err != nil { + t.Errorf("unexpected error %v while closing the postgres driver.", err) + } } func TestCreateDriverUsingInvalidDBInstance(t *testing.T) { - db, _, err := sqlmock.New() if err != nil { - t.Fatalf("Error opening stub database connection: %s", err) + t.Fatalf("error opening stub database connection: %s", err) } _, err = NewFromDB(db) if err == nil { - t.Error("Expected error when creating Postgres driver with a non-Postgres database instance, but there was no error") + t.Error("expected error when creating Postgres driver with a non-Postgres database instance, but there was no error") } } func TestCreateDriverUsingDBInstance(t *testing.T) { - postgresHost := os.Getenv("POSTGRES_HOST") database := "migrationtest" @@ -194,7 +208,12 @@ func TestCreateDriverUsingDBInstance(t *testing.T) { t.Fatal(err) } - defer connection.Close() + defer func() { + err := connection.Close() + if err != nil { + t.Errorf("unexpected error while closing the postgres connection: %v", err) + } + }() _, err = connection.Exec("CREATE DATABASE " + database) @@ -203,20 +222,28 @@ func TestCreateDriverUsingDBInstance(t *testing.T) { } defer func() { - connection.Exec("DROP DATABASE IF EXISTS " + database) + _, err := connection.Exec("DROP DATABASE IF EXISTS " + database) + if err != nil { + t.Errorf("unexpected error while dropping the postgres database %s: %v", database, err) + } }() db, err := sql.Open("postgres", "postgres://postgres:@"+postgresHost+"/"+database+"?sslmode=disable") if err != nil { - t.Fatalf("Could not open Postgres connection: %s", err) + t.Fatalf("could not open Postgres connection: %s", err) } driver, err := NewFromDB(db) if err != nil { - t.Errorf("Unable to create Postgres driver: %s", err) + t.Errorf("unable to create postgres driver: %s", err) } - defer driver.Close() + defer func() { + err := driver.Close() + if err != nil { + t.Errorf("unexpected error %v while closing the postgres driver.", err) + } + }() } diff --git a/driver/sqlite/sqlite.go b/driver/sqlite/sqlite.go index cbcba20..94b009e 100644 --- a/driver/sqlite/sqlite.go +++ b/driver/sqlite/sqlite.go @@ -10,6 +10,7 @@ import ( "github.com/mattn/go-sqlite3" ) +// Driver is the sqlite migration.Driver implementation type Driver struct { db *sql.DB useTransactions bool @@ -17,7 +18,7 @@ type Driver struct { const sqliteTableName = "schema_migration" -// NewSQLite creates a new Driver driver. +// New creates a new Driver driver. // The DSN is documented here: https://godoc.org/github.com/mattn/go-sqlite3#SQLiteDriver.Open func New(dsn string, useTransactions bool) (m.Driver, error) { @@ -43,8 +44,8 @@ func New(dsn string, useTransactions bool) (m.Driver, error) { return d, nil } +// NewFromDB returns a sqlite driver from a sql.db func NewFromDB(db *sql.DB) (m.Driver, error) { - if _, ok := db.Driver().(*sqlite3.SQLiteDriver); !ok { return nil, errors.New("database instance is not using the postgres driver") } @@ -104,7 +105,7 @@ func (driver *Driver) Migrate(migration *m.PlannedMigration) (err error) { defer func() { if err != nil { if errRb := tx.Rollback(); errRb != nil { - err = fmt.Errorf("Error rolling back: %s\n%s", errRb, err) + err = fmt.Errorf("error rolling back: %s\n%s", errRb, err) } return } @@ -115,25 +116,25 @@ func (driver *Driver) Migrate(migration *m.PlannedMigration) (err error) { if _, err = tx.Exec(statement); err != nil { - return fmt.Errorf("Error executing statement: %s\n%s", err, statement) + return fmt.Errorf("error executing statement: %s\n%s", err, statement) } } if _, err = tx.Exec(insertVersion, migration.ID); err != nil { - return fmt.Errorf("Error updating migration versions: %s", err) + return fmt.Errorf("error updating migration versions: %s", err) } } else { for _, statement := range migrationStatements.Statements { if _, err := driver.db.Exec(statement); err != nil { - return fmt.Errorf("Error executing statement: %s\n%s", err, statement) + return fmt.Errorf("error executing statement: %s\n%s", err, statement) } } if _, err = driver.db.Exec(insertVersion, migration.ID); err != nil { - return fmt.Errorf("Error updating migration versions: %s", err) + return fmt.Errorf("error updating migration versions: %s", err) } } @@ -150,7 +151,9 @@ func (driver *Driver) Versions() ([]string, error) { return versions, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() for rows.Next() { var version string diff --git a/driver/sqlite/sqlite_test.go b/driver/sqlite/sqlite_test.go index bab88ce..0fcf5c3 100644 --- a/driver/sqlite/sqlite_test.go +++ b/driver/sqlite/sqlite_test.go @@ -12,13 +12,11 @@ import ( ) func TestSQLiteDriver(t *testing.T) { - for _, useTransactions := range []bool{true, false} { - driver, err := New("file::memory:?cache=shared&_busy_timeout=50000", useTransactions) if err != nil { - t.Errorf("Unable to open connection to server: %s", err) + t.Errorf("unable to open connection to server: %s", err) } migrations := []*migration.PlannedMigration{ @@ -71,25 +69,25 @@ func TestSQLiteDriver(t *testing.T) { err = driver.Migrate(migrations[0]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } _, err = driver.(*Driver).db.Exec("INSERT INTO test_table1 (id) values (1)") if err != nil { - t.Errorf("Unexpected error while testing if migration succeeded: %s", err) + t.Errorf("unexpected error while testing if migration succeeded: %s", err) } _, err = driver.(*Driver).db.Exec("INSERT INTO test_table2 (id) values (1)") if err != nil { - t.Errorf("Unexpected error while testing if migration succeeded: %s", err) + t.Errorf("unexpected error while testing if migration succeeded: %s", err) } err = driver.Migrate(migrations[1]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } if _, err := driver.(*Driver).db.Exec("INSERT INTO test_table2 (id) values (1)"); err != nil { @@ -97,26 +95,26 @@ func TestSQLiteDriver(t *testing.T) { reg := regexp.MustCompile(`^no such table: .+`) if !reg.MatchString(err.Error()) { - t.Errorf("Received an error while inserting into a non-existent table, but it was not a undefined_table error: %s", err) + t.Errorf("received an error while inserting into a non-existent table, but it was not a undefined_table error: %s", err) } } else { - t.Error("Expected an error while inserting into non-existent table, but did not receive any.") + t.Error("expected an error while inserting into non-existent table, but did not receive any.") } err = driver.Migrate(migrations[2]) if err == nil { - t.Error("Expected an error while executing invalid statement, but did not receive any.") + t.Error("expected an error while executing invalid statement, but did not receive any.") } versions, err := driver.Versions() if err != nil { - t.Errorf("Unexpected error while retriving version information: %s", err) + t.Errorf("unexpected error while retriving version information: %s", err) } if len(versions) != 2 { - t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions)) + t.Errorf("expected %d versions to be applied, %d was actually applied.", 2, len(versions)) } migrations[1].Direction = migration.Down @@ -124,42 +122,46 @@ func TestSQLiteDriver(t *testing.T) { err = driver.Migrate(migrations[1]) if err != nil { - t.Errorf("Unexpected error while running migration: %s", err) + t.Errorf("unexpected error while running migration: %s", err) } versions, err = driver.Versions() if err != nil { - t.Errorf("Unexpected error while retriving version information: %s", err) + t.Errorf("unexpected error while retriving version information: %s", err) } if len(versions) != 1 { - t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions)) + t.Errorf("expected %d versions to be applied, %d was actually applied", 2, len(versions)) } - driver.(*Driver).db.Exec("DROP TABLE IF EXISTS " + sqliteTableName) + _, err = driver.(*Driver).db.Exec("DROP TABLE IF EXISTS " + sqliteTableName) + if err != nil { + t.Errorf("unexpected error %v while droping the table: %s", err, sqliteTableName) + } - driver.Close() + err = driver.Close() + if err != nil { + t.Errorf("unexpected error %v while closing the sqlite driver", err) + } } } func TestCreateDriverUsingInvalidDBInstance(t *testing.T) { - db, _, err := sqlmock.New() if err != nil { - t.Fatalf("Error opening stub database connection: %s", err) + t.Fatalf("error opening stub database connection: %s", err) } _, err = NewFromDB(db) if err == nil { - t.Error("Expected error when creating SQLite driver with a non-SQLite database instance, but there was no error") + t.Error("expected error when creating SQLite driver with a non-SQLite database instance, but there was no error") } } func TestCreateDriverUsingDBInstance(t *testing.T) { - db, err := sql.Open("sqlite3", "file::memory:?cache=shared&_busy_timeout=50000") if err != nil { @@ -169,8 +171,13 @@ func TestCreateDriverUsingDBInstance(t *testing.T) { driver, err := NewFromDB(db) if err != nil { - t.Errorf("Unable to create SQLite driver: %s", err) + t.Errorf("unable to create SQLite driver: %s", err) } - defer driver.Close() + defer func() { + err := driver.Close() + if err != nil { + t.Errorf("unexpected error %v while closing the sqlite driver", err) + } + }() } diff --git a/migration.go b/migration.go index e8d2f96..3864de6 100644 --- a/migration.go +++ b/migration.go @@ -11,6 +11,7 @@ import ( "github.com/Boostport/migration/parser" ) +// Direction type up/down type Direction int // String returns a string representation of the direction @@ -25,6 +26,7 @@ func (d Direction) String() string { } } +// Constants for direction const ( Up Direction = iota Down @@ -64,6 +66,7 @@ func (m Migration) isNumeric() bool { return len(m.NumberPrefixMatches()) > 0 } +// NumberPrefixMatches returns a list of string matches func (m Migration) NumberPrefixMatches() []string { return numberPrefixRegex.FindStringSubmatch(m.ID) } diff --git a/migration_test.go b/migration_test.go index ca0dd53..8225249 100644 --- a/migration_test.go +++ b/migration_test.go @@ -33,7 +33,6 @@ func (m *mockDriver) Close() error { } func (m *mockDriver) Migrate(migration *PlannedMigration) error { - var migrationStatements *parser.ParsedMigration if migration.Direction == Up { @@ -49,7 +48,7 @@ func (m *mockDriver) Migrate(migration *PlannedMigration) error { } if strings.Contains(errStatement, "error") { - return errors.New("Error executing migration.") + return errors.New("error executing migration") } versionIndex := -1 diff --git a/parser/parser.go b/parser/parser.go index 56ba352..8e8a1ac 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -16,20 +16,19 @@ const ( optionEndStatement = "EndStatement" ) +// ParsedMigration is a parsed migration type ParsedMigration struct { UseTransaction bool Statements []string } func splitStatementsBySemicolon(buf string) []string { - statements := strings.SplitAfter(buf, ";") for i, statement := range statements { trimmed := strings.TrimSpace(statement) - if trimmed == "" && i > 0 { - statements[i-1] = statements[i-1] + statements[i] + statements[i-1] += statements[i] statements = statements[:i+copy(statements[i:], statements[i+1:])] } } @@ -37,8 +36,8 @@ func splitStatementsBySemicolon(buf string) []string { return statements } +// Parse reads a migration and returns a parsed migrations func Parse(r io.Reader) (*ParsedMigration, error) { - p := &ParsedMigration{ UseTransaction: true, Statements: []string{}, @@ -87,11 +86,8 @@ func Parse(r io.Reader) (*ParsedMigration, error) { buf.Reset() } - } else { - - if _, err := buf.WriteString(line); err != nil { + } else if _, err := buf.WriteString(line); err != nil { return p, errors.New("error writing line to buffer") - } } isFirstLine = false diff --git a/source.go b/source.go index fc1d9de..8b6c772 100644 --- a/source.go +++ b/source.go @@ -26,13 +26,13 @@ type GoBindataMigrationSource struct { Dir string } +// ListMigrationFiles returns a list of gobindata migration files func (a GoBindataMigrationSource) ListMigrationFiles() ([]string, error) { - return a.AssetDir(a.Dir) } +// GetMigrationFile gets a gobindata migration file func (a GoBindataMigrationSource) GetMigrationFile(name string) (io.Reader, error) { - file, err := a.Asset(path.Join(a.Dir, name)) if err != nil { @@ -42,13 +42,14 @@ func (a GoBindataMigrationSource) GetMigrationFile(name string) (io.Reader, erro return bytes.NewReader(file), nil } -// Avoids pulling in the packr library for everyone, mimics the bits of +// PackrBox avoids pulling in the packr library for everyone, mimics the bits of // packr.Box that we need. type PackrBox interface { List() []string Find(name string) ([]byte, error) } +// PackrMigrationSource holds the box and dir info type PackrMigrationSource struct { Box PackrBox @@ -56,8 +57,8 @@ type PackrMigrationSource struct { Dir string } -func (p PackrMigrationSource)ListMigrationFiles() ([]string, error) { - +// ListMigrationFiles returns a list of packr migration files +func (p PackrMigrationSource) ListMigrationFiles() ([]string, error) { files := p.Box.List() var migrations []string @@ -70,7 +71,7 @@ func (p PackrMigrationSource)ListMigrationFiles() ([]string, error) { prefix = fmt.Sprintf("%s/", dir) } - for _, file := range files{ + for _, file := range files { if !strings.HasPrefix(file, prefix) { continue @@ -88,7 +89,8 @@ func (p PackrMigrationSource)ListMigrationFiles() ([]string, error) { return migrations, nil } -func (p PackrMigrationSource)GetMigrationFile(name string) (io.Reader, error) { +// GetMigrationFile gets a packr migration file +func (p PackrMigrationSource) GetMigrationFile(name string) (io.Reader, error) { file, err := p.Box.Find(path.Join(p.Dir, name)) return bytes.NewReader(file), err @@ -100,8 +102,8 @@ type MemoryMigrationSource struct { Files map[string]string } +// ListMigrationFiles returns a list of memory migration files func (m MemoryMigrationSource) ListMigrationFiles() ([]string, error) { - var files []string for file := range m.Files { @@ -111,13 +113,13 @@ func (m MemoryMigrationSource) ListMigrationFiles() ([]string, error) { return files, nil } +// GetMigrationFile gets a memory migration file func (m MemoryMigrationSource) GetMigrationFile(name string) (io.Reader, error) { - content, ok := m.Files[name] if !ok { - return nil, fmt.Errorf("The migration file %s does not exist.", name) + return nil, fmt.Errorf("the migration file %s does not exist", name) } return strings.NewReader(content), nil -} \ No newline at end of file +} diff --git a/wercker.yml b/wercker.yml index 9523f1b..bf83f92 100644 --- a/wercker.yml +++ b/wercker.yml @@ -1,6 +1,8 @@ box: id: golang:1.11-alpine cmd: /bin/sh + env: + GO111MODULE: on no-response-timeout: 20 @@ -11,6 +13,7 @@ services: MYSQL_ALLOW_EMPTY_PASSWORD: yes - postgres:10-alpine build: + base-path: /go/src/github.com/Boostport/migration steps: - script: name: install code climate @@ -29,7 +32,18 @@ build: - script: name: set up build tools code: | - apk --no-cache --no-progress add build-base git + apk --no-cache --no-progress add build-base git make + + - script: + name: install golangci-lint + code: | + wget -O - -q https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOBIN) v1.12.5 + chmod +x $(go env GOBIN)/golangci-lint + + - script: + name: make sanity-check + code: | + make sanity-check - script: name: go test