Record the 39.6 percent line and 44.3 percent function coverage baseline measured with the AKGL_COVERAGE build, and plan twelve test suites to raise it to roughly 70 percent lines and 80 percent functions. Tier the work by what each suite needs: pure logic suites for physics, json_helpers, heap, and savegame handling that require no renderer; renderer suites gated behind a shared offscreen harness; and controller and frame-loop coverage deferred. Note that vendored dependency shared objects are absent from the loader path in a fresh out-of-tree build, which reports every test as zero coverage, and record ten suspected defects found while reading the uncovered code so the new tests assert correct behavior rather than pinning the current one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
556 lines
31 KiB
Markdown
556 lines
31 KiB
Markdown
# 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_<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`.
|
||
|
||
---
|
||
|
||
## 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
|
||
also leaves the previous sprite's refcount incremented. When
|
||
`akgl_heap_release_character()` drops the character refcount to zero, it clears
|
||
the character registry entry and zeroes the structure without enumerating
|
||
`state_sprites`, decrementing the bound sprites, or destroying the SDL property
|
||
map. Implement binding removal/replacement so each removed binding releases
|
||
exactly one sprite reference. On final character release, enumerate every
|
||
remaining binding (the existing `akgl_character_state_sprites_iterate()` release
|
||
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.
|