Improve code cov by adding various tests

Author: philipch07

active_tcp_test.go

@@ -7,7 +7,9 @@
 package ice
 
 import (
+	"context"
 	"fmt"
+	"io"
 	"net"
 	"net/netip"
 	"sync/atomic"
@@ -290,3 +292,121 @@ func TestActiveTCP_Respect_NetworkTypes(t *testing.T) {
 	require.NoError(t, tcpListener.Close())
 	require.Equal(t, uint64(0), atomic.LoadUint64(&incomingTCPCount))
 }
+
+func TestNewActiveTCPConn_LocalAddrError_EarlyReturn(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	logger := logging.NewDefaultLoggerFactory().NewLogger("ice")
+
+	// an invalid local address so getTCPAddrOnInterface fails at ResolveTCPAddr.
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	ra := netip.MustParseAddrPort("127.0.0.1:1")
+
+	a := newActiveTCPConn(ctx, "this_is_not_a_valid_addr", ra, logger)
+
+	require.NotNil(t, a)
+	require.True(t, a.closed.Load(), "should be closed on early return error")
+	la := a.LocalAddr()
+	require.NotNil(t, la)
+}
+
+func TestActiveTCPConn_ReadLoop_BufferWriteError(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	tcpListener, err := net.Listen("tcp", "127.0.0.1:0") // nolint: noctx
+	require.NoError(t, err)
+	defer func() { _ = tcpListener.Close() }()
+
+	ra := netip.MustParseAddrPort(tcpListener.Addr().String())
+	logger := logging.NewDefaultLoggerFactory().NewLogger("ice")
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	a := newActiveTCPConn(ctx, "127.0.0.1:0", ra, logger)
+	require.NotNil(t, a)
+
+	srvConn, err := tcpListener.Accept()
+	require.NoError(t, err)
+
+	require.NoError(t, a.readBuffer.Close())
+
+	_, err = writeStreamingPacket(srvConn, []byte("ping"))
+	require.NoError(t, err)
+
+	require.NoError(t, a.Close())
+	_ = srvConn.Close()
+}
+
+func TestActiveTCPConn_WriteLoop_WriteStreamingError(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	tcpListener, err := net.Listen("tcp", "127.0.0.1:0") // nolint: noctx
+	require.NoError(t, err)
+	defer func() { _ = tcpListener.Close() }()
+
+	ra := netip.MustParseAddrPort(tcpListener.Addr().String())
+	logger := logging.NewDefaultLoggerFactory().NewLogger("ice")
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	a := newActiveTCPConn(ctx, "127.0.0.1:0", ra, logger)
+	require.NotNil(t, a)
+
+	srvConn, err := tcpListener.Accept()
+	require.NoError(t, err)
+
+	_ = srvConn.Close()
+
+	_, _ = a.WriteTo([]byte("data"), nil)
+
+	require.NoError(t, a.Close())
+}
+
+func TestActiveTCPConn_LocalAddr_DefaultWhenUnset(t *testing.T) {
+	defer test.CheckRoutines(t)()
+	defer test.TimeOut(3 * time.Second).Stop()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	invalidLocal := "127.0.0.1:65536"
+	remote := netip.MustParseAddrPort("127.0.0.1:1")
+
+	log := logging.NewDefaultLoggerFactory().NewLogger("ice")
+
+	a := newActiveTCPConn(ctx, invalidLocal, remote, log)
+	require.NotNil(t, a)
+	require.True(t, a.closed.Load(), "expected early-return closed state")
+
+	la := a.LocalAddr()
+	ta, ok := la.(*net.TCPAddr)
+	require.True(t, ok, "LocalAddr() should return *net.TCPAddr")
+	require.Nil(t, ta.IP, "fallback *net.TCPAddr should be zero value (nil IP)")
+	require.Equal(t, 0, ta.Port, "fallback *net.TCPAddr should be zero value (port 0)")
+	require.Equal(t, "", ta.Zone, "fallback *net.TCPAddr should be zero value (empty zone)")
+}
+
+func TestActiveTCPConn_SetDeadlines_ReturnEOF(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	invalidLocal := "127.0.0.1:65536"
+	remote := netip.MustParseAddrPort("127.0.0.1:1")
+	log := logging.NewDefaultLoggerFactory().NewLogger("ice")
+
+	a := newActiveTCPConn(ctx, invalidLocal, remote, log)
+	require.NotNil(t, a)
+	require.True(t, a.closed.Load(), "expected early-return closed state")
+
+	err := a.SetReadDeadline(time.Now())
+	require.ErrorIs(t, err, io.EOF)
+
+	err = a.SetWriteDeadline(time.Now())
+	require.ErrorIs(t, err, io.EOF)
+}

addr_test.go

@@ -0,0 +1,133 @@
+// SPDX-FileCopyrightText: 2023 The Pion community <https://pion.ly>
+// SPDX-License-Identifier: MIT
+
+//go:build !js
+// +build !js
+
+package ice
+
+import (
+	"net"
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// A net.Addr type that parseAddr doesn't handle.
+type unknownAddr struct{}
+
+func (unknownAddr) Network() string { return "unknown" }
+func (unknownAddr) String() string  { return "unknown-addr" }
+
+func TestParseAddrFromIface_ErrFromParseAddr(t *testing.T) {
+	in := unknownAddr{}
+	ip, port, nt, err := parseAddrFromIface(in, "eth0")
+
+	require.Error(t, err, "expected error from parseAddr for unknown net.Addr type")
+	require.Zero(t, port)
+	require.Zero(t, nt)
+	require.True(t, !ip.IsValid(), "ip should be zero value when error is returned")
+}
+
+func TestParseAddr_ErrorBranches(t *testing.T) {
+	t.Run("IPNet invalid IP -> error", func(t *testing.T) {
+		// length 1 slice -> ipAddrToNetIP fails
+		_, _, _, err := parseAddr(&net.IPNet{IP: net.IP{1}})
+		var convErr ipConvertError
+		require.ErrorAs(t, err, &convErr)
+	})
+
+	t.Run("IPAddr invalid IP -> error", func(t *testing.T) {
+		_, _, _, err := parseAddr(&net.IPAddr{IP: net.IP{1}, Zone: "eth0"})
+		var convErr ipConvertError
+		require.ErrorAs(t, err, &convErr)
+	})
+
+	t.Run("UDPAddr invalid IP -> error", func(t *testing.T) {
+		_, _, _, err := parseAddr(&net.UDPAddr{IP: net.IP{1}, Port: 3478})
+		var convErr ipConvertError
+		require.ErrorAs(t, err, &convErr)
+	})
+
+	t.Run("TCPAddr invalid IP -> error", func(t *testing.T) {
+		_, _, _, err := parseAddr(&net.TCPAddr{IP: net.IP{1}, Port: 3478})
+		var convErr ipConvertError
+		require.ErrorAs(t, err, &convErr)
+	})
+
+	t.Run("Unknown net.Addr type -> addrParseError", func(t *testing.T) {
+		_, _, _, err := parseAddr(unknownAddr{})
+		var ap addrParseError
+		require.ErrorAs(t, err, &ap)
+	})
+}
+
+func TestParseAddr_IPAddr_Success(t *testing.T) {
+	ip := net.ParseIP("fe80::1")
+	require.NotNil(t, ip)
+
+	gotIP, port, nt, err := parseAddr(&net.IPAddr{IP: ip, Zone: "lo0"})
+	require.NoError(t, err)
+	require.Equal(t, 0, port)
+	require.Equal(t, NetworkType(0), nt)
+	require.True(t, gotIP.Is6())
+	require.Equal(t, "lo0", gotIP.Zone())
+	require.Equal(t, 0, gotIP.Compare(netip.MustParseAddr("fe80::1%lo0").Unmap()))
+}
+
+func TestAddrParseError_Error(t *testing.T) {
+	e := addrParseError{addr: &net.TCPAddr{}}
+	require.Equal(t,
+		"do not know how to parse address type *net.TCPAddr",
+		e.Error(),
+	)
+}
+
+func TestIPConvertError_Error(t *testing.T) {
+	e := ipConvertError{ip: []byte("bad-ip")}
+	require.Equal(t,
+		"failed to convert IP 'bad-ip' to netip.Addr",
+		e.Error(),
+	)
+}
+
+func TestIPAddrToNetIP_Error_InvalidBytes(t *testing.T) {
+	bad := []byte{1} // invalid length -> AddrFromSlice returns ok=false
+	got, err := ipAddrToNetIP(bad, "")
+	require.Equal(t, netip.Addr{}, got, "should return zero addr on error")
+	require.Error(t, err)
+	require.IsType(t, ipConvertError{}, err)
+	require.Contains(t, err.Error(), "failed to convert IP")
+}
+
+func TestIPAddrToNetIP_OK_IPv4(t *testing.T) {
+	ipv4 := []byte{1, 2, 3, 4}
+	got, err := ipAddrToNetIP(ipv4, "")
+	require.NoError(t, err)
+	require.True(t, got.Is4())
+
+	want := netip.AddrFrom4([4]byte{1, 2, 3, 4})
+	require.Equal(t, want, got)
+}
+
+func TestAddrEqual_FirstParseError(t *testing.T) {
+	a := unknownAddr{}
+	b := &net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 9999}
+
+	require.False(t, addrEqual(a, b))
+}
+
+func TestAddrEqual_SecondParseError(t *testing.T) {
+	a := &net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 9999}
+	b := unknownAddr{}
+
+	require.False(t, addrEqual(a, b))
+}
+
+func TestAddrEqual_SameTypeIPPort(t *testing.T) {
+	a := &net.UDPAddr{IP: net.IPv4(10, 0, 0, 1), Port: 4242}
+	b := &net.UDPAddr{IP: net.IPv4(10, 0, 0, 1), Port: 4242}
+
+	require.True(t, addrEqual(a, b))
+}

agent_handlers_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/pion/transport/v3/test"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestConnectionStateNotifier(t *testing.T) {
@@ -70,3 +71,119 @@ func TestConnectionStateNotifier(t *testing.T) {
 		notifer.Close(true)
 	})
 }
+
+func TestHandlerNotifier_Close_AlreadyClosed(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	notifier := &handlerNotifier{
+		connectionStateFunc: func(ConnectionState) {},
+		candidateFunc:       func(Candidate) {},
+		candidatePairFunc:   func(*CandidatePair) {},
+		done:                make(chan struct{}),
+	}
+
+	// first close
+	notifier.Close(false)
+
+	isClosed := func(ch <-chan struct{}) bool {
+		select {
+		case <-ch:
+			return true
+		default:
+			return false
+		}
+	}
+	assert.True(t, isClosed(notifier.done), "expected h.done to be closed after first Close")
+
+	// second close should hit `case <-h.done` and return immediately
+	// without blocking on the WaitGroup.
+	finished := make(chan struct{}, 1)
+	go func() {
+		notifier.Close(true)
+		close(finished)
+	}()
+
+	assert.Eventually(t, func() bool {
+		select {
+		case <-finished:
+			return true
+		default:
+			return false
+		}
+	}, 250*time.Millisecond, 10*time.Millisecond, "second Close(true) did not return promptly")
+
+	// ensure still closed afterwards
+	assert.True(t, isClosed(notifier.done), "expected h.done to remain closed after second Close")
+
+	// sanity: no enqueues should start after close.
+	require.False(t, notifier.running)
+	require.Zero(t, len(notifier.connectionStates))
+	require.Zero(t, len(notifier.candidates))
+	require.Zero(t, len(notifier.selectedCandidatePairs))
+}
+
+func TestHandlerNotifier_EnqueueConnectionState_AfterClose(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	connCh := make(chan struct{}, 1)
+	notifier := &handlerNotifier{
+		connectionStateFunc: func(ConnectionState) { connCh <- struct{}{} },
+		done:                make(chan struct{}),
+	}
+
+	notifier.Close(false)
+	notifier.EnqueueConnectionState(ConnectionStateConnected)
+
+	assert.Never(t, func() bool {
+		select {
+		case <-connCh:
+			return true
+		default:
+			return false
+		}
+	}, 250*time.Millisecond, 10*time.Millisecond, "connectionStateFunc should not be called after close")
+}
+
+func TestHandlerNotifier_EnqueueCandidate_AfterClose(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	candidateCh := make(chan struct{}, 1)
+	h := &handlerNotifier{
+		candidateFunc: func(Candidate) { candidateCh <- struct{}{} },
+		done:          make(chan struct{}),
+	}
+
+	h.Close(false)
+	h.EnqueueCandidate(nil)
+
+	assert.Never(t, func() bool {
+		select {
+		case <-candidateCh:
+			return true
+		default:
+			return false
+		}
+	}, 250*time.Millisecond, 10*time.Millisecond, "candidateFunc should not be called after close")
+}
+
+func TestHandlerNotifier_EnqueueSelectedCandidatePair_AfterClose(t *testing.T) {
+	defer test.CheckRoutines(t)()
+
+	pairCh := make(chan struct{}, 1)
+	h := &handlerNotifier{
+		candidatePairFunc: func(*CandidatePair) { pairCh <- struct{}{} },
+		done:              make(chan struct{}),
+	}
+
+	h.Close(false)
+	h.EnqueueSelectedCandidatePair(nil)
+
+	assert.Never(t, func() bool {
+		select {
+		case <-pairCh:
+			return true
+		default:
+			return false
+		}
+	}, 250*time.Millisecond, 10*time.Millisecond, "candidatePairFunc should not be called after close")
+}