test: run the gfx examples — no build variant ever did (#886) - #901
Conversation
There was a problem hiding this comment.
Pull request overview
This PR closes a long-standing test coverage gap by ensuring the gfx_ example programs under examples/ are executed when the current build actually supports the gfx extension (rather than being unconditionally skipped by content). It does this by adding a gfx capability probe and running gfx demos headlessly under SDL’s dummy video driver with a short timeout and a memory cap, and updates CI and the changelog accordingly.
Changes:
- Gate example skipping on gfx build capability and run gfx demos headlessly (treating
rc=124timeout as success for interactive event loops). - Update CI job step name to reflect gfx examples being exercised in the gfx build suite run.
- Document the change (and historical context) in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/run_all_tests.sh | Adds gfx capability probing and executes gfx examples under dummy SDL driver with timeout+memory cap instead of unconditional content-based skipping. |
| CHANGELOG.md | Records the new coverage behavior and the motivation/history from #886. |
| .github/workflows/ci.yml | Renames the gfx-suite CI step to reflect that gfx examples are now exercised. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| EX_GFX_PROBE=$(mktemp /tmp/eigs_ex_gfx_XXXXXX.eigs) | ||
| echo 'print of (gfx_text_width of ["m", 1])' > "$EX_GFX_PROBE" | ||
| EX_HAS_GFX=0 | ||
| if ! ./eigenscript "$EX_GFX_PROBE" 2>&1 | grep -q "undefined variable"; then EX_HAS_GFX=1; fi | ||
| rm -f "$EX_GFX_PROBE" |
| EX_OUT=$( cd "$(dirname "$f")" && ulimit -v 2000000 2>/dev/null; \ | ||
| cd "$(dirname "$f")" && SDL_VIDEODRIVER=dummy timeout 3 \ | ||
| "$EIGS_ABS" "$(basename "$f")" </dev/null 2>&1 ); EX_RC=$? |
391328f to
e4784c8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/run_all_tests.sh:3922
- The gfx-example runner hardcodes
timeout 3, but this script explicitly supports macOS/BSD userlands by falling back togtimeout(or no wrapper) viaEIGS_TMO. Callingtimeoutdirectly will fail on systems that only havegtimeout, and it also ignores the existing detection logic. Reuse the detected wrapper command for the short gfx-demo timeout, and skip gfx demos if no timeout wrapper is available.
# UI run can take the whole machine.
EX_OUT=$( cd "$(dirname "$f")" && ulimit -v 2000000 2>/dev/null; \
cd "$(dirname "$f")" && SDL_VIDEODRIVER=dummy timeout 3 \
"$EIGS_ABS" "$(basename "$f")" </dev/null 2>&1 ); EX_RC=$?
tests/run_all_tests.sh:3890
- The section header comment implies rc=124 means the program reached parse/module-load/layout successfully, but rc=124 only indicates the
timeoutwrapper fired. That’s a reasonable proxy for these demos, but it’s not guaranteed (a hang earlier in startup would also be rc=124). Consider tightening the wording so it’s clear timeout is being used as a proxy rather than a definitive signal.
# the dummy driver no quit event ever arrives, so reaching it means timing out.
# That is the PASS signal here — rc 124 means the program got through parse,
# module load, widget construction and layout without erroring, which is the
# failure class this section exists to catch. It deliberately does NOT verify
# loop behavior; [132] and the lib/ui sections own that.
Section [97] skipped example programs by CONTENT:
if grep -qE 'gfx_|net_listen' "$f"; then EX_SKIP=...; continue; fi
That is unconditional. `make gfx` builds the extension and [132]
exercises the real SDL renderer, but the nine gfx examples were skipped
in EVERY variant, so nothing covered them anywhere. The suite reported
it honestly ("10 gfx skipped"); the gap was that nothing else covered
them either.
The skip is now gated on build CAPABILITY — the same probe [132] uses —
and under a gfx build each demo runs against the dummy video driver with
a memory cap (an unbounded UI run can take the whole box).
A gfx demo ends in `ui.app_loop`, an interactive event loop that no quit
event ever reaches headlessly, so REACHING it (rc 124) is the pass
signal: the program got through parse, module load, widget construction
and layout without erroring. That is the failure class this section
exists to catch. It deliberately does not verify loop behavior — [132]
and the lib/ui sections own that — and the header says so rather than
implying more coverage than it has.
Validated by planting the issue's own bug back in (a call to the private
`ui._layout` in ui_hex.eigs): the section fails with
"gfx demo errored before its event loop". Both paths checked — with gfx
the example count goes 72 -> 81; without it, 72 run and 10 skip as
before.
Reporting honestly: the broken example the issue names is ALREADY fixed.
`b49e85c` — an unrelated sandbox/zlib PR — deleted the offending line by
accident after the issue was filed. Which is the issue's point exactly:
with nothing running these, they break and get fixed invisibly. This
commit closes the coverage gap, not a live breakage.
The net demo still needs `make net` and a free port, so it stays
content-skipped, and the summary line now says which reason applied.
Suite 3863/3863 under `make gfx`, 3804/3804 under `make` (release).
Closes #886
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e4784c8 to
8559b7a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/run_all_tests.sh:3900
- gfx examples are treated as runnable whenever gfx is present, but the gfx-demo path later relies on
timeoutto terminate the headless event loop. On systems withouttimeout/gtimeout(which this runner explicitly supports by degrading), the suite would hang when it hits the first gfx example. Gate gfx-demo inclusion on the presence of the timeout wrapper, and emit a distinct header when gfx exists but no wrapper is available.
EX_GFX_PROBE=$(mktemp /tmp/eigs_ex_gfx_XXXXXX.eigs)
echo 'print of (gfx_text_width of ["m", 1])' > "$EX_GFX_PROBE"
EX_HAS_GFX=0
if ! ./eigenscript "$EX_GFX_PROBE" 2>&1 | grep -q "undefined variable"; then EX_HAS_GFX=1; fi
rm -f "$EX_GFX_PROBE"
tests/run_all_tests.sh:3922
- The gfx-demo execution path hard-codes
timeout 3, which (a) bypasses the runner’s timeout/gtimeout detection (breaking macOS where onlygtimeoutmay exist) and (b) doesn’t match the PR description’s stated 12s budget for reachingui.app_loop, which could cause flakes on slower CI. Use the detected timeout wrapper and a configurable/appropriate budget.
if grep -q 'gfx_' "$f"; then
if [ "$EX_HAS_GFX" != "1" ]; then EX_SKIP=$((EX_SKIP + 1)); continue; fi
# #886: reaching the event loop (rc 124) is the pass; any other
# nonzero rc is a real setup failure. Memory-capped — an unbounded
# UI run can take the whole machine.
EX_OUT=$( cd "$(dirname "$f")" && ulimit -v 2000000 2>/dev/null; \
cd "$(dirname "$f")" && SDL_VIDEODRIVER=dummy timeout 3 \
"$EIGS_ABS" "$(basename "$f")" </dev/null 2>&1 ); EX_RC=$?
tests/run_all_tests.sh:3954
- The [97] PASS summary says examples “run clean” even when gfx demos are expected to time out as the success signal, and it keys off EX_HAS_GFX rather than whether gfx demos actually ran (e.g., gfx build but no timeout wrapper). Adjust the message/branching to reflect the real behavior and the new EX_CAN_RUN_GFX gate.
if [ "$EX_HAS_GFX" = "1" ]; then
echo " PASS: all $EX_PASS example programs run clean (gfx demos included; $EX_SKIP net skipped)"
else
echo " PASS: all $EX_PASS example programs run clean ($EX_SKIP gfx/net skipped — no gfx build)"
fi
Section [97] skipped example programs by content:
That is unconditional.
make gfxbuilds the extension and [132] exercises the real SDL renderer — but the ninegfx_examples were skipped in every variant, so nothing covered them anywhere. The suite reported this honestly ("10 gfx skipped"); the gap was that nothing else covered them either.What changed
The skip is now gated on build capability (the same probe [132] uses). Under a gfx build each demo runs against the dummy video driver, memory-capped — an unbounded UI run can take the whole machine.
A gfx demo ends in
ui.app_loop, an interactive event loop that no quit event ever reaches headlessly. So reaching it (rc 124) is the pass signal: the program got through parse, module load, widget construction and layout without erroring, which is the failure class this section exists to catch. It deliberately does not verify loop behavior — [132] and the lib/ui sections own that — and the section header says so rather than implying more coverage than it has.Validation
Planted the issue's own bug back in (a call to the private
ui._layoutinui_hex.eigs):Both paths checked:
make gfxmake(release)One honest correction to the issue
The broken example it names is already fixed.
b49e85c— an unrelated sandbox/zlib PR — deleted theui._layout of [root, 0, 0]line by accident, after the issue was filed:All nine gfx demos currently reach their event loop cleanly. So this PR closes the coverage gap, not a live breakage — and that incidental fix is the issue's own argument made concrete: with nothing running these, they break and get fixed invisibly.
This is a tests-and-CI change only (
tests/run_all_tests.sh, a CI step name, CHANGELOG); no runtime source is touched, so the sanitizer gates are unaffected and CI's own ASan job covers them.Closes #886
🤖 Generated with Claude Code