Skip to content

Commit

Permalink
fix: use require.NoError instead of t.Fatal(err) in client package
Browse files Browse the repository at this point in the history
Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
mmorel-35 committed Oct 9, 2024
1 parent 17fb752 commit 3a57f76
Show file tree
Hide file tree
Showing 19 changed files with 162 additions and 375 deletions.
71 changes: 18 additions & 53 deletions client/internal/v2/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"testing"
"time"

"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 +130,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 +149,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")
}
require.Errorf(t, err, "expected non-nil error, got nil")
}

type nilAction struct{}
Expand Down Expand Up @@ -182,9 +178,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")
}
require.Errorf(t, err, "expected non-nil error, got nil")
}

type checkableReadCloser struct {
Expand Down Expand Up @@ -219,9 +213,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 +301,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 +786,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 +828,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 +837,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 +858,7 @@ 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 +878,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,10 +900,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 {
t.Errorf("non-nil error: %#v", err)
Expand Down Expand Up @@ -957,16 +934,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 +970,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: %#v", 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 +1016,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: %#v", i)

pinned := hc.endpoints[hc.pinned].String()
if pinned != want {
Expand Down Expand Up @@ -1082,9 +1049,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
5 changes: 2 additions & 3 deletions client/internal/v2/members_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"reflect"
"testing"

"github.com/stretchr/testify/require"
"go.etcd.io/etcd/client/pkg/v3/types"
)

Expand Down Expand Up @@ -312,9 +313,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
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

0 comments on commit 3a57f76

Please sign in to comment.