From 89fca8007b5ca98a47b6a72cfd94c11e228d9b16 Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Sun, 2 Aug 2026 02:17:17 -0400 Subject: [PATCH] Assert the log line a failed report leaves behind A mutation run over src/runtime.c found that deleting the `LOG_ERROR_WITH_MESSAGE` in `report_and_reraise()` left the whole suite green. That line *is* the visible half of the "a failed report must not swallow the program's own error" fix -- without it the secondary failure goes nowhere and the only evidence is the original error still being the one raised. Nothing looked at it, so nothing noticed. `tests/trap_verbs.c` now redirects `akerr_log_method` into a buffer and drives the path with a sink that refuses every write, which is the only way left to make reporting fail now that the `TRAP` dispatch no longer allocates. It asserts the log line and that `errclass` still records the program's own error, and it kills the mutant. TODO.md gains a section for what the run found and what it did not: it was cut off at 551 of 997 mutants after ninety minutes, so 446 are unexamined and a full run belongs in the release workflow rather than here. The three regions this work touched were all covered, and the survivors in the skipped-block guard are named and assessed -- three of the four are equivalent for any program that can actually be written, and the one that is not would need a test with a loop on line 1. Also recorded: src/variable.c at 68.2% with every mutant on the inline-storage lines killed, and src/runtime_trap.c at 82.5% with no survivors in `set_error_variables()`. Co-Authored-By: Claude Opus 5 (1M context) --- TODO.md | 33 ++++++++++++++++ tests/trap_verbs.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/TODO.md b/TODO.md index 2c54190..325d473 100644 --- a/TODO.md +++ b/TODO.md @@ -1858,6 +1858,39 @@ the reference's semantics reproduced faithfully; the third is the port's own. reentrant" and "do not grow the runtime by a third for a scratch buffer" are both right and picking between them is a decision. +### Mutation-testing gaps in the files the game work touched + +Recorded the way ยง8's CI note records `src/audio_tables.c`: a real test gap rather than a +reason to avoid the file. + +34. **`src/runtime.c` has never had a complete mutation run, and the one attempted here + was cut off at 551 of 997 mutants** after ninety minutes. What it did cover included + all three regions the value-pool, trap and skipped-block work touched, and the + survivors there are worth naming: + + - `report_and_reraise()`: **closed.** Deleting the `LOG_ERROR_WITH_MESSAGE` survived, + because nothing looked at the log -- and that line *is* the visible half of item + 33's second fix. `tests/trap_verbs.c` now redirects `akerr_log_method` into a buffer + and drives the path with a sink that refuses every write, which kills it. + - The skipped-block guard (`src/runtime.c`, the `BEND` test): four survivors, and + three of them are equivalent for any program that can be written -- `loopFirstLine + != 0` mutated to `!= 1` differs only for a loop on line 1, `waitingForCommand[0]` + to `[1]` only for a one-character wait, and flipping `strcmp(...) == 0` to `!= 0` + moves the release from the `FOR` to the `NEXT` that must follow it. A test with a + loop on line 1 would kill the first. The other two are noted rather than chased. + - A NULL-argument guard in `akbasic_runtime_reserve_globals()`, which is the same + survivor class every file in this tree has: nothing calls these with NULL. + + The remaining 446 mutants are unexamined. A full run is a release-workflow job, not a + per-commit one -- `.gitea/workflows/release.yaml` already mutates the whole tree -- and + what this entry is for is that the *partial* result has been read rather than filed + unopened. + + Measured alongside: `src/variable.c` scores 68.2% with **every mutant on the inline- + storage lines killed**, and `src/runtime_trap.c` 82.5% with **no survivors** in + `set_error_variables()`. Both files' remaining survivors are pre-existing and are + mostly NULL guards and `MAX - 1` arithmetic in string helpers. + ### Found while writing a game against the interpreter A complete Breakout, sprites and text grid and keyboard, running for minutes at a time. Nothing diff --git a/tests/trap_verbs.c b/tests/trap_verbs.c index 8cd9e61..9ca57d2 100644 --- a/tests/trap_verbs.c +++ b/tests/trap_verbs.c @@ -11,6 +11,8 @@ * dialect has no bare variable names. See TODO.md section 5. */ +#include +#include #include #include @@ -291,6 +293,98 @@ static void test_handler_runs_with_a_full_variable_table(void) harness_stop(); } +/* + * A sink whose writes fail, and a place to catch what libakerror logs. + * + * Between them these drive the one path that has no BASIC-visible output at all: + * reporting an error failing while an error is already being reported. + */ +static char LOGGED[2048]; + +static void capture_log(const char *format, ...) +{ + va_list args; + size_t used = strlen(LOGGED); + + va_start(args, format); + vsnprintf(LOGGED + used, sizeof(LOGGED) - used, format, args); + va_end(args); +} + +static akerr_ErrorContext AKERR_NOIGNORE *failing_write(akbasic_TextSink *self, const char *text) +{ + PREPARE_ERROR(errctx); + + (void)self; (void)text; + FAIL_RETURN(errctx, AKERR_IO, "this sink always fails"); +} + +static akerr_ErrorContext AKERR_NOIGNORE *failing_clear(akbasic_TextSink *self) +{ + PREPARE_ERROR(errctx); + + (void)self; + FAIL_RETURN(errctx, AKERR_IO, "this sink always fails"); +} + +/** + * @brief A failure while *reporting* does not replace the program's own error. + * + * `report_and_reraise()` used a plain `PASS` around the report, so anything that + * went wrong while reporting became the error that propagated -- and the one the + * program actually made was gone. A user was told "could not write to the sink" + * about a subscript that was out of range. + * + * Provoked here by a sink that refuses every write, which is the only way left + * to make reporting fail now that the `TRAP` dispatch no longer allocates. The + * two assertions are the two halves of the fix: the secondary failure is + * *logged*, where a developer will see it, and the *original* status is what + * comes back out. + * + * The log assertion needs `akerr_log_method` redirected, and that is the point: + * a mutation run showed the log line could be deleted with the whole suite still + * passing, because nothing looked at it. TODO.md section 6 item 33. + */ +static void test_a_failed_report_keeps_the_original_error(void) +{ + akerr_ErrorLogFunction saved = akerr_log_method; + akerr_ErrorContext *raised = NULL; + + LOGGED[0] = '\0'; + TEST_REQUIRE_OK(harness_start(NULL)); + /* Out of bounds, which is what the program did wrong. */ + TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME, + "10 DIM Q#(2)\n" + "20 PRINT Q#(9)\n")); + TEST_REQUIRE_OK(akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN)); + + HARNESS_SINK.write = failing_write; + HARNESS_SINK.writeln = failing_write; + HARNESS_SINK.clear = failing_clear; + akerr_log_method = capture_log; + raised = akbasic_runtime_run(&HARNESS_RUNTIME, 0); + akerr_log_method = saved; + test_discard_error(raised); + + /* + * The secondary failure is logged rather than swallowed, and it says which + * one it was: "could not report" names the reporting, and the status on the + * end of the line is the *sink's* AKERR_IO. A mutation run showed this line + * could be deleted with the whole suite still green, because nothing looked + * at it. + */ + TEST_REQUIRE(strstr(LOGGED, "could not report a BASIC error") != NULL, + "the failure to report must be logged, got \"%s\"", LOGGED); + + /* + * And the program's own error is still what happened: the class was recorded + * before the write that failed, so the run ended as a runtime error rather + * than carrying on past a statement that did not work. + */ + TEST_REQUIRE_INT(HARNESS_RUNTIME.errclass, AKBASIC_ERRCLASS_RUNTIME); + harness_stop(); +} + int main(void) { test_trap_catches(); @@ -304,5 +398,6 @@ int main(void) test_err_function(); test_error_globals_are_reserved(); test_handler_runs_with_a_full_variable_table(); + test_a_failed_report_keeps_the_original_error(); return akbasic_test_failures; }