andrew/libakgl
1
0
Fork 0
Code Issues Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

Add controller test suite and reach 72 percent line coverage
Some checks failed
libakgl CI Build / cmake_build (push) Failing after 20s
Details
libakgl CI Build / mutation_test (push) Failing after 17s
Details

Browse Source 
Cover control map capacity, the default binding set, keyboard and gamepad
dispatch including cross-device rejection, the dpad handlers, and device
add and remove against the dummy SDL drivers. Synthesize SDL events directly
rather than pumping the event queue, so the suite needs no real hardware.

Fix the three-way appstate check in the four gamepad handlers, which tested
appstate in place of both the event pointer and the looked-up player actor,
so a null event or a missing player was dereferenced instead of reported.

Record the new coverage status in TODO.md along with the seven defects the
suites uncovered and fixed, eleven that remain open, and the reason the
build tree has to precede LD_LIBRARY_PATH for the CTest run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Andrew Kesterson
2026-07-30 02:08:38 -04:00
parent 22162db2da
commit bc782fbffe
4 changed files with 750 additions and 520 deletions
Download Patch File Download Diff File Expand all files Collapse all files

698
TODO.md
View File

@@ -1,8 +1,8 @@
# TODO
## Coverage baseline
## Coverage status
Generated 2026-07-30 with:
Generated with:
```sh
cmake -S . -B build-coverage -DAKGL_COVERAGE=ON -DCMAKE_BUILD_TYPE=Debug
@@ -12,528 +12,199 @@ 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.
**Line coverage 72.2%, function coverage 78.6%** (1534/2125 lines), up from a
39.6% / 44.3% baseline. `character` is the one intentionally failing suite;
everything else passes.
Baseline with that fixed — `character` is the known intentionally-failing test,
everything else passes:
| File | Lines | Functions |
|---|---|---|
| `src/actor.c` | 205/258 (80%) | 16/18 |
| `src/assets.c` | 0/21 (0%) | 0/1 |
| `src/character.c` | 104/118 (88%) | 6/7 |
| `src/controller.c` | 220/243 (90%) | 9/9 |
| `src/draw.c` | 0/13 (0%) | 0/1 |
| `src/game.c` | 124/230 (54%) | 10/15 |
| `src/heap.c` | 116/116 (100%) | 12/12 |
| `src/json_helpers.c` | 111/111 (100%) | 11/11 |
| `src/physics.c` | 140/140 (100%) | 10/10 |
| `src/registry.c` | 76/102 (74%) | 11/12 |
| `src/renderer.c` | 7/70 (10%) | 1/7 |
| `src/sprite.c` | 93/101 (92%) | 5/5 |
| `src/staticstring.c` | 16/17 (94%) | 2/2 |
| `src/text.c` | 0/28 (0%) | 0/2 |
| `src/tilemap.c` | 201/426 (47%) | 10/20 |
| `src/util.c` | 121/131 (92%) | 7/8 |
| 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%** |
Branch coverage reads 18.6% 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 them unreachable in normal
operation — `src/game.c` reports over 1700 branches across 230 lines. Track line
and function coverage; treat branch coverage as a relative signal within a file.
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.
### Suites
Goal for the work below: **line coverage 39.6% → ~70%, function coverage 44.3% →
~80%**, without adding any test that needs a real display.
Every suite is registered through the `AKGL_TEST_SUITES` list in
`CMakeLists.txt`, which drives target creation, CTest registration, the
`WORKING_DIRECTORY`/`TIMEOUT` properties, the link line, and the
`FIXTURES_REQUIRED akgl_coverage` list together. Adding `tests/<name>.c` and the
name to that list is all a new suite needs; it can no longer be accidentally
left out of the coverage fixture.
---
Shared assertion helpers are in `tests/testutil.h`: `TEST_ASSERT`,
`TEST_ASSERT_FEQ`, `TEST_EXPECT_STATUS`, `TEST_EXPECT_OK`, `TEST_EXPECT_ANY_ERROR`,
and `TEST_ASSERT_FLAG`. All except the last expand to a `break` on failure, so
they belong directly inside an `ATTEMPT` block, not inside a loop nested in one.
## Suspected defects found while reading the uncovered code
Done:
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.
- `tests/physics.c` — both backends, the factory, and the full simulation loop
including thrust clamping, drag, layer masking, parent/child positioning, and
logic-interrupt handling. 100%.
- `tests/heap.c` — pool exhaustion for all five pools, refcount clamping,
recursive child release, registry cleanup on release. 100%.
- `tests/json_helpers.c` — every typed accessor, both string accessors'
allocate-vs-reuse paths, array bounds, and `akgl_get_json_with_default`. 100%.
- `tests/controller.c` — control map push and capacity, the default binding set,
keyboard and gamepad dispatch including cross-device rejection, the dpad
handlers, and device add/remove against the dummy drivers. 90%.
- `tests/game.c` — version gating, the save/load roundtrip, foreign-save
rejection, truncated-table detection, the state lock, and FPS accounting. 54%.
- `tests/actor.c` — extended with the eight control-map handlers, automatic
facing, movement logic, the animation frame state machine, `akgl_actor_update`,
and character/sprite binding lookups. 80%.
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.**
## Remaining work
### Needs the offscreen renderer harness
`src/renderer.c` (63 lines), `src/text.c` (28), `src/draw.c` (13),
`src/assets.c` (21), `akgl_actor_render`/`actor_visible` in `src/actor.c` (53),
and the drawing half of `src/tilemap.c` all need a live `renderer` global.
Build `tests/harness.c` / `tests/harness.h` with `akgl_test_init_headless()` and
`akgl_test_shutdown_headless()`: set the dummy video and audio drivers,
`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. The new suites set the driver hints inline and need
no window, so they are unaffected.
Then:
- **`tests/renderer.c`** — `akgl_render_init2d` populating all six function
pointers and the camera; frame start/end against a NULL `sdl_renderer`; the
rotated `draw_texture` path including `angle != 0` with a NULL center; the
`draw_mesh` "not implemented" stub; and `draw_world` layer ordering. Note
`defflags` at `src/renderer.c:113` is uninitialized until the `if` body runs.
- **`tests/text.c`**, **`tests/draw.c`**, **`tests/assets.c`** — font loading
into `AKGL_REGISTRY_FONT`, text rendering with wrap on and off, background
drawing at zero/negative/oversized dimensions, and BGM loading into
`AKGL_REGISTRY_MUSIC` under the dummy audio driver.
- **`tests/tilemap.c` extensions** — `akgl_tilemap_draw`, `_draw_tileset`, and
`akgl_tilemap_load_layer_image`.
### Does not need a renderer
- **`src/tilemap.c`** — `akgl_tilemap_scale_actor` is pure math over three
branches (`src/tilemap.c:824-830`); `akgl_get_json_properties_number`,
`_float`, and `_double` need only a JSON snippet; `akgl_tilemap_load_physics`
needs a fixture with the physics property block present, absent, and
malformed. Together roughly 100 of the 225 uncovered lines.
- **`src/game.c`** — `akgl_game_init`, `akgl_game_update`, `akgl_game_lowfps`,
and `akgl_game_updateFPS`'s frame loop need a window; revisit after the
harness lands.
- **`src/registry.c`** — `akgl_registry_load_properties` needs a fixture with a
`properties` object, plus the missing-file, missing-key, and wrong-value-type
cases. Assert the loop at `src/registry.c:148-158` does not leak the string
heap.
## Defects
### Fixed while building the suites
Each was found by a test written to assert correct behavior.
1. **`akgl_physics_simulate` dereferenced `self` before its NULL check.**
`src/physics.c:132` read `self->gravity_time` at declaration time, three
lines above `FAIL_ZERO_RETURN(e, self, ...)`. A NULL backend segfaulted.
2. **`akgl_game_save` never flushed or closed its stream.** `CLEANUP` and
`PROCESS` were transposed, which put the `fclose` inside the `PROCESS`
switch, where it only ran if an error context existed and reported success.
An ordinary save produced an empty file.
3. **`akgl_game_save_actors` wrote name-table terminators from a single char.**
`aksl_fwrite((void *)&nullval, 1, AKGL_ACTOR_MAX_NAME_LENGTH, fp)` emitted
127 bytes of adjacent stack memory into the save file and produced a sentinel
the loader could not recognize.
4. **`akgl_game_load_objectnamemap` swallowed read failures.** `CATCH` used
directly inside `while (1)` breaks the loop, not the function, so a truncated
or corrupt name table loaded as a successful game.
5. **`akgl_Actor_cmhf_up_on` and `_down_on` dereferenced `basechar` unguarded**,
unlike their left and right counterparts.
6. **`akgl_actor_logic_movement` checked `actor` twice** instead of checking
`actor->basechar` before dereferencing it.
7. **The gamepad handlers checked `appstate` three times each**, so a NULL event
or a missing player actor was never caught and `player->state` was
dereferenced regardless.
### Known and still open
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,
so the function always passes and the image assertions in `tests/sprite.c`
assert nothing.
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.**
loop; it should be `dest->layers[i].texture`. It also never NULLs the
pointers, so a second release is a use-after-free.
3. **`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/registry.c:27`), so `AKGL_REGISTRY_PROPERTIES` stays 0 unless the
caller initializes it separately. `akgl_set_property` is then a silent no-op
and `akgl_get_property` always returns the caller's default, which means
`akgl_physics_init_arcade` and `akgl_render_init2d` silently ignore
configuration. `akgl_game_init` does call it; a caller that does not use
`akgl_game_init` does not get it.
4. **`akgl_path_relative_from` is a stub that leaks.** `src/util.c:105` claims a
heap string, never writes `*dst`, and never releases it. 256 calls exhaust
the string pool.
5. **`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.
the surfaces share dimensions, pitch, or format.
6. **`akgl_string_initialize` overflows by four bytes when `init` is NULL.**
`src/staticstring.c:17` does `memset(&obj->data, 0x00, sizeof(akgl_String))`,
but `data` starts four bytes into the struct (after `refcount`), so the memset
runs four bytes past the end.
7. **Savegame name lengths disagree between writer and reader.**
`akgl_game_save_actors` writes spritesheet names at
`AKGL_SPRITE_SHEET_MAX_FILENAME_LENGTH` (512) and character names at
`AKGL_SPRITE_MAX_CHARACTER_NAME_LENGTH` (128), but `akgl_game_load` reads
every table at `AKGL_ACTOR_MAX_NAME_LENGTH` (128). A save with any registered
spritesheet cannot be read back. The current roundtrip test passes only
because empty registries write nothing but zeroed sentinels.
8. **Heap acquire functions are asymmetric.** `akgl_heap_next_string` increments
`refcount`; `next_actor`, `next_sprite`, `next_spritesheet`, and
`next_character` do not. `tests/heap.c` pins the current behavior and says so;
decide whether to make them symmetric or document the split.
9. **`tests/util.c` defines `test_akgl_collide_point_rectangle_logic` but
`main()` never calls it.**
10. **`controller.h` declares functions that do not exist.** It declares
`akgl_controller_handle_button_down`, `_button_up`, `_added`, and `_removed`,
but `src/controller.c` defines them as `gamepad_handle_*`. Anything compiled
against the header alone fails to link.
11. **`akgl_controller_pushmap` and `akgl_controller_default` accept negative map
ids.** Both check `controlmapid >= AKGL_MAX_CONTROL_MAPS` but not
`controlmapid < 0`, so a negative id indexes before `GAME_ControlMaps`.
---
## Build notes
## 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_<feature> tests/<feature>.c)
add_test(NAME <feature> COMMAND test_<feature>)
target_link_libraries(test_<feature> 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`.
---
The vendored SDL satellite libraries are built into per-project subdirectories
that are not on the loader's default search path, and `LD_LIBRARY_PATH` is
searched ahead of RPATH — so a developer who has previously run `rebuild.sh` had
their installed `libakgl.so` shadow the one under test, and a developer who had
not saw every test abort before `main()` with "cannot open shared object file".
`CMakeLists.txt` now sets `BUILD_RPATH` on the library, the utility, and every
test target, and prepends the build tree to `LD_LIBRARY_PATH` for the CTest run.
## Carried over
@@ -551,5 +222,6 @@ New suites in dependency order: `physics`, `json_helpers`, `heap`, `game`,
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.
`akgl_character_state_sprites_iterate` is still uncovered and is the natural
place to start; `tests/actor.c` now covers `akgl_character_sprite_get` and the
composite-state binding behavior it depends on.