Skip to content

Commit

Permalink
Merge pull request #18753 from mmorel-35/tests/testifier/require-error
Browse files Browse the repository at this point in the history
fix: use require.NoError instead of t.Fatal(err) in tests package (part 1)
  • Loading branch information
serathius authored Oct 24, 2024
2 parents 6d2b232 + 4da20aa commit 2f36df8
Show file tree
Hide file tree
Showing 30 changed files with 262 additions and 441 deletions.
7 changes: 4 additions & 3 deletions tests/common/alarm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/tests/v3/framework/config"
"go.etcd.io/etcd/tests/v3/framework/testutils"
Expand Down Expand Up @@ -103,9 +105,8 @@ func TestAlarm(t *testing.T) {
}

// put one more key below quota
if err := cc.Put(ctx, "4th_test", smallbuf, config.PutOptions{}); err != nil {
t.Fatal(err)
}
err = cc.Put(ctx, "4th_test", smallbuf, config.PutOptions{})
require.NoError(t, err)
})
}

Expand Down
15 changes: 6 additions & 9 deletions tests/common/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,14 +378,12 @@ func TestAuthTxn(t *testing.T) {
// keys with 2 suffix are granted to test-user, see Line 399
grantedKeys := []string{"c2", "s2", "f2"}
for _, key := range keys {
if err := cc.Put(ctx, key, "v", config.PutOptions{}); err != nil {
t.Fatal(err)
}
err := cc.Put(ctx, key, "v", config.PutOptions{})
require.NoError(t, err)
}
for _, key := range grantedKeys {
if err := cc.Put(ctx, key, "v", config.PutOptions{}); err != nil {
t.Fatal(err)
}
err := cc.Put(ctx, key, "v", config.PutOptions{})
require.NoError(t, err)
}

require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth")
Expand All @@ -394,9 +392,8 @@ func TestAuthTxn(t *testing.T) {

// grant keys to test-user
for _, key := range grantedKeys {
if _, err := rootAuthClient.RoleGrantPermission(ctx, testRoleName, key, "", clientv3.PermissionType(clientv3.PermReadWrite)); err != nil {
t.Fatal(err)
}
_, err := rootAuthClient.RoleGrantPermission(ctx, testRoleName, key, "", clientv3.PermissionType(clientv3.PermReadWrite))
require.NoError(t, err)
}
for _, req := range reqs {
resp, err := testUserAuthClient.Txn(ctx, req.compare, req.ifSuccess, req.ifFail, config.TxnOptions{
Expand Down
8 changes: 2 additions & 6 deletions tests/common/member_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,19 +249,15 @@ func TestMemberRemove(t *testing.T) {
// It ensures that `MemberRemove` function does not return an "etcdserver: server stopped" error.
func memberToRemove(ctx context.Context, t *testing.T, client intf.Client, clusterSize int) (memberID uint64, clusterID uint64) {
listResp, err := client.MemberList(ctx, false)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)

clusterID = listResp.Header.ClusterId
if clusterSize == 1 {
memberID = listResp.Members[0].ID
} else {
// get status of the specific member that client has connected to
statusResp, err := client.Status(ctx)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)

// choose a member that client has not connected to
for _, m := range listResp.Members {
Expand Down
17 changes: 5 additions & 12 deletions tests/e2e/cluster_downgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,13 @@ func downgradeEnable(t *testing.T, epc *e2e.EtcdProcessCluster, ver *semver.Vers
c := epc.Etcdctl()
testutils.ExecuteWithTimeout(t, 20*time.Second, func() {
err := c.DowngradeEnable(context.TODO(), ver.String())
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
})
}

func stopEtcd(t *testing.T, ep e2e.EtcdProcess) {
if err := ep.Stop(); err != nil {
t.Fatal(err)
}
err := ep.Stop()
require.NoError(t, err)
}

func validateVersion(t *testing.T, cfg *e2e.EtcdProcessClusterConfig, member e2e.EtcdProcess, expect version.Versions) {
Expand Down Expand Up @@ -260,14 +257,10 @@ func leader(t *testing.T, epc *e2e.EtcdProcessCluster) e2e.EtcdProcess {
Endpoints: endpoints,
DialTimeout: 3 * time.Second,
})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
defer cli.Close()
resp, err := cli.Status(ctx, endpoints[0])
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
if resp.Header.GetMemberId() == resp.Leader {
return epc.Procs[i]
}
Expand Down
17 changes: 5 additions & 12 deletions tests/e2e/corrupt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,12 @@ func corruptTest(cx ctlCtx) {
cx.t.Log("connecting clientv3...")
eps := cx.epc.EndpointsGRPC()
cli1, err := clientv3.New(clientv3.Config{Endpoints: []string{eps[1]}, DialTimeout: 3 * time.Second})
if err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, err)
defer cli1.Close()

sresp, err := cli1.Status(context.TODO(), eps[0])
cx.t.Logf("checked status sresp:%v err:%v", sresp, err)
if err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, err)
id0 := sresp.Header.GetMemberId()

cx.t.Log("stopping etcd[0]...")
Expand All @@ -86,16 +82,13 @@ func corruptTest(cx ctlCtx) {
// corrupting first member by modifying backend offline.
fp := datadir.ToBackendFileName(cx.epc.Procs[0].Config().DataDirPath)
cx.t.Logf("corrupting backend: %v", fp)
if err = cx.corruptFunc(fp); err != nil {
cx.t.Fatal(err)
}
err = cx.corruptFunc(fp)
require.NoError(cx.t, err)

cx.t.Log("restarting etcd[0]")
ep := cx.epc.Procs[0]
proc, err := e2e.SpawnCmd(append([]string{ep.Config().ExecPath}, ep.Config().Args...), cx.envMap)
if err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, err)
defer proc.Stop()

cx.t.Log("waiting for etcd[0] failure...")
Expand Down
61 changes: 24 additions & 37 deletions tests/e2e/ctl_v3_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,33 +67,26 @@ func ctlV3PutFailPerm(cx ctlCtx, key, val string) error {
}

func authSetupTestUser(cx ctlCtx) {
if err := ctlV3User(cx, []string{"add", "test-user", "--interactive=false"}, "User test-user created", []string{"pass"}); err != nil {
cx.t.Fatal(err)
}
if err := e2e.SpawnWithExpectWithEnv(append(cx.PrefixArgs(), "role", "add", "test-role"), cx.envMap, expect.ExpectedResponse{Value: "Role test-role created"}); err != nil {
cx.t.Fatal(err)
}
if err := ctlV3User(cx, []string{"grant-role", "test-user", "test-role"}, "Role test-role is granted to user test-user", nil); err != nil {
cx.t.Fatal(err)
}
err := ctlV3User(cx, []string{"add", "test-user", "--interactive=false"}, "User test-user created", []string{"pass"})
require.NoError(cx.t, err)
err = e2e.SpawnWithExpectWithEnv(append(cx.PrefixArgs(), "role", "add", "test-role"), cx.envMap, expect.ExpectedResponse{Value: "Role test-role created"})
require.NoError(cx.t, err)
err = ctlV3User(cx, []string{"grant-role", "test-user", "test-role"}, "Role test-role is granted to user test-user", nil)
require.NoError(cx.t, err)
cmd := append(cx.PrefixArgs(), "role", "grant-permission", "test-role", "readwrite", "foo")
if err := e2e.SpawnWithExpectWithEnv(cmd, cx.envMap, expect.ExpectedResponse{Value: "Role test-role updated"}); err != nil {
cx.t.Fatal(err)
}
err = e2e.SpawnWithExpectWithEnv(cmd, cx.envMap, expect.ExpectedResponse{Value: "Role test-role updated"})
require.NoError(cx.t, err)
}

func authTestMemberUpdate(cx ctlCtx) {
if err := authEnable(cx); err != nil {
cx.t.Fatal(err)
}
err := authEnable(cx)
require.NoError(cx.t, err)

cx.user, cx.pass = "root", "root"
authSetupTestUser(cx)

mr, err := getMemberList(cx, false)
if err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, err)

// ordinary user cannot update a member
cx.user, cx.pass = "test-user", "pass"
Expand All @@ -105,31 +98,25 @@ func authTestMemberUpdate(cx ctlCtx) {

// root can update a member
cx.user, cx.pass = "root", "root"
if err = ctlV3MemberUpdate(cx, memberID, peerURL); err != nil {
cx.t.Fatal(err)
}
err = ctlV3MemberUpdate(cx, memberID, peerURL)
require.NoError(cx.t, err)
}

func authTestCertCN(cx ctlCtx) {
if err := authEnable(cx); err != nil {
cx.t.Fatal(err)
}
err := authEnable(cx)
require.NoError(cx.t, err)

cx.user, cx.pass = "root", "root"
if err := ctlV3User(cx, []string{"add", "example.com", "--interactive=false"}, "User example.com created", []string{""}); err != nil {
cx.t.Fatal(err)
}
if err := e2e.SpawnWithExpectWithEnv(append(cx.PrefixArgs(), "role", "add", "test-role"), cx.envMap, expect.ExpectedResponse{Value: "Role test-role created"}); err != nil {
cx.t.Fatal(err)
}
if err := ctlV3User(cx, []string{"grant-role", "example.com", "test-role"}, "Role test-role is granted to user example.com", nil); err != nil {
cx.t.Fatal(err)
}
err = ctlV3User(cx, []string{"add", "example.com", "--interactive=false"}, "User example.com created", []string{""})
require.NoError(cx.t, err)
err = e2e.SpawnWithExpectWithEnv(append(cx.PrefixArgs(), "role", "add", "test-role"), cx.envMap, expect.ExpectedResponse{Value: "Role test-role created"})
require.NoError(cx.t, err)
err = ctlV3User(cx, []string{"grant-role", "example.com", "test-role"}, "Role test-role is granted to user example.com", nil)
require.NoError(cx.t, err)

// grant a new key
if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "hoo", "", false}); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "hoo", "", false})
require.NoError(cx.t, err)

// try a granted key
cx.user, cx.pass = "", ""
Expand All @@ -139,7 +126,7 @@ func authTestCertCN(cx ctlCtx) {

// try a non-granted key
cx.user, cx.pass = "", ""
err := ctlV3PutFailPerm(cx, "baz", "bar")
err = ctlV3PutFailPerm(cx, "baz", "bar")
require.ErrorContains(cx.t, err, "permission denied")
}

Expand Down
15 changes: 6 additions & 9 deletions tests/e2e/ctl_v3_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,8 @@ func testIssue6361(t *testing.T) {
t.Log("Writing some keys...")
kvs := []kv{{"foo1", "val1"}, {"foo2", "val2"}, {"foo3", "val3"}}
for i := range kvs {
if err = e2e.SpawnWithExpect(append(prefixArgs, "put", kvs[i].key, kvs[i].val), expect.ExpectedResponse{Value: "OK"}); err != nil {
t.Fatal(err)
}
err = e2e.SpawnWithExpect(append(prefixArgs, "put", kvs[i].key, kvs[i].val), expect.ExpectedResponse{Value: "OK"})
require.NoError(t, err)
}

fpath := filepath.Join(t.TempDir(), "test.snapshot")
Expand Down Expand Up @@ -244,9 +243,8 @@ func testIssue6361(t *testing.T) {

t.Log("Ensuring the restored member has the correct data...")
for i := range kvs {
if err = e2e.SpawnWithExpect(append(prefixArgs, "get", kvs[i].key), expect.ExpectedResponse{Value: kvs[i].val}); err != nil {
t.Fatal(err)
}
err = e2e.SpawnWithExpect(append(prefixArgs, "get", kvs[i].key), expect.ExpectedResponse{Value: kvs[i].val})
require.NoError(t, err)
}

t.Log("Adding new member into the cluster")
Expand Down Expand Up @@ -281,9 +279,8 @@ func testIssue6361(t *testing.T) {

t.Log("Ensuring added member has data from incoming snapshot...")
for i := range kvs {
if err = e2e.SpawnWithExpect(append(prefixArgs, "get", kvs[i].key), expect.ExpectedResponse{Value: kvs[i].val}); err != nil {
t.Fatal(err)
}
err = e2e.SpawnWithExpect(append(prefixArgs, "get", kvs[i].key), expect.ExpectedResponse{Value: kvs[i].val})
require.NoError(t, err)
}

t.Log("Stopping the second member")
Expand Down
25 changes: 10 additions & 15 deletions tests/e2e/etcd_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ func TestEtcdMultiPeer(t *testing.T) {
}

for _, p := range procs {
if err := e2e.WaitReadyExpectProc(context.TODO(), p, e2e.EtcdServerReadyLines); err != nil {
t.Fatal(err)
}
err := e2e.WaitReadyExpectProc(context.TODO(), p, e2e.EtcdServerReadyLines)
require.NoError(t, err)
}
}

Expand Down Expand Up @@ -247,9 +246,8 @@ func TestEtcdPeerCNAuth(t *testing.T) {
} else {
expect = []string{"remote error: tls: bad certificate"}
}
if err := e2e.WaitReadyExpectProc(context.TODO(), p, expect); err != nil {
t.Fatal(err)
}
err := e2e.WaitReadyExpectProc(context.TODO(), p, expect)
require.NoError(t, err)
}
}

Expand Down Expand Up @@ -337,9 +335,8 @@ func TestEtcdPeerMultiCNAuth(t *testing.T) {
} else {
expect = []string{"remote error: tls: bad certificate"}
}
if err := e2e.WaitReadyExpectProc(context.TODO(), p, expect); err != nil {
t.Fatal(err)
}
err := e2e.WaitReadyExpectProc(context.TODO(), p, expect)
require.NoError(t, err)
}
}

Expand Down Expand Up @@ -413,9 +410,8 @@ func TestEtcdPeerNameAuth(t *testing.T) {
} else {
expect = []string{"client certificate authentication failed"}
}
if err := e2e.WaitReadyExpectProc(context.TODO(), p, expect); err != nil {
t.Fatal(err)
}
err := e2e.WaitReadyExpectProc(context.TODO(), p, expect)
require.NoError(t, err)
}
}

Expand Down Expand Up @@ -522,9 +518,8 @@ func TestEtcdPeerLocalAddr(t *testing.T) {
} else {
expect = []string{"x509: certificate is valid for 127.0.0.1, not "}
}
if err := e2e.WaitReadyExpectProc(context.TODO(), p, expect); err != nil {
t.Fatal(err)
}
err := e2e.WaitReadyExpectProc(context.TODO(), p, expect)
require.NoError(t, err)
}
}

Expand Down
7 changes: 4 additions & 3 deletions tests/integration/clientv3/lease/lease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/concurrency"
Expand Down Expand Up @@ -546,9 +548,8 @@ func TestLeaseTimeToLive(t *testing.T) {
kv := clus.RandClient()
keys := []string{"foo1", "foo2"}
for i := range keys {
if _, err = kv.Put(context.TODO(), keys[i], "bar", clientv3.WithLease(resp.ID)); err != nil {
t.Fatal(err)
}
_, err = kv.Put(context.TODO(), keys[i], "bar", clientv3.WithLease(resp.ID))
require.NoError(t, err)
}

// linearized read to ensure Puts propagated to server backing lapi
Expand Down
Loading

0 comments on commit 2f36df8

Please sign in to comment.