ErrRecordNotFound from LookupWhere() when no rows found (#59)

internal/db/db.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/golang-migrate/migrate/v4"
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/watchtower/internal/db/migrations"
 	"github.com/jinzhu/gorm"
 	"github.com/ory/dockertest"
@@ -123,23 +124,33 @@ func StartDbInDocker(dialect string) (cleanup func() error, retURL, container st
 
 // InitStore will execute the migrations needed to initialize the store for tests
 func InitStore(dialect string, cleanup func() error, url string) error {
+	var mErr *multierror.Error
 	// run migrations
 	source, err := migrations.NewMigrationSource(dialect)
 	if err != nil {
-		cleanup()
-		return fmt.Errorf("error creating migration driver: %w", err)
+		mErr = multierror.Append(mErr, fmt.Errorf("error creating migration driver: %w", err))
+		if err := cleanup(); err != nil {
+			mErr = multierror.Append(mErr, fmt.Errorf("error cleaning up from creating driver: %w",err)
+		}
+		return mErr.ErrorOrNil()
 	}
 	m, err := migrate.NewWithSourceInstance("httpfs", source, url)
 	if err != nil {
-		cleanup()
-		return fmt.Errorf("error creating migrations: %w", err)
+		mErr = multierror.Append(mErr, fmt.Errorf("error creating migrations: %w", err))
+		if err := cleanup(); err != nil {
+			mErr = multierror.Append(mErr, fmt.Errorf("error cleaning up from creating migrations: %w", err))
+		}
+		return mErr.ErrorOrNil()
+
 	}
 	if err := m.Up(); err != nil && err != migrate.ErrNoChange {
-		cleanup()
-		return fmt.Errorf("error running migrations: %w", err)
+		mErr = multierror.Append(mErr, fmt.Errorf("error running migrations: %w", err))
+		if err := cleanup(); err != nil {
+			mErr = multierror.Append(mErr, fmt.Errorf("error cleaning up from running migrations: %w" err))
+		}
+		return mErr.ErrorOrNil()
 	}
-
-	return nil
+	return mErr.ErrorOrNil()
 }
 
 // cleanupDockerResource will clean up the dockertest resources (postgres)

internal/db/db_test.go

@@ -9,7 +9,11 @@ func TestOpen(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	type args struct {
 		dbType        DbType
 		connectionUrl string
@@ -60,7 +64,11 @@ func TestMigrate(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	type args struct {
 		connectionUrl       string
 		migrationsDirectory string

internal/db/domains_test.go

@@ -22,7 +22,11 @@ returning id;
 	)
 
 	cleanup, conn, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	defer conn.Close()
 
 	db := conn.DB()
@@ -82,7 +86,11 @@ returning id;
 	)
 
 	cleanup, conn, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	defer conn.Close()
 
 	db := conn.DB()

internal/db/migrations/driver_test.go

@@ -100,10 +100,10 @@ func Test_fakeFile_Read(t *testing.T) {
 	assert := assert.New(t)
 	t.Run("valid", func(t *testing.T) {
 		ff, err := newFakeFile("postgres", "migrations/01_domain_types.up.sql")
-		assert.Nil(err)
+		assert.NoError(err)
 		buf := make([]byte, len(ff.bytes))
 		n, err := ff.Read(buf)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(len(buf), n)
 	})
 }
@@ -112,14 +112,14 @@ func Test_fakeFile_Seek(t *testing.T) {
 	assert := assert.New(t)
 	t.Run("valid", func(t *testing.T) {
 		ff, err := newFakeFile("postgres", "migrations/01_domain_types.up.sql")
-		assert.Nil(err)
+		assert.NoError(err)
 		buf := make([]byte, len(ff.bytes))
 		n, err := ff.Seek(10, 0)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(int64(10), n)
 
 		n2, err := ff.Read(buf)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(len(ff.bytes)-10, n2)
 	})
 }
@@ -131,9 +131,9 @@ func Test_fakeFile_Close(t *testing.T) {
 			dialect: "postgres",
 		}
 		f, err := m.Open("migrations/01_domain_types.up.sql")
-		assert.Nil(err)
+		assert.NoError(err)
 		err = f.Close()
-		assert.Nil(err)
+		assert.NoError(err)
 	})
 }
 
@@ -142,9 +142,9 @@ func Test_fakeFile_Stat(t *testing.T) {
 	t.Run("valid", func(t *testing.T) {
 		name := "migrations/01_domain_types.up.sql"
 		ff, err := newFakeFile("postgres", name)
-		assert.Nil(err)
+		assert.NoError(err)
 		info, err := ff.Stat()
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(ff.name, info.Name())
 		assert.Equal(int64(len(ff.bytes)), info.Size())
 		assert.Equal(os.ModePerm, info.Mode())
@@ -158,19 +158,19 @@ func Test_fakeFile_Readdir(t *testing.T) {
 	t.Run("valid", func(t *testing.T) {
 		name := "migrations/01_domain_types.up.sql"
 		ff, err := newFakeFile("postgres", name)
-		assert.Nil(err)
+		assert.NoError(err)
 		info, err := ff.Readdir(0)
-		assert.Nil(err)
-		assert.True(info != nil)
+		assert.NoError(err)
+		assert.NotNil(info)
 
 		info, err = ff.Readdir(1)
-		assert.Nil(err)
-		assert.True(info != nil)
+		assert.NoError(err)
+		assert.NotNil(info)
 		assert.Equal(1, len(info))
 
 		info, err = ff.Readdir(0)
-		assert.Nil(err)
-		assert.True(info != nil)
+		assert.NoError(err)
+		assert.NotNil(info)
 		// we don't want to count "migrations", so we're len - 1
 		assert.Equal(len(postgresMigrations)-1, len(info))
 	})

internal/db/option_test.go

@@ -1,7 +1,6 @@
 package db
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/hashicorp/watchtower/internal/oplog"
@@ -17,7 +16,7 @@ func Test_getOpts(t *testing.T) {
 		opts := GetOpts()
 		testOpts := getDefaultOptions()
 		testOpts.withOplog = false
-		assert.True(reflect.DeepEqual(opts, testOpts))
+		assert.Equal(opts, testOpts)
 
 		wrapper := TestWrapper(t)
 		md := oplog.Metadata{
@@ -33,33 +32,32 @@ func Test_getOpts(t *testing.T) {
 			wrapper:  wrapper,
 			metadata: md,
 		}
-		assert.True(reflect.DeepEqual(opts, testOpts))
+		assert.Equal(opts, testOpts)
 	})
 	t.Run("WithLookup", func(t *testing.T) {
 		// test default of true
 		opts := GetOpts()
 		testOpts := getDefaultOptions()
 		testOpts.withLookup = false
-		assert.True(reflect.DeepEqual(opts, testOpts))
+		assert.Equal(opts, testOpts)
 
 		// try setting to false
 		opts = GetOpts(WithLookup(true))
 		testOpts = getDefaultOptions()
 		testOpts.withLookup = true
-		assert.True(reflect.DeepEqual(opts, testOpts))
-
+		assert.Equal(opts, testOpts)
 	})
 	t.Run("WithDebug", func(t *testing.T) {
 		// test default of false
 		opts := GetOpts()
 		testOpts := getDefaultOptions()
 		testOpts.withDebug = false
-		assert.True(reflect.DeepEqual(opts, testOpts))
+		assert.Equal(opts, testOpts)
 
 		// try setting to true
 		opts = GetOpts(WithDebug(true))
 		testOpts = getDefaultOptions()
 		testOpts.withDebug = true
-		assert.True(reflect.DeepEqual(opts, testOpts))
+		assert.Equal(opts, testOpts)
 	})
 }

internal/db/read_writer.go

@@ -365,6 +365,9 @@ func (rw *Db) addOplog(ctx context.Context, opType OpType, opts Options, i inter
 		oplogArgs.wrapper,
 		ticketer,
 	)
+	if err != nil {
+		return err
+	}
 	var entryOp oplog.OpType
 	switch opType {
 	case CreateOp:
@@ -489,7 +492,13 @@ func (rw *Db) LookupWhere(ctx context.Context, resource interface{}, where strin
 	if reflect.ValueOf(resource).Kind() != reflect.Ptr {
 		return errors.New("error interface parameter must to be a pointer for lookup by")
 	}
-	return rw.underlying.Where(where, args...).First(resource).Error
+	if err := rw.underlying.Where(where, args...).First(resource).Error; err != nil {
+		if err == gorm.ErrRecordNotFound {
+			return ErrRecordNotFound
+		}
+		return err
+	}
+	return nil
 }
 
 // SearchWhere will search for all the resources it can find using a where clause with parameters
@@ -500,5 +509,9 @@ func (rw *Db) SearchWhere(ctx context.Context, resources interface{}, where stri
 	if reflect.ValueOf(resources).Kind() != reflect.Ptr {
 		return errors.New("error interface parameter must to be a pointer for search by")
 	}
-	return rw.underlying.Where(where, args...).Find(resources).Error
+	if err := rw.underlying.Where(where, args...).Find(resources).Error; err != nil {
+		// searching with a slice parameter does not return a gorm.ErrRecordNotFound
+		return err
+	}
+	return nil
 }

internal/db/read_writer_test.go

@@ -16,52 +16,56 @@ import (
 func TestDb_Update(t *testing.T) {
 	// intentionally not run with t.Parallel so we don't need to use DoTx for the Update tests
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("simple", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		user.Name = "friendly-" + id
 		rowsUpdated, err := w.Update(context.Background(), user, []string{"Name"})
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(1, rowsUpdated)
 
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Name, user.Name)
 	})
 	t.Run("valid-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		user.Name = "friendly-" + id
@@ -76,49 +80,49 @@ func TestDb_Update(t *testing.T) {
 					"resource-public-id": []string{user.GetPublicId()},
 				}),
 		)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(1, rowsUpdated)
 
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Name, user.Name)
 
 		var metadata store.Metadata
 		err = db.Where("key = ? and value = ?", "resource-public-id", user.PublicId).First(&metadata).Error
-		assert.Nil(err)
+		assert.NoError(err)
 
 		var foundEntry oplog.Entry
 		err = db.Where("id = ?", metadata.EntryId).First(&foundEntry).Error
-		assert.Nil(err)
+		assert.NoError(err)
 	})
 	t.Run("nil-tx", func(t *testing.T) {
 		w := Db{underlying: nil}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		rowsUpdated, err := w.Update(context.Background(), user, nil)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(0, rowsUpdated)
 		assert.Equal("update underlying db is nil", err.Error())
 	})
 	t.Run("no-wrapper-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		user.Name = "friendly-" + id
@@ -131,26 +135,26 @@ func TestDb_Update(t *testing.T) {
 					"project":    []string{"central-info-systems", "local-info-systems"},
 				}),
 		)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(0, rowsUpdated)
 		assert.Equal("error no wrapper WithOplog", err.Error())
 	})
 	t.Run("no-metadata-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		user.Name = "friendly-" + id
@@ -160,7 +164,7 @@ func TestDb_Update(t *testing.T) {
 				nil,
 			),
 		)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(0, rowsUpdated)
 		assert.Equal("error no metadata for WithOplog", err.Error())
 	})
@@ -169,35 +173,39 @@ func TestDb_Update(t *testing.T) {
 func TestDb_Create(t *testing.T) {
 	// intentionally not run with t.Parallel so we don't need to use DoTx for the Create tests
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("simple", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
-		assert.True(user.GetCreateTime() != nil)
-		assert.True(user.GetUpdateTime() != nil)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
+		assert.NotEmpty(user.GetCreateTime())
+		assert.NotEmpty(user.GetUpdateTime())
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 	})
 	t.Run("valid-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(
 			context.Background(),
@@ -211,22 +219,22 @@ func TestDb_Create(t *testing.T) {
 				},
 			),
 		)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 	})
 	t.Run("no-wrapper-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(
 			context.Background(),
@@ -240,15 +248,15 @@ func TestDb_Create(t *testing.T) {
 				},
 			),
 		)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error no wrapper WithOplog", err.Error())
 	})
 	t.Run("no-metadata-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(
 			context.Background(),
@@ -258,18 +266,18 @@ func TestDb_Create(t *testing.T) {
 				nil,
 			),
 		)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error no metadata for WithOplog", err.Error())
 	})
 	t.Run("nil-tx", func(t *testing.T) {
 		w := Db{underlying: nil}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("create underlying db is nil", err.Error())
 	})
 }
@@ -277,54 +285,58 @@ func TestDb_Create(t *testing.T) {
 func TestDb_LookupByName(t *testing.T) {
 	t.Parallel()
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("simple", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "fn-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.Name = "fn-" + id
 		err = w.LookupByName(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 	})
 	t.Run("tx-nil,", func(t *testing.T) {
 		w := Db{}
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.Name = "fn-name"
 		err = w.LookupByName(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error underlying db nil for lookup by name", err.Error())
 	})
 	t.Run("no-friendly-name-set", func(t *testing.T) {
 		w := Db{underlying: db}
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		err = w.LookupByName(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error name empty string for lookup by name", err.Error())
 	})
 	t.Run("not-found", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.Name = "fn-" + id
 		err = w.LookupByName(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(ErrRecordNotFound, err)
 	})
 }
@@ -332,54 +344,58 @@ func TestDb_LookupByName(t *testing.T) {
 func TestDb_LookupByPublicId(t *testing.T) {
 	t.Parallel()
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("simple", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.PublicId != "")
+		assert.NoError(err)
+		assert.NotEmpty(user.PublicId)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 	})
 	t.Run("tx-nil,", func(t *testing.T) {
 		w := Db{}
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error underlying db nil for lookup by public id", err.Error())
 	})
 	t.Run("no-public-id-set", func(t *testing.T) {
 		w := Db{underlying: db}
 		foundUser, err := db_test.NewTestUser()
 		foundUser.PublicId = ""
-		assert.Nil(err)
+		assert.NoError(err)
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error public id empty string for lookup by public id", err.Error())
 	})
 	t.Run("not-found", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = id
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(ErrRecordNotFound, err)
 	})
 }
@@ -387,122 +403,135 @@ func TestDb_LookupByPublicId(t *testing.T) {
 func TestDb_LookupWhere(t *testing.T) {
 	t.Parallel()
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("simple", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.PublicId != "")
+		assert.NoError(err)
+		assert.NotEmpty(user.PublicId)
 
 		var foundUser db_test.TestUser
 		err = w.LookupWhere(context.Background(), &foundUser, "public_id = ?", user.PublicId)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 	})
 	t.Run("tx-nil,", func(t *testing.T) {
 		w := Db{}
 		var foundUser db_test.TestUser
 		err := w.LookupWhere(context.Background(), &foundUser, "public_id = ?", 1)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error underlying db nil for lookup by", err.Error())
 	})
 	t.Run("not-found", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 
 		var foundUser db_test.TestUser
 		err = w.LookupWhere(context.Background(), &foundUser, "public_id = ?", id)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(ErrRecordNotFound, err)
+		assert.True(errors.Is(err, ErrRecordNotFound))
 	})
 	t.Run("bad-where", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 
 		var foundUser db_test.TestUser
 		err = w.LookupWhere(context.Background(), &foundUser, "? = ?", id)
-		assert.True(err != nil)
+		assert.Error(err)
 	})
 }
 
 func TestDb_SearchWhere(t *testing.T) {
 	t.Parallel()
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("simple", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.PublicId != "")
+		assert.NoError(err)
+		assert.NotEmpty(user.PublicId)
 
 		var foundUsers []db_test.TestUser
 		err = w.SearchWhere(context.Background(), &foundUsers, "public_id = ?", user.PublicId)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUsers[0].Id, user.Id)
 	})
 	t.Run("tx-nil,", func(t *testing.T) {
 		w := Db{}
 		var foundUsers []db_test.TestUser
 		err := w.SearchWhere(context.Background(), &foundUsers, "public_id = ?", 1)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("error underlying db nil for search by", err.Error())
 	})
 	t.Run("not-found", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 
 		var foundUsers []db_test.TestUser
 		err = w.SearchWhere(context.Background(), &foundUsers, "public_id = ?", id)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(0, len(foundUsers))
 	})
 	t.Run("bad-where", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 
 		var foundUsers []db_test.TestUser
 		err = w.SearchWhere(context.Background(), &foundUsers, "? = ?", id)
-		assert.True(err != nil)
+		assert.Error(err)
 	})
 }
 
 func TestDb_DB(t *testing.T) {
 	t.Parallel()
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("valid", func(t *testing.T) {
 		w := Db{underlying: db}
 		d, err := w.DB()
-		assert.Nil(err)
-		assert.True(d != nil)
+		assert.NoError(err)
+		assert.NotNil(d)
 		err = d.Ping()
-		assert.Nil(err)
+		assert.NoError(err)
 	})
 	t.Run("nil-tx", func(t *testing.T) {
 		w := Db{underlying: nil}
 		d, err := w.DB()
-		assert.True(err != nil)
-		assert.True(d == nil)
+		assert.Error(err)
+		assert.Nil(d)
 		assert.Equal("underlying db is nil", err.Error())
 	})
 }
@@ -510,7 +539,11 @@ func TestDb_DB(t *testing.T) {
 func TestDb_DoTx(t *testing.T) {
 	t.Parallel()
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 
@@ -525,7 +558,7 @@ func TestDb_DoTx(t *testing.T) {
 				}
 				return nil
 			})
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(8, got.Retries)
 		assert.Equal(9, attempts) // attempted 1 + 8 retries
 	})
@@ -540,7 +573,7 @@ func TestDb_DoTx(t *testing.T) {
 				}
 				return nil
 			})
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(1, got.Retries)
 		assert.Equal(2, attempts) // attempted 1 + 8 retries
 	})
@@ -555,7 +588,7 @@ func TestDb_DoTx(t *testing.T) {
 				}
 				return nil
 			})
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(2, got.Retries)
 		assert.Equal(3, attempts) // attempted 1 + 8 retries
 	})
@@ -570,7 +603,7 @@ func TestDb_DoTx(t *testing.T) {
 				}
 				return nil
 			})
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(3, got.Retries)
 		assert.Equal(4, attempts) // attempted 1 + 8 retries
 	})
@@ -578,7 +611,7 @@ func TestDb_DoTx(t *testing.T) {
 		w := &Db{underlying: db}
 		attempts := 0
 		got, err := w.DoTx(context.Background(), 0, ExpBackoff{}, func(Writer) error { attempts += 1; return nil })
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(RetryInfo{}, got)
 		assert.Equal(1, attempts)
 	})
@@ -586,22 +619,22 @@ func TestDb_DoTx(t *testing.T) {
 		w := &Db{nil}
 		attempts := 0
 		got, err := w.DoTx(context.Background(), 1, ExpBackoff{}, func(Writer) error { attempts += 1; return nil })
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(RetryInfo{}, got)
 		assert.Equal("do underlying db is nil", err.Error())
 	})
 	t.Run("not-a-retry-err", func(t *testing.T) {
 		w := &Db{underlying: db}
 		got, err := w.DoTx(context.Background(), 1, ExpBackoff{}, func(Writer) error { return errors.New("not a retry error") })
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(RetryInfo{}, got)
-		assert.True(err != oplog.ErrTicketAlreadyRedeemed)
+		assert.NotEqual(err, oplog.ErrTicketAlreadyRedeemed)
 	})
 	t.Run("too-many-retries", func(t *testing.T) {
 		w := &Db{underlying: db}
 		attempts := 0
 		got, err := w.DoTx(context.Background(), 2, ExpBackoff{}, func(Writer) error { attempts += 1; return oplog.ErrTicketAlreadyRedeemed })
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(3, got.Retries)
 		assert.Equal("Too many retries: 3 of 3", err.Error())
 	})
@@ -675,43 +708,47 @@ func TestDb_DoTx(t *testing.T) {
 func TestDb_Delete(t *testing.T) {
 	// intentionally not run with t.Parallel so we don't need to use DoTx for the Create tests
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("simple", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
-		assert.True(user.GetCreateTime() != nil)
-		assert.True(user.GetUpdateTime() != nil)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
+		assert.NotNil(user.GetCreateTime())
+		assert.NotNil(user.GetUpdateTime())
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		rowsDeleted, err := w.Delete(context.Background(), user)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(1, rowsDeleted)
 
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(ErrRecordNotFound, err)
 	})
 	t.Run("valid-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(
 			context.Background(),
@@ -725,14 +762,14 @@ func TestDb_Delete(t *testing.T) {
 				},
 			),
 		)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		rowsDeleted, err := w.Delete(
@@ -747,19 +784,19 @@ func TestDb_Delete(t *testing.T) {
 				},
 			),
 		)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(1, rowsDeleted)
 
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(ErrRecordNotFound, err)
 	})
 	t.Run("no-wrapper-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(
 			context.Background(),
@@ -773,14 +810,14 @@ func TestDb_Delete(t *testing.T) {
 				},
 			),
 		)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		rowsDeleted, err := w.Delete(
@@ -795,16 +832,16 @@ func TestDb_Delete(t *testing.T) {
 				},
 			),
 		)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(0, rowsDeleted)
 		assert.Equal("error no wrapper WithOplog", err.Error())
 	})
 	t.Run("no-metadata-WithOplog", func(t *testing.T) {
 		w := Db{underlying: db}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(
 			context.Background(),
@@ -818,14 +855,14 @@ func TestDb_Delete(t *testing.T) {
 				},
 			),
 		)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		foundUser, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		foundUser.PublicId = user.PublicId
 		err = w.LookupByPublicId(context.Background(), foundUser)
-		assert.Nil(err)
+		assert.NoError(err)
 		assert.Equal(foundUser.Id, user.Id)
 
 		rowsDeleted, err := w.Delete(
@@ -836,19 +873,19 @@ func TestDb_Delete(t *testing.T) {
 				nil,
 			),
 		)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal(0, rowsDeleted)
 		assert.Equal("error no metadata for WithOplog", err.Error())
 	})
 	t.Run("nil-tx", func(t *testing.T) {
 		w := Db{underlying: nil}
 		id, err := uuid.GenerateUUID()
-		assert.Nil(err)
+		assert.NoError(err)
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		user.Name = "foo-" + id
 		err = w.Create(context.Background(), user)
-		assert.True(err != nil)
+		assert.Error(err)
 		assert.Equal("create underlying db is nil", err.Error())
 	})
 }
@@ -856,28 +893,34 @@ func TestDb_Delete(t *testing.T) {
 func TestDb_ScanRows(t *testing.T) {
 	t.Parallel()
 	cleanup, db, _ := TestSetup(t, "postgres")
-	defer cleanup()
+	defer func() {
+		if err := cleanup(); err != nil {
+			t.Error(err)
+		}
+	}()
 	assert := assert.New(t)
 	defer db.Close()
 	t.Run("valid", func(t *testing.T) {
 		w := Db{underlying: db}
 		user, err := db_test.NewTestUser()
-		assert.Nil(err)
+		assert.NoError(err)
 		err = w.Create(context.Background(), user)
-		assert.Nil(err)
-		assert.True(user.Id != 0)
+		assert.NoError(err)
+		assert.NotEmpty(user.Id)
 
 		tx, err := w.DB()
+		assert.NoError(err)
 		where := "select * from db_test_user where name in ($1, $2)"
 		rows, err := tx.Query(where, "alice", "bob")
-		defer rows.Close()
+		assert.NoError(err)
+		defer func() { err := rows.Close(); assert.NoError(err) }()
 		for rows.Next() {
 			u, err := db_test.NewTestUser()
-			assert.Nil(err)
+			assert.NoError(err)
 
 			// scan the row into your Gorm struct
 			err = w.ScanRows(rows, &u)
-			assert.Nil(err)
+			assert.NoError(err)
 			assert.Equal(user.PublicId, u.PublicId)
 		}
 	})

internal/db/testing.go

@@ -13,10 +13,7 @@ import (
 
 // setup the tests (initialize the database one-time and intialized testDatabaseURL)
 func TestSetup(t *testing.T, dialect string) (func() error, *gorm.DB, string) {
-	cleanup := func() error { return nil }
-	var url string
-	var err error
-	cleanup, url, _, err = InitDbInDocker(dialect)
+	cleanup, url, _, err := InitDbInDocker(dialect)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/servers/controller/handlers/projects/service_test.go

@@ -22,7 +22,7 @@ import (
 
 func createDefaultProjectAndRepo(t *testing.T) (*iam.Scope, *iam.Repository) {
 	t.Helper()
-	cleanup, conn := db.TestSetup(t, "postgres")
+	cleanup, conn, _ := db.TestSetup(t, "postgres")
 	t.Cleanup(func() {
 		conn.Close()
 		cleanup()