From e3dff15500b623e72f50a547bb643d822a30a0fe Mon Sep 17 00:00:00 2001 From: Olivier Biot Date: Sat, 29 Aug 2026 15:41:55 +0800 Subject: [PATCH] ci: retry once when vitest loses its browser page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The browser suite aborts at a random point with "Browser connection was closed while running tests", naming a different victim spec every time and reporting zero failed tests. Measured at 4 deaths in 21 local runs; landing a PR that changed nothing but Markdown took three attempts. Vitest 4.1.11 runs each worker as one long-lived orchestrator page and dispatches every spec over a single websocket. Its ws close handler destroys the session and rejects the pending `createTesters`, so losing that socket kills the whole run — the in-page client does retry, but the session is already gone. Underneath, the browser intermittently severs a renderer's network-service channels: every websocket for that page dies at once, the server sees a graceful 1001 while the page sees 1006, and nothing crashes. Ruled out by measurement, not argument: memory (2.46 GB peak of 64 GB), CPU starvation (5/5 clean under load), browser crash (no crash event or signal), page navigation (zero in ~90 instrumented runs), worker concurrency (single worker was worse), and the earlier drawmesh_bench fix (skips in ~30 ms, no stall in any log). Launching the full Chromium build rather than the headless shell fixes it locally — 20 consecutive clean runs — but does nothing on CI: 0 of 4 attempts passed. So that is not the fix, and this retries instead. The retry condition is deliberately narrow: the page must have been lost AND no test may have failed. A genuine failure prints "N failed" in the summary and exits immediately without a second attempt. Both branches were checked against real captured logs from this investigation. texture.spec.js is a latent bug found on the way, unrelated to the retry. Its app is built with `video.AUTO` while the test asserts `boundTextures` and `currentTextureUnit` — WebGL texture-unit bookkeeping that no WebGPU batcher keeps. It passes only because the headless browser has no GPU and falls back to WebGL; anyone running the suite where WebGPU is available fails on it. Now gated on the renderer rather than on the fields being present, so a WebGL regression that drops them still fails instead of silently skipping. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N --- .github/workflows/main.yml | 39 +++++++++++++++++++++++++- packages/melonjs/tests/texture.spec.js | 15 ++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e97a1ed76..024280a07 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -63,7 +63,44 @@ jobs: key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} restore-keys: ${{ runner.os }}-pnpm-store- - run: pnpm i - - run: pnpm test + # Vitest 4.1.11 runs each worker as one long-lived orchestrator page and + # dispatches every spec file over a single websocket. Its ws close handler + # destroys the session and rejects the pending `createTesters`, so if that + # socket drops the whole run dies — the in-page client does try to + # reconnect, but the session is already gone. Underneath, the browser + # intermittently severs a renderer's network-service channels: every + # websocket for that page dies at once, the server sees a graceful 1001 + # while the page sees 1006, and no process crashes. + # + # Measured at 4 deaths in 21 local runs, and it cost three attempts to + # land a PR that changed nothing but Markdown. Not our code: no test ever + # fails, and the spec it names is just whichever was in flight. Launching + # the full Chromium build fixes it locally (20 runs clean) but does + # nothing here (0 of 4), so this retries instead. + # + # The condition is deliberately narrow — the page must have been lost AND + # no test may have failed. A real failure prints "N failed" in the summary + # and exits without a retry. + - name: Test (retries once if vitest loses its browser page) + # the container has no default bash, so GitHub would run this under + # dash, which has no `pipefail` — and without it the exit status of + # `pnpm test` is masked by `tee` + shell: bash + run: | + set -o pipefail + for attempt in 1 2; do + if pnpm test 2>&1 | tee "test-$attempt.log"; then + exit 0 + fi + if ! grep -q "Browser connection was closed" "test-$attempt.log" \ + || grep -qE "Test Files.*[0-9]+ failed" "test-$attempt.log"; then + echo "::error::test failed for a real reason, not the known browser flake" + exit 1 + fi + echo "::warning::vitest lost its browser page (upstream flake) — attempt $attempt" + done + echo "::error::vitest lost its browser page on both attempts" + exit 1 - run: pnpm -F @melonjs/matter-adapter test - run: pnpm -F @melonjs/planck-adapter test - run: pnpm -F @melonjs/debug-plugin test diff --git a/packages/melonjs/tests/texture.spec.js b/packages/melonjs/tests/texture.spec.js index 7853482a1..b8aa30a9f 100644 --- a/packages/melonjs/tests/texture.spec.js +++ b/packages/melonjs/tests/texture.spec.js @@ -6,6 +6,7 @@ import { Sprite, TextureAtlas, video, + WebGLRenderer, } from "../src/index.js"; import Renderer from "../src/video/renderer.js"; @@ -287,8 +288,18 @@ describe("Texture", () => { if (compositor) { expect(flushed).toBe(true); - expect(compositor.boundTextures.length).toEqual(0); - expect(compositor.currentTextureUnit).toEqual(-1); + // `boundTextures` / `currentTextureUnit` are WebGL texture-unit + // bookkeeping and exist on no WebGPU batcher, which binds per + // draw instead. The app is built with `video.AUTO`, so which + // backend answers depends on the machine — headless SwiftShader + // lands on WebGL, a real GPU on WebGPU. The cache assertions + // above are backend-agnostic and run either way; only these two + // are gated, and on the renderer rather than on the field being + // present, so a WebGL regression that drops them still fails. + if (app.renderer instanceof WebGLRenderer) { + expect(compositor.boundTextures.length).toEqual(0); + expect(compositor.currentTextureUnit).toEqual(-1); + } // restore original compositor.flush = originalFlush; }