refactor(tests): clean up tests

Signed-off-by: Deluan <deluan@navidrome.org>

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(&regularUser)
-			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(&regularUser)
-			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(&regularUser)
-			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
+			found := slices.IndexFunc(users, func(u model.User) bool { return u.ID == testUser.ID })
-			for i := range users {
+			Expect(found).ToNot(Equal(-1))
-				if users[i].ID == testUser.ID {
-					foundUser = &users[i]
-					break
-				}
-			}
 
+			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() {
+			defer func() { _ = repo.(*userRepository).delete(squirrel.Eq{"id": userWithoutLibs.ID}) }()
-				_ = 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))