Skip to content

Commit 9f0059e

Browse files
deluandeluan
committed
refactor(tests): clean up tests
Signed-off-by: Deluan <deluan@navidrome.org>
1 parent 159aa28 commit 9f0059e

File tree

1 file changed

+39
-42
lines changed

1 file changed

+39
-42
lines changed

‎persistence/user_repository_test.go

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package persistence
33
import (
44
"context"
55
"errors"
6+
"slices"
67

78
"github.com/Masterminds/squirrel"
89
"github.com/deluan/rest"
@@ -19,7 +20,7 @@ var _ = Describe("UserRepository", func() {
1920
var repo model.UserRepository
2021

2122
BeforeEach(func() {
22-
repo = NewUserRepository(log.NewContext(context.TODO()), GetDBXBuilder())
23+
repo = NewUserRepository(log.NewContext(GinkgoT().Context()), GetDBXBuilder())
2324
})
2425

2526
Describe("Put/Get/FindByUsername", func() {
@@ -80,7 +81,7 @@ var _ = Describe("UserRepository", func() {
8081
It("does nothing if passwords are not specified", func() {
8182
user := &model.User{ID: "2", UserName: "johndoe"}
8283
err := validatePasswordChange(user, loggedUser)
83-
Expect(err).To(BeNil())
84+
Expect(err).ToNot(HaveOccurred())
8485
})
8586

8687
Context("Autogenerated password (used with Reverse Proxy Authentication)", func() {
@@ -92,7 +93,7 @@ var _ = Describe("UserRepository", func() {
9293
It("does nothing if passwords are not specified", func() {
9394
user = *loggedUser
9495
err := validatePasswordChange(&user, loggedUser)
95-
Expect(err).To(BeNil())
96+
Expect(err).ToNot(HaveOccurred())
9697
})
9798
It("does not requires currentPassword for regular user", func() {
9899
user = *loggedUser
@@ -119,7 +120,7 @@ var _ = Describe("UserRepository", func() {
119120
user := &model.User{ID: "2", UserName: "johndoe"}
120121
user.NewPassword = "new"
121122
err := validatePasswordChange(user, loggedUser)
122-
Expect(err).To(BeNil())
123+
Expect(err).ToNot(HaveOccurred())
123124
})
124125
It("requires currentPassword to change its own", func() {
125126
user := *loggedUser
@@ -157,7 +158,7 @@ var _ = Describe("UserRepository", func() {
157158
user.CurrentPassword = "abc123"
158159
user.NewPassword = "new"
159160
err := validatePasswordChange(&user, loggedUser)
160-
Expect(err).To(BeNil())
161+
Expect(err).ToNot(HaveOccurred())
161162
})
162163
})
163164

@@ -201,10 +202,11 @@ var _ = Describe("UserRepository", func() {
201202
user.CurrentPassword = "abc123"
202203
user.NewPassword = "new"
203204
err := validatePasswordChange(&user, loggedUser)
204-
Expect(err).To(BeNil())
205+
Expect(err).ToNot(HaveOccurred())
205206
})
206207
})
207208
})
209+
208210
Describe("validateUsernameUnique", func() {
209211
var repo *tests.MockedUserRepo
210212
var existingUser *model.User
@@ -275,16 +277,16 @@ var _ = Describe("UserRepository", func() {
275277
Describe("GetUserLibraries", func() {
276278
It("returns empty list when user has no library associations", func() {
277279
libraries, err := repo.GetUserLibraries("non-existent-user")
278-
Expect(err).To(BeNil())
280+
Expect(err).ToNot(HaveOccurred())
279281
Expect(libraries).To(HaveLen(0))
280282
})
281283

282284
It("returns user's associated libraries", func() {
283285
err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID})
284-
Expect(err).To(BeNil())
286+
Expect(err).ToNot(HaveOccurred())
285287

286288
libraries, err := repo.GetUserLibraries(userID)
287-
Expect(err).To(BeNil())
289+
Expect(err).ToNot(HaveOccurred())
288290
Expect(libraries).To(HaveLen(2))
289291

290292
libIDs := []int{libraries[0].ID, libraries[1].ID}
@@ -296,39 +298,39 @@ var _ = Describe("UserRepository", func() {
296298
It("sets user's library associations", func() {
297299
libraryIDs := []int{library1.ID, library2.ID}
298300
err := repo.SetUserLibraries(userID, libraryIDs)
299-
Expect(err).To(BeNil())
301+
Expect(err).ToNot(HaveOccurred())
300302

301303
libraries, err := repo.GetUserLibraries(userID)
302-
Expect(err).To(BeNil())
304+
Expect(err).ToNot(HaveOccurred())
303305
Expect(libraries).To(HaveLen(2))
304306
})
305307

306308
It("replaces existing associations", func() {
307309
// Set initial associations
308310
err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID})
309-
Expect(err).To(BeNil())
311+
Expect(err).ToNot(HaveOccurred())
310312

311313
// Replace with just one library
312314
err = repo.SetUserLibraries(userID, []int{library1.ID})
313-
Expect(err).To(BeNil())
315+
Expect(err).ToNot(HaveOccurred())
314316

315317
libraries, err := repo.GetUserLibraries(userID)
316-
Expect(err).To(BeNil())
318+
Expect(err).ToNot(HaveOccurred())
317319
Expect(libraries).To(HaveLen(1))
318320
Expect(libraries[0].ID).To(Equal(library1.ID))
319321
})
320322

321323
It("removes all associations when passed empty slice", func() {
322324
// Set initial associations
323325
err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID})
324-
Expect(err).To(BeNil())
326+
Expect(err).ToNot(HaveOccurred())
325327

326328
// Remove all
327329
err = repo.SetUserLibraries(userID, []int{})
328-
Expect(err).To(BeNil())
330+
Expect(err).ToNot(HaveOccurred())
329331

330332
libraries, err := repo.GetUserLibraries(userID)
331-
Expect(err).To(BeNil())
333+
Expect(err).ToNot(HaveOccurred())
332334
Expect(libraries).To(HaveLen(0))
333335
})
334336
})
@@ -347,7 +349,7 @@ var _ = Describe("UserRepository", func() {
347349

348350
// Count initial libraries
349351
existingLibs, err := libRepo.GetAll()
350-
Expect(err).To(BeNil())
352+
Expect(err).ToNot(HaveOccurred())
351353
initialLibCount = len(existingLibs)
352354

353355
library1 = model.Library{ID: 0, Name: "Admin Test Library 1", Path: "/admin/test/path1"}
@@ -377,11 +379,11 @@ var _ = Describe("UserRepository", func() {
377379
}
378380

379381
err := repo.Put(&adminUser)
380-
Expect(err).To(BeNil())
382+
Expect(err).ToNot(HaveOccurred())
381383

382384
// Admin should automatically have access to all libraries (including existing ones)
383385
libraries, err := repo.GetUserLibraries(adminUser.ID)
384-
Expect(err).To(BeNil())
386+
Expect(err).ToNot(HaveOccurred())
385387
Expect(libraries).To(HaveLen(initialLibCount + 2)) // Initial libraries + our 2 test libraries
386388

387389
libIDs := make([]int, len(libraries))
@@ -403,20 +405,20 @@ var _ = Describe("UserRepository", func() {
403405
}
404406

405407
err := repo.Put(&regularUser)
406-
Expect(err).To(BeNil())
408+
Expect(err).ToNot(HaveOccurred())
407409

408410
// Give them access to just one library
409411
err = repo.SetUserLibraries(regularUser.ID, []int{library1.ID})
410-
Expect(err).To(BeNil())
412+
Expect(err).ToNot(HaveOccurred())
411413

412414
// Promote to admin
413415
regularUser.IsAdmin = true
414416
err = repo.Put(&regularUser)
415-
Expect(err).To(BeNil())
417+
Expect(err).ToNot(HaveOccurred())
416418

417419
// Should now have access to all libraries (including existing ones)
418420
libraries, err := repo.GetUserLibraries(regularUser.ID)
419-
Expect(err).To(BeNil())
421+
Expect(err).ToNot(HaveOccurred())
420422
Expect(libraries).To(HaveLen(initialLibCount + 2)) // Initial libraries + our 2 test libraries
421423

422424
libIDs := make([]int, len(libraries))
@@ -438,11 +440,11 @@ var _ = Describe("UserRepository", func() {
438440
}
439441

440442
err := repo.Put(&regularUser)
441-
Expect(err).To(BeNil())
443+
Expect(err).ToNot(HaveOccurred())
442444

443445
// Regular user should be assigned to default libraries (library ID 1 from migration)
444446
libraries, err := repo.GetUserLibraries(regularUser.ID)
445-
Expect(err).To(BeNil())
447+
Expect(err).ToNot(HaveOccurred())
446448
Expect(libraries).To(HaveLen(1))
447449
Expect(libraries[0].ID).To(Equal(1))
448450
Expect(libraries[0].DefaultNewUsers).To(BeTrue())
@@ -492,18 +494,19 @@ var _ = Describe("UserRepository", func() {
492494

493495
It("populates Libraries field when getting a single user", func() {
494496
user, err := repo.Get(testUser.ID)
495-
Expect(err).To(BeNil())
497+
Expect(err).ToNot(HaveOccurred())
496498
Expect(user.Libraries).To(HaveLen(2))
497499

498500
libIDs := []int{user.Libraries[0].ID, user.Libraries[1].ID}
499501
Expect(libIDs).To(ContainElements(library1.ID, library2.ID))
500502

501503
// Check that library details are properly populated
502504
for _, lib := range user.Libraries {
503-
if lib.ID == library1.ID {
505+
switch lib.ID {
506+
case library1.ID:
504507
Expect(lib.Name).To(Equal("Field Test Library 1"))
505508
Expect(lib.Path).To(Equal("/field/test/path1"))
506-
} else if lib.ID == library2.ID {
509+
case library2.ID:
507510
Expect(lib.Name).To(Equal("Field Test Library 2"))
508511
Expect(lib.Path).To(Equal("/field/test/path2"))
509512
}
@@ -512,17 +515,13 @@ var _ = Describe("UserRepository", func() {
512515

513516
It("populates Libraries field when getting all users", func() {
514517
users, err := repo.(*userRepository).GetAll()
515-
Expect(err).To(BeNil())
518+
Expect(err).ToNot(HaveOccurred())
516519

517520
// Find our test user in the results
518-
var foundUser *model.User
519-
for i := range users {
520-
if users[i].ID == testUser.ID {
521-
foundUser = &users[i]
522-
break
523-
}
524-
}
521+
found := slices.IndexFunc(users, func(u model.User) bool { return u.ID == testUser.ID })
522+
Expect(found).ToNot(Equal(-1))
525523

524+
foundUser := users[found]
526525
Expect(foundUser).ToNot(BeNil())
527526
Expect(foundUser.Libraries).To(HaveLen(2))
528527

@@ -532,7 +531,7 @@ var _ = Describe("UserRepository", func() {
532531

533532
It("populates Libraries field when finding user by username", func() {
534533
user, err := repo.FindByUsername(testUser.UserName)
535-
Expect(err).To(BeNil())
534+
Expect(err).ToNot(HaveOccurred())
536535
Expect(user.Libraries).To(HaveLen(2))
537536

538537
libIDs := []int{user.Libraries[0].ID, user.Libraries[1].ID}
@@ -550,12 +549,10 @@ var _ = Describe("UserRepository", func() {
550549
IsAdmin: false,
551550
}
552551
Expect(repo.Put(&userWithoutLibs)).To(BeNil())
553-
defer func() {
554-
_ = repo.(*userRepository).delete(squirrel.Eq{"id": userWithoutLibs.ID})
555-
}()
552+
defer func() { _ = repo.(*userRepository).delete(squirrel.Eq{"id": userWithoutLibs.ID}) }()
556553

557554
user, err := repo.Get(userWithoutLibs.ID)
558-
Expect(err).To(BeNil())
555+
Expect(err).ToNot(HaveOccurred())
559556
Expect(user.Libraries).ToNot(BeNil())
560557
// Regular users should be assigned to default libraries (library ID 1 from migration)
561558
Expect(user.Libraries).To(HaveLen(1))

0 commit comments

Comments
 (0)