Skip to content

Commit 386bb92

Browse files
committed
refactor: changed the test design to use global functional variables instead of interfaces
Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>
1 parent ee9bfef commit 386bb92

File tree

2 files changed

+44
-47
lines changed

2 files changed

+44
-47
lines changed

cmd/harbor/root/user/delete.go

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,40 +22,29 @@ import (
2222
"github.com/spf13/cobra"
2323
)
2424

25-
type UserDeleter interface {
26-
UserDelete(userID int64) error
27-
GetUserIDByName(username string) (int64, error)
28-
GetUserIDFromUser() int64
29-
}
30-
type DefaultUserDeleter struct{}
31-
32-
func (d *DefaultUserDeleter) UserDelete(userID int64) error {
33-
return api.DeleteUser(userID)
34-
}
35-
func (d *DefaultUserDeleter) GetUserIDByName(username string) (int64, error) {
36-
return api.GetUsersIdByName(username)
37-
}
38-
func (d *DefaultUserDeleter) GetUserIDFromUser() int64 {
39-
return prompt.GetUserIdFromUser()
40-
}
25+
var (
26+
userDeleteAPI = api.DeleteUser
27+
getUserIDByName = api.GetUsersIdByName
28+
getUserIDFromUser = prompt.GetUserIdFromUser
29+
)
4130

42-
func DeleteUser(args []string, userDeleter UserDeleter) {
31+
func DeleteUser(args []string) {
4332
// If there are command line arguments, process them concurrently.
4433
if len(args) > 0 {
4534
var wg sync.WaitGroup
4635
errChan := make(chan error, len(args)) // Channel to collect errors
4736

4837
for _, arg := range args {
4938
// Retrieve user ID by name.
50-
userID, err := userDeleter.GetUserIDByName(arg)
39+
userID, err := getUserIDByName(arg)
5140
if err != nil {
5241
log.Errorf("failed to get user id for '%s': %v", arg, err)
5342
continue
5443
}
5544
wg.Add(1)
5645
go func(userID int64) {
5746
defer wg.Done()
58-
if err := userDeleter.UserDelete(userID); err != nil {
47+
if err := userDeleteAPI(userID); err != nil {
5948
errChan <- err
6049
}
6150
}(userID)
@@ -77,8 +66,8 @@ func DeleteUser(args []string, userDeleter UserDeleter) {
7766
}
7867
} else {
7968
// Interactive mode: get the user ID from the prompt.
80-
userID := userDeleter.GetUserIDFromUser()
81-
if err := userDeleter.UserDelete(userID); err != nil {
69+
userID := getUserIDFromUser()
70+
if err := userDeleteAPI(userID); err != nil {
8271
if isUnauthorizedError(err) {
8372
log.Error("Permission denied: Admin privileges are required to execute this command.")
8473
} else {
@@ -95,8 +84,7 @@ func UserDeleteCmd() *cobra.Command {
9584
Short: "delete user by name or id", // nope it only deletes by name
9685
Args: cobra.MinimumNArgs(0),
9786
Run: func(cmd *cobra.Command, args []string) {
98-
d := &DefaultUserDeleter{}
99-
DeleteUser(args, d)
87+
DeleteUser(args)
10088
},
10189
}
10290

cmd/harbor/root/user/delete_test.go

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,14 @@ import (
2323
"github.com/stretchr/testify/assert"
2424
)
2525

26-
type MockUserDeleter struct {
26+
type mockUserDeleter struct {
2727
mu sync.Mutex
2828
id map[string]int64
2929
user map[int64]string
30-
userCnt int
3130
expectAuthError bool
3231
}
3332

34-
func (m *MockUserDeleter) UserDelete(userID int64) error {
33+
func (m *mockUserDeleter) userDelete(userID int64) error {
3534
m.mu.Lock()
3635
defer m.mu.Unlock()
3736
if m.expectAuthError {
@@ -44,7 +43,7 @@ func (m *MockUserDeleter) UserDelete(userID int64) error {
4443
}
4544
return fmt.Errorf("user %d not found", userID)
4645
}
47-
func (m *MockUserDeleter) GetUserIDByName(username string) (int64, error) {
46+
func (m *mockUserDeleter) getUserIDByName(username string) (int64, error) {
4847
m.mu.Lock()
4948
defer m.mu.Unlock()
5049
if v, ok := m.id[username]; ok {
@@ -53,13 +52,12 @@ func (m *MockUserDeleter) GetUserIDByName(username string) (int64, error) {
5352
return 0, fmt.Errorf(`Username %s not found`, username)
5453
}
5554
}
56-
func (m *MockUserDeleter) GetUserIDFromUser() int64 {
55+
func (m *mockUserDeleter) getUserIDFromUser() int64 {
5756
return 999
5857
}
5958

60-
func initMockUserDeleter(userCnt int, expectAuthError bool) *MockUserDeleter {
61-
m := &MockUserDeleter{
62-
userCnt: userCnt,
59+
func newMockUserDeleter(userCnt int, expectAuthError bool) *mockUserDeleter {
60+
m := &mockUserDeleter{
6361
expectAuthError: expectAuthError,
6462
id: make(map[string]int64),
6563
user: make(map[int64]string),
@@ -68,65 +66,76 @@ func initMockUserDeleter(userCnt int, expectAuthError bool) *MockUserDeleter {
6866
m.id[fmt.Sprintf("test%d", i+1)] = int64(i + 1)
6967
m.user[int64(i+1)] = fmt.Sprintf("test%d", i+1)
7068
}
69+
getUserIDByName = m.getUserIDByName
70+
getUserIDFromUser = m.getUserIDFromUser
71+
userDeleteAPI = m.userDelete
7172
return m
7273
}
7374
func TestDeleteUser(t *testing.T) {
75+
origUserDelete := userDeleteAPI
76+
origGetUserIDByName := getUserIDByName
77+
origGetUserIDFromUser := getUserIDFromUser
78+
defer func() {
79+
userDeleteAPI = origUserDelete
80+
getUserIDByName = origGetUserIDByName
81+
getUserIDFromUser = origGetUserIDFromUser
82+
}()
7483
tests := []struct {
7584
name string
76-
setup func() *MockUserDeleter
85+
setup func() *mockUserDeleter
7786
args []string
7887
notExpectedID []int64
7988
expectedErr string
8089
}{
8190
{
8291
name: "successfully delete single user",
83-
setup: func() *MockUserDeleter {
84-
return initMockUserDeleter(5, false)
92+
setup: func() *mockUserDeleter {
93+
return newMockUserDeleter(5, false)
8594
},
8695
args: []string{"test1"},
8796
notExpectedID: []int64{1},
8897
expectedErr: "",
8998
},
9099
{
91100
name: "successfully delete multiple users",
92-
setup: func() *MockUserDeleter {
93-
return initMockUserDeleter(5, false)
101+
setup: func() *mockUserDeleter {
102+
return newMockUserDeleter(5, false)
94103
},
95104
args: []string{"test1", "test3", "test5"},
96105
notExpectedID: []int64{1, 3, 5},
97106
expectedErr: "",
98107
},
99108
{
100109
name: "delete non-existent user logs error",
101-
setup: func() *MockUserDeleter {
102-
return initMockUserDeleter(5, false)
110+
setup: func() *mockUserDeleter {
111+
return newMockUserDeleter(5, false)
103112
},
104113
args: []string{"nonexistent"},
105114
notExpectedID: []int64{},
106115
expectedErr: "failed to get user id",
107116
},
108117
{
109118
name: "permission denied error",
110-
setup: func() *MockUserDeleter {
111-
return initMockUserDeleter(5, true)
119+
setup: func() *mockUserDeleter {
120+
return newMockUserDeleter(5, true)
112121
},
113122
args: []string{"test1"},
114123
notExpectedID: []int64{},
115124
expectedErr: "Permission denied",
116125
},
117126
{
118127
name: "mixed existing and non-existing users",
119-
setup: func() *MockUserDeleter {
120-
return initMockUserDeleter(5, false)
128+
setup: func() *mockUserDeleter {
129+
return newMockUserDeleter(5, false)
121130
},
122131
args: []string{"test1", "nonexistent", "test3"},
123132
notExpectedID: []int64{1, 3},
124133
expectedErr: "failed to get user id",
125134
},
126135
{
127136
name: "delete with empty args does not error",
128-
setup: func() *MockUserDeleter {
129-
m := initMockUserDeleter(5, false)
137+
setup: func() *mockUserDeleter {
138+
m := newMockUserDeleter(5, false)
130139
m.user[999] = "promptuser"
131140
m.id["promptuser"] = 999
132141
return m
@@ -137,8 +146,8 @@ func TestDeleteUser(t *testing.T) {
137146
},
138147
{
139148
name: "delete all users",
140-
setup: func() *MockUserDeleter {
141-
return initMockUserDeleter(3, false)
149+
setup: func() *mockUserDeleter {
150+
return newMockUserDeleter(3, false)
142151
},
143152
args: []string{"test1", "test2", "test3"},
144153
notExpectedID: []int64{1, 2, 3},
@@ -154,7 +163,7 @@ func TestDeleteUser(t *testing.T) {
154163
defer log.SetOutput(originalLogOutput)
155164

156165
m := tt.setup()
157-
DeleteUser(tt.args, m)
166+
DeleteUser(tt.args)
158167

159168
logs := buf.String()
160169

0 commit comments

Comments
 (0)