From cf4c0e4afa81744c58d38828f17f0f7f076732ee Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Sat, 9 Nov 2024 21:20:52 +0100 Subject: [PATCH] fix: use testify instead of t.Fatal in tests/common package (part 2) Signed-off-by: Matthieu MOREL --- server/etcdserver/server.go | 2 +- tests/common/alarm_test.go | 2 +- tests/common/auth_test.go | 67 ++++++++++++++++---------------- tests/common/compact_test.go | 20 ++-------- tests/common/defrag_test.go | 15 +++---- tests/common/endpoint_test.go | 12 ++---- tests/common/kv_test.go | 38 ++++++------------ tests/common/member_test.go | 33 ++++------------ tests/common/role_test.go | 39 ++++++++----------- tests/common/status_test.go | 12 ++---- tests/common/txn_test.go | 8 +--- tests/common/user_test.go | 41 ++++++------------- tests/common/wait_leader_test.go | 21 +++++----- tests/common/watch_test.go | 4 +- 14 files changed, 108 insertions(+), 206 deletions(-) diff --git a/server/etcdserver/server.go b/server/etcdserver/server.go index 446606431a4..2677e929f98 100644 --- a/server/etcdserver/server.go +++ b/server/etcdserver/server.go @@ -1197,7 +1197,7 @@ func (s *EtcdServer) ForceSnapshot() { } func (s *EtcdServer) snapshotIfNeededAndCompactRaftLog(ep *etcdProgress) { - //TODO: Remove disk snapshot in v3.7 + // TODO: Remove disk snapshot in v3.7 shouldSnapshotToDisk := s.shouldSnapshotToDisk(ep) shouldSnapshotToMemory := s.shouldSnapshotToMemory(ep) if !shouldSnapshotToDisk && !shouldSnapshotToMemory { diff --git a/tests/common/alarm_test.go b/tests/common/alarm_test.go index 17a677b4003..67d83ddf5be 100644 --- a/tests/common/alarm_test.go +++ b/tests/common/alarm_test.go @@ -110,7 +110,7 @@ func TestAlarmlistOnMemberRestart(t *testing.T) { testutils.ExecuteUntil(ctx, t, func() { for i := 0; i < 6; i++ { _, err := cc.AlarmList(ctx) - require.NoErrorf(t, err, "Unexpected error") + require.NoError(t, err) } clus.Members()[0].Stop() diff --git a/tests/common/auth_test.go b/tests/common/auth_test.go index c9544b9076d..06e5c8f3865 100644 --- a/tests/common/auth_test.go +++ b/tests/common/auth_test.go @@ -79,9 +79,9 @@ func TestAuthDisable(t *testing.T) { // confirm put succeeded resp, err := cc.Get(ctx, "hoo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "hoo" || string(resp.Kvs[0].Value) != "bar" { - t.Fatalf("want key value pair 'hoo', 'bar' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'hoo', 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "hoo", string(resp.Kvs[0].Key), "want key value pair 'hoo', 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "bar", string(resp.Kvs[0].Value), "want key value pair 'hoo', 'bar' but got %+v", resp.Kvs) }) } @@ -173,9 +173,9 @@ func TestAuthRoleUpdate(t *testing.T) { // confirm put succeeded resp, err := testUserAuthClient.Get(ctx, "hoo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "hoo" || string(resp.Kvs[0].Value) != "bar" { - t.Fatalf("want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "hoo", string(resp.Kvs[0].Key), "want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "bar", string(resp.Kvs[0].Value), "want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) // revoke the newly granted key _, err = rootAuthClient.RoleRevokePermission(ctx, testRoleName, "hoo", "") require.NoError(t, err) @@ -184,9 +184,9 @@ func TestAuthRoleUpdate(t *testing.T) { // confirm a key still granted can be accessed resp, err = testUserAuthClient.Get(ctx, "foo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "foo" || string(resp.Kvs[0].Value) != "bar" { - t.Fatalf("want key value pair 'foo' 'bar' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "foo", string(resp.Kvs[0].Key), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "bar", string(resp.Kvs[0].Value), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) }) } @@ -209,9 +209,9 @@ func TestAuthUserDeleteDuringOps(t *testing.T) { // confirm put succeeded resp, err := testUserAuthClient.Get(ctx, "foo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "foo" || string(resp.Kvs[0].Value) != "bar" { - t.Fatalf("want key value pair 'foo' 'bar' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "foo", string(resp.Kvs[0].Key), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "bar", string(resp.Kvs[0].Value), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) // delete the user _, err = rootAuthClient.UserDelete(ctx, testUserName) require.NoError(t, err) @@ -240,9 +240,9 @@ func TestAuthRoleRevokeDuringOps(t *testing.T) { // confirm put succeeded resp, err := testUserAuthClient.Get(ctx, "foo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "foo" || string(resp.Kvs[0].Value) != "bar" { - t.Fatalf("want key value pair 'foo' 'bar' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "foo", string(resp.Kvs[0].Key), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "bar", string(resp.Kvs[0].Value), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) // create a new role _, err = rootAuthClient.RoleAdd(ctx, "test-role2") require.NoError(t, err) @@ -258,9 +258,9 @@ func TestAuthRoleRevokeDuringOps(t *testing.T) { // confirm put succeeded resp, err = testUserAuthClient.Get(ctx, "hoo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "hoo" || string(resp.Kvs[0].Value) != "bar" { - t.Fatalf("want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "hoo", string(resp.Kvs[0].Key), "want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "bar", string(resp.Kvs[0].Value), "want key value pair 'hoo' 'bar' but got %+v", resp.Kvs) // revoke a role from the user _, err = rootAuthClient.UserRevokeRole(ctx, testUserName, testRoleName) require.NoError(t, err) @@ -272,9 +272,9 @@ func TestAuthRoleRevokeDuringOps(t *testing.T) { // confirm put succeeded resp, err = testUserAuthClient.Get(ctx, "hoo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "hoo" || string(resp.Kvs[0].Value) != "bar2" { - t.Fatalf("want key value pair 'hoo' 'bar2' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'hoo' 'bar2' but got %+v", resp.Kvs) + require.Equalf(t, "hoo", string(resp.Kvs[0].Key), "want key value pair 'hoo' 'bar2' but got %+v", resp.Kvs) + require.Equalf(t, "bar2", string(resp.Kvs[0].Value), "want key value pair 'hoo' 'bar2' but got %+v", resp.Kvs) }) } @@ -296,9 +296,9 @@ func TestAuthWriteKey(t *testing.T) { require.NoError(t, rootAuthClient.Put(ctx, "foo", "bar", config.PutOptions{})) resp, err := rootAuthClient.Get(ctx, "foo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "foo" || string(resp.Kvs[0].Value) != "bar" { - t.Fatalf("want key value pair 'foo' 'bar' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "foo", string(resp.Kvs[0].Key), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) + require.Equalf(t, "bar", string(resp.Kvs[0].Value), "want key value pair 'foo' 'bar' but got %+v", resp.Kvs) // try invalid user _, err = clus.Client(WithAuth("a", "b")) require.ErrorContains(t, err, AuthenticationFailed) @@ -308,9 +308,9 @@ func TestAuthWriteKey(t *testing.T) { // confirm put succeeded resp, err = testUserAuthClient.Get(ctx, "foo", config.GetOptions{}) require.NoError(t, err) - if len(resp.Kvs) != 1 || string(resp.Kvs[0].Key) != "foo" || string(resp.Kvs[0].Value) != "bar2" { - t.Fatalf("want key value pair 'foo' 'bar2' but got %+v", resp.Kvs) - } + require.Lenf(t, resp.Kvs, 1, "want key value pair 'foo' 'bar2' but got %+v", resp.Kvs) + require.Equalf(t, "foo", string(resp.Kvs[0].Key), "want key value pair 'foo' 'bar2' but got %+v", resp.Kvs) + require.Equalf(t, "bar2", string(resp.Kvs[0].Value), "want key value pair 'foo' 'bar2' but got %+v", resp.Kvs) // try bad password _, err = clus.Client(WithAuth(testUserName, "badpass")) @@ -403,7 +403,7 @@ func TestAuthTxn(t *testing.T) { Interactive: true, }) if req.expectError { - require.Contains(t, err.Error(), req.expectResults[0]) + require.ErrorContains(t, err, req.expectResults[0]) } else { require.NoError(t, err) require.Equal(t, req.expectResults, getRespValues(resp)) @@ -468,9 +468,9 @@ func TestAuthLeaseKeepAlive(t *testing.T) { gresp, err := rootAuthClient.Get(ctx, "key", config.GetOptions{}) require.NoError(t, err) - if len(gresp.Kvs) != 1 || string(gresp.Kvs[0].Key) != "key" || string(gresp.Kvs[0].Value) != "value" { - t.Fatalf("want kv pair ('key', 'value') but got %v", gresp.Kvs) - } + require.Lenf(t, gresp.Kvs, 1, "want kv pair ('key', 'value') but got %v", gresp.Kvs) + require.Equalf(t, "key", string(gresp.Kvs[0].Key), "want kv pair ('key', 'value') but got %v", gresp.Kvs) + require.Equalf(t, "value", string(gresp.Kvs[0].Value), "want kv pair ('key', 'value') but got %v", gresp.Kvs) }) } @@ -561,9 +561,8 @@ func TestAuthLeaseGrantLeases(t *testing.T) { leaseID := resp.ID lresp, err := rootAuthClient.Leases(ctx) require.NoError(t, err) - if len(lresp.Leases) != 1 || lresp.Leases[0].ID != leaseID { - t.Fatalf("want %v leaseID but got %v leases", leaseID, lresp.Leases) - } + require.Lenf(t, lresp.Leases, 1, "want %v leaseID but got %v leases", leaseID, lresp.Leases) + require.Equalf(t, lresp.Leases[0].ID, leaseID, "want %v leaseID but got %v leases", leaseID, lresp.Leases) }) }) } diff --git a/tests/common/compact_test.go b/tests/common/compact_test.go index a1ded59b0b2..412c4621550 100644 --- a/tests/common/compact_test.go +++ b/tests/common/compact_test.go @@ -16,7 +16,6 @@ package common import ( "context" - "strings" "testing" "time" @@ -52,8 +51,7 @@ func TestCompact(t *testing.T) { testutils.ExecuteUntil(ctx, t, func() { kvs := []testutils.KV{{Key: "key", Val: "val1"}, {Key: "key", Val: "val2"}, {Key: "key", Val: "val3"}} for i := range kvs { - err := cc.Put(ctx, kvs[i].Key, kvs[i].Val, config.PutOptions{}) - require.NoErrorf(t, err, "compactTest #%d: put kv error", i) + require.NoErrorf(t, cc.Put(ctx, kvs[i].Key, kvs[i].Val, config.PutOptions{}), "compactTest #%d: put kv error", i) } get, err := cc.Get(ctx, "key", config.GetOptions{Revision: 3}) require.NoErrorf(t, err, "compactTest: Get kv by revision error") @@ -65,22 +63,10 @@ func TestCompact(t *testing.T) { require.NoErrorf(t, err, "compactTest: Compact error") _, err = cc.Get(ctx, "key", config.GetOptions{Revision: 3}) - if err != nil { - if !strings.Contains(err.Error(), "required revision has been compacted") { - t.Fatalf("compactTest: Get compact key error (%v)", err) - } - } else { - t.Fatalf("expected '...has been compacted' error, got ") - } + require.ErrorContainsf(t, err, "required revision has been compacted", "compactTest: Get compact key error (%v)", err) _, err = cc.Compact(ctx, 2, tc.options) - if err != nil { - if !strings.Contains(err.Error(), "required revision has been compacted") { - t.Fatal(err) - } - } else { - t.Fatalf("expected '...has been compacted' error, got ") - } + require.ErrorContains(t, err, "required revision has been compacted") }) }) } diff --git a/tests/common/defrag_test.go b/tests/common/defrag_test.go index 4509ad51d97..d0af0e68c4a 100644 --- a/tests/common/defrag_test.go +++ b/tests/common/defrag_test.go @@ -19,6 +19,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "go.etcd.io/etcd/tests/v3/framework/config" "go.etcd.io/etcd/tests/v3/framework/testutils" ) @@ -34,17 +36,10 @@ func TestDefragOnline(t *testing.T) { defer clus.Close() kvs := []testutils.KV{{Key: "key", Val: "val1"}, {Key: "key", Val: "val2"}, {Key: "key", Val: "val3"}} for i := range kvs { - if err := cc.Put(ctx, kvs[i].Key, kvs[i].Val, config.PutOptions{}); err != nil { - t.Fatalf("compactTest #%d: put kv error (%v)", i, err) - } + require.NoErrorf(t, cc.Put(ctx, kvs[i].Key, kvs[i].Val, config.PutOptions{}), "compactTest #%d: put kv error", i) } _, err := cc.Compact(ctx, 4, config.CompactOption{Physical: true, Timeout: 10 * time.Second}) - if err != nil { - t.Fatalf("defrag_test: compact with revision error (%v)", err) - } - - if err = cc.Defragment(ctx, options); err != nil { - t.Fatalf("defrag_test: defrag error (%v)", err) - } + require.NoErrorf(t, err, "defrag_test: compact with revision error (%v)", err) + require.NoErrorf(t, cc.Defragment(ctx, options), "defrag_test: defrag error (%v)", err) }) } diff --git a/tests/common/endpoint_test.go b/tests/common/endpoint_test.go index eeef011f3cc..570b59d7f17 100644 --- a/tests/common/endpoint_test.go +++ b/tests/common/endpoint_test.go @@ -35,9 +35,7 @@ func TestEndpointStatus(t *testing.T) { cc := testutils.MustClient(clus.Client()) testutils.ExecuteUntil(ctx, t, func() { _, err := cc.Status(ctx) - if err != nil { - t.Fatalf("get endpoint status error: %v", err) - } + require.NoErrorf(t, err, "get endpoint status error: %v", err) }) } @@ -53,9 +51,7 @@ func TestEndpointHashKV(t *testing.T) { for i := 0; i < 10; i++ { key := fmt.Sprintf("key-%d", i) value := fmt.Sprintf("value-%d", i) - if err := cc.Put(ctx, key, value, config.PutOptions{}); err != nil { - t.Fatalf("count not put key %q, err: %s", key, err) - } + require.NoErrorf(t, cc.Put(ctx, key, value, config.PutOptions{}), "count not put key %q", key) } t.Log("Check all members' Hash and HashRevision") @@ -82,8 +78,6 @@ func TestEndpointHealth(t *testing.T) { defer clus.Close() cc := testutils.MustClient(clus.Client()) testutils.ExecuteUntil(ctx, t, func() { - if err := cc.Health(ctx); err != nil { - t.Fatalf("get endpoint health error: %v", err) - } + require.NoErrorf(t, cc.Health(ctx), "get endpoint health error") }) } diff --git a/tests/common/kv_test.go b/tests/common/kv_test.go index 988a15f17cd..593ea1e19a0 100644 --- a/tests/common/kv_test.go +++ b/tests/common/kv_test.go @@ -40,22 +40,12 @@ func TestKVPut(t *testing.T) { testutils.ExecuteUntil(ctx, t, func() { key, value := "foo", "bar" - if err := cc.Put(ctx, key, value, config.PutOptions{}); err != nil { - t.Fatalf("count not put key %q, err: %s", key, err) - } + require.NoErrorf(t, cc.Put(ctx, key, value, config.PutOptions{}), "count not put key %q", key) resp, err := cc.Get(ctx, key, config.GetOptions{}) - if err != nil { - t.Fatalf("count not get key %q, err: %s", key, err) - } - if len(resp.Kvs) != 1 { - t.Errorf("Unexpected length of response, got %d", len(resp.Kvs)) - } - if string(resp.Kvs[0].Key) != key { - t.Errorf("Unexpected key, want %q, got %q", key, resp.Kvs[0].Key) - } - if string(resp.Kvs[0].Value) != value { - t.Errorf("Unexpected value, want %q, got %q", value, resp.Kvs[0].Value) - } + require.NoErrorf(t, err, "count not get key %q, err: %s", key, err) + assert.Lenf(t, resp.Kvs, 1, "Unexpected length of response, got %d", len(resp.Kvs)) + assert.Equalf(t, string(resp.Kvs[0].Key), key, "Unexpected key, want %q, got %q", key, resp.Kvs[0].Key) + assert.Equalf(t, string(resp.Kvs[0].Value), value, "Unexpected value, want %q, got %q", value, resp.Kvs[0].Value) }) }) } @@ -80,9 +70,7 @@ func TestKVGet(t *testing.T) { ) for i := range kvs { - if err := cc.Put(ctx, kvs[i], "bar", config.PutOptions{}); err != nil { - t.Fatalf("count not put key %q, err: %s", kvs[i], err) - } + require.NoErrorf(t, cc.Put(ctx, kvs[i], "bar", config.PutOptions{}), "count not put key %q", kvs[i]) } tests := []struct { begin string @@ -110,9 +98,7 @@ func TestKVGet(t *testing.T) { } for _, tt := range tests { resp, err := cc.Get(ctx, tt.begin, tt.options) - if err != nil { - t.Fatalf("count not get key %q, err: %s", tt.begin, err) - } + require.NoErrorf(t, err, "count not get key %q, err: %s", tt.begin, err) kvs := testutils.KeysFromGetResponse(resp) assert.Equal(t, tt.wkv, kvs) } @@ -180,8 +166,7 @@ func TestKVDelete(t *testing.T) { } for _, tt := range tests { for i := range kvs { - err := cc.Put(ctx, kvs[i], "bar", config.PutOptions{}) - require.NoErrorf(t, err, "count not put key %q", kvs[i]) + require.NoErrorf(t, cc.Put(ctx, kvs[i], "bar", config.PutOptions{}), "count not put key %q", kvs[i]) } del, err := cc.Delete(ctx, tt.deleteKey, tt.options) require.NoErrorf(t, err, "count not get key %q, err", tt.deleteKey) @@ -228,9 +213,10 @@ func TestKVGetNoQuorum(t *testing.T) { testutils.ExecuteUntil(ctx, t, func() { key := "foo" _, err := cc.Get(ctx, key, tc.options) - gotError := err != nil - if gotError != tc.wantError { - t.Fatalf("Unexpected result, wantError: %v, gotErr: %v, err: %s", tc.wantError, gotError, err) + if tc.wantError { + require.Error(t, err) + } else { + require.NoError(t, err) } }) }) diff --git a/tests/common/member_test.go b/tests/common/member_test.go index 5433e0560cb..7d03eaa2221 100644 --- a/tests/common/member_test.go +++ b/tests/common/member_test.go @@ -44,10 +44,7 @@ func TestMemberList(t *testing.T) { resp, err := cc.MemberList(ctx, false) require.NoErrorf(t, err, "could not get member list") expectNum := len(clus.Members()) - gotNum := len(resp.Members) - if expectNum != gotNum { - t.Fatalf("number of members not equal, expect: %d, got: %d", expectNum, gotNum) - } + require.Lenf(t, resp.Members, expectNum, "unexpected number of members") assert.Eventually(t, func() bool { resp, err := cc.MemberList(ctx, false) if err != nil { @@ -138,15 +135,9 @@ func TestMemberAdd(t *testing.T) { require.ErrorContains(t, err, "etcdserver: unhealthy cluster") } else { require.NoErrorf(t, err, "MemberAdd failed") - if addResp.Member == nil { - t.Fatalf("MemberAdd failed, expected: member != nil, got: member == nil") - } - if addResp.Member.ID == 0 { - t.Fatalf("MemberAdd failed, expected: ID != 0, got: ID == 0") - } - if len(addResp.Member.PeerURLs) == 0 { - t.Fatalf("MemberAdd failed, expected: non-empty PeerURLs, got: empty PeerURLs") - } + require.NotNilf(t, addResp.Member, "MemberAdd failed, expected: member != nil, got: member == nil") + require.NotZerof(t, addResp.Member.ID, "MemberAdd failed, expected: ID != 0, got: ID == 0") + require.NotEmptyf(t, addResp.Member.PeerURLs, "MemberAdd failed, expected: non-empty PeerURLs, got: empty PeerURLs") } }) }) @@ -224,16 +215,10 @@ func TestMemberRemove(t *testing.T) { require.NoErrorf(t, err, "MemberRemove failed") t.Logf("removeResp.Members:%v", removeResp.Members) - if removeResp.Header.ClusterId != clusterID { - t.Fatalf("MemberRemove failed, expected ClusterID: %d, got: %d", clusterID, removeResp.Header.ClusterId) - } - if len(removeResp.Members) != c.ClusterSize-1 { - t.Fatalf("MemberRemove failed, expected length of members: %d, got: %d", c.ClusterSize-1, len(removeResp.Members)) - } + require.Equalf(t, removeResp.Header.ClusterId, clusterID, "MemberRemove failed, expected ClusterID: %d, got: %d", clusterID, removeResp.Header.ClusterId) + require.Lenf(t, removeResp.Members, c.ClusterSize-1, "MemberRemove failed, expected length of members: %d, got: %d", c.ClusterSize-1, len(removeResp.Members)) for _, m := range removeResp.Members { - if m.ID == memberID { - t.Fatalf("MemberRemove failed, member(id=%d) is still in cluster", memberID) - } + require.NotEqualf(t, m.ID, memberID, "MemberRemove failed, member(id=%d) is still in cluster", memberID) } }) }) @@ -264,9 +249,7 @@ func memberToRemove(ctx context.Context, t *testing.T, client intf.Client, clust break } } - if memberID == 0 { - t.Fatalf("memberToRemove failed. listResp:%v, statusResp:%v", listResp, statusResp) - } + require.NotZerof(t, memberID, "memberToRemove failed. listResp:%v, statusResp:%v", listResp, statusResp) } return memberID, clusterID } diff --git a/tests/common/role_test.go b/tests/common/role_test.go index e196902fd1b..7dda25d303b 100644 --- a/tests/common/role_test.go +++ b/tests/common/role_test.go @@ -16,7 +16,6 @@ package common import ( "context" - "strings" "testing" "time" @@ -40,7 +39,7 @@ func TestRoleAdd_Simple(t *testing.T) { testutils.ExecuteUntil(ctx, t, func() { _, err := cc.RoleAdd(ctx, "root") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) }) }) } @@ -55,15 +54,11 @@ func TestRoleAdd_Error(t *testing.T) { cc := testutils.MustClient(clus.Client()) testutils.ExecuteUntil(ctx, t, func() { _, err := cc.RoleAdd(ctx, "test-role") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) _, err = cc.RoleAdd(ctx, "test-role") - if err == nil || !strings.Contains(err.Error(), rpctypes.ErrRoleAlreadyExist.Error()) { - t.Fatalf("want (%v) error, but got (%v)", rpctypes.ErrRoleAlreadyExist, err) - } + require.ErrorContainsf(t, err, rpctypes.ErrRoleAlreadyExist.Error(), "want (%v) error, but got (%v)", rpctypes.ErrRoleAlreadyExist, err) _, err = cc.RoleAdd(ctx, "") - if err == nil || !strings.Contains(err.Error(), rpctypes.ErrRoleEmpty.Error()) { - t.Fatalf("want (%v) error, but got (%v)", rpctypes.ErrRoleEmpty, err) - } + require.ErrorContainsf(t, err, rpctypes.ErrRoleEmpty.Error(), "want (%v) error, but got (%v)", rpctypes.ErrRoleEmpty, err) }) } @@ -76,15 +71,15 @@ func TestRootRole(t *testing.T) { cc := testutils.MustClient(clus.Client()) testutils.ExecuteUntil(ctx, t, func() { _, err := cc.RoleAdd(ctx, "root") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) resp, err := cc.RoleGet(ctx, "root") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) t.Logf("get role resp %+v", resp) // granting to root should be refused by server and a no-op _, err = cc.RoleGrantPermission(ctx, "root", "foo", "", clientv3.PermissionType(clientv3.PermReadWrite)) - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) resp2, err := cc.RoleGet(ctx, "root") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) t.Logf("get role resp %+v", resp2) }) } @@ -98,19 +93,17 @@ func TestRoleGrantRevokePermission(t *testing.T) { cc := testutils.MustClient(clus.Client()) testutils.ExecuteUntil(ctx, t, func() { _, err := cc.RoleAdd(ctx, "role1") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) _, err = cc.RoleGrantPermission(ctx, "role1", "bar", "", clientv3.PermissionType(clientv3.PermRead)) - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) _, err = cc.RoleGrantPermission(ctx, "role1", "bar", "", clientv3.PermissionType(clientv3.PermWrite)) - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) _, err = cc.RoleGrantPermission(ctx, "role1", "bar", "foo", clientv3.PermissionType(clientv3.PermReadWrite)) - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) _, err = cc.RoleRevokePermission(ctx, "role1", "foo", "") - if err == nil || !strings.Contains(err.Error(), rpctypes.ErrPermissionNotGranted.Error()) { - t.Fatalf("want error (%v), but got (%v)", rpctypes.ErrPermissionNotGranted, err) - } + require.ErrorContainsf(t, err, rpctypes.ErrPermissionNotGranted.Error(), "want error (%v), but got (%v)", rpctypes.ErrPermissionNotGranted, err) _, err = cc.RoleRevokePermission(ctx, "role1", "bar", "foo") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) }) } @@ -123,8 +116,8 @@ func TestRoleDelete(t *testing.T) { cc := testutils.MustClient(clus.Client()) testutils.ExecuteUntil(ctx, t, func() { _, err := cc.RoleAdd(ctx, "role1") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) _, err = cc.RoleDelete(ctx, "role1") - require.NoErrorf(t, err, "want no error, but got") + require.NoError(t, err) }) } diff --git a/tests/common/status_test.go b/tests/common/status_test.go index b3e317530be..519bb07e435 100644 --- a/tests/common/status_test.go +++ b/tests/common/status_test.go @@ -39,19 +39,13 @@ func TestStatus(t *testing.T) { testutils.ExecuteUntil(ctx, t, func() { rs, err := cc.Status(ctx) require.NoErrorf(t, err, "could not get status") - if len(rs) != tc.config.ClusterSize { - t.Fatalf("wrong number of status responses. expected:%d, got:%d ", tc.config.ClusterSize, len(rs)) - } + require.Lenf(t, rs, tc.config.ClusterSize, "wrong number of status responses. expected:%d, got:%d ", tc.config.ClusterSize, len(rs)) memberIDs := make(map[uint64]struct{}) for _, r := range rs { - if r == nil { - t.Fatalf("status response is nil") - } + require.NotNilf(t, r, "status response is nil") memberIDs[r.Header.MemberId] = struct{}{} } - if len(rs) != len(memberIDs) { - t.Fatalf("found duplicated members") - } + require.Lenf(t, rs, len(memberIDs), "found duplicated members") }) }) } diff --git a/tests/common/txn_test.go b/tests/common/txn_test.go index 975248fc80e..b9c1b688c2f 100644 --- a/tests/common/txn_test.go +++ b/tests/common/txn_test.go @@ -73,9 +73,7 @@ func TestTxnSucc(t *testing.T) { resp, err := cc.Txn(ctx, req.compare, req.ifSuccess, req.ifFail, config.TxnOptions{ Interactive: true, }) - if err != nil { - t.Errorf("Txn returned error: %s", err) - } + require.NoErrorf(t, err, "Txn returned error: %s", err) assert.Equal(t, req.expectResults, getRespValues(resp)) } }) @@ -113,9 +111,7 @@ func TestTxnFail(t *testing.T) { resp, err := cc.Txn(ctx, req.compare, req.ifSuccess, req.ifFail, config.TxnOptions{ Interactive: true, }) - if err != nil { - t.Errorf("Txn returned error: %s", err) - } + require.NoErrorf(t, err, "Txn returned error: %s", err) assert.Equal(t, req.expectResults, getRespValues(resp)) } }) diff --git a/tests/common/user_test.go b/tests/common/user_test.go index d36118f5d0e..7f189b000b7 100644 --- a/tests/common/user_test.go +++ b/tests/common/user_test.go @@ -75,18 +75,10 @@ func TestUserAdd_Simple(t *testing.T) { testutils.ExecuteUntil(ctx, t, func() { resp, err := cc.UserAdd(ctx, nc.username, nc.password, config.UserAddOptions{NoPassword: nc.noPassword}) if nc.expectedError != "" { - if err != nil { - assert.ErrorContains(t, err, nc.expectedError) - return - } - - t.Fatalf("expected user creation to fail") - } - - require.NoErrorf(t, err, "expected no error") - - if resp == nil { - t.Fatalf("unexpected nil response to successful user creation") + require.ErrorContainsf(t, err, nc.expectedError, "expected user creation to fail") + } else { + require.NoError(t, err) + require.NotNilf(t, resp, "unexpected nil response to successful user creation") } }) }) @@ -112,8 +104,7 @@ func TestUserAdd_DuplicateUserNotAllowed(t *testing.T) { require.NoErrorf(t, err, "first user creation should succeed") _, err = cc.UserAdd(ctx, user, password, config.UserAddOptions{}) - require.Errorf(t, err, "duplicate user creation should fail") - assert.Contains(t, err.Error(), "etcdserver: user name already exists") + assert.ErrorContains(t, err, "etcdserver: user name already exists") }) }) } @@ -133,9 +124,7 @@ func TestUserList(t *testing.T) { // No Users Yet resp, err := cc.UserList(ctx) require.NoErrorf(t, err, "user listing should succeed") - if len(resp.Users) != 0 { - t.Fatalf("expected no pre-existing users, found: %q", resp.Users) - } + require.Emptyf(t, resp.Users, "expected no pre-existing users, found: %q", resp.Users) user := "barb" password := "rhubarb" @@ -146,9 +135,7 @@ func TestUserList(t *testing.T) { // Users! resp, err = cc.UserList(ctx) require.NoErrorf(t, err, "user listing should succeed") - if len(resp.Users) != 1 { - t.Fatalf("expected one user, found: %q", resp.Users) - } + require.Lenf(t, resp.Users, 1, "expected one user, found: %q", resp.Users) }) }) } @@ -173,9 +160,7 @@ func TestUserDelete(t *testing.T) { resp, err := cc.UserList(ctx) require.NoErrorf(t, err, "user listing should succeed") - if len(resp.Users) != 1 { - t.Fatalf("expected one user, found: %q", resp.Users) - } + require.Lenf(t, resp.Users, 1, "expected one user, found: %q", resp.Users) // Delete barb, sorry barb! _, err = cc.UserDelete(ctx, user) @@ -183,14 +168,11 @@ func TestUserDelete(t *testing.T) { resp, err = cc.UserList(ctx) require.NoErrorf(t, err, "user listing should succeed") - if len(resp.Users) != 0 { - t.Fatalf("expected no users after deletion, found: %q", resp.Users) - } + require.Emptyf(t, resp.Users, "expected no users after deletion, found: %q", resp.Users) // Try to delete barb again _, err = cc.UserDelete(ctx, user) - require.Errorf(t, err, "deleting a non-existent user should fail") - assert.Contains(t, err.Error(), "user name not found") + assert.ErrorContains(t, err, "user name not found") }) }) } @@ -218,8 +200,7 @@ func TestUserChangePassword(t *testing.T) { require.NoErrorf(t, err, "user password change should succeed") err = cc.UserChangePass(ctx, "non-existent-user", newPassword) - require.Errorf(t, err, "user password change for non-existent user should fail") - assert.Contains(t, err.Error(), "user name not found") + assert.ErrorContains(t, err, "user name not found") }) }) } diff --git a/tests/common/wait_leader_test.go b/tests/common/wait_leader_test.go index faa1f716cd7..5f9a8ba0572 100644 --- a/tests/common/wait_leader_test.go +++ b/tests/common/wait_leader_test.go @@ -19,6 +19,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "go.etcd.io/etcd/tests/v3/framework/config" ) @@ -33,9 +35,8 @@ func TestWaitLeader(t *testing.T) { defer clus.Close() leader := clus.WaitLeader(t) - if leader < 0 || leader >= len(clus.Members()) { - t.Fatalf("WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", leader, len(clus.Members())) - } + require.GreaterOrEqualf(t, leader, 0, "WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", leader, len(clus.Members())) + require.Lessf(t, leader, len(clus.Members()), "WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", leader, len(clus.Members())) }) } } @@ -61,19 +62,15 @@ func TestWaitLeader_MemberStop(t *testing.T) { defer clus.Close() lead1 := clus.WaitLeader(t) - if lead1 < 0 || lead1 >= len(clus.Members()) { - t.Fatalf("WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", lead1, len(clus.Members())) - } + require.GreaterOrEqualf(t, lead1, 0, "WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", lead1, len(clus.Members())) + require.Lessf(t, lead1, len(clus.Members()), "WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", lead1, len(clus.Members())) clus.Members()[lead1].Stop() lead2 := clus.WaitLeader(t) - if lead2 < 0 || lead2 >= len(clus.Members()) { - t.Fatalf("WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", lead2, len(clus.Members())) - } + require.GreaterOrEqualf(t, lead2, 0, "WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", lead2, len(clus.Members())) + require.Lessf(t, lead2, len(clus.Members()), "WaitLeader failed for the leader index (%d) is out of range, cluster member count: %d", lead2, len(clus.Members())) - if lead1 == lead2 { - t.Fatalf("WaitLeader failed for the leader(index=%d) did not change as expected after a member stopped", lead1) - } + require.NotEqualf(t, lead1, lead2, "WaitLeader failed for the leader(index=%d) did not change as expected after a member stopped", lead1) }) } } diff --git a/tests/common/watch_test.go b/tests/common/watch_test.go index 67187d59698..fe6d6b0c8f2 100644 --- a/tests/common/watch_test.go +++ b/tests/common/watch_test.go @@ -73,9 +73,7 @@ func TestWatch(t *testing.T) { for _, tt := range tests { wCtx, wCancel := context.WithCancel(ctx) wch := cc.Watch(wCtx, tt.watchKey, tt.opts) - if wch == nil { - t.Fatalf("failed to watch %s", tt.watchKey) - } + require.NotNilf(t, wch, "failed to watch %s", tt.watchKey) for j := range tt.puts { err := cc.Put(ctx, tt.puts[j].Key, tt.puts[j].Val, config.PutOptions{})