Skip to content

Commit 4228628

Browse files
mmorel-35mmorel-35
committed
fix: use testify instead of t.Fatal or t.Error in server package
Signed-off-by: Matthieu MOREL <[email protected]>
1 parent 40b856e commit 4228628

18 files changed

+129
-296
lines changed

‎server/auth/jwt_test.go

+5-16
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ package auth
1616

1717
import (
1818
"context"
19-
"errors"
2019
"fmt"
2120
"testing"
2221
"time"
@@ -103,12 +102,8 @@ func testJWTInfo(t *testing.T, opts map[string]string) {
103102
t.Fatalf("%#v", aerr)
104103
}
105104
ai, ok := jwt.info(ctx, token, 123)
106-
if !ok {
107-
t.Fatalf("failed to authenticate with token %s", token)
108-
}
109-
if ai.Revision != 123 {
110-
t.Fatalf("expected revision 123, got %d", ai.Revision)
111-
}
105+
require.Truef(t, ok, "failed to authenticate with token %s", token)
106+
require.Equalf(t, uint64(123), ai.Revision, "expected revision 123, got %d", ai.Revision)
112107
ai, ok = jwt.info(ctx, "aaa", 120)
113108
if ok || ai != nil {
114109
t.Fatalf("expected aaa to fail to authenticate, got %+v", ai)
@@ -128,21 +123,15 @@ func testJWTInfo(t *testing.T, opts map[string]string) {
128123
}
129124

130125
ai, ok := verify.info(ctx, token, 123)
131-
if !ok {
132-
t.Fatalf("failed to authenticate with token %s", token)
133-
}
134-
if ai.Revision != 123 {
135-
t.Fatalf("expected revision 123, got %d", ai.Revision)
136-
}
126+
require.Truef(t, ok, "failed to authenticate with token %s", token)
127+
require.Equalf(t, uint64(123), ai.Revision, "expected revision 123, got %d", ai.Revision)
137128
ai, ok = verify.info(ctx, "aaa", 120)
138129
if ok || ai != nil {
139130
t.Fatalf("expected aaa to fail to authenticate, got %+v", ai)
140131
}
141132

142133
_, aerr := verify.assign(ctx, "abc", 123)
143-
if !errors.Is(aerr, ErrVerifyOnly) {
144-
t.Fatalf("unexpected error when attempting to sign with public key: %v", aerr)
145-
}
134+
require.ErrorIsf(t, aerr, ErrVerifyOnly, "unexpected error when attempting to sign with public key: %v", aerr)
146135
})
147136
}
148137
}

‎server/auth/store_test.go

+44-108
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"time"
2626

2727
"github.com/stretchr/testify/assert"
28+
"github.com/stretchr/testify/require"
2829
"go.uber.org/zap/zaptest"
2930
"golang.org/x/crypto/bcrypt"
3031
"google.golang.org/grpc/metadata"
@@ -64,9 +65,7 @@ func TestNewAuthStoreRevision(t *testing.T) {
6465
defer as.Close()
6566
new := as.Revision()
6667

67-
if old != new {
68-
t.Fatalf("expected revision %d, got %d", old, new)
69-
}
68+
require.Equalf(t, old, new, "expected revision %d, got %d", old, new)
7069
}
7170

7271
// TestNewAuthStoreBcryptCost ensures that NewAuthStore uses default when given bcrypt-cost is invalid
@@ -80,9 +79,7 @@ func TestNewAuthStoreBcryptCost(t *testing.T) {
8079
for _, invalidCost := range invalidCosts {
8180
as := NewAuthStore(zaptest.NewLogger(t), newBackendMock(), tp, invalidCost)
8281
defer as.Close()
83-
if as.BcryptCost() != bcrypt.DefaultCost {
84-
t.Fatalf("expected DefaultCost when bcryptcost is invalid")
85-
}
82+
require.Equalf(t, bcrypt.DefaultCost, as.BcryptCost(), "expected DefaultCost when bcryptcost is invalid")
8683
}
8784
}
8885

@@ -162,22 +159,17 @@ func TestUserAdd(t *testing.T) {
162159
const userName = "foo"
163160
ua := &pb.AuthUserAddRequest{Name: userName, Options: &authpb.UserAddOptions{NoPassword: false}}
164161
_, err := as.UserAdd(ua) // add an existing user
165-
if err == nil {
166-
t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err)
167-
}
168-
if !errors.Is(err, ErrUserAlreadyExist) {
169-
t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err)
170-
}
162+
require.Errorf(t, err, "expected %v, got %v", ErrUserAlreadyExist, err)
163+
require.ErrorIsf(t, err, ErrUserAlreadyExist, "expected %v, got %v", ErrUserAlreadyExist, err)
171164

172165
ua = &pb.AuthUserAddRequest{Name: "", Options: &authpb.UserAddOptions{NoPassword: false}}
173166
_, err = as.UserAdd(ua) // add a user with empty name
174167
if !errors.Is(err, ErrUserEmpty) {
175168
t.Fatal(err)
176169
}
177170

178-
if _, ok := as.rangePermCache[userName]; !ok {
179-
t.Fatalf("user %s should be added but it doesn't exist in rangePermCache", userName)
180-
}
171+
_, ok := as.rangePermCache[userName]
172+
require.Truef(t, ok, "user %s should be added but it doesn't exist in rangePermCache", userName)
181173
}
182174

183175
func TestRecover(t *testing.T) {
@@ -188,9 +180,7 @@ func TestRecover(t *testing.T) {
188180
as.enabled = false
189181
as.Recover(as.be)
190182

191-
if !as.IsAuthEnabled() {
192-
t.Fatalf("expected auth enabled got disabled")
193-
}
183+
require.Truef(t, as.IsAuthEnabled(), "expected auth enabled got disabled")
194184
}
195185

196186
func TestRecoverWithEmptyRangePermCache(t *testing.T) {
@@ -202,19 +192,13 @@ func TestRecoverWithEmptyRangePermCache(t *testing.T) {
202192
as.rangePermCache = map[string]*unifiedRangePermissions{}
203193
as.Recover(as.be)
204194

205-
if !as.IsAuthEnabled() {
206-
t.Fatalf("expected auth enabled got disabled")
207-
}
195+
require.Truef(t, as.IsAuthEnabled(), "expected auth enabled got disabled")
208196

209-
if len(as.rangePermCache) != 3 {
210-
t.Fatalf("rangePermCache should have permission information for 3 users (\"root\" and \"foo\",\"foo-no-user-options\"), but has %d information", len(as.rangePermCache))
211-
}
212-
if _, ok := as.rangePermCache["root"]; !ok {
213-
t.Fatal("user \"root\" should be created by setupAuthStore() but doesn't exist in rangePermCache")
214-
}
215-
if _, ok := as.rangePermCache["foo"]; !ok {
216-
t.Fatal("user \"foo\" should be created by setupAuthStore() but doesn't exist in rangePermCache")
217-
}
197+
require.Lenf(t, as.rangePermCache, 3, "rangePermCache should have permission information for 3 users (\"root\" and \"foo\",\"foo-no-user-options\"), but has %d information", len(as.rangePermCache))
198+
_, ok := as.rangePermCache["root"]
199+
require.Truef(t, ok, "user \"root\" should be created by setupAuthStore() but doesn't exist in rangePermCache")
200+
_, ok = as.rangePermCache["foo"]
201+
require.Truef(t, ok, "user \"foo\" should be created by setupAuthStore() but doesn't exist in rangePermCache")
218202
}
219203

220204
func TestCheckPassword(t *testing.T) {
@@ -223,12 +207,8 @@ func TestCheckPassword(t *testing.T) {
223207

224208
// auth a non-existing user
225209
_, err := as.CheckPassword("foo-test", "bar")
226-
if err == nil {
227-
t.Fatalf("expected %v, got %v", ErrAuthFailed, err)
228-
}
229-
if !errors.Is(err, ErrAuthFailed) {
230-
t.Fatalf("expected %v, got %v", ErrAuthFailed, err)
231-
}
210+
require.Errorf(t, err, "expected %v, got %v", ErrAuthFailed, err)
211+
require.ErrorIsf(t, err, ErrAuthFailed, "expected %v, got %v", ErrAuthFailed, err)
232212

233213
// auth an existing user with correct password
234214
_, err = as.CheckPassword("foo", "bar")
@@ -238,12 +218,8 @@ func TestCheckPassword(t *testing.T) {
238218

239219
// auth an existing user but with wrong password
240220
_, err = as.CheckPassword("foo", "")
241-
if err == nil {
242-
t.Fatalf("expected %v, got %v", ErrAuthFailed, err)
243-
}
244-
if !errors.Is(err, ErrAuthFailed) {
245-
t.Fatalf("expected %v, got %v", ErrAuthFailed, err)
246-
}
221+
require.Errorf(t, err, "expected %v, got %v", ErrAuthFailed, err)
222+
require.ErrorIsf(t, err, ErrAuthFailed, "expected %v, got %v", ErrAuthFailed, err)
247223
}
248224

249225
func TestUserDelete(t *testing.T) {
@@ -260,16 +236,11 @@ func TestUserDelete(t *testing.T) {
260236

261237
// delete a non-existing user
262238
_, err = as.UserDelete(ud)
263-
if err == nil {
264-
t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
265-
}
266-
if !errors.Is(err, ErrUserNotFound) {
267-
t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
268-
}
239+
require.Errorf(t, err, "expected %v, got %v", ErrUserNotFound, err)
240+
require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err)
269241

270-
if _, ok := as.rangePermCache[userName]; ok {
271-
t.Fatalf("user %s should be deleted but it exists in rangePermCache", userName)
272-
}
242+
_, ok := as.rangePermCache[userName]
243+
require.Falsef(t, ok, "user %s should be deleted but it exists in rangePermCache", userName)
273244
}
274245

275246
func TestUserDeleteAndPermCache(t *testing.T) {
@@ -286,13 +257,10 @@ func TestUserDeleteAndPermCache(t *testing.T) {
286257

287258
// delete a non-existing user
288259
_, err = as.UserDelete(ud)
289-
if !errors.Is(err, ErrUserNotFound) {
290-
t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
291-
}
260+
require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err)
292261

293-
if _, ok := as.rangePermCache[deletedUserName]; ok {
294-
t.Fatalf("user %s should be deleted but it exists in rangePermCache", deletedUserName)
295-
}
262+
_, ok := as.rangePermCache[deletedUserName]
263+
require.Falsef(t, ok, "user %s should be deleted but it exists in rangePermCache", deletedUserName)
296264

297265
// add a new user
298266
const newUser = "bar"
@@ -302,9 +270,8 @@ func TestUserDeleteAndPermCache(t *testing.T) {
302270
t.Fatal(err)
303271
}
304272

305-
if _, ok := as.rangePermCache[newUser]; !ok {
306-
t.Fatalf("user %s should exist but it doesn't exist in rangePermCache", deletedUserName)
307-
}
273+
_, ok = as.rangePermCache[newUser]
274+
require.Truef(t, ok, "user %s should exist but it doesn't exist in rangePermCache", deletedUserName)
308275
}
309276

310277
func TestUserChangePassword(t *testing.T) {
@@ -330,12 +297,8 @@ func TestUserChangePassword(t *testing.T) {
330297

331298
// change a non-existing user
332299
_, err = as.UserChangePassword(&pb.AuthUserChangePasswordRequest{Name: "foo-test", HashedPassword: encodePassword("bar")})
333-
if err == nil {
334-
t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
335-
}
336-
if !errors.Is(err, ErrUserNotFound) {
337-
t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
338-
}
300+
require.Errorf(t, err, "expected %v, got %v", ErrUserNotFound, err)
301+
require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err)
339302

340303
// change a user（user option is nil) password
341304
_, err = as.UserChangePassword(&pb.AuthUserChangePasswordRequest{Name: "foo-no-user-options", HashedPassword: encodePassword("bar")})
@@ -393,21 +356,15 @@ func TestHasRole(t *testing.T) {
393356

394357
// checks role reflects correctly
395358
hr := as.HasRole("foo", "role-test")
396-
if !hr {
397-
t.Fatal("expected role granted, got false")
398-
}
359+
require.Truef(t, hr, "expected role granted, got false")
399360

400361
// checks non existent role
401362
hr = as.HasRole("foo", "non-existent-role")
402-
if hr {
403-
t.Fatal("expected role not found, got true")
404-
}
363+
require.Falsef(t, hr, "expected role not found, got true")
405364

406365
// checks non existent user
407366
hr = as.HasRole("nouser", "role-test")
408-
if hr {
409-
t.Fatal("expected user not found got true")
410-
}
367+
require.Falsef(t, hr, "expected user not found got true")
411368
}
412369

413370
func TestIsOpPermitted(t *testing.T) {
@@ -470,9 +427,7 @@ func TestGetUser(t *testing.T) {
470427
if err != nil {
471428
t.Fatal(err)
472429
}
473-
if u == nil {
474-
t.Fatal("expect user not nil, got nil")
475-
}
430+
require.NotNilf(t, u, "expect user not nil, got nil")
476431
expected := []string{"role-test"}
477432

478433
assert.Equal(t, expected, u.Roles)
@@ -801,18 +756,13 @@ func TestUserRevokePermission(t *testing.T) {
801756
t.Fatal(err)
802757
}
803758

804-
if _, ok := as.rangePermCache[userName]; !ok {
805-
t.Fatalf("User %s should have its entry in rangePermCache", userName)
806-
}
759+
_, ok := as.rangePermCache[userName]
760+
require.Truef(t, ok, "User %s should have its entry in rangePermCache", userName)
807761
unifiedPerm := as.rangePermCache[userName]
808762
pt1 := adt.NewBytesAffinePoint([]byte("WriteKeyBegin"))
809-
if !unifiedPerm.writePerms.Contains(pt1) {
810-
t.Fatal("rangePermCache should contain WriteKeyBegin")
811-
}
763+
require.Truef(t, unifiedPerm.writePerms.Contains(pt1), "rangePermCache should contain WriteKeyBegin")
812764
pt2 := adt.NewBytesAffinePoint([]byte("OutOfRange"))
813-
if unifiedPerm.writePerms.Contains(pt2) {
814-
t.Fatal("rangePermCache should not contain OutOfRange")
815-
}
765+
require.Falsef(t, unifiedPerm.writePerms.Contains(pt2), "rangePermCache should not contain OutOfRange")
816766

817767
u, err := as.UserGet(&pb.AuthUserGetRequest{Name: userName})
818768
if err != nil {
@@ -1003,12 +953,8 @@ func TestRecoverFromSnapshot(t *testing.T) {
1003953

1004954
ua := &pb.AuthUserAddRequest{Name: "foo", Options: &authpb.UserAddOptions{NoPassword: false}}
1005955
_, err := as.UserAdd(ua) // add an existing user
1006-
if err == nil {
1007-
t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err)
1008-
}
1009-
if !errors.Is(err, ErrUserAlreadyExist) {
1010-
t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err)
1011-
}
956+
require.Errorf(t, err, "expected %v, got %v", ErrUserAlreadyExist, err)
957+
require.ErrorIsf(t, err, ErrUserAlreadyExist, "expected %v, got %v", ErrUserAlreadyExist, err)
1012958

1013959
ua = &pb.AuthUserAddRequest{Name: "", Options: &authpb.UserAddOptions{NoPassword: false}}
1014960
_, err = as.UserAdd(ua) // add a user with empty name
@@ -1025,9 +971,7 @@ func TestRecoverFromSnapshot(t *testing.T) {
1025971
as2 := NewAuthStore(zaptest.NewLogger(t), as.be, tp, bcrypt.MinCost)
1026972
defer as2.Close()
1027973

1028-
if !as2.IsAuthEnabled() {
1029-
t.Fatal("recovering authStore from existing backend failed")
1030-
}
974+
require.Truef(t, as2.IsAuthEnabled(), "recovering authStore from existing backend failed")
1031975

1032976
ul, err := as.UserList(&pb.AuthUserListRequest{})
1033977
if err != nil {
@@ -1167,9 +1111,7 @@ func testAuthInfoFromCtxWithRoot(t *testing.T, opts string) {
11671111
if aerr != nil {
11681112
t.Fatal(err)
11691113
}
1170-
if ai == nil {
1171-
t.Fatal("expected non-nil *AuthInfo")
1172-
}
1114+
require.NotNilf(t, ai, "expected non-nil *AuthInfo")
11731115
if ai.Username != "root" {
11741116
t.Errorf("expected user name 'root', got %+v", ai)
11751117
}
@@ -1188,9 +1130,7 @@ func TestUserNoPasswordAdd(t *testing.T) {
11881130

11891131
ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, "dummy")
11901132
_, err = as.Authenticate(ctx, username, "")
1191-
if !errors.Is(err, ErrAuthFailed) {
1192-
t.Fatalf("expected %v, got %v", ErrAuthFailed, err)
1193-
}
1133+
require.ErrorIsf(t, err, ErrAuthFailed, "expected %v, got %v", ErrAuthFailed, err)
11941134
}
11951135

11961136
func TestUserAddWithOldLog(t *testing.T) {
@@ -1227,10 +1167,6 @@ func TestUserChangePasswordWithOldLog(t *testing.T) {
12271167

12281168
// change a non-existing user
12291169
_, err = as.UserChangePassword(&pb.AuthUserChangePasswordRequest{Name: "foo-test", HashedPassword: encodePassword("bar")})
1230-
if err == nil {
1231-
t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
1232-
}
1233-
if !errors.Is(err, ErrUserNotFound) {
1234-
t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
1235-
}
1170+
require.Errorf(t, err, "expected %v, got %v", ErrUserNotFound, err)
1171+
require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err)
12361172
}

‎server/config/config_test.go

+4-9
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"net/url"
1919
"testing"
2020

21+
"github.com/stretchr/testify/require"
2122
"go.uber.org/zap/zaptest"
2223

2324
"go.etcd.io/etcd/client/pkg/v3/types"
@@ -28,9 +29,7 @@ func mustNewURLs(t *testing.T, urls []string) []url.URL {
2829
return nil
2930
}
3031
u, err := types.NewURLs(urls)
31-
if err != nil {
32-
t.Fatalf("error creating new URLs from %q: %v", urls, err)
33-
}
32+
require.NoErrorf(t, err, "error creating new URLs from %q: %v", urls, err)
3433
return u
3534
}
3635

@@ -48,9 +47,7 @@ func TestConfigVerifyBootstrapWithoutClusterAndDiscoveryURLFail(t *testing.T) {
4847

4948
func TestConfigVerifyExistingWithDiscoveryURLFail(t *testing.T) {
5049
cluster, err := types.NewURLsMap("node1=http://127.0.0.1:2380")
51-
if err != nil {
52-
t.Fatalf("NewCluster error: %v", err)
53-
}
50+
require.NoErrorf(t, err, "NewCluster error: %v", err)
5451
c := &ServerConfig{
5552
Name: "node1",
5653
DiscoveryURL: "http://127.0.0.1:2379/abcdefg",
@@ -139,9 +136,7 @@ func TestConfigVerifyLocalMember(t *testing.T) {
139136

140137
for i, tt := range tests {
141138
cluster, err := types.NewURLsMap(tt.clusterSetting)
142-
if err != nil {
143-
t.Fatalf("#%d: Got unexpected error: %v", i, err)
144-
}
139+
require.NoErrorf(t, err, "#%d: Got unexpected error: %v", i, err)
145140
cfg := ServerConfig{
146141
Name: "node1",
147142
InitialPeerURLsMap: cluster,

0 commit comments

Comments
 (0)