diff --git a/TODO.md b/TODO.md index 55d8866..691b070 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,542 @@ # TODO +## Coverage baseline + +Generated 2026-07-30 with: + +```sh +cmake -S . -B build-coverage -DAKGL_COVERAGE=ON -DCMAKE_BUILD_TYPE=Debug +cmake --build build-coverage --parallel +ctest --test-dir build-coverage --output-on-failure +``` + +Reports land in `build-coverage/coverage/` (`index.html`, `coverage.xml`). + +Note: the vendored SDL/akerror/akstdlib/jansson shared objects are not on the +default loader path in a fresh out-of-tree build, so every test that links them +dies with `error while loading shared libraries` before `main()` runs. Export +`LD_LIBRARY_PATH` over `build-coverage/deps/{SDL,SDL_image,SDL_ttf,SDL_mixer,libakerror,libakstdlib}` +first, or the report will read 0% for reasons that have nothing to do with the +tests. Fixing this properly (see "Test infrastructure" below) is a prerequisite +for trustworthy CI coverage numbers. + +Baseline with that fixed — `character` is the known intentionally-failing test, +everything else passes: + +| File | Lines | Branches | Functions | +|---|---|---|---| +| `src/assets.c` | 0/21 (0%) | 0/88 | 0/1 | +| `src/controller.c` | 0/243 (0%) | 0/660 | 0/9 | +| `src/draw.c` | 0/13 (0%) | 0/4 | 0/1 | +| `src/game.c` | 0/224 (0%) | 0/1702 | 0/15 | +| `src/physics.c` | 0/138 (0%) | 0/765 | 0/10 | +| `src/text.c` | 0/28 (0%) | 0/146 | 0/2 | +| `src/renderer.c` | 7/70 (10%) | 5/430 | 1/7 | +| `src/actor.c` | 61/256 (24%) | 76/1020 | 4/18 | +| `src/tilemap.c` | 201/426 (47%) | 209/3150 | 10/20 | +| `src/registry.c` | 54/102 (53%) | 63/582 | 8/12 | +| `src/json_helpers.c` | 86/111 (78%) | 73/710 | 9/11 | +| `src/character.c` | 93/118 (79%) | 74/806 | 5/7 | +| `src/sprite.c` | 88/101 (87%) | 112/810 | 4/5 | +| `src/util.c` | 121/131 (92%) | 185/1057 | 7/8 | +| `src/staticstring.c` | 16/17 (94%) | 9/64 | 2/2 | +| `src/heap.c` | 111/116 (96%) | 87/266 | 12/12 | +| **TOTAL** | **838/2115 (39.6%)** | **893/12260 (7.3%)** | **44.3%** | + +The 7.3% branch figure is misleading and should not be used as a target. The +`akerror` control-flow macros (`ATTEMPT`/`CATCH`/`PROCESS`/`FINISH`, +`FAIL_*_RETURN`) expand into large branch trees per call site, most of which are +unreachable in normal operation — `src/game.c` reports 1702 branches across only +224 lines. Track **line and function coverage**; treat branch coverage as a +relative signal within a single file, not an absolute goal. + +Goal for the work below: **line coverage 39.6% → ~70%, function coverage 44.3% → +~80%**, without adding any test that needs a real display. + +--- + +## Suspected defects found while reading the uncovered code + +Write the tests below to assert **correct** behavior, so these fail loudly rather +than getting baked in. Each is listed against the suite that will surface it. + +1. **`akgl_render_and_compare()` compares a texture against itself.** + `src/util.c:228` and `src/util.c:245` both draw `t1`; `t2` is never rendered. + The function therefore always passes, which silently neuters the image + assertions in `tests/sprite.c`. → Suite U2. +2. **`akgl_tilemap_release()` double-frees tileset textures.** + `src/tilemap.c:849` destroys `dest->tilesets[i].texture` inside the *layers* + loop; it should be `dest->layers[i].texture`. It also never NULLs the pointers + or memsets the map, so a second release is a use-after-free. → Suite T3. +3. **`akgl_physics_simulate()` dereferences `self` before the NULL check.** + `src/physics.c:132` reads `self->gravity_time` at declaration time; + `FAIL_ZERO_RETURN(e, self, ...)` is not until line 135. Passing `NULL` + segfaults instead of returning `AKERR_NULLPOINTER`. → Suite P4. +4. **`akgl_path_relative_from()` is a stub that leaks.** + `src/util.c:105` acquires `dirnamestr` from the string heap, never writes + `*dst`, and never releases the string. Calling it `AKGL_MAX_HEAP_STRING` (256) + times exhausts the heap. → Suite U1. +5. **`akgl_registry_init()` never initializes the properties registry.** + `akgl_registry_init_properties()` is not called from `akgl_registry_init()` + (`src/registry.c:27`), so `AKGL_REGISTRY_PROPERTIES` stays 0. Every + `akgl_set_property()` is then a silent no-op against property set 0 and + `akgl_get_property()` always returns the caller's default — which means + `akgl_physics_init_arcade()` and `akgl_render_init2d()` silently ignore + configuration. → Suite R1. +6. **`akgl_compare_sdl_surfaces()` memcmps without checking geometry.** + `src/util.c:208` compares `s1->pitch * s1->h` bytes of `s2` without verifying + the two surfaces share dimensions, pitch, or format. → Suite U2. +7. **Heap acquire functions are asymmetric.** + `akgl_heap_next_string()` increments `refcount`; `next_actor`, `next_sprite`, + `next_spritesheet`, and `next_character` do not (`src/heap.c:57-121`). Callers + that acquire-then-release a non-string object drive the refcount negative-ish + or free a live object. → Suite H1; decide whether to fix or document. +8. **`tests/util.c` defines `test_akgl_collide_point_rectangle_logic()` but + `main()` never calls it** (`tests/util.c:134` vs. `tests/util.c:312-317`). + One-line fix; do it as part of Suite U3. +9. **`akgl_actor_logic_movement()` null-checks the wrong pointer.** + `src/actor.c:130` checks `actor` twice; the message says `actor->basechar` + but `basechar` is never validated before it is dereferenced on the next line. + → Suite A2. +10. **`akgl_Actor_cmhf_up_on()` / `_down_on()` dereference `basechar` unguarded.** + `src/actor.c:385` and `:413` read `obj->basechar->ay` with no + `FAIL_ZERO_RETURN` on `basechar`, unlike their left/right counterparts + (`:326`, `:355`). → Suite A1. + +--- + +## Tier 1 — new suites, no renderer required + +These are pure logic or property-map code. They need `akgl_heap_init()` and the +relevant `akgl_registry_init_*()` calls, nothing else. Highest coverage per unit +of effort. + +### Suite P — `tests/physics.c` → target `test_physics`, CTest name `physics` + +Covers `src/physics.c` (0% → target ~85%). 10 uncovered functions. + +- **P1 `test_physics_null_backend`** — `akgl_physics_init_null()` installs + `akgl_physics_null_{gravity,collide,move}` and `akgl_physics_simulate`; assert + each field is the expected function pointer. Assert `AKERR_NULLPOINTER` for + `init_null(NULL)` and for each null-backend function called with `self == NULL`. + Assert the null gravity/move functions leave a populated actor's `x/y/z`, + `ex/ey/ez`, and `vx/vy/vz` bit-identical. +- **P2 `test_physics_arcade_gravity`** — table-driven over + `gravity_{x,y,z}` ∈ {0, positive, negative} × `dt` ∈ {0, 0.5, 1.0}. Assert the + sign conventions in `src/physics.c:56-67`: `ex -= gx*dt`, `ey += gy*dt`, + `ez -= gz*dt`, and that a zero component leaves its axis untouched. Assert + `AKERR_NULLPOINTER` for null `self` and null `actor`. +- **P3 `test_physics_arcade_move`** — assert `x += vx*dt` on all three axes for + positive, negative, and zero `dt`; assert `dt == 0` is a no-op. Assert + `akgl_physics_arcade_collide()` returns `AKERR_API` ("Not implemented") so the + stub is pinned until it is written. +- **P4 `test_physics_simulate_iteration`** — the big one. Build a small heap by + hand (`akgl_heap_init()`, then `akgl_heap_next_actor()` + `akgl_actor_initialize()`), + attach a stub `movementlogicfunc`, and drive `akgl_physics_simulate()` with a + null backend to isolate the loop. Assert: + - actors with `refcount == 0` are skipped; + - actors with `basechar == NULL` are skipped; + - a child actor (`parent != NULL`) gets `x = parent->x + vx` and does **not** + go through gravity/move; + - `AKGL_ITERATOR_OP_LAYERMASK` skips actors whose `layer != opflags->layerid`, + and `opflags == NULL` selects the documented defaults; + - thrust clamps: set `tx` beyond `sx` in both directions and assert + `tx == ±sx` exactly (`src/physics.c:174-194`), including the `sz` axis which + has a clamp but no accumulate; + - `MOVING_LEFT`/`MOVING_RIGHT` accumulate `tx += ax*dt`, `MOVING_UP`/`_DOWN` + accumulate `ty += ay*dt`, and neither fires for an idle actor; + - drag: `ex -= ex*drag_x*dt` for nonzero drag, untouched for zero drag; + - `vx == ex + tx` after the pass; + - a `movementlogicfunc` that raises `AKGL_ERR_LOGICINTERRUPT` is swallowed and + the loop continues to the next actor. + - **Defect 3**: `akgl_physics_simulate(NULL, NULL)` must return + `AKERR_NULLPOINTER`, not crash. +- **P5 `test_physics_factory`** — `"null"` and `"arcade"` dispatch to the right + initializer; an unknown name returns `AKERR_KEY`; null `self` and null `type` + return `AKERR_NULLPOINTER`. Add cases for the `strncmp` prefix matching in + `src/physics.c:232-239`: `"nullx"` and `"arcade_extra"` currently match — pin + whichever behavior is intended. +- **P6 `test_physics_arcade_properties`** — with `AKGL_REGISTRY_PROPERTIES` + initialized and `physics.gravity.y` / `physics.drag.x` set via + `akgl_set_property()`, assert `akgl_physics_init_arcade()` reads them into the + backend. Assert the documented defaults (`0.0`) when unset. Depends on Suite R1 + resolving defect 5. + +### Suite R — extend `tests/registry.c` + +Covers `akgl_registry_init_properties`, `akgl_set_property`, `akgl_get_property`, +`akgl_registry_load_properties` (`src/registry.c` 53% → target ~90%). + +- **R1 `test_registry_properties_roundtrip`** — `akgl_registry_init_properties()` + then set/get a value; assert the default is returned for a missing key and the + stored value wins for a present key. **Defect 5**: assert that a plain + `akgl_registry_init()` leaves properties usable — this is expected to fail + until `akgl_registry_init_properties()` is added to `akgl_registry_init()`. +- **R2 `test_registry_property_nullpointers`** — `akgl_set_property(NULL, x)`, + `(x, NULL)`, `akgl_get_property(NULL, ...)`, `(x, NULL, ...)` all + `AKERR_NULLPOINTER`. Assert `akgl_get_property` with `*dest == NULL` allocates + from the string heap (`src/registry.c:178`) and with a preallocated `*dest` + reuses it. +- **R3 `test_registry_load_properties`** — new fixture + `tests/assets/snippets/test_properties.json` with a `properties` object of + several string values. Assert every key lands in the registry. Error cases: + `NULL` filename → `AKERR_NULLPOINTER`; nonexistent file → error not crash; + a file with no `properties` key → `AKERR_KEY`; a `properties` value that is a + number rather than a string → `AKERR_TYPE`. Also assert the loop at + `src/registry.c:148-158` does not leak the string heap: record free slots + before and after and require them equal. +- **R4 `test_registry_init_actor_state_strings`** — assert all + `AKGL_ACTOR_MAX_STATES` (32) names from `AKGL_ACTOR_STATE_STRING_NAMES` map to + `1 << i`, and that a name not in the table reads back as 0. +- **R5 `test_registry_init_idempotent`** — `akgl_registry_init_actor()` destroys + and recreates on re-init (`src/registry.c:47`) while the other seven + initializers leak their previous `SDL_PropertiesID`. Pin the intended + behavior; if the leak is unintended, this is the test that catches the fix. + +### Suite J — extend `tests/` for `src/json_helpers.c` + +New file `tests/json_helpers.c` → target `test_json_helpers`, CTest name +`json_helpers` (78% → target ~95%). Currently `json_helpers` has no dedicated +suite; it is covered only incidentally through sprite/character/tilemap loading. + +- **J1 `test_json_double_value`** — `akgl_get_json_double_value()` is entirely + uncovered. Assert value retrieval, `AKERR_KEY` for a missing key, `AKERR_TYPE` + for a string value, `AKERR_NULLPOINTER` for a null object. Assert it accepts + both a JSON integer and a JSON real (it uses `json_is_number`). +- **J2 `test_json_with_default`** — `akgl_get_json_with_default()` is entirely + uncovered and is the odd one out: it takes a *prior* error context. Assert + that a `NULL` error is a no-op success; that an `AKERR_KEY` error copies + `defsize` bytes from `defval` into `dest`; that an `AKERR_INDEX`-group error + does the same; and that an unrelated error (e.g. `AKERR_TYPE`) is *not* + swallowed. Assert null `defval`/`dest` with a non-null error → + `AKERR_NULLPOINTER`. +- **J3 `test_json_string_heap_allocation`** — for both + `akgl_get_json_string_value()` and `akgl_get_json_array_index_string()`, + assert the `*dest == NULL` path acquires and initializes a heap string + (`src/json_helpers.c:83`, `:137`) and the non-null path overwrites in place. + Assert a JSON string longer than `AKGL_MAX_STRING_LENGTH` is truncated rather + than overflowing (`strncpy` at `:91` does not NUL-terminate on truncation — + assert the intended behavior explicitly). +- **J4 `test_json_array_index_bounds`** — negative index, index == length, and + index > length all return `AKERR_OUTOFBOUNDS` for the object/integer/string + variants. Wrong element type returns `AKERR_TYPE`. +- **J5 `test_json_helpers_nullpointers`** — sweep all 11 entry points with null + `obj`, null `key`, and null `dest`. Several currently omit the `key`/`dest` + checks that `akgl_get_json_string_value()` has; pin what each should do. + +Fixture: one `tests/assets/snippets/test_json_helpers.json` holding an object +with a string, integer, real, boolean, nested object, and a mixed-type array. + +### Suite H — extend `tests/` for `src/heap.c` + +New file `tests/heap.c` → target `test_heap`, CTest name `heap` (96% lines but +the five uncovered lines are every exhaustion path). + +- **H1 `test_heap_exhaustion`** — for each pool, claim every slot and assert the + next acquire returns `AKGL_ERR_HEAP`: actors (`AKGL_MAX_HEAP_ACTOR` = 64), + sprites (1024), spritesheets (1024), characters (256), strings (256). Because + only `akgl_heap_next_string()` increments the refcount (**defect 7**), the + non-string loops must bump `refcount` by hand — write the test to make that + asymmetry explicit and add an assertion recording which behavior is intended. +- **H2 `test_heap_release_refcounting`** — release below zero is clamped + (`refcount > 0` guard); release at 1 zeroes the object and clears its registry + entry; release of a still-referenced object leaves data intact. +- **H3 `test_heap_release_actor_children`** — an actor with children releases + them recursively (`src/heap.c:132-136`); assert a child shared by two parents + is not double-released, and that a full `AKGL_ACTOR_MAX_CHILDREN` (8) set is + walked. +- **H4 `test_heap_release_nullpointers`** — all five release functions return + `AKERR_NULLPOINTER` for `NULL`. +- **H5 `test_heap_init_clears`** — dirty every pool, call `akgl_heap_init()`, + assert all slots are zeroed and `akgl_heap_init_actor()` alone clears only + actors. + +### Suite S — extend `tests/staticstring.c` + +- **S1 `test_string_copy_count`** — the only uncovered line in the file is the + `strncpy` failure branch (`src/staticstring.c:32`), which is unreachable; + instead pin the `count == 0` → `AKGL_MAX_STRING_LENGTH` default + (`:28`) versus an explicit short `count`, and assert a short count does not + NUL-terminate (current `strncpy` semantics) so callers know. +- **S2 `test_string_initialize_truncation`** — initialize with a string longer + than `AKGL_MAX_STRING_LENGTH` (`PATH_MAX`) and assert the documented + truncation. Assert the `init == NULL` branch zeroes `data` and still sets + `refcount = 1` — note `src/staticstring.c:17` memsets `sizeof(akgl_String)` + starting at `&obj->data`, which writes past `data` if `data` is not the last + member; assert the struct's trailing fields survive. + +### Suite A — extend `tests/actor.c` + +Covers the 14 uncovered actor functions (24% → target ~75%). Everything except +`akgl_actor_render` and `actor_visible` is renderer-free. + +- **A1 `test_actor_control_map_handlers`** — the eight `akgl_Actor_cmhf_*` + functions. For each `_on`: assert `FACE_ALL | MOVING_ALL` is cleared, the + correct `MOVING_*` and `FACE_*` bits are set, and `ax`/`ay` picks up + `±basechar->ax/ay`. For each `_off`: assert `ax/ex/tx/vx` (or the `y` set) are + zeroed and only the matching `MOVING_*` bit is cleared — note `_off` does not + clear the `FACE_*` bit, which is deliberate; pin it. Assert + `AKERR_NULLPOINTER` for null actor and null event on all eight. + **Defect 10**: `up_on`/`down_on` with `basechar == NULL` must return + `AKERR_NULLPOINTER` rather than segfault. +- **A2 `test_actor_logic_movement`** — assert `sx/sy/sz` are copied from + `basechar`, and the four `MOVING_*` states select the right `ax`/`ay` sign. + Assert the "neither left nor right" case leaves `ax` at its prior value + (current behavior — there is no else branch). **Defect 9**: null `basechar` + must return `AKERR_NULLPOINTER`. +- **A3 `test_actor_logic_changeframe`** — pure state machine, table-driven over + (`loop`, `loopReverse`, `curSpriteReversing`, `curSpriteFrameId`, `frames`): + forward advance mid-animation; wrap to 0 at the end when not reversing; + enter reverse at the end when `loop && loopReverse`; leave reverse at frame 0; + single-frame sprite (`frames == 1`); `frames == 0` (currently underflows + `curSpriteFrameId` — assert the intended guard). Assert null actor → + `AKERR_NULLPOINTER`; note `curSprite` is dereferenced unchecked at + `src/actor.c:96`, so add a null-`curSprite` case too. +- **A4 `test_actor_automatic_face`** — `movement_controls_face == false` leaves + state untouched; `true` clears `FACE_ALL` and applies the documented + left > right > up > down precedence; simultaneous left+up resolves to left; + no movement bits leaves no face bits. +- **A5 `test_actor_update`** — with a character/sprite pair wired up, assert + `changeframefunc` fires only when `curtime - curSpriteFrameTimer >= speed`, + that `curSpriteFrameTimer` is then updated, and that a state with no bound + sprite (`AKERR_KEY` from `akgl_character_sprite_get`) is swallowed as success + (`src/actor.c:167`). Null actor and null `basechar` → `AKERR_NULLPOINTER`. +- **A6 `test_actor_add_child_limits`** — extend the existing coverage: filling + all 8 slots then adding a 9th returns `AKERR_OUTOFBOUNDS`; adding a child that + already has a parent returns `AKERR_RELATIONSHIP`; the child's `refcount` is + incremented exactly once. +- **A7 `test_registry_iterate_actor_flags`** — drive + `akgl_registry_iterate_actor()` with each `AKGL_ITERATOR_OP_*` combination via + stub `updatefunc`/`renderfunc` that record calls. Assert `LAYERMASK` filters, + `UPDATE` and `RENDER` fire independently, and the `else` branch resets + `scale` to 1.0 when `TILEMAPSCALE` is absent. Note the `break` at + `src/actor.c:303` exits the `ATTEMPT` block — assert it skips the actor rather + than aborting the enumeration. + +### Suite C — extend `tests/character.c` + +Covers `akgl_character_sprite_get` and `akgl_character_state_sprites_iterate` +(79% → target ~95%). The existing `character` test is the intentionally-failing +one; add these as separate functions so they can be enabled independently. + +- **C1 `test_character_sprite_get`** — add a sprite for a composite state and + read it back; assert an unbound state returns `AKERR_KEY`; assert null + `basechar` and null `dest` return `AKERR_NULLPOINTER`; assert state 0 and a + negative state are handled (`SDL_itoa` renders both). +- **C2 `test_character_state_sprites_iterate`** — with `AKGL_ITERATOR_OP_RELEASE` + set, assert each bound sprite's refcount drops by one; without the flag, + assert nothing changes; assert a null `userdata` and a null `name` are logged + and skipped rather than crashing. +- **C3 `test_character_load_json_errors`** — new snippet fixtures for: a + `sprite_mappings` entry naming a sprite absent from the registry + (`AKERR_NULLPOINTER`, `src/character.c:163`); a `state` array containing an + unknown state name (`AKERR_KEY`, `:115`); a missing `speedtime` key. Assert + that on failure the character heap slot is released and the registry entry is + not left dangling. Note `src/character.c:223` logs `obj->name` after the + cleanup block has potentially zeroed it — assert the failure path does not + read freed data. + +### Suite G — `tests/game.c` → target `test_game`, CTest name `game` + +Covers the renderer-free half of `src/game.c` (0% → target ~45%). Skip +`akgl_game_init`, `akgl_game_update`, `akgl_game_lowfps`, and `akgl_game_updateFPS` +for now; they need a window and a running loop. + +- **G1 `test_game_load_versioncmp`** — pure function, entirely testable. + Equal versions succeed; differing major/minor/patch return `AKERR_API`; a + malformed current version and a malformed save version each return + `AKERR_VALUE` with the right message; all three null arguments return + `AKERR_NULLPOINTER`. Assert `semver_free()` runs on both paths (no leak across + 1000 iterations). +- **G2 `test_game_save_load_roundtrip`** — write to a temp path with + `akgl_game_save()`, read back with `akgl_game_load()`, assert `game` is + restored bit-identical. Then assert the rejection paths: a save whose `name` + differs, whose `uri` differs, and whose `libversion` differs each return + `AKERR_API`. Null path → `AKERR_NULLPOINTER`; unwritable path → IO error. +- **G3 `test_game_save_actors_tables`** — with a couple of actors, sprites, + spritesheets, and characters registered, save and then parse the resulting + file by hand: assert each of the four name→pointer tables is present, in + order, and terminated by the NUL name + NUL pointer sentinel that + `akgl_game_load_objectnamemap()` looks for. +- **G4 `test_game_load_objectnamemap_truncation`** — feed a deliberately + truncated table (EOF before the sentinel). The `while (1)` loop at + `src/game.c:340` relies on `aksl_fread` raising; assert it returns an error + rather than looping forever. Give this test an aggressive CTest `TIMEOUT`. +- **G5 `test_game_state_lock`** — assert `akgl_game_state_lock()` / + `_unlock()` pair correctly and that a double-lock and an unlock-without-lock + behave as documented. + +### Suite U — extend `tests/util.c` + +- **U1 `test_path_relative_from`** — `akgl_path_relative_from()` is entirely + uncovered. Assert it writes `*dst`, that the result is the dirname of the + resolved `from`, and that null `path`/`from` return `AKERR_NULLPOINTER`. + **Defect 4**: this is expected to fail — the function never assigns `*dst` and + leaks its heap string. Add a loop of 300 calls asserting the string heap does + not run dry. +- **U2 `test_render_and_compare_detects_difference`** — build two textures with + deliberately different pixels and assert `akgl_render_and_compare()` reports + `AKERR_VALUE`. **Defect 1**: expected to fail today. Add + `test_compare_sdl_surfaces_geometry`: surfaces with mismatched `w`/`h`/`pitch` + must be rejected before the memcmp (**defect 6**). This suite needs a + renderer — see Tier 2 harness. +- **U3 `test_path_relative_errors`** — `akgl_path_relative()` with a path that + resolves in the CWD, a path that only resolves under `root`, and a path that + resolves under neither (assert `ENOENT` propagates). Assert + `akgl_path_relative_root()` returns `AKERR_OUTOFBOUNDS` when + `strlen(root) + strlen(path) >= AKGL_MAX_STRING_LENGTH`, and that both + functions release their heap strings on the failure path. **Defect 8**: wire + the existing `test_akgl_collide_point_rectangle_logic()` into `main()`. + +--- + +## Tier 2 — suites that need the offscreen renderer harness + +### Test infrastructure (do this first) + +1. **Fix the loader path.** Set `BUILD_RPATH`/`INSTALL_RPATH` on the test targets + to the vendored dependency directories, or add + `ENVIRONMENT_MODIFICATION "LD_LIBRARY_PATH=path_list_prepend:..."` to the + `set_tests_properties()` block in `CMakeLists.txt:141`. Without this, CI + coverage numbers are meaningless (see the note at the top). +2. **Add `tests/harness.c` / `tests/harness.h`** with + `akgl_test_init_headless()` and `akgl_test_shutdown_headless()`: set + `SDL_VIDEODRIVER=dummy` and `SDL_AUDIODRIVER=dummy`, `SDL_Init()`, + `akgl_heap_init()`, `akgl_registry_init()`, create a software + `SDL_CreateWindowAndRenderer` and point the global `renderer` at it. Four + existing tests hand-roll this today (`tests/sprite.c:194`, + `tests/character.c:200`, `tests/tilemap.c:421`, `tests/charviewer.c:42`) — + collapse them onto the shared harness in the same change. +3. **Add a `tests/assets/snippets/` fixture per new JSON case** rather than + embedding JSON in C string literals, matching the existing + `test_tilemap_get_json_tilemap_property.json` convention. + +### Suite T — extend `tests/tilemap.c` + +Covers the 10 uncovered tilemap functions (47% → target ~75%). + +- **T1 `test_tilemap_properties_numeric`** — `akgl_get_json_properties_number`, + `_float`, and `_double` are all uncovered. Assert each reads the right + `type` discriminator; note `akgl_get_json_properties_double()` asks for type + `"float"` (`src/tilemap.c:138`) — pin whether that is intended. Assert a + type mismatch returns `AKERR_TYPE` and a missing property returns `AKERR_KEY`. +- **T2 `test_tilemap_scale_actor`** — pure math, no renderer. Three branches at + `src/tilemap.c:824-830`: `y <= p_vanishing_y` → `p_vanishing_scale`; + `y >= p_foreground_y` → `p_foreground_scale`; between → the linear + interpolation. Assert both exact boundaries and a midpoint. Null map and null + actor → `AKERR_NULLPOINTER`. +- **T3 `test_tilemap_release`** — **defect 2**. Load `tests/assets/testmap.tmj`, + release, and assert every tileset and layer texture pointer is NULLed and the + map is safe to release a second time. Under the current implementation the + layers loop frees tileset textures again; this test should fail until fixed. + Run it under ASan if available. +- **T4 `test_tilemap_load_physics`** — `akgl_tilemap_load_physics()` is + uncovered. Fixture snippets for a map with a full physics property block, one + with the block absent (assert documented defaults), and one with a + malformed value. +- **T5 `test_tilemap_load_layer_image`** — uncovered. Fixture with an image + layer; assert the texture loads, the layer is recorded, and a missing image + file yields `AKGL_ERR_SDL` rather than a null-texture crash later. +- **T6 `test_tilemap_load_bounds`** — assert a map declaring more than + `AKGL_TILEMAP_MAX_TILESETS` (16) tilesets or `AKGL_TILEMAP_MAX_LAYERS` (16) + layers returns `AKERR_OUTOFBOUNDS` rather than writing past the arrays. +- **T7 `test_tilemap_draw`** — with the dummy renderer, assert + `akgl_tilemap_draw()` and `akgl_tilemap_draw_tileset()` complete for each + layer index, that an out-of-range `layeridx` is rejected, and that a viewport + entirely outside the map draws nothing without erroring. + +### Suite N — `tests/renderer.c` → target `test_renderer`, CTest name `renderer` + +Covers `src/renderer.c` (10% → target ~70%). + +- **N1 `test_render_init2d`** — with `game.screenwidth`/`screenheight` + properties set, assert `akgl_render_init2d()` populates all six function + pointers and sets `camera` to `{0, 0, w, h}`. Assert null `self` → + `AKERR_NULLPOINTER` and that the two heap strings it acquires are released + (`src/renderer.c:31-32`) even on the failure path — they currently are not, + because `PASS` returns early on `SDL_CreateWindowAndRenderer` failure. +- **N2 `test_render_frame_start_end`** — assert `AKERR_NULLPOINTER` when + `self->sdl_renderer` is NULL; assert success against the dummy renderer. +- **N3 `test_render_2d_draw_texture`** — the rotated path + (`angle != 0`) is uncovered: assert `angle != 0` with `center == NULL` returns + `AKERR_NULLPOINTER`, and that a valid rotation succeeds. Assert null `self` + and null `texture` are rejected. +- **N4 `test_render_2d_draw_mesh`** — pin the `AKERR_API` "Not implemented" stub. +- **N5 `test_render_2d_draw_world`** — assert layer ordering (tilemap layer *i* + drawn before the actors on layer *i*), that `refcount == 0` actors are + skipped, and that `opflags == NULL` zero-initializes the default iterator. + Note `defflags` at `src/renderer.c:113` is uninitialized until the `if` body + runs — assert the non-null-`opflags` path does not read it. + +### Suite X — `tests/text.c`, `tests/draw.c`, `tests/assets.c` + +Small files, 0% each, cheap wins once the harness exists. + +- **X1 `test_text_loadfont`** — load a TTF from a new `tests/assets/` fixture; + assert it lands in `AKGL_REGISTRY_FONT`, that a missing file errors, and that + a null name/path is rejected. +- **X2 `test_text_rendertextat`** — render into the dummy renderer; assert a + null font and null text are rejected and that `wraplength` 0 vs. positive both + work. +- **X3 `test_draw_background`** — `akgl_draw_background()` returns `void` and + cannot report failure; assert it does not crash for zero, negative, and + oversized dimensions. +- **X4 `test_load_start_bgm`** — with the dummy audio driver, assert a valid + file loads into `AKGL_REGISTRY_MUSIC`, a missing file errors, and a null + filename is rejected. + +--- + +## Tier 3 — deferred + +### Suite K — `tests/controller.c` → target `test_controller` + +`src/controller.c` is 0% across 9 functions and 243 lines, but every entry point +is driven by real SDL gamepad/keyboard events. Worth doing, but only after +Tier 1 and 2 land. + +- Synthesize `SDL_Event` structs directly and call + `akgl_controller_handle_event()` — no physical device needed for + `SDL_EVENT_KEY_DOWN`/`_UP`, and `SDL_EVENT_GAMEPAD_ADDED`/`_REMOVED` can be + pushed onto the SDL event queue. +- `akgl_controller_pushmap()` and `akgl_controller_default()` are the most + testable: assert a control map is registered and retrievable, that an + out-of-range `controlmapid` is rejected, and that `akgl_controller_default()` + binds the eight `akgl_Actor_cmhf_*` handlers to the expected keys. +- `akgl_controller_list_keyboards()` and `_open_gamepads()` should at minimum be + asserted not to crash with zero devices attached. + +### Deferred `src/game.c` entry points + +`akgl_game_init`, `akgl_game_update`, `akgl_game_updateFPS`, and +`akgl_game_lowfps` need a window and a live frame loop. Revisit once Suite N +proves the dummy renderer is stable enough to run a bounded number of frames. + +--- + +## Registration checklist + +Each new suite needs, in `CMakeLists.txt`: + +```cmake +add_executable(test_ tests/.c) +add_test(NAME COMMAND test_) +target_link_libraries(test_ PRIVATE akstdlib::akstdlib akerror::akerror akgl + SDL3::SDL3 SDL3_ttf::SDL3_ttf SDL3_image::SDL3_image SDL3_mixer::SDL3_mixer jansson::jansson -lm) +``` + +and its name added to the `set_tests_properties(...)` list at +`CMakeLists.txt:141` **and** to the `FIXTURES_REQUIRED akgl_coverage` list at +`CMakeLists.txt:167` — a suite missing from the second list runs outside the +coverage fixture and its counters are discarded by `coverage_reset`. + +New suites in dependency order: `physics`, `json_helpers`, `heap`, `game`, +`renderer`, `text`, `draw`, `assets`, `controller`. + +--- + +## Carried over + 1. **Make character-to-sprite state bindings release their references symmetrically.** `akgl_character_sprite_add()` increments the sprite refcount for every state-map binding, but no corresponding removal API exists. Replacing a state binding @@ -13,3 +550,6 @@ path may be reusable), release each reference, destroy `state_sprites`, and then clear the character. Add tests for removal, replacement, duplicate sprite bindings across multiple states, and final character release. + + Suites C2 and H2 above are the coverage side of this item; the API work is + still outstanding.