Update lint rules, use testify/assert

Author: RyanPotat

.golangci.yml

@@ -19,12 +19,16 @@ linters-settings:
             recommendations:
               - errors
   forbidigo:
+    analyze-types: true
     forbid:
       - ^fmt.Print(f|ln)?$
       - ^log.(Panic|Fatal|Print)(f|ln)?$
       - ^os.Exit$
       - ^panic$
       - ^print(ln)?$
+      - p: ^testing.T.(Error|Errorf|Fatal|Fatalf|Fail|FailNow)$
+        pkg: ^testing$
+        msg: "use testify/assert instead"
   varnamelen:
     max-distance: 12
     min-name-length: 2
@@ -127,9 +131,12 @@ issues:
   exclude-dirs-use-default: false
   exclude-rules:
     # Allow complex tests and examples, better to be self contained
-    - path: (examples|main\.go|_test\.go)
+    - path: (examples|main\.go)
       linters:
+        - gocognit
         - forbidigo
+    - path: _test\.go
+      linters:
         - gocognit
 
     # Allow forbidden identifiers in CLI commands

client_test.go

@@ -108,15 +108,11 @@ func TestClientWithSTUN(t *testing.T) {
 		// Block until go routine is started to make two almost parallel requests
 		<-started
 
-		if _, err = client.SendBindingRequestTo(to); err != nil {
-			t.Fatal(err)
-		}
+		_, err = client.SendBindingRequestTo(to)
+		assert.NoError(t, err)
 
 		<-finished
-		if err1 != nil {
-			t.Fatal(err)
-		}
-
+		assert.NoErrorf(t, err1, "should succeed: %v", err)
 		assert.NoError(t, pc.Close())
 	})
 
@@ -244,8 +240,7 @@ func TestTCPClient(t *testing.T) {
 
 	peerAddr, err := net.ResolveUDPAddr("udp", "127.0.0.1:12345")
 	require.NoError(t, err)
-	err = client.CreatePermission(peerAddr)
-	require.NoError(t, err)
+	require.NoError(t, client.CreatePermission(peerAddr))
 
 	var cid proto.ConnectionID = 5
 	transactionID := [stun.TransactionIDSize]byte{1, 2, 3}
@@ -259,8 +254,7 @@ func TestTCPClient(t *testing.T) {
 	msg, err := stun.Build(attrs...)
 	require.NoError(t, err)
 
-	err = client.handleSTUNMessage(msg.Raw, peerAddr)
-	require.NoError(t, err)
+	require.NoError(t, client.handleSTUNMessage(msg.Raw, peerAddr))
 
 	// Shutdown
 	require.NoError(t, allocation.Close())

internal/allocation/allocation_manager_test.go

@@ -35,9 +35,7 @@ func TestManager(t *testing.T) {
 
 	network := "udp4"
 	turnSocket, err := net.ListenPacket(network, "0.0.0.0:0")
-	if err != nil {
-		panic(err)
-	}
+	assert.NoError(t, err)
 
 	for _, tc := range tt {
 		f := tc.f
@@ -54,15 +52,17 @@ func subTestCreateInvalidAllocation(t *testing.T, turnSocket net.PacketConn) {
 	m, err := newTestManager()
 	assert.NoError(t, err)
 
-	if a, err := m.CreateAllocation(nil, turnSocket, 0, proto.DefaultLifetime); a != nil || err == nil {
-		t.Errorf("Illegally created allocation with nil FiveTuple")
-	}
-	if a, err := m.CreateAllocation(randomFiveTuple(), nil, 0, proto.DefaultLifetime); a != nil || err == nil {
-		t.Errorf("Illegally created allocation with nil turnSocket")
-	}
-	if a, err := m.CreateAllocation(randomFiveTuple(), turnSocket, 0, 0); a != nil || err == nil {
-		t.Errorf("Illegally created allocation with 0 lifetime")
-	}
+	a, err := m.CreateAllocation(nil, turnSocket, 0, proto.DefaultLifetime)
+	assert.Nil(t, a, "Illegally created allocation with nil FiveTuple")
+	assert.Error(t, err, "Illegally created allocation with nil FiveTuple")
+
+	a, err = m.CreateAllocation(randomFiveTuple(), nil, 0, proto.DefaultLifetime)
+	assert.Nil(t, a, "Illegally created allocation with nil turnSocket")
+	assert.Error(t, err, "Illegally created allocation with nil turnSocket")
+
+	a, err = m.CreateAllocation(randomFiveTuple(), turnSocket, 0, 0)
+	assert.Nil(t, a, "Illegally created allocation with 0 lifetime")
+	assert.Error(t, err, "Illegally created allocation with 0 lifetime")
 }
 
 // Test valid Allocation creations.
@@ -73,13 +73,12 @@ func subTestCreateAllocation(t *testing.T, turnSocket net.PacketConn) {
 	assert.NoError(t, err)
 
 	fiveTuple := randomFiveTuple()
-	if a, err := m.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime); a == nil || err != nil {
-		t.Errorf("Failed to create allocation %v %v", a, err)
-	}
+	a, err := m.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime)
+	assert.NotNil(t, a, "Failed to create allocation")
+	assert.NoError(t, err, "Failed to create allocation")
 
-	if a := m.GetAllocation(fiveTuple); a == nil {
-		t.Errorf("Failed to get allocation right after creation")
-	}
+	a = m.GetAllocation(fiveTuple)
+	assert.NotNil(t, a, "Failed to get allocation right after creation")
 }
 
 // Test that two allocations can't be created with the same FiveTuple.
@@ -90,13 +89,13 @@ func subTestCreateAllocationDuplicateFiveTuple(t *testing.T, turnSocket net.Pack
 	assert.NoError(t, err)
 
 	fiveTuple := randomFiveTuple()
-	if a, err := m.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime); a == nil || err != nil {
-		t.Errorf("Failed to create allocation %v %v", a, err)
-	}
+	a, err := m.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime)
+	assert.NotNil(t, a, "Failed to create allocation")
+	assert.NoError(t, err, "Failed to create allocation")
 
-	if a, err := m.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime); a != nil || err == nil {
-		t.Errorf("Was able to create allocation with same FiveTuple twice")
-	}
+	a, err = m.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime)
+	assert.Nil(t, a, "Was able to create allocation with same FiveTuple twice")
+	assert.Error(t, err, "Was able to create allocation with same FiveTuple twice")
 }
 
 func subTestDeleteAllocation(t *testing.T, turnSocket net.PacketConn) {
@@ -106,18 +105,16 @@ func subTestDeleteAllocation(t *testing.T, turnSocket net.PacketConn) {
 	assert.NoError(t, err)
 
 	fiveTuple := randomFiveTuple()
-	if a, err := manager.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime); a == nil || err != nil {
-		t.Errorf("Failed to create allocation %v %v", a, err)
-	}
+	a, err := manager.CreateAllocation(fiveTuple, turnSocket, 0, proto.DefaultLifetime)
+	assert.NotNil(t, a, "Failed to create allocation")
+	assert.NoError(t, err, "Failed to create allocation")
 
-	if a := manager.GetAllocation(fiveTuple); a == nil {
-		t.Errorf("Failed to get allocation right after creation")
-	}
+	a = manager.GetAllocation(fiveTuple)
+	assert.NotNil(t, a, "Failed to get allocation right after creation")
 
 	manager.DeleteAllocation(fiveTuple)
-	if a := manager.GetAllocation(fiveTuple); a != nil {
-		t.Errorf("Get allocation with %v should be nil after delete", fiveTuple)
-	}
+	a = manager.GetAllocation(fiveTuple)
+	assert.Nilf(t, a, "Failed to delete allocation %v", fiveTuple)
 }
 
 // Test that allocation should be closed if timeout.
@@ -134,19 +131,15 @@ func subTestAllocationTimeout(t *testing.T, turnSocket net.PacketConn) {
 		fiveTuple := randomFiveTuple()
 
 		a, err := m.CreateAllocation(fiveTuple, turnSocket, 0, lifetime)
-		if err != nil {
-			t.Errorf("Failed to create allocation with %v", fiveTuple)
-		}
+		assert.NoErrorf(t, err, "Failed to create allocation with %v", fiveTuple)
 
 		allocations[index] = a
 	}
 
 	// Make sure all allocations timeout
 	time.Sleep(lifetime + time.Second)
 	for _, alloc := range allocations {
-		if !isClose(alloc.RelaySocket) {
-			t.Error("Allocation relay socket should be closed if lifetime timeout")
-		}
+		assert.True(t, isClose(alloc.RelaySocket), "Allocation relay socket should be closed if lifetime timeout")
 	}
 }
 
@@ -166,15 +159,10 @@ func subTestManagerClose(t *testing.T, turnSocket net.PacketConn) {
 
 	// Make a1 timeout
 	time.Sleep(2 * time.Second)
-
-	if err := manager.Close(); err != nil {
-		t.Errorf("Manager close with error: %v", err)
-	}
+	assert.NoError(t, manager.Close())
 
 	for _, alloc := range allocations {
-		if !isClose(alloc.RelaySocket) {
-			t.Error("Manager's allocations should be closed")
-		}
+		assert.True(t, isClose(alloc.RelaySocket), "Manager's allocations should be closed")
 	}
 }
 

internal/allocation/allocation_test.go

@@ -51,19 +51,13 @@ func subTestGetPermission(t *testing.T) {
 	alloc := NewAllocation(nil, nil, nil)
 
 	addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	addr2, err := net.ResolveUDPAddr("udp", "127.0.0.1:3479")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	addr3, err := net.ResolveUDPAddr("udp", "127.0.0.2:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	perms := &Permission{
 		Addr: addr,
@@ -95,9 +89,7 @@ func subTestAddPermission(t *testing.T) {
 	alloc := NewAllocation(nil, nil, nil)
 
 	addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	p := &Permission{
 		Addr: addr,
@@ -116,9 +108,7 @@ func subTestRemovePermission(t *testing.T) {
 	alloc := NewAllocation(nil, nil, nil)
 
 	addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	p := &Permission{
 		Addr: addr,
@@ -141,9 +131,7 @@ func subTestAddChannelBind(t *testing.T) {
 	alloc := NewAllocation(nil, nil, nil)
 
 	addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	c := NewChannelBind(proto.MinChannelNumber, addr, nil)
 
@@ -167,9 +155,7 @@ func subTestGetChannelByNumber(t *testing.T) {
 	alloc := NewAllocation(nil, nil, nil)
 
 	addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	c := NewChannelBind(proto.MinChannelNumber, addr, nil)
 
@@ -188,9 +174,7 @@ func subTestGetChannelByAddr(t *testing.T) {
 	alloc := NewAllocation(nil, nil, nil)
 
 	addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	c := NewChannelBind(proto.MinChannelNumber, addr, nil)
 
@@ -210,9 +194,7 @@ func subTestRemoveChannelBind(t *testing.T) {
 	alloc := NewAllocation(nil, nil, nil)
 
 	addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	c := NewChannelBind(proto.MinChannelNumber, addr, nil)
 
@@ -250,9 +232,7 @@ func subTestAllocationClose(t *testing.T) {
 	network := "udp"
 
 	l, err := net.ListenPacket(network, "0.0.0.0:0")
-	if err != nil {
-		panic(err)
-	}
+	assert.NoError(t, err)
 
 	alloc := NewAllocation(nil, nil, nil)
 	alloc.RelaySocket = l
@@ -261,18 +241,15 @@ func subTestAllocationClose(t *testing.T) {
 
 	// Add channel
 	addr, err := net.ResolveUDPAddr(network, "127.0.0.1:3478")
-	if err != nil {
-		t.Fatalf("failed to resolve: %s", err)
-	}
+	assert.NoError(t, err)
 
 	c := NewChannelBind(proto.MinChannelNumber, addr, nil)
 	_ = alloc.AddChannelBind(c, proto.DefaultLifetime)
 
 	// Add permission
 	alloc.AddPermission(NewPermission(addr, nil))
 
-	err = alloc.Close()
-	assert.Nil(t, err, "should succeed")
+	assert.Nil(t, alloc.Close(), "should succeed")
 	assert.True(t, isClose(alloc.RelaySocket), "should be closed")
 }
 
@@ -285,15 +262,11 @@ func subTestPacketHandler(t *testing.T) {
 
 	// TURN server initialization
 	turnSocket, err := net.ListenPacket(network, "127.0.0.1:0")
-	if err != nil {
-		panic(err)
-	}
+	assert.NoError(t, err)
 
 	// Client listener initialization
 	clientListener, err := net.ListenPacket(network, "127.0.0.1:0")
-	if err != nil {
-		panic(err)
-	}
+	assert.NoError(t, err)
 
 	dataCh := make(chan []byte)
 	// Client listener read data
@@ -314,17 +287,13 @@ func subTestPacketHandler(t *testing.T) {
 		DstAddr: turnSocket.LocalAddr(),
 	}, turnSocket, 0, proto.DefaultLifetime)
 
-	assert.Nil(t, err, "should succeed")
+	assert.NoError(t, err, "should succeed")
 
 	peerListener1, err := net.ListenPacket(network, "127.0.0.1:0")
-	if err != nil {
-		panic(err)
-	}
+	assert.NoError(t, err)
 
 	peerListener2, err := net.ListenPacket(network, "127.0.0.1:0")
-	if err != nil {
-		panic(err)
-	}
+	assert.NoError(t, err)
 
 	// Add permission with peer1 address
 	alloc.AddPermission(NewPermission(peerListener1.LocalAddr(), manager.log))
@@ -345,12 +314,10 @@ func subTestPacketHandler(t *testing.T) {
 	assert.True(t, stun.IsMessage(data), "should be stun message")
 
 	var msg stun.Message
-	err = stun.Decode(data, &msg)
-	assert.Nil(t, err, "decode data to stun message failed")
+	assert.NoError(t, stun.Decode(data, &msg), "decode data to stun message failed")
 
 	var msgData proto.Data
-	err = msgData.GetFrom(&msg)
-	assert.Nil(t, err, "get data from stun message failed")
+	assert.NoError(t, msgData.GetFrom(&msg), "get data from stun message failed")
 	assert.Equal(t, targetText, string(msgData), "get message doesn't equal the target text")
 
 	// Test for channel bind and channel data
@@ -364,8 +331,7 @@ func subTestPacketHandler(t *testing.T) {
 	channelData := proto.ChannelData{
 		Raw: data,
 	}
-	err = channelData.Decode()
-	assert.Nil(t, err, fmt.Sprintf("channel data decode with error: %v", err))
+	assert.NoError(t, channelData.Decode(), fmt.Sprintf("channel data decode with error: %v", err))
 	assert.Equal(t, channelBind.Number, channelData.Number, "get channel data's number is invalid")
 	assert.Equal(t, targetText2, string(channelData.Data), "get data doesn't equal the target text.")