Gate Godot-runtime tests at merge via affected-selection (probe-gate second corpus); retire run_godot_tests.sh nf-1wa ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/1154 by tools/beads/import_github.py's reconcile pass.

From Aedan (chat, 2026-08-20, verbatim):

> additional consideration is run_godot_tests should be in presubmit, but again we only want to run tests affected by the PR, again I wonder if bazel solves this systemically. This is separate issue to 1144

The gap

run_godot_tests.sh sits outside every gate: not a presubmit lane, not in the merge-time probe gate, a hand-maintained scene list in shell, and (per #1144) its failure mode is a silent infinite hang. Nothing catches a regression in the Godot-runtime tests — the STYLE_GUIDE §1.2 test class exists but is unenforced at merge time (E8: main must always work).

The affected-selection machinery already exists — this is a second consumer, not a new system

Design 005b built exactly the requested shape for probes: tools/pr/affected_probes.py (diff → affected set, T1) feeding tools/pr/probe_gate.py's subset run at merge (T2/T2a), backgrounded from a dev worktree. Headless Godot tests are the same execution shape as probes — a headless scene launch that self-asserts — so the natural fix is:

A. Fold *_test.tscn scenes into the merge gate as a second corpus (recommended default). Teach the affected-selection path to map a diff onto Godot test scenes the same way it maps onto probes (game-dir intersection; shared/ → all), and have probe_gate.py launch them in the same sweep with the same timeout/reporting machinery. Then delete run_godot_tests.sh (B12, and the shell policy has wanted it gone anyway — it already carries a load-bearing || true war story).

Placement note: "in presubmit" resolves to "in the merge gate", not a fifth presubmit lane. Presubmit is deliberately diff-agnostic ("all four lanes every time", ~40s); affected-only selection needs a before/after diff, which only the merge path has (merge_pr.py already computes the exact pre-merge tree for affected_probes). A diff-agnostic run-everything lane is the thing the "only tests affected by the PR" requirement rules out as the corpus grows.

B. (Deeper, optional) Ask whether .GodotTest.cs should exist as a separate category at all. The two extant tests overlap heavily with what probes already do: #1143's golden-image test is probe-screenshot territory, and most of ControllerNavTest's checks are expect_focus-shaped. The residual distinction is arbitrary C#-side assertions (e.g. scroll.FollowFocus) that the JSON step vocabulary can't express — but that vocabulary is already extensible per-game (expect_chit, expect_bar). If the answer is "a GodotTest is just a probe written in C#", option A's second corpus is the permanent shape; if the categories merge, even better. Worth a look while doing A, not a prerequisite.

Bazel: already litigated — cite design 005 §4A, don't re-run it

Design 005 §4 option A evaluated Bazel in its strongest form (custom Starlark rules + a Roslyn-driven aspect generating the C# graph, per Aedan's own correction quoted there) and rejected it explicitly at current scalerules_dotnet at 0.18.x, a third declaration of every file, Godot never modelled upstream — while stating the rejection is *current-scale, not standing*, and keeping the long-term option open. Adding a two-test Godot corpus to the existing selection machinery does not move that scale math. The trigger for reopening 005 §4A is corpus/game/session growth making the bespoke selection creak (the #620 shared-cache pressure case), not this gap.

Ordering

Blocked-ish on #1144: gating on these tests today gates on an infinite hang. Either land #1144's rename first, or lead with a timeout wrapper so a broken test fails noisily (D2) — the timeout is wanted regardless.

Dependencies

None.

Comments

No comments.

Add a comment