modify foreground test to document behavior

Signed-off-by: Jeremy Drouillard <[email protected]>

Author: jerm-dro

test/e2e/helpers.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"syscall"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2" //nolint:staticcheck // Standard practice for Ginkgo
@@ -42,6 +43,9 @@ type THVCommand struct {
 	env    []string
 	dir    string
 	stdin  string
+
+	// cmd is the underlying exec.Cmd once a Run method is called.
+	cmd *exec.Cmd
 }
 
 // NewTHVCommand creates a new ToolHive command
@@ -82,24 +86,54 @@ func (c *THVCommand) RunWithTimeout(timeout time.Duration) (string, string, erro
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
 
-	cmd := exec.CommandContext(ctx, c.config.THVBinary, c.args...) //nolint:gosec // Intentional for e2e testing
-	cmd.Env = c.env
+	c.cmd = exec.CommandContext(ctx, c.config.THVBinary, c.args...) //nolint:gosec // Intentional for e2e testing
+	c.cmd.Env = c.env
 	if c.dir != "" {
-		cmd.Dir = c.dir
+		c.cmd.Dir = c.dir
 	}
 	if c.stdin != "" {
-		cmd.Stdin = strings.NewReader(c.stdin)
+		c.cmd.Stdin = strings.NewReader(c.stdin)
 	}
 
 	var stdout, stderr strings.Builder
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
+	c.cmd.Stdout = &stdout
+	c.cmd.Stderr = &stderr
 
-	err := cmd.Run()
+	err := c.cmd.Run()
 
 	return stdout.String(), stderr.String(), err
 }
 
+// InterruptAndWaitForExit interrupts the command and waits for it to exit.
+// If the context is cancelled, it returns an error.
+func (c *THVCommand) InterruptAndWaitForExit(ctx context.Context) error {
+	if c.cmd.ProcessState != nil {
+		// The command has already exited, so we can return early.
+		// Calling wait again will produce an error if called after the command has exited.
+		return nil
+	}
+	err := c.cmd.Process.Signal(syscall.SIGINT)
+	if err != nil {
+		return fmt.Errorf("failed to interrupt command: %w", err)
+	}
+
+	exitCh := make(chan struct{}, 1)
+	go func() {
+		err = c.cmd.Wait()
+		if err != nil {
+			GinkgoWriter.Printf("command exited with error after interrupt: %v", err)
+		}
+		exitCh <- struct{}{}
+	}()
+
+	select {
+	case <-ctx.Done():
+		return fmt.Errorf("context cancelled while waiting for command to exit")
+	case <-exitCh:
+		return nil
+	}
+}
+
 // ExpectSuccess runs the command and expects it to succeed
 func (c *THVCommand) ExpectSuccess() (string, string) {
 	stdout, stderr, err := c.Run()

test/e2e/osv_mcp_server_test.go

@@ -464,28 +464,31 @@ var _ = Describe("OsvMcpServer", Label("mcp", "streamable-http", "e2e"), Serial,
 				serverName := generateUniqueServerName("osv-foreground-test")
 
 				// 1) Start the foreground process in the background (goroutine) with a generous timeout.
-				done := make(chan struct{})
 				fgStdout := ""
 				fgStderr := ""
+
+				// maintain a reference to the command so we can interrupt it when we're done.
+				runCommand := e2e.NewTHVCommand(
+					config, "run",
+					"--name", serverName,
+					"--transport", "streamable-http",
+					"--foreground",
+					"osv",
+				)
 				go func() {
-					out, errOut, _ := e2e.NewTHVCommand(
-						config, "run",
-						"--name", serverName,
-						"--transport", "streamable-http",
-						"--foreground",
-						"osv",
-					).RunWithTimeout(5 * time.Minute)
+					out, errOut, _ := runCommand.RunWithTimeout(5 * time.Minute)
 					fgStdout, fgStderr = out, errOut
-					close(done)
 				}()
 
 				// Always try to stop the server at the end so the goroutine returns.
 				defer func() {
-					_, _, _ = e2e.NewTHVCommand(config, "stop", serverName).Run()
-					select {
-					case <-done:
-					case <-time.After(15 * time.Second):
-						// Nothing else we can signal directly; the RunWithTimeout will eventually kill it.
+					// If this takes too long, the timeout willl kill the server eventually.
+					ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+					defer cancel()
+					err := runCommand.InterruptAndWaitForExit(ctx)
+					if err != nil {
+						// This may be safe to ignore if the server is already stopped.
+						GinkgoWriter.Printf("Error interrupting foreground server during last cleanup: %v\n", err)
 					}
 				}()
 
@@ -530,15 +533,11 @@ var _ = Describe("OsvMcpServer", Label("mcp", "streamable-http", "e2e"), Serial,
 
 				// 6) Stop the server; this should unblock the goroutine.
 				By("stopping the foreground server")
-				_, _ = e2e.NewTHVCommand(config, "stop", serverName).ExpectSuccess()
-
-				// Wait for the run goroutine to exit.
-				select {
-				case <-done:
-					// ok
-				case <-time.After(15 * time.Second):
-					Fail("foreground run did not exit after stop; stdout="+fgStdout+" stderr="+fgStderr, 1)
-				}
+
+				ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+				defer cancel()
+				err = runCommand.InterruptAndWaitForExit(ctx)
+				Expect(err).ToNot(HaveOccurred(), "server should be stopped; stdout="+fgStdout+" stderr="+fgStderr)
 
 				// 7) Workload should be stopped via workload manager.
 				By("verifying workload status is stopped via workload manager")
@@ -556,9 +555,11 @@ var _ = Describe("OsvMcpServer", Label("mcp", "streamable-http", "e2e"), Serial,
 					return workload.Status
 				}, 15*time.Second, 200*time.Millisecond).Should(Equal(runtime.WorkloadStatusStopped), "workload should be in stopped status after stop")
 
-				// 8) Verify status file still exists with stopped status (it's not deleted, just marked as stopped)
-				By("verifying status file still exists after stop")
-				Expect(statusFileExists(serverName)).To(BeTrue(), "status file should still exist after stop")
+				// 8) Verify status file does NOT exist. Interrupting a foreground server should delete the status file.
+				// We may want to change this behavior and prefer the status to remain in a stopped state.
+				// For now, this test documents the current behavior.
+				By("verifying status file does not exist after stop")
+				Expect(!statusFileExists(serverName)).To(BeTrue(), "status file should not exist after stop")
 
 			})
 		})