fix flaky CI tests

.github/actions/setup-php/action.yaml

@@ -12,6 +12,8 @@ runs:
       with:
         php-version: ${{ inputs.php-version }}
         ini-file: development
+        # JIT's fixed-address mappings can race Go thread creation during startup.
+        ini-values: opcache.jit=disable
         coverage: none
         tools: none
       env:

caddy/admin_test.go

@@ -21,7 +21,7 @@ import (
 
 func TestRestartWorkerViaAdminApi(t *testing.T) {
 	tester := caddytest.NewTester(t)
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999
@@ -54,7 +54,7 @@ func TestRestartWorkerViaAdminApi(t *testing.T) {
 
 func TestShowTheCorrectThreadDebugStatus(t *testing.T) {
 	tester := caddytest.NewTester(t)
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999
@@ -77,6 +77,12 @@ func TestShowTheCorrectThreadDebugStatus(t *testing.T) {
 		}
 		`, "caddyfile")
 
+	// Regular threads finish their startup transition asynchronously.
+	require.Eventually(t, func() bool {
+		threads := frankenphp.DebugState().ThreadDebugStates
+		return len(threads) == 3 && threads[0].State == "ready"
+	}, 5*time.Second, 10*time.Millisecond, "regular PHP thread did not become ready")
+
 	debugState := getDebugState(t, tester)
 
 	// assert that the correct threads are present in the thread info
@@ -89,7 +95,7 @@ func TestShowTheCorrectThreadDebugStatus(t *testing.T) {
 
 func TestThreadDebugStateMetricsAfterRequests(t *testing.T) {
 	tester := caddytest.NewTester(t)
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999
@@ -132,7 +138,7 @@ func TestAutoScaleWorkerThreads(t *testing.T) {
 	maxTries := 10
 	requestsPerTry := 200
 	tester := caddytest.NewTester(t)
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999
@@ -189,7 +195,7 @@ func TestAutoScaleRegularThreadsOnAutomaticThreadLimit(t *testing.T) {
 	maxTries := 10
 	requestsPerTry := 200
 	tester := caddytest.NewTester(t)
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999
@@ -278,7 +284,7 @@ func getNumThreads(t *testing.T, tester *caddytest.Tester) int {
 func TestAddModuleWorkerViaAdminApi(t *testing.T) {
 	// Initialize a server with admin API enabled
 	tester := caddytest.NewTester(t)
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999

caddy/caddy_test.go

@@ -23,12 +23,11 @@ import (
 )
 
 // initServer initializes a Caddy test server and waits for it to be ready.
-// After InitServer, it polls the server to handle a race condition on macOS where
-// SO_REUSEPORT can briefly route connections to the old listener being shut down,
-// resulting in "connection reset by peer".
+// It polls the HTTP endpoint after loading the configuration to wait for the
+// listener to start accepting requests.
 func initServer(t *testing.T, tester *caddytest.Tester, config string, format string) {
 	t.Helper()
-	tester.InitServer(config, format)
+	initTestServer(t, tester, config, format)
 
 	client := &http.Client{Timeout: 1 * time.Second}
 	require.Eventually(t, func() bool {
@@ -65,6 +64,9 @@ func escapeMetricLabel(s string) string {
 }
 
 func TestMain(m *testing.M) {
+	// Bound helpers using http.Get, including diagnostics and metrics requests.
+	http.DefaultClient.Timeout = caddytest.Default.TestRequestTimeout
+
 	// setup custom environment vars for TestOsEnv
 	if os.Setenv("ENV1", "value1") != nil || os.Setenv("ENV2", "value2") != nil {
 		fmt.Println("Failed to set environment variables for tests")
@@ -98,8 +100,8 @@ func TestPHP(t *testing.T) {
 		wg.Add(1)
 
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -161,8 +163,8 @@ func TestWorker(t *testing.T) {
 		wg.Add(1)
 
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -213,9 +215,9 @@ func TestGlobalAndModuleWorker(t *testing.T) {
 		wg.Add(1)
 
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=module")
 			tester.AssertGetResponse("http://localhost:"+testPortTwo+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=global")
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -287,9 +289,9 @@ func TestNamedModuleWorkers(t *testing.T) {
 		wg.Add(1)
 
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=one")
 			tester.AssertGetResponse("http://localhost:"+testPortTwo+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=two")
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -722,8 +724,8 @@ func TestMetrics(t *testing.T) {
 	for i := range 10 {
 		wg.Add(1)
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -798,8 +800,8 @@ func TestWorkerMetrics(t *testing.T) {
 	for i := range 10 {
 		wg.Add(1)
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -955,8 +957,8 @@ func TestNamedWorkerMetrics(t *testing.T) {
 	for i := range 10 {
 		wg.Add(1)
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -1049,8 +1051,8 @@ func TestAutoWorkerConfig(t *testing.T) {
 	for i := range 10 {
 		wg.Add(1)
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -1228,7 +1230,7 @@ func TestOsEnv(t *testing.T) {
 	require.NoError(t, listener.Close())
 
 	tester := caddytest.NewTester(t)
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999
@@ -1285,11 +1287,11 @@ func TestMaxWaitTime(t *testing.T) {
 	wg.Add(10)
 	for range 10 {
 		go func() {
+			defer wg.Done()
 			statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10", t)
 			if statusCode == http.StatusServiceUnavailable {
 				success.Store(true)
 			}
-			wg.Done()
 		}()
 	}
 	wg.Wait()
@@ -1332,11 +1334,11 @@ func TestMaxWaitTimeWorker(t *testing.T) {
 	wg.Add(10)
 	for range 10 {
 		go func() {
+			defer wg.Done()
 			statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10&iteration=1", t)
 			if statusCode == http.StatusServiceUnavailable {
 				success.Store(true)
 			}
-			wg.Done()
 		}()
 	}
 	wg.Wait()
@@ -1425,8 +1427,8 @@ func TestMultiWorkersMetrics(t *testing.T) {
 	for i := range 10 {
 		wg.Add(1)
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -1533,8 +1535,8 @@ func TestDisabledMetrics(t *testing.T) {
 	for i := range 10 {
 		wg.Add(1)
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -1642,8 +1644,8 @@ func TestWorkerRestart(t *testing.T) {
 	for i := range 10 {
 		wg.Add(1)
 		go func(i int) {
+			defer wg.Done()
 			tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/worker-restart.php?i=%d", i), http.StatusOK, fmt.Sprintf("Counter (%d)", i))
-			wg.Done()
 		}(i)
 	}
 	wg.Wait()
@@ -1806,7 +1808,7 @@ func TestDd(t *testing.T) {
 func TestOpcacheReset(t *testing.T) {
 	tester := caddytest.NewTester(t)
 	tester.Client.Timeout = 60 * time.Second
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999

caddy/hotreload_test.go

@@ -9,7 +9,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
@@ -30,7 +29,7 @@ func TestHotReload(t *testing.T) {
 	// SSE roundtrip below on slow CI runners (notably emulated armv7).
 	// 30s keeps the test bounded so a real regression fails fast.
 	tester.Client.Timeout = 30 * time.Second
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			debug
 			skip_install_trust
@@ -53,49 +52,56 @@ func TestHotReload(t *testing.T) {
 			}
 		`, "caddyfile")
 
-	var connected, received sync.WaitGroup
-
-	connected.Add(1)
-	received.Go(func() {
-		cx, cancel := context.WithCancel(t.Context())
-		req, _ := http.NewRequest(http.MethodGet, "http://localhost:"+testPort+u, nil)
-		req = req.WithContext(cx)
-		resp := tester.AssertResponseCode(req, http.StatusOK)
-
+	cx, cancel := context.WithCancel(t.Context())
+	defer cancel()
+	req, err := http.NewRequestWithContext(cx, http.MethodGet, "http://localhost:"+testPort+u, nil)
+	require.NoError(t, err)
+	resp := tester.AssertResponseCode(req, http.StatusOK)
+	defer func() { require.NoError(t, resp.Body.Close()) }()
+
+	// Wait for the first bytes before changing the file, so the subscription
+	// is ready. A failed request or read must not leave a readiness wait blocked.
+	buf := make([]byte, 1024)
+	_, err = resp.Body.Read(buf)
+	require.NoError(t, err)
+
+	// Return read failures to the test goroutine. Buffer the result so this
+	// goroutine can exit even if a failed file write ends the test first.
+	received := make(chan error, 1)
+	go func() {
 		var receivedBody strings.Builder
-
-		buf := make([]byte, 1024)
-		isConnected := false
 		for {
 			n, err := resp.Body.Read(buf)
-			if n > 0 {
-				receivedBody.Write(buf[:n])
-			}
-			if !isConnected {
-				// wait for the first bytes before marking the client as connected
-				isConnected = true
-				connected.Done()
-			}
+			receivedBody.Write(buf[:n])
 			if strings.Contains(receivedBody.String(), "index.php") {
-				cancel()
-
-				break
+				received <- nil
+				return
 			}
-			// Surface the read error only after checking the buffer: on
-			// Windows the SSE server sometimes flushes the event and closes
-			// the connection in the same syscall, so Read returns (n>0, EOF)
-			// and we'd otherwise fail despite having the data we wanted.
+			// A read may return both the expected event and an error.
+			if err != nil {
+				received <- err
+				return
+			}
+		}
+	}()
+
+	// The native watcher starts asynchronously, independently of the SSE
+	// subscription. Retry changes until one is observed, leaving more time
+	// between writes than the watcher's 150 ms debounce interval.
+	ticker := time.NewTicker(250 * time.Millisecond)
+	defer ticker.Stop()
+waitForReload:
+	for {
+		require.NoError(t, os.WriteFile(indexFile, []byte("<?=$_SERVER['FRANKENPHP_HOT_RELOAD'];"), 0644))
+		select {
+		case err := <-received:
 			require.NoError(t, err)
+			break waitForReload
+		case <-ticker.C:
 		}
-
-		require.NoError(t, resp.Body.Close())
-	})
-
-	connected.Wait()
-
-	require.NoError(t, os.WriteFile(indexFile, []byte("<?=$_SERVER['FRANKENPHP_HOT_RELOAD'];"), 0644))
-
-	received.Wait()
+	}
+	cancel()
+	require.NoError(t, resp.Body.Close())
 
 	tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, u)
 }

caddy/module_test.go

@@ -16,7 +16,7 @@ func TestRootBehavesTheSameOutsideAndInsidePhpServer(t *testing.T) {
 	expectedFileResponse, _ := os.ReadFile("../testdata/files/static.txt")
 	hostWithRootOutside := "http://localhost:" + testPort
 	hostWithRootInside := "http://localhost:" + testPortTwo
-	tester.InitServer(`
+	initTestServer(t, tester, `
 		{
 			skip_install_trust
 			admin localhost:2999