diff --git a/internal/app/app_jobs_e2e_test.go b/internal/app/app_jobs_e2e_test.go index 13f94c3..6e57757 100644 --- a/internal/app/app_jobs_e2e_test.go +++ b/internal/app/app_jobs_e2e_test.go @@ -148,7 +148,7 @@ func TestE2E_SpawnAgentToolViaSlashCommand(t *testing.T) { if err != nil { t.Fatalf("jobs.NewManager: %v", err) } - defer func() { _ = mgr.Shutdown(2 * time.Second) }() + defer func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }() // Build a minimal App by hand so we don't have to stand up every // Deps field. The fields we need for this test are: deps.Jobs, diff --git a/internal/app/app_lifecycle_parity_test.go b/internal/app/app_lifecycle_parity_test.go index cac92d2..a5870f3 100644 --- a/internal/app/app_lifecycle_parity_test.go +++ b/internal/app/app_lifecycle_parity_test.go @@ -7,6 +7,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/packetcode/packetcode/internal/agent" + "github.com/packetcode/packetcode/internal/testwait" "github.com/packetcode/packetcode/internal/ui/components/agentview" ) @@ -28,7 +29,7 @@ func TestFirstVisibleProgressStopsThinkingSpinner(t *testing.T) { func TestLeftArrowOpensAgentsOnlyFromEmptyIdleInput(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - t.Cleanup(func() { _ = mgr.Shutdown(2 * time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }) r.app.input.Reset() r.app.handleKey(tea.KeyMsg{Type: tea.KeyLeft}) if !r.app.agentView.Visible() { @@ -46,7 +47,7 @@ func TestLeftArrowOpensAgentsOnlyFromEmptyIdleInput(t *testing.T) { func TestAgentWorkspaceTaskPromptCanClearReturnAndSpawn(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - t.Cleanup(func() { _ = mgr.Shutdown(2 * time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }) r.app.showAgentView() r.app.handleKey(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("n")}) @@ -84,7 +85,7 @@ func TestAgentWorkspaceTaskPromptCanClearReturnAndSpawn(t *testing.T) { func TestAgentWorkspaceListActionsAreNotSwallowedByTaskInput(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - t.Cleanup(func() { _ = mgr.Shutdown(2 * time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }) _, _ = r.app.handleSpawnCommand([]string{"inspect the renderer"}) r.app.showAgentView() diff --git a/internal/app/app_slashcmd_test.go b/internal/app/app_slashcmd_test.go index 5b9d62b..062c307 100644 --- a/internal/app/app_slashcmd_test.go +++ b/internal/app/app_slashcmd_test.go @@ -22,6 +22,7 @@ import ( "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/session" "github.com/packetcode/packetcode/internal/statusline" + "github.com/packetcode/packetcode/internal/testwait" "github.com/packetcode/packetcode/internal/tools" "github.com/packetcode/packetcode/internal/ui/components/agentview" "github.com/packetcode/packetcode/internal/ui/components/approval" @@ -1459,7 +1460,7 @@ func TestApp_Queue_ListDropAndClear(t *testing.T) { func TestApp_Agents_ListUsesBackgroundJobs(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer func() { _ = mgr.Shutdown(2 * time.Second) }() + defer func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "audit fixtures", @@ -1483,7 +1484,7 @@ func TestApp_Agents_ListUsesBackgroundJobs(t *testing.T) { func TestApp_Agents_DetailOpensJobsPanel(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer func() { _ = mgr.Shutdown(2 * time.Second) }() + defer func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "inspect flaky test", @@ -1503,7 +1504,7 @@ func TestApp_Agents_DetailOpensJobsPanel(t *testing.T) { func TestApp_Agents_NotFoundUsesAgentLabel(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer func() { _ = mgr.Shutdown(2 * time.Second) }() + defer func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }() r.app.handleSlashCommand("agents", []string{"missing"}, "/agents missing") convContains(t, r.app, "[agent:missing not found]") @@ -1512,7 +1513,7 @@ func TestApp_Agents_NotFoundUsesAgentLabel(t *testing.T) { func TestApp_Agents_ViewDoesNotOverrideJobsPanelOverlay(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer func() { _ = mgr.Shutdown(2 * time.Second) }() + defer func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "inspect overlay", @@ -1646,7 +1647,7 @@ func TestApp_ResizeRecomposesBuiltInStatusLine(t *testing.T) { func TestApp_HandleJobUpdate_IgnoresStaleSeqAndDedupesTerminal(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer func() { _ = mgr.Shutdown(2 * time.Second) }() + defer func() { _ = mgr.Shutdown(testwait.Timeout(2 * time.Second)) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "finish once", diff --git a/internal/jobs/abandoned_test.go b/internal/jobs/abandoned_test.go index ec7118d..879a1d9 100644 --- a/internal/jobs/abandoned_test.go +++ b/internal/jobs/abandoned_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/packetcode/packetcode/internal/testwait" "github.com/packetcode/packetcode/internal/tools" ) @@ -133,7 +134,7 @@ func TestCancel_RecordsRequestAndStaysCancelled(t *testing.T) { // loadPersistedJobs reconciles anything still marked running — so reading // the directory under a live manager would race the flush and rewrite the // very record under test. - require.NoError(t, mgr.Shutdown(2*time.Second)) + require.NoError(t, mgr.Shutdown(testwait.Timeout(2*time.Second))) reloaded, _, unread, lerr := loadPersistedJobs(jobsDir, "") require.NoError(t, lerr) diff --git a/internal/jobs/manager_test.go b/internal/jobs/manager_test.go index 231d3bf..bcb0663 100644 --- a/internal/jobs/manager_test.go +++ b/internal/jobs/manager_test.go @@ -18,6 +18,7 @@ import ( "github.com/packetcode/packetcode/internal/config" "github.com/packetcode/packetcode/internal/permissions" "github.com/packetcode/packetcode/internal/provider" + "github.com/packetcode/packetcode/internal/testwait" "github.com/packetcode/packetcode/internal/tools" ) @@ -322,7 +323,7 @@ func TestManager_Shutdown_Persists(t *testing.T) { return running == N }) - require.NoError(t, mgr.Shutdown(2*time.Second)) + require.NoError(t, mgr.Shutdown(testwait.Timeout(2*time.Second))) for _, id := range ids { snap, ok := mgr.Get(id) diff --git a/internal/jobs/persistence_test.go b/internal/jobs/persistence_test.go index 23dcdf8..64d4d70 100644 --- a/internal/jobs/persistence_test.go +++ b/internal/jobs/persistence_test.go @@ -10,6 +10,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/testwait" ) // TestSaveSnapshot_AtomicWrite confirms the temp-file-then-rename @@ -175,7 +177,7 @@ func TestManagerPersistenceDebouncesNonterminalUpdates(t *testing.T) { dir := t.TempDir() mgr, _, err := NewManager(Config{JobsDir: dir}) require.NoError(t, err) - t.Cleanup(func() { _ = mgr.Shutdown(time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(testwait.Timeout(time.Second)) }) mgr.persistDelay = time.Hour for seq := int64(1); seq <= 3; seq++ { @@ -197,7 +199,7 @@ func TestManagerPersistenceTerminalFlushIsSynchronous(t *testing.T) { dir := t.TempDir() mgr, _, err := NewManager(Config{JobsDir: dir}) require.NoError(t, err) - t.Cleanup(func() { _ = mgr.Shutdown(time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(testwait.Timeout(time.Second)) }) mgr.persistDelay = time.Hour require.NoError(t, mgr.savePersistedSnapshot(persistedJob{ID: "terminal", State: "running", Seq: 1})) require.NoError(t, mgr.savePersistedSnapshot(persistedJob{ID: "terminal", State: "completed", Seq: 2, Summary: "done"})) @@ -219,7 +221,7 @@ func TestManagerShutdownFlushesPendingSnapshots(t *testing.T) { mgr.persistDelay = time.Hour require.NoError(t, mgr.savePersistedSnapshot(persistedJob{ID: "shutdown", State: "running", Seq: 7, LastMessage: "latest"})) - require.NoError(t, mgr.Shutdown(time.Second)) + require.NoError(t, mgr.Shutdown(testwait.Timeout(time.Second))) got, ok := readPersistedJob(filepath.Join(dir, "shutdown.json")) require.True(t, ok) assert.Equal(t, int64(7), got.Seq) diff --git a/internal/jobs/testhelpers_test.go b/internal/jobs/testhelpers_test.go index 9798f41..3ff5e49 100644 --- a/internal/jobs/testhelpers_test.go +++ b/internal/jobs/testhelpers_test.go @@ -197,8 +197,25 @@ func newTestManager(t *testing.T, prov provider.Provider, opts ...func(*Config)) } mgr, _, err := NewManager(cfg) require.NoError(t, err) + // Scaled, and reported rather than discarded. + // + // Shutdown returns when its timeout expires whether or not the workers + // stopped. Two seconds was ample on an idle machine and not ample under a + // parallel suite, and what happened then was not a slow test but a broken + // one: the workers were still running, still writing job records into the + // t.TempDir() Go was about to remove. On Windows a directory that is + // gaining files cannot be removed, so the test died in cleanup with + // "TempDir RemoveAll cleanup: ... The directory is not empty" -- naming + // neither the worker, nor the timeout, nor even the assertion it never + // reached. TestResubmit_IsOnlyAllowedOnce failed that way. + // + // The error says "N workers still running after ...", which is the one + // sentence that would have identified this immediately. Discarding it was + // how a shutdown that did not shut anything down looked like success. t.Cleanup(func() { - _ = mgr.Shutdown(2 * time.Second) + if err := mgr.Shutdown(testwait.Timeout(2 * time.Second)); err != nil { + t.Errorf("jobs manager did not shut down cleanly: %v", err) + } }) return mgr, tr }