test: avoid redundant fixture work

Reuse immutable fixtures, combine Git queries and avoid large data copies to reduce test overhead while retaining behavior checks.

Assisted-by: Codex:GPT-6

internal/pkg/process/group_windows_test.go

@@ -6,7 +6,6 @@ package process
 import (
 	"os/exec"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 )
@@ -23,16 +22,13 @@ func TestGroupClosePropagatesToOrphan(t *testing.T) {
 	childPID := ready()
 
 	require.NoError(t, cmd.Wait())
-	require.Eventually(t, func() bool {
-		return !processAlive(cmd.Process.Pid)
-	}, 20*time.Second, 200*time.Millisecond, "the step process should have exited on its own")
+	require.True(t, cmd.ProcessState.Exited(), "the step process should have exited on its own")
 
 	require.True(t, processAlive(childPID), "orphan should outlive its parent")
 
+	waitForExit := newProcessExitWaiter(t, childPID)
 	require.NoError(t, group.Close())
-	require.Eventually(t, func() bool {
-		return !processAlive(childPID)
-	}, 20*time.Second, 200*time.Millisecond, "closing the job must terminate the orphan")
+	waitForExit()
 }
 
 // TestGroupNestsWithKiller: assigned to the group first and the step's Killer
@@ -53,10 +49,9 @@ func TestGroupNestsWithKiller(t *testing.T) {
 	require.NoError(t, err, "the step's job object must nest inside the group's")
 	defer killer.Close()
 
+	waitForExit := newProcessExitWaiter(t, cmd.Process.Pid)
 	require.NoError(t, killer.Kill())
-	require.Eventually(t, func() bool {
-		return !processAlive(cmd.Process.Pid)
-	}, 20*time.Second, 200*time.Millisecond, "cancelling the step should still kill its tree")
+	waitForExit()
 }
 
 // TestGroupNilSafe covers the fallback path: when the job object cannot be

internal/pkg/process/killer_unix_test.go

@@ -9,8 +9,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"path/filepath"
-	"strconv"
 	"strings"
 	"syscall"
 	"testing"
@@ -46,23 +44,8 @@ func processAlive(pid int) bool {
 	return true
 }
 
-// TestKillerKillsTree verifies that a process group captured by the killer is
-// terminated together with a child the process spawns afterwards. This mirrors
-// a step or post-task script that launches a child which spawns further
-// processes, where cancelling must take down the whole tree, not just the
-// direct child.
 func TestKillerKillsTree(t *testing.T) {
-	dir := t.TempDir()
-	pidFile := filepath.Join(dir, "child.pid")
-
-	// Parent shell backgrounds a long-lived child (writing its PID to a file)
-	// and then sleeps. With job control off (non-interactive sh) the backgrounded
-	// child stays in the parent's process group, so the group kill must reach it.
-	script := fmt.Sprintf(`sleep 600 & echo $! > %q; sleep 600`, pidFile)
-	cmd := exec.Command("/bin/sh", "-c", script)
-	// Launch as its own process-group leader, exactly like a real process does
-	// (see SysProcAttr), so the killer's PGID == the process PID.
-	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	cmd, ready := newTestProcess(t, "parent", false)
 	require.NoError(t, cmd.Start())
 	t.Cleanup(func() {
 		_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
@@ -73,25 +56,10 @@ func TestKillerKillsTree(t *testing.T) {
 	require.NoError(t, err)
 	defer killer.Close()
 
-	// Wait for the backgrounded child PID to be reported.
-	var childPID int
-	require.Eventually(t, func() bool {
-		b, e := os.ReadFile(pidFile)
-		if e != nil {
-			return false
-		}
-		s := strings.TrimSpace(string(b))
-		if s == "" {
-			return false
-		}
-		childPID, _ = strconv.Atoi(s)
-		return childPID > 0 && processAlive(childPID)
-	}, 20*time.Second, 100*time.Millisecond, "child process should start")
+	childPID := ready()
+	require.True(t, processAlive(childPID), "child process should start")
 
-	// Killing the group must terminate both the parent and the backgrounded child.
 	require.NoError(t, killer.Kill())
-	// Reap the parent so it does not linger as a zombie (which would still report
-	// as alive); SIGKILL makes Wait return promptly.
 	_ = cmd.Wait()
 
 	require.Eventually(t, func() bool {

internal/pkg/process/killer_windows_test.go

@@ -5,7 +5,6 @@ package process
 
 import (
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sys/windows"
@@ -25,6 +24,23 @@ func processAlive(pid int) bool {
 	return code == stillActive
 }
 
+func newProcessExitWaiter(t *testing.T, pids ...int) func() {
+	t.Helper()
+	handles := make([]windows.Handle, 0, len(pids))
+	for _, pid := range pids {
+		handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid))
+		require.NoError(t, err)
+		t.Cleanup(func() { require.NoError(t, windows.CloseHandle(handle)) })
+		handles = append(handles, handle)
+	}
+	return func() {
+		t.Helper()
+		result, err := windows.WaitForMultipleObjects(handles, true, 20000)
+		require.NoError(t, err)
+		require.EqualValues(t, windows.WAIT_OBJECT_0, result, "processes should exit: %v", pids)
+	}
+}
+
 func TestKillerKillsTree(t *testing.T) {
 	cmd, ready := newTestProcess(t, "parent", false)
 	require.NoError(t, cmd.Start())
@@ -39,9 +55,7 @@ func TestKillerKillsTree(t *testing.T) {
 	childPID := ready()
 	require.True(t, processAlive(childPID), "child process should start")
 
+	waitForExit := newProcessExitWaiter(t, cmd.Process.Pid, childPID)
 	require.NoError(t, killer.Kill())
-
-	require.Eventually(t, func() bool {
-		return !processAlive(cmd.Process.Pid) && !processAlive(childPID)
-	}, 20*time.Second, 200*time.Millisecond, "parent and child should both be terminated")
+	waitForExit()
 }