Skip to content

run_all_tests.sh: .eigs test blocks have no runaway guard — an infinite loop STALLS the suite instead of failing it #648

Description

@InauguralPhysicist

Finding

The runner guards the examples against a runaway but not the test blocks. An infinite loop in any .eigs test file hangs the entire suite until the CI job's hard limit, instead of reporting a failure.

# tests/run_all_tests.sh:79 — check_eigs_suite, used by every .eigs test block:
out=$(./eigenscript "../tests/$file" </dev/null 2>&1); rc=$?     # <- no guard

# tests/run_all_tests.sh:3074 — the [97] examples section:
if command -v timeout >/dev/null 2>&1; then EX_TMO="timeout 60"  # <- guarded

Same runner, same risk, opposite treatment.

Why it's not hypothetical

#637 was exactly this bug in shipped code: pad_left with an empty pad char spun forever. Its regression tests (PL4/PR5, added in #646) sit inside check_eigs_suite. Verified: reverting lib/string.eigs while keeping those tests makes the suite hangexit=124 only because I wrapped it in timeout by hand.

So today's protection against a #637 regression is "CI eventually times out and someone reads the log", not "a test fails." The difference is a red X versus a mystery — and the mystery costs a debugging session before anyone even learns which block stalled.

Why it matters more than it looks

Two of today's flakes (#616, #604) each cost a review cycle largely because a red result was ambiguous. A stall is worse than either: no failing assertion, no section name, no output — just a job that dies at the limit. It's the same failure class this repo already decided against everywhere else (rc_ok gates on exit codes precisely so a crash after correct output can't pass).

Fix

Wrap check_eigs_suite's invocation the way [97] already does — the pattern, the fallback (gtimeout on BSD userland), and the "no wrapper if absent" degradation are all already written 3000 lines below:

out=$($EIGS_TMO ./eigenscript "../tests/$file" </dev/null 2>&1); rc=$?

with rc=124 reported as a timeout failure naming the block, rather than folding into the generic path.

Pick the budget deliberately. #616 is open precisely because [97]'s timeout 60 is too tight for invariant_weak.eigs under ASan (60.5s standalone). A test-block guard is a runaway backstop, not a latency assertion — it should be generous (180s+) so it never fires on a slow-but-working test on the N3350. Worth fixing #616's guard in the same pass, since it's the same judgment.

Test

A scratch .eigs with loop while 1 == 1: run through check_eigs_suite must report a named failure and let the suite continue, rather than hang.


Found by @Nitjsefnie while fixing #637 (#646) — spotted as an out-of-scope note on their own PR, which is exactly where the good ones come from.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions