sneak/rgoue
1
0
Fork 0
Code Issues 10 Pull Requests Actions Packages Projects Releases Wiki Activity

Compare commits

base: sneak:main
Branches Tags
sneak:main sneak:next sneak:refactor/lint-adoption sneak:modern-rogue sneak:c-master
sneak:v0.0.1
..
compare: sneak:cb7ddfdb6790084789d648ab68faa65192f81f1e
Branches Tags
sneak:main sneak:next sneak:refactor/lint-adoption sneak:modern-rogue sneak:c-master
sneak:v0.0.1

1 Commits
main ... cb7ddfdb67

Author SHA1 Message Date
clawbot
cb7ddfdb67 test: drive the autosave race test to a condition, not a turn count (closes #36) 
TestAutoSaveOnSignalRacesTurnLoop failed intermittently under load. The
captured failure text settles what it was: driveUntilDone's

    t.Fatal("the turn loop ran out of turns before the saves were taken")

with no WARNING: DATA RACE anywhere in the log. The handoff is fine; the
test's own drive loop ran out of its fixed 1000-turn budget first.

Confirmed by instrumenting the loop to report the turns it actually
used. The count tracks scheduling pressure and nothing else: about
60-120 turns at host load ~57 with the whole machine to spread over, 418
at GOMAXPROCS=4, 539 and 655 at 2 and 1, and past 1000 under the doubled
load of the verbose rerun the test target performs after a failure. The
turns spent between one save being answered and the next request
arriving are not work, they are the saving goroutine's wake-up latency,
so a fixed turn count is a wall-clock assumption in disguise. Raising it
would hide the flake, not fix it.

So the budget is gone rather than larger. driveUntilDone drives until
the saving goroutine finishes and nothing else. Termination still holds,
it just belongs to the code under test: every AutoSaveOnSignal returns
within the timeout it is handed, and g.sigSave is one deep, so an
unserviced request stays in the channel and every later call finds it
full and fails at once. A dead handoff therefore releases the saving
goroutine after one autoSaveWait however many saves were asked for, and
what fails is the real assertion - "saves taken = 0, want 25" - rather
than "out of turns".

Removing the cap exposed a second assumption underneath it. testTerm
answers space and newline for ever once its script is exhausted, and
neither key takes a turn, so command(), which loops until the player
consumes one, never returns; the old cap was silently sized to the
script. An uncapped drive wedged inside a single command() call. The two
drive tests now use driveTerm, a headless terminal whose script repeats,
so every key it hands out takes a turn.

The guard is undiminished, shown by mutation and reverted afterwards.
Reverting the fix from the earlier signal-autosave work - AutoSaveOnSignal
replaced by a direct g.autoSave(), encoding on the calling goroutine -
still fails the test with 139 DATA RACE reports, snapshotHeader reading
what executeCommand writes. Removing serviceAutoSaveRequest from
command() still fails it too, now in 10s with "saves taken = 0, want 25"
instead of by hanging.

Under load: at GOMAXPROCS=2 on a 48-core host at load ~150, with an
unrelated deliberate failure in the tree so every run took the verbose
rerun, the old code failed 8 of 8 runs and the new code 0 of 8. Also
green across 24 concurrent unconstrained runs at load ~120, 10 runs
alongside a spinner load, and 5 runs each at GOMAXPROCS 1, 2 and 4.

No non-test code changed. make check green, lint 0 issues, .golangci.yml
byte-identical.
 2026-08-09 16:45:34 +00:00
8 changed files with 48 additions and 261 deletions
Expand all files Collapse all files

8
.dockerignore
View File

@@ -1,8 +0,0 @@
# Part of the lint gate: only what reaches the container is linted, so
# excluding a self-contained Go source here drops it from the lint silently.
# Never exclude Go sources, go.mod/go.sum or .golangci.yml.
.git
# Generated artifacts only; `make build` puts a multi-megabyte binary here
# and it would otherwise be shipped into the build context.
/build/

1
.gitignore vendored
View File

@@ -1,5 +1,4 @@
*.log
*.out
*.test
/build/
/rogue

20
Dockerfile.lint
View File

@@ -1,20 +0,0 @@
# Lint image, built by script/lint: golangci-lint runs as a build step, so
# a successful build is a clean lint.
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps
WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
# This stage must stay the one that runs golangci-lint, and its name must
# match $stage in script/lint. --target halts the build at this stage, so
# moving the lint step to another stage, or adding a stage after this one,
# is not caught.
FROM deps AS lint
COPY . .
RUN golangci-lint run --config .golangci.yml ./...

47
Makefile
View File

@@ -1,47 +1,18 @@
# Development convenience targets. This repo is exempt from the standard
# policy scaffold (no CI config, no REPO_POLICIES.md, no application
# Dockerfile) except for the lint container: per sneak's 2026-08-09
# ruling, linting runs in docker only, so Dockerfile.lint and script/lint
# are part of this repo. This Makefile is otherwise only a thin wrapper
# around the Go toolchain and prettier so `make fmt` / `make check` behave
# the same as in sneak's other repos.
# policy scaffold (no Dockerfile, CI, or REPO_POLICIES.md); this Makefile
# is only a thin wrapper around the Go toolchain, golangci-lint, and
# prettier so `make fmt` / `make check` behave the same as in sneak's
# other repos.
GO_PKGS := ./...
MD_FILES := $(shell git ls-files '*.md')
PRETTIER := prettier --tab-width 4 --prose-wrap always
# Every generated artifact goes here, and the whole directory is
# git-ignored. Targets that write outside it can commit their output.
BUILD_DIR := build
BIN := $(BUILD_DIR)/rogue
COVERPROF := $(BUILD_DIR)/coverage.out
COVERHTML := $(BUILD_DIR)/coverage.html
.PHONY: check fmt fmt-check lint test
.PHONY: build check cover cover-html fmt fmt-check lint test
# Format, lint, and test — the full local pre-commit gate. Keep this list
# to targets that write nothing into the working tree.
# Format, lint, and test — the full local pre-commit gate.
check: fmt-check lint test
# Build the executable into $(BUILD_DIR). `go build -o` does not create the
# parent directory.
build:
@mkdir -p $(BUILD_DIR)
go build -o $(BIN) ./cmd/rogue
# Per-function coverage, for finding which functions are untested. The
# percentage `make test` prints is a per-package total and cannot answer
# that. Writes files, so it stays out of `check`.
cover:
@mkdir -p $(BUILD_DIR)
go test -timeout 30s -coverprofile=$(COVERPROF) $(GO_PKGS)
go tool cover -func=$(COVERPROF)
# Render the same profile as annotated source.
cover-html: cover
go tool cover -html=$(COVERPROF) -o $(COVERHTML)
@echo "wrote $(COVERHTML)"
# Format Go and Markdown in place.
fmt:
gofmt -w .
@@ -55,11 +26,9 @@ fmt-check:
fi
$(PRETTIER) --check $(MD_FILES)
# Run the house linter. golangci-lint is never installed on the host: the
# work happens inside the pinned container built by Dockerfile.lint, and
# this target is a thin shim over the script that builds it.
# Run the house linter (config in .golangci.yml).
lint:
./script/lint
golangci-lint run $(GO_PKGS)
# Run the test suite. Quiet on success; on failure, rerun verbosely for the
# full output and still fail the target (the first run already proved the

24
README.md
View File

@@ -21,19 +21,19 @@ original program structure and the design of this port.
Requires Go 1.25 or later and a terminal at least 80x24.
```bash
make build
./build/rogue
go build ./cmd/rogue
./rogue
```
```bash
# Restore a saved game
./build/rogue ~/rogue.save
./rogue ~/rogue.save
# View high scores
./build/rogue -s
./rogue -s
# Test the death screen (demo mode)
./build/rogue -d
./rogue -d
```
## In-game commands
@@ -57,7 +57,7 @@ Press `?` in game for the full list.
export ROGUEOPTS="name=YourName,terse,jump,fruit=mango"
# Wizard (debug) mode, with a reproducible dungeon
ROGUE_WIZARD=1 SEED=12345 ./build/rogue
ROGUE_WIZARD=1 SEED=12345 ./rogue
```
The scoreboard is kept in `~/.rogue.scores`. Save files are Go gob snapshots
@@ -77,14 +77,10 @@ sequences, dungeon-generation golden checks, and an RNG compatibility test
against the original C generator.
For development, the `Makefile` wraps the toolchain: `make fmt` (gofmt +
prettier), `make lint` (`script/lint`, which runs golangci-lint inside the
pinned container built from `Dockerfile.lint` — it is never installed on the
host, so docker is required), `make test` (the suite, under the race detector
with coverage and a timeout), `make check` (all three), `make build` (the
executable), and `make cover` / `make cover-html` (per-function coverage, and
the same profile as annotated source at `build/coverage.html`). Everything they
generate lands in the git-ignored `build/`. Use the targets rather than the
toolchain directly — they carry the flags the project relies on.
prettier), `make lint` (golangci-lint), `make test` (the suite, under the race
detector with coverage and a timeout), and `make check` (all three). Use the
targets rather than invoking `go test` directly — they carry the flags the
project relies on.
## License

96
TODO.md
View File

@@ -35,55 +35,6 @@ is finished.
# Completed Steps
- 2026-08-10 `make cover` added (https://git.eeqj.de/sneak/rgoue/issues/17).
`make cover` writes `build/coverage.out` and prints the per-function report;
`make cover-html` renders the same profile to `build/coverage.html`. The
per-package percentage `make test` prints cannot say _which_ function is
untested, which is how the coverage gaps closed so far had to be found — by
grepping test files for identifiers.
Neither target is in `check`, and neither may be added to it: both write
files, and `make check` must not modify the working tree.
- 2026-08-10 `make build` added (https://git.eeqj.de/sneak/rgoue/issues/19). The
executable is built to `build/rogue`; `README.md` no longer contains a raw
`go` invocation anywhere. `build` is in neither `check` nor `test` —
`make check` stays `fmt-check lint test` and still writes nothing into the
working tree.
Generated artifacts now all live under `build/`, which `.gitignore` covers
as a whole. Anything written outside it is committable, so a target that
puts its output elsewhere reintroduces the stray-artifact problem.
- 2026-08-10 Linting moved into a container
(https://git.eeqj.de/sneak/rgoue/issues/41). `golangci-lint` is no longer
invoked on the host anywhere in the repo: `Dockerfile.lint` pins
`golangci/golangci-lint:v2.12.2` by digest and runs the linter as a build
step, so a successful build is a clean lint, and `make lint` is now a shim
over `script/lint`. This is what killed the false green seen earlier, where a
branch that was genuinely red with a `goconst` finding reported `0 issues` off
the shared host cache; a container per run has its own cache and lock.
`script/lint` builds with `--target "$stage"`, `--no-cache-filter="$stage"`
and `--output=type=cacheonly`. The durable property to check when touching
any of this: the lint stage executes on every run and is never served from
cache. Three things no tooling checks, left to whoever edits the gate —
`$stage` must match the stage name in `Dockerfile.lint`; that stage must
stay the one running `golangci-lint`, since `--target` halts the build
there; and `.dockerignore` governs what reaches the container, so excluding
a self-contained Go source drops it from the lint silently.
Verified rather than assumed, since a green docker build is the classic
false green: two consecutive runs on an unchanged tree each showed the
`golangci-lint run` layer executing and reporting `0 issues.` while the
`deps` layers reported `CACHED`; the same build with `--no-cache-filter`
removed reported that layer `CACHED`, so the re-execution is attributable to
the flag rather than to a changed context; deliberate violations failed the
build naming the specific finding and reverted clean; a stage-name typo
failed loudly at exit 1; and a Go file excluded via `.dockerignore` reported
`0 issues.` at exit 0 with the violation still in the tree. Wall-clock
durations vary per host and per run, so they are not recorded here.
- 2026-08-09 `TestAutoSaveOnSignalRacesTurnLoop` de-flaked at the cause
(`fix/autosave-turn-budget-36`, closes #36). The failure text was captured
before anything was changed and it is **not** a data race: the assertion was
@@ -107,18 +58,12 @@ is finished.
So the budget is gone rather than larger. `driveUntilDone` now drives until
the saving goroutine finishes and nothing else. Termination is not lost, it
just belongs to the code under test instead of to the test: every
`AutoSaveOnSignal` returns within the timeout it is handed, so the saving
goroutine always finishes. A handoff that has stopped answering costs one
`autoSaveWait` in total — `g.sigSave` is one deep, so an unserviced request
stays in the channel and every later call finds it full and fails at once —
and the failure is then the real assertion (`saves taken = 0, want 25`)
instead of "out of turns". The worst case is not that one: a handoff that
drains each request but slower than `autoSaveWait` costs one timeout per
save, `wantSaves × autoSaveWait` = 250s, which would run past the 30s
package timeout instead of reaching the assertion. It takes ~10s of
scheduler starvation per save against a measured 0.12s per 1000 turns, so it
is remote, and the turn cap did not bound it either. The comment in the test
states that bound rather than the optimistic one.
`AutoSaveOnSignal` returns within the timeout it is handed, and `g.sigSave`
is one deep, so an unserviced request stays in the channel and every later
call finds it full and fails at once — a dead handoff releases the saving
goroutine after one `autoSaveWait` however many saves were asked for, and
the failure is then the real assertion (`saves taken = 0, want 25`) instead
of "out of turns".
Removing the cap exposed a second assumption underneath it, which is the
reason this is not a one-line diff. `testTerm` answers space and newline for
@@ -127,14 +72,7 @@ is finished.
old cap was silently sized to the script (4000 characters, two per turn,
against 1000 turns). An uncapped drive wedged inside a single `command()`
call. The two drive tests therefore use a new `driveTerm`, a headless
terminal whose script repeats. Repeating is necessary but not sufficient,
and the test says so: `' '` clears `After` outright and all eight movement
keys clear it on a refused step, so a script of only those keys wedges just
as `testTerm`'s tail did. What makes the wedge impossible is that the cycle
always holds an _unconditional_ turn-taker, and these scripts hold two —
`'.'` (empty handler) and `'s'` (`search`, which writes `After` on no path),
neither refusable by blocked-in-all-directions, `Held`, a bear trap, or
`NoCommand > 0`. Removing both would bring the wedge back.
terminal whose script repeats, so every key it hands out takes a turn.
Both halves of the definition of done were demonstrated by mutation, with
the deliberately-broken tree reverted afterwards and `.golangci.yml` left
@@ -754,11 +692,7 @@ is finished.
24 long lines wrapped or their comments tightened, control bytes in
`term/tcell.go` as character literals, and two `wsl_v5` defer cuddles. The
repo has no golangci-lint version pin to bump (no Dockerfile or CI;
`make lint` runs whatever `golangci-lint` is on the host). Superseded
2026-08-10: there is a pin now, and no host lint path — `Dockerfile.lint` pins
the linter image by digest and `script/lint` runs it in a container. See the
2026-08-10 entry at the top of this section
(https://git.eeqj.de/sneak/rgoue/issues/41).
`make lint` runs whatever `golangci-lint` is on the host).
- 2026-07-24 Seed compatibility — item tables (seed-compat): instrumented the C
reference on modern-rogue with a DUMP mode (testdata/c_seedcompat.patch) that
@@ -906,13 +840,7 @@ is finished.
per-game dungeon dimensions instead of the 80x24 constants; open design
questions are resize policy, gameplay tuning at larger sizes, and a --classic
80x24 mode.
2. Note: this repo is exempt from the standard policy scaffold, but the
exemption is narrower than it was. A minimal dev Makefile
(fmt/fmt-check/lint/test/check targets) exists per sneak's 2026-07-07
request. `Dockerfile.lint` and `script/lint` are now also permitted, and
required, along with the `.dockerignore` that scopes their build context:
sneak's 2026-08-09 ruling (https://git.eeqj.de/sneak/rgoue/issues/41) is that
every repo lints in a container invoked through `script/lint`, and being
later and explicit it overrides the 2026-07-07 exemption for those three
files only. Still do not add: CI config, `REPO_POLICIES.md`, an application
`Dockerfile`, or any other `script/` entrypoint.
2. Note: this repo is exempt from the standard policy scaffold. A minimal dev
Makefile (fmt/fmt-check/lint/test/check targets) exists per sneak's
2026-07-07 request, but do not add a Dockerfile, CI config, or
REPO_POLICIES.md.

70
game/autosave_test.go
View File

@@ -17,8 +17,7 @@ import (
// expect the save to be taken. It is long enough that a loaded machine
// cannot turn a working handoff into a spurious failure, and it is never
// actually waited out on a passing run. It is also what bounds
// driveUntilDone, by way of the saving goroutine it waits for — see
// there for what that bound comes to.
// driveUntilDone, by way of the saving goroutine it waits for.
const autoSaveWait = 10 * time.Second
// TestAutoSaveOnSignalRacesTurnLoop is the test issue #24 exists for: it
@@ -37,9 +36,7 @@ func TestAutoSaveOnSignalRacesTurnLoop(t *testing.T) {
// Same mix as TestTurnLoopCrashSweep — the spaces answer any --More--
// prompt — on a driveTerm, so the drive can run for as long as the
// saves take rather than for as long as a script lasts. The '.' and
// the 's' are what make an unbounded drive safe, and at least one of
// the two has to stay in the cycle: see driveTerm.
// saves take rather than for as long as a script lasts.
term := &driveTerm{script: []byte("h j k l y u b n s . ")}
g := New(Params{Seed: 20260809, Term: term})
@@ -92,28 +89,16 @@ func TestAutoSaveOnSignalRacesTurnLoop(t *testing.T) {
// guessed cannot be guessed right, so there is no budget.
//
// Dropping it costs no termination guarantee, because the bound belongs
// to the code under test and not to this loop: each AutoSaveOnSignal
// call returns within the timeout the caller hands it, so the saving
// goroutine always finishes and done always closes. That bound is worth
// stating exactly, because it is not one autoSaveWait.
//
// A handoff that has stopped answering altogether costs one, in total,
// however many saves were asked for. g.sigSave
// is one deep, so the unserviced request stays in the channel and every
// later call finds it full and reports failure immediately — measured
// at 10.0s for 25 saves with the service point deleted from command().
// What fails is then the caller's own assertion, the count of saves
// actually taken, which says far more than "out of turns" ever did.
//
// A handoff that still drains every request but takes longer than
// autoSaveWait to do it is the worst case, and costs one timeout per
// save: wantSaves * autoSaveWait, 250s at these constants, which would
// run past the package timeout rather than reach the assertion. It
// takes about ten seconds of scheduler starvation per save to get
// there, against a regime measured at 0.12s per 1000 turns, so it is
// remote — and the 1000-turn cap did not bound it either, a turn count
// being no kind of time bound. `go test -timeout 30s` is the backstop
// under all of it.
// to the code under test and not to this loop. Each AutoSaveOnSignal
// call returns within the timeout the caller hands it, and g.sigSave is
// one deep: once a request goes unserviced it stays in the channel, so
// every later call finds it full and reports failure immediately. A
// handoff that has stopped answering therefore releases the saving
// goroutine after one autoSaveWait however many saves were asked for,
// done closes, and the caller's own assertion — the count of saves
// actually taken — is what fails, which says far more than "out of
// turns" ever did. `go test -timeout 30s` remains the backstop under
// that.
//
// The one thing the caller does have to supply is a terminal that can
// feed an unbounded drive: see driveTerm.
@@ -482,34 +467,9 @@ func mkBlockedGame(t *testing.T, term Terminal) *RogueGame {
// `if !g.After { ntimes++ }` in command.c — never returns. A drive with
// a turn cap sized to its script never notices; a drive that runs until
// the saves are taken wedges inside a single command() call, which is
// what a first attempt at issue #36 did.
//
// Repeating the script is necessary but nowhere near sufficient, and
// the difference is what anyone editing one of these scripts has to
// know. Most keys take a turn only conditionally. ' ' is the "legal
// illegal command" and clears After outright (tables.go). All eight
// movement keys clear it whenever the step is refused: a wall or the
// map edge (move.go moveResolve), an illegal diagonal (moveTarget), or
// a confused step that lands back in place (moveHero). A script of
// nothing but those keys wedges exactly the way testTerm's tail does,
// repetition or no repetition — with the script set to just " " this
// drive hits the 30s package timeout inside command().
//
// What actually makes the wedge impossible is that the cycle always
// contains at least one *unconditional* turn-taker, and the scripts
// here carry two: '.', the rest command, whose handler is empty, and
// 's', search, which writes After on no path. Nothing refuses either
// one — not being blocked in all eight directions, not Held, not stuck
// in a bear trap, and not NoCommand > 0, where playTurn skips
// executeCommand altogether and After is simply left true. Trim both
// out and the wedge this test exists to remove comes straight back.
//
// One further precondition, from what this fake does not supply:
// testTerm's tail answered a newline every other read and this does
// not. Nothing reachable from these scripts asks for one — waitFor('\n')
// sits on the death and score paths (rip.go, score.go), which fortify
// prevents from ever being reached — but a script that could reach them
// would park in waitFor for ever.
// what a first attempt at issue #36 did. Cycling a script of commands
// that all take a turn removes the failure mode instead of sizing
// around it.
type driveTerm struct {
script []byte
pos int

37
script/lint
View File

@@ -1,37 +0,0 @@
#!/bin/sh
# script/lint: lint in docker. golangci-lint is never installed on the host.
#
# Traps, each of which yields a green run over an unlinted or partly linted
# tree:
#
# 1. --target and --no-cache-filter must both stay, and $stage must match
# the stage name in Dockerfile.lint. BuildKit ignores --no-cache-filter
# when no stage matches its argument, serving the lint layer from cache
# without a word; --target rejects a name that is not in the file, which
# is what makes the single $stage safe.
#
# 2. --target checks that the stage exists, not that it is the stage
# running golangci-lint, and it halts the build there. Moving the lint
# step to another stage, or adding a stage after it, is not caught.
#
# 3. .dockerignore decides what reaches the container, and only what
# reaches it is linted. Excluding a self-contained Go file drops it from
# the lint silently. Never exclude Go sources, go.mod/go.sum or
# .golangci.yml.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Must match the stage name in Dockerfile.lint.
stage=lint
main() {
cd "$ROOT"
docker build \
--target "$stage" \
--no-cache-filter="$stage" \
--output=type=cacheonly \
-f Dockerfile.lint .
}
main "$@"