Skip to content

test: stabilize unix socket test paths #90

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

Closed
YuminosukeSato wants to merge 1 commit into from
Closed

test: stabilize unix socket test paths #90

Show file tree
Hide file tree
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
13 changes: 4 additions & 9 deletions pkg/pyproc/connection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@ package pyproc
import (
"context"
"net"
"path/filepath"
"testing"
"time"
)

func TestConnectToWorker_Success(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "test.sock")
socketPath := tempSocketPath(t, "connect-ok")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand All @@ -36,8 +34,7 @@ func TestConnectToWorker_Success(t *testing.T) {

func TestConnectToWorker_Timeout(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "nonexistent.sock")
socketPath := tempSocketPath(t, "connect-timeout")

start := time.Now()
_, err := ConnectToWorker(socketPath, 200*time.Millisecond)
Expand Down Expand Up @@ -104,8 +101,7 @@ func TestSleepWithCtx_ContextAlreadyCanceled(t *testing.T) {

func TestConnectToWorker_TimeoutDuringSleep(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "nonexistent.sock")
socketPath := tempSocketPath(t, "connect-timeout-sleep")

// Use very short timeout to ensure timeout occurs during sleep
// The timeout must be shorter than defaultSleepDuration (100ms) but long enough
Expand All @@ -129,8 +125,7 @@ func TestConnectToWorker_TimeoutDuringSleep(t *testing.T) {

func TestConnectToWorker_TimeoutAfterSleep(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "nonexistent.sock")
socketPath := tempSocketPath(t, "connect-timeout-after")

// Use timeout longer than one sleep cycle (100ms) to ensure at least one
// sleep completes successfully. Try multiple times to increase likelihood
Expand Down
7 changes: 6 additions & 1 deletion pkg/pyproc/pool_external_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,19 @@ package pyproc
import (
"net"
"os"
"path/filepath"
"testing"
"time"
)

// startTestSocket creates a temporary Unix socket listener for external pool tests.
func startTestSocket(t *testing.T) (net.Listener, string) {
t.Helper()
f, err := os.CreateTemp("/tmp", "pyproc-ext-pool-*.sock")
baseDir := filepath.Join(os.TempDir(), "pyproc")
if err := os.MkdirAll(baseDir, 0755); err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
f, err := os.CreateTemp(baseDir, "pyproc-ext-pool-*.sock")
Comment on lines +14 to +18
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

Creating a predictable shared temp directory with permissions 0755 in a potentially shared temp location can be avoided in tests by using a per-process/per-test directory (os.MkdirTemp or t.TempDir) when possible, or by using more restrictive permissions (commonly 0700) for user-owned temp dirs. If the shared dir is needed to keep paths short, consider tightening permissions and/or centralizing this logic via the same helper used elsewhere to reduce duplication across tests.

Copilot uses AI. Check for mistakes.
if err != nil {
t.Fatalf("failed to create temp file: %v", err)
}
Expand Down
27 changes: 16 additions & 11 deletions pkg/pyproc/pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,23 @@
if cfg.MaxInFlight == 0 {
cfg.MaxInFlight = 1
}
if cfg.MaxInFlightPerWorker == 0 {

Check failure on line 57 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

cfg.MaxInFlightPerWorker undefined (type PoolConfig has no field or method MaxInFlightPerWorker)
cfg.MaxInFlightPerWorker = 1

Check failure on line 58 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

cfg.MaxInFlightPerWorker undefined (type PoolConfig has no field or method MaxInFlightPerWorker)
Comment on lines +57 to +58
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 6, 2026

Choose a reason for hiding this comment

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

P1 Badge Remove references to non-existent pool concurrency fields

This helper now assigns cfg.MaxInFlightPerWorker and initializes workerAvailable, shutdownCh, and inflightGate, but those members are not defined in this commit’s PoolConfig (pkg/pyproc/config.go) or Pool/poolWorker (pkg/pyproc/pool.go). As a result, the test package no longer compiles, so CI cannot run the UDS-path stabilization tests introduced here.

Useful? React with 👍 / 👎.

}
p := &Pool{
opts: PoolOptions{Config: cfg},
logger: NewLogger(LoggingConfig{Level: "error", Format: "json"}),
workers: make([]*poolWorker, len(workers)),
semaphore: make(chan struct{}, cfg.Workers*cfg.MaxInFlight),
activeRequests: make(map[uint64]*activeRequest),
opts: PoolOptions{Config: cfg},
logger: NewLogger(LoggingConfig{Level: "error", Format: "json"}),
workers: make([]*poolWorker, len(workers)),
semaphore: make(chan struct{}, cfg.MaxInFlight),
workerAvailable: make(chan struct{}, cfg.Workers*cfg.MaxInFlightPerWorker),

Check failure on line 65 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

cfg.MaxInFlightPerWorker undefined (type PoolConfig has no field or method MaxInFlightPerWorker)

Check failure on line 65 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

unknown field workerAvailable in struct literal of type Pool
shutdownCh: make(chan struct{}),

Check failure on line 66 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

unknown field shutdownCh in struct literal of type Pool
Comment on lines 60 to +66
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

newPoolWithWorkers sizes p.workers from len(workers) but sizes workerAvailable from cfg.Workers. If cfg.Workers differs from len(workers) in any test, this can produce incorrect channel sizing and lead to hangs/flaky behavior. Prefer sizing internal channels consistently from len(workers) (or ensure cfg.Workers is set from len(workers) inside this helper before channel creation).

Copilot uses AI. Check for mistakes.
activeRequests: make(map[uint64]*activeRequest),
}
for i, w := range workers {
p.workers[i] = &poolWorker{
worker: w,
connPool: make(chan net.Conn, cfg.MaxInFlight),
worker: w,
connPool: make(chan net.Conn, cfg.MaxInFlightPerWorker),

Check failure on line 72 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

cfg.MaxInFlightPerWorker undefined (type PoolConfig has no field or method MaxInFlightPerWorker)
inflightGate: make(chan struct{}, cfg.MaxInFlightPerWorker),

Check failure on line 73 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

cfg.MaxInFlightPerWorker undefined (type PoolConfig has no field or method MaxInFlightPerWorker)

Check failure on line 73 in pkg/pyproc/pool_test.go

View workflow job for this annotation

GitHub Actions / Go test 

unknown field inflightGate in struct literal of type poolWorker
}
}
return p
Expand Down Expand Up @@ -189,8 +195,7 @@
}

func TestPoolStartPrepopulateAndHealth(t *testing.T) {
tmp := t.TempDir()
paths := []string{filepath.Join(tmp, "w0.sock"), filepath.Join(tmp, "w1.sock")}
paths := []string{tempSocketPath(t, "prepop-w0"), tempSocketPath(t, "prepop-w1")}
servers := []func(){
startUnixServer(t, paths[0], func(req protocol.Request) *protocol.Response {
resp, _ := protocol.NewResponse(req.ID, map[string]string{"ok": "yes"})
Expand Down Expand Up @@ -333,7 +338,7 @@
}

func TestPoolCallCreatesConnectionAndReturns(t *testing.T) {
path := "/tmp/pool-creates-conn.sock"
path := tempSocketPath(t, "pool-create-conn")
stop := startUnixServer(t, path, func(req protocol.Request) *protocol.Response {
resp, _ := protocol.NewResponse(req.ID, map[string]string{"ok": "yes"})
return resp
Expand Down Expand Up @@ -551,7 +556,7 @@
}

func TestPoolConnect(t *testing.T) {
path := filepath.Join(t.TempDir(), "connect.sock")
path := tempSocketPath(t, "connect")
stop := startUnixServer(t, path, func(req protocol.Request) *protocol.Response {
resp, _ := protocol.NewResponse(req.ID, map[string]string{"ok": "yes"})
return resp
Expand Down
10 changes: 3 additions & 7 deletions pkg/pyproc/socket_hmac_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"encoding/hex"
"net"
"path/filepath"
"testing"
)

Expand Down Expand Up @@ -83,8 +82,7 @@ func TestSecretFromHex_InvalidHex(t *testing.T) {

func TestHMACAuthClientServer(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "hmac-test.sock")
socketPath := tempSocketPath(t, "hmac-test")

secret := []byte("shared-secret-key-for-testing")
serverAuth := NewHMACAuth(secret)
Expand Down Expand Up @@ -125,8 +123,7 @@ func TestHMACAuthClientServer(t *testing.T) {

func TestHMACAuthWrongSecret(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "w.sock")
socketPath := tempSocketPath(t, "hmac-wrong")

serverAuth := NewHMACAuth([]byte("server-secret"))
clientAuth := NewHMACAuth([]byte("wrong-client-secret"))
Expand Down Expand Up @@ -166,8 +163,7 @@ func TestHMACAuthWrongSecret(t *testing.T) {

func TestNewHMACListener(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "hmac-listener.sock")
socketPath := tempSocketPath(t, "hmac-listener")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down
19 changes: 19 additions & 0 deletions pkg/pyproc/testutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package pyproc

import (
"fmt"
"os"
"path/filepath"
"testing"
"time"
)

func tempSocketPath(t *testing.T, prefix string) string {
t.Helper()
base := filepath.Join(os.TempDir(), "pyproc")
if err := os.MkdirAll(base, 0755); err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
name := fmt.Sprintf("%s-%d.sock", prefix, time.Now().UnixNano())
return filepath.Join(base, name)
Comment on lines +13 to +18
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

tempSocketPath uses os.TempDir() which is often a long per-user path on macOS (e.g., /var/folders/...) and can still exceed Unix socket path length limits. Since the PR goal is to avoid macOS path issues, consider using a short fixed base dir on Unix (commonly /tmp/pyproc) or otherwise enforcing a short base path specifically for UDS tests.

Copilot uses AI. Check for mistakes.
}
8 changes: 4 additions & 4 deletions pkg/pyproc/transport_multiplexed_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func TestNewMultiplexedTransport_NonExistentSocket(t *testing.T) {

func TestMultiplexedTransport_CallAfterClose(t *testing.T) {
requireUnixSocket(t)
socketPath := "/tmp/mux-call-close.sock"
socketPath := tempSocketPath(t, "mux-call-close")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestMultiplexedTransport_CallAfterClose(t *testing.T) {

func TestMultiplexedTransport_IsHealthy(t *testing.T) {
requireUnixSocket(t)
socketPath := "/tmp/mux-health.sock"
socketPath := tempSocketPath(t, "mux-health")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -323,7 +323,7 @@ func TestMultiplexedTransport_IsHealthy(t *testing.T) {

func TestMultiplexedTransport_ReadError(t *testing.T) {
requireUnixSocket(t)
socketPath := "/tmp/mux-read-error.sock"
socketPath := tempSocketPath(t, "mux-read-error")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -362,7 +362,7 @@ func TestMultiplexedTransport_ReadError(t *testing.T) {

func TestMultiplexedTransport_DoubleClose(t *testing.T) {
requireUnixSocket(t)
socketPath := "/tmp/mux-dclose.sock"
socketPath := tempSocketPath(t, "mux-dclose")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down
16 changes: 6 additions & 10 deletions pkg/pyproc/transport_uds_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package pyproc
import (
"context"
"net"
"path/filepath"
"testing"
"time"

Expand Down Expand Up @@ -54,8 +53,7 @@ func TestNewUDSTransport_NonExistentSocket(t *testing.T) {

func TestUDSTransport_CallAfterClose(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "t.sock")
socketPath := tempSocketPath(t, "uds-call-close")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -93,8 +91,7 @@ func TestUDSTransport_CallAfterClose(t *testing.T) {

func TestUDSTransport_Health(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "h.sock")
socketPath := tempSocketPath(t, "uds-health")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -131,7 +128,7 @@ func TestUDSTransport_Health(t *testing.T) {

func TestUDSTransport_Reconnect(t *testing.T) {
requireUnixSocket(t)
socketPath := "/tmp/reconnect-test.sock"
socketPath := tempSocketPath(t, "uds-reconnect")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -166,7 +163,7 @@ func TestUDSTransport_Reconnect(t *testing.T) {

func TestUDSTransport_ReconnectFail(t *testing.T) {
requireUnixSocket(t)
socketPath := "/tmp/reconnect-fail.sock"
socketPath := tempSocketPath(t, "uds-reconnect-fail")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -202,7 +199,7 @@ func TestUDSTransport_ReconnectFail(t *testing.T) {

func TestUDSTransport_PingFail(t *testing.T) {
requireUnixSocket(t)
socketPath := "/tmp/ping-fail.sock"
socketPath := tempSocketPath(t, "uds-ping-fail")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down Expand Up @@ -243,8 +240,7 @@ func TestUDSTransport_PingFail(t *testing.T) {

func TestUDSTransport_DoubleClose(t *testing.T) {
requireUnixSocket(t)
tmpDir := t.TempDir()
socketPath := filepath.Join(tmpDir, "d.sock")
socketPath := tempSocketPath(t, "uds-dclose")

listener, err := net.Listen("unix", socketPath)
if err != nil {
Expand Down
7 changes: 6 additions & 1 deletion pkg/pyproc/worker_external_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"net"
"os"
"path/filepath"
"testing"
"time"
)
Expand All @@ -12,7 +13,11 @@ import (
// It uses /tmp directly to avoid macOS socket path length limits.
func startTestUnixListener(t *testing.T) (net.Listener, string) {
t.Helper()
f, err := os.CreateTemp("/tmp", "pyproc-test-*.sock")
baseDir := filepath.Join(os.TempDir(), "pyproc")
if err := os.MkdirAll(baseDir, 0755); err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
f, err := os.CreateTemp(baseDir, "pyproc-test-*.sock")
Comment on lines 13 to +20
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The comment says this helper uses /tmp directly to avoid macOS socket path length limits, but the updated implementation uses os.TempDir() (which is frequently not /tmp on macOS). Update the comment to match the new behavior, or adjust the code to actually use a short /tmp-based path so the comment and intent remain correct.

Copilot uses AI. Check for mistakes.
if err != nil {
t.Fatalf("failed to create temp file: %v", err)
}
Expand Down
Loading