Skip to content

Commit 12b2f28

Browse files
rgarciargarcia
authored
feat: improve e2e test infrastructure (#132)
## Summary This PR contributes improvements to the e2e test infrastructure: ### Test Infrastructure Improvements - **`ContainerOptions` struct** - Adds `HostAccess` option for tests that need `--add-host=host.docker.internal:host-gateway` to reach services on the host machine - **`runContainerWithOptions()`** - New function supporting `ContainerOptions` for flexible container configuration - **`getContainerLogs()`** - New helper to retrieve the last N lines of container logs for debugging failed tests - **`waitHTTPOrExitWithLogs()`** - Enhanced wait function that: - Captures container logs on failure for easier debugging - Periodically logs container status during the wait for better visibility in CI - **`--no-sandbox` auto-injection** - Automatically adds `--no-sandbox` to `CHROMIUM_FLAGS` for CI environments where unprivileged user namespaces may be disabled (e.g., Ubuntu 24.04 GitHub Actions) - **`--tmpfs` mode fix** - Changed from `:size=2g` to `:size=2g,mode=1777` for proper `/dev/shm` permissions - **Sequential test execution** - Added `-p 1` flag to run tests sequentially, avoiding port conflicts since all e2e tests bind to the same ports (10001, 9222) ## Test plan - [ ] CI passes with all e2e tests - [ ] Verify tests run sequentially without port conflicts - [ ] Verify container logs are captured on test failure
1 parent 63a28df commit 12b2f28

File tree

5 files changed

+113
-15
lines changed

5 files changed

+113
-15
lines changed

‎.github/workflows/server-test.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,5 @@ jobs:
6060
run: make test
6161
working-directory: server
6262
env:
63-
# Prefer explicit images if passed from the caller; otherwise use the commit short sha
64-
E2E_IMAGE_TAG: ${{ steps.vars.outputs.short_sha }}
63+
E2E_CHROMIUM_HEADFUL_IMAGE: onkernel/chromium-headful:${{ steps.vars.outputs.short_sha }}
64+
E2E_CHROMIUM_HEADLESS_IMAGE: onkernel/chromium-headless:${{ steps.vars.outputs.short_sha }}

‎server/Makefile‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ dev: build $(RECORDING_DIR)
3535

3636
test:
3737
go vet ./...
38-
go test -v -race ./...
38+
# Run tests sequentially (-p 1) to avoid port conflicts in e2e tests
39+
# (all e2e tests bind to the same ports: 10001, 9222)
40+
go test -v -race -p 1 ./...
3941

4042
clean:
4143
@rm -rf $(BIN_DIR)

‎server/e2e/e2e_chromium_restart_bench_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ func TestChromiumRestartTiming(t *testing.T) {
316316
defer cancel()
317317

318318
t.Logf("Waiting for API...")
319-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready")
319+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready")
320320

321321
t.Logf("Waiting for DevTools...")
322322
require.NoError(t, waitTCP(ctx, "127.0.0.1:9222"), "DevTools not ready")

‎server/e2e/e2e_chromium_test.go‎

Lines changed: 105 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func TestDisplayResolutionChange(t *testing.T) {
112112
defer cancel()
113113

114114
logger.Info("[setup]", "action", "waiting for API", "url", apiBaseURL+"/spec.yaml")
115-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
115+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
116116

117117
client, err := apiClient()
118118
require.NoError(t, err, "failed to create API client: %v", err)
@@ -212,7 +212,7 @@ func TestExtensionUploadAndActivation(t *testing.T) {
212212
ctx, cancel := context.WithTimeout(baseCtx, 3*time.Minute)
213213
defer cancel()
214214

215-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
215+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
216216

217217
// Wait for DevTools
218218
_, err = waitDevtoolsWS(ctx)
@@ -306,7 +306,7 @@ func TestScreenshotHeadless(t *testing.T) {
306306
ctx, cancel := context.WithTimeout(baseCtx, 2*time.Minute)
307307
defer cancel()
308308

309-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
309+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
310310

311311
client, err := apiClient()
312312
require.NoError(t, err)
@@ -357,7 +357,7 @@ func TestScreenshotHeadful(t *testing.T) {
357357
ctx, cancel := context.WithTimeout(baseCtx, 2*time.Minute)
358358
defer cancel()
359359

360-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
360+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
361361

362362
client, err := apiClient()
363363
require.NoError(t, err)
@@ -402,7 +402,7 @@ func TestInputEndpointsSmoke(t *testing.T) {
402402
ctx, cancel := context.WithTimeout(baseCtx, 2*time.Minute)
403403
defer cancel()
404404

405-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
405+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
406406

407407
client, err := apiClient()
408408
require.NoError(t, err)
@@ -445,17 +445,46 @@ func isPNG(data []byte) bool {
445445
return true
446446
}
447447

448+
// ContainerOptions configures container startup behavior
449+
type ContainerOptions struct {
450+
// HostAccess adds --add-host=host.docker.internal:host-gateway for tests
451+
// that need to reach services on the host machine
452+
HostAccess bool
453+
}
454+
448455
func runContainer(ctx context.Context, image, name string, env map[string]string) (*exec.Cmd, <-chan error, error) {
456+
return runContainerWithOptions(ctx, image, name, env, ContainerOptions{})
457+
}
458+
459+
func runContainerWithOptions(ctx context.Context, image, name string, env map[string]string, opts ContainerOptions) (*exec.Cmd, <-chan error, error) {
449460
logger := logctx.FromContext(ctx)
450461
args := []string{
451462
"run",
452463
"--name", name,
453464
"--privileged",
454465
"-p", "10001:10001", // API server
455-
"-p", "9222:9222", // DevTools proxy
456-
"--tmpfs", "/dev/shm:size=2g",
466+
"-p", "9222:9222", // DevTools proxy
467+
"--tmpfs", "/dev/shm:size=2g,mode=1777",
457468
}
469+
470+
if opts.HostAccess {
471+
args = append(args, "--add-host=host.docker.internal:host-gateway")
472+
}
473+
474+
// Ensure CHROMIUM_FLAGS includes --no-sandbox for CI environments where
475+
// unprivileged user namespaces may be disabled (e.g., Ubuntu 24.04 GitHub Actions)
476+
// Create a copy to avoid mutating the caller's map
477+
envCopy := make(map[string]string)
458478
for k, v := range env {
479+
envCopy[k] = v
480+
}
481+
if _, ok := envCopy["CHROMIUM_FLAGS"]; !ok {
482+
envCopy["CHROMIUM_FLAGS"] = "--no-sandbox"
483+
} else if !strings.Contains(envCopy["CHROMIUM_FLAGS"], "--no-sandbox") {
484+
envCopy["CHROMIUM_FLAGS"] = envCopy["CHROMIUM_FLAGS"] + " --no-sandbox"
485+
}
486+
487+
for k, v := range envCopy {
459488
args = append(args, "-e", fmt.Sprintf("%s=%s", k, v))
460489
}
461490
args = append(args, image)
@@ -515,6 +544,73 @@ func stopContainer(ctx context.Context, name string) error {
515544
return nil
516545
}
517546

547+
// getContainerLogs retrieves the last N lines of container logs for debugging.
548+
// Uses a fresh context with its own timeout to avoid issues when the parent context is cancelled.
549+
func getContainerLogs(_ context.Context, name string, tailLines int) string {
550+
// Use a fresh context with generous timeout - the parent context may be cancelled
551+
logCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
552+
defer cancel()
553+
554+
cmd := exec.CommandContext(logCtx, "docker", "logs", "--tail", fmt.Sprintf("%d", tailLines), name)
555+
output, err := cmd.CombinedOutput()
556+
if err != nil {
557+
return fmt.Sprintf("failed to get container logs: %v", err)
558+
}
559+
return string(output)
560+
}
561+
562+
// waitHTTPOrExitWithLogs waits for HTTP endpoint and captures container logs on failure.
563+
// It also periodically logs container status during the wait for better visibility.
564+
func waitHTTPOrExitWithLogs(ctx context.Context, url string, exitCh <-chan error, containerName string) error {
565+
logger := logctx.FromContext(ctx)
566+
567+
// Start a background goroutine to periodically show container status
568+
// Use a separate stopCh that we close to signal the goroutine to stop,
569+
// avoiding the race condition of sending to a potentially closed channel
570+
stopCh := make(chan struct{})
571+
doneCh := make(chan struct{})
572+
go func() {
573+
defer close(doneCh)
574+
ticker := time.NewTicker(15 * time.Second)
575+
defer ticker.Stop()
576+
for {
577+
select {
578+
case <-ctx.Done():
579+
return
580+
case <-stopCh:
581+
return
582+
case <-ticker.C:
583+
// Check if container is still running
584+
checkCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
585+
cmd := exec.CommandContext(checkCtx, "docker", "inspect", "--format", "{{.State.Status}} (pid={{.State.Pid}}, started={{.State.StartedAt}})", containerName)
586+
out, err := cmd.Output()
587+
cancel()
588+
if err == nil {
589+
logger.Info("[container-status]", "container", containerName, "status", strings.TrimSpace(string(out)))
590+
}
591+
// Also show last few log lines
592+
recentLogs := getContainerLogs(ctx, containerName, 10)
593+
if recentLogs != "" && !strings.Contains(recentLogs, "failed to get") {
594+
logger.Info("[container-logs]", "recent_output", strings.TrimSpace(recentLogs))
595+
}
596+
}
597+
}
598+
}()
599+
600+
err := waitHTTPOrExit(ctx, url, exitCh)
601+
602+
// Signal the status goroutine to stop and wait for it to finish
603+
close(stopCh)
604+
<-doneCh
605+
606+
if err != nil {
607+
// Capture container logs for debugging
608+
logs := getContainerLogs(ctx, containerName, 100)
609+
return fmt.Errorf("%w\n\nContainer logs (last 100 lines):\n%s", err, logs)
610+
}
611+
return nil
612+
}
613+
518614
func waitHTTPOrExit(ctx context.Context, url string, exitCh <-chan error) error {
519615
client := &http.Client{Timeout: 5 * time.Second}
520616
ticker := time.NewTicker(500 * time.Millisecond)
@@ -756,7 +852,7 @@ func TestCDPTargetCreation(t *testing.T) {
756852
defer cancel()
757853

758854
logger.Info("[test]", "action", "waiting for API")
759-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready")
855+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready")
760856

761857
// Wait for CDP endpoint to be ready (via the devtools proxy)
762858
logger.Info("[test]", "action", "waiting for CDP endpoint")
@@ -830,7 +926,7 @@ func TestWebBotAuthInstallation(t *testing.T) {
830926
defer cancel()
831927

832928
logger.Info("[setup]", "action", "waiting for API", "url", apiBaseURL+"/spec.yaml")
833-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
929+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
834930

835931
// Build mock web-bot-auth extension zip in-memory
836932
extDir := t.TempDir()

‎server/e2e/e2e_playwright_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func TestPlaywrightExecuteAPI(t *testing.T) {
3636
ctx, cancel := context.WithTimeout(baseCtx, 2*time.Minute)
3737
defer cancel()
3838

39-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
39+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
4040

4141
client, err := apiClient()
4242
require.NoError(t, err)
@@ -114,7 +114,7 @@ func TestPlaywrightDaemonRecovery(t *testing.T) {
114114
ctx, cancel := context.WithTimeout(baseCtx, 3*time.Minute)
115115
defer cancel()
116116

117-
require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err)
117+
require.NoError(t, waitHTTPOrExitWithLogs(ctx, apiBaseURL+"/spec.yaml", exitCh, name), "api not ready: %v", err)
118118

119119
client, err := apiClient()
120120
require.NoError(t, err)

0 commit comments

Comments
 (0)