diff --git a/internal/db/db.go b/internal/db/db.go index f4e73ad1c..869b3700b 100644 --- a/internal/db/db.go +++ b/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) diff --git a/internal/db/db_test.go b/internal/db/db_test.go index 583105620..7f7210104 100644 --- a/internal/db/db_test.go +++ b/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 diff --git a/internal/db/domains_test.go b/internal/db/domains_test.go index 55052cbb1..5378870a2 100644 --- a/internal/db/domains_test.go +++ b/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() diff --git a/internal/db/migrations/driver_test.go b/internal/db/migrations/driver_test.go index 648057ff8..75d993fc5 100644 --- a/internal/db/migrations/driver_test.go +++ b/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)) }) diff --git a/internal/db/option_test.go b/internal/db/option_test.go index 03863fc05..f5895d93f 100644 --- a/internal/db/option_test.go +++ b/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) }) } diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index b6bd29fdd..5a540f27d 100644 --- a/internal/db/read_writer.go +++ b/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 } diff --git a/internal/db/read_writer_test.go b/internal/db/read_writer_test.go index c8f53238a..b2cf23689 100644 --- a/internal/db/read_writer_test.go +++ b/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) } }) diff --git a/internal/db/testing.go b/internal/db/testing.go index eb7a21107..2c9edc5e0 100644 --- a/internal/db/testing.go +++ b/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) } diff --git a/internal/servers/controller/handlers/projects/service_test.go b/internal/servers/controller/handlers/projects/service_test.go index 2c6ed6735..6e15ff48c 100644 --- a/internal/servers/controller/handlers/projects/service_test.go +++ b/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()