From 4ab4c3843947c2550eefbe2a17250100e5b7728d Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Sat, 30 Nov 2024 15:14:45 +0100 Subject: [PATCH] fix: use testify instead of t.Fatal or t.Error in server package Signed-off-by: Matthieu MOREL --- server/auth/jwt_test.go | 21 +--- server/auth/store_test.go | 152 +++++++----------------- server/config/config_test.go | 13 +- server/embed/config_test.go | 36 ++---- server/embed/etcd_test.go | 8 +- server/embed/serve_test.go | 8 +- server/etcdmain/config_test.go | 10 +- server/etcdserver/bootstrap_test.go | 22 ++-- server/etcdserver/corrupt_test.go | 33 ++--- server/etcdserver/server_test.go | 9 +- server/storage/schema/actions_test.go | 9 +- server/storage/schema/changes_test.go | 6 +- server/storage/schema/migration_test.go | 5 +- server/storage/schema/schema_test.go | 5 +- server/storage/schema/version_test.go | 9 +- server/storage/wal/repair_test.go | 9 +- server/storage/wal/wal_bench_test.go | 5 +- server/storage/wal/wal_test.go | 65 +++------- 18 files changed, 129 insertions(+), 296 deletions(-) diff --git a/server/auth/jwt_test.go b/server/auth/jwt_test.go index ff0e4c41989..8085c4c1f50 100644 --- a/server/auth/jwt_test.go +++ b/server/auth/jwt_test.go @@ -16,7 +16,6 @@ package auth import ( "context" - "errors" "fmt" "testing" "time" @@ -103,12 +102,8 @@ func testJWTInfo(t *testing.T, opts map[string]string) { t.Fatalf("%#v", aerr) } ai, ok := jwt.info(ctx, token, 123) - if !ok { - t.Fatalf("failed to authenticate with token %s", token) - } - if ai.Revision != 123 { - t.Fatalf("expected revision 123, got %d", ai.Revision) - } + require.Truef(t, ok, "failed to authenticate with token %s", token) + require.Equalf(t, uint64(123), ai.Revision, "expected revision 123, got %d", ai.Revision) ai, ok = jwt.info(ctx, "aaa", 120) if ok || ai != nil { t.Fatalf("expected aaa to fail to authenticate, got %+v", ai) @@ -128,21 +123,15 @@ func testJWTInfo(t *testing.T, opts map[string]string) { } ai, ok := verify.info(ctx, token, 123) - if !ok { - t.Fatalf("failed to authenticate with token %s", token) - } - if ai.Revision != 123 { - t.Fatalf("expected revision 123, got %d", ai.Revision) - } + require.Truef(t, ok, "failed to authenticate with token %s", token) + require.Equalf(t, uint64(123), ai.Revision, "expected revision 123, got %d", ai.Revision) ai, ok = verify.info(ctx, "aaa", 120) if ok || ai != nil { t.Fatalf("expected aaa to fail to authenticate, got %+v", ai) } _, aerr := verify.assign(ctx, "abc", 123) - if !errors.Is(aerr, ErrVerifyOnly) { - t.Fatalf("unexpected error when attempting to sign with public key: %v", aerr) - } + require.ErrorIsf(t, aerr, ErrVerifyOnly, "unexpected error when attempting to sign with public key: %v", aerr) }) } } diff --git a/server/auth/store_test.go b/server/auth/store_test.go index c8cd5cad7cc..b6b81be568d 100644 --- a/server/auth/store_test.go +++ b/server/auth/store_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" "golang.org/x/crypto/bcrypt" "google.golang.org/grpc/metadata" @@ -64,9 +65,7 @@ func TestNewAuthStoreRevision(t *testing.T) { defer as.Close() new := as.Revision() - if old != new { - t.Fatalf("expected revision %d, got %d", old, new) - } + require.Equalf(t, old, new, "expected revision %d, got %d", old, new) } // TestNewAuthStoreBcryptCost ensures that NewAuthStore uses default when given bcrypt-cost is invalid @@ -80,9 +79,7 @@ func TestNewAuthStoreBcryptCost(t *testing.T) { for _, invalidCost := range invalidCosts { as := NewAuthStore(zaptest.NewLogger(t), newBackendMock(), tp, invalidCost) defer as.Close() - if as.BcryptCost() != bcrypt.DefaultCost { - t.Fatalf("expected DefaultCost when bcryptcost is invalid") - } + require.Equalf(t, bcrypt.DefaultCost, as.BcryptCost(), "expected DefaultCost when bcryptcost is invalid") } } @@ -162,12 +159,8 @@ func TestUserAdd(t *testing.T) { const userName = "foo" ua := &pb.AuthUserAddRequest{Name: userName, Options: &authpb.UserAddOptions{NoPassword: false}} _, err := as.UserAdd(ua) // add an existing user - if err == nil { - t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err) - } - if !errors.Is(err, ErrUserAlreadyExist) { - t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err) - } + require.Errorf(t, err, "expected %v, got %v", ErrUserAlreadyExist, err) + require.ErrorIsf(t, err, ErrUserAlreadyExist, "expected %v, got %v", ErrUserAlreadyExist, err) ua = &pb.AuthUserAddRequest{Name: "", Options: &authpb.UserAddOptions{NoPassword: false}} _, err = as.UserAdd(ua) // add a user with empty name @@ -175,9 +168,8 @@ func TestUserAdd(t *testing.T) { t.Fatal(err) } - if _, ok := as.rangePermCache[userName]; !ok { - t.Fatalf("user %s should be added but it doesn't exist in rangePermCache", userName) - } + _, ok := as.rangePermCache[userName] + require.Truef(t, ok, "user %s should be added but it doesn't exist in rangePermCache", userName) } func TestRecover(t *testing.T) { @@ -188,9 +180,7 @@ func TestRecover(t *testing.T) { as.enabled = false as.Recover(as.be) - if !as.IsAuthEnabled() { - t.Fatalf("expected auth enabled got disabled") - } + require.Truef(t, as.IsAuthEnabled(), "expected auth enabled got disabled") } func TestRecoverWithEmptyRangePermCache(t *testing.T) { @@ -202,19 +192,13 @@ func TestRecoverWithEmptyRangePermCache(t *testing.T) { as.rangePermCache = map[string]*unifiedRangePermissions{} as.Recover(as.be) - if !as.IsAuthEnabled() { - t.Fatalf("expected auth enabled got disabled") - } + require.Truef(t, as.IsAuthEnabled(), "expected auth enabled got disabled") - if len(as.rangePermCache) != 3 { - t.Fatalf("rangePermCache should have permission information for 3 users (\"root\" and \"foo\",\"foo-no-user-options\"), but has %d information", len(as.rangePermCache)) - } - if _, ok := as.rangePermCache["root"]; !ok { - t.Fatal("user \"root\" should be created by setupAuthStore() but doesn't exist in rangePermCache") - } - if _, ok := as.rangePermCache["foo"]; !ok { - t.Fatal("user \"foo\" should be created by setupAuthStore() but doesn't exist in rangePermCache") - } + 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)) + _, ok := as.rangePermCache["root"] + require.Truef(t, ok, "user \"root\" should be created by setupAuthStore() but doesn't exist in rangePermCache") + _, ok = as.rangePermCache["foo"] + require.Truef(t, ok, "user \"foo\" should be created by setupAuthStore() but doesn't exist in rangePermCache") } func TestCheckPassword(t *testing.T) { @@ -223,12 +207,8 @@ func TestCheckPassword(t *testing.T) { // auth a non-existing user _, err := as.CheckPassword("foo-test", "bar") - if err == nil { - t.Fatalf("expected %v, got %v", ErrAuthFailed, err) - } - if !errors.Is(err, ErrAuthFailed) { - t.Fatalf("expected %v, got %v", ErrAuthFailed, err) - } + require.Errorf(t, err, "expected %v, got %v", ErrAuthFailed, err) + require.ErrorIsf(t, err, ErrAuthFailed, "expected %v, got %v", ErrAuthFailed, err) // auth an existing user with correct password _, err = as.CheckPassword("foo", "bar") @@ -238,12 +218,8 @@ func TestCheckPassword(t *testing.T) { // auth an existing user but with wrong password _, err = as.CheckPassword("foo", "") - if err == nil { - t.Fatalf("expected %v, got %v", ErrAuthFailed, err) - } - if !errors.Is(err, ErrAuthFailed) { - t.Fatalf("expected %v, got %v", ErrAuthFailed, err) - } + require.Errorf(t, err, "expected %v, got %v", ErrAuthFailed, err) + require.ErrorIsf(t, err, ErrAuthFailed, "expected %v, got %v", ErrAuthFailed, err) } func TestUserDelete(t *testing.T) { @@ -260,16 +236,11 @@ func TestUserDelete(t *testing.T) { // delete a non-existing user _, err = as.UserDelete(ud) - if err == nil { - t.Fatalf("expected %v, got %v", ErrUserNotFound, err) - } - if !errors.Is(err, ErrUserNotFound) { - t.Fatalf("expected %v, got %v", ErrUserNotFound, err) - } + require.Errorf(t, err, "expected %v, got %v", ErrUserNotFound, err) + require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err) - if _, ok := as.rangePermCache[userName]; ok { - t.Fatalf("user %s should be deleted but it exists in rangePermCache", userName) - } + _, ok := as.rangePermCache[userName] + require.Falsef(t, ok, "user %s should be deleted but it exists in rangePermCache", userName) } func TestUserDeleteAndPermCache(t *testing.T) { @@ -286,13 +257,10 @@ func TestUserDeleteAndPermCache(t *testing.T) { // delete a non-existing user _, err = as.UserDelete(ud) - if !errors.Is(err, ErrUserNotFound) { - t.Fatalf("expected %v, got %v", ErrUserNotFound, err) - } + require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err) - if _, ok := as.rangePermCache[deletedUserName]; ok { - t.Fatalf("user %s should be deleted but it exists in rangePermCache", deletedUserName) - } + _, ok := as.rangePermCache[deletedUserName] + require.Falsef(t, ok, "user %s should be deleted but it exists in rangePermCache", deletedUserName) // add a new user const newUser = "bar" @@ -302,9 +270,8 @@ func TestUserDeleteAndPermCache(t *testing.T) { t.Fatal(err) } - if _, ok := as.rangePermCache[newUser]; !ok { - t.Fatalf("user %s should exist but it doesn't exist in rangePermCache", deletedUserName) - } + _, ok = as.rangePermCache[newUser] + require.Truef(t, ok, "user %s should exist but it doesn't exist in rangePermCache", deletedUserName) } func TestUserChangePassword(t *testing.T) { @@ -330,12 +297,8 @@ func TestUserChangePassword(t *testing.T) { // change a non-existing user _, err = as.UserChangePassword(&pb.AuthUserChangePasswordRequest{Name: "foo-test", HashedPassword: encodePassword("bar")}) - if err == nil { - t.Fatalf("expected %v, got %v", ErrUserNotFound, err) - } - if !errors.Is(err, ErrUserNotFound) { - t.Fatalf("expected %v, got %v", ErrUserNotFound, err) - } + require.Errorf(t, err, "expected %v, got %v", ErrUserNotFound, err) + require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err) // change a user（user option is nil) password _, err = as.UserChangePassword(&pb.AuthUserChangePasswordRequest{Name: "foo-no-user-options", HashedPassword: encodePassword("bar")}) @@ -393,21 +356,15 @@ func TestHasRole(t *testing.T) { // checks role reflects correctly hr := as.HasRole("foo", "role-test") - if !hr { - t.Fatal("expected role granted, got false") - } + require.Truef(t, hr, "expected role granted, got false") // checks non existent role hr = as.HasRole("foo", "non-existent-role") - if hr { - t.Fatal("expected role not found, got true") - } + require.Falsef(t, hr, "expected role not found, got true") // checks non existent user hr = as.HasRole("nouser", "role-test") - if hr { - t.Fatal("expected user not found got true") - } + require.Falsef(t, hr, "expected user not found got true") } func TestIsOpPermitted(t *testing.T) { @@ -470,9 +427,7 @@ func TestGetUser(t *testing.T) { if err != nil { t.Fatal(err) } - if u == nil { - t.Fatal("expect user not nil, got nil") - } + require.NotNilf(t, u, "expect user not nil, got nil") expected := []string{"role-test"} assert.Equal(t, expected, u.Roles) @@ -801,18 +756,13 @@ func TestUserRevokePermission(t *testing.T) { t.Fatal(err) } - if _, ok := as.rangePermCache[userName]; !ok { - t.Fatalf("User %s should have its entry in rangePermCache", userName) - } + _, ok := as.rangePermCache[userName] + require.Truef(t, ok, "User %s should have its entry in rangePermCache", userName) unifiedPerm := as.rangePermCache[userName] pt1 := adt.NewBytesAffinePoint([]byte("WriteKeyBegin")) - if !unifiedPerm.writePerms.Contains(pt1) { - t.Fatal("rangePermCache should contain WriteKeyBegin") - } + require.Truef(t, unifiedPerm.writePerms.Contains(pt1), "rangePermCache should contain WriteKeyBegin") pt2 := adt.NewBytesAffinePoint([]byte("OutOfRange")) - if unifiedPerm.writePerms.Contains(pt2) { - t.Fatal("rangePermCache should not contain OutOfRange") - } + require.Falsef(t, unifiedPerm.writePerms.Contains(pt2), "rangePermCache should not contain OutOfRange") u, err := as.UserGet(&pb.AuthUserGetRequest{Name: userName}) if err != nil { @@ -1003,12 +953,8 @@ func TestRecoverFromSnapshot(t *testing.T) { ua := &pb.AuthUserAddRequest{Name: "foo", Options: &authpb.UserAddOptions{NoPassword: false}} _, err := as.UserAdd(ua) // add an existing user - if err == nil { - t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err) - } - if !errors.Is(err, ErrUserAlreadyExist) { - t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err) - } + require.Errorf(t, err, "expected %v, got %v", ErrUserAlreadyExist, err) + require.ErrorIsf(t, err, ErrUserAlreadyExist, "expected %v, got %v", ErrUserAlreadyExist, err) ua = &pb.AuthUserAddRequest{Name: "", Options: &authpb.UserAddOptions{NoPassword: false}} _, err = as.UserAdd(ua) // add a user with empty name @@ -1025,9 +971,7 @@ func TestRecoverFromSnapshot(t *testing.T) { as2 := NewAuthStore(zaptest.NewLogger(t), as.be, tp, bcrypt.MinCost) defer as2.Close() - if !as2.IsAuthEnabled() { - t.Fatal("recovering authStore from existing backend failed") - } + require.Truef(t, as2.IsAuthEnabled(), "recovering authStore from existing backend failed") ul, err := as.UserList(&pb.AuthUserListRequest{}) if err != nil { @@ -1167,9 +1111,7 @@ func testAuthInfoFromCtxWithRoot(t *testing.T, opts string) { if aerr != nil { t.Fatal(err) } - if ai == nil { - t.Fatal("expected non-nil *AuthInfo") - } + require.NotNilf(t, ai, "expected non-nil *AuthInfo") if ai.Username != "root" { t.Errorf("expected user name 'root', got %+v", ai) } @@ -1188,9 +1130,7 @@ func TestUserNoPasswordAdd(t *testing.T) { ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, "dummy") _, err = as.Authenticate(ctx, username, "") - if !errors.Is(err, ErrAuthFailed) { - t.Fatalf("expected %v, got %v", ErrAuthFailed, err) - } + require.ErrorIsf(t, err, ErrAuthFailed, "expected %v, got %v", ErrAuthFailed, err) } func TestUserAddWithOldLog(t *testing.T) { @@ -1227,10 +1167,6 @@ func TestUserChangePasswordWithOldLog(t *testing.T) { // change a non-existing user _, err = as.UserChangePassword(&pb.AuthUserChangePasswordRequest{Name: "foo-test", HashedPassword: encodePassword("bar")}) - if err == nil { - t.Fatalf("expected %v, got %v", ErrUserNotFound, err) - } - if !errors.Is(err, ErrUserNotFound) { - t.Fatalf("expected %v, got %v", ErrUserNotFound, err) - } + require.Errorf(t, err, "expected %v, got %v", ErrUserNotFound, err) + require.ErrorIsf(t, err, ErrUserNotFound, "expected %v, got %v", ErrUserNotFound, err) } diff --git a/server/config/config_test.go b/server/config/config_test.go index 069dc9e1315..b1ae74b789f 100644 --- a/server/config/config_test.go +++ b/server/config/config_test.go @@ -18,6 +18,7 @@ import ( "net/url" "testing" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" "go.etcd.io/etcd/client/pkg/v3/types" @@ -28,9 +29,7 @@ func mustNewURLs(t *testing.T, urls []string) []url.URL { return nil } u, err := types.NewURLs(urls) - if err != nil { - t.Fatalf("error creating new URLs from %q: %v", urls, err) - } + require.NoErrorf(t, err, "error creating new URLs from %q: %v", urls, err) return u } @@ -48,9 +47,7 @@ func TestConfigVerifyBootstrapWithoutClusterAndDiscoveryURLFail(t *testing.T) { func TestConfigVerifyExistingWithDiscoveryURLFail(t *testing.T) { cluster, err := types.NewURLsMap("node1=http://127.0.0.1:2380") - if err != nil { - t.Fatalf("NewCluster error: %v", err) - } + require.NoErrorf(t, err, "NewCluster error: %v", err) c := &ServerConfig{ Name: "node1", DiscoveryURL: "http://127.0.0.1:2379/abcdefg", @@ -139,9 +136,7 @@ func TestConfigVerifyLocalMember(t *testing.T) { for i, tt := range tests { cluster, err := types.NewURLsMap(tt.clusterSetting) - if err != nil { - t.Fatalf("#%d: Got unexpected error: %v", i, err) - } + require.NoErrorf(t, err, "#%d: Got unexpected error: %v", i, err) cfg := ServerConfig{ Name: "node1", InitialPeerURLsMap: cluster, diff --git a/server/embed/config_test.go b/server/embed/config_test.go index 8ba8e17dd09..b01a79c9f0d 100644 --- a/server/embed/config_test.go +++ b/server/embed/config_test.go @@ -231,9 +231,7 @@ func TestConfigFileFeatureGates(t *testing.T) { cfg, err := ConfigFromFile(tmpfile.Name()) if tc.expectErr { - if err == nil { - t.Fatal("expect parse error") - } + require.Errorf(t, err, "expect parse error") return } if err != nil { @@ -262,17 +260,11 @@ func TestUpdateDefaultClusterFromName(t *testing.T) { // in case of 'etcd --name=abc' exp := fmt.Sprintf("%s=%s://localhost:%s", cfg.Name, oldscheme, lpport) _, _ = cfg.UpdateDefaultClusterFromName(defaultInitialCluster) - if exp != cfg.InitialCluster { - t.Fatalf("initial-cluster expected %q, got %q", exp, cfg.InitialCluster) - } + require.Equalf(t, exp, cfg.InitialCluster, "initial-cluster expected %q, got %q", exp, cfg.InitialCluster) // advertise peer URL should not be affected - if origpeer != cfg.AdvertisePeerUrls[0].String() { - t.Fatalf("advertise peer url expected %q, got %q", origadvc, cfg.AdvertisePeerUrls[0].String()) - } + require.Equalf(t, origpeer, cfg.AdvertisePeerUrls[0].String(), "advertise peer url expected %q, got %q", origadvc, cfg.AdvertisePeerUrls[0].String()) // advertise client URL should not be affected - if origadvc != cfg.AdvertiseClientUrls[0].String() { - t.Fatalf("advertise client url expected %q, got %q", origadvc, cfg.AdvertiseClientUrls[0].String()) - } + require.Equalf(t, origadvc, cfg.AdvertiseClientUrls[0].String(), "advertise client url expected %q, got %q", origadvc, cfg.AdvertiseClientUrls[0].String()) } // TestUpdateDefaultClusterFromNameOverwrite ensures that machine's default host is only used @@ -291,25 +283,15 @@ func TestUpdateDefaultClusterFromNameOverwrite(t *testing.T) { lpport := cfg.ListenPeerUrls[0].Port() cfg.ListenPeerUrls[0] = url.URL{Scheme: cfg.ListenPeerUrls[0].Scheme, Host: fmt.Sprintf("0.0.0.0:%s", lpport)} dhost, _ := cfg.UpdateDefaultClusterFromName(defaultInitialCluster) - if dhost != defaultHostname { - t.Fatalf("expected default host %q, got %q", defaultHostname, dhost) - } + require.Equalf(t, dhost, defaultHostname, "expected default host %q, got %q", defaultHostname, dhost) aphost, apport := cfg.AdvertisePeerUrls[0].Hostname(), cfg.AdvertisePeerUrls[0].Port() - if apport != lpport { - t.Fatalf("advertise peer url got different port %s, expected %s", apport, lpport) - } - if aphost != defaultHostname { - t.Fatalf("advertise peer url expected machine default host %q, got %q", defaultHostname, aphost) - } + require.Equalf(t, apport, lpport, "advertise peer url got different port %s, expected %s", apport, lpport) + require.Equalf(t, aphost, defaultHostname, "advertise peer url expected machine default host %q, got %q", defaultHostname, aphost) expected := fmt.Sprintf("%s=%s://%s:%s", cfg.Name, oldscheme, defaultHostname, lpport) - if expected != cfg.InitialCluster { - t.Fatalf("initial-cluster expected %q, got %q", expected, cfg.InitialCluster) - } + require.Equalf(t, expected, cfg.InitialCluster, "initial-cluster expected %q, got %q", expected, cfg.InitialCluster) // advertise client URL should not be affected - if origadvc != cfg.AdvertiseClientUrls[0].String() { - t.Fatalf("advertise-client-url expected %q, got %q", origadvc, cfg.AdvertiseClientUrls[0].String()) - } + require.Equalf(t, origadvc, cfg.AdvertiseClientUrls[0].String(), "advertise-client-url expected %q, got %q", origadvc, cfg.AdvertiseClientUrls[0].String()) } func TestInferLocalAddr(t *testing.T) { diff --git a/server/embed/etcd_test.go b/server/embed/etcd_test.go index 206cbb737b4..18bc1e3bbfb 100644 --- a/server/embed/etcd_test.go +++ b/server/embed/etcd_test.go @@ -15,10 +15,11 @@ package embed import ( - "errors" "net/url" "testing" + "github.com/stretchr/testify/require" + "go.etcd.io/etcd/client/pkg/v3/transport" ) @@ -33,7 +34,6 @@ func TestEmptyClientTLSInfo_createMetricsListener(t *testing.T) { Scheme: "https", Host: "localhost:8080", } - if _, err := e.createMetricsListener(murl); !errors.Is(err, ErrMissingClientTLSInfoForMetricsURL) { - t.Fatalf("expected error %v, got %v", ErrMissingClientTLSInfoForMetricsURL, err) - } + _, err := e.createMetricsListener(murl) + require.ErrorIsf(t, err, ErrMissingClientTLSInfoForMetricsURL, "expected error %v, got %v", ErrMissingClientTLSInfoForMetricsURL, err) } diff --git a/server/embed/serve_test.go b/server/embed/serve_test.go index 6150beeecf1..487a82c915c 100644 --- a/server/embed/serve_test.go +++ b/server/embed/serve_test.go @@ -15,12 +15,13 @@ package embed import ( - "errors" "fmt" "net/url" "os" "testing" + "github.com/stretchr/testify/require" + "go.etcd.io/etcd/server/v3/auth" ) @@ -44,9 +45,8 @@ func TestStartEtcdWrongToken(t *testing.T) { cfg.Dir = tdir cfg.AuthToken = "wrong-token" - if _, err := StartEtcd(cfg); !errors.Is(err, auth.ErrInvalidAuthOpts) { - t.Fatalf("expected %v, got %v", auth.ErrInvalidAuthOpts, err) - } + _, err := StartEtcd(cfg) + require.ErrorIsf(t, err, auth.ErrInvalidAuthOpts, "expected %v, got %v", auth.ErrInvalidAuthOpts, err) } func newEmbedURLs(n int) (urls []url.URL) { diff --git a/server/etcdmain/config_test.go b/server/etcdmain/config_test.go index f002102897a..aad33554a4f 100644 --- a/server/etcdmain/config_test.go +++ b/server/etcdmain/config_test.go @@ -24,6 +24,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" "go.etcd.io/etcd/pkg/v3/featuregate" @@ -328,9 +329,8 @@ func TestConfigIsNewCluster(t *testing.T) { for i, tt := range tests { cfg := newConfig() args := []string{"--initial-cluster-state", tests[i].state} - if err := cfg.parse(args); err != nil { - t.Fatalf("#%d: unexpected clusterState.Set error: %v", i, err) - } + err := cfg.parse(args) + require.NoErrorf(t, err, "#%d: unexpected clusterState.Set error: %v", i, err) if g := cfg.ec.IsNewCluster(); g != tt.wIsNew { t.Errorf("#%d: isNewCluster = %v, want %v", i, g, tt.wIsNew) } @@ -458,9 +458,7 @@ func TestParseFeatureGateFlags(t *testing.T) { cfg := newConfig() err := cfg.parse(tc.args) if tc.expectErr { - if err == nil { - t.Fatal("expect parse error") - } + require.Errorf(t, err, "expect parse error") return } if err != nil { diff --git a/server/etcdserver/bootstrap_test.go b/server/etcdserver/bootstrap_test.go index a692a68566c..6e3b6bafe03 100644 --- a/server/etcdserver/bootstrap_test.go +++ b/server/etcdserver/bootstrap_test.go @@ -27,6 +27,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" bolt "go.etcd.io/bbolt" @@ -90,9 +91,7 @@ func TestBootstrapExistingClusterNoWALMaxLearner(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cluster, err := types.NewURLsMap("node0=http://localhost:2380,node1=http://localhost:2381,node2=http://localhost:2382") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoErrorf(t, err, "unexpected error: %v", err) cfg := config.ServerConfig{ Name: "node0", InitialPeerURLsMap: cluster, @@ -104,8 +103,8 @@ func TestBootstrapExistingClusterNoWALMaxLearner(t *testing.T) { if hasError != tt.hasError { t.Errorf("expected error: %v got: %v", tt.hasError, err) } - if hasError && !strings.Contains(err.Error(), tt.expectedError.Error()) { - t.Fatalf("expected error to contain: %q, got: %q", tt.expectedError.Error(), err.Error()) + if hasError { + require.Containsf(t, err.Error(), tt.expectedError.Error(), "expected error to contain: %q, got: %q", tt.expectedError.Error(), err.Error()) } }) } @@ -177,9 +176,7 @@ func TestBootstrapBackend(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { dataDir, err := createDataDir(t) - if err != nil { - t.Fatalf("Failed to create the data dir, unexpected error: %v", err) - } + require.NoErrorf(t, err, "Failed to create the data dir, unexpected error: %v", err) cfg := config.ServerConfig{ Name: "demoNode", @@ -189,9 +186,8 @@ func TestBootstrapBackend(t *testing.T) { } if tt.prepareData != nil { - if err = tt.prepareData(cfg); err != nil { - t.Fatalf("failed to prepare data, unexpected error: %v", err) - } + err = tt.prepareData(cfg) + require.NoErrorf(t, err, "failed to prepare data, unexpected error: %v", err) } haveWAL := wal.Exist(cfg.WALDir()) @@ -207,8 +203,8 @@ func TestBootstrapBackend(t *testing.T) { if hasError != expectedHasError { t.Errorf("expected error: %v got: %v", expectedHasError, err) } - if hasError && !strings.Contains(err.Error(), tt.expectedError.Error()) { - t.Fatalf("expected error to contain: %q, got: %q", tt.expectedError.Error(), err.Error()) + if hasError { + require.Containsf(t, err.Error(), tt.expectedError.Error(), "expected error to contain: %q, got: %q", tt.expectedError.Error(), err.Error()) } if backend.ci.ConsistentIndex() != tt.expectedConsistentIdx { diff --git a/server/etcdserver/corrupt_test.go b/server/etcdserver/corrupt_test.go index 6001542d348..603e0954edf 100644 --- a/server/etcdserver/corrupt_test.go +++ b/server/etcdserver/corrupt_test.go @@ -28,6 +28,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zaptest" @@ -550,27 +551,17 @@ func TestHashKVHandler(t *testing.T) { t.Run(tt.name, func(t *testing.T) { hashReq := &pb.HashKVRequest{Revision: int64(revision)} hashReqBytes, err := json.Marshal(hashReq) - if err != nil { - t.Fatalf("failed to marshal request: %v", err) - } + require.NoErrorf(t, err, "failed to marshal request: %v", err) req, err := http.NewRequest(http.MethodGet, srv.URL+PeerHashKVPath, bytes.NewReader(hashReqBytes)) - if err != nil { - t.Fatalf("failed to create request: %v", err) - } + require.NoErrorf(t, err, "failed to create request: %v", err) req.Header.Set("X-Etcd-Cluster-ID", strconv.FormatUint(uint64(tt.remoteClusterID), 16)) resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("failed to get http response: %v", err) - } + require.NoErrorf(t, err, "failed to get http response: %v", err) body, err := io.ReadAll(resp.Body) resp.Body.Close() - if err != nil { - t.Fatalf("unexpected io.ReadAll error: %v", err) - } - if resp.StatusCode != tt.wcode { - t.Fatalf("#%d: code = %d, want %d", i, resp.StatusCode, tt.wcode) - } + require.NoErrorf(t, err, "unexpected io.ReadAll error: %v", err) + require.Equalf(t, resp.StatusCode, tt.wcode, "#%d: code = %d, want %d", i, resp.StatusCode, tt.wcode) if resp.StatusCode != http.StatusOK { if !strings.Contains(string(body), tt.wKeyWords) { t.Errorf("#%d: body: %s, want body to contain keywords: %s", i, body, tt.wKeyWords) @@ -580,16 +571,10 @@ func TestHashKVHandler(t *testing.T) { hashKVResponse := pb.HashKVResponse{} err = json.Unmarshal(body, &hashKVResponse) - if err != nil { - t.Fatalf("unmarshal response error: %v", err) - } + require.NoErrorf(t, err, "unmarshal response error: %v", err) hashValue, _, err := etcdSrv.KV().HashStorage().HashByRev(int64(revision)) - if err != nil { - t.Fatalf("etcd server hash failed: %v", err) - } - if hashKVResponse.Hash != hashValue.Hash { - t.Fatalf("hash value inconsistent: %d != %d", hashKVResponse.Hash, hashValue) - } + require.NoErrorf(t, err, "etcd server hash failed: %v", err) + require.Equalf(t, hashKVResponse.Hash, hashValue.Hash, "hash value inconsistent: %d != %d", hashKVResponse.Hash, hashValue) }) } } diff --git a/server/etcdserver/server_test.go b/server/etcdserver/server_test.go index 265bce38f56..72ccf01649d 100644 --- a/server/etcdserver/server_test.go +++ b/server/etcdserver/server_test.go @@ -136,13 +136,10 @@ func TestApplyRepeat(t *testing.T) { if err != nil { t.Fatal(err) } - if len(act) == 0 { - t.Fatalf("expected len(act)=0, got %d", len(act)) - } + require.NotEmptyf(t, act, "expected len(act)=0, got %d", len(act)) - if err = <-stopc; err != nil { - t.Fatalf("error on stop (%v)", err) - } + err = <-stopc + require.NoErrorf(t, err, "error on stop (%v)", err) } type uberApplierMock struct{} diff --git a/server/storage/schema/actions_test.go b/server/storage/schema/actions_test.go index d3cb812c11e..a1acb697dec 100644 --- a/server/storage/schema/actions_test.go +++ b/server/storage/schema/actions_test.go @@ -21,6 +21,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" "go.etcd.io/etcd/server/v3/storage/backend" @@ -71,9 +72,7 @@ func TestActionIsReversible(t *testing.T) { be, _ := betesting.NewTmpBackend(t, time.Microsecond, 10) defer be.Close() tx := be.BatchTx() - if tx == nil { - t.Fatal("batch tx is nil") - } + require.NotNilf(t, tx, "batch tx is nil") tx.Lock() defer tx.Unlock() UnsafeCreateMetaBucket(tx) @@ -128,9 +127,7 @@ func TestActionListRevert(t *testing.T) { be, _ := betesting.NewTmpBackend(t, time.Microsecond, 10) defer be.Close() tx := be.BatchTx() - if tx == nil { - t.Fatal("batch tx is nil") - } + require.NotNilf(t, tx, "batch tx is nil") tx.Lock() defer tx.Unlock() diff --git a/server/storage/schema/changes_test.go b/server/storage/schema/changes_test.go index 05b8d49cf44..40cca68aae6 100644 --- a/server/storage/schema/changes_test.go +++ b/server/storage/schema/changes_test.go @@ -18,6 +18,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + betesting "go.etcd.io/etcd/server/v3/storage/backend/testing" ) @@ -39,9 +41,7 @@ func TestUpgradeDowngrade(t *testing.T) { be, _ := betesting.NewTmpBackend(t, time.Microsecond, 10) defer be.Close() tx := be.BatchTx() - if tx == nil { - t.Fatal("batch tx is nil") - } + require.NotNilf(t, tx, "batch tx is nil") tx.Lock() defer tx.Unlock() UnsafeCreateMetaBucket(tx) diff --git a/server/storage/schema/migration_test.go b/server/storage/schema/migration_test.go index 9ebe80dcb05..2b1d563864d 100644 --- a/server/storage/schema/migration_test.go +++ b/server/storage/schema/migration_test.go @@ -22,6 +22,7 @@ import ( "github.com/coreos/go-semver/semver" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" "go.etcd.io/etcd/api/v3/version" @@ -168,9 +169,7 @@ func TestMigrationStepExecute(t *testing.T) { be, _ := betesting.NewTmpBackend(t, time.Microsecond, 10) defer be.Close() tx := be.BatchTx() - if tx == nil { - t.Fatal("batch tx is nil") - } + require.NotNilf(t, tx, "batch tx is nil") tx.Lock() defer tx.Unlock() diff --git a/server/storage/schema/schema_test.go b/server/storage/schema/schema_test.go index 3d9bd1065c3..bb9a4f3ca32 100644 --- a/server/storage/schema/schema_test.go +++ b/server/storage/schema/schema_test.go @@ -20,6 +20,7 @@ import ( "github.com/coreos/go-semver/semver" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap" "go.etcd.io/etcd/api/v3/etcdserverpb" @@ -298,9 +299,7 @@ func setupBackendData(t *testing.T, ver semver.Version, overrideKeys func(tx bac t.Helper() be, tmpPath := betesting.NewTmpBackend(t, time.Microsecond, 10) tx := be.BatchTx() - if tx == nil { - t.Fatal("batch tx is nil") - } + require.NotNilf(t, tx, "batch tx is nil") tx.Lock() UnsafeCreateMetaBucket(tx) if overrideKeys != nil { diff --git a/server/storage/schema/version_test.go b/server/storage/schema/version_test.go index 63442fc24d5..fc774c6a2db 100644 --- a/server/storage/schema/version_test.go +++ b/server/storage/schema/version_test.go @@ -20,6 +20,7 @@ import ( "github.com/coreos/go-semver/semver" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" "go.etcd.io/bbolt" @@ -59,9 +60,7 @@ func TestVersion(t *testing.T) { lg := zaptest.NewLogger(t) be, tmpPath := betesting.NewTmpBackend(t, time.Microsecond, 10) tx := be.BatchTx() - if tx == nil { - t.Fatal("batch tx is nil") - } + require.NotNilf(t, tx, "batch tx is nil") tx.Lock() tx.UnsafeCreateBucket(Meta) UnsafeSetStorageVersion(tx, semver.New(tc.version)) @@ -105,9 +104,7 @@ func TestVersionSnapshot(t *testing.T) { t.Run(tc.version, func(t *testing.T) { be, tmpPath := betesting.NewTmpBackend(t, time.Microsecond, 10) tx := be.BatchTx() - if tx == nil { - t.Fatal("batch tx is nil") - } + require.NotNilf(t, tx, "batch tx is nil") tx.Lock() tx.UnsafeCreateBucket(Meta) UnsafeSetStorageVersion(tx, semver.New(tc.version)) diff --git a/server/storage/wal/repair_test.go b/server/storage/wal/repair_test.go index 585a7c3bd66..8f05b197806 100644 --- a/server/storage/wal/repair_test.go +++ b/server/storage/wal/repair_test.go @@ -15,7 +15,6 @@ package wal import ( - "errors" "fmt" "io" "os" @@ -203,13 +202,9 @@ func TestRepairFailDeleteDir(t *testing.T) { t.Fatal(err) } _, _, _, err = w.ReadAll() - if !errors.Is(err, io.ErrUnexpectedEOF) { - t.Fatalf("err = %v, want error %v", err, io.ErrUnexpectedEOF) - } + require.ErrorIsf(t, err, io.ErrUnexpectedEOF, "err = %v, want error %v", err, io.ErrUnexpectedEOF) w.Close() os.RemoveAll(p) - if Repair(zaptest.NewLogger(t), p) { - t.Fatal("expect 'Repair' fail on unexpected directory deletion") - } + require.Falsef(t, Repair(zaptest.NewLogger(t), p), "expect 'Repair' fail on unexpected directory deletion") } diff --git a/server/storage/wal/wal_bench_test.go b/server/storage/wal/wal_bench_test.go index c8996dc1275..61c8d42fab6 100644 --- a/server/storage/wal/wal_bench_test.go +++ b/server/storage/wal/wal_bench_test.go @@ -17,6 +17,7 @@ package wal import ( "testing" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" "go.etcd.io/raft/v3/raftpb" @@ -38,9 +39,7 @@ func benchmarkWriteEntry(b *testing.B, size int, batch int) { p := b.TempDir() w, err := Create(zaptest.NewLogger(b), p, []byte("somedata")) - if err != nil { - b.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(b, err, "err = %v, want nil", err) data := make([]byte, size) for i := 0; i < size; i++ { data[i] = byte(i) diff --git a/server/storage/wal/wal_test.go b/server/storage/wal/wal_test.go index a9e6dc84910..2ec062999ba 100644 --- a/server/storage/wal/wal_test.go +++ b/server/storage/wal/wal_test.go @@ -48,9 +48,7 @@ func TestNew(t *testing.T) { p := t.TempDir() w, err := Create(zaptest.NewLogger(t), p, []byte("somedata")) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) if g := filepath.Base(w.tail().Name()); g != walName(0, 0) { t.Errorf("name = %+v, want %+v", g, walName(0, 0)) } @@ -74,20 +72,15 @@ func TestNew(t *testing.T) { var wb bytes.Buffer e := newEncoder(&wb, 0, 0) err = e.encode(&walpb.Record{Type: CrcType, Crc: 0}) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) err = e.encode(&walpb.Record{Type: MetadataType, Data: []byte("somedata")}) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) r := &walpb.Record{ Type: SnapshotType, Data: pbutil.MustMarshal(&walpb.Snapshot{}), } - if err = e.encode(r); err != nil { - t.Fatalf("err = %v, want nil", err) - } + err = e.encode(r) + require.NoErrorf(t, err, "err = %v, want nil", err) e.flush() if !bytes.Equal(gd, wb.Bytes()) { t.Errorf("data = %v, want %v", gd, wb.Bytes()) @@ -168,9 +161,7 @@ func TestCreateFailFromPollutedDir(t *testing.T) { os.WriteFile(filepath.Join(p, "test.wal"), []byte("data"), os.ModeTemporary) _, err := Create(zaptest.NewLogger(t), p, []byte("data")) - if !errors.Is(err, os.ErrExist) { - t.Fatalf("expected %v, got %v", os.ErrExist, err) - } + require.ErrorIsf(t, err, os.ErrExist, "expected %v, got %v", os.ErrExist, err) } func TestWalCleanup(t *testing.T) { @@ -182,17 +173,11 @@ func TestWalCleanup(t *testing.T) { logger := zaptest.NewLogger(t) w, err := Create(logger, p, []byte("")) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) w.cleanupWAL(logger) fnames, err := fileutil.ReadDir(testRoot) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } - if len(fnames) != 1 { - t.Fatalf("expected 1 file under %v, got %v", testRoot, len(fnames)) - } + require.NoErrorf(t, err, "err = %v, want nil", err) + require.Lenf(t, fnames, 1, "expected 1 file under %v, got %v", testRoot, len(fnames)) pattern := fmt.Sprintf(`%s.broken\.[\d]{8}\.[\d]{6}\.[\d]{1,6}?`, filepath.Base(p)) match, _ := regexp.MatchString(pattern, fnames[0]) if !match { @@ -210,9 +195,7 @@ func TestCreateFailFromNoSpaceLeft(t *testing.T) { SegmentSizeBytes = math.MaxInt64 _, err := Create(zaptest.NewLogger(t), p, []byte("data")) - if err == nil { // no space left on device - t.Fatalf("expected error 'no space left on device', got nil") - } + require.Errorf(t, err, "expected error 'no space left on device', got nil") // no space left on device } func TestNewForInitedDir(t *testing.T) { @@ -234,9 +217,7 @@ func TestOpenAtIndex(t *testing.T) { f.Close() w, err := Open(zaptest.NewLogger(t), dir, walpb.Snapshot{}) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) if g := filepath.Base(w.tail().Name()); g != walName(0, 0) { t.Errorf("name = %+v, want %+v", g, walName(0, 0)) } @@ -253,9 +234,7 @@ func TestOpenAtIndex(t *testing.T) { f.Close() w, err = Open(zaptest.NewLogger(t), dir, walpb.Snapshot{Index: 5}) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) if g := filepath.Base(w.tail().Name()); g != wname { t.Errorf("name = %+v, want %+v", g, wname) } @@ -414,9 +393,7 @@ func TestSaveWithCut(t *testing.T) { w.Close() neww, err := Open(zaptest.NewLogger(t), p, walpb.Snapshot{}) - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) defer neww.Close() wname := walName(1, index) if g := filepath.Base(neww.tail().Name()); g != wname { @@ -695,9 +672,7 @@ func TestOpenForRead(t *testing.T) { } defer w2.Close() _, _, ents, err := w2.ReadAll() - if err != nil { - t.Fatalf("err = %v, want nil", err) - } + require.NoErrorf(t, err, "err = %v, want nil", err) if g := ents[len(ents)-1].Index; g != 9 { t.Errorf("last index read = %d, want %d", g, 9) } @@ -730,9 +705,7 @@ func TestOpenWithMaxIndex(t *testing.T) { defer w2.Close() _, _, _, err = w2.ReadAll() - if !errors.Is(err, ErrSliceOutOfRange) { - t.Fatalf("err = %v, want ErrSliceOutOfRange", err) - } + require.ErrorIsf(t, err, ErrSliceOutOfRange, "err = %v, want ErrSliceOutOfRange", err) } func TestSaveEmpty(t *testing.T) { @@ -851,9 +824,7 @@ func TestTailWriteNoSlackSpace(t *testing.T) { if rerr != nil { t.Fatal(rerr) } - if len(ents) != 5 { - t.Fatalf("got entries %+v, expected 5 entries", ents) - } + require.Lenf(t, ents, 5, "got entries %+v, expected 5 entries", ents) // write more entries for i := 6; i <= 10; i++ { es := []raftpb.Entry{{Index: uint64(i), Term: 1, Data: []byte{byte(i)}}} @@ -992,9 +963,7 @@ func TestOpenOnTornWrite(t *testing.T) { t.Fatal(rerr) } wEntries := (clobberIdx - 1) + overwriteEntries - if len(ents) != wEntries { - t.Fatalf("expected len(ents) = %d, got %d", wEntries, len(ents)) - } + require.Equalf(t, len(ents), wEntries, "expected len(ents) = %d, got %d", wEntries, len(ents)) } func TestRenameFail(t *testing.T) {