Skip to content

Add test coverage for NetUDPWrapper methods in pkg/services #1524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fincamd wants to merge 1 commit into
base: devel
Choose a base branch
from
+57 −0
Open

Add test coverage for NetUDPWrapper methods in pkg/services #1524

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions pkg/services/udp_proxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,63 @@
}
}

func TestNetUDPWrapper(t *testing.T) {

@fincamd fincamd Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decide whether these tests can run in parallel with t.parallel() and then, if we do use t.Fatal() instead of t.Errorf().

Try to convert this in to table-driven testing so that we don't have several t.Run() tasks

wrapper := &NetUDPWrapper{}

t.Run("ResolveUDPAddr - valid address", func(t *testing.T) {
addr, err := wrapper.ResolveUDPAddr("udp", "127.0.0.1:8080")
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if addr == nil {

Check failure on line 387 in pkg/services/udp_proxy_test.go

View workflow job for this annotation

GitHub Actions / lint-receptor 

SA5011(related information): this check suggests that the pointer can be nil (staticcheck)
t.Error("Expected non-nil address")
}
if addr.Port != 8080 {

Check failure on line 390 in pkg/services/udp_proxy_test.go

View workflow job for this annotation

GitHub Actions / lint-receptor 

SA5011: possible nil pointer dereference (staticcheck)
t.Errorf("Expected port 8080, got %d", addr.Port)
}
})
Comment on lines +382 to +393

@coderabbitai coderabbitai Bot Feb 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Use t.Fatal to stop execution after the nil check, preventing a nil pointer dereference.

t.Error logs a failure but does not stop the test, so execution falls through to addr.Port on line 390 even when addr is nil, causing a panic. The staticcheck tool correctly flags this (SA5011).

🐛 Proposed fix 
 	addr, err := wrapper.ResolveUDPAddr("udp", "127.0.0.1:8080")
 	if err != nil {
 		t.Errorf("Expected no error, got %v", err)
 	}
 	if addr == nil {
-		t.Error("Expected non-nil address")
+		t.Fatal("Expected non-nil address")
 	}
 	if addr.Port != 8080 {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
t.Run("ResolveUDPAddr - valid address", func(t *testing.T) {
addr, err := wrapper.ResolveUDPAddr("udp", "127.0.0.1:8080")
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if addr == nil {
t.Error("Expected non-nil address")
}
if addr.Port != 8080 {
t.Errorf("Expected port 8080, got %d", addr.Port)
}
})
t.Run("ResolveUDPAddr - valid address", func(t *testing.T) {
addr, err := wrapper.ResolveUDPAddr("udp", "127.0.0.1:8080")
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if addr == nil {
t.Fatal("Expected non-nil address")
}
if addr.Port != 8080 {
t.Errorf("Expected port 8080, got %d", addr.Port)
}
})
🧰 Tools
🪛 GitHub Check: lint-receptor

[failure] 387-387:
SA5011(related information): this check suggests that the pointer can be nil (staticcheck)

[failure] 390-390:
SA5011: possible nil pointer dereference (staticcheck)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/services/udp_proxy_test.go` around lines 382 - 393, The test's
nil-address check uses t.Error which doesn't stop execution, allowing a nil
dereference when accessing addr.Port; in the "ResolveUDPAddr - valid address"
t.Run block, replace the addr nil check to use t.Fatal (or call return after
logging) so the test stops immediately when addr is nil before touching
addr.Port; locate the wrapper.ResolveUDPAddr invocation and the addr variable in
that test and change the nil-check to t.Fatal to prevent the SA5011 panic.


t.Run("ResolveUDPAddr - invalid address", func(t *testing.T) {
_, err := wrapper.ResolveUDPAddr("udp", "invalid:address:format")
if err == nil {
t.Error("Expected error for invalid address, got nil")
}
})

t.Run("ListenUDP - bind to localhost", func(t *testing.T) {
addr := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}
conn, err := wrapper.ListenUDP("udp", addr)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if conn != nil {
defer conn.Close()
}
})

t.Run("DialUDP - dial localhost", func(t *testing.T) {
// First, create a listener to dial to
listenerAddr := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}
listener, err := wrapper.ListenUDP("udp", listenerAddr)
if err != nil {
t.Fatalf("Failed to create listener: %v", err)
}
defer listener.Close()

// Get the actual port the listener is using
localAddr := listener.LocalAddr().(*net.UDPAddr)

// Now dial to that address
conn, err := wrapper.DialUDP("udp", nil, localAddr)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if conn != nil {
defer conn.Close()
}
Comment on lines +402 to +432

@coderabbitai coderabbitai Bot Feb 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing non-nil assertion for conn after a successful call in both ListenUDP and DialUDP subtests.

if conn != nil { defer conn.Close() } silently passes if conn is nil without an error. A successful call should always produce a non-nil connection; the test should assert that explicitly.

🛡️ Proposed fix (apply the same pattern to both subtests) 
 	conn, err := wrapper.ListenUDP("udp", addr)
 	if err != nil {
 		t.Errorf("Expected no error, got %v", err)
 	}
-	if conn != nil {
-		defer conn.Close()
+	if conn == nil {
+		t.Fatal("Expected non-nil connection")
 	}
+	defer conn.Close()
 	conn, err := wrapper.DialUDP("udp", nil, localAddr)
 	if err != nil {
 		t.Errorf("Expected no error, got %v", err)
 	}
-	if conn != nil {
-		defer conn.Close()
+	if conn == nil {
+		t.Fatal("Expected non-nil connection")
 	}
+	defer conn.Close()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
t.Run("ListenUDP - bind to localhost", func(t *testing.T) {
addr := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}
conn, err := wrapper.ListenUDP("udp", addr)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if conn != nil {
defer conn.Close()
}
})
t.Run("DialUDP - dial localhost", func(t *testing.T) {
// First, create a listener to dial to
listenerAddr := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}
listener, err := wrapper.ListenUDP("udp", listenerAddr)
if err != nil {
t.Fatalf("Failed to create listener: %v", err)
}
defer listener.Close()
// Get the actual port the listener is using
localAddr := listener.LocalAddr().(*net.UDPAddr)
// Now dial to that address
conn, err := wrapper.DialUDP("udp", nil, localAddr)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if conn != nil {
defer conn.Close()
}
t.Run("ListenUDP - bind to localhost", func(t *testing.T) {
addr := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}
conn, err := wrapper.ListenUDP("udp", addr)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if conn == nil {
t.Fatal("Expected non-nil connection")
}
defer conn.Close()
})
t.Run("DialUDP - dial localhost", func(t *testing.T) {
// First, create a listener to dial to
listenerAddr := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}
listener, err := wrapper.ListenUDP("udp", listenerAddr)
if err != nil {
t.Fatalf("Failed to create listener: %v", err)
}
defer listener.Close()
// Get the actual port the listener is using
localAddr := listener.LocalAddr().(*net.UDPAddr)
// Now dial to that address
conn, err := wrapper.DialUDP("udp", nil, localAddr)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
if conn == nil {
t.Fatal("Expected non-nil connection")
}
defer conn.Close()
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/services/udp_proxy_test.go` around lines 402 - 432, The tests for
wrapper.ListenUDP and wrapper.DialUDP currently only defer conn.Close() when
conn != nil, which hides a nil-conn success case; after each call to
wrapper.ListenUDP and wrapper.DialUDP assert that conn is non-nil (e.g., if conn
== nil { t.Fatalf("expected non-nil conn from ListenUDP/DialUDP") }) and only
then defer conn.Close(); reference the conn variable and the wrapper.ListenUDP /
wrapper.DialUDP calls when making this change so both subtests have the explicit
non-nil check and proper defer.

})
}

func TestRunUDPProxyServiceOutbound(t *testing.T) {
var mockNetceptor *mock_services.MockNetcForUDPProxy
var mockNetter *mock_net_interface.MockNetterUDP
Expand Down
Loading