From adf3eab01b66eee97a4c8a182990bb38275ef1b1 Mon Sep 17 00:00:00 2001
From: viettranx <viettranx@gmail.com>
Date: Sun, 12 Apr 2026 07:17:48 +0700
Subject: [PATCH] test(acp): extract processCloseTimeout var for fast-test
 override
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TestProcessPool_Close_TimesOutSlowProcess was burning 5s of wall
time per run to exercise the "fake process never exits" path,
because Close() used a hard-coded `time.After(5 * time.Second)`.

Extract the timeout as a package var `processCloseTimeout` (default
5s — production unchanged) so the test can override to 20ms via
t.Cleanup-scoped swap. Same pattern as the recent cron/vault/
facebook speedups in commit 0c44149f.

Result: acp race suite 17s → 1.6s. No production behavior change.
---
 internal/providers/acp/process.go      |  8 +++++++-
 internal/providers/acp/process_test.go | 22 ++++++++++++----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/internal/providers/acp/process.go b/internal/providers/acp/process.go
index 80012d15..1b0644d3 100644
--- a/internal/providers/acp/process.go
+++ b/internal/providers/acp/process.go
@@ -215,6 +215,12 @@ func (pp *ProcessPool) reapLoop() {
 	}
 }
 
+// processCloseTimeout is the per-process max wait during ProcessPool.Close
+// before we log a warning and move on. Production default = 5s. Exposed as a
+// package var so tests can override to a few ms, avoiding 5s of dead CI time
+// when exercising the "process never exits" path.
+var processCloseTimeout = 5 * time.Second
+
 // Close shuts down all processes gracefully.
 func (pp *ProcessPool) Close() error {
 	pp.closeOnce.Do(func() {
@@ -225,7 +231,7 @@ func (pp *ProcessPool) Close() error {
 			// Wait briefly for process to exit
 			select {
 			case <-proc.exited:
-			case <-time.After(5 * time.Second):
+			case <-time.After(processCloseTimeout):
 				slog.Warn("acp: process did not exit in time", "session_key", key)
 			}
 			pp.processes.Delete(key)
diff --git a/internal/providers/acp/process_test.go b/internal/providers/acp/process_test.go
index c98a32bb..e58b6f49 100644
--- a/internal/providers/acp/process_test.go
+++ b/internal/providers/acp/process_test.go
@@ -118,9 +118,16 @@ func TestProcessPool_Close_WithFakeProcesses(t *testing.T) {
 }
 
 func TestProcessPool_Close_TimesOutSlowProcess(t *testing.T) {
+	// Override the per-process close timeout so the test doesn't burn 5s
+	// of wall time just to exercise the "fake process never exits" path.
+	// Production behavior is unchanged — only this test sees 20ms.
+	saved := processCloseTimeout
+	processCloseTimeout = 20 * time.Millisecond
+	t.Cleanup(func() { processCloseTimeout = saved })
+
 	pp := NewProcessPool("x", nil, "", time.Minute)
 
-	// Insert a fake process that never exits (simulates slow shutdown)
+	// Insert a fake process that never exits (simulates slow shutdown).
 	neverExit := make(chan struct{}) // never closed
 	proc := &ACPProcess{
 		exited: neverExit,
@@ -130,23 +137,18 @@ func TestProcessPool_Close_TimesOutSlowProcess(t *testing.T) {
 	proc.conn = &Conn{done: make(chan struct{})}
 	pp.processes.Store("slow", proc)
 
-	// Override the 5s timeout by closing pp.done directly to trigger Close fast.
-	// We test that Close() returns even when a process doesn't exit.
-	// Use a very short real test — Close() internally waits up to 5s per process.
-	// Instead, verify that Close does not block indefinitely by checking it finishes
-	// within a reasonable window (the pool's internal 5s + buffer).
+	// Close must return promptly even though the fake process never exits,
+	// because the timeout branch fires after processCloseTimeout elapses.
 	done := make(chan error, 1)
 	go func() { done <- pp.Close() }()
 
-	// The test just needs to complete; the real 5s wait happens internally.
-	// Use a long timeout to avoid flakiness on slow CI.
 	select {
 	case err := <-done:
 		if err != nil {
 			t.Fatalf("Close returned error: %v", err)
 		}
-	case <-time.After(10 * time.Second):
-		t.Fatal("Close did not return within 10 seconds for slow process")
+	case <-time.After(1 * time.Second):
+		t.Fatal("Close did not return within 1 second even with fast timeout override")
 	}
 }