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.

Sign up for GitHub

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 client package #18705

Merged
merged 1 commit into from
Oct 10, 2024
Merged

fix: use require.NoError instead of t.Fatal(err) in client package #18705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 21 additions & 52 deletions client/internal/v2/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.etcd.io/etcd/api/v3/version"
"go.etcd.io/etcd/client/pkg/v3/testutil"
)
Expand Down Expand Up @@ -129,10 +132,7 @@ func TestSimpleHTTPClientDoSuccess(t *testing.T) {
}

resp, body, err := c.Do(context.Background(), &fakeAction{})
if err != nil {
t.Fatalf("incorrect error value: want=nil got=%v", err)
}

require.NoErrorf(t, err, "incorrect error value")
wantCode := http.StatusTeapot
if wantCode != resp.StatusCode {
t.Fatalf("invalid response code: want=%d got=%d", wantCode, resp.StatusCode)
Expand All @@ -151,9 +151,7 @@ func TestSimpleHTTPClientDoError(t *testing.T) {
tr.errchan <- errors.New("fixture")

_, _, err := c.Do(context.Background(), &fakeAction{})
if err == nil {
t.Fatalf("expected non-nil error, got nil")
}
assert.Errorf(t, err, "expected non-nil error, got nil")
}

type nilAction struct{}
Expand Down Expand Up @@ -182,9 +180,7 @@ func TestSimpleHTTPClientDoCancelContext(t *testing.T) {
tr.finishCancel <- struct{}{}

_, _, err := c.Do(context.Background(), &fakeAction{})
if err == nil {
t.Fatalf("expected non-nil error, got nil")
}
assert.Errorf(t, err, "expected non-nil error, got nil")
}

type checkableReadCloser struct {
Expand Down Expand Up @@ -219,9 +215,7 @@ func TestSimpleHTTPClientDoCancelContextResponseBodyClosed(t *testing.T) {
}()

_, _, err := c.Do(ctx, &fakeAction{})
if err == nil {
t.Fatalf("expected non-nil error, got nil")
}
require.Errorf(t, err, "expected non-nil error, got nil")

if !body.closed {
t.Fatalf("expected closed body")
Expand Down Expand Up @@ -309,9 +303,7 @@ func TestSimpleHTTPClientDoHeaderTimeout(t *testing.T) {

select {
case err := <-errc:
if err == nil {
t.Fatalf("expected non-nil error, got nil")
}
require.Errorf(t, err, "expected non-nil error, got nil")
case <-time.After(time.Second):
t.Fatalf("unexpected timeout when waiting for the test to finish")
}
Expand Down Expand Up @@ -796,9 +788,7 @@ func TestHTTPClusterClientSync(t *testing.T) {
rand: rand.New(rand.NewSource(0)),
}
err := hc.SetEndpoints([]string{"http://127.0.0.1:2379"})
if err != nil {
t.Fatalf("unexpected error during setup: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during setup")

want := []string{"http://127.0.0.1:2379"}
got := hc.Endpoints()
Expand Down Expand Up @@ -840,9 +830,7 @@ func TestHTTPClusterClientSyncFail(t *testing.T) {
rand: rand.New(rand.NewSource(0)),
}
err := hc.SetEndpoints([]string{"http://127.0.0.1:2379"})
if err != nil {
t.Fatalf("unexpected error during setup: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during setup")

want := []string{"http://127.0.0.1:2379"}
got := hc.Endpoints()
Expand All @@ -851,9 +839,7 @@ func TestHTTPClusterClientSyncFail(t *testing.T) {
}

err = hc.Sync(context.Background())
if err == nil {
t.Fatalf("got nil error during Sync")
}
require.Errorf(t, err, "got nil error during Sync")

got = hc.Endpoints()
if !reflect.DeepEqual(want, got) {
Expand All @@ -874,9 +860,8 @@ func TestHTTPClusterClientAutoSyncCancelContext(t *testing.T) {
rand: rand.New(rand.NewSource(0)),
}
err := hc.SetEndpoints([]string{"http://127.0.0.1:2379"})
if err != nil {
t.Fatalf("unexpected error during setup: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during setup")

ctx, cancel := context.WithCancel(context.Background())
cancel()

Expand All @@ -896,9 +881,7 @@ func TestHTTPClusterClientAutoSyncFail(t *testing.T) {
rand: rand.New(rand.NewSource(0)),
}
err := hc.SetEndpoints([]string{"http://127.0.0.1:2379"})
if err != nil {
t.Fatalf("unexpected error during setup: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during setup")

err = hc.AutoSync(context.Background(), time.Hour)
if !strings.HasPrefix(err.Error(), ErrClusterUnavailable.Error()) {
Expand All @@ -920,9 +903,7 @@ func TestHTTPClusterClientGetVersion(t *testing.T) {
rand: rand.New(rand.NewSource(0)),
}
err := hc.SetEndpoints([]string{"http://127.0.0.1:4003", "http://127.0.0.1:2379", "http://127.0.0.1:4001", "http://127.0.0.1:4002"})
if err != nil {
t.Fatalf("unexpected error during setup: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during setup")

actual, err := hc.GetVersion(context.Background())
if err != nil {
Expand Down Expand Up @@ -957,16 +938,12 @@ func TestHTTPClusterClientSyncPinEndpoint(t *testing.T) {
rand: rand.New(rand.NewSource(0)),
}
err := hc.SetEndpoints([]string{"http://127.0.0.1:4003", "http://127.0.0.1:2379", "http://127.0.0.1:4001", "http://127.0.0.1:4002"})
if err != nil {
t.Fatalf("unexpected error during setup: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during setup")
pinnedEndpoint := hc.endpoints[hc.pinned]

for i := 0; i < 3; i++ {
err = hc.Sync(context.Background())
if err != nil {
t.Fatalf("#%d: unexpected error during Sync: %#v", i, err)
}
require.NoErrorf(t, err, "#%d: unexpected error during Sync", i)

if g := hc.endpoints[hc.pinned]; g != pinnedEndpoint {
t.Errorf("#%d: pinned endpoint = %v, want %v", i, g, pinnedEndpoint)
Expand Down Expand Up @@ -997,16 +974,12 @@ func TestHTTPClusterClientSyncUnpinEndpoint(t *testing.T) {
rand: rand.New(rand.NewSource(0)),
}
err := hc.SetEndpoints([]string{"http://127.0.0.1:4003", "http://127.0.0.1:2379", "http://127.0.0.1:4001", "http://127.0.0.1:4002"})
if err != nil {
t.Fatalf("unexpected error during setup: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during setup")
wants := []string{"http://127.0.0.1:2379", "http://127.0.0.1:4001", "http://127.0.0.1:4002"}

for i := 0; i < 3; i++ {
err = hc.Sync(context.Background())
if err != nil {
t.Fatalf("#%d: unexpected error during Sync: %#v", i, err)
}
require.NoErrorf(t, err, "#%d: unexpected error during Sync", i)

if g := hc.endpoints[hc.pinned]; g.String() != wants[i] {
t.Errorf("#%d: pinned endpoint = %v, want %v", i, g, wants[i])
Expand Down Expand Up @@ -1047,9 +1020,7 @@ func TestHTTPClusterClientSyncPinLeaderEndpoint(t *testing.T) {

for i, want := range wants {
err := hc.Sync(context.Background())
if err != nil {
t.Fatalf("#%d: unexpected error during Sync: %#v", i, err)
}
require.NoErrorf(t, err, "#%d: unexpected error during Sync", i)

pinned := hc.endpoints[hc.pinned].String()
if pinned != want {
Expand Down Expand Up @@ -1082,9 +1053,7 @@ func TestHTTPClusterClientResetPinRandom(t *testing.T) {
for i := 0; i < round; i++ {
hc := &httpClusterClient{rand: rand.New(rand.NewSource(int64(i)))}
err := hc.SetEndpoints([]string{"http://127.0.0.1:4001", "http://127.0.0.1:4002", "http://127.0.0.1:4003"})
if err != nil {
t.Fatalf("#%d: reset error (%v)", i, err)
}
require.NoErrorf(t, err, "#%d: reset error (%v)", i)
if hc.endpoints[hc.pinned].String() == "http://127.0.0.1:4001" {
pinNum++
}
Expand Down
2 changes: 1 addition & 1 deletion client/internal/v2/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.23
toolchain go1.23.2

require (
github.com/stretchr/testify v1.9.0
go.etcd.io/etcd/api/v3 v3.6.0-alpha.0
go.etcd.io/etcd/client/pkg/v3 v3.6.0-alpha.0
sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6
Expand All @@ -14,7 +15,6 @@ require (
github.com/coreos/go-semver v0.3.1 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/stretchr/testify v1.9.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

Expand Down
6 changes: 3 additions & 3 deletions client/internal/v2/members_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"reflect"
"testing"

"github.com/stretchr/testify/require"

"go.etcd.io/etcd/client/pkg/v3/types"
)

Expand Down Expand Up @@ -312,9 +314,7 @@ func TestMemberCreateRequestMarshal(t *testing.T) {
want := []byte(`{"peerURLs":["http://127.0.0.1:8081","https://127.0.0.1:8080"]}`)

got, err := json.Marshal(&req)
if err != nil {
t.Fatalf("Marshal returned unexpected err=%v", err)
}
require.NoErrorf(t, err, "Marshal returned unexpected err")

if !reflect.DeepEqual(want, got) {
t.Fatalf("Failed to marshal memberCreateRequest: want=%s, got=%s", want, got)
Expand Down
80 changes: 23 additions & 57 deletions client/pkg/fileutil/fileutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,14 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"
)

func TestIsDirWriteable(t *testing.T) {
tmpdir := t.TempDir()
if err := IsDirWriteable(tmpdir); err != nil {
t.Fatalf("unexpected IsDirWriteable error: %v", err)
}
if err := os.Chmod(tmpdir, 0444); err != nil {
t.Fatalf("unexpected os.Chmod error: %v", err)
}
require.NoErrorf(t, IsDirWriteable(tmpdir), "unexpected IsDirWriteable error")
require.NoErrorf(t, os.Chmod(tmpdir, 0444), "unexpected os.Chmod error")
me, err := user.Current()
if err != nil {
// err can be non-nil when cross compiled
Expand All @@ -59,13 +56,9 @@ func TestCreateDirAll(t *testing.T) {
tmpdir := t.TempDir()

tmpdir2 := filepath.Join(tmpdir, "testdir")
if err := CreateDirAll(zaptest.NewLogger(t), tmpdir2); err != nil {
t.Fatal(err)
}
require.NoError(t, CreateDirAll(zaptest.NewLogger(t), tmpdir2))

if err := os.WriteFile(filepath.Join(tmpdir2, "text.txt"), []byte("test text"), PrivateFileMode); err != nil {
t.Fatal(err)
}
require.NoError(t, os.WriteFile(filepath.Join(tmpdir2, "text.txt"), []byte("test text"), PrivateFileMode))

if err := CreateDirAll(zaptest.NewLogger(t), tmpdir2); err == nil || !strings.Contains(err.Error(), "to be empty, got") {
t.Fatalf("unexpected error %v", err)
Expand All @@ -84,9 +77,7 @@ func TestExist(t *testing.T) {
}

f, err := os.CreateTemp(os.TempDir(), "fileutil")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
f.Close()

if g := Exist(f.Name()); !g {
Expand All @@ -107,9 +98,7 @@ func TestDirEmpty(t *testing.T) {
}

file, err := os.CreateTemp(dir, "new_file")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
file.Close()

if DirEmpty(dir) {
Expand All @@ -122,42 +111,31 @@ func TestDirEmpty(t *testing.T) {

func TestZeroToEnd(t *testing.T) {
f, err := os.CreateTemp(os.TempDir(), "fileutil")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
defer os.Remove(f.Name())
defer f.Close()

// Ensure 0 size is a nop so zero-to-end on an empty file won't give EINVAL.
if err = ZeroToEnd(f); err != nil {
t.Fatal(err)
}
require.NoError(t, ZeroToEnd(f))

b := make([]byte, 1024)
for i := range b {
b[i] = 12
}
if _, err = f.Write(b); err != nil {
t.Fatal(err)
}
if _, err = f.Seek(512, io.SeekStart); err != nil {
t.Fatal(err)
}
if err = ZeroToEnd(f); err != nil {
t.Fatal(err)
}
_, err = f.Write(b)
require.NoError(t, err)
_, err = f.Seek(512, io.SeekStart)
require.NoError(t, err)
require.NoError(t, ZeroToEnd(f))
off, serr := f.Seek(0, io.SeekCurrent)
if serr != nil {
t.Fatal(serr)
}
require.NoError(t, serr)
if off != 512 {
t.Fatalf("expected offset 512, got %d", off)
}

b = make([]byte, 512)
if _, err = f.Read(b); err != nil {
t.Fatal(err)
}
_, err = f.Read(b)
require.NoError(t, err)
for i := range b {
if b[i] != 0 {
t.Errorf("expected b[%d] = 0, got %d", i, b[i])
Expand All @@ -170,9 +148,7 @@ func TestDirPermission(t *testing.T) {

tmpdir2 := filepath.Join(tmpdir, "testpermission")
// create a new dir with 0700
if err := CreateDirAll(zaptest.NewLogger(t), tmpdir2); err != nil {
t.Fatal(err)
}
require.NoError(t, CreateDirAll(zaptest.NewLogger(t), tmpdir2))
// check dir permission with mode different than created dir
if err := CheckDirPermission(tmpdir2, 0600); err == nil {
t.Errorf("expected error, got nil")
Expand All @@ -182,14 +158,10 @@ func TestDirPermission(t *testing.T) {
func TestRemoveMatchFile(t *testing.T) {
tmpdir := t.TempDir()
f, err := os.CreateTemp(tmpdir, "tmp")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
f.Close()
f, err = os.CreateTemp(tmpdir, "foo.tmp")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
f.Close()

err = RemoveMatchFile(zaptest.NewLogger(t), tmpdir, func(fileName string) bool {
Expand All @@ -199,17 +171,13 @@ func TestRemoveMatchFile(t *testing.T) {
t.Errorf("expected nil, got error")
}
fnames, err := ReadDir(tmpdir)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
if len(fnames) != 1 {
t.Errorf("expected exist 1 files, got %d", len(fnames))
}

f, err = os.CreateTemp(tmpdir, "tmp")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
f.Close()
err = RemoveMatchFile(zaptest.NewLogger(t), tmpdir, func(fileName string) bool {
os.Remove(filepath.Join(tmpdir, fileName))
Expand All @@ -226,7 +194,5 @@ func TestTouchDirAll(t *testing.T) {
TouchDirAll(nil, tmpdir)
}, "expected panic with nil log")

if err := TouchDirAll(zaptest.NewLogger(t), tmpdir); err != nil {
t.Fatal(err)
}
assert.NoError(t, TouchDirAll(zaptest.NewLogger(t), tmpdir))
}
Loading
Loading