From 9f0059e13f2019b7f54fe7f8e1661e681d0166fd Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 23 Jul 2025 11:02:07 -0400 Subject: [PATCH] refactor(tests): clean up tests Signed-off-by: Deluan --- persistence/user_repository_test.go | 81 ++++++++++++++--------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 24223857..7c0707ec 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -3,6 +3,7 @@ package persistence import ( "context" "errors" + "slices" "github.com/Masterminds/squirrel" "github.com/deluan/rest" @@ -19,7 +20,7 @@ var _ = Describe("UserRepository", func() { var repo model.UserRepository BeforeEach(func() { - repo = NewUserRepository(log.NewContext(context.TODO()), GetDBXBuilder()) + repo = NewUserRepository(log.NewContext(GinkgoT().Context()), GetDBXBuilder()) }) Describe("Put/Get/FindByUsername", func() { @@ -80,7 +81,7 @@ var _ = Describe("UserRepository", func() { It("does nothing if passwords are not specified", func() { user := &model.User{ID: "2", UserName: "johndoe"} err := validatePasswordChange(user, loggedUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) Context("Autogenerated password (used with Reverse Proxy Authentication)", func() { @@ -92,7 +93,7 @@ var _ = Describe("UserRepository", func() { It("does nothing if passwords are not specified", func() { user = *loggedUser err := validatePasswordChange(&user, loggedUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) It("does not requires currentPassword for regular user", func() { user = *loggedUser @@ -119,7 +120,7 @@ var _ = Describe("UserRepository", func() { user := &model.User{ID: "2", UserName: "johndoe"} user.NewPassword = "new" err := validatePasswordChange(user, loggedUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) It("requires currentPassword to change its own", func() { user := *loggedUser @@ -157,7 +158,7 @@ var _ = Describe("UserRepository", func() { user.CurrentPassword = "abc123" user.NewPassword = "new" err := validatePasswordChange(&user, loggedUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) }) @@ -201,10 +202,11 @@ var _ = Describe("UserRepository", func() { user.CurrentPassword = "abc123" user.NewPassword = "new" err := validatePasswordChange(&user, loggedUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) }) }) + Describe("validateUsernameUnique", func() { var repo *tests.MockedUserRepo var existingUser *model.User @@ -275,16 +277,16 @@ var _ = Describe("UserRepository", func() { Describe("GetUserLibraries", func() { It("returns empty list when user has no library associations", func() { libraries, err := repo.GetUserLibraries("non-existent-user") - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(0)) }) It("returns user's associated libraries", func() { err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID}) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(2)) libIDs := []int{libraries[0].ID, libraries[1].ID} @@ -296,24 +298,24 @@ var _ = Describe("UserRepository", func() { It("sets user's library associations", func() { libraryIDs := []int{library1.ID, library2.ID} err := repo.SetUserLibraries(userID, libraryIDs) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(2)) }) It("replaces existing associations", func() { // Set initial associations err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID}) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Replace with just one library err = repo.SetUserLibraries(userID, []int{library1.ID}) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(1)) Expect(libraries[0].ID).To(Equal(library1.ID)) }) @@ -321,14 +323,14 @@ var _ = Describe("UserRepository", func() { It("removes all associations when passed empty slice", func() { // Set initial associations err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID}) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Remove all err = repo.SetUserLibraries(userID, []int{}) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(0)) }) }) @@ -347,7 +349,7 @@ var _ = Describe("UserRepository", func() { // Count initial libraries existingLibs, err := libRepo.GetAll() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) initialLibCount = len(existingLibs) library1 = model.Library{ID: 0, Name: "Admin Test Library 1", Path: "/admin/test/path1"} @@ -377,11 +379,11 @@ var _ = Describe("UserRepository", func() { } err := repo.Put(&adminUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Admin should automatically have access to all libraries (including existing ones) libraries, err := repo.GetUserLibraries(adminUser.ID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(initialLibCount + 2)) // Initial libraries + our 2 test libraries libIDs := make([]int, len(libraries)) @@ -403,20 +405,20 @@ var _ = Describe("UserRepository", func() { } err := repo.Put(®ularUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Give them access to just one library err = repo.SetUserLibraries(regularUser.ID, []int{library1.ID}) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Promote to admin regularUser.IsAdmin = true err = repo.Put(®ularUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Should now have access to all libraries (including existing ones) libraries, err := repo.GetUserLibraries(regularUser.ID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(initialLibCount + 2)) // Initial libraries + our 2 test libraries libIDs := make([]int, len(libraries)) @@ -438,11 +440,11 @@ var _ = Describe("UserRepository", func() { } err := repo.Put(®ularUser) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Regular user should be assigned to default libraries (library ID 1 from migration) libraries, err := repo.GetUserLibraries(regularUser.ID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(libraries).To(HaveLen(1)) Expect(libraries[0].ID).To(Equal(1)) Expect(libraries[0].DefaultNewUsers).To(BeTrue()) @@ -492,7 +494,7 @@ var _ = Describe("UserRepository", func() { It("populates Libraries field when getting a single user", func() { user, err := repo.Get(testUser.ID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(user.Libraries).To(HaveLen(2)) libIDs := []int{user.Libraries[0].ID, user.Libraries[1].ID} @@ -500,10 +502,11 @@ var _ = Describe("UserRepository", func() { // Check that library details are properly populated for _, lib := range user.Libraries { - if lib.ID == library1.ID { + switch lib.ID { + case library1.ID: Expect(lib.Name).To(Equal("Field Test Library 1")) Expect(lib.Path).To(Equal("/field/test/path1")) - } else if lib.ID == library2.ID { + case library2.ID: Expect(lib.Name).To(Equal("Field Test Library 2")) Expect(lib.Path).To(Equal("/field/test/path2")) } @@ -512,17 +515,13 @@ var _ = Describe("UserRepository", func() { It("populates Libraries field when getting all users", func() { users, err := repo.(*userRepository).GetAll() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Find our test user in the results - var foundUser *model.User - for i := range users { - if users[i].ID == testUser.ID { - foundUser = &users[i] - break - } - } + found := slices.IndexFunc(users, func(u model.User) bool { return u.ID == testUser.ID }) + Expect(found).ToNot(Equal(-1)) + foundUser := users[found] Expect(foundUser).ToNot(BeNil()) Expect(foundUser.Libraries).To(HaveLen(2)) @@ -532,7 +531,7 @@ var _ = Describe("UserRepository", func() { It("populates Libraries field when finding user by username", func() { user, err := repo.FindByUsername(testUser.UserName) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(user.Libraries).To(HaveLen(2)) libIDs := []int{user.Libraries[0].ID, user.Libraries[1].ID} @@ -550,12 +549,10 @@ var _ = Describe("UserRepository", func() { IsAdmin: false, } Expect(repo.Put(&userWithoutLibs)).To(BeNil()) - defer func() { - _ = repo.(*userRepository).delete(squirrel.Eq{"id": userWithoutLibs.ID}) - }() + defer func() { _ = repo.(*userRepository).delete(squirrel.Eq{"id": userWithoutLibs.ID}) }() user, err := repo.Get(userWithoutLibs.ID) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(user.Libraries).ToNot(BeNil()) // Regular users should be assigned to default libraries (library ID 1 from migration) Expect(user.Libraries).To(HaveLen(1))