Skip to content

Commit ee8ecb7

Browse files
matovalmatoval
committed
Fix test isolation and CI compatibility issues
Address code review feedback to improve test reliability and prevent resource leaks. Changes: - Fix global MainInstance leaks by using cancellable contexts - Restore original MainInstance in defer to prevent test pollution - Use ephemeral ports (Port: 0) for UDP tests to avoid CI collisions - Use t.TempDir() for unique Unix socket paths per subtest - Add conditional skip for IPv6 tests when IPv6 is unavailable - Fix octal literal formatting (0600 -> 0o600) per gofumpt All tests pass with proper cleanup and isolation. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 19d03d3 commit ee8ecb7

File tree

3 files changed

+109
-39
lines changed

3 files changed

+109
-39
lines changed

‎pkg/services/command_test.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,14 @@ func TestCommandSvcCfgRun(t *testing.T) {
137137
},
138138
}
139139

140-
netceptor.MainInstance = netceptor.New(context.Background(), "test_command_svc_cfg_run")
140+
// Save original instance and create cancellable context
141+
originalInstance := netceptor.MainInstance
142+
ctx, cancel := context.WithCancel(context.Background())
143+
netceptor.MainInstance = netceptor.New(ctx, "test_command_svc_cfg_run")
144+
defer func() {
145+
cancel()
146+
netceptor.MainInstance = originalInstance
147+
}()
141148

142149
for _, tc := range testCases {
143150
t.Run(tc.name, func(t *testing.T) {

‎pkg/services/udp_proxy_test.go‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"errors"
66
"fmt"
77
"net"
8+
"os"
9+
"syscall"
810
"testing"
911

1012
"github.com/ansible/receptor/pkg/logger"
@@ -539,6 +541,16 @@ func TestNetUDPWrapper_ListenUDP(t *testing.T) {
539541
addr := &net.UDPAddr{IP: net.IPv6loopback, Port: 0}
540542
conn, err := wrapper.ListenUDP("udp", addr)
541543
if err != nil {
544+
// Skip if IPv6 is not available (check for specific syscall errors)
545+
if opErr, ok := err.(*net.OpError); ok {
546+
if sysErr, ok := opErr.Err.(*os.SyscallError); ok {
547+
if errno, ok := sysErr.Err.(syscall.Errno); ok {
548+
if errno == syscall.EAFNOSUPPORT || errno == syscall.EADDRNOTAVAIL {
549+
t.Skipf("IPv6 not available: %v", err)
550+
}
551+
}
552+
}
553+
}
542554
t.Fatalf("expected no error, got %v", err)
543555
}
544556
if conn == nil {
@@ -593,7 +605,7 @@ func TestUDPProxyInboundCfgRun(t *testing.T) {
593605
{
594606
name: "Valid UDP proxy inbound configuration",
595607
configObj: UDPProxyInboundCfg{
596-
Port: 8080,
608+
Port: 0, // Use ephemeral port
597609
BindAddr: "127.0.0.1",
598610
RemoteNode: "node1",
599611
RemoteService: "service1",
@@ -602,15 +614,22 @@ func TestUDPProxyInboundCfgRun(t *testing.T) {
602614
{
603615
name: "Valid UDP proxy inbound with default bind address",
604616
configObj: UDPProxyInboundCfg{
605-
Port: 8081,
617+
Port: 0, // Use ephemeral port
606618
BindAddr: "0.0.0.0",
607619
RemoteNode: "node2",
608620
RemoteService: "service2",
609621
},
610622
},
611623
}
612624

613-
netceptor.MainInstance = netceptor.New(context.Background(), "test_udp_proxy_inbound_cfg_run")
625+
// Save original instance and create cancellable context
626+
originalInstance := netceptor.MainInstance
627+
ctx, cancel := context.WithCancel(context.Background())
628+
netceptor.MainInstance = netceptor.New(ctx, "test_udp_proxy_inbound_cfg_run")
629+
defer func() {
630+
cancel()
631+
netceptor.MainInstance = originalInstance
632+
}()
614633

615634
for _, tc := range testCases {
616635
t.Run(tc.name, func(t *testing.T) {
@@ -650,7 +669,14 @@ func TestUDPProxyOutboundCfgRun(t *testing.T) {
650669
},
651670
}
652671

653-
netceptor.MainInstance = netceptor.New(context.Background(), "test_udp_proxy_outbound_cfg_run")
672+
// Save original instance and create cancellable context
673+
originalInstance := netceptor.MainInstance
674+
ctx, cancel := context.WithCancel(context.Background())
675+
netceptor.MainInstance = netceptor.New(ctx, "test_udp_proxy_outbound_cfg_run")
676+
defer func() {
677+
cancel()
678+
netceptor.MainInstance = originalInstance
679+
}()
654680

655681
for _, tc := range testCases {
656682
t.Run(tc.name, func(t *testing.T) {

‎pkg/services/unix_proxy_test.go‎

Lines changed: 71 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -83,50 +83,69 @@ func TestUnixProxyInboundCfgRun(t *testing.T) {
8383
name string
8484
expectError bool
8585
expectedErrorMessage string
86-
configObj UnixProxyInboundCfg
86+
getConfigObj func(sockPath string) UnixProxyInboundCfg
8787
}
8888

8989
testCases := []testCase{
9090
{
9191
name: "Valid unix proxy inbound configuration",
92-
configObj: UnixProxyInboundCfg{
93-
Filename: "/tmp/test-receptor-unix-inbound.sock",
94-
Permissions: 0600,
95-
RemoteNode: "node1",
96-
RemoteService: "service1",
92+
getConfigObj: func(sockPath string) UnixProxyInboundCfg {
93+
return UnixProxyInboundCfg{
94+
Filename: sockPath,
95+
Permissions: 0o600,
96+
RemoteNode: "node1",
97+
RemoteService: "service1",
98+
}
9799
},
98100
},
99101
{
100102
name: "Valid unix proxy inbound with custom permissions",
101-
configObj: UnixProxyInboundCfg{
102-
Filename: "/tmp/test-receptor-unix-inbound2.sock",
103-
Permissions: 0660,
104-
RemoteNode: "node2",
105-
RemoteService: "service2",
103+
getConfigObj: func(sockPath string) UnixProxyInboundCfg {
104+
return UnixProxyInboundCfg{
105+
Filename: sockPath,
106+
Permissions: 0o660,
107+
RemoteNode: "node2",
108+
RemoteService: "service2",
109+
}
106110
},
107111
},
108112
{
109113
name: "Invalid TLS configuration",
110114
expectError: true,
111115
expectedErrorMessage: "unknown TLS config invalid-tls",
112-
configObj: UnixProxyInboundCfg{
113-
Filename: "/tmp/test-receptor-unix-inbound3.sock",
114-
Permissions: 0600,
115-
RemoteNode: "node3",
116-
RemoteService: "service3",
117-
TLS: "invalid-tls",
116+
getConfigObj: func(sockPath string) UnixProxyInboundCfg {
117+
return UnixProxyInboundCfg{
118+
Filename: sockPath,
119+
Permissions: 0o600,
120+
RemoteNode: "node3",
121+
RemoteService: "service3",
122+
TLS: "invalid-tls",
123+
}
118124
},
119125
},
120126
}
121127

122-
netceptor.MainInstance = netceptor.New(context.Background(), "test_unix_proxy_inbound_cfg_run")
128+
// Save original instance and create cancellable context
129+
originalInstance := netceptor.MainInstance
130+
ctx, cancel := context.WithCancel(context.Background())
131+
netceptor.MainInstance = netceptor.New(ctx, "test_unix_proxy_inbound_cfg_run")
132+
defer func() {
133+
cancel()
134+
netceptor.MainInstance = originalInstance
135+
}()
123136

124137
for _, tc := range testCases {
125138
t.Run(tc.name, func(t *testing.T) {
126-
// Clean up socket file if it exists
127-
defer os.Remove(tc.configObj.Filename)
139+
// Create unique socket path for this subtest
140+
tmpDir := t.TempDir()
141+
sockPath := tmpDir + "/test.sock"
142+
configObj := tc.getConfigObj(sockPath)
143+
144+
// Clean up socket file if it exists before and after
145+
os.Remove(sockPath)
146+
defer os.Remove(sockPath)
128147

129-
err := tc.configObj.Run()
148+
err := configObj.Run()
130149
if tc.expectError {
131150
if err == nil {
132151
t.Error("expected error but got nil")
@@ -145,41 +164,59 @@ func TestUnixProxyOutboundCfgRun(t *testing.T) {
145164
name string
146165
expectError bool
147166
expectedErrorMessage string
148-
configObj UnixProxyOutboundCfg
167+
getConfigObj func(sockPath string) UnixProxyOutboundCfg
149168
}
150169

151170
testCases := []testCase{
152171
{
153172
name: "Valid unix proxy outbound configuration",
154-
configObj: UnixProxyOutboundCfg{
155-
Service: "unix1",
156-
Filename: "/tmp/test-receptor-unix-outbound.sock",
173+
getConfigObj: func(sockPath string) UnixProxyOutboundCfg {
174+
return UnixProxyOutboundCfg{
175+
Service: "unix1",
176+
Filename: sockPath,
177+
}
157178
},
158179
},
159180
{
160181
name: "Valid unix proxy outbound with different socket",
161-
configObj: UnixProxyOutboundCfg{
162-
Service: "unix2",
163-
Filename: "/tmp/test-receptor-unix-outbound2.sock",
182+
getConfigObj: func(sockPath string) UnixProxyOutboundCfg {
183+
return UnixProxyOutboundCfg{
184+
Service: "unix2",
185+
Filename: sockPath,
186+
}
164187
},
165188
},
166189
{
167190
name: "Invalid TLS configuration",
168191
expectError: true,
169192
expectedErrorMessage: "unknown TLS config invalid-tls",
170-
configObj: UnixProxyOutboundCfg{
171-
Service: "unix3",
172-
Filename: "/tmp/test-receptor-unix-outbound3.sock",
173-
TLS: "invalid-tls",
193+
getConfigObj: func(sockPath string) UnixProxyOutboundCfg {
194+
return UnixProxyOutboundCfg{
195+
Service: "unix3",
196+
Filename: sockPath,
197+
TLS: "invalid-tls",
198+
}
174199
},
175200
},
176201
}
177202

178-
netceptor.MainInstance = netceptor.New(context.Background(), "test_unix_proxy_outbound_cfg_run")
203+
// Save original instance and create cancellable context
204+
originalInstance := netceptor.MainInstance
205+
ctx, cancel := context.WithCancel(context.Background())
206+
netceptor.MainInstance = netceptor.New(ctx, "test_unix_proxy_outbound_cfg_run")
207+
defer func() {
208+
cancel()
209+
netceptor.MainInstance = originalInstance
210+
}()
179211

180212
for _, tc := range testCases {
181213
t.Run(tc.name, func(t *testing.T) {
182-
err := tc.configObj.Run()
214+
// Create unique socket path for this subtest
215+
tmpDir := t.TempDir()
216+
sockPath := tmpDir + "/test.sock"
217+
configObj := tc.getConfigObj(sockPath)
218+
219+
err := configObj.Run()
183220
if tc.expectError {
184221
if err == nil {
185222
t.Error("expected error but got nil")

0 commit comments

Comments
 (0)