Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use require.NoError instead of t.Fatal(err) in tests package (part 1) #18753

Merged
merged 1 commit into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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