Unit test coverage: rings (game/rings.go has zero tests) #5

Closed
opened 2026-08-09 03:40:50 +02:00 by clawbot · 2 comments
Collaborator

Problem

game/rings.go (182 lines, 6 functions) has no test coverage at all. Of
the 32 tests in the suite, not one exercises ring wear/removal, hand
selection, or the ring food-drain contribution.

This is the first of three units splitting the TODO.md Next Step
("broaden unit test coverage where playtesting finds thin spots — rings,
sticks, wizard commands") into commit-sized pieces.

Untested surface

Function What it does
ringOn() put on a ring; rejects non-rings, full hands
pickRingHand() choose which hand when both are free
ringOff() take off a ring
gethand() prompt for a hand, validate the answer
ringEat(hand) per-ring food-consumption contribution
ringNum(obj) display suffix for a ring's bonus

ringEat is the highest-value target: it feeds the hunger clock, so a wrong
value is a slow, silent gameplay divergence from C that no current test would
catch.

Definition of done

  1. A new game/rings_test.go covering, at minimum:
    • Putting on a ring populates the correct hand; the ring's effect is
      applied (e.g. AddStrength changes strength, Dexterity changes the
      relevant stat).
    • Taking off a ring reverses the effect and clears the hand.
    • Attempting to wear a ring with both hands occupied is rejected, with no
      state change.
    • Attempting to put on a non-ring object is rejected.
    • ringEat returns the correct per-kind value for every ring kind that C
      assigns a nonzero food cost, cross-checked against the C source on the
      origin/c-master branch (rings.c), with the C values quoted in the
      test or its comments.
    • ringNum output for a bonus and a non-bonus ring.
  2. Every ring kind in the table is either exercised or explicitly noted in a
    comment as intentionally not exercised, with the reason.
  3. make check fully green.
  4. TODO.md updated in the same commit (this is one third of the current
    Next Step — narrow the step rather than marking it complete).
  5. Commit title ends with (closes #N).

Implementation requirements

  • Verify expected values against the C reference, do not assert whatever
    the Go code currently returns. The C sources are on the origin/c-master
    branch — read them with git show origin/c-master:rings.c. Do NOT check out
    or modify that branch. A test that merely locks in current behavior is
    worthless for a port; the whole point is catching divergence from C.
  • If you find a genuine divergence from C, stop and report it — do not
    "fix" gameplay in a test-coverage commit, and do not write the test to
    match the Go bug.
  • Tests must call t.Parallel() (the paralleltest linter is enforced and
    the suite is at 0 issues — keep it there).
  • White-box tests in package game need the
    //nolint:testpackage // white-box tests reach unexported state (approved 2026-07-07)
    file header, matching the existing test files.
  • make targets only — never run go test directly.
  • Do NOT modify .golangci.yml.
  • No behavior changes to game code in this commit.
  • Never mention Claude or Anthropic anywhere.
## Problem `game/rings.go` (182 lines, 6 functions) has **no test coverage at all**. Of the 32 tests in the suite, not one exercises ring wear/removal, hand selection, or the ring food-drain contribution. This is the first of three units splitting the `TODO.md` Next Step ("broaden unit test coverage where playtesting finds thin spots — rings, sticks, wizard commands") into commit-sized pieces. ## Untested surface | Function | What it does | | --- | --- | | `ringOn()` | put on a ring; rejects non-rings, full hands | | `pickRingHand()` | choose which hand when both are free | | `ringOff()` | take off a ring | | `gethand()` | prompt for a hand, validate the answer | | `ringEat(hand)` | per-ring food-consumption contribution | | `ringNum(obj)` | display suffix for a ring's bonus | `ringEat` is the highest-value target: it feeds the hunger clock, so a wrong value is a slow, silent gameplay divergence from C that no current test would catch. ## Definition of done 1. A new `game/rings_test.go` covering, at minimum: - Putting on a ring populates the correct hand; the ring's effect is applied (e.g. `AddStrength` changes strength, `Dexterity` changes the relevant stat). - Taking off a ring reverses the effect and clears the hand. - Attempting to wear a ring with both hands occupied is rejected, with no state change. - Attempting to put on a non-ring object is rejected. - `ringEat` returns the correct per-kind value for every ring kind that C assigns a nonzero food cost, cross-checked against the C source on the `origin/c-master` branch (`rings.c`), with the C values quoted in the test or its comments. - `ringNum` output for a bonus and a non-bonus ring. 2. Every ring kind in the table is either exercised or explicitly noted in a comment as intentionally not exercised, with the reason. 3. `make check` fully green. 4. `TODO.md` updated in the same commit (this is one third of the current Next Step — narrow the step rather than marking it complete). 5. Commit title ends with ` (closes #N)`. ## Implementation requirements - **Verify expected values against the C reference**, do not assert whatever the Go code currently returns. The C sources are on the `origin/c-master` branch — read them with `git show origin/c-master:rings.c`. Do NOT check out or modify that branch. A test that merely locks in current behavior is worthless for a port; the whole point is catching divergence from C. - If you find a genuine divergence from C, **stop and report it** — do not "fix" gameplay in a test-coverage commit, and do not write the test to match the Go bug. - Tests must call `t.Parallel()` (the `paralleltest` linter is enforced and the suite is at 0 issues — keep it there). - White-box tests in `package game` need the `//nolint:testpackage // white-box tests reach unexported state (approved 2026-07-07)` file header, matching the existing test files. - `make` targets only — never run `go test` directly. - Do NOT modify `.golangci.yml`. - No behavior changes to game code in this commit. - Never mention Claude or Anthropic anywhere.
Author
Collaborator

Implementation plan

Branch test/rings-coverage off main @ 2f7a0d9, one commit, new file
game/rings_test.go (white-box, package game, //nolint:testpackage
header, t.Parallel() everywhere). No game-code changes.

C reference read via git show origin/c-master:rings.c /
origin/c-master:rogue.h / origin/c-master:things.c — no checkout of
those branches.

C facts the tests will be written against

  • rogue.h 275-289: R_PROTECT 0 … R_SUSTARM 13, MAXRINGS 14;
    LEFT 0, RIGHT 1 (rogue.h 122-123). Go's RingKind iota order in
    game/types.go 303-316 matches index-for-index, so uses[o_which] and
    ringUses[RingKind] are the same lookup.
  • rings.c ring_eat uses[], verbatim: 1 R_PROTECT, 1 R_ADDSTR,
    1 R_SUSTSTR, -3 R_SEARCH, -5 R_SEEINVIS, 0 R_NOP, 0 R_AGGR,
    -3 R_ADDHIT, -3 R_ADDDAM, 2 R_REGEN, -2 R_DIGEST, 0
    R_TELEPORT, 1 R_STEALTH, 1 R_SUSTARM.
  • eat = (rnd(-eat) == 0) for negatives — a 1-in-n chance of 1, not a
    literal cost — then if (o_which == R_DIGEST) eat = -eat, so slow
    digestion yields 0 or -1.
  • ring_num uses the otherwise macro, which rogue.h 53 defines as
    break;default — so the four labels R_PROTECT / R_ADDSTR / R_ADDDAM /
    R_ADDHIT fall through to one sprintf and every other kind returns
    "" from the default arm. R_ADDHIT is the dexterity ring, i.e. Go's
    RingDexterity; R_ADDDAM is RingIncreaseDamage.

Tests

  1. ringOn with both hands free: prompts gethand, populates the chosen
    hand, applies the effect (add-strength changes Stats.Str;
    see-invisible sets CanSeeInvisible; aggravate wakes a sleeping
    monster). One hand already full: the free hand is taken with no
    prompt.
  2. ringOff reverses it (dropRing: chg_str(-o_arm), unsee +
    extinguish) and clears the hand; both-hands case prompts; no-rings
    case gives C's two wordings; a cursed ring refuses with
    you can't. It appears to be cursed and stays on.
  3. Both hands occupied -> you already have a ring on each hand /
    wearing two, with a full before/after state comparison asserting
    nothing moved.
  4. Non-ring object -> it would be difficult to wrap that around a finger / not a ring; already-worn ring -> is_current rejection.
  5. gethand: l/L/r/R/ESC, plus reprompt after a bad key.
  6. ringEat over all 14 kinds. Positive entries: exact value and
    zero RNG consumption (C's positive path never calls rnd). Negative
    entries: the game's Rng is snapshotted, ringEat is called, and the
    snapshot is replayed through rnd(n) transcribed from C — pinning
    both the denominator and that exactly one rnd call happens — plus a
    frequency check over thousands of trials so a wrong denominator fails
    loudly. Empty hand -> 0.
  7. ringNum: [+2] / [-1] for each of the four C labels, "" for an
    unknown ring, "" for a known kind outside the labels (the
    break;default arm).

Every one of the 14 kinds is exercised by the ringEat table; kinds with
no ring_on/dropcheck effect get a comment saying so rather than a
made-up assertion.

Verification

make check (fmt-check, lint, test with -race -cover), re-run whole
after any fix since it short-circuits. Lint runs with a private empty
GOLANGCI_LINT_CACHE, retried on the parallel-run lock error, and
accepted only if it names no path outside the worktree. Each substantive
assertion is mutation-proved: the behaviour is broken in the game code,
the run must fail on that test and only that test, then reverted. Results
reported on the PR.

TODO.md gets a Completed Steps entry in the same commit; the Next Step
is narrowed to sticks + wizard commands (#6, #7) rather than rotated.

If any expected value turns out to diverge from C, I stop and report
instead of writing the test to match the Go code.

## Implementation plan Branch `test/rings-coverage` off `main` @ 2f7a0d9, one commit, new file `game/rings_test.go` (white-box, `package game`, `//nolint:testpackage` header, `t.Parallel()` everywhere). No game-code changes. C reference read via `git show origin/c-master:rings.c` / `origin/c-master:rogue.h` / `origin/c-master:things.c` — no checkout of those branches. ### C facts the tests will be written against - `rogue.h` 275-289: `R_PROTECT 0 … R_SUSTARM 13`, `MAXRINGS 14`; `LEFT 0`, `RIGHT 1` (`rogue.h` 122-123). Go's `RingKind` iota order in `game/types.go` 303-316 matches index-for-index, so `uses[o_which]` and `ringUses[RingKind]` are the same lookup. - `rings.c ring_eat` `uses[]`, verbatim: `1` R_PROTECT, `1` R_ADDSTR, `1` R_SUSTSTR, `-3` R_SEARCH, `-5` R_SEEINVIS, `0` R_NOP, `0` R_AGGR, `-3` R_ADDHIT, `-3` R_ADDDAM, `2` R_REGEN, `-2` R_DIGEST, `0` R_TELEPORT, `1` R_STEALTH, `1` R_SUSTARM. - `eat = (rnd(-eat) == 0)` for negatives — a 1-in-n chance of 1, not a literal cost — then `if (o_which == R_DIGEST) eat = -eat`, so slow digestion yields 0 or **-1**. - `ring_num` uses the `otherwise` macro, which `rogue.h` 53 defines as `break;default` — so the four labels R_PROTECT / R_ADDSTR / R_ADDDAM / R_ADDHIT fall through to one `sprintf` and every other kind returns `""` from the default arm. R_ADDHIT is the dexterity ring, i.e. Go's `RingDexterity`; R_ADDDAM is `RingIncreaseDamage`. ### Tests 1. `ringOn` with both hands free: prompts `gethand`, populates the chosen hand, applies the effect (add-strength changes `Stats.Str`; see-invisible sets `CanSeeInvisible`; aggravate wakes a sleeping monster). One hand already full: the free hand is taken with no prompt. 2. `ringOff` reverses it (`dropRing`: `chg_str(-o_arm)`, `unsee` + `extinguish`) and clears the hand; both-hands case prompts; no-rings case gives C's two wordings; a cursed ring refuses with `you can't. It appears to be cursed` and stays on. 3. Both hands occupied -> `you already have a ring on each hand` / `wearing two`, with a full before/after state comparison asserting nothing moved. 4. Non-ring object -> `it would be difficult to wrap that around a finger` / `not a ring`; already-worn ring -> `is_current` rejection. 5. `gethand`: `l`/`L`/`r`/`R`/ESC, plus reprompt after a bad key. 6. `ringEat` over all 14 kinds. Positive entries: exact value **and** zero RNG consumption (C's positive path never calls `rnd`). Negative entries: the game's `Rng` is snapshotted, `ringEat` is called, and the snapshot is replayed through `rnd(n)` transcribed from C — pinning both the denominator and that exactly one `rnd` call happens — plus a frequency check over thousands of trials so a wrong denominator fails loudly. Empty hand -> 0. 7. `ringNum`: `[+2]` / `[-1]` for each of the four C labels, `""` for an unknown ring, `""` for a known kind outside the labels (the `break;default` arm). Every one of the 14 kinds is exercised by the `ringEat` table; kinds with no `ring_on`/`dropcheck` effect get a comment saying so rather than a made-up assertion. ### Verification `make check` (fmt-check, lint, test with `-race -cover`), re-run whole after any fix since it short-circuits. Lint runs with a private empty `GOLANGCI_LINT_CACHE`, retried on the parallel-run lock error, and accepted only if it names no path outside the worktree. Each substantive assertion is mutation-proved: the behaviour is broken in the game code, the run must fail on that test and only that test, then reverted. Results reported on the PR. `TODO.md` gets a Completed Steps entry in the same commit; the Next Step is narrowed to sticks + wizard commands (#6, #7) rather than rotated. If any expected value turns out to diverge from C, I stop and report instead of writing the test to match the Go code.
Author
Collaborator

Implemented in PR #34 (branch test/rings-coverage, commit c61e282):
#34

New game/rings_test.go — 17 tests, 44 subtests, no game-code changes —
covering ringOn, pickRingHand, ringOff, gethand, ringEat and
ringNum, plus dropRing (the things.c dropcheck arm that actually takes
a worn ring off). Package coverage 53.7% -> 56.2%.

Every item in the definition of done is met: correct hand populated and
effect applied; removal reverses the effect and clears the hand; wearing
with both hands occupied rejected with a full before/after state check; a
non-ring rejected; ringEat pinned for all fourteen kinds against C's
uses[] (not just the nonzero ones), both hands; ringNum for bonus and
non-bonus rings, known and unknown. make check is fully green with
golangci-lint at 0 issues. TODO.md has its Completed Steps entry in the
same commit, and Next Step is narrowed to sticks (#6) and wizard commands
(#7) rather than rotated.

No divergence from C was found, so nothing was filed and no gameplay
was touched. The C was read only with git show origin/c-master:... and is
quoted in the test file and in the PR comment, including both flagged
quirks: a negative uses[] entry is a one-in-n chance of 1 rather than a
literal cost, and R_DIGEST's sign flip makes slow digestion return 0 or
-1; ring_num's otherwise macro is break;default (rogue.h 53), so its
four labels fall through to one sprintf and everything else returns ""
from a default arm.

Verified with 23 mutations, each applied to the game code alone and
reverted, each failing its own test and only its own — including stripping
all three ring_on effect arms (3 of 3 failed) and altering three
ringUses entries (exactly those three subtests failed). The full table is
in the PR comment. One mutation exposed a defect in the tests themselves: an
unaccepted hand key made gethand loop on the headless terminal's filler
input, so the test died of the 30s timeout instead of failing on its
assertion. Scripted hand answers now carry an abort tail, and that mutation
fails cleanly.

Every ring kind is exercised by the ringEat table; the eleven that are
inert at wear time in C are documented at the foot of the file as
deliberately not given a wear/remove test, with the files their powers
actually live in.

Implemented in PR #34 (branch `test/rings-coverage`, commit `c61e282`): https://git.eeqj.de/sneak/rgoue/pulls/34 New `game/rings_test.go` — 17 tests, 44 subtests, no game-code changes — covering `ringOn`, `pickRingHand`, `ringOff`, `gethand`, `ringEat` and `ringNum`, plus `dropRing` (the `things.c dropcheck` arm that actually takes a worn ring off). Package coverage 53.7% -> 56.2%. Every item in the definition of done is met: correct hand populated and effect applied; removal reverses the effect and clears the hand; wearing with both hands occupied rejected with a full before/after state check; a non-ring rejected; `ringEat` pinned for all fourteen kinds against C's `uses[]` (not just the nonzero ones), both hands; `ringNum` for bonus and non-bonus rings, known and unknown. `make check` is fully green with golangci-lint at 0 issues. `TODO.md` has its Completed Steps entry in the same commit, and `Next Step` is narrowed to sticks (#6) and wizard commands (#7) rather than rotated. **No divergence from C was found**, so nothing was filed and no gameplay was touched. The C was read only with `git show origin/c-master:...` and is quoted in the test file and in the PR comment, including both flagged quirks: a negative `uses[]` entry is a one-in-n chance of 1 rather than a literal cost, and `R_DIGEST`'s sign flip makes slow digestion return 0 or -1; `ring_num`'s `otherwise` macro is `break;default` (`rogue.h` 53), so its four labels fall through to one `sprintf` and everything else returns `""` from a default arm. Verified with 23 mutations, each applied to the game code alone and reverted, each failing its own test and only its own — including stripping all three `ring_on` effect arms (3 of 3 failed) and altering three `ringUses` entries (exactly those three subtests failed). The full table is in the PR comment. One mutation exposed a defect in the tests themselves: an unaccepted hand key made `gethand` loop on the headless terminal's filler input, so the test died of the 30s timeout instead of failing on its assertion. Scripted hand answers now carry an abort tail, and that mutation fails cleanly. Every ring kind is exercised by the `ringEat` table; the eleven that are inert at wear time in C are documented at the foot of the file as deliberately not given a wear/remove test, with the files their powers actually live in.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/rgoue#5