Skip to content

test(sudoku): add 24 tests for core/solvers.py (cycle 80)#1906

Merged
jsboige merged 1 commit into
mainfrom
test/sudoku-solvers
May 30, 2026
Merged

test(sudoku): add 24 tests for core/solvers.py (cycle 80)#1906
jsboige merged 1 commit into
mainfrom
test/sudoku-solvers

Conversation

@jsboige
Copy link
Copy Markdown
Owner

@jsboige jsboige commented May 30, 2026

Summary

Adds 24 unit tests for scripts/sudoku/core/solvers.py — Norvig-style constraint propagation solver.

Test classes (24 tests)

Class Tests Target
TestIsValidPuzzle 9 Row/col/box duplicate detection, empty grid, partial grid
TestSolveSudoku 11 Easy/hard/empty puzzles, unsolvable, preserves givens, return type
TestCrossInvariants 4 Validity consistency, deterministic results

Coverage

  • solve_sudoku(puzzle_81): full solver (constraint propagation + MRV backtracking)
  • is_valid_puzzle(puzzle_81): duplicate check in rows/cols/boxes
  • Edge cases: empty puzzle, complete grid, single empty cell, unsolvable (row/col/box conflicts)

Verification

24 passed in 0.20s
Full suite: 1422 passed, 0 regression

Context

  • po-2024 test-coverage lane (T1: sudoku core)
  • LIVE: 2 callers (eval_notebook.py, cpu_diagnostic.py)
  • Pure computation: no I/O, no GPU, no network
  • Cumulative: 524 + 24 = 548 tests across cycles 69-80

Copy link
Copy Markdown
Collaborator

@clusterManager-Myia clusterManager-Myia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[NanoClaw]

Well-structured test suite. 24 tests across 3 classes with clear separation of concerns, good edge-case coverage, and correct assertion patterns. A few observations:

Nit (non-blocking):

  • test_duplicate_in_row (L84-86): The comment on L85 says "row 0 already has 5 at index 0 — now duplicate at [0,0]" but the p[0] = 5 assignment is a no-op (the value was already 5). The comment on L86 correctly self-corrects, but the first comment is misleading. Consider removing L85 or simplifying to just: p[1] = 5 # duplicate 5 in row 0 with p[0].

Minor observations:

  • sys.path.insert for import resolution (L17-19) works but is fragile relative to the execution context. If tests are ever run from a different working directory, Path(__file__).resolve() should still hold, so this is acceptable for a scripts-local test — just noting it as something a CI runner could trip on.
  • TestCrossInvariants.test_solve_produces_valid_puzzle is functionally identical to TestSolveSudoku.test_easy_puzzle in terms of setup and assertions (same puzzle, same validity check). It adds semantic value as a cross-invariant test, but the duplication is worth being aware of if the easy puzzle string ever changes.
  • pytest is imported but never used (no pytest.mark, pytest.raises, or fixtures). Harmless, but import pytest on L7 could be removed if no parametrized or fixture-based tests are planned.

Verified:

  • All puzzle grids validated: solved grid is a correct complete Sudoku, easy/hard puzzle givens have no duplicates and easy givens match the solved grid.
  • No security concerns (no secrets, credentials, or sensitive patterns).
  • Test count matches PR title: 24 test_* methods.
  • Assertion patterns are sound: is True/is None/is not None used correctly, numpy array equality and masking used appropriately.

Copy link
Copy Markdown

@myia-ai-01 myia-ai-01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ai-01 forensic APPROVE] Tests-only PR (+296/-0, 1 fichier). Cible VERIFIEE LIVE : scripts/sudoku/core/solvers.py existe (3697 B), exporte solve_sudoku (L86) + is_valid_puzzle (L108) — les 2 symboles testes — et a un caller reel (scripts/sudoku/cpu_diagnostic.py:29). scripts/sudoku/ cataloge dans docs/scripts-reference.md. Pas de dead-code (lecon #1876/#1885). Aucune cellule notebook touchee.

@jsboige jsboige merged commit c7362d3 into main May 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants