1 Commits

Author SHA1 Message Date
ba444a2002 Unit-test the eight trap effects against the C reference (closes #14)
`trapHandlers` had eight entries and zero direct tests, on the one
subsystem besides combat that can kill the hero outright. New
`game/traps_test.go` covers all eight arms of `move.c be_trapped`, the
prologue every trap runs through, and the `rust_armor` tail `T_RUST`
calls. Test-only: no game code changes.

Every expected value is transcribed from `origin/c-master` (`move.c`,
`misc.c`, `fight.c`, `monsters.c`, `rogue.h`) and quoted in the file. No
divergence from C was found.

The trap set is `rogue.h` 192-200: there is no separate "poison dart"
kind — `T_DART` is the poisoned dart — and `T_MYST`, the eleven-way
`rnd(11)` message switch, is the eighth. Details the tests are built
around: `BEARTIME`/`SLEEPTIME` are `spread(3)`/`spread(5)`, both of which
reduce to `rnd(0)` and so cost no random number, which is asserted as
well as their values; `T_ARROW` swings at `s_lvl - 1` and `T_DART` at
`s_lvl + 1`; and the strength loss is gated on `!ISWEARING(R_SUSTSTR) &&
!save(VS_POISON)`, whose short circuit means the ring saves a random draw
as well as the strength.

Damage dice and swing arguments are checked by sweeps rather than single
shots: `rnd(n)` is "raw value % n", so one draw cannot separate a d6 from
a d5, and a forced hit or miss cannot see a wrong `at_lvl`. Both shapes
were forced by mutation runs that the single-shot versions survived.

`be_trapped` takes a coordinate, and which coordinate decides whether
`T_TELEP`'s `mvaddch(tc, TRAP)` does anything. Sprung under the hero
(`move.go` 105-108, `case Floor`) the line is redundant: `tc` is the
hero's square, already stamped `TRAP` by the prologue and redrawn by
`teleport()`'s opening `mvaddch(hero, floor_at())`. Walked onto
(`move.go` 94-98, `case Trap`) it is the only writer: `tc` is the square
being stepped onto while the hero still stands on the previous one,
`leave_room` writes blanks and never `TRAP`, and the arm returns before
`finishMove` so no `look()` follows. Both shapes are tested.

The two death messages are deliberately uncovered: each is printed
immediately before `death()`, which reaches `myExit` and `os.Exit`, so
provoking either would kill the test binary. The hero is pinned with
`fortify()` and the damage is checked by replaying C's arithmetic. They
are the only two: `rust_armor`'s `|| ISWEARING(R_SUSTARM)` operand and
its `if (!to_death)` message suppression are covered as well.
2026-08-09 16:01:19 +00:00
2 changed files with 170 additions and 34 deletions

59
TODO.md
View File

@@ -73,7 +73,7 @@ of Completed Steps.
- 2026-08-09 Trap unit-test coverage (`test/traps-coverage`, closes #14):
`trapHandlers` had eight entries and **zero** direct tests, on the one
subsystem besides combat that can kill the hero outright. New
`game/traps_test.go` (17 tests, 13 subtests) covers all eight arms of
`game/traps_test.go` (19 tests, 15 subtests) covers all eight arms of
`move.c be_trapped`, the prologue every trap runs through, and the
`rust_armor` tail `T_RUST` calls. Package coverage 56.2% -> 57.9% measured on
`main` at `bf820e3`, the branch point, before the sticks tests landed. Every
@@ -111,31 +111,50 @@ of Completed Steps.
`at_lvl` or a dropped `op_arm`, because both arms are reachable at any level
and swing spends one `rnd(20)` regardless.
Mutation-proved, 31 mutations, each reverted. Two were **not** caught on the
first pass and the tests were strengthened until they were, which is the
useful part of the record. (a) Deleting `new_level()` from `T_DOOR` left the
suite green: `be_trapped`'s own prologue stamps the trap glyph into the cell
the hero fell through, so "the map changed" is true even with no new level
dug. The test now counts differing cells — exactly one can change that way —
and also requires the staircase to move and the hero to be re-placed. (b)
The `roll(1,6)` case above. One line is recorded as **not isolable**:
`T_TELEP`'s `mvaddch(tc, TRAP)`, whose C comment claims `look()` will not
redraw the vacated square. It is in fact redundant in both C and this port —
the prologue has already set `p_ch` to `TRAP`, `teleport()` opens by drawing
`floor_at()` (which returns `chat(hero)`) over the departing square, and
`leave_room` only blanks squares showing `FLOOR` — so no reachable
configuration makes deleting it observable. That is stated in the test
rather than papered over. The other 29 each failed their own test and only
their own; two also moved `TestAutoSaveOnSignalRacesTurnLoop`, which drives
real turns and is legitimately sensitive to `BEARTIME` and to armor rusting.
Mutation-proved, 33 mutations, each reverted, and every one of them is now
caught. Three were **not** caught on the first pass and the tests were
strengthened until they were, which is the useful part of the record. (a)
Deleting `new_level()` from `T_DOOR` left the suite green: `be_trapped`'s
own prologue stamps the trap glyph into the cell the hero fell through, so
"the map changed" is true even with no new level dug. The test now counts
differing cells — exactly one can change that way — and also requires the
staircase to move and the hero to be re-placed. (b) The `roll(1,6)` case
above.
(c) **`be_trapped` takes a coordinate, and which coordinate decides whether
`T_TELEP`'s `mvaddch(tc, TRAP)` does anything.** Deleting that line first
left the suite green, and the first draft wrote that off as an unavoidable
redundancy — wrongly, because the test only exercised one of the two call
sites. `move.go` 105-108 (`case Floor`) springs a trap under the hero and
passes `p.Pos`; there `tc` **is** the hero's square, the prologue has
already set its `p_ch` to `TRAP`, and `teleport()` opens by drawing
`floor_at()` — which returns `chat(hero)` — over it, so the glyph is on
screen before the line runs. But `move.go` 94-98 (`case Trap`), the ordinary
walk onto a hidden trap, passes `nh`, the square being stepped **onto**,
with the hero still on the previous square: `teleport()`'s opening `mvaddch`
paints the old square, `leave_room` writes blanks and never `TRAP`, and
nothing calls `look()` afterwards because the `case Trap` arm returns before
`finishMove` for a teleporter. There `mvaddch(tc, TRAP)` is the only writer,
exactly as C's comment says.
`TestTrapTeleportDrawsTheTrapOnTheSquareSteppedOnto` springs the trap at a
floor square next to the hero and pins it: unmutated the screen at `tc`
reads `^`, with the line deleted it reads `.`.
The other 30 each failed their own test and only their own; two also moved
`TestAutoSaveOnSignalRacesTurnLoop`, which drives real turns and is
legitimately sensitive to `BEARTIME` and to armor rusting.
Deliberately uncovered: the two death messages, "an arrow killed you" and "a
poisoned dart killed you". Each is printed immediately before `death()`,
which reaches `myExit` and `os.Exit`, so provoking either would take the
test binary with it; the hero is pinned with `fortify()` and the damage
rolls are checked by replaying C's arithmetic instead of by letting HP reach
zero. `Next Step` deliberately not rotated: #14 was an out-of-band gap found
while surveying, not part of the rings/sticks/wizard step.
zero. They are the only two: `rust_armor`'s `|| ISWEARING(R_SUSTARM)`
operand and its `if (!to_death)` suppression of the rust-vanishes message,
the last predicates that had no assertion, are pinned by
`TestTrapRustHonoursTheRingAndTheToDeathFlag`. This entry does **not**
rotate `Next Step`: #14 was an out-of-band gap found while surveying, not
part of the rings/sticks/wizard step.
- 2026-08-09 Ring unit-test coverage (`test/rings-coverage`, closes #5): the
first third of the standing coverage step. `game/rings.go` had **zero** tests

View File

@@ -617,21 +617,19 @@ func assertArrowOnFloor(
}
// TestTrapTeleportMovesTheHeroAndDrawsTheTrap covers case T_TELEP: the
// hero is relocated and the vacated square is left showing the trap.
// hero is relocated and the square he was standing on is left showing
// the trap.
//
// One honest caveat, found by mutating the arm and watching this test
// stay green. C's second line carries the comment "since the hero's
// leaving, look() won't put a TRAP down for us, so we have to do it
// ourself", but the mvaddch it justifies is in fact redundant in both C
// and this port: be_trapped's prologue has already set the cell's p_ch to
// TRAP, and teleport() opens by drawing floor_at() over the departing
// square — floor_at() returns chat(hero) — so the glyph is on screen
// before the line runs, and rooms.c leave_room only blanks squares that
// are showing FLOOR. The screen assertion below is therefore a true
// statement about the end state and not an isolation of that one call;
// no reachable configuration makes it one. Left in place rather than
// deleted: the property is what a player sees, and it would fail if
// either of the two writes that actually produce it went away.
// be_trapped takes a coordinate, and the two call sites pass different
// ones. This test is the tc == hero shape: move.go's "case Floor" arm
// springs a trap the hero is already standing on and passes p.Pos. In
// that shape C's mvaddch(tc, TRAP) is not what puts the glyph on screen
// — the prologue has already set the cell's p_ch to TRAP and teleport()
// opens by drawing floor_at(), which returns chat(hero), over the
// departing square — so this test asserts the end state a player sees
// and does not isolate that one call. The other shape, walking onto the
// trap, is where the line is the only writer; that is
// TestTrapTeleportDrawsTheTrapOnTheSquareSteppedOnto below.
func TestTrapTeleportMovesTheHeroAndDrawsTheTrap(t *testing.T) {
t.Parallel()
@@ -662,6 +660,71 @@ func TestTrapTeleportMovesTheHeroAndDrawsTheTrap(t *testing.T) {
}
}
// steppedOnSquare picks a square next to the hero that is drawn as plain
// floor, standing in for the square do_move is about to step onto. It
// must not be the hero's own square: that is the one shape in which
// teleport() redraws the trap glyph by itself.
func steppedOnSquare(t *testing.T, g *RogueGame, hero Coord) Coord {
t.Helper()
for _, d := range []Coord{
{Y: -1, X: -1}, {Y: -1, X: 0}, {Y: -1, X: 1},
{Y: 0, X: -1}, {Y: 0, X: 1},
{Y: 1, X: -1}, {Y: 1, X: 0}, {Y: 1, X: 1},
} {
c := Coord{Y: hero.Y + d.Y, X: hero.X + d.X}
if g.Level.Char(c.Y, c.X) == Floor &&
g.scr.Std.MvInch(c.Y, c.X) == Floor {
return c
}
}
t.Fatalf("no plain floor square next to the hero at %v", hero)
return Coord{}
}
// TestTrapTeleportDrawsTheTrapOnTheSquareSteppedOnto covers the second
// line of case T_TELEP, "mvaddch(tc->y, tc->x, TRAP)", in the shape that
// makes it load-bearing — the ordinary walk onto a hidden trap.
//
// move.go's "case Trap" arm (move.c do_move) passes nh, the square being
// stepped *onto*, while the hero is still standing on the previous
// square. So teleport()'s opening mvaddch(hero, floor_at()) paints that
// previous square and not tc, and rooms.c leave_room writes blanks and
// never TRAP. The mvaddch is then the only thing that puts the glyph
// where the player has just discovered a trap, which is exactly what C's
// comment claims: "since the hero's leaving, look() won't put a TRAP
// down for us, so we have to do it ourself".
//
// Nothing later covers for it, either. The case Trap arm returns before
// finishMove when the trap was a teleporter, so this direct springTrap
// call is the whole of that path, and look() only ever redraws the nine
// squares around the hero's new position.
func TestTrapTeleportDrawsTheTrapOnTheSquareSteppedOnto(t *testing.T) {
t.Parallel()
g := mkTrapGame(t)
hero := g.Player.Pos
tc := steppedOnSquare(t, g, hero)
plantTrap(g, tc, TrapTeleport)
if got := g.springTrap(tc); got != TrapTeleport {
t.Errorf("springTrap returned %d, want T_TELEP (%d)", got, TrapTeleport)
}
if g.Player.Pos == hero || g.Player.Pos == tc {
t.Fatalf("the hero is at %v after being teleported off %v; "+
"he started on %v and must have gone somewhere else",
g.Player.Pos, tc, hero)
}
if ch := g.scr.Std.MvInch(tc.Y, tc.X); ch != Trap {
t.Errorf("the square stepped onto shows %q, want the TRAP glyph %q",
ch, Trap)
}
}
// TestTrapDartMissesTheHero covers the first arm of case T_DART,
// "if (!swing(pstats.s_lvl+1, pstats.s_arm, 1)) msg(...)". Note the
// **plus** one, the opposite of T_ARROW's minus one. Nothing else may
@@ -885,6 +948,60 @@ func TestTrapRustSoaksTheHero(t *testing.T) {
}
}
// TestTrapRustHonoursTheRingAndTheToDeathFlag covers the two predicates
// of move.c rust_armor that the table above cannot reach, because every
// row of it takes the left branch through ISPROT and leaves to_death
// clear:
//
// if ((arm->o_flags & ISPROT) || ISWEARING(R_SUSTARM))
// {
// if (!to_death)
// msg("the rust vanishes instantly");
// }
//
// No armor here is ISPROT, so the ring is the only thing that can save
// it, and the second row then checks that fighting to the death
// swallows the message while still saving the armor. Both rows expect an
// unrusted o_arm; what separates them is which message the line is left
// showing.
func TestTrapRustHonoursTheRingAndTheToDeathFlag(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
toDeath bool
wantMsg string
}{
{"the ring alone saves the armor", false, cRustVanishes},
{"to_death swallows the message", true, cGushOfWater},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
g := mkTrapGame(t)
g.ToDeath = tc.toDeath
armor := mkArmor(ArmorPlateMail, 3)
g.Player.CurArmor = armor
g.Player.CurRing[Left] = mkRing(RingMaintainArmor, 0)
pos := g.Player.Pos
plantTrap(g, pos, TrapRust)
g.springTrap(pos)
if armor.ArmorClass != 3 {
t.Errorf("o_arm = %d with R_SUSTARM worn, want it held at 3",
armor.ArmorClass)
}
if g.Msgs.Huh != tc.wantMsg {
t.Errorf("message = %q, want %q", g.Msgs.Huh, tc.wantMsg)
}
})
}
}
// TestTrapRustAlwaysAnnouncesTheWater pins the gush in the case where a
// second message follows it and Msgs.Huh can therefore no longer see it.
// C prints the gush unconditionally, *before* rust_armor is called, so a