From f8425b872909fa2680842cc5d55c355c63bfa7ff Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Fri, 31 Jul 2026 08:15:08 -0400 Subject: [PATCH] Gate mutation testing on a measured score, not an inherited one The threshold had been 80 against src/stdlib.c alone. There are three more sources now and nobody had measured them, so that number was a guess carried forward. Measured: 72.3%, 188 of a 260-mutant sample from the 1701 the four sources generate. Gate set to 65 -- a ratchet with headroom for the runner and for the sample shifting as sources change, not a target. A sample rather than the whole set, because 1701 rebuilds and test runs is hours. --max-mutants samples by even index rather than at random, so the same 260 run every time and the gate stays reproducible; sampling all four files beats exhausting one of them, which is what this job did before. 72.3% against the 89.6% reported at 0.1.0 is a change in denominator, not a regression in the tests. That figure covered one 561-line file; this covers four totalling 1716 lines, and most of the added surface is argument validation whose mutants are frequently *equivalent* -- 12 of the 72 survivors are `errno = 0` deleted from a wrapper whose libc call always sets errno, which no test that could be written would catch. The README breaks all 72 down and says which are worth acting on; TODO.md 2.4 carries the three clusters that are. Two of them were real and are fixed here and in the previous commit: the right child's `depth + 1` in the depth-first walk, and aksl_tree_remove on an empty tree, which without its guard dereferences NULL. Neither had a test; both do now. That is what the harness is for. Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/ci.yaml | 34 +++++++++++++++++------- .githooks/pre-push | 6 ++++- README.md | 56 +++++++++++++++++++++++++++++----------- TODO.md | 30 ++++++++++++++++++++- tests/test_collections.c | 20 ++++++++++++++ 5 files changed, 120 insertions(+), 26 deletions(-) diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 87ee98e..1ffaf68 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -136,21 +136,37 @@ jobs: sudo apt-get update -y sudo apt-get install -y cmake gcc moreutils python3 # Verify the tests actually catch bugs: break the library many ways and - # confirm the suite fails. Gated on src/stdlib.c (fast, deterministic); - # run the full default target locally for the macro header as well. + # confirm the suite fails. # - # The threshold is a ratchet, not a quality bar. The score is 89.6% - # (155/173 killed) now that TODO.md sections 1.2-1.6 have tests; it was - # 46.8% when only the list and tree functions were covered. 80 leaves - # headroom for the runner while still failing on a real regression (tests - # deleted, or new untested code added). The 18 survivors are listed in the - # published report -- each one is a missing assertion. + # A sample rather than the whole set. The four sources generate 1701 + # mutants and each one is a full rebuild and re-run of the suite, which is + # hours. --max-mutants samples by even index, not at random, so the same + # 260 run every time and the gate is reproducible -- and sampling all four + # files beats exhausting one of them, which is what this job used to do. + # + # The threshold is a ratchet, not a quality bar. The measured score is + # 72.3% (188/260). 65 leaves headroom for the runner and for the sample + # shifting as sources change, while still failing on a real regression -- + # a test deleted, or new untested code added. + # + # It is well below the 89.6% this job reported at 0.1.0, and that is a + # change in denominator rather than in the tests: that figure covered one + # 561-line file, this one covers four totalling 1716 lines, most of the + # new surface being argument validation whose mutants are frequently + # equivalent. `errno = 0` deleted from a wrapper whose libc call always + # sets errno cannot be distinguished by any test that could be written. + # The survivors worth acting on are named in TODO.md; raise the gate as + # they become assertions. - name: mutation testing run: | python3 scripts/mutation_test.py \ --target src/stdlib.c \ + --target src/string.c \ + --target src/stream.c \ + --target src/collections.c \ + --max-mutants 260 \ --junit mutation-junit.xml \ - --threshold 80 + --threshold 65 # Publish even when the threshold gate fails, so survivors are visible -- # each one is a missing test. Display-only (fail_on_failure: false); the # --threshold above is the gate. annotate_only avoids the Checks API 404 diff --git a/.githooks/pre-push b/.githooks/pre-push index 81e9922..e15b411 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -30,7 +30,7 @@ set -u # Keep in sync with the --threshold in .gitea/workflows/ci.yaml, so a push that # would fail CI fails here first. -MUTATION_THRESHOLD="${AKSL_MUTATION_THRESHOLD:-80}" +MUTATION_THRESHOLD="${AKSL_MUTATION_THRESHOLD:-65}" ZERO_SHA=0000000000000000000000000000000000000000 @@ -102,11 +102,15 @@ if [ "${AKSL_HOOK_MUTATION:-0}" = "1" ]; then echo "pre-push: mutation testing, threshold ${MUTATION_THRESHOLD}% (this takes a while)" # Deliberately not wrapped in run(): this one is slow enough that you want # to watch it make progress. + # Sampled, like CI: the full set is 1701 mutants and each is a rebuild plus + # a full test run. --max-mutants samples by even index rather than at + # random, so this is the same 260 CI gates on. if ! python3 scripts/mutation_test.py \ --target src/stdlib.c \ --target src/string.c \ --target src/stream.c \ --target src/collections.c \ + --max-mutants "${AKSL_HOOK_MUTANTS:-260}" \ --threshold "$MUTATION_THRESHOLD"; then echo >&2 echo "pre-push: mutation score below ${MUTATION_THRESHOLD}%. Push aborted." >&2 diff --git a/README.md b/README.md index 59c0f9e..6adffec 100644 --- a/README.md +++ b/README.md @@ -424,23 +424,49 @@ run prints every survivor with `file:line` and the exact edit. The harness never touches your working tree — it copies the repo to a scratch directory and mutates the copy. -CI runs the `src/stdlib.c` set with `--threshold 80`. That is a regression ratchet -rather than a quality bar: the current score is **89.6% (155/173 killed)**, up -from 46.8% before the wrapper tests landed. Raise the threshold as the remaining -survivors are turned into assertions. +CI runs a 260-mutant sample across all four sources with `--threshold 65`. +A sample, because 1701 mutants each needing a full rebuild and test run is +hours — and `--max-mutants` samples by *even index*, not at random, so the same +260 run every time and the gate is reproducible. Sampling all four files beats +exhausting one of them, which is what this job used to do. -The 18 survivors cluster in three places, and each names a real gap rather than a -test-harness artifact: +**Where it stands: 72.3% (188/260 killed).** That is a ratchet with headroom, +not a target. -- **Statements whose absence nothing observes** — deleting `free(ptr)`, - `obj->next = NULL`, or a `SUCCEED_RETURN` leaves behaviour the suite does not - look at (a leak, a stale pointer, a success that was already NULL). -- **The `aksl_list_append` cycle/tail walk** (`tail = slow`, `slow = slow->next`, - `tail = fast`) — the function is broken in exactly this area (`TODO.md` §2.1.1), - so its known-failing test cannot pin the internals yet. -- **`lalloc`/`lfree` defaulting in `aksl_tree_iterate`** — dead parameters - (§2.2.8): they are defaulted and then never called, so inverting the guard - changes nothing observable. +It is well below the 89.6% this project reported at 0.1.0, and the difference is +denominator rather than tests. That figure covered one 561-line file; this covers +four totalling 1716 lines, and most of the new surface is argument validation +whose mutants are frequently *equivalent* — a mutation that cannot change +observable behaviour, so no test could ever kill it. The clearest example: + +```c +errno = 0; /* delete this line */ +*dst = malloc(size); +FAIL_ZERO_RETURN(e, *dst, AKSL_ERRNO_OR(ENOMEM), "%zu bytes", size); +``` + +Deleting the `errno = 0` is undetectable, because `malloc` always sets `errno` +when it fails. The line is still right to have — it is what makes +`AKSL_ERRNO_OR`'s contract sound, and it matters for the calls that *don't* set +`errno` — but no test distinguishes the two versions. Twelve of the 72 survivors +are that line in twelve different wrappers. + +The survivors do break down usefully: + +| Survivors | What | Verdict | +|---|---|---| +| 12 | `errno = 0` deleted before a call that always sets `errno` | Equivalent. Not a missing test. | +| 14 | `FAIL_*` guards deleted or their constants shifted | Mixed — the constant shifts are undetectable where the test names the same constant symbolically; the deletions are real. | +| 7 | `SUCCEED_RETURN` deleted | The function falls off the end and returns whatever is in the return register, which is often NULL by luck. Needs an assertion on a side effect, not on the status. | +| 3 | `va_end` deleted | Undetectable on x86-64 SysV, where `va_end` is a no-op. Real UB, invisible here. | +| 3 | `FINISH(e, true)` → `FINISH(e, false)` | Real: an error swallowed instead of propagated. Worth a test. | +| 33 | the rest | Individually listed with `file:line` and the exact edit in the published report. | + +Two of them were real gaps and are now fixed: the depth-first walk's `depth + 1` +on the *right* child (nothing had ever recursed right more than three deep, so a +right-leaning tree would have blown the stack the depth cap exists to protect), +and `aksl_tree_remove` on an empty tree, which without its guard dereferences +NULL. Both are in the suite now — which is what the harness is for. ## The pre-push hook diff --git a/TODO.md b/TODO.md index f89b9db..ebb06e7 100644 --- a/TODO.md +++ b/TODO.md @@ -16,6 +16,7 @@ second, what is missing last. | Line coverage | 99.5% (1708/1716) | | Function coverage | 100% (154/154) | | Doxygen | 100% of 154, gated — `cmake --build build --target docs` fails on an undocumented function, parameter or return | +| Mutation score | 72.3% (188/260 sampled from 1701), gated at 65 | The six confirmed defects that used to head this file are fixed and `AKSL_KNOWN_FAILING_TESTS` is empty. What they were, and what changed as a @@ -139,7 +140,34 @@ message can name the caller's full version. to participate, that is the line to change, and the `#if` in `tests/test_version.c` is the test that encodes the rule. -### 2.4 Four uncovered `HANDLE(e, AKERR_ITERATOR_BREAK)` lines +### 2.4 Surviving mutants worth turning into assertions + +The mutation harness samples 260 of 1701 mutants and kills 72.3% of them. Most of +the 72 survivors are equivalent mutants rather than missing tests — `README.md` +has the full breakdown — but three clusters are real work: + +- **`FINISH(e, true)` → `FINISH(e, false)`, 3 survivors.** An error swallowed + instead of propagated out of an `ATTEMPT` block, and nothing notices. Each one + is a call whose failure path is exercised but whose *propagation* is not: the + test asserts the status the callee raised without checking it came from the + callee rather than being re-raised locally. `tests/test_pool.c`'s origin + assertions are the shape of the fix. +- **`SUCCEED_RETURN` deleted, 7 survivors.** The function falls off the end and + returns whatever is in the return register, which is NULL often enough to pass. + These need an assertion on the *side effect* — the buffer that was filled, the + node that was linked — rather than on the returned status. +- **`FAIL_*` guards deleted, ~6 of the 14 in that group.** Each is an argument + check nothing drives. The other 8 in the group are constant shifts that no test + can catch, because the test names the same constant symbolically and moves with + it. + +Two survivors in this class were real and are fixed: the right child's +`depth + 1` in the depth-first walk, and `aksl_tree_remove` on an empty tree. + +**What closing them would touch.** Only `tests/`. Raise the `--threshold` in +`.gitea/workflows/ci.yaml` and `.githooks/pre-push` in step, as a ratchet. + +### 2.5 Four uncovered `HANDLE(e, AKERR_ITERATOR_BREAK)` lines Macro artifacts rather than gaps. In libakerror that macro begins with the `break;` belonging to `PROCESS`'s `case 0:` arm, reachable only when a callback diff --git a/tests/test_collections.c b/tests/test_collections.c index 9677d2e..d7ac571 100644 --- a/tests/test_collections.c +++ b/tests/test_collections.c @@ -907,6 +907,25 @@ static int test_tree_remove_all_three_cases(void) return 0; } +/* + * Removing from an empty tree. Found by mutation testing: deleting the + * `FAIL_ZERO_RETURN(e, *root, ...)` guard survived the whole suite, because + * nothing had ever called remove on a tree with no root -- which without the + * guard walks straight into tree_replace and dereferences NULL. + */ +static int test_tree_remove_from_an_empty_tree(void) +{ + int value = 1; + aksl_TreeNode node; + aksl_TreeNode *root = NULL; + + AKSL_CHECK_OK(aksl_tree_node_init(&node, &value)); + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_tree_remove(&root, &node), + AKERR_VALUE, "tree is empty"); + AKSL_CHECK(root == NULL); + return 0; +} + /* * The mirror images of the cases above: a node that is its parent's *right* * child, and a node whose only child is on the left. Both take different @@ -1112,6 +1131,7 @@ int main(void) AKSL_RUN(failures, test_tree_remove_mirrored_shapes); AKSL_RUN(failures, test_tree_remove_with_a_distant_successor); AKSL_RUN(failures, test_tree_remove_the_only_node); + AKSL_RUN(failures, test_tree_remove_from_an_empty_tree); AKSL_RUN(failures, test_tree_height_and_count); AKSL_RUN(failures, test_tree_free_all);