Restore 100% coverage and fix cli.main's mutable-default Game() - #3
Merged
Conversation
Backfills the test surface the new UI suite left uncovered so the fast pytest run (`pytest -m 'not slow'`) finishes with no missed lines. Three small additions in tests/test_game.py also pick up the engine paths previously only exercised by the long Carlsen / Nakamura PGN parses, so coverage is now 100% with or without the slow tests. Source-side trims: - `if __name__ == "__main__":` blocks on chess.py and cli.py marked `# pragma: no cover` — the wrapper(...) call inside is never reachable from the test harness. - cli.main's SELECT_PROM KeyError branch marked pragma — after the PROMOTION_PIECE_ORDS gate, every letter that reaches the lookup is guaranteed to be in PIECE_NAME_TYPE_MAP. The except is left in as defensive code with a one-line note. Test-side additions: - Menu rendering: title row, items with the optional A_DIM/A_BOLD third-tuple attribute, and the previously-uncovered PPAGE / NPAGE / HOME / END navigation keys. - Chess sub-menu drivers: draw_save_menu / draw_load_menu / draw_cfg_menu / draw_cfg_log_style_menu (both the choose-a-style and ESC-out branches) / draw_about. - Chess save/load entrypoints: save_pgn stub, save_json (both the failed-handler-then-ESC path and the successful-handler-then- KeyboardInterrupt path), load_json, load_pgn. - Chess._save_json full matrix: no active game, success, FileNotFound / generic exception branches. - Chess._load_json: missing file, generic error, and a real round- trip via Game.save → _load_json. - Chess._load_pgn: empty PGN file branch and the generic-exception branch. - Chess._fileprompt edit keys: DOWN/UP, RIGHT, VT (Ctrl+K), HOME, END, DC (Delete). - Removed an unused `_run_fileprompt` helper that was the last uncovered region in tests/test_chess_ui.py. - cli.py: synthetic ilog/elog tests that drive the 4-column overflow break path; `TestRun.test_run_delegates_to_curses_wrapper_with_main` covers the run() entrypoint. - tests/test_game.py: direct parse_pgn_move tests for queen-side castling (O-O-O) and the `=Q` promotion suffix, plus a small synthetic PGN exercising PGNFile.game()'s auto-promote callback.
`def main(window, game: Game = Game(), ...)` evaluated the Game() default at module-import time, so every caller that omitted the argument shared a single mutable Game. Today every production call site (Chess.play) passes `game=` explicitly, so the bug is dormant — but the moment a second default-args invocation lands (or someone calls run() in the same process twice), state from one game would leak into the next. Change the default to None and construct a fresh Game inside the function body. Adds two tests: - a behavioral regression test that drives main() twice without passing `game=` and asserts a move on the second call isn't rejected as illegal (which it would be if state had leaked). - a structural guard that asserts inspect.signature(main).parameters ["game"].default is None, so a future refactor that re-introduces `Game()` as the default trips the test immediately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes that sit on top of what PR #2 merged:
chess.pyandcli.pythey didn't reach, so coveralls dropped below 100%. This brings every file back to full coverage on both the fast suite (make test) and the full sweep (make test-all).Game()argument incli.main—def main(window, game: Game = Game(), ...)evaluated theGame()default at module-import time, so every caller that omittedgame=shared a single mutable engine. Today every production call site passesgame=explicitly, so the bug is dormant — but fixing it now means we don't have to think about it later.Commits
930ed8f— Restore 100% coverage across chess.py, cli.py, and game/game.py.# pragma: no coveron theif __name__ == "__main__":guards inchess.pyandcli.py(untestable wrapper invocations), plus the unreachableKeyErrorarm incli.main'sSELECT_PROMblock (thePROMOTION_PIECE_ORDSfilter from PR Add cli.py UI tests, mark slow PGN tests, fix promotion filter #2 guarantees the lookup succeeds; theexceptis left in as defensive code with a one-line note).A_DIM/A_BOLDthird-tuple attribute, every previously-uncovered navigation key (PPAGE/NPAGE/HOME/END), everyChess.draw_*sub-menu, the full save/load entrypoints (save_pgnstub,save_jsonhappy + failure paths,load_json,load_pgn,_save_jsonmatrix,_load_pgnempty-file and generic-error branches,_load_jsonround-trip viaGame.save), and the missing_filepromptedit keys (DOWN/UP, RIGHT, VT, HOME, END, DC). Plus a synthetic ilog/elog overflow test that hits the 4-column break path incli.py, arun()entrypoint test, and three direct unit tests intest_game.pythat pick up queen-side castling parse, the=Qpromotion suffix parse, and a small synthetic PGN exercisingPGNFile.game()'s auto-promote callback. Removed one unused_run_fileprompthelper.8f72cf3— Fix mutable-default Game() argument in cli.main. Default flipped toNonewithgame = Game()inside the body. Two regression tests:main()twice withoutgame=and asserts the second call's first move isn't rejected as illegal (which it would be if state had leaked from the first call).inspect.signature(main).parameters["game"].default is None— a future refactor that re-introducesGame()as the default trips this immediately.Test plan
make test— fast suite (191 passed, 2 deselected) in ~1.6 s.make test-all— full sweep including Carlsen/Nakamura; matches CI behavior and reaches 100% coverage (2884 statements, 0 missing).coverage reportaftermake testshows TOTAL 100%.https://claude.ai/code/session_01RnQvxtXfn9JFTN8jpNyriL
Generated by Claude Code