summaryrefslogtreecommitdiff
path: root/src/docs/TESTING_PLAN.md
blob: 7d2bfc28ff0d34c304c5debcafe83497cc504f8c (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
# Testing plan: closing coverage gaps in test*.c

Written 2026-09-16 after an audit of every `src/test*.c` file and how
(or whether) each is wired into `main.c`'s self-test sequence. Goal
per Scott: not more test code for its own sake -- find the
years-old bugs that asserts have been missing. Priorities: search,
eval, generate (not exclusively). Perf testing is also explicitly
in scope.

## Dead/disconnected infrastructure (fix first -- near-zero cost)

Several tests already exist but don't run. Recovering this coverage is
strictly cheaper than writing anything new, so do this before anything
else below.

1. **SEE has zero active correctness testing.** `testsee.c`'s own
   header comment admits it: `DebugSEE`/`DebugSEERecursive` (a
   brute-force make/unmake exchange-walk oracle meant to check `SEE()`
   against) is wrapped in `#ifdef TEST_BROKEN` -- a define nothing
   sets. `testsee.c`'s only *called* functions (`TestGetAttacks`,
   `TestIsAttackedBB`) test attack generation, not SEE. Given SEE is
   what `GetMoveSee`/extensions/QSearch pruning all trust (and is the
   exact code reworked in commit c0ac3d7), this is the single
   highest-value fix here. Get `TEST_BROKEN` -> live (`TEST`), fix
   whatever bitrot surfaces, and confirm it's called from `main.c`.
2. **`TestEvalWithKnownPositions`** (`testeval.c`, ~20 hand-curated
   "side X should be better here" positions from chess literature) is
   defined but **never called from anywhere** -- not `main.c`, not
   `command.c`. Wire it into the self-test sequence.
3. **`TestRecognExhaustiveKNKP`** (`testrecogn.c`, exhaustive KNKP
   endgame-recognizer enumeration) is defined and prototyped in
   `chess.h` but referenced only in a comment -- never invoked. Only
   the smaller table-driven `TestRecogn` runs. Wire it in.
4. **The hash table has no correctness self-test at all.**
   `testhash.c` contains `SanityCheckHashEntry` (an entry-shape
   validator) and `AnalyzeFullHashTable` -- but the latter is a
   post-search *diagnostic dump* called from `root.c`, not a test, and
   it's never invoked from the `TEST` self-test sequence. See "New
   test: hash correctness" below for what should replace this gap, not
   just a wiring fix.
5. **`TestEvalWithSymmetry`** only runs a *single* random symmetric
   position per invocation, and only when a human types `eval` at the
   prompt in an `EVAL_DUMP` build (`command.c:2203`) -- not part of the
   automated suite, never loops. See "New test: eval symmetry fuzz"
   below.

## New tests, by area

### Search (thinnest area relative to its bug surface)

- `TestSearch` (`testsearch.c`) runs 20 random positions at fixed
  **depth 2** only, checking solely "returned move is legal" and
  "didn't blow the time limit."
  - **DONE -- hash-consistency test** (`TestSearchHashConsistency`,
    testsearch.c): searches a position to depth D twice in a row with
    a warm hash table (scores must match), and separately confirms a
    depth-D2 search seeded by a shallower depth-D1 warm-up agrees with
    a from-scratch depth-D2 search. Wired into the `TEST=1` self-test
    sequence (main.c).
  - **DONE (partially) -- QSearch sanity test**
    (`TestQSearchSanity`, testsearch.c): checks search termination
    (no `INVALID_SCORE`/garbage) and a whole-search node-count
    ceiling. Caveat found and documented in the function's own header
    comment: the self-test suite builds with plain `gmake TEST=1`,
    which doesn't define `PERF_COUNTERS` -- every `sCounters` field
    (including `u64QNodeCount`, the qsearch-only counter this was
    originally meant to check) is wrapped in `chess.h`'s `INC()` macro,
    a no-op without that define. Confirmed live (`sd 3; go` on a
    `TEST`-only binary reports "0 qnodes" regardless of real qsearch
    activity). The test currently checks
    `ctx->sCounters.tree.u64TotalNodeCount` instead (root.c's
    unconditional per-node `++`, always live) -- a real but strictly
    weaker signal, since it can't distinguish "qsearch exploded" from
    "full-width search exploded." Building a `PERF_COUNTERS`-enabled
    `TEST` profile (or adding an always-live qsearch-only counter) is
    the natural follow-up to close this for real. The hand-auditable-
    tree idea (`Trace()` every qsearch node on a small position) is
    still worth doing but wasn't implemented this session.
  - **NOT DONE -- determinism test** (same position/depth searched
    twice, PV must match): explicitly descoped -- Scott's call is that
    a stable-PV requirement makes the test break every time move
    ordering changes for unrelated reasons, which is just noise, not a
    real regression signal. A same-*score* (not same-PV) check is
    already covered by the hash-consistency test above.
  - **NOT DONE -- MP-vs-single-thread agreement test**: still open.
    `split.c` has no dedicated correctness test.
  - **DONE (as an EPD suite, not a C test) -- null-move/zugzwang
    soundness**: `tests/zugzwang.epd` (5 positions, run via the new
    `epd` command -- see below) checks actual best-move outcome
    instead of an internal counter. An earlier attempt asserted
    against `ctx->sCounters.extension.uZugzwang` (the verification-
    search hit counter) using the 3 classic literature zugzwang
    positions, but none of them structurally reach that code path on
    this build: 2 have too little material for
    `WeShouldTryNullmovePruning`'s own `> 2` non-pawn-count gate to
    attempt null-move pruning at all, and the 3rd (despite landing in
    the eligible window) gets resolved directly off this box's local
    Syzygy tablebase before search ever touches null-move logic --
    confirmed live via the `TABLEBASE MOVE` trace line. Removed in
    favor of the EPD suite, which doesn't care about internal
    mechanism, only the final answer. See `testsearch.c`'s comment
    where `TestNullMoveZugzwang` used to be for the full writeup.

**New infrastructure that came out of chasing the above (search.c
touches none of this, but it's now how any future EPD-based search
test should be added):**

- **`epd <file>` engine command** (`EpdCommand`, script.c) parses and
  runs a raw EPD suite file directly -- no perl preprocessing step
  first. Backed by `ParseEpdRecord` (fen.c), real per-record EPD
  parsing (4 position fields, optional FEN-style halfmove/fullmove
  counters, then `;`-separated `bm`/`am`/`id` opcodes).
- **Found and fixed while building this**: `tests/convert_epd.pl` (the
  old perl-based EPD-to-script converter) had a real, silent-
  corruption bug -- its `bm`/`am` regex used `[^b]+` as the filler
  between the board and the opcode, which breaks on *every*
  Black-to-move record (the side-to-move field is literally the
  character `b`), and since `$fen`/`$id`/`$solution` were unscoped
  globals rather than per-line locals, a failed match silently carried
  the *previous* record's values forward instead of erroring.
  Confirmed live: feeding it a 5-position batch with one Black-to-move
  entry produced a byte-for-byte duplicate of the previous record for
  that one line, no diagnostic at all. The script is left in place
  (marked superseded in its own header) in case something still shells
  out to it, but new suites should use `epd` instead.
  Cross-checked the new parser against the old pipeline on
  `ecm_ringers`: `script` (old `.ep_` format) and `epd` (new
  `tests/ecm_ringers.epd`, produced by `ep_to_epd.pl`, see next item)
  produced byte-identical node counts and solve stats.
- **`tests/ep_to_epd.pl`**: one-off reverse converter, turns the
  existing `setboard`/`solution`/`avoid`/`id`/`go` script-format
  suites back into real EPD now that `epd` can read it directly. Run
  against every genuine tactical suite in `tests/` (not `bench.ep_` or
  `trial.ep_`, which use `sd`/`st`/`easy`/`quit` and aren't EPD
  underneath) -- produced `.epd` siblings for `arasan`,
  `ecm_confident_quick`, `ecm_confident_sample2`, `ecm_confident`,
  `ecm_hard_quick`, `ecm_hard`, `ecm_late`, `ecm_quick`, `ecm_ringers`,
  `ecm-gcp`, `ecm-hard`, `ecm`, `ecm016_only`, `LCT2`, `mes400`,
  `newiq`, `quick`, `swiss`, `wac`. `ecm.epd` came out to 879 records,
  matching the `606/879`/`622/879` solve-count baselines already
  quoted elsewhere in CLAUDE.md.

### Eval

- Automate `TestEvalWithSymmetry` into a real fuzz loop (N random
  symmetric positions, assert `Eval() == 0` for all, as part of the
  standard self-test run) instead of the current single-shot
  manual-only invocation.
- **Adversarial/degenerate-position term-bounds test**: 0-pawn
  positions, 9-queen positions, fully-open-board positions -- extend
  `GenerateRandomLegalPosition` or hand-build cases, assert no eval
  term produces an out-of-range score (reuse the existing
  `EvalTrace`/`EVAL_DUMP` term-tracing machinery for this).
- Audit for any **incrementally-maintained eval state** (pawn hash,
  king-safety cache) and add a from-scratch-recompute-vs-incremental
  consistency check if any exists -- classic "works until the hash
  collides" bug class.
- **Eval speed regression test**: no benchmark currently exists for
  eval itself despite it being the named NPS bottleneck vs. Crafty
  (see CLAUDE.md). Same interleaved-comparison shape as the existing
  `TestGenerate*Speed` benchmarks, broken down by game phase.

### Generate (best-covered area, but gaps remain)

- Perft (`TestMoveGenerator`) only checks to **ply 4** even though
  `uLeaves[]` already has reference values to ply 6/7 sitting unused --
  extend to a `--perft-deep` slow path, and add perft-**divide**
  (per-root-move leaf breakdown) to localize failures to a specific
  move instead of just tripping a panic.
- Add perft positions specifically chosen to stress **en passant,
  underpromotion, and castling-through-check** individually (the
  current 5 FENs give general coverage but weren't chosen for this).
- `TestGenerateAllMovesSpeed` and the per-piece-type speed tests are
  **pure microbenchmarks with no correctness assertion** -- they print
  cycles and never diff the move *sets* produced by the mailbox vs.
  bitboard generators being compared. Add a sorted-move-list-equality
  assertion alongside each (both generators already run back-to-back
  in the same loop, so this is nearly free) -- otherwise a bitboard
  generator silently missing a move class just prints a suspiciously
  fast number and passes.
- `GetMoveSee`'s new non-pawn-capture SEE path (c0ac3d7) has no
  dedicated test yet -- covered by the SEE oracle fix above once live.

### Move/Position plumbing

- Reasonably solid already (`TestMakeUnmakeMove`, `TestExposesCheck`,
  `TestIsAttacked`). Add a **long-game fuzz test**: play N random games
  of ~200 plies each (not just 1-ply-at-a-time exercises), asserting
  `VerifyPositionConsistency` and `ComputeSig`-from-scratch agreement
  with the incrementally-maintained signature at every ply. Confirm
  `VerifyPositionConsistency` is actually compiled live in the
  `TEST`/`DEBUG` build, not just present in source.

### Hash (currently zero correctness coverage)

New test needed, not just a wiring fix:
- Store/probe roundtrip correctness.
- Replacement-scheme behavior under forced collisions (fill table past
  capacity with distinct signatures, confirm replacement policy
  behaves as designed, not just "doesn't crash").
- `ADJUST_MATE_IN_N` ply-distance adjustment on `HASH_FLAG_EXACT`
  entries specifically -- the exact mechanism CLAUDE.md flags as
  audited-by-hand but not test-covered after this session's `ASSERT`
  removals in `QSearch()`.

### Recognizer

- Wire in `TestRecognExhaustiveKNKP` (see dead-infra list above).
  Consider the same exhaustive-enumeration treatment for any other
  small recognized endgame class in `recogn.c` (KPK, KRK) if one
  exists without a test.

## Perf testing

Current coverage is narrow: five isolated per-piece-type
mailbox-vs-bitboard microbenchmarks (`TestGenerate*Speed`), nothing for
search, eval, or hash.

- **Eval NPS/cycles-per-call benchmark**, same interleaved-comparison
  shape as the existing generate benchmarks, by game phase -- eval is
  the named bottleneck (CLAUDE.md's 1.5M vs 7.5M nps gap vs. Crafty)
  and has no dedicated speed test today.
- **Whole-search NPS benchmark**: `bench.c` already exists for this --
  confirm whether it's part of the automated `TEST` self-test pass or
  purely a manual `bench` command; if manual-only, wire a
  fixed-node-budget version into the automated sequence so a
  regression surfaces without someone remembering to run it by hand.
- **Hash-probe-under-load benchmark**: cycles/probe at low occupancy
  vs. near-full table -- replacement-scheme cost can regress silently
  today with nothing to measure it.
- **Stored baselines with pass/fail thresholds**, not print-only
  output: today every perf test prints cycles/call and leaves
  interpretation to a human reading the log. Store a last-known-good
  baseline (same idea as `head_reference/` for solve-counts) and fail
  loud if a call site regresses past some tolerance.

## Suggested priority order

1. Un-bitrot the three dead tests (SEE oracle,
   `TestEvalWithKnownPositions`, `TestRecognExhaustiveKNKP`) --
   near-zero cost, immediate coverage recovery.
2. Hash-table correctness test (store/probe/replace/mate-adjust) --
   currently the single biggest correctness blind spot.
3. Search determinism + MP-vs-single-thread agreement test -- targets
   the exact class of bug most likely to hide in threading/hash/
   extension interactions that per-function unit tests can't see.
4. Extend perft to deeper plies + divide output; add move-set-equality
   assertions to the existing generate speed benchmarks (turns
   dead-weight microbenchmarks into correctness tests for free).
5. Eval: automate the symmetry fuzz test (loop N, not 1) and add
   adversarial/degenerate-position term-bounds checks.
6. Round out perf testing per the list above, with stored baselines
   and pass/fail thresholds instead of print-only output.

## "Fuzz" testing

**Implemented: `fuzz_test.sh [binary] [depth] [batch_size] [cpus]`**
(defaults: `./_typhoon`, `sd 6`, 200 positions/batch, `--cpus 2`). Loops
forever: each iteration draws a fresh random batch of `setboard`/`go`
records from the full 400,000-line / 200k-position
`tests/cross_sectional_200k.ep_` pool, runs them through a `DEBUG=1`
build under gdb, and scans the gdb output for a crash/assert signature
before starting the next batch. Stops (exit 1) and prints the offending
batch + engine log paths as soon as one is found; otherwise runs
indefinitely (`^C` to stop). Logs land under `/usr/local/tmp/typhoon/fuzz/`
(one sampled `.ep_` batch + one `.gdb.txt` + one engine `--logfile` per
iteration).

**Two build/invocation bugs found and fixed getting this working, neither
a sandbox limitation despite first appearances:**

1. **The DEBUG binary must be built with `MP=1` as well as `DEBUG=1`**
   (`gmake clean; gmake DEBUG=1 MP=1`) -- `--cpus` is compiled in only
   under `#ifdef MP` (`main.c:311`). A `DEBUG=1`-only build silently
   treats `--cpus` and its numeric argument as two separate unrecognized
   arguments (`main.c:400`'s "Error (unknown argument)" path) and falls
   back to the 1-cpu default with no hard failure -- easy to miss, and it
   also means `split.c`'s parallel-search code never actually links in,
   defeating the point of passing `--cpus 2` at all.
2. **`gdb -batch -ex "handle SIGTRAP nostop noprint pass" -ex run ...`
   breaks thread control against this MP/pthread build specifically**,
   producing `ptrace (PT_SUSPEND): Operation not permitted` followed by
   `Child process unexpectedly missing: No child processes` -- looked
   exactly like a sandboxed environment blocking ptrace outright (and was
   initially misdiagnosed as that), but reproduced identically outside
   any harness/sandbox, and disappeared entirely once that one `-ex` line
   was dropped. `fuzz_test.sh` omits it; a plain `run`/`bt full`/
   `info registers` gdb batch invocation debugs the MP binary fine,
   including catching a genuine `int3` breakpoint from a failed `ASSERT`.

Not yet done: no crash has been found by a run so far, so the
crash-detection path (the `grep -qE` in `fuzz_test.sh`) is exercised by
its own logic reading correctly, not yet by a real hit. Worth periodically
running this for stretches (it's unbounded/`^C`-driven, unlike the other
gates) rather than treating it as a one-time pass/fail check like
`debug_smoke_test.sh`. See also: `./debug_smoke_test.sh` (fast, curated
sample, single pass/fail) for the pre-commit-sized version of the same
idea.