testing: add testifylint and fix it all up (#843)

.golangci.yaml

@@ -0,0 +1,14 @@
+linters:
+  disable-all: true
+  enable:
+    # Default
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+    - gofmt
+    # Added
+    - testifylint
+    - misspell

database/store_test.go

@@ -35,10 +35,11 @@ func TestDialectStore(t *testing.T) {
 		db, err := sql.Open("sqlite", ":memory:")
 		require.NoError(t, err)
 		testStore(context.Background(), t, database.DialectSQLite3, db, func(t *testing.T, err error) {
+			t.Helper()
 			var sqliteErr *sqlite.Error
 			ok := errors.As(err, &sqliteErr)
 			require.True(t, ok)
-			require.Equal(t, sqliteErr.Code(), 1) // Generic error (SQLITE_ERROR)
+			require.Equal(t, 1, sqliteErr.Code()) // Generic error (SQLITE_ERROR)
 			require.Contains(t, sqliteErr.Error(), "table test_goose_db_version already exists")
 		})
 	})
@@ -58,11 +59,11 @@ func TestDialectStore(t *testing.T) {
 		require.NoError(t, insert(db, 2))
 		res, err := store.ListMigrations(context.Background(), db)
 		require.NoError(t, err)
-		require.Equal(t, len(res), 3)
+		require.Len(t, res, 3)
 		// Check versions are in descending order: [2, 3, 1]
-		require.EqualValues(t, res[0].Version, 2)
-		require.EqualValues(t, res[1].Version, 3)
-		require.EqualValues(t, res[2].Version, 1)
+		require.EqualValues(t, 2, res[0].Version)
+		require.EqualValues(t, 3, res[1].Version)
+		require.EqualValues(t, 1, res[2].Version)
 	})
 }
 
@@ -103,7 +104,7 @@ func testStore(
 	err = runConn(ctx, db, func(conn *sql.Conn) error {
 		res, err := store.ListMigrations(ctx, conn)
 		require.NoError(t, err)
-		require.Equal(t, len(res), 0)
+		require.Empty(t, res, 0)
 		return nil
 	})
 	require.NoError(t, err)
@@ -125,7 +126,7 @@ func testStore(
 	err = runConn(ctx, db, func(conn *sql.Conn) error {
 		res, err := store.ListMigrations(ctx, conn)
 		require.NoError(t, err)
-		require.Equal(t, len(res), 6)
+		require.Len(t, res, 6)
 		// Check versions are in descending order.
 		for i := 0; i < 6; i++ {
 			require.EqualValues(t, res[i].Version, 5-i)
@@ -151,7 +152,7 @@ func testStore(
 	err = runConn(ctx, db, func(conn *sql.Conn) error {
 		res, err := store.ListMigrations(ctx, conn)
 		require.NoError(t, err)
-		require.Equal(t, len(res), 3)
+		require.Len(t, res, 3)
 		// Check that the remaining versions are in descending order.
 		for i := 0; i < 3; i++ {
 			require.EqualValues(t, res[i].Version, 2-i)
@@ -165,8 +166,8 @@ func testStore(
 		err = runConn(ctx, db, func(conn *sql.Conn) error {
 			res, err := store.GetMigration(ctx, conn, int64(i))
 			require.NoError(t, err)
-			require.Equal(t, res.IsApplied, true)
-			require.Equal(t, res.Timestamp.IsZero(), false)
+			require.True(t, res.IsApplied)
+			require.False(t, res.Timestamp.IsZero())
 			return nil
 		})
 		require.NoError(t, err)
@@ -180,7 +181,7 @@ func testStore(
 		require.NoError(t, err)
 		latest, err := store.GetLatestVersion(ctx, tx)
 		require.NoError(t, err)
-		require.EqualValues(t, latest, 1)
+		require.EqualValues(t, 1, latest)
 		return nil
 	})
 	require.NoError(t, err)
@@ -190,7 +191,7 @@ func testStore(
 		require.NoError(t, err)
 		latest, err := store.GetLatestVersion(ctx, conn)
 		require.NoError(t, err)
-		require.EqualValues(t, latest, 0)
+		require.EqualValues(t, 0, latest)
 		return nil
 	})
 	require.NoError(t, err)
@@ -204,7 +205,7 @@ func testStore(
 	err = runConn(ctx, db, func(conn *sql.Conn) error {
 		res, err := store.ListMigrations(ctx, conn)
 		require.NoError(t, err)
-		require.Equal(t, len(res), 0)
+		require.Empty(t, res)
 		return nil
 	})
 	require.NoError(t, err)
@@ -213,7 +214,7 @@ func testStore(
 	err = runConn(ctx, db, func(conn *sql.Conn) error {
 		_, err := store.GetMigration(ctx, conn, 0)
 		require.Error(t, err)
-		require.True(t, errors.Is(err, database.ErrVersionNotFound))
+		require.ErrorIs(t, err, database.ErrVersionNotFound)
 		return nil
 	})
 	require.NoError(t, err)

globals_test.go

@@ -12,12 +12,12 @@ func TestNewGoMigration(t *testing.T) {
 	t.Run("valid_both_nil", func(t *testing.T) {
 		m := NewGoMigration(1, nil, nil)
 		// roundtrip
-		require.Equal(t, m.Version, int64(1))
-		require.Equal(t, m.Type, TypeGo)
-		require.Equal(t, m.Registered, true)
-		require.Equal(t, m.Next, int64(-1))
-		require.Equal(t, m.Previous, int64(-1))
-		require.Equal(t, m.Source, "")
+		require.EqualValues(t, 1, m.Version)
+		require.Equal(t, TypeGo, m.Type)
+		require.True(t, m.Registered)
+		require.EqualValues(t, -1, m.Next)
+		require.EqualValues(t, -1, m.Previous)
+		require.Equal(t, "", m.Source)
 		require.Nil(t, m.UpFnNoTxContext)
 		require.Nil(t, m.DownFnNoTxContext)
 		require.Nil(t, m.UpFnContext)
@@ -26,10 +26,10 @@ func TestNewGoMigration(t *testing.T) {
 		require.Nil(t, m.DownFn)
 		require.Nil(t, m.UpFnNoTx)
 		require.Nil(t, m.DownFnNoTx)
-		require.True(t, m.goUp != nil)
-		require.True(t, m.goDown != nil)
-		require.Equal(t, m.goUp.Mode, TransactionEnabled)
-		require.Equal(t, m.goDown.Mode, TransactionEnabled)
+		require.NotNil(t, m.goUp)
+		require.NotNil(t, m.goDown)
+		require.Equal(t, TransactionEnabled, m.goUp.Mode)
+		require.Equal(t, TransactionEnabled, m.goDown.Mode)
 	})
 	t.Run("all_set", func(t *testing.T) {
 		// This will eventually be an error when registering migrations.
@@ -39,14 +39,14 @@ func TestNewGoMigration(t *testing.T) {
 			&GoFunc{RunTx: func(context.Context, *sql.Tx) error { return nil }, RunDB: func(context.Context, *sql.DB) error { return nil }},
 		)
 		// check only functions
-		require.True(t, m.UpFn != nil)
-		require.True(t, m.UpFnContext != nil)
-		require.True(t, m.UpFnNoTx != nil)
-		require.True(t, m.UpFnNoTxContext != nil)
-		require.True(t, m.DownFn != nil)
-		require.True(t, m.DownFnContext != nil)
-		require.True(t, m.DownFnNoTx != nil)
-		require.True(t, m.DownFnNoTxContext != nil)
+		require.NotNil(t, m.UpFn)
+		require.NotNil(t, m.UpFnContext)
+		require.NotNil(t, m.UpFnNoTx)
+		require.NotNil(t, m.UpFnNoTxContext)
+		require.NotNil(t, m.DownFn)
+		require.NotNil(t, m.DownFnContext)
+		require.NotNil(t, m.DownFnNoTx)
+		require.NotNil(t, m.DownFnNoTxContext)
 	})
 }
 
@@ -93,12 +93,12 @@ func TestTransactionMode(t *testing.T) {
 		m := NewGoMigration(1, nil, nil)
 		err = SetGlobalMigrations(m)
 		require.NoError(t, err)
-		require.Equal(t, len(registeredGoMigrations), 1)
+		require.Len(t, registeredGoMigrations, 1)
 		registered := registeredGoMigrations[1]
-		require.True(t, registered.goUp != nil)
-		require.True(t, registered.goDown != nil)
-		require.Equal(t, registered.goUp.Mode, TransactionEnabled)
-		require.Equal(t, registered.goDown.Mode, TransactionEnabled)
+		require.NotNil(t, registered.goUp)
+		require.NotNil(t, registered.goDown)
+		require.Equal(t, TransactionEnabled, registered.goUp.Mode)
+		require.Equal(t, TransactionEnabled, registered.goDown.Mode)
 
 		migration2 := NewGoMigration(2, nil, nil)
 		// reset so we can check the default is set
@@ -131,12 +131,12 @@ func TestLegacyFunctions(t *testing.T) {
 
 	assertMigration := func(t *testing.T, m *Migration, version int64) {
 		t.Helper()
-		require.Equal(t, m.Version, version)
-		require.Equal(t, m.Type, TypeGo)
-		require.Equal(t, m.Registered, true)
-		require.Equal(t, m.Next, int64(-1))
-		require.Equal(t, m.Previous, int64(-1))
-		require.Equal(t, m.Source, "")
+		require.Equal(t, version, m.Version)
+		require.Equal(t, TypeGo, m.Type)
+		require.True(t, m.Registered)
+		require.EqualValues(t, -1, m.Next)
+		require.EqualValues(t, -1, m.Previous)
+		require.Equal(t, "", m.Source)
 	}
 
 	t.Run("all_tx", func(t *testing.T) {
@@ -145,7 +145,7 @@ func TestLegacyFunctions(t *testing.T) {
 			NewGoMigration(1, &GoFunc{RunTx: runTx}, &GoFunc{RunTx: runTx}),
 		)
 		require.NoError(t, err)
-		require.Equal(t, len(registeredGoMigrations), 1)
+		require.Len(t, registeredGoMigrations, 1)
 		m := registeredGoMigrations[1]
 		assertMigration(t, m, 1)
 		// Legacy functions.
@@ -168,7 +168,7 @@ func TestLegacyFunctions(t *testing.T) {
 			NewGoMigration(2, &GoFunc{RunDB: runDB}, &GoFunc{RunDB: runDB}),
 		)
 		require.NoError(t, err)
-		require.Equal(t, len(registeredGoMigrations), 1)
+		require.Len(t, registeredGoMigrations, 1)
 		m := registeredGoMigrations[2]
 		assertMigration(t, m, 2)
 		// Legacy functions.

goose_cli_test.go

@@ -24,7 +24,7 @@ func TestFullBinary(t *testing.T) {
 	cli := buildGooseCLI(t, false)
 	out, err := cli.run("--version")
 	require.NoError(t, err)
-	require.Equal(t, out, "goose version: "+gooseTestBinaryVersion+"\n")
+	require.Equal(t, "goose version: "+gooseTestBinaryVersion+"\n", out)
 }
 
 func TestLiteBinary(t *testing.T) {
@@ -35,7 +35,7 @@ func TestLiteBinary(t *testing.T) {
 		t.Parallel()
 		out, err := cli.run("--version")
 		require.NoError(t, err)
-		require.Equal(t, out, "goose version: "+gooseTestBinaryVersion+"\n")
+		require.Equal(t, "goose version: "+gooseTestBinaryVersion+"\n", out)
 	})
 	t.Run("default_binary", func(t *testing.T) {
 		t.Parallel()
@@ -113,7 +113,7 @@ func TestLiteBinary(t *testing.T) {
 		createEmptyFile(t, dir, "20230826163151_delta.go")
 		total, err := os.ReadDir(dir)
 		require.NoError(t, err)
-		require.Equal(t, len(total), 4)
+		require.Len(t, total, 4)
 		migrationFiles := []struct {
 			name     string
 			fileType string
@@ -137,13 +137,13 @@ func TestLiteBinary(t *testing.T) {
 		}
 		total, err = os.ReadDir(dir)
 		require.NoError(t, err)
-		require.Equal(t, len(total), 7)
+		require.Len(t, total, 7)
 		out, err := cli.run("-dir="+dir, "fix")
 		require.NoError(t, err)
 		require.Contains(t, out, "RENAMED")
 		files, err := os.ReadDir(dir)
 		require.NoError(t, err)
-		require.Equal(t, len(files), 7)
+		require.Len(t, files, 7)
 		expected := []string{
 			"00001_alpha.sql",
 			"00003_bravo.sql",
@@ -175,6 +175,7 @@ func (g gooseBinary) run(params ...string) (string, error) {
 // buildGooseCLI builds goose test binary, which is used for testing goose CLI. It is built with all
 // drivers enabled, unless lite is true, in which case all drivers are disabled except sqlite3
 func buildGooseCLI(t *testing.T, lite bool) gooseBinary {
+	t.Helper()
 	binName := "goose-test"
 	dir := t.TempDir()
 	output := filepath.Join(dir, binName)

goose_embed_test.go

@@ -46,14 +46,14 @@ func TestEmbeddedMigrations(t *testing.T) {
 		require.NoError(t, err)
 		ver, err = goose.GetDBVersion(db)
 		require.NoError(t, err)
-		require.EqualValues(t, ver, 0)
+		require.EqualValues(t, 0, ver)
 	})
 	t.Run("create_uses_os_fs", func(t *testing.T) {
 		dir := t.TempDir()
 		err := goose.Create(db, dir, "test", "sql")
 		require.NoError(t, err)
 		paths, _ := filepath.Glob(filepath.Join(dir, "*test.sql"))
-		require.NotZero(t, len(paths))
+		require.NotEmpty(t, paths)
 		err = goose.Fix(dir)
 		require.NoError(t, err)
 		_, err = os.Stat(filepath.Join(dir, "00001_test.sql"))

internal/dialect/store.go

@@ -123,7 +123,12 @@ func (s *store) DeleteVersionNoTx(ctx context.Context, db *sql.DB, tableName str
 	return err
 }
 
-func (s *store) GetMigration(ctx context.Context, db *sql.DB, tableName string, version int64) (*GetMigrationResult, error) {
+func (s *store) GetMigration(
+	ctx context.Context,
+	db *sql.DB,
+	tableName string,
+	version int64,
+) (*GetMigrationResult, error) {
 	q := s.querier.GetMigrationByVersion(tableName)
 	var timestamp time.Time
 	var isApplied bool