diff --git a/docs/IMPROVEMENT-LOG.md b/docs/IMPROVEMENT-LOG.md index bec9569..ff74794 100644 --- a/docs/IMPROVEMENT-LOG.md +++ b/docs/IMPROVEMENT-LOG.md @@ -266,3 +266,20 @@ Notes: Local macOS builds looked for Sys at the app root while SunPad packages Next: complete clean iOS/tvOS CI for this PR; with a person, perform the actual desktop save/reload check before marking 1.3 done, then the backed-up device session for 1.4. No new release or performance claim. + +### 2026-10-13 tasks 2.4-2.5: reject stalled and incomplete host probes +Result: two reproduced tooling defects fixed; measurement tasks remain open +Pins: SunPad main 1f2bc05 plus these script changes; runtime pins unchanged +Checkpoints: eight synthetic pipe/process regressions pass on Apple's Python + 3.9; bootstrap --sources-only and the complete source suite pass +Evidence: controller FIFO tests and benchmark runner fixtures in tests/ +Notes: An existing FIFO without a reader previously blocked beyond the input + tool's timeout. Connection now uses a nonblocking open and a bounded retry; + connected command writes retain their existing behavior. A runner exiting + during settling or after its first measurement previously could produce a + successful benchmark row. The benchmark now rejects either early exit and + records the exit status. Both failure cases were reproduced before fixing + them. All fixtures use synthetic inputs; no game or device data is involved. +Next: complete the frame breakdown and timing fields, then the three repeated + traversal routes. These tooling checks are not gameplay or speedup evidence; + the maintainer's deferred iPhone candidate check remains the device gate. diff --git a/docs/IMPROVEMENT-PLAN.md b/docs/IMPROVEMENT-PLAN.md index 5bf5f6e..50e8bcd 100644 --- a/docs/IMPROVEMENT-PLAN.md +++ b/docs/IMPROVEMENT-PLAN.md @@ -328,8 +328,8 @@ human for a device session or a hands-on verdict. | 2.1 | Symbol map fetch and verify | done | none | S (fixture); D for the real hash check | | 2.2 | Profile symbolizer | done | 2.1 | S | | 2.3 | Frame breakdown counters | in progress | none | D | -| 2.4 | Unlimited-speed desktop benchmark | in progress | none | D | -| 2.5 | Developer warp and the three routes | in progress | 2.1 | D | +| 2.4 | Unlimited-speed desktop benchmark | in progress (incomplete runs rejected; timing fields pending) | none | D | +| 2.5 | Developer warp and the three routes | in progress (pipe connection timeout covered; traversal routes pending) | 2.1 | D | | 2.6 | Baseline measurement and ranking | ready | 2.2-2.5 | D, H | | 3.1 | Chunk-validation flag for direct calls | ready | 2.6 entry gate | D | | 3.2 | Hook and mod bitmap | ready | 3.1 | D | diff --git a/scripts/bench-route.sh b/scripts/bench-route.sh index 0a7965d..c0c619a 100755 --- a/scripts/bench-route.sh +++ b/scripts/bench-route.sh @@ -34,7 +34,12 @@ echo "load-start=$(sysctl -n vm.loadavg 2>/dev/null || true)" > "$LOG" MODERNGEKKO_PERF_LOG=5 MODERNGEKKO_DEV_WARP=$WARP \ "$RUN" --game "$GAME" --module "$MODULE" --user-dir "$USER_DIR" --no-mods --graphics Metal >>"$LOG" 2>&1 & PID=$! +SLEEP_PID= stop() { + if [[ -n "$SLEEP_PID" ]]; then + kill "$SLEEP_PID" 2>/dev/null || true + wait "$SLEEP_PID" 2>/dev/null || true + fi kill -INT "$PID" 2>/dev/null || true for _ in $(seq 10); do kill -0 "$PID" 2>/dev/null || break; sleep 1; done kill -KILL "$PID" 2>/dev/null || true @@ -42,15 +47,35 @@ stop() { rm -rf "$USER_DIR" } trap stop EXIT +check_runner() { + if ! kill -0 "$PID" 2>/dev/null; then + local status=0 + wait "$PID" || status=$? + echo "bench-route: runner exited (status $status); see $LOG" >&2 + return 1 + fi +} +sleep_running() { + sleep "$1" & + SLEEP_PID=$! + while kill -0 "$SLEEP_PID" 2>/dev/null; do + check_runner || return 1 + sleep 0.1 + done + wait "$SLEEP_PID" + SLEEP_PID= + check_runner +} for _ in $(seq 600); do grep -q '\[warp\] entered' "$LOG" && break - kill -0 "$PID" 2>/dev/null || { echo "bench-route: runner exited; see $LOG" >&2; exit 1; } + check_runner sleep 1 done grep -q '\[warp\] entered' "$LOG" || { echo "bench-route: area never reached; see $LOG" >&2; exit 1; } -sleep "$SETTLE" +echo '[bench] settle-start' >> "$LOG" +sleep_running "$SETTLE" echo '[bench] measure-start' >> "$LOG" -sleep "$MEASURE" +sleep_running "$MEASURE" echo '[bench] measure-end' >> "$LOG" echo "load-end=$(sysctl -n vm.loadavg 2>/dev/null || true)" >> "$LOG" diff --git a/scripts/check-repository.sh b/scripts/check-repository.sh index 3f65bdb..5930514 100755 --- a/scripts/check-repository.sh +++ b/scripts/check-repository.sh @@ -9,6 +9,8 @@ python3 scripts/dependency-lock.py python3 tests/test_dependency_lock.py python3 tests/test_build_jobs.py python3 tests/test_package_output.py +python3 tests/test_gcpipe.py +python3 tests/test_bench_route.py ./tests/test-extraction-progress.sh ./tests/test-ipa-module.sh diff --git a/scripts/gcpipe.py b/scripts/gcpipe.py index e99b161..3679eaf 100644 --- a/scripts/gcpipe.py +++ b/scripts/gcpipe.py @@ -13,14 +13,15 @@ Buttons: A B X Y Z START L R D_UP D_DOWN D_LEFT D_RIGHT -This script opens the FIFO for writing (blocking until the runtime opens it for -reading), then executes the requested sequence with wall-clock timing so the +This script waits up to --open-timeout for the runtime to open the FIFO for +reading, then executes the requested sequence with wall-clock timing so the game sees a believable button timeline. """ from __future__ import annotations import argparse +import errno import json import os import sys @@ -38,12 +39,22 @@ def __init__(self, pipe_path: Path, timeout_s: float = 60.0): deadline = time.monotonic() + timeout_s while time.monotonic() < deadline: try: - self.fd = os.open(pipe_path, os.O_WRONLY) + fd = os.open(pipe_path, os.O_WRONLY | os.O_NONBLOCK) + except OSError as error: + if error.errno not in (errno.ENOENT, errno.ENXIO): + raise + else: + # Bound only the connection wait; keep command writes blocking. + try: + os.set_blocking(fd, True) + except OSError: + os.close(fd) + raise + self.fd = fd return - except FileNotFoundError: - time.sleep(0.25) - except BlockingIOError: - time.sleep(0.25) + remaining = deadline - time.monotonic() + if remaining > 0: + time.sleep(min(0.25, remaining)) raise RuntimeError(f"pipe never opened for reading: {pipe_path}") def send(self, command: str) -> None: diff --git a/tests/README.md b/tests/README.md index 2d68358..c1b84ef 100644 --- a/tests/README.md +++ b/tests/README.md @@ -25,6 +25,8 @@ Focused source regression gates: ./tests/test-diagnostics.sh ./tests/test-iphone-touch-layout-defaults.sh ./tests/test-game-data-setup.sh +python3 tests/test_gcpipe.py +python3 tests/test_bench_route.py ``` Before publishing or merging release-hardening work, run the combined diff --git a/tests/test_bench_route.py b/tests/test_bench_route.py new file mode 100644 index 0000000..4542e07 --- /dev/null +++ b/tests/test_bench_route.py @@ -0,0 +1,95 @@ +"""Exercise the benchmark's process handling without game data or a renderer.""" + +import csv +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + + +ROOT = Path(__file__).resolve().parents[1] + + +class BenchRouteTests(unittest.TestCase): + def run_benchmark(self, mode): + with tempfile.TemporaryDirectory(prefix="sunpad-bench-test-") as temp: + root = Path(temp) + script = root / "scripts/bench-route.sh" + script.parent.mkdir() + shutil.copyfile(ROOT / "scripts/bench-route.sh", script) + runner = root / "ref/ModernGekko/build-desktop-tools-public/moderngekko-run" + runner.parent.mkdir(parents=True) + runner.write_text("""#!/usr/bin/env python3 +import os, signal, time +from pathlib import Path +signal.signal(signal.SIGINT, lambda *_: exit(0)) +log = Path(os.environ['BENCH_TEST_LOG']) +mode = os.environ['BENCH_TEST_MODE'] +print('[warp] entered stage 9', flush=True) +while True: + text = log.read_text() + if '[bench] settle-start' in text and mode == 'settle': + exit(37) + if '[bench] measure-start' in text: + print('[perf] t=5.0 fps=60.00 vps=120.00 speed=2.0000 ' + 'efb_copies=25 efb_immediate=0 efb_wait_pct=10.00', flush=True) + if mode == 'measure': + exit(42) + time.sleep(0.02) +""") + runner.chmod(0o755) + dol = root / "ref/ModernGekko-Template/extracted/Super-Mario-Sunshine/sys/main.dol" + dol.parent.mkdir(parents=True) + dol.touch() + module = root / "fixture-module.dylib" + module.touch() + active = root / "ref/ModernGekko-Template/build/modules-macos14/GMSE01/active-module.txt" + active.parent.mkdir(parents=True) + active.write_text(str(module)) + bin_dir = root / "bin" + bin_dir.mkdir() + git = bin_dir / "git" + git.write_text("#!/bin/sh\necho fixture\n") + git.chmod(0o755) + env = dict(os.environ, PATH=str(bin_dir) + os.pathsep + os.environ["PATH"], + BENCH_TEST_MODE=mode, + BENCH_TEST_LOG=str(root / "artifacts/bench/test-noki.log")) + result = subprocess.run( + ["bash", str(script), "noki", "test", "0.3", "0.3"], + env=env, capture_output=True, text=True, timeout=8, + ) + csv_path = root / "artifacts/bench/results.csv" + rows = [] + if csv_path.exists(): + with csv_path.open() as handle: + rows = list(csv.DictReader(handle)) + log = Path(env["BENCH_TEST_LOG"]).read_text() + return result, rows, log + + def test_complete_run_writes_one_result(self): + result, rows, log = self.run_benchmark("complete") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(len(rows), 1) + self.assertEqual(rows[0]["vps"], "120.0") + self.assertIn("[bench] measure-end", log) + + def test_exit_during_settle_does_not_write_result(self): + result, rows, log = self.run_benchmark("settle") + self.assertNotEqual(result.returncode, 0) + self.assertIn("runner exited (status 37)", result.stderr) + self.assertEqual(rows, []) + self.assertNotIn("[bench] measure-start", log) + + def test_exit_after_measurement_row_does_not_write_partial_result(self): + result, rows, log = self.run_benchmark("measure") + self.assertNotEqual(result.returncode, 0) + self.assertIn("runner exited (status 42)", result.stderr) + self.assertEqual(rows, []) + self.assertIn("[perf]", log) + self.assertNotIn("[bench] measure-end", log) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_gcpipe.py b/tests/test_gcpipe.py new file mode 100644 index 0000000..3ef0431 --- /dev/null +++ b/tests/test_gcpipe.py @@ -0,0 +1,101 @@ +"""Exercise controller-pipe connection using synthetic FIFOs only.""" +import importlib.util +import os +from pathlib import Path +import subprocess +import sys +import tempfile +import time +import unittest +from unittest import mock + + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts/gcpipe.py" +SPEC = importlib.util.spec_from_file_location("gcpipe", SCRIPT) +gcpipe = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(gcpipe) + + +class PadWriterTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="sunpad pipe ") + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.pipe = self.root / "controller" + + def assert_connection_times_out(self): + # Keep a regression of the old blocking open from hanging the suite. + try: + result = subprocess.run( + [sys.executable, str(SCRIPT), "--pipe", str(self.pipe), + "--open-timeout", "0.05", "--tap", "A"], + capture_output=True, text=True, timeout=1.5) + except subprocess.TimeoutExpired: + self.fail("FIFO connection exceeded its open timeout") + self.assertNotEqual(result.returncode, 0) + self.assertIn("pipe never opened for reading", result.stderr) + + def test_missing_pipe_times_out(self): + self.assert_connection_times_out() + + def test_existing_fifo_without_reader_times_out(self): + os.mkfifo(self.pipe) + self.assert_connection_times_out() + + def test_short_timeout_does_not_wait_a_full_poll_interval(self): + clock = [10.0] + + def sleep(seconds): + clock[0] += seconds + + with mock.patch.object(gcpipe.time, "monotonic", side_effect=lambda: clock[0]), \ + mock.patch.object(gcpipe.time, "sleep", side_effect=sleep): + with self.assertRaisesRegex(RuntimeError, "pipe never opened"): + gcpipe.PadWriter(self.pipe, timeout_s=0.05) + self.assertAlmostEqual(clock[0], 10.05) + + def test_delayed_reader_receives_commands_with_blocking_writer(self): + os.mkfifo(self.pipe) + code = """ +import os +from pathlib import Path +import runpy +import sys +writer = runpy.run_path(sys.argv[1])["PadWriter"](Path(sys.argv[2]), timeout_s=2) +try: + assert os.get_blocking(writer.fd), "connected writes must remain blocking" + writer.tap("A", hold_s=0) + writer.set_stick("MAIN", 0.5, -0.5) + writer.set_trigger("R", 1) +finally: + writer.close() +""" + process = subprocess.Popen( + [sys.executable, "-c", code, str(SCRIPT), str(self.pipe)], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + try: + time.sleep(0.15) + self.assertIsNone(process.poll(), "writer did not wait for a reader") + reader = os.open(self.pipe, os.O_RDONLY | os.O_NONBLOCK) + try: + stdout, stderr = process.communicate(timeout=3) + self.assertEqual(process.returncode, 0, stdout + stderr) + self.assertEqual( + os.read(reader, 4096), + b"PRESS A\nRELEASE A\nSET MAIN 0.750 0.750\nSET R 1.000\n") + finally: + os.close(reader) + finally: + if process.poll() is None: + process.kill() + process.communicate() + + def test_unexpected_open_error_is_not_retried(self): + with mock.patch.object(gcpipe.time, "sleep") as sleep: + with self.assertRaises(IsADirectoryError): + gcpipe.PadWriter(self.root, timeout_s=1) + sleep.assert_not_called() + + +if __name__ == "__main__": + unittest.main()