Repository: pion/ice
Pull request #564 (2023-05-07 15:03:14)

Extract TestOnSelectedCandidatePairChange to its own file
Also made refactoring of the test.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pion/ice/pull/564?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Patch coverage has no change and project coverage change: **`-53.19`** :warning:
> Comparison is base [(`8c5b099`)](https://app.codecov.io/gh/pion/ice/commit/8c5b0991ef3bb070e47afda96faf090e8bf94be6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 78.18% compared to head [(`f0898bd`)](https://app.codecov.io/gh/pion/ice/pull/564?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 25.00%.

> :exclamation: Current head f0898bd differs from pull request most recent head 0f61764. Consider uploading reports for the commit 0f61764 to get more accurate results

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           master     #564       +/-   ##
===========================================
- Coverage   78.18%   25.00%   -53.19%     
===========================================
  Files          40       37        -3     
  Lines        4291     4244       -47     
===========================================
- Hits         3355     1061     -2294     
- Misses        726     3143     +2417     
+ Partials      210       40      -170     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `25.00% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

[see 28 files with indirect coverage changes](https://app.codecov.io/gh/pion/ice/pull/564/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pion/ice/pull/564?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/internal/fakenet/mock_conn.go b/internal/fakenet/mock_conn.go
new file mode 100644
index 0000000..cc98849
--- /dev/null
+++ b/internal/fakenet/mock_conn.go
@@ -0,0 +1,23 @@
+// SPDX-FileCopyrightText: 2023 The Pion community <https://pion.ly>
+// SPDX-License-Identifier: MIT
+
+//go:build !js
+// +build !js
+
+package fakenet
+
+import (
+	"net"
+	"time"
+)
+
+// MockPacketConn for tests
+type MockPacketConn struct{}
+
+func (m *MockPacketConn) ReadFrom([]byte) (n int, addr net.Addr, err error) { return 0, nil, nil } //nolint:revive
+func (m *MockPacketConn) WriteTo([]byte, net.Addr) (n int, err error)       { return 0, nil }      //nolint:revive
+func (m *MockPacketConn) Close() error                                      { return nil }         //nolint:revive
+func (m *MockPacketConn) LocalAddr() net.Addr                               { return nil }         //nolint:revive
+func (m *MockPacketConn) SetDeadline(time.Time) error                       { return nil }         //nolint:revive
+func (m *MockPacketConn) SetReadDeadline(time.Time) error                   { return nil }         //nolint:revive
+func (m *MockPacketConn) SetWriteDeadline(time.Time) error                  { return nil }         //nolint:revive


Test patch:
diff --git a/agent_get_best_valid_candidate_pair_test.go b/agent_get_best_valid_candidate_pair_test.go
index 57fac81..1bb6265 100644
--- a/agent_get_best_valid_candidate_pair_test.go
+++ b/agent_get_best_valid_candidate_pair_test.go
@@ -55,69 +55,3 @@ type TestAgentGetBestValidCandidatePairFixture struct {
 	prflxRemote Candidate
 	hostRemote  Candidate
 }
-
-func newHostRemote(t *testing.T) *CandidateHost {
-	remoteHostConfig := &CandidateHostConfig{
-		Network:   "udp",
-		Address:   "1.2.3.5",
-		Port:      12350,
-		Component: 1,
-	}
-	hostRemote, err := NewCandidateHost(remoteHostConfig)
-	require.NoError(t, err)
-	return hostRemote
-}
-
-func newPrflxRemote(t *testing.T) *CandidatePeerReflexive {
-	prflxConfig := &CandidatePeerReflexiveConfig{
-		Network:   "udp",
-		Address:   "10.10.10.2",
-		Port:      19217,
-		Component: 1,
-		RelAddr:   "4.3.2.1",
-		RelPort:   43211,
-	}
-	prflxRemote, err := NewCandidatePeerReflexive(prflxConfig)
-	require.NoError(t, err)
-	return prflxRemote
-}
-
-func newSrflxRemote(t *testing.T) *CandidateServerReflexive {
-	srflxConfig := &CandidateServerReflexiveConfig{
-		Network:   "udp",
-		Address:   "10.10.10.2",
-		Port:      19218,
-		Component: 1,
-		RelAddr:   "4.3.2.1",
-		RelPort:   43212,
-	}
-	srflxRemote, err := NewCandidateServerReflexive(srflxConfig)
-	require.NoError(t, err)
-	return srflxRemote
-}
-
-func newRelayRemote(t *testing.T) *CandidateRelay {
-	relayConfig := &CandidateRelayConfig{
-		Network:   "udp",
-		Address:   "1.2.3.4",
-		Port:      12340,
-		Component: 1,
-		RelAddr:   "4.3.2.1",
-		RelPort:   43210,
-	}
-	relayRemote, err := NewCandidateRelay(relayConfig)
-	require.NoError(t, err)
-	return relayRemote
-}
-
-func newHostLocal(t *testing.T) *CandidateHost {
-	localHostConfig := &CandidateHostConfig{
-		Network:   "udp",
-		Address:   "192.168.1.1",
-		Port:      19216,
-		Component: 1,
-	}
-	hostLocal, err := NewCandidateHost(localHostConfig)
-	require.NoError(t, err)
-	return hostLocal
-}
diff --git a/agent_on_selected_candidate_pair_change_test.go b/agent_on_selected_candidate_pair_change_test.go
new file mode 100644
index 0000000..78cb4d5
--- /dev/null
+++ b/agent_on_selected_candidate_pair_change_test.go
@@ -0,0 +1,48 @@
+// SPDX-FileCopyrightText: 2023 The Pion community <https://pion.ly>
+// SPDX-License-Identifier: MIT
+
+//go:build !js
+// +build !js
+
+package ice
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestOnSelectedCandidatePairChange(t *testing.T) {
+	agent, candidatePair := fixtureTestOnSelectedCandidatePairChange(t)
+
+	callbackCalled := make(chan struct{}, 1)
+	err := agent.OnSelectedCandidatePairChange(func(local, remote Candidate) {
+		close(callbackCalled)
+	})
+	require.NoError(t, err)
+
+	err = agent.run(context.Background(), func(ctx context.Context, agent *Agent) {
+		agent.setSelectedPair(candidatePair)
+	})
+	require.NoError(t, err)
+
+	<-callbackCalled
+	require.NoError(t, agent.Close())
+}
+
+func fixtureTestOnSelectedCandidatePairChange(t *testing.T) (*Agent, *CandidatePair) {
+	agent, err := NewAgent(&AgentConfig{})
+	require.NoError(t, err)
+
+	candidatePair := makeCandidatePair(t)
+	return agent, candidatePair
+}
+
+func makeCandidatePair(t *testing.T) *CandidatePair {
+	hostLocal := newHostLocal(t)
+	relayRemote := newRelayRemote(t)
+
+	candidatePair := newCandidatePair(hostLocal, relayRemote, false)
+	return candidatePair
+}
diff --git a/agent_test.go b/agent_test.go
index 198ad4b..b7d6dea 100644
--- a/agent_test.go
+++ b/agent_test.go
@@ -16,6 +16,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pion/ice/v2/internal/fakenet"
 	"github.com/pion/logging"
 	"github.com/pion/stun"
 	"github.com/pion/transport/v2/test"
@@ -24,72 +25,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-type mockPacketConn struct{}
-
-func (m *mockPacketConn) ReadFrom([]byte) (n int, addr net.Addr, err error) { return 0, nil, nil }
-func (m *mockPacketConn) WriteTo([]byte, net.Addr) (n int, err error)       { return 0, nil }
-func (m *mockPacketConn) Close() error                                      { return nil }
-func (m *mockPacketConn) LocalAddr() net.Addr                               { return nil }
-func (m *mockPacketConn) SetDeadline(time.Time) error                       { return nil }
-func (m *mockPacketConn) SetReadDeadline(time.Time) error                   { return nil }
-func (m *mockPacketConn) SetWriteDeadline(time.Time) error                  { return nil }
-
-func TestOnSelectedCandidatePairChange(t *testing.T) {
-	report := test.CheckRoutines(t)
-	defer report()
-
-	// Avoid deadlocks?
-	defer test.TimeOut(1 * time.Second).Stop()
-
-	a, err := NewAgent(&AgentConfig{})
-	if err != nil {
-		t.Fatalf("Failed to create agent: %s", err)
-	}
-
-	callbackCalled := make(chan struct{}, 1)
-	if err = a.OnSelectedCandidatePairChange(func(local, remote Candidate) {
-		close(callbackCalled)
-	}); err != nil {
-		t.Fatalf("Failed to set agent OnCandidatePairChange callback: %s", err)
-	}
-
-	hostConfig := &CandidateHostConfig{
-		Network:   "udp",
-		Address:   "192.168.1.1",
-		Port:      19216,
-		Component: 1,
-	}
-	hostLocal, err := NewCandidateHost(hostConfig)
-	if err != nil {
-		t.Fatalf("Failed to construct local host candidate: %s", err)
-	}
-
-	relayConfig := &CandidateRelayConfig{
-		Network:   "udp",
-		Address:   "1.2.3.4",
-		Port:      12340,
-		Component: 1,
-		RelAddr:   "4.3.2.1",
-		RelPort:   43210,
-	}
-	relayRemote, err := NewCandidateRelay(relayConfig)
-	if err != nil {
-		t.Fatalf("Failed to construct remote relay candidate: %s", err)
-	}
-
-	// Select the pair
-	if err = a.run(context.Background(), func(ctx context.Context, agent *Agent) {
-		p := newCandidatePair(hostLocal, relayRemote, false)
-		agent.setSelectedPair(p)
-	}); err != nil {
-		t.Fatalf("Failed to setValidPair(): %s", err)
-	}
-
-	// Ensure that the callback fired on setting the pair
-	<-callbackCalled
-	assert.NoError(t, a.Close())
-}
-
 type BadAddr struct{}
 
 func (ba *BadAddr) Network() string {
@@ -133,7 +68,7 @@ func TestHandlePeerReflexive(t *testing.T) {
 				Component: 1,
 			}
 			local, err := NewCandidateHost(&hostConfig)
-			local.conn = &mockPacketConn{}
+			local.conn = &fakenet.MockPacketConn{}
 			if err != nil {
 				t.Fatalf("failed to create a new candidate: %v", err)
 			}
@@ -224,7 +159,7 @@ func TestHandlePeerReflexive(t *testing.T) {
 				Component: 1,
 			}
 			local, err := NewCandidateHost(&hostConfig)
-			local.conn = &mockPacketConn{}
+			local.conn = &fakenet.MockPacketConn{}
 			if err != nil {
 				t.Fatalf("failed to create a new candidate: %v", err)
 			}
@@ -445,7 +380,7 @@ func TestInboundValidity(t *testing.T) {
 		Component: 1,
 	}
 	local, err := NewCandidateHost(&hostConfig)
-	local.conn = &mockPacketConn{}
+	local.conn = &fakenet.MockPacketConn{}
 	if err != nil {
 		t.Fatalf("failed to create a new candidate: %v", err)
 	}
@@ -547,7 +482,7 @@ func TestInboundValidity(t *testing.T) {
 			Component: 1,
 		}
 		local, err := NewCandidateHost(&hostConfig)
-		local.conn = &mockPacketConn{}
+		local.conn = &fakenet.MockPacketConn{}
 		if err != nil {
 			t.Fatalf("failed to create a new candidate: %v", err)
 		}
diff --git a/test_utils.go b/test_utils.go
new file mode 100644
index 0000000..235fda3
--- /dev/null
+++ b/test_utils.go
@@ -0,0 +1,79 @@
+// SPDX-FileCopyrightText: 2023 The Pion community <https://pion.ly>
+// SPDX-License-Identifier: MIT
+
+//go:build !js
+// +build !js
+
+package ice
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func newHostRemote(t *testing.T) *CandidateHost {
+	remoteHostConfig := &CandidateHostConfig{
+		Network:   "udp",
+		Address:   "1.2.3.5",
+		Port:      12350,
+		Component: 1,
+	}
+	hostRemote, err := NewCandidateHost(remoteHostConfig)
+	require.NoError(t, err)
+	return hostRemote
+}
+
+func newPrflxRemote(t *testing.T) *CandidatePeerReflexive {
+	prflxConfig := &CandidatePeerReflexiveConfig{
+		Network:   "udp",
+		Address:   "10.10.10.2",
+		Port:      19217,
+		Component: 1,
+		RelAddr:   "4.3.2.1",
+		RelPort:   43211,
+	}
+	prflxRemote, err := NewCandidatePeerReflexive(prflxConfig)
+	require.NoError(t, err)
+	return prflxRemote
+}
+
+func newSrflxRemote(t *testing.T) *CandidateServerReflexive {
+	srflxConfig := &CandidateServerReflexiveConfig{
+		Network:   "udp",
+		Address:   "10.10.10.2",
+		Port:      19218,
+		Component: 1,
+		RelAddr:   "4.3.2.1",
+		RelPort:   43212,
+	}
+	srflxRemote, err := NewCandidateServerReflexive(srflxConfig)
+	require.NoError(t, err)
+	return srflxRemote
+}
+
+func newRelayRemote(t *testing.T) *CandidateRelay {
+	relayConfig := &CandidateRelayConfig{
+		Network:   "udp",
+		Address:   "1.2.3.4",
+		Port:      12340,
+		Component: 1,
+		RelAddr:   "4.3.2.1",
+		RelPort:   43210,
+	}
+	relayRemote, err := NewCandidateRelay(relayConfig)
+	require.NoError(t, err)
+	return relayRemote
+}
+
+func newHostLocal(t *testing.T) *CandidateHost {
+	localHostConfig := &CandidateHostConfig{
+		Network:   "udp",
+		Address:   "192.168.1.1",
+		Port:      19216,
+		Component: 1,
+	}
+	hostLocal, err := NewCandidateHost(localHostConfig)
+	require.NoError(t, err)
+	return hostLocal
+}