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

Signed-off-by: Matthieu MOREL <[email protected]>
etcd-io · Dec 11, 2024 · 0a71106 · 0a71106
1 parent 39d9cd7
commit 0a71106
Show file tree

Hide file tree

Showing 15 changed files with 103 additions and 207 deletions.
diff --git a/pkg/adt/interval_tree_test.go b/pkg/adt/interval_tree_test.go
@@ -18,6 +18,8 @@ import (
 	"math/rand"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 // TestIntervalTreeInsert tests interval tree insertion.
@@ -53,9 +55,7 @@ func TestIntervalTreeInsert(t *testing.T) {
 
 	tr := ivt.(*intervalTree)
 	visits := tr.visitLevel()
-	if !reflect.DeepEqual(expected, visits) {
-		t.Fatalf("level order expected %v, got %v", expected, visits)
-	}
+	require.Truef(t, reflect.DeepEqual(expected, visits), "level order expected %v, got %v", expected, visits)
 }
 
 // TestIntervalTreeSelfBalanced ensures range tree is self-balanced after inserting ranges to the tree.
@@ -107,13 +107,9 @@ func TestIntervalTreeSelfBalanced(t *testing.T) {
 
 	tr := ivt.(*intervalTree)
 	visits := tr.visitLevel()
-	if !reflect.DeepEqual(expected, visits) {
-		t.Fatalf("level order expected %v, got %v", expected, visits)
-	}
+	require.Truef(t, reflect.DeepEqual(expected, visits), "level order expected %v, got %v", expected, visits)
 
-	if visits[len(visits)-1].depth != 3 {
-		t.Fatalf("expected self-balanced tree with last level 3, but last level got %d", visits[len(visits)-1].depth)
-	}
+	require.Equalf(t, 3, visits[len(visits)-1].depth, "expected self-balanced tree with last level 3, but last level got %d", visits[len(visits)-1].depth)
 }
 
 // TestIntervalTreeDelete ensures delete operation maintains red-black tree properties.
@@ -214,15 +210,11 @@ func TestIntervalTreeDelete(t *testing.T) {
 		{root: NewInt64Interval(953, 954), color: red, left: newInt64EmptyInterval(), right: newInt64EmptyInterval(), depth: 4},
 	}
 	visitsBeforeDelete := tr.visitLevel()
-	if !reflect.DeepEqual(expectedBeforeDelete, visitsBeforeDelete) {
-		t.Fatalf("level order after insertion expected %v, got %v", expectedBeforeDelete, visitsBeforeDelete)
-	}
+	require.Truef(t, reflect.DeepEqual(expectedBeforeDelete, visitsBeforeDelete), "level order after insertion expected %v, got %v", expectedBeforeDelete, visitsBeforeDelete)
 
 	// delete the node "514"
 	range514 := NewInt64Interval(514, 515)
-	if deleted := tr.Delete(NewInt64Interval(514, 515)); !deleted {
-		t.Fatalf("range %v not deleted", range514)
-	}
+	require.Truef(t, tr.Delete(NewInt64Interval(514, 515)), "range %v not deleted", range514)
 
 	expectedAfterDelete514 := []visitedInterval{
 		{root: NewInt64Interval(510, 511), color: black, left: NewInt64Interval(82, 83), right: NewInt64Interval(830, 831), depth: 0},
@@ -246,15 +238,11 @@ func TestIntervalTreeDelete(t *testing.T) {
 		{root: NewInt64Interval(953, 954), color: red, left: newInt64EmptyInterval(), right: newInt64EmptyInterval(), depth: 4},
 	}
 	visitsAfterDelete514 := tr.visitLevel()
-	if !reflect.DeepEqual(expectedAfterDelete514, visitsAfterDelete514) {
-		t.Fatalf("level order after deleting '514' expected %v, got %v", expectedAfterDelete514, visitsAfterDelete514)
-	}
+	require.Truef(t, reflect.DeepEqual(expectedAfterDelete514, visitsAfterDelete514), "level order after deleting '514' expected %v, got %v", expectedAfterDelete514, visitsAfterDelete514)
 
 	// delete the node "11"
 	range11 := NewInt64Interval(11, 12)
-	if deleted := tr.Delete(NewInt64Interval(11, 12)); !deleted {
-		t.Fatalf("range %v not deleted", range11)
-	}
+	require.Truef(t, tr.Delete(NewInt64Interval(11, 12)), "range %v not deleted", range11)
 
 	expectedAfterDelete11 := []visitedInterval{
 		{root: NewInt64Interval(510, 511), color: black, left: NewInt64Interval(383, 384), right: NewInt64Interval(830, 831), depth: 0},
@@ -277,9 +265,7 @@ func TestIntervalTreeDelete(t *testing.T) {
 		{root: NewInt64Interval(953, 954), color: red, left: newInt64EmptyInterval(), right: newInt64EmptyInterval(), depth: 4},
 	}
 	visitsAfterDelete11 := tr.visitLevel()
-	if !reflect.DeepEqual(expectedAfterDelete11, visitsAfterDelete11) {
-		t.Fatalf("level order after deleting '11' expected %v, got %v", expectedAfterDelete11, visitsAfterDelete11)
-	}
+	require.Truef(t, reflect.DeepEqual(expectedAfterDelete11, visitsAfterDelete11), "level order after deleting '11' expected %v, got %v", expectedAfterDelete11, visitsAfterDelete11)
 }
 
 func TestIntervalTreeIntersects(t *testing.T) {
@@ -323,9 +309,7 @@ func TestIntervalTreeStab(t *testing.T) {
 	ivt.Insert(NewStringInterval("0", "3"), 0)
 
 	tr := ivt.(*intervalTree)
-	if tr.root.max.Compare(StringComparable("8")) != 0 {
-		t.Fatalf("wrong root max got %v, expected 8", tr.root.max)
-	}
+	require.Equalf(t, 0, tr.root.max.Compare(StringComparable("8")), "wrong root max got %v, expected 8", tr.root.max)
 	if x := len(ivt.Stab(NewStringPoint("0"))); x != 3 {
 		t.Errorf("got %d, expected 3", x)
 	}
@@ -381,12 +365,8 @@ func TestIntervalTreeRandom(t *testing.T) {
 	for ab := range ivs {
 		for xy := range ivs {
 			v := xy.x + int64(rand.Intn(int(xy.y-xy.x)))
-			if slen := len(ivt.Stab(NewInt64Point(v))); slen == 0 {
-				t.Fatalf("expected %v stab non-zero for [%+v)", v, xy)
-			}
-			if !ivt.Intersects(NewInt64Point(v)) {
-				t.Fatalf("did not get %d as expected for [%+v)", v, xy)
-			}
+			require.NotEmptyf(t, ivt.Stab(NewInt64Point(v)), "expected %v stab non-zero for [%+v)", v, xy)
+			require.Truef(t, ivt.Intersects(NewInt64Point(v)), "did not get %d as expected for [%+v)", v, xy)
 		}
 		if !ivt.Delete(NewInt64Interval(ab.x, ab.y)) {
 			t.Errorf("did not delete %v as expected", ab)

diff --git a/pkg/crc/crc_test.go b/pkg/crc/crc_test.go
@@ -8,15 +8,16 @@ import (
 	"hash/crc32"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 // TestHash32 tests that Hash32 provided by this package can take an initial
 // crc and behaves exactly the same as the standard one in the following calls.
 func TestHash32(t *testing.T) {
 	stdhash := crc32.New(crc32.IEEETable)
-	if _, err := stdhash.Write([]byte("test data")); err != nil {
-		t.Fatalf("unexpected write error: %v", err)
-	}
+	_, err := stdhash.Write([]byte("test data"))
+	require.NoErrorf(t, err, "unexpected write error: %v", err)
 	// create a new hash with stdhash.Sum32() as initial crc
 	hash := New(stdhash.Sum32(), crc32.IEEETable)
 
@@ -38,12 +39,10 @@ func TestHash32(t *testing.T) {
 	}
 
 	// write something
-	if _, err := stdhash.Write([]byte("test data")); err != nil {
-		t.Fatalf("unexpected write error: %v", err)
-	}
-	if _, err := hash.Write([]byte("test data")); err != nil {
-		t.Fatalf("unexpected write error: %v", err)
-	}
+	_, err = stdhash.Write([]byte("test data"))
+	require.NoErrorf(t, err, "unexpected write error: %v", err)
+	_, err = hash.Write([]byte("test data"))
+	require.NoErrorf(t, err, "unexpected write error: %v", err)
 	wsum32 = stdhash.Sum32()
 	if g := hash.Sum32(); g != wsum32 {
 		t.Errorf("Sum32 after write = %d, want %d", g, wsum32)

diff --git a/pkg/expect/expect_test.go b/pkg/expect/expect_test.go
@@ -29,27 +29,17 @@ import (
 
 func TestExpectFunc(t *testing.T) {
 	ep, err := NewExpect("echo", "hello world")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	wstr := "hello world\r\n"
 	l, eerr := ep.ExpectFunc(context.Background(), func(a string) bool { return len(a) > 10 })
-	if eerr != nil {
-		t.Fatal(eerr)
-	}
-	if l != wstr {
-		t.Fatalf(`got "%v", expected "%v"`, l, wstr)
-	}
-	if cerr := ep.Close(); cerr != nil {
-		t.Fatal(cerr)
-	}
+	require.NoError(t, eerr)
+	require.Equalf(t, l, wstr, `got "%v", expected "%v"`, l, wstr)
+	require.NoError(t, ep.Close())
 }
 
 func TestExpectFuncTimeout(t *testing.T) {
 	ep, err := NewExpect("tail", "-f", "/dev/null")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	go func() {
 		// It's enough to have "talkative" process to stuck in the infinite loop of reading
 		for {
@@ -66,21 +56,15 @@ func TestExpectFuncTimeout(t *testing.T) {
 
 	require.ErrorIs(t, err, context.DeadlineExceeded)
 
-	if err = ep.Stop(); err != nil {
-		t.Fatal(err)
-	}
-
-	err = ep.Close()
-	require.ErrorContains(t, err, "unexpected exit code [143]")
+	require.NoError(t, ep.Stop())
+	require.ErrorContains(t, ep.Close(), "unexpected exit code [143]")
 	require.Equal(t, 143, ep.exitCode)
 }
 
 func TestExpectFuncExitFailure(t *testing.T) {
 	// tail -x should not exist and return a non-zero exit code
 	ep, err := NewExpect("tail", "-x")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
 	defer cancel()
@@ -95,9 +79,7 @@ func TestExpectFuncExitFailure(t *testing.T) {
 func TestExpectFuncExitFailureStop(t *testing.T) {
 	// tail -x should not exist and return a non-zero exit code
 	ep, err := NewExpect("tail", "-x")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
 	defer cancel()
@@ -109,81 +91,50 @@ func TestExpectFuncExitFailureStop(t *testing.T) {
 	exitCode, err := ep.ExitCode()
 	require.Equal(t, 1, exitCode)
 	require.NoError(t, err)
-
-	if err = ep.Stop(); err != nil {
-		t.Fatal(err)
-	}
-	err = ep.Close()
-	require.ErrorContains(t, err, "unexpected exit code [1]")
+	require.NoError(t, ep.Stop())
+	require.ErrorContains(t, ep.Close(), "unexpected exit code [1]")
 	exitCode, err = ep.ExitCode()
 	require.Equal(t, 1, exitCode)
 	require.NoError(t, err)
 }
 
 func TestEcho(t *testing.T) {
 	ep, err := NewExpect("echo", "hello world")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	ctx := context.Background()
 	l, eerr := ep.ExpectWithContext(ctx, ExpectedResponse{Value: "world"})
-	if eerr != nil {
-		t.Fatal(eerr)
-	}
+	require.NoError(t, eerr)
 	wstr := "hello world"
-	if l[:len(wstr)] != wstr {
-		t.Fatalf(`got "%v", expected "%v"`, l, wstr)
-	}
-	if cerr := ep.Close(); cerr != nil {
-		t.Fatal(cerr)
-	}
-	if _, eerr = ep.ExpectWithContext(ctx, ExpectedResponse{Value: "..."}); eerr == nil {
-		t.Fatalf("expected error on closed expect process")
-	}
+	require.Equalf(t, l[:len(wstr)], wstr, `got "%v", expected "%v"`, l, wstr)
+	require.NoError(t, ep.Close())
+	_, eerr = ep.ExpectWithContext(ctx, ExpectedResponse{Value: "..."})
+	require.Errorf(t, eerr, "expected error on closed expect process")
 }
 
 func TestLineCount(t *testing.T) {
 	ep, err := NewExpect("printf", "1\n2\n3")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	wstr := "3"
 	l, eerr := ep.ExpectWithContext(context.Background(), ExpectedResponse{Value: wstr})
-	if eerr != nil {
-		t.Fatal(eerr)
-	}
-	if l != wstr {
-		t.Fatalf(`got "%v", expected "%v"`, l, wstr)
-	}
-	if ep.LineCount() != 3 {
-		t.Fatalf("got %d, expected 3", ep.LineCount())
-	}
-	if cerr := ep.Close(); cerr != nil {
-		t.Fatal(cerr)
-	}
+	require.NoError(t, eerr)
+	require.Equalf(t, l, wstr, `got "%v", expected "%v"`, l, wstr)
+	require.Equalf(t, 3, ep.LineCount(), "got %d, expected 3", ep.LineCount())
+	require.NoError(t, ep.Close())
 }
 
 func TestSend(t *testing.T) {
 	ep, err := NewExpect("tr", "a", "b")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := ep.Send("a\r"); err != nil {
-		t.Fatal(err)
-	}
-	if _, err := ep.ExpectWithContext(context.Background(), ExpectedResponse{Value: "b"}); err != nil {
-		t.Fatal(err)
-	}
-	if err := ep.Stop(); err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
+	err = ep.Send("a\r")
+	require.NoError(t, err)
+	_, err = ep.ExpectWithContext(context.Background(), ExpectedResponse{Value: "b"})
+	require.NoError(t, err)
+	require.NoError(t, ep.Stop())
 }
 
 func TestSignal(t *testing.T) {
 	ep, err := NewExpect("sleep", "100")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	ep.Signal(os.Interrupt)
 	donec := make(chan struct{})
 	go func() {
@@ -267,8 +218,7 @@ func TestResponseMatchRegularExpr(t *testing.T) {
 				require.Error(t, err)
 			}
 
-			cerr := ep.Close()
-			require.NoError(t, cerr)
+			require.NoError(t, ep.Close())
 		})
 	}
 }
diff --git a/pkg/flags/flag_test.go b/pkg/flags/flag_test.go
@@ -19,6 +19,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"go.uber.org/zap/zaptest"
 )
 
@@ -41,9 +42,8 @@ func TestSetFlagsFromEnv(t *testing.T) {
 		"a": "",
 		"b": "bar",
 	} {
-		if got := fs.Lookup(f).Value.String(); got != want {
-			t.Fatalf("flag %q=%q, want %q", f, got, want)
-		}
+		got := fs.Lookup(f).Value.String()
+		require.Equalf(t, want, got, "flag %q=%q, want %q", f, got, want)
 	}
 
 	// now read the env and verify flags were updated as expected
@@ -85,7 +85,5 @@ func TestSetFlagsFromEnvParsingError(t *testing.T) {
 			break
 		}
 	}
-	if err != nil {
-		t.Fatalf("unexpected error %v", err)
-	}
+	require.NoErrorf(t, err, "unexpected error %v", err)
 }
diff --git a/pkg/flags/strings_test.go b/pkg/flags/strings_test.go
@@ -17,6 +17,8 @@ package flags
 import (
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestStringsValue(t *testing.T) {
@@ -30,8 +32,6 @@ func TestStringsValue(t *testing.T) {
 	}
 	for i := range tests {
 		ss := []string(*NewStringsValue(tests[i].s))
-		if !reflect.DeepEqual(tests[i].exp, ss) {
-			t.Fatalf("#%d: expected %q, got %q", i, tests[i].exp, ss)
-		}
+		require.Truef(t, reflect.DeepEqual(tests[i].exp, ss), "#%d: expected %q, got %q", i, tests[i].exp, ss)
 	}
 }
diff --git a/pkg/flags/uint32_test.go b/pkg/flags/uint32_test.go
@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUint32Value(t *testing.T) {
@@ -101,9 +102,7 @@ func TestUint32FromFlag(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			fs := flag.NewFlagSet("etcd", flag.ContinueOnError)
 			fs.Var(NewUint32Value(tc.defaultVal), flagName, "Maximum concurrent streams that each client can open at a time.")
-			if err := fs.Parse(tc.arguments); err != nil {
-				t.Fatalf("Unexpected error: %v\n", err)
-			}
+			require.NoError(t, fs.Parse(tc.arguments))
 			actualMaxStream := Uint32FromFlag(fs, flagName)
 			assert.Equal(t, tc.expectedVal, actualMaxStream)
 		})