Skip to content

Add cli.py UI tests, mark slow PGN tests, fix promotion filter - #2

Merged
BrandonZacharie merged 4 commits into
mainfrom
claude/review-chess-py-wktEg
May 22, 2026
Merged

Add cli.py UI tests, mark slow PGN tests, fix promotion filter#2
BrandonZacharie merged 4 commits into
mainfrom
claude/review-chess-py-wktEg

Conversation

@BrandonZacharie

@BrandonZacharie BrandonZacharie commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

Three pieces of work the second-pass review surfaced:

  1. UI tests for cli.py — the game-rendering layer that handles board draw, input modes, and the move/promotion loop had no coverage. Same MagicMock-the-curses-Window pattern as test_chess_ui.py; the suite includes a real end-to-end promotion run that drives cli.main from getch through game.promote.
  2. Slow-test marker — tests/Carlsen.pgn (107k lines) and tests/Nakamura.pgn (140k lines) made daily pytest runs take 7+ minutes. Tagged as pytest.mark.slow, skipped by default, still exercised on CI.
  3. Promotion-filter fix — found via the new UI tests and confirmed end-to-end: get_input filtered every keystroke through BOARD_CELL_ORDS, which dropped Q, R, and N from SELECT_PROM mode. Players could only promote to a Bishop.

Commits

  • b738330 — UI tests for cli.py. 29 baseline tests covering draw_head, draw_input_prompt, draw_input_err, draw_input_cursor, draw_input, draw_notes, get_input, draw_board / draw_ilog / draw_elog, and the main() loop (ESC exit, one full move round-trip, ValueError and IllegalMoveError both forwarded to draw_input_err). Pinned the newly-noticed promotion bug as a strict-xfail in the unit-level get_input test.
  • daacdee — Mark heavy PGN tests as slow. Registered a slow marker in pyproject.toml, added addopts = "-m 'not slow'" so plain pytest skips them, added a make test-all target, and updated .github/workflows/python-app.yml to use --override-ini='addopts=' so CI still runs the full sweep. Day-to-day pytest now finishes in ~1.1 s instead of ~7 minutes.
  • 5a294e5 — End-to-end promotion integration test. Drives cli.main through a real 5-move promoting capture (1. e4 d5 2. exd5 e6 3. dxe6 Bc5 4. exf7+ Kf8 then F7→G8) and parametrizes over B / Q / R / N. Before the fix, only B cleared the engine's promotable cell; Q / R / N were pinned strict-xfail.
  • 15a9011 — Fix the SELECT_PROM filter. Introduce PROMOTION_PIECE_ORDS = ord("QqRrBbNn") and gate get_input's inner loop on that set when mode is InputMode.SELECT_PROM. Other modes are unchanged. Removed the four strict-xfail markers; added test_select_prom_rejects_board_cell_letters so a future regression that lets arbitrary board letters into SELECT_PROM also gets caught.

Why the engine tests didn't catch the promotion bug

The engine's promote() and parse_pgn_move() are perfectly fine — both engine tests and the Carlsen/Nakamura PGN parses call them directly with the piece type, never through a keystroke. The bug lives entirely in the UI input filter, which had no tests at all until this PR. The new end-to-end test in 5a294e5 is the first thing in the suite that walks the full getch → game.promote path, so any future regression in either layer gets caught.

Test plan

  • make test — fast suite (151 passed, 2 deselected) in ~1 s.
  • make test-all — full sweep including Carlsen/Nakamura; matches CI behavior.
  • Manual: open a game, push the f-pawn to promotion, press Q — now produces a Queen.
  • Manual: same scenario with R and N — produces Rook and Knight respectively.
  • Manual: regression check on B — still produces a Bishop.

https://claude.ai/code/session_01RnQvxtXfn9JFTN8jpNyriL

Generated by Claude Code

claude added 4 commits May 21, 2026 22:39
@claude
Add UI tests for cli.py rendering and game loop
b738330 
Mirrors the test_chess_ui.py mocking style: the curses Window and
the module-level curs_set helper are replaced with MagicMocks so the
draw_* helpers, get_input, and the main() state machine can be
exercised without a terminal.

Coverage:
- draw_head / draw_input_prompt / draw_input_err
- draw_input_cursor / draw_input echo offsets
- draw_notes (current turn and SELECT_PROM's _last_turn branch)
- get_input filtering, count-per-mode, ESC -> KeyboardInterrupt
- draw_board dispatch to draw_ilog / draw_elog
- draw_ilog / draw_elog against a real Game with a couple of moves
- main() loop: ESC exit, one full move round-trip, ValueError and
  IllegalMoveError both forwarded to draw_input_err

Also pins a newly-noticed bug as strict-xfail: get_input's
BOARD_CELL_ORDS filter blocks every promotion piece letter except
'B' from reaching SELECT_PROM, so players cannot promote to Queen,
Rook, Knight, or King today.
@claude
Mark the heavy PGN tests as 'slow' and skip them by default
daacdee 
The Carlsen.pgn (107k lines) and Nakamura.pgn (140k lines) cases of
test_load_pgn_file took several minutes each, so a plain `pytest` ran
for 7+ minutes during day-to-day work. The two small fixtures
(test1.pgn, test2.pgn) cover the parsing logic on their own.

- Register a `slow` marker and set addopts = "-m 'not slow'" in
  pyproject.toml so `pytest` and `make test` deselect the slow cases
  by default. Daily runs now finish in ~1 s.
- Tag the Carlsen and Nakamura parametrize entries with mark.slow.
- Add a `make test-all` target and update the CI workflow to use
  --override-ini='addopts=' so the full sweep still runs on every
  push / PR.
@claude
Add end-to-end promotion test driving cli.main()
5a294e5 
Drives the full game loop through a real 5-move promoting capture
(1. e4 d5 2. exd5 e6 3. dxe6 Bc5 4. exf7+ Kf8 then F7->G8) and
attempts each of the four legal promotion letters in turn. After
each run we check whether the engine's promotable cell has cleared.

Empirically demonstrates the SELECT_PROM filter bug end-to-end:
'B' promotes, while 'Q' / 'R' / 'N' are silently dropped by
get_input's BOARD_CELL_ORDS gate and the pawn stays unpromoted.
The latter three are pinned as strict-xfail.
@claude
Fix SELECT_PROM filter so all four promotion letters reach main()
15a9011 
get_input filtered every keystroke through BOARD_CELL_ORDS (the chess
board's row/column labels), but those characters and the promotion
piece letters only overlap on 'B'. The result: in SELECT_PROM mode
the input loop silently dropped 'Q', 'R', and 'N', leaving the player
unable to promote to anything other than Bishop. Empirically verified
by driving cli.main() through a real 5-move promotion sequence
(1. e4 d5 2. exd5 e6 3. dxe6 Bc5 4. exf7+ Kf8 f7xg8=X) for each
piece letter — only Bishop completed the promotion before this fix.

Introduce PROMOTION_PIECE_ORDS ("QqRrBbNn") and gate the inner loop
on whichever set matches the current mode. Other modes are unchanged.

The three integration-level strict-xfail markers (Queen / Rook /
Knight promotion) and the unit-level one (test_select_prom_accepts_
queen_rook_knight) flip green; comments updated accordingly. A new
test_select_prom_rejects_board_cell_letters case guards against a
future regression that allows arbitrary board letters in SELECT_PROM.
@BrandonZacharie
BrandonZacharie merged commit a1f7363 into main May 22, 2026
3 checks passed
This was referenced May 23, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BrandonZacharie @claude