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 testify instead of t.Fatal or t.Error in client package (part 1) #18967

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+90 −191
Open

fix: use testify instead of t.Fatal or t.Error in client package (part 1) #18967

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
64 changes: 16 additions & 48 deletions client/internal/v2/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,10 @@ func TestSimpleHTTPClientDoSuccess(t *testing.T) {
resp, body, err := c.Do(context.Background(), &fakeAction{})
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)
}
require.Equalf(t, wantCode, resp.StatusCode, "invalid response code: want=%d got=%d", wantCode, resp.StatusCode)

wantBody := []byte("foo")
if !reflect.DeepEqual(wantBody, body) {
t.Fatalf("invalid response body: want=%q got=%q", wantBody, body)
}
require.Truef(t, reflect.DeepEqual(wantBody, body), "invalid response body: want=%q got=%q", wantBody, body)
}

func TestSimpleHTTPClientDoError(t *testing.T) {
Expand All @@ -167,9 +163,7 @@ func TestSimpleHTTPClientDoNilRequest(t *testing.T) {
tr.errchan <- errors.New("fixture")

_, _, err := c.Do(context.Background(), &nilAction{})
if !errors.Is(err, ErrNoRequest) {
t.Fatalf("expected non-nil error, got nil")
}
require.ErrorIsf(t, err, ErrNoRequest, "expected non-nil error, got nil")
}

func TestSimpleHTTPClientDoCancelContext(t *testing.T) {
Expand Down Expand Up @@ -217,9 +211,7 @@ func TestSimpleHTTPClientDoCancelContextResponseBodyClosed(t *testing.T) {
_, _, err := c.Do(ctx, &fakeAction{})
require.Errorf(t, err, "expected non-nil error, got nil")

if !body.closed {
t.Fatalf("expected closed body")
}
require.Truef(t, body.closed, "expected closed body")
}

type blockingBody struct {
Expand Down Expand Up @@ -250,13 +242,9 @@ func TestSimpleHTTPClientDoCancelContextResponseBodyClosedWithBlockingBody(t *te
}()

_, _, err := c.Do(ctx, &fakeAction{})
if !errors.Is(err, context.Canceled) {
t.Fatalf("expected %+v, got %+v", context.Canceled, err)
}
require.ErrorIsf(t, err, context.Canceled, "expected %+v, got %+v", context.Canceled, err)

if !body.closed {
t.Fatalf("expected closed body")
}
require.Truef(t, body.closed, "expected closed body")
}

func TestSimpleHTTPClientDoCancelContextWaitForRoundTrip(t *testing.T) {
Expand Down Expand Up @@ -558,9 +546,7 @@ func TestRedirectedHTTPAction(t *testing.T) {
}
got := act.HTTPRequest(url.URL{Scheme: "http", Host: "baz.example.com", Path: "/pang"})

if !reflect.DeepEqual(want, got) {
t.Fatalf("HTTPRequest is %#v, want %#v", want, got)
}
require.Truef(t, reflect.DeepEqual(want, got), "HTTPRequest is %#v, want %#v", want, got)
}

func TestRedirectFollowingHTTPClient(t *testing.T) {
Expand Down Expand Up @@ -793,32 +779,22 @@ func TestHTTPClusterClientSync(t *testing.T) {

want := []string{"http://127.0.0.1:2379"}
got := hc.Endpoints()
if !reflect.DeepEqual(want, got) {
t.Fatalf("incorrect endpoints: want=%#v got=%#v", want, got)
}
require.Truef(t, reflect.DeepEqual(want, got), "incorrect endpoints: want=%#v got=%#v", want, got)

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

want = []string{"http://127.0.0.1:2379", "http://127.0.0.1:4001", "http://127.0.0.1:4002", "http://127.0.0.1:4003"}
got = hc.Endpoints()
sort.Strings(got)
if !reflect.DeepEqual(want, got) {
t.Fatalf("incorrect endpoints post-Sync: want=%#v got=%#v", want, got)
}
require.Truef(t, reflect.DeepEqual(want, got), "incorrect endpoints post-Sync: want=%#v got=%#v", want, got)

err = hc.SetEndpoints([]string{"http://127.0.0.1:4009"})
if err != nil {
t.Fatalf("unexpected error during reset: %#v", err)
}
require.NoErrorf(t, err, "unexpected error during reset: %#v", err)

want = []string{"http://127.0.0.1:4009"}
got = hc.Endpoints()
if !reflect.DeepEqual(want, got) {
t.Fatalf("incorrect endpoints post-reset: want=%#v got=%#v", want, got)
}
require.Truef(t, reflect.DeepEqual(want, got), "incorrect endpoints post-reset: want=%#v got=%#v", want, got)
}

func TestHTTPClusterClientSyncFail(t *testing.T) {
Expand All @@ -835,17 +811,13 @@ func TestHTTPClusterClientSyncFail(t *testing.T) {

want := []string{"http://127.0.0.1:2379"}
got := hc.Endpoints()
if !reflect.DeepEqual(want, got) {
t.Fatalf("incorrect endpoints: want=%#v got=%#v", want, got)
}
require.Truef(t, reflect.DeepEqual(want, got), "incorrect endpoints: want=%#v got=%#v", want, got)

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

got = hc.Endpoints()
if !reflect.DeepEqual(want, got) {
t.Fatalf("incorrect endpoints after failed Sync: want=%#v got=%#v", want, got)
}
require.Truef(t, reflect.DeepEqual(want, got), "incorrect endpoints after failed Sync: want=%#v got=%#v", want, got)
}

func TestHTTPClusterClientAutoSyncCancelContext(t *testing.T) {
Expand All @@ -867,9 +839,7 @@ func TestHTTPClusterClientAutoSyncCancelContext(t *testing.T) {
cancel()

err = hc.AutoSync(ctx, time.Hour)
if !errors.Is(err, context.Canceled) {
t.Fatalf("incorrect error value: want=%v got=%v", context.Canceled, err)
}
require.ErrorIsf(t, err, context.Canceled, "incorrect error value: want=%v got=%v", context.Canceled, err)
}

func TestHTTPClusterClientAutoSyncFail(t *testing.T) {
Expand All @@ -885,9 +855,7 @@ func TestHTTPClusterClientAutoSyncFail(t *testing.T) {
require.NoErrorf(t, err, "unexpected error during setup")

err = hc.AutoSync(context.Background(), time.Hour)
if !strings.HasPrefix(err.Error(), ErrClusterUnavailable.Error()) {
t.Fatalf("incorrect error value: want=%v got=%v", ErrClusterUnavailable, err)
}
require.Truef(t, strings.HasPrefix(err.Error(), ErrClusterUnavailable.Error()), "incorrect error value: want=%v got=%v", ErrClusterUnavailable, err)
}

func TestHTTPClusterClientGetVersion(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions client/internal/v2/keys_bench_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"reflect"
"strings"
"testing"

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

func createTestNode(size int) *Node {
Expand Down Expand Up @@ -54,9 +56,7 @@ func benchmarkResponseUnmarshalling(b *testing.B, children, size int) {
header.Add("X-Etcd-Index", "123456")
response := createTestResponse(children, size)
body, err := json.Marshal(response)
if err != nil {
b.Fatal(err)
}
require.NoError(b, err)

b.ResetTimer()
newResponse := new(Response)
Expand Down
8 changes: 2 additions & 6 deletions client/internal/v2/members_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,7 @@ func TestV2MembersURL(t *testing.T) {
Path: "/pants/v2/members",
}

if !reflect.DeepEqual(want, got) {
t.Fatalf("v2MembersURL got %#v, want %#v", got, want)
}
require.Truef(t, reflect.DeepEqual(want, got), "v2MembersURL got %#v, want %#v", got, want)
}

func TestMemberUnmarshal(t *testing.T) {
Expand Down Expand Up @@ -316,9 +314,7 @@ func TestMemberCreateRequestMarshal(t *testing.T) {
got, err := json.Marshal(&req)
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)
}
require.Truef(t, reflect.DeepEqual(want, got), "Failed to marshal memberCreateRequest: want=%s, got=%s", want, got)
}

func TestHTTPMembersAPIAddSuccess(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion client/pkg/fileutil/dir_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (

const (
// PrivateDirMode grants owner to make/remove files inside the directory.
PrivateDirMode = 0777
PrivateDirMode = 0o777
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line seems to come from the gofumpt change, not directly to testify.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed but as golangci-lint is not executed on windows platform it will never be detected in this file. Shall it be keot or reverted here ?

)

// OpenDir opens a directory in windows with write access for syncing.
Expand Down
24 changes: 6 additions & 18 deletions client/pkg/fileutil/fileutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ func TestIsDirWriteable(t *testing.T) {
// Chmod is not supported under windows.
t.Skipf("running as a superuser or in windows")
}
if err := IsDirWriteable(tmpdir); err == nil {
t.Fatalf("expected IsDirWriteable to error")
}
require.Errorf(t, IsDirWriteable(tmpdir), "expected IsDirWriteable to error")
}

func TestCreateDirAll(t *testing.T) {
Expand All @@ -72,9 +70,7 @@ func TestExist(t *testing.T) {
t.Skip(err)
}
defer os.RemoveAll(fdir)
if !Exist(fdir) {
t.Fatalf("expected Exist true, got %v", Exist(fdir))
}
require.Truef(t, Exist(fdir), "expected Exist true, got %v", Exist(fdir))

f, err := os.CreateTemp(os.TempDir(), "fileutil")
require.NoError(t, err)
Expand All @@ -93,20 +89,14 @@ func TestExist(t *testing.T) {
func TestDirEmpty(t *testing.T) {
dir := t.TempDir()

if !DirEmpty(dir) {
t.Fatalf("expected DirEmpty true, got %v", DirEmpty(dir))
}
require.Truef(t, DirEmpty(dir), "expected DirEmpty true, got %v", DirEmpty(dir))

file, err := os.CreateTemp(dir, "new_file")
require.NoError(t, err)
file.Close()

if DirEmpty(dir) {
t.Fatalf("expected DirEmpty false, got %v", DirEmpty(dir))
}
if DirEmpty(file.Name()) {
t.Fatalf("expected DirEmpty false, got %v", DirEmpty(file.Name()))
}
require.Falsef(t, DirEmpty(dir), "expected DirEmpty false, got %v", DirEmpty(dir))
require.Falsef(t, DirEmpty(file.Name()), "expected DirEmpty false, got %v", DirEmpty(file.Name()))
}

func TestZeroToEnd(t *testing.T) {
Expand All @@ -129,9 +119,7 @@ func TestZeroToEnd(t *testing.T) {
require.NoError(t, ZeroToEnd(f))
off, serr := f.Seek(0, io.SeekCurrent)
require.NoError(t, serr)
if off != 512 {
t.Fatalf("expected offset 512, got %d", off)
}
require.Equalf(t, int64(512), off, "expected offset 512, got %d", off)

b = make([]byte, 512)
_, err = f.Read(b)
Expand Down
8 changes: 2 additions & 6 deletions client/pkg/fileutil/read_dir_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ func TestReadDir(t *testing.T) {
fs, err := ReadDir(tmpdir)
require.NoErrorf(t, err, "error calling ReadDir")
wfs := []string{"abc", "def", "ghi", "xyz"}
if !reflect.DeepEqual(fs, wfs) {
t.Fatalf("ReadDir: got %v, want %v", fs, wfs)
}
require.Truef(t, reflect.DeepEqual(fs, wfs), "ReadDir: got %v, want %v", fs, wfs)

files = []string{"def.wal", "abc.wal", "xyz.wal", "ghi.wal"}
for _, f := range files {
Expand All @@ -45,9 +43,7 @@ func TestReadDir(t *testing.T) {
fs, err = ReadDir(tmpdir, WithExt(".wal"))
require.NoErrorf(t, err, "error calling ReadDir")
wfs = []string{"abc.wal", "def.wal", "ghi.wal", "xyz.wal"}
if !reflect.DeepEqual(fs, wfs) {
t.Fatalf("ReadDir: got %v, want %v", fs, wfs)
}
require.Truef(t, reflect.DeepEqual(fs, wfs), "ReadDir: got %v, want %v", fs, wfs)
}

func writeFunc(t *testing.T, path string) {
Expand Down
14 changes: 5 additions & 9 deletions client/pkg/srv/srv_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/require"

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

Expand Down Expand Up @@ -152,12 +154,8 @@ func TestSRVGetCluster(t *testing.T) {
urls := testutil.MustNewURLs(t, tt.urls)
str, err := GetCluster(tt.scheme, tt.service, name, "example.com", urls)

if hasErr(err) != tt.werr {
t.Fatalf("%d: err = %#v, want = %#v", i, err, tt.werr)
}
if strings.Join(str, ",") != tt.expected {
t.Errorf("#%d: cluster = %s, want %s", i, str, tt.expected)
}
require.Equalf(t, hasErr(err), tt.werr, "%d: err = %#v, want = %#v", i, err, tt.werr)
require.Equalf(t, tt.expected, strings.Join(str, ","), "#%d: cluster = %s, want %s", i, str, tt.expected)
}
}

Expand Down Expand Up @@ -255,9 +253,7 @@ func TestSRVDiscover(t *testing.T) {

srvs, err := GetClient("etcd-client", "example.com", "")

if hasErr(err) != tt.werr {
t.Fatalf("%d: err = %#v, want = %#v", i, err, tt.werr)
}
require.Equalf(t, hasErr(err), tt.werr, "%d: err = %#v, want = %#v", i, err, tt.werr)
if srvs == nil {
if len(tt.expected) > 0 {
t.Errorf("#%d: srvs = nil, want non-nil", i)
Expand Down
6 changes: 3 additions & 3 deletions client/pkg/tlsutil/cipher_suites_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ package tlsutil
import (
"crypto/tls"
"testing"

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

func TestGetCipherSuite_not_existing(t *testing.T) {
_, ok := GetCipherSuite("not_existing")
if ok {
t.Fatal("Expected not ok")
}
require.Falsef(t, ok, "Expected not ok")
}

func CipherSuiteExpectedToExist(tb testing.TB, cipher string, expectedID uint16) {
Expand Down
5 changes: 2 additions & 3 deletions client/pkg/transport/keepalive_listener_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ func TestNewKeepAliveListener(t *testing.T) {
go http.Get("http://" + ln.Addr().String())
conn, err := ln.Accept()
require.NoErrorf(t, err, "unexpected Accept error")
if _, ok := conn.(*keepAliveConn); !ok {
t.Fatalf("Unexpected conn type: %T, wanted *keepAliveConn", conn)
}
_, ok := conn.(*keepAliveConn)
require.Truef(t, ok, "Unexpected conn type: %T, wanted *keepAliveConn", conn)
conn.Close()
ln.Close()

Expand Down
Loading
Loading