Update lint rules, force testify/assert for tests

Use testify's assert package instead of the standard library's testing
package.

Author: JoeTurki

.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,14 +131,15 @@ 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
     - path: cmd
       linters:
         - forbidigo
-  max-issues-per-linter: 0
-  max-same-issues: 0

errors_test.go

@@ -6,6 +6,8 @@ package interceptor
 import (
 	"errors"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestMultiError(t *testing.T) {
@@ -25,20 +27,13 @@ func TestMultiError(t *testing.T) {
 	})
 	str := "err1\nerr2\nerr3"
 
-	if errs.Error() != str {
-		t.Errorf("String representation doesn't match, expected: %s, got: %s", errs.Error(), str)
-	}
+	assert.Equal(t, str, errs.Error(), "String representation doesn't match")
 
 	errIs, ok := errs.(multiError) //nolint
-	if !ok {
-		t.Fatal("FlattenErrs returns non-multiError")
-	}
+	assert.True(t, ok, "FlattenErrs returns non-multiError")
+
 	for i := 0; i < 3; i++ {
-		if !errIs.Is(rawErrs[i]) {
-			t.Errorf("'%+v' should contains '%v'", errs, rawErrs[i])
-		}
-	}
-	if errIs.Is(rawErrs[3]) {
-		t.Errorf("'%+v' should not contains '%v'", errs, rawErrs[3])
+		assert.True(t, errIs.Is(rawErrs[i]), "Should contain error %d", i)
 	}
+	assert.False(t, errIs.Is(rawErrs[3]), "Should not contain error %d", 3)
 }

internal/rtpbuffer/rtpbuffer_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/pion/rtp"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -35,14 +36,8 @@ func TestRTPBuffer(t *testing.T) {
 			for _, n := range nums {
 				seq := start + n
 				packet := sb.Get(seq)
-				if packet == nil {
-					t.Errorf("packet not found: %d", seq)
-
-					continue
-				}
-				if packet.Header().SequenceNumber != seq {
-					t.Errorf("packet for %d returned with incorrect SequenceNumber: %d", seq, packet.Header().SequenceNumber)
-				}
+				assert.NotNil(t, packet, "packet not found: %d", seq)
+				assert.Equal(t, seq, packet.Header().SequenceNumber, "packet for %d returned with incorrect SequenceNumber", seq)
 				packet.Release()
 			}
 		}
@@ -51,9 +46,7 @@ func TestRTPBuffer(t *testing.T) {
 			for _, n := range nums {
 				seq := start + n
 				packet := sb.Get(seq)
-				if packet != nil {
-					t.Errorf("packet found for %d: %d", seq, packet.Header().SequenceNumber)
-				}
+				assert.Nil(t, packet, "packet found for %d", seq)
 			}
 		}
 
@@ -99,17 +92,14 @@ func TestRTPBuffer_WithRTX(t *testing.T) {
 			for _, n := range nums {
 				seq := start + n
 				packet := sb.Get(seq)
-				if packet == nil {
-					t.Errorf("packet not found: %d", seq)
-
-					continue
-				}
-				if packet.Header().SSRC != 1 && packet.Header().PayloadType != 1 {
-					t.Errorf(
-						"packet for %d returned with incorrect SSRC : %d and PayloadType: %d",
-						seq, packet.Header().SSRC, packet.Header().PayloadType,
-					)
-				}
+				assert.NotNil(t, packet, "packet not found: %d", seq)
+
+				assert.True(
+					t,
+					packet.Header().SSRC == 1 && packet.Header().PayloadType == 1,
+					"packet for %d returned with incorrect SSRC : %d and PayloadType: %d",
+					seq, packet.Header().SSRC, packet.Header().PayloadType,
+				)
 				packet.Release()
 			}
 		}
@@ -118,9 +108,7 @@ func TestRTPBuffer_WithRTX(t *testing.T) {
 			for _, n := range nums {
 				seq := start + n
 				packet := sb.Get(seq)
-				if packet != nil {
-					t.Errorf("packet found for %d: %d", seq, packet.Header().SequenceNumber)
-				}
+				assert.Nil(t, packet, "packet found for %d", seq)
 			}
 		}
 

internal/test/mock_stream_test.go

@@ -22,11 +22,11 @@ func TestMockStream(t *testing.T) {
 	select {
 	case <-mockStream.WrittenRTCP():
 	case <-time.After(10 * time.Millisecond):
-		t.Error("rtcp packet written but not found")
+		assert.Fail(t, "rtcp packet written but not found")
 	}
 	select {
 	case <-mockStream.WrittenRTCP():
-		t.Error("single rtcp packet written, but multiple found")
+		assert.Fail(t, "single rtcp packet written, but multiple found")
 	case <-time.After(10 * time.Millisecond):
 	}
 
@@ -35,41 +35,37 @@ func TestMockStream(t *testing.T) {
 	select {
 	case <-mockStream.WrittenRTP():
 	case <-time.After(10 * time.Millisecond):
-		t.Error("rtp packet written but not found")
+		assert.Fail(t, "rtp packet written but not found")
 	}
 	select {
 	case <-mockStream.WrittenRTP():
-		t.Error("single rtp packet written, but multiple found")
+		assert.Fail(t, "single rtp packet written, but multiple found")
 	case <-time.After(10 * time.Millisecond):
 	}
 
 	mockStream.ReceiveRTCP([]rtcp.Packet{&rtcp.PictureLossIndication{}})
 	select {
 	case r := <-mockStream.ReadRTCP():
-		if r.Err != nil {
-			t.Errorf("read rtcp returned error: %v", r.Err)
-		}
+		assert.NoError(t, r.Err, "read rtcp returned error")
 	case <-time.After(10 * time.Millisecond):
-		t.Error("rtcp packet received but not read")
+		assert.Fail(t, "rtcp packet received but not read")
 	}
 	select {
 	case r := <-mockStream.ReadRTCP():
-		t.Errorf("single rtcp packet received, but multiple read: %v", r)
+		assert.Fail(t, "single rtcp packet received, but multiple read: %v", r)
 	case <-time.After(10 * time.Millisecond):
 	}
 
 	mockStream.ReceiveRTP(&rtp.Packet{})
 	select {
 	case r := <-mockStream.ReadRTP():
-		if r.Err != nil {
-			t.Errorf("read rtcp returned error: %v", r.Err)
-		}
+		assert.NoError(t, r.Err, "read rtp returned error")
 	case <-time.After(10 * time.Millisecond):
-		t.Error("rtp packet received but not read")
+		assert.Fail(t, "rtp packet received but not read")
 	}
 	select {
 	case r := <-mockStream.ReadRTP():
-		t.Errorf("single rtp packet received, but multiple read: %v", r)
+		assert.Fail(t, "single rtp packet received, but multiple read: %v", r)
 	case <-time.After(10 * time.Millisecond):
 	}
 

pkg/gcc/send_side_bwe_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/pion/interceptor/pkg/twcc"
 	"github.com/pion/rtcp"
 	"github.com/pion/rtp"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -37,13 +38,13 @@ func (m *mockTWCCResponder) Write(pkts []rtcp.Packet, attributes interceptor.Att
 // the congestion controller.
 type mockGCCWriteStream struct {
 	twccResponder *mockTWCCResponder
+	t             *testing.T
 }
 
 func (m *mockGCCWriteStream) Write(header *rtp.Header, payload []byte, _ interceptor.Attributes) (int, error) {
+	m.t.Helper()
 	pkt, err := (&rtp.Packet{Header: *header, Payload: payload}).Marshal()
-	if err != nil {
-		panic(err)
-	}
+	assert.NoError(m.t, err)
 
 	m.twccResponder.rtpChan <- pkt
 
@@ -63,10 +64,11 @@ func TestSendSideBWE(t *testing.T) {
 	require.NotNil(t, bwe)
 
 	gccMock := &mockGCCWriteStream{
-		&mockTWCCResponder{
+		twccResponder: &mockTWCCResponder{
 			bwe,
 			make(chan []byte, 500),
 		},
+		t: t,
 	}
 
 	twccSender, err := (&twcc.SenderInterceptorFactory{}).NewInterceptor("")
@@ -86,12 +88,10 @@ func TestSendSideBWE(t *testing.T) {
 	rtpWriter = twccWriter.BindLocalStream(streamInfo, rtpWriter)
 
 	for i := 0; i <= 100; i++ {
-		if _, err = rtpWriter.Write(&rtp.Header{SSRC: 1, Extensions: []rtp.Extension{}}, rtpPayload, nil); err != nil {
-			panic(err)
-		}
-		if _, _, err = twccInboundRTP.Read(buffer, nil); err != nil {
-			panic(err)
-		}
+		_, err = rtpWriter.Write(&rtp.Header{SSRC: 1, Extensions: []rtp.Extension{}}, rtpPayload, nil)
+		assert.NoError(t, err)
+		_, _, err = twccInboundRTP.Read(buffer, nil)
+		assert.NoError(t, err)
 	}
 
 	// Sending a stream with zero loss and no RTT should increase estimate

pkg/mock/interceptor_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/pion/interceptor"
+	"github.com/stretchr/testify/assert"
 )
 
 //nolint:cyclop
@@ -21,23 +22,25 @@ func TestInterceptor(t *testing.T) {
 	t.Run("Default", func(t *testing.T) {
 		testInterceptor := &Interceptor{}
 
-		if testInterceptor.BindRTCPWriter(dummyRTCPWriter) != dummyRTCPWriter {
-			t.Error("Default BindRTCPWriter should return given writer")
-		}
-		if testInterceptor.BindRTCPReader(dummyRTCPReader) != dummyRTCPReader {
-			t.Error("Default BindRTCPReader should return given reader")
-		}
-		if testInterceptor.BindLocalStream(dummyStreamInfo, dummyRTPWriter) != dummyRTPWriter {
-			t.Error("Default BindLocalStream should return given writer")
-		}
+		assert.Equal(
+			t, dummyRTCPWriter, testInterceptor.BindRTCPWriter(dummyRTCPWriter),
+			"Default BindRTCPWriter should return given writer",
+		)
+		assert.Equal(
+			t, dummyRTCPReader, testInterceptor.BindRTCPReader(dummyRTCPReader),
+			"Default BindRTCPReader should return given reader",
+		)
+		assert.Equal(
+			t, dummyRTPWriter, testInterceptor.BindLocalStream(dummyStreamInfo, dummyRTPWriter),
+			"Default BindLocalStream should return given writer",
+		)
 		testInterceptor.UnbindLocalStream(dummyStreamInfo)
-		if testInterceptor.BindRemoteStream(dummyStreamInfo, dummyRTPReader) != dummyRTPReader {
-			t.Error("Default BindRemoteStream should return given reader")
-		}
+		assert.Equal(
+			t, dummyRTPReader, testInterceptor.BindRemoteStream(dummyStreamInfo, dummyRTPReader),
+			"Default BindRemoteStream should return given writer",
+		)
 		testInterceptor.UnbindRemoteStream(dummyStreamInfo)
-		if testInterceptor.Close() != nil {
-			t.Error("Default Close should return nil")
-		}
+		assert.NoError(t, testInterceptor.Close(), "Default Close should return nil")
 	})
 	t.Run("Custom", func(t *testing.T) {
 		var (
@@ -83,44 +86,38 @@ func TestInterceptor(t *testing.T) {
 			},
 		}
 
-		if testInterceptor.BindRTCPWriter(dummyRTCPWriter) != dummyRTCPWriter {
-			t.Error("Mocked BindRTCPWriter should return given writer")
-		}
-		if testInterceptor.BindRTCPReader(dummyRTCPReader) != dummyRTCPReader {
-			t.Error("Mocked BindRTCPReader should return given reader")
-		}
-		if testInterceptor.BindLocalStream(dummyStreamInfo, dummyRTPWriter) != dummyRTPWriter {
-			t.Error("Mocked BindLocalStream should return given writer")
-		}
+		assert.Equal(
+			t, dummyRTCPWriter, testInterceptor.BindRTCPWriter(dummyRTCPWriter),
+			"Mocked BindRTCPWriter should return given writer",
+		)
+		assert.Equal(
+			t, dummyRTCPReader, testInterceptor.BindRTCPReader(dummyRTCPReader),
+			"Mocked BindRTCPReader should return given reader",
+		)
+		assert.Equal(
+			t, dummyRTPWriter, testInterceptor.BindLocalStream(dummyStreamInfo, dummyRTPWriter),
+			"Mocked BindLocalStream should return given writer",
+		)
 		testInterceptor.UnbindLocalStream(dummyStreamInfo)
-		if testInterceptor.BindRemoteStream(dummyStreamInfo, dummyRTPReader) != dummyRTPReader {
-			t.Error("Mocked BindRemoteStream should return given reader")
-		}
+		assert.Equal(
+			t, dummyRTPReader, testInterceptor.BindRemoteStream(dummyStreamInfo, dummyRTPReader),
+			"Mocked BindRemoteStream should return given writer",
+		)
 		testInterceptor.UnbindRemoteStream(dummyStreamInfo)
-		if testInterceptor.Close() != nil {
-			t.Error("Mocked Close should return nil")
-		}
+		assert.NoError(t, testInterceptor.Close(), "Mocked Close should return nil")
 
-		if cnt := atomic.LoadUint32(&cntBindRTCPWriter); cnt != 1 {
-			t.Errorf("BindRTCPWriterFn is expected to be called once, but called %d times", cnt)
-		}
-		if cnt := atomic.LoadUint32(&cntBindRTCPReader); cnt != 1 {
-			t.Errorf("BindRTCPReaderFn is expected to be called once, but called %d times", cnt)
-		}
-		if cnt := atomic.LoadUint32(&cntBindLocalStream); cnt != 1 {
-			t.Errorf("BindLocalStreamFn is expected to be called once, but called %d times", cnt)
-		}
-		if cnt := atomic.LoadUint32(&cntUnbindLocalStream); cnt != 1 {
-			t.Errorf("UnbindLocalStreamFn is expected to be called once, but called %d times", cnt)
-		}
-		if cnt := atomic.LoadUint32(&cntBindRemoteStream); cnt != 1 {
-			t.Errorf("BindRemoteStreamFn is expected to be called once, but called %d times", cnt)
-		}
-		if cnt := atomic.LoadUint32(&cntUnbindRemoteStream); cnt != 1 {
-			t.Errorf("UnbindRemoteStreamFn is expected to be called once, but called %d times", cnt)
-		}
-		if cnt := atomic.LoadUint32(&cntClose); cnt != 1 {
-			t.Errorf("CloseFn is expected to be called once, but called %d times", cnt)
-		}
+		assert.Equal(t, uint32(1), atomic.LoadUint32(&cntBindRTCPWriter), "BindRTCPWriterFn is expected to be called once")
+		assert.Equal(t, uint32(1), atomic.LoadUint32(&cntBindRTCPReader), "BindRTCPReaderFn is expected to be called once")
+		assert.Equal(t, uint32(1), atomic.LoadUint32(&cntBindLocalStream), "BindLocalStreamFn is expected to be called once")
+		assert.Equal(
+			t, uint32(1), atomic.LoadUint32(&cntUnbindLocalStream), "UnbindLocalStreamFn is expected to be called once",
+		)
+		assert.Equal(
+			t, uint32(1), atomic.LoadUint32(&cntBindRemoteStream), "BindRemoteStreamFn is expected to be called once",
+		)
+		assert.Equal(
+			t, uint32(1), atomic.LoadUint32(&cntUnbindRemoteStream), "UnbindRemoteStreamFn is expected to be called once",
+		)
+		assert.Equal(t, uint32(1), atomic.LoadUint32(&cntClose), "CloseFn is expected to be called once")
 	})
 }