test: stabilize unix socket test paths by YuminosukeSato · Pull Request #90 · YuminosukeSato/pyproc

YuminosukeSato · 2026-02-06T14:12:56Z

Summary

Stabilize Unix socket test paths with a shared helper to avoid macOS path limits and permission issues.

Changes

Add tempSocketPath test helper.
Update UDS-related tests to use the helper.

Test plan

go test -v -race ./...

Copilot

Pull request overview

Stabilizes Unix domain socket (UDS) test paths by introducing a shared helper and migrating existing tests away from per-test temp directories and hardcoded /tmp socket paths.

Changes:

Added a shared tempSocketPath helper for generating shorter, consistent UDS paths.
Updated multiple UDS-related tests to use tempSocketPath.
Adjusted some external-test socket temp directory creation to use a shared temp subdirectory.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/pyproc/worker_external_test.go	Moves external worker test socket temp files into a shared temp subdir
pkg/pyproc/transport_uds_test.go	Uses `tempSocketPath` for UDS transport test sockets
pkg/pyproc/transport_multiplexed_test.go	Uses `tempSocketPath` for multiplexed transport UDS sockets
pkg/pyproc/testutil_test.go	Introduces `tempSocketPath` helper
pkg/pyproc/socket_hmac_test.go	Uses `tempSocketPath` for HMAC listener/client-server tests
pkg/pyproc/pool_test.go	Switches pool-related tests to `tempSocketPath` and adjusts pool test helper initialization
pkg/pyproc/pool_external_test.go	Moves external pool test socket temp files into a shared temp subdir
pkg/pyproc/connection_test.go	Uses `tempSocketPath` for worker connection tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T14:14:50Z

pkg/pyproc/testutil_test.go

+	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)


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 · 2026-02-06T14:14:51Z

pkg/pyproc/worker_external_test.go

 // 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")


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 · 2026-02-06T14:14:51Z

pkg/pyproc/pool_external_test.go

+	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")


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 · 2026-02-06T14:14:51Z

pkg/pyproc/pool_test.go

 	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),
+		shutdownCh:      make(chan struct{}),


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).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f8a8cef78

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-02-06T14:16:43Z

pkg/pyproc/pool_test.go

+	if cfg.MaxInFlightPerWorker == 0 {
+		cfg.MaxInFlightPerWorker = 1


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 👍 / 👎.

PR #92 used 21 turns and hit the limit. Increasing to 30 to allow more comprehensive reviews without artificial cutoffs. Observed usage: - PR #90: 18 turns (/bin/zsh.51) - PR #92: 21 turns (/bin/zsh.36) Expected cost: ~/bin/zsh.40-0.60 per PR

* ci: increase max-turns from 20 to 30 PR #92 used 21 turns and hit the limit. Increasing to 30 to allow more comprehensive reviews without artificial cutoffs. Observed usage: - PR #90: 18 turns (/bin/zsh.51) - PR #92: 21 turns (/bin/zsh.36) Expected cost: ~/bin/zsh.40-0.60 per PR * ci: remove Claude Code workflow Remove automated Claude Code reviews. Manual code reviews will be used instead.

YuminosukeSato · 2026-02-06T15:17:06Z

Merged into PR #91 (pr/2-core-dispatch includes all commits from this PR)

Copilot AI review requested due to automatic review settings February 6, 2026 14:12

github-actions bot added lang/go Go code changes area/pool Worker pool management labels Feb 6, 2026

Copilot AI reviewed Feb 6, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Feb 6, 2026

View reviewed changes

YuminosukeSato mentioned this pull request Feb 6, 2026

ci: increase Claude Code max-turns to 20 #94

Closed

YuminosukeSato force-pushed the pr/1-test-infra branch from 0f8a8ce to 29967bc Compare February 6, 2026 14:27

YuminosukeSato mentioned this pull request Feb 6, 2026

ci: increase max-turns from 20 to 30 #96

Closed

test: stabilize unix socket test paths

7c7a7c1

YuminosukeSato force-pushed the pr/1-test-infra branch from 29967bc to 7c7a7c1 Compare February 6, 2026 14:46

YuminosukeSato closed this Feb 6, 2026

YuminosukeSato mentioned this pull request Feb 6, 2026

feat: serialize dispatch per worker with inflight limits #98

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: stabilize unix socket test paths#90

test: stabilize unix socket test paths#90
YuminosukeSato wants to merge 1 commit intomainfrom
pr/1-test-infra

YuminosukeSato commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Feb 6, 2026

Uh oh!

YuminosukeSato commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if cfg.MaxInFlightPerWorker == 0 {
		cfg.MaxInFlightPerWorker = 1

Conversation

YuminosukeSato commented Feb 6, 2026

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

YuminosukeSato commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants