From 04f18d339d6ed189082a7d7a3ed9ebd5401f82a4 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 19 May 2026 00:04:07 +0300 Subject: Fix LLM e2e runner: precheck server, correct S10 table name precheckServer() polls /healthz once before iterating scenarios. When the server is down, the runner now exits 2 with a clear startup hint instead of spawning Playwright per scenario (each timing out and filing a duplicate "did not become healthy" task via ask add). Also corrects S10-progress-batch.md: the YAML db assertion referenced `progress` but the actual table (per internal/repository/playback_progress.go) is `playback_progress`, with `media_id` as the joinable column. Verified: precheck cleanly exits 2 when the server is down (15 s timeout); full LLM e2e suite passes 13/13 with the server up. Co-Authored-By: Claude Opus 4.7 --- player-server/test/e2e-llm/runner/index.ts | 67 ++++++++++++++++++++++ .../test/e2e-llm/scenarios/S10-progress-batch.md | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/player-server/test/e2e-llm/runner/index.ts b/player-server/test/e2e-llm/runner/index.ts index fb973c9..4ec03c0 100644 --- a/player-server/test/e2e-llm/runner/index.ts +++ b/player-server/test/e2e-llm/runner/index.ts @@ -329,6 +329,67 @@ function runScenario(scenario: Scenario): boolean { return false; } +// --------------------------------------------------------------------------- +// Server precheck +// --------------------------------------------------------------------------- + +// PLAYER_URL is the base URL the harness uses (matches the README + Playwright +// config). Centralising the constant keeps the precheck and any future direct +// HTTP calls aligned with the rest of the harness. +const PLAYER_URL = process.env['PLAYER_URL'] || 'http://localhost:8080'; + +// How long to wait for /healthz before declaring the server missing. The +// Playwright suite waits up to 15 s per scenario internally; doing the same +// up-front means we surface a missing server as a single clear error instead +// of one Playwright failure (and one bogus ask task) per scenario. +const PRECHECK_TIMEOUT_MS = 15_000; +const PRECHECK_POLL_INTERVAL_MS = 200; + +/** + * waitForServer polls `${PLAYER_URL}/healthz` until it responds 2xx or the + * deadline passes. Returns true if the server became healthy, false otherwise. + * Using a synchronous loop (spawnSync sleep) keeps the runner's overall + * control flow synchronous, matching how runScenario invokes Playwright. + */ +function waitForServer(timeoutMs: number): boolean { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + // Use curl for the probe so we don't need to depend on a Node fetch + // shim — keeps the harness usable on older Node where fetch is missing. + const probe = spawnSync( + 'curl', + ['-fsS', '-o', '/dev/null', '-w', '%{http_code}', `${PLAYER_URL}/healthz`], + { encoding: 'utf8' }, + ); + if (probe.status === 0 && probe.stdout.trim().startsWith('2')) return true; + spawnSync('sleep', [String(PRECHECK_POLL_INTERVAL_MS / 1000)]); + } + return false; +} + +/** + * precheckServer fails fast with a clear, actionable message if the server + * is not reachable. Without this, each scenario would individually fail with + * the same "did not become healthy" error and openAskTask would file one + * spurious task per scenario — drowning the queue in duplicates of the same + * root cause. Returning a non-zero exit before the scenario loop avoids both. + */ +function precheckServer(): void { + if (waitForServer(PRECHECK_TIMEOUT_MS)) return; + + console.error( + `[runner] ERROR: Player server at ${PLAYER_URL} is not reachable on /healthz ` + + `after ${PRECHECK_TIMEOUT_MS / 1000}s.`, + ); + console.error('[runner] Start it from player-server/ before running the e2e suite:'); + console.error( + '[runner] MEDIA_ROOT=./testmedia SECURE_COOKIES=false ' + + 'DB_PATH=/tmp/player-e2e-llm.db ./player', + ); + console.error('[runner] Override the target URL with PLAYER_URL.'); + process.exit(2); +} + // --------------------------------------------------------------------------- // Entry point // --------------------------------------------------------------------------- @@ -346,6 +407,12 @@ function main(): void { process.exit(0); } + // Fail fast with a clear hint if the server is down. Without this the + // runner would spawn Playwright per scenario, time out on each, and file + // a duplicate ask task per failure — exactly what produced the legacy + // "Server at http://localhost:8080 did not become healthy" task pile. + precheckServer(); + console.log(`[runner] Running ${scenarios.length} scenario(s)…`); let failures = 0; diff --git a/player-server/test/e2e-llm/scenarios/S10-progress-batch.md b/player-server/test/e2e-llm/scenarios/S10-progress-batch.md index ad6ead9..a5a635c 100644 --- a/player-server/test/e2e-llm/scenarios/S10-progress-batch.md +++ b/player-server/test/e2e-llm/scenarios/S10-progress-batch.md @@ -6,7 +6,7 @@ preconditions: server_state: running # server running with an existing admin account and at least two media items fixtures: [] assertions: - - db: "SELECT id FROM progress WHERE position_seconds=30" + - db: "SELECT media_id FROM playback_progress WHERE position_seconds=30" - status_code: "POST /api/v1/progress/batch 200" skip: false --- -- cgit v1.2.3