test-framework: Add headless ROM regression suite

[rene]

Description

Adds a test harness that boots ROMs through the emulator core with no SDL, no X server and no audio device, and turns each run into a PASS/FAIL/SKIP verdict. Intended for bulk regression testing while fixing mappers and timing: a full 790-ROM sweep takes a few minutes and reports exactly which titles changed state.

harness/ rnes_headless.c plus a null HAL, linking the emulator core
from ../src without the SDL front end
rnestest/ ROM list handling, iNES/NES 2.0 header decoding, the verdict
engine and JSON/JUnit/GitHub reporters
tests/ unit tests for the verdict engine and the header decoder

A ROM fails on definitive signals only -- crash, hang, CPU jam, a blank screen, or a CPU stuck in a tight loop -- each of which must hold across the whole sample window, so a screen that animates even once passes. Freeze detection is opt-in because plenty of working games hold a static title screen for several seconds.

"Blank" is measured by content coverage, the share of pixels differing from the dominant colour, rather than by counting distinct colours. Copyright cards and story text are white-on-black -- exactly two colours -- yet perfectly healthy; counting colours alone marked ~28 working titles blank.

roms/ is empty by design: only license-clean ROMs belong there, so the CI job stays green and does nothing until some are added.

[rene]

@claude

[github-actions]

Claude finished @rene's task —— View job

---

Code Review

• Gather context
• Read C source files (null_hal.c, rnes_headless.c)
• Read Python source files
• Post inline review comments
• Final summary

---

Overall this is a well-structured addition. The headless harness is cleanly separated from the SDL front-end, the verdict heuristics are thoughtfully designed to minimise false positives, and the Python layer is properly layered. A few issues worth addressing before merging:

---

test-framework/harness/rnes_headless.c

Line 304 — parse_buttons: silent truncation of long button names

char buf[64];
strncpy(buf, list, sizeof(buf) - 1);

The buffer is only 64 bytes. A crafted or malformed --buttons argument longer than 63 bytes is silently truncated, which may split a token mid-name (e.g. "right" becomes "righ"), producing a silent no-op rather than an error. Since list comes directly from argv, prefer checking strlen(list) first and bailing out with a usage message, or use snprintf and detect truncation.

---

Lines 495–500 — frames_run++ double-increment on hang

uint64_t frame_ticks = run_one_frame(tick_cap);
if (frame_ticks == 0) {
    status_ok = 0;
    frames_run++; /* count the attempted frame */
    break;
}

frames_run is the loop variable; the for header will not execute its increment on a break, so the manual frames_run++ is correct. However the comment "count the attempted frame" is slightly misleading: frames_run at this point already equals the 0-based index of the attempted frame. After the manual increment it becomes frames_run + 1 which is the 1-based count — consistent with how it is used in the JSON output ("frames_run": frames_run). This is fine, but the comment should clarify it's converting from 0-based index to 1-based count to avoid confusion for future readers.

---

Line 120 — HT_SIZE and HT_COUNT global state not reset between calls

static uint32_t ht_color[HT_SIZE];
static uint32_t ht_count[HT_SIZE];
static uint8_t  ht_used[HT_SIZE];

analyze_framebuffer resets ht_used with memset (line 134) but does not reset ht_color or ht_count. For correctness this is fine since a slot is only read when ht_used[idx] is set. However, ht_count values from a previous call persist in unused slots, which makes the table harder to reason about in isolation and would silently corrupt results if a bug ever skipped the ht_used guard. A one-line memset(ht_count, 0, sizeof(ht_count)) alongside the existing memset(ht_used, ...) would be safer and costs nothing at this frame rate.

---

Lines 540–557 — Both adaptive extensions may race to set limit

if (frames_run + 1 >= total_frames && !screen_changed &&
    freeze_frames > limit && ...) {
    limit = freeze_frames;
}
if (frames_run + 1 >= total_frames && !content_seen &&
    blank_frames > limit && ...) {
    limit = blank_frames;
    blank_extended = 1;
}

If both conditions are true simultaneously, the second if overwrites limit with blank_frames (which may be smaller than freeze_frames set by the first block), silently discarding the freeze extension. The blank_extended flag is only set in the second block, so the early-exit condition at line 564 may fire before the freeze confirmation is complete. The two extensions should use limit = max(limit, freeze_frames) / limit = max(limit, blank_frames) to be composable.

---

test-framework/harness/null_hal.c

Line 120–127 — Detached audio drain thread accesses APU ring buffer after _exit

static void *audio_drain_loop(void *arg)
{
    for (;;)
        apu_get_samples(NULL, buf, sizeof(buf));
}
...
pthread_detach(drain_th);

rnes_headless.c calls _exit(EXIT_OK) (line 613) to bypass atexit handlers and avoid joining the drain thread. On Linux _exit terminates all threads immediately, so the drain thread is killed at the same moment the emulation loop finishes — this is intentional and safe. However, if the code is ever ported to a platform where _exit does not terminate all threads (e.g. embedded libc), the drain thread will access freed APU state. A comment referencing the _exit call in rnes_headless.c (or a pthread_cancel with cancellation point inside apu_get_samples) would make the dependency explicit.

---

test-framework/rnestest/runner.py

Lines 99–107 — _parse_report accepts any JSON object on any line

for line in reversed(stdout.strip().splitlines()):
    line = line.strip()
    if line.startswith("{") and line.endswith("}"):
        try:
            return json.loads(line)
        except json.JSONDecodeError:
            return None
return None

The harness guarantees exactly one JSON object, but if the emulator core ever prints a one-line debug dict to stdout (e.g. a mapper tracing {...}), the last such line would be accepted as the report. Since the harness redirects core output through the same stdout, this is a latent issue. Consider adding a sentinel key check: if "status" in parsed before returning, so only genuine harness reports are accepted.

---

test-framework/rnestest/romheader.py

Lines 39 — iNES detection condition is overly strict

if (flag7 & 0x0C) == 0 and sflags == 0:
    fmt = FMT_INES
elif (flag7 & 0x0C) == 0x0C:
    fmt = FMT_NES2
else:
    fmt = FMT_UNKNOWN

When (flag7 & 0x0C) == 0 but sflags != 0 (dirty bytes 12–15, common in older ROMs dumped with metadata in the header padding), the format falls through to FMT_UNKNOWN and then the 12-bit NES 2.0 mapper formula is used, yielding a wrong mapper number. The comment says this mirrors romdec.c, so this is intentional parity, but it means some perfectly playable iNES ROMs will get FMT_UNKNOWN-derived mapper numbers that don't match any implemented mapper and will be pre-skipped. If this is causing unexpected SKIPs in practice, adding a secondary iNES fallback ((flag7 & 0x0C) == 0 with non-zero sflags → treat as iNES with 8-bit mapper) would reduce the false-skip rate for older dumps.

---

test-framework/Makefile

Line 54 — No -Werror or sanitizer flags in CI build

CFLAGS ?= -O2 -Wall

The harness only uses CFLAGS ?=, so CI could easily inject -fsanitize=address,undefined or -Werror to catch issues automatically. Consider adding a sanitize target or documenting the recommended CI flags in the README so the test infrastructure itself benefits from the same discipline as the emulator core.

---