Ban usage of t.Fatal and t.Error (ava-labs#1453)

.golangci.yml

@@ -71,6 +71,7 @@ linters-settings:
       - 'require\.ErrorContains$(# ErrorIs should be used instead)?'
       - 'require\.EqualValues$(# Equal should be used instead)?'
       - 'require\.NotEqualValues$(# NotEqual should be used instead)?'
+      - '^(t|b|tb|f)\.(Fatal|Fatalf|Error|Errorf)$(# the require library should be used instead)?'
     exclude_godoc_examples: false
   # https://golangci-lint.run/usage/linters#gosec
   gosec:

cache/lru_cache_benchmark_test.go

@@ -7,6 +7,8 @@ import (
 	"crypto/rand"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/ava-labs/avalanchego/ids"
 )
 
@@ -16,9 +18,8 @@ func BenchmarkLRUCachePutSmall(b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		for i := 0; i < smallLen; i++ {
 			var id ids.ID
-			if _, err := rand.Read(id[:]); err != nil {
-				b.Fatal(err)
-			}
+			_, err := rand.Read(id[:])
+			require.NoError(b, err)
 			cache.Put(id, n)
 		}
 		b.StopTimer()
@@ -33,9 +34,8 @@ func BenchmarkLRUCachePutMedium(b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		for i := 0; i < mediumLen; i++ {
 			var id ids.ID
-			if _, err := rand.Read(id[:]); err != nil {
-				b.Fatal(err)
-			}
+			_, err := rand.Read(id[:])
+			require.NoError(b, err)
 			cache.Put(id, n)
 		}
 		b.StopTimer()
@@ -50,9 +50,8 @@ func BenchmarkLRUCachePutLarge(b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		for i := 0; i < largeLen; i++ {
 			var id ids.ID
-			if _, err := rand.Read(id[:]); err != nil {
-				b.Fatal(err)
-			}
+			_, err := rand.Read(id[:])
+			require.NoError(b, err)
 			cache.Put(id, n)
 		}
 		b.StopTimer()

cache/unique_cache_test.go

@@ -6,6 +6,8 @@ package cache
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/ava-labs/avalanchego/ids"
 )
 
@@ -23,50 +25,32 @@ func (e *evictable[_]) Evict() {
 }
 
 func TestEvictableLRU(t *testing.T) {
+	require := require.New(t)
+
 	cache := EvictableLRU[ids.ID, *evictable[ids.ID]]{}
 
 	expectedValue1 := &evictable[ids.ID]{id: ids.ID{1}}
-	if returnedValue := cache.Deduplicate(expectedValue1); returnedValue != expectedValue1 {
-		t.Fatalf("Returned unknown value")
-	} else if expectedValue1.evicted != 0 {
-		t.Fatalf("Value was evicted unexpectedly")
-	} else if returnedValue := cache.Deduplicate(expectedValue1); returnedValue != expectedValue1 {
-		t.Fatalf("Returned unknown value")
-	} else if expectedValue1.evicted != 0 {
-		t.Fatalf("Value was evicted unexpectedly")
-	}
+	require.Equal(expectedValue1, cache.Deduplicate(expectedValue1))
+	require.Zero(expectedValue1.evicted)
+	require.Equal(expectedValue1, cache.Deduplicate(expectedValue1))
+	require.Zero(expectedValue1.evicted)
 
 	expectedValue2 := &evictable[ids.ID]{id: ids.ID{2}}
 	returnedValue := cache.Deduplicate(expectedValue2)
-	switch {
-	case returnedValue != expectedValue2:
-		t.Fatalf("Returned unknown value")
-	case expectedValue1.evicted != 1:
-		t.Fatalf("Value should have been evicted")
-	case expectedValue2.evicted != 0:
-		t.Fatalf("Value was evicted unexpectedly")
-	}
+	require.Equal(expectedValue2, returnedValue)
+	require.Equal(1, expectedValue1.evicted)
+	require.Zero(expectedValue2.evicted)
 
 	cache.Size = 2
 
 	expectedValue3 := &evictable[ids.ID]{id: ids.ID{2}}
 	returnedValue = cache.Deduplicate(expectedValue3)
-	switch {
-	case returnedValue != expectedValue2:
-		t.Fatalf("Returned unknown value")
-	case expectedValue1.evicted != 1:
-		t.Fatalf("Value should have been evicted")
-	case expectedValue2.evicted != 0:
-		t.Fatalf("Value was evicted unexpectedly")
-	}
+	require.Equal(expectedValue2, returnedValue)
+	require.Equal(1, expectedValue1.evicted)
+	require.Zero(expectedValue2.evicted)
 
 	cache.Flush()
-	switch {
-	case expectedValue1.evicted != 1:
-		t.Fatalf("Value should have been evicted")
-	case expectedValue2.evicted != 1:
-		t.Fatalf("Value should have been evicted")
-	case expectedValue3.evicted != 0:
-		t.Fatalf("Value was evicted unexpectedly")
-	}
+	require.Equal(1, expectedValue1.evicted)
+	require.Equal(1, expectedValue2.evicted)
+	require.Zero(expectedValue3.evicted)
 }

chains/atomic/gsharedmemory/shared_memory_test.go

@@ -43,10 +43,10 @@ func TestInterface(t *testing.T) {
 }
 
 func wrapSharedMemory(t *testing.T, sm atomic.SharedMemory, db database.Database) (atomic.SharedMemory, io.Closer) {
+	require := require.New(t)
+
 	listener, err := grpcutils.NewListener()
-	if err != nil {
-		t.Fatalf("Failed to create listener: %s", err)
-	}
+	require.NoError(err)
 	serverCloser := grpcutils.ServerCloser{}
 
 	server := grpcutils.NewServer()
@@ -56,9 +56,7 @@ func wrapSharedMemory(t *testing.T, sm atomic.SharedMemory, db database.Database
 	go grpcutils.Serve(listener, server)
 
 	conn, err := grpcutils.Dial(listener.Addr().String())
-	if err != nil {
-		t.Fatalf("Failed to dial: %s", err)
-	}
+	require.NoError(err)
 
 	rpcsm := NewClient(sharedmemorypb.NewSharedMemoryClient(conn))
 	return rpcsm, conn

chains/atomic/memory_test.go

@@ -6,6 +6,8 @@ package atomic
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/ava-labs/avalanchego/database/memdb"
 	"github.com/ava-labs/avalanchego/ids"
 )
@@ -19,32 +21,26 @@ func TestSharedID(t *testing.T) {
 	sharedID0 := sharedID(blockchainID0, blockchainID1)
 	sharedID1 := sharedID(blockchainID1, blockchainID0)
 
-	if sharedID0 != sharedID1 {
-		t.Fatalf("SharedMemory.sharedID should be communitive")
-	}
+	require.Equal(t, sharedID0, sharedID1)
 }
 
 func TestMemoryMakeReleaseLock(t *testing.T) {
+	require := require.New(t)
+
 	m := NewMemory(memdb.New())
 
 	sharedID := sharedID(blockchainID0, blockchainID1)
 
 	lock0 := m.makeLock(sharedID)
 
-	if lock1 := m.makeLock(sharedID); lock0 != lock1 {
-		t.Fatalf("Memory.makeLock should have returned the same lock")
-	}
+	require.Equal(lock0, m.makeLock(sharedID))
 	m.releaseLock(sharedID)
 
-	if lock2 := m.makeLock(sharedID); lock0 != lock2 {
-		t.Fatalf("Memory.makeLock should have returned the same lock")
-	}
+	require.Equal(lock0, m.makeLock(sharedID))
 	m.releaseLock(sharedID)
 	m.releaseLock(sharedID)
 
-	if lock3 := m.makeLock(sharedID); lock0 == lock3 {
-		t.Fatalf("Memory.releaseLock should have returned freed the lock")
-	}
+	require.Equal(lock0, m.makeLock(sharedID))
 	m.releaseLock(sharedID)
 }
 
@@ -54,9 +50,7 @@ func TestMemoryUnknownFree(t *testing.T) {
 	sharedID := sharedID(blockchainID0, blockchainID1)
 
 	defer func() {
-		if recover() == nil {
-			t.Fatalf("Should have panicked due to an unknown free")
-		}
+		require.NotNil(t, recover())
 	}()
 
 	m.releaseLock(sharedID)

config/config_test.go

@@ -451,9 +451,10 @@ func TestGetSubnetConfigsFromFile(t *testing.T) {
 			v := setupViper(configFilePath)
 			subnetConfigs, err := getSubnetConfigs(v, []ids.ID{subnetID})
 			require.ErrorIs(err, test.expectedErr)
-			if test.expectedErr == nil {
-				test.testF(require, subnetConfigs)
+			if test.expectedErr != nil {
+				return
 			}
+			test.testF(require, subnetConfigs)
 		})
 	}
 }
@@ -544,9 +545,10 @@ func TestGetSubnetConfigsFromFlags(t *testing.T) {
 
 			subnetConfigs, err := getSubnetConfigs(v, []ids.ID{subnetID})
 			require.ErrorIs(err, test.expectedErr)
-			if test.expectedErr == nil {
-				test.testF(require, subnetConfigs)
+			if test.expectedErr != nil {
+				return
 			}
+			test.testF(require, subnetConfigs)
 		})
 	}
 }
@@ -560,9 +562,11 @@ func setupConfigJSON(t *testing.T, rootPath string, value string) string {
 
 // setups file creates necessary path and writes value to it.
 func setupFile(t *testing.T, path string, fileName string, value string) {
-	require.NoError(t, os.MkdirAll(path, 0o700))
+	require := require.New(t)
+
+	require.NoError(os.MkdirAll(path, 0o700))
 	filePath := filepath.Join(path, fileName)
-	require.NoError(t, os.WriteFile(filePath, []byte(value), 0o600))
+	require.NoError(os.WriteFile(filePath, []byte(value), 0o600))
 }
 
 func setupViperFlags() *viper.Viper {

genesis/genesis_test.go

@@ -164,10 +164,8 @@ func TestValidateConfig(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			require := require.New(t)
-
 			err := validateConfig(test.networkID, test.config, genesisStakingCfg)
-			require.ErrorIs(err, test.expectedErr)
+			require.ErrorIs(t, err, test.expectedErr)
 		})
 	}
 }
@@ -230,9 +228,9 @@ func TestGenesisFromFile(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			// test loading of genesis from file
-
 			require := require.New(t)
+
+			// test loading of genesis from file
 			var customFile string
 			if len(test.customConfig) > 0 {
 				customFile = filepath.Join(t.TempDir(), "config.json")
@@ -304,9 +302,9 @@ func TestGenesisFromFlag(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			// test loading of genesis content from flag/env-var
-
 			require := require.New(t)
+
+			// test loading of genesis content from flag/env-var
 			var genBytes []byte
 			if len(test.customConfig) == 0 {
 				// try loading a default config

ids/aliases_test.go

@@ -30,6 +30,5 @@ func TestPrimaryAliasOrDefaultTest(t *testing.T) {
 	require.Equal(res, id1.String())
 
 	expected := "Batman"
-	res = aliaser.PrimaryAliasOrDefault(id2)
-	require.Equal(expected, res)
+	require.Equal(expected, aliaser.PrimaryAliasOrDefault(id2))
 }