Skip to content

Commit aa5e605

Browse files
rkcoder101rkcoder101
committed
test/Wrote unit tests for GetUsers (which was separated out in the last commit) in cmd/harbor/root/user/list.go, setup interface for mocking the API
Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>
1 parent 65854bd commit aa5e605

File tree

2 files changed

+320
-6
lines changed

2 files changed

+320
-6
lines changed

‎cmd/harbor/root/user/list.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"io"
1919
"os"
2020

21+
"github.com/goharbor/go-client/pkg/sdk/v2.0/client/user"
2122
"github.com/goharbor/go-client/pkg/sdk/v2.0/models"
2223
"github.com/goharbor/harbor-cli/pkg/api"
2324
"github.com/goharbor/harbor-cli/pkg/utils"
@@ -27,19 +28,28 @@ import (
2728
"github.com/spf13/viper"
2829
)
2930

30-
func GetUsers(opts api.ListFlags) ([]*models.UserResp, error) {
31+
type UserLister interface {
32+
UserList(opts ...api.ListFlags) (*user.ListUsersOK, error)
33+
}
34+
type DefaultUserLister struct{}
35+
36+
func (d *DefaultUserLister) UserList(opts ...api.ListFlags) (*user.ListUsersOK, error) {
37+
return api.ListUsers(opts...)
38+
}
39+
40+
func GetUsers(opts api.ListFlags, userLister UserLister) ([]*models.UserResp, error) {
3141
var allUsers []*models.UserResp
3242

33-
if opts.PageSize > 100 {
34-
return nil, fmt.Errorf("page size should be less than or equal to 100")
43+
if opts.PageSize > 100 || opts.PageSize < 0 {
44+
return nil, fmt.Errorf("page size should be greater than or equal to 0 and less than or equal to 100")
3545
}
3646

3747
if opts.PageSize == 0 {
3848
opts.PageSize = 100
3949
opts.Page = 1
4050

4151
for {
42-
response, err := api.ListUsers(opts)
52+
response, err := userLister.UserList(opts)
4353
if err != nil {
4454
if isUnauthorizedError(err) {
4555
return nil, fmt.Errorf("Permission denied: Admin privileges are required to execute this command.")
@@ -55,7 +65,7 @@ func GetUsers(opts api.ListFlags) ([]*models.UserResp, error) {
5565
opts.Page++
5666
}
5767
} else {
58-
response, err := api.ListUsers(opts)
68+
response, err := userLister.UserList(opts)
5969
if err != nil {
6070
if isUnauthorizedError(err) {
6171
return nil, fmt.Errorf("Permission denied: Admin privileges are required to execute this command.")
@@ -92,7 +102,8 @@ func UserListCmd() *cobra.Command {
92102
Args: cobra.ExactArgs(0),
93103
Aliases: []string{"ls"},
94104
RunE: func(cmd *cobra.Command, args []string) error {
95-
allUsers, err := GetUsers(opts)
105+
defaultUserLister := &DefaultUserLister{}
106+
allUsers, err := GetUsers(opts, defaultUserLister)
96107
if err != nil {
97108
return err
98109
}

‎cmd/harbor/root/user/list_test.go‎

Lines changed: 303 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,17 @@ package user
1616

1717
import (
1818
"bytes"
19+
"cmp"
1920
"encoding/json"
21+
"fmt"
22+
"slices"
2023
"strings"
2124
"testing"
2225

2326
"github.com/go-openapi/strfmt"
27+
"github.com/goharbor/go-client/pkg/sdk/v2.0/client/user"
2428
"github.com/goharbor/go-client/pkg/sdk/v2.0/models"
29+
"github.com/goharbor/harbor-cli/pkg/api"
2530
log "github.com/sirupsen/logrus"
2631
"github.com/spf13/viper"
2732
"go.yaml.in/yaml/v4"
@@ -172,3 +177,301 @@ func TestPrintUsers(t *testing.T) {
172177
})
173178
}
174179
}
180+
181+
/*
182+
[X] make a test opts, only Page and PageSize fields needs to be there
183+
[X] make a helper to populate the users
184+
[X] mock api.List
185+
[X] mock authentication (no need, just make the api mock to pass an error with 403 in case you need to check for unauthenticated user)
186+
[X] test error logs
187+
[X] check if the users received are correct
188+
189+
*/
190+
/*
191+
Logic for mocking api.ListUsers
192+
-> should return (*user.ListUsersOK, error) ; Payload should be populated with the users
193+
-> shoud return the PageSize*(Page-1) th user till PageSize*(Page) -1 th user (0 based indexing)
194+
Helper for populating users:
195+
-> models.UserResp ko append krna hoga api.ListFlags type ke payload me
196+
*/
197+
type MockUserLister struct {
198+
numberOfUsersforTesting int64
199+
usersForTesting []*models.UserResp
200+
expectAuthError bool
201+
}
202+
203+
func (m *MockUserLister) populateUsers() []*models.UserResp {
204+
users := make([]*models.UserResp, 0, m.numberOfUsersforTesting)
205+
for i := 0; i < int(m.numberOfUsersforTesting); i++ {
206+
user := &models.UserResp{
207+
UserID: int64(i + 1),
208+
}
209+
users = append(users, user)
210+
}
211+
m.usersForTesting = users
212+
return users
213+
}
214+
func (m *MockUserLister) UserList(opts ...api.ListFlags) (*user.ListUsersOK, error) {
215+
if m.expectAuthError {
216+
return nil, fmt.Errorf("403")
217+
}
218+
res := &user.ListUsersOK{}
219+
if len(opts) == 0 {
220+
return res, nil
221+
}
222+
listFlags := opts[0]
223+
page, pageSize := listFlags.Page, listFlags.PageSize
224+
users := m.populateUsers()
225+
lo, hi := max(pageSize*(page-1), 0), min(pageSize*page, m.numberOfUsersforTesting)
226+
if lo >= m.numberOfUsersforTesting {
227+
return res, nil
228+
}
229+
res.Payload = users[lo:hi]
230+
return res, nil
231+
}
232+
func TestGetUsers(t *testing.T) {
233+
usersAreEqual := func(u1, u2 []*models.UserResp) bool {
234+
if len(u1) != len(u2) {
235+
return false
236+
}
237+
slices.SortFunc(u1, func(a, b *models.UserResp) int {
238+
return cmp.Compare(a.UserID, b.UserID)
239+
})
240+
slices.SortFunc(u2, func(a, b *models.UserResp) int {
241+
return cmp.Compare(a.UserID, b.UserID)
242+
})
243+
for i := 0; i < len(u1); i++ {
244+
if u1[i].UserID != u2[i].UserID {
245+
return false
246+
}
247+
}
248+
return true
249+
}
250+
tests := []struct {
251+
name string
252+
setup func() (api.ListFlags, *MockUserLister)
253+
wantError bool
254+
errContains string
255+
}{
256+
{
257+
name: "fetch specific page with valid page size",
258+
setup: func() (api.ListFlags, *MockUserLister) {
259+
opts := api.ListFlags{
260+
Page: 2,
261+
PageSize: 50,
262+
}
263+
m := &MockUserLister{
264+
numberOfUsersforTesting: 102,
265+
expectAuthError: false,
266+
}
267+
return opts, m
268+
},
269+
wantError: false,
270+
},
271+
{
272+
name: "fetch all users with page size 0 (multiple pages)",
273+
setup: func() (api.ListFlags, *MockUserLister) {
274+
opts := api.ListFlags{
275+
Page: 1,
276+
PageSize: 0,
277+
}
278+
m := &MockUserLister{
279+
numberOfUsersforTesting: 250,
280+
expectAuthError: false,
281+
}
282+
return opts, m
283+
},
284+
wantError: false,
285+
},
286+
{
287+
name: "fetch all users when total is exactly divisible by 100",
288+
setup: func() (api.ListFlags, *MockUserLister) {
289+
opts := api.ListFlags{
290+
Page: 1,
291+
PageSize: 0,
292+
}
293+
m := &MockUserLister{
294+
numberOfUsersforTesting: 200,
295+
expectAuthError: false,
296+
}
297+
return opts, m
298+
},
299+
wantError: false,
300+
},
301+
{
302+
name: "fetch first page with page size 10",
303+
setup: func() (api.ListFlags, *MockUserLister) {
304+
opts := api.ListFlags{
305+
Page: 1,
306+
PageSize: 10,
307+
}
308+
m := &MockUserLister{
309+
numberOfUsersforTesting: 50,
310+
expectAuthError: false,
311+
}
312+
return opts, m
313+
},
314+
wantError: false,
315+
},
316+
{
317+
name: "fetch last page with partial results",
318+
setup: func() (api.ListFlags, *MockUserLister) {
319+
opts := api.ListFlags{
320+
Page: 3,
321+
PageSize: 10,
322+
}
323+
m := &MockUserLister{
324+
numberOfUsersforTesting: 25,
325+
expectAuthError: false,
326+
}
327+
return opts, m
328+
},
329+
wantError: false,
330+
},
331+
{
332+
name: "fetch page beyond available data returns empty",
333+
setup: func() (api.ListFlags, *MockUserLister) {
334+
opts := api.ListFlags{
335+
Page: 10,
336+
PageSize: 10,
337+
}
338+
m := &MockUserLister{
339+
numberOfUsersforTesting: 5,
340+
expectAuthError: false,
341+
}
342+
return opts, m
343+
},
344+
wantError: false,
345+
},
346+
{
347+
name: "fetch with maximum allowed page size 100",
348+
setup: func() (api.ListFlags, *MockUserLister) {
349+
opts := api.ListFlags{
350+
Page: 1,
351+
PageSize: 100,
352+
}
353+
m := &MockUserLister{
354+
numberOfUsersforTesting: 150,
355+
expectAuthError: false,
356+
}
357+
return opts, m
358+
},
359+
wantError: false,
360+
},
361+
{
362+
name: "fetch with zero users in database",
363+
setup: func() (api.ListFlags, *MockUserLister) {
364+
opts := api.ListFlags{
365+
Page: 1,
366+
PageSize: 10,
367+
}
368+
m := &MockUserLister{
369+
numberOfUsersforTesting: 0,
370+
expectAuthError: false,
371+
}
372+
return opts, m
373+
},
374+
wantError: false,
375+
},
376+
{
377+
name: "page size exceeds maximum (101)",
378+
setup: func() (api.ListFlags, *MockUserLister) {
379+
opts := api.ListFlags{
380+
Page: 1,
381+
PageSize: 101,
382+
}
383+
m := &MockUserLister{
384+
numberOfUsersforTesting: 50,
385+
expectAuthError: false,
386+
}
387+
return opts, m
388+
},
389+
wantError: true,
390+
errContains: "page size should be greater than or equal to 0 and less than or equal to 100",
391+
},
392+
{
393+
name: "page size is negative",
394+
setup: func() (api.ListFlags, *MockUserLister) {
395+
opts := api.ListFlags{
396+
Page: 1,
397+
PageSize: -1,
398+
}
399+
m := &MockUserLister{
400+
numberOfUsersforTesting: 50,
401+
expectAuthError: false,
402+
}
403+
return opts, m
404+
},
405+
wantError: true,
406+
errContains: "page size should be greater than or equal to 0 and less than or equal to 100",
407+
},
408+
{
409+
name: "authentication error returns permission denied",
410+
setup: func() (api.ListFlags, *MockUserLister) {
411+
opts := api.ListFlags{
412+
Page: 1,
413+
PageSize: 10,
414+
}
415+
m := &MockUserLister{
416+
numberOfUsersforTesting: 50,
417+
expectAuthError: true,
418+
}
419+
return opts, m
420+
},
421+
wantError: true,
422+
errContains: "Permission denied",
423+
},
424+
{
425+
name: "authentication error during fetch all",
426+
setup: func() (api.ListFlags, *MockUserLister) {
427+
opts := api.ListFlags{
428+
Page: 1,
429+
PageSize: 0,
430+
}
431+
m := &MockUserLister{
432+
numberOfUsersforTesting: 50,
433+
expectAuthError: true,
434+
}
435+
return opts, m
436+
},
437+
wantError: true,
438+
errContains: "Permission denied",
439+
},
440+
}
441+
for _, tt := range tests {
442+
t.Run(tt.name, func(t *testing.T) {
443+
opts, m := tt.setup()
444+
allUsers, err := GetUsers(opts, m)
445+
446+
// Check if we expected an error but did not get one (or vice-versa)
447+
if (err != nil) != tt.wantError {
448+
t.Fatalf("GetUsers() error presence mismatch: got error %v, wantError %v", err, tt.wantError)
449+
}
450+
if tt.wantError && !strings.Contains(err.Error(), tt.errContains) {
451+
t.Errorf("Expected error to contain '%s', got '%s'", tt.errContains, err.Error())
452+
}
453+
if !tt.wantError {
454+
// check if the returned allUsers are correct according to our mock database of users (which is m.usersForTesting)
455+
if opts.PageSize == 0 {
456+
if !usersAreEqual(allUsers, m.usersForTesting) {
457+
t.Errorf("Expected all of the users to be returned")
458+
}
459+
} else {
460+
requiredPage, requiredPageSize := opts.Page, opts.PageSize
461+
start := max(requiredPageSize*(requiredPage-1), 0)
462+
end := min(requiredPageSize*requiredPage, m.numberOfUsersforTesting)
463+
464+
if start >= m.numberOfUsersforTesting {
465+
if len(allUsers) != 0 {
466+
t.Errorf("Expected empty result for page beyond data, got %d users", len(allUsers))
467+
}
468+
} else {
469+
if !usersAreEqual(allUsers, m.usersForTesting[start:end]) {
470+
t.Errorf("Expected different set of users")
471+
}
472+
}
473+
}
474+
}
475+
})
476+
}
477+
}

0 commit comments

Comments
 (0)