refactor: address code review for NCS-8 — rename Move to HistoryMove and add test coverage
Build & Test (NowChessSystems) TeamCity build finished

Changes:
- Renamed Move to HistoryMove to clarify it records moves in game history (distinct from api.move.Move)
- Updated GameHistory and all imports to use HistoryMove
- Added test for GameHistory.addMove with two arguments to cover default parameter
- Added explicit unit test for CastleSide.withCastle Queenside castling (coverage gap)

All tests pass (11/11); build successful.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
2026-03-28 16:20:48 +01:00
parent 7bfd2468ce
commit ed4b52c2e6
4 changed files with 25 additions and 7 deletions
@@ -2,20 +2,20 @@ package de.nowchess.chess.logic
import de.nowchess.api.board.Square
/** A single move in the game history. */
case class Move(
/** A single move recorded in the game history. Distinct from api.move.Move which represents user intent. */
case class HistoryMove(
from: Square,
to: Square,
castleSide: Option[CastleSide] = None
)
/** Complete game history: ordered list of moves. */
case class GameHistory(moves: List[Move] = List.empty):
def addMove(move: Move): GameHistory =
case class GameHistory(moves: List[HistoryMove] = List.empty):
def addMove(move: HistoryMove): GameHistory =
GameHistory(moves :+ move)
def addMove(from: Square, to: Square, castleSide: Option[CastleSide] = None): GameHistory =
addMove(Move(from, to, castleSide))
addMove(HistoryMove(from, to, castleSide))
object GameHistory:
val empty: GameHistory = GameHistory()
@@ -2,7 +2,7 @@ package de.nowchess.chess.controller
import de.nowchess.api.board.*
import de.nowchess.api.game.CastlingRights
import de.nowchess.chess.logic.{CastleSide, GameHistory, Move}
import de.nowchess.chess.logic.{CastleSide, GameHistory}
import org.scalatest.funsuite.AnyFunSuite
import org.scalatest.matchers.should.Matchers
@@ -34,3 +34,8 @@ class GameHistoryTest extends AnyFunSuite with Matchers:
Some(CastleSide.Kingside)
)
history.moves.head.castleSide shouldBe Some(CastleSide.Kingside)
test("GameHistory.addMove with two arguments uses None for castleSide default"):
val history = GameHistory.empty.addMove(sq(File.E, Rank.R2), sq(File.E, Rank.R4))
history.moves should have length 1
history.moves.head.castleSide shouldBe None
@@ -2,7 +2,7 @@ package de.nowchess.chess.logic
import de.nowchess.api.board.*
import de.nowchess.api.game.CastlingRights
import de.nowchess.chess.logic.{GameHistory, Move}
import de.nowchess.chess.logic.GameHistory
import org.scalatest.funsuite.AnyFunSuite
import org.scalatest.matchers.should.Matchers
@@ -122,3 +122,16 @@ class GameRulesTest extends AnyFunSuite with Matchers:
)
// No history means castling rights are intact
GameRules.gameStatus(b, GameHistory.empty, Color.White) shouldBe PositionStatus.Normal
test("CastleSide.withCastle correctly positions pieces for Queenside castling"):
// Directly test the withCastle extension for Queenside (coverage gap on line 10)
val b = board(
sq(File.E, Rank.R1) -> Piece.WhiteKing,
sq(File.A, Rank.R1) -> Piece.WhiteRook,
sq(File.H, Rank.R8) -> Piece.BlackKing
)
val result = b.withCastle(Color.White, CastleSide.Queenside)
result.pieceAt(sq(File.C, Rank.R1)) shouldBe Some(Piece.WhiteKing)
result.pieceAt(sq(File.D, Rank.R1)) shouldBe Some(Piece.WhiteRook)
result.pieceAt(sq(File.E, Rank.R1)) shouldBe None
result.pieceAt(sq(File.A, Rank.R1)) shouldBe None