test(acp): extract processCloseTimeout var for fast-test override

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.
This commit is contained in:
viettranx committed 2026-04-12 07:17:48 +07:00
1 parent 0c44149fad
commit adf3eab01b
2 files changed
+19 -11

No files matched your search

+7 -1
View File
@@ -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)
+12 -10
View File
@@ -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")
}
}