Skip to content

Walk entry had no CI coverage: a source regex and a spec that skips on every runner #725

Description

@TortoiseWolfe

CLOSED AS ALREADY DONE — verified 2026-08-18

The title's central claim — walk entry has no CI coverage of any kind — is no longer true: a real component-render unit test runs in Test (20.x) and the walk-reachability E2E assertion now runs ungated on chromium in the REQUIRED local lane; only the pixel guard still self-skips on software GL.

Evidence

Commit 93a58d1 "fix(#725): three guards that could not fail on the thing they guard (#731)" split the spec: tests/e2e/twin-walk-visible.spec.ts:71-104 is now its own test gated only on !hasWebGL (test.skip(!hasWebGL, ...)) with a hard expect(page.locator('[data-stance]').first()).toBeVisible({timeout:120_000}); the software-GL skip now sits only on the pixel test at :125-131. That spec is NOT in chromium-gen's testIgnore (playwright.config.ts:232-241), so it executes in e2e-local.yml's chromium-gen 1/6..6/6 shards (.github/workflows/e2e-local.yml:153-158) — the required E2E (local) result lane. The source-regex guard is gone: tests/unit/walk-world-lazy.test.ts no longer exists, replaced by tests/unit/walk-world-lazy.test.tsx which renders the real component. A numeric no-GPU spawn-height proxy (issue option 2) also already exists at tests/unit/walk-spawn-height.test.ts, and the skip is documented in place (issue option 3).

What remains, stated rather than implied by closing

One genuine residual: pixel coverage still needs a GPU runner (issue option 1), and commit 93a58d1 explicitly declined to close the producer-chain gap (TwinWorld/WideCity -> onMeshReady handovers) because exercising it under jsdom needs a second, larger harness. Both are deferred-arc infra decisions — better as a fresh, accurately-titled ticket than leaving this one open under a premise that is now false. PR #731 used Refs #725 instead of Closes, which is why it is still open.

Found by a full reconciliation of every un-deferred open issue against actual repo
state (/goal triage, 55 issues). This issue's body described a world that had moved —
the recurring shape in this backlog, and the reason the triage reconciles rather than
trusting titles.


Walk-mode entry — the flagship interaction of the twin — had no CI coverage of any kind when
the deadlock in the linked issue shipped. Two guards existed and neither could fail.

1. The unit guard was a regex over source text

tests/unit/walk-world-lazy.test.ts:56-62 asserted that this line had been typed:

wantsWalkWorldRef.current = mode === 'walk';
if (mode === 'walk') tryBuildWalkRef.current?.();

Its own comment says why: "Without this the gate would be a permanent off-switch and walk mode
would never acquire collision at all — a far worse bug than the one being fixed."
The author
correctly predicted the failure and then wrote an assertion that could not detect it, because
nothing asked whether any path reached mode === 'walk'.

2. The E2E that drives this exact URL cannot run on CI runners

tests/e2e/twin-walk-visible.spec.ts navigates /chatt/?diorama&walk and screenshots the
frame — precisely the regression — and then:

test.skip(/swiftshader|llvmpipe|softwarerasterizer|swrast|software/i.test(gpu.renderer), ...)

CI runners are software-rendered, so it has never executed there. It lives in
playwright.visual.config.ts and is effectively a dev-machine-only tool. The skip is correct in
itself — a pixel guard under SwiftShader cannot tell "the app is dark" from "the environment
cannot draw" — but the consequence was never accounted for.

What is now covered, and what still is not

tests/unit/walk-world-lazy.test.tsx renders the real component with only the environment
mocked and proves walk is reachable, that the BVH stays lazy on ?ortho/plain routes, that
repeat arrivals do not rebuild (#703), and that early colliders are not lost (#702). It needs no
GPU and no backend, so it runs in Test (20.x).

Still uncovered in CI: anything about pixels — that the scene is lit, that geometry is
visible, that you are standing on the terrain rather than under it. The #651 "spawned below the
city" class of bug remains invisible to CI. Options worth weighing:

  • a GPU-enabled runner for a small visual job,
  • an assertion on the controller's spawn Y vs the sampler (numeric, no GPU) as a cheap proxy for
    "not underground",
  • accepting the gap explicitly and documenting it where the skip is written.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority:p3Low — defer (heavy work, polish, nice-to-haves)

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions