From 32ad603fee14ce059e731171a0589847bdb55f08 Mon Sep 17 00:00:00 2001 From: Michael Fridman Date: Fri, 27 Jan 2023 08:30:31 -0500 Subject: [PATCH] Add golangci-lint and fix errors (#456) --- .github/workflows/lint.yml | 42 +++++++++++++++++++++++++++++ Makefile | 8 ++++++ cmd/goose/main.go | 5 +++- goose_test.go | 3 ++- internal/check/check.go | 8 ------ migrate.go | 4 +-- migration.go | 4 +-- migration_sql.go | 11 ++++---- tests/clickhouse/clickhouse_test.go | 6 ++--- tests/e2e/allow_missing_test.go | 2 +- tests/e2e/migrations_test.go | 10 +++---- tests/e2e/no_versioning_test.go | 2 +- tests/vertica/vertica_test.go | 2 +- 13 files changed, 77 insertions(+), 30 deletions(-) create mode 100644 .github/workflows/lint.yml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 000000000..7f1e0163f --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,42 @@ +name: golangci +on: + push: + branches: + - master + - main + pull_request: +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-go@v3 + with: + go-version: 1.19.x + check-latest: true + cache: true + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version + version: v1.50.1 + + # Optional: working directory, useful for monorepos + # working-directory: somedir + + # Optional: golangci-lint command line arguments. + # args: --issues-exit-code=0 + + # Optional: show only new issues if it's a pull request. The default value is `false`. + # only-new-issues: true + + # Optional: if set to true then the all caching functionality will be complete disabled, + # takes precedence over all other caching options. + # skip-cache: true + + # Optional: if set to true then the action don't cache or restore ~/go/pkg. + # skip-pkg-cache: true + + # Optional: if set to true then the action don't cache or restore ~/.cache/go-build. + # skip-build-cache: true diff --git a/Makefile b/Makefile index 45ccd86e7..1525a6bf2 100644 --- a/Makefile +++ b/Makefile @@ -40,3 +40,11 @@ start-postgres: .PHONY: clean clean: @find . -type f -name '*.FAIL' -delete + +.PHONY: lint +lint: tools + @golangci-lint run ./... --fix + +.PHONY: tools +tools: + @go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.1 diff --git a/cmd/goose/main.go b/cmd/goose/main.go index a98e59dad..bb03e3676 100644 --- a/cmd/goose/main.go +++ b/cmd/goose/main.go @@ -36,7 +36,10 @@ var ( func main() { flags.Usage = usage - flags.Parse(os.Args[1:]) + if err := flags.Parse(os.Args[1:]); err != nil { + log.Fatalf("failed to parse args: %v", err) + return + } if *version { if buildInfo, ok := debug.ReadBuildInfo(); ok && buildInfo != nil && gooseVersion == "" { diff --git a/goose_test.go b/goose_test.go index 285cf4ffb..422d3b090 100644 --- a/goose_test.go +++ b/goose_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/pressly/goose/v3/internal/check" _ "modernc.org/sqlite" ) @@ -172,7 +173,7 @@ func TestEmbeddedMigrations(t *testing.T) { } SetBaseFS(fsys) - SetDialect("sqlite3") + check.NoError(t, SetDialect("sqlite3")) t.Cleanup(func() { SetBaseFS(nil) }) t.Run("Migration cycle", func(t *testing.T) { diff --git a/internal/check/check.go b/internal/check/check.go index 177f7d022..96f242933 100644 --- a/internal/check/check.go +++ b/internal/check/check.go @@ -84,11 +84,3 @@ func reflectToInt64(v interface{}) (int64, error) { } return 0, fmt.Errorf("invalid number: must be int64 type: got:%T", v) } - -func reflectToStr(v interface{}) (string, error) { - switch typ := v.(type) { - case string: - return reflect.ValueOf(typ).String(), nil - } - return "", fmt.Errorf("invalid string: got:%T", v) -} diff --git a/migrate.go b/migrate.go index 0e93f8889..c92f7a8a2 100644 --- a/migrate.go +++ b/migrate.go @@ -346,14 +346,14 @@ func createVersionTable(db *sql.DB) error { d := GetDialect() if _, err := txn.Exec(d.createVersionTableSQL()); err != nil { - txn.Rollback() + _ = txn.Rollback() return err } version := 0 applied := true if _, err := txn.Exec(d.insertVersionSQL(), version, applied); err != nil { - txn.Rollback() + _ = txn.Rollback() return err } diff --git a/migration.go b/migration.go index 191063090..495eb757a 100644 --- a/migration.go +++ b/migration.go @@ -164,13 +164,13 @@ func runGoMigration( if fn != nil { // Run go migration function. if err := fn(tx); err != nil { - tx.Rollback() + _ = tx.Rollback() return fmt.Errorf("failed to run go migration: %w", err) } } if recordVersion { if err := insertOrDeleteVersion(tx, version, direction); err != nil { - tx.Rollback() + _ = tx.Rollback() return fmt.Errorf("failed to update version: %w", err) } } diff --git a/migration_sql.go b/migration_sql.go index 49968b33d..70c2e07fc 100644 --- a/migration_sql.go +++ b/migration_sql.go @@ -30,7 +30,7 @@ func runSQLMigration(db *sql.DB, statements []string, useTx bool, v int64, direc verboseInfo("Executing statement: %s\n", clearStatement(query)) if err = execQuery(tx.Exec, query); err != nil { verboseInfo("Rollback transaction") - tx.Rollback() + _ = tx.Rollback() return fmt.Errorf("failed to execute SQL query %q: %w", clearStatement(query), err) } } @@ -39,13 +39,13 @@ func runSQLMigration(db *sql.DB, statements []string, useTx bool, v int64, direc if direction { if err := execQuery(tx.Exec, GetDialect().insertVersionSQL(), v, direction); err != nil { verboseInfo("Rollback transaction") - tx.Rollback() + _ = tx.Rollback() return fmt.Errorf("failed to insert new goose version: %w", err) } } else { if err := execQuery(tx.Exec, GetDialect().deleteVersionSQL(), v); err != nil { verboseInfo("Rollback transaction") - tx.Rollback() + _ = tx.Rollback() return fmt.Errorf("failed to delete goose version: %w", err) } } @@ -95,12 +95,13 @@ func execQuery(fn func(string, ...interface{}) (sql.Result, error), query string }() t := time.Now() - + ticker := time.NewTicker(time.Minute) + defer ticker.Stop() for { select { case err := <-ch: return err - case <-time.Tick(time.Minute): + case <-ticker.C: verboseInfo("Executing statement still in progress for %v", time.Since(t).Round(time.Second)) } } diff --git a/tests/clickhouse/clickhouse_test.go b/tests/clickhouse/clickhouse_test.go index fb2643510..1d7242908 100644 --- a/tests/clickhouse/clickhouse_test.go +++ b/tests/clickhouse/clickhouse_test.go @@ -21,7 +21,7 @@ func TestClickUpDownAll(t *testing.T) { check.NoError(t, err) t.Cleanup(cleanup) - goose.SetDialect("clickhouse") + check.NoError(t, goose.SetDialect("clickhouse")) retryCheckTableMutation := func(table string) func() error { return func() error { @@ -88,7 +88,7 @@ func TestClickHouseFirstThree(t *testing.T) { check.NoError(t, err) t.Cleanup(cleanup) - goose.SetDialect("clickhouse") + check.NoError(t, goose.SetDialect("clickhouse")) err = goose.Up(db, migrationDir) check.NoError(t, err) @@ -158,7 +158,7 @@ func TestRemoteImportMigration(t *testing.T) { check.NoError(t, err) t.Cleanup(cleanup) - goose.SetDialect("clickhouse") + check.NoError(t, goose.SetDialect("clickhouse")) err = goose.Up(db, migrationDir) check.NoError(t, err) diff --git a/tests/e2e/allow_missing_test.go b/tests/e2e/allow_missing_test.go index 37a769c74..33cae57c5 100644 --- a/tests/e2e/allow_missing_test.go +++ b/tests/e2e/allow_missing_test.go @@ -321,7 +321,7 @@ func setupTestDB(t *testing.T, version int64) *sql.DB { db, err := newDockerDB(t) check.NoError(t, err) - goose.SetDialect(*dialect) + check.NoError(t, goose.SetDialect(*dialect)) // Create goose table. current, err := goose.EnsureDBVersion(db) diff --git a/tests/e2e/migrations_test.go b/tests/e2e/migrations_test.go index fa58a9cba..b0bd949ed 100644 --- a/tests/e2e/migrations_test.go +++ b/tests/e2e/migrations_test.go @@ -17,7 +17,7 @@ func TestMigrateUpWithReset(t *testing.T) { db, err := newDockerDB(t) check.NoError(t, err) - goose.SetDialect(*dialect) + check.NoError(t, goose.SetDialect(*dialect)) migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion) check.NoError(t, err) check.NumberNotZero(t, len(migrations)) @@ -46,7 +46,7 @@ func TestMigrateUpWithRedo(t *testing.T) { db, err := newDockerDB(t) check.NoError(t, err) - goose.SetDialect(*dialect) + check.NoError(t, goose.SetDialect(*dialect)) migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion) check.NoError(t, err) @@ -84,7 +84,7 @@ func TestMigrateUpTo(t *testing.T) { ) db, err := newDockerDB(t) check.NoError(t, err) - goose.SetDialect(*dialect) + check.NoError(t, goose.SetDialect(*dialect)) migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion) check.NoError(t, err) check.NumberNotZero(t, len(migrations)) @@ -106,7 +106,7 @@ func TestMigrateUpByOne(t *testing.T) { db, err := newDockerDB(t) check.NoError(t, err) - goose.SetDialect(*dialect) + check.NoError(t, goose.SetDialect(*dialect)) migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion) check.NoError(t, err) check.NumberNotZero(t, len(migrations)) @@ -137,7 +137,7 @@ func TestMigrateFull(t *testing.T) { db, err := newDockerDB(t) check.NoError(t, err) - goose.SetDialect(*dialect) + check.NoError(t, goose.SetDialect(*dialect)) migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion) check.NoError(t, err) check.NumberNotZero(t, len(migrations)) diff --git a/tests/e2e/no_versioning_test.go b/tests/e2e/no_versioning_test.go index c9767122c..9945c29f9 100644 --- a/tests/e2e/no_versioning_test.go +++ b/tests/e2e/no_versioning_test.go @@ -20,7 +20,7 @@ func TestNoVersioning(t *testing.T) { ) db, err := newDockerDB(t) check.NoError(t, err) - goose.SetDialect(*dialect) + check.NoError(t, goose.SetDialect(*dialect)) err = goose.Up(db, migrationsDir) check.NoError(t, err) diff --git a/tests/vertica/vertica_test.go b/tests/vertica/vertica_test.go index e36c2ac42..018b929ba 100644 --- a/tests/vertica/vertica_test.go +++ b/tests/vertica/vertica_test.go @@ -26,7 +26,7 @@ func TestVerticaUpDownAll(t *testing.T) { check.NoError(t, err) t.Cleanup(cleanup) - goose.SetDialect("vertica") + check.NoError(t, goose.SetDialect("vertica")) goose.SetTableName("goose_db_version")