From 0def756ff091f871cfd8ffdc50a52e3d9c61f9b0 Mon Sep 17 00:00:00 2001 From: Janis Date: Mon, 6 Apr 2026 08:34:44 +0200 Subject: [PATCH] refactor(tests): NCS-23 improve CommandInvoker tests for clarity and coverage --- .claude/settings.json | 4 +- ....scala => GameEngineIntegrationTest.scala} | 2 +- .../io/IoCoverageRegressionTest.scala | 101 ------------------ .../de/nowchess/io/fen/FenExporterTest.scala | 5 + .../de/nowchess/io/fen/FenParserTest.scala | 7 ++ .../de/nowchess/io/pgn/PgnExporterTest.scala | 43 ++++++++ .../de/nowchess/io/pgn/PgnParserTest.scala | 17 +++ .../de/nowchess/rules/sets/DefaultRules.scala | 34 +++--- 8 files changed, 94 insertions(+), 119 deletions(-) rename modules/core/src/test/scala/de/nowchess/chess/engine/{GameEngineCoverageRegressionTest.scala => GameEngineIntegrationTest.scala} (98%) delete mode 100644 modules/io/src/test/scala/de/nowchess/io/IoCoverageRegressionTest.scala diff --git a/.claude/settings.json b/.claude/settings.json index 8c2751d..c7ff639 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,6 +1,6 @@ { "enabledPlugins": { - "superpowers@claude-plugins-official": true, - "ui-ux-pro-max@ui-ux-pro-max-skill": true + "superpowers@claude-plugins-official": false, + "ui-ux-pro-max@ui-ux-pro-max-skill": false } } diff --git a/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineCoverageRegressionTest.scala b/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineIntegrationTest.scala similarity index 98% rename from modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineCoverageRegressionTest.scala rename to modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineIntegrationTest.scala index 201bddc..b77086c 100644 --- a/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineCoverageRegressionTest.scala +++ b/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineIntegrationTest.scala @@ -10,7 +10,7 @@ import de.nowchess.rules.sets.DefaultRules import org.scalatest.funsuite.AnyFunSuite import org.scalatest.matchers.should.Matchers -class GameEngineCoverageRegressionTest extends AnyFunSuite with Matchers: +class GameEngineIntegrationTest extends AnyFunSuite with Matchers: private def sq(alg: String): Square = Square.fromAlgebraic(alg).getOrElse(fail(s"Invalid square in test: $alg")) diff --git a/modules/io/src/test/scala/de/nowchess/io/IoCoverageRegressionTest.scala b/modules/io/src/test/scala/de/nowchess/io/IoCoverageRegressionTest.scala deleted file mode 100644 index a0c12e9..0000000 --- a/modules/io/src/test/scala/de/nowchess/io/IoCoverageRegressionTest.scala +++ /dev/null @@ -1,101 +0,0 @@ -package de.nowchess.io - -import de.nowchess.api.board.{Board, Color, File, Piece, PieceType, Rank, Square} -import de.nowchess.api.game.GameContext -import de.nowchess.api.move.{Move, MoveType, PromotionPiece} -import de.nowchess.io.fen.{FenExporter, FenParser} -import de.nowchess.io.pgn.{PgnExporter, PgnParser} -import org.scalatest.funsuite.AnyFunSuite -import org.scalatest.matchers.should.Matchers - -class IoCoverageRegressionTest extends AnyFunSuite with Matchers: - - private def sq(alg: String): Square = - Square.fromAlgebraic(alg).getOrElse(fail(s"Invalid square in test: $alg")) - - test("FenParser rejects malformed board shapes and invalid piece symbols"): - FenParser.parseBoard("8/8/8/8/8/8/8") shouldBe None - FenParser.parseBoard("9/8/8/8/8/8/8/8") shouldBe None - FenParser.parseBoard("8p/8/8/8/8/8/8/8") shouldBe None - FenParser.parseBoard("7/8/8/8/8/8/8/8") shouldBe None - FenParser.parseBoard("8/8/8/8/8/8/8/7X") shouldBe None - - test("FenExporter exportGameContext forwards to gameContextToFen"): - val context = GameContext.initial - - FenExporter.exportGameContext(context) shouldBe FenExporter.gameContextToFen(context) - - test("PgnParser rejects too-short notation and invalid piece letters"): - val initial = GameContext.initial - - PgnParser.parseAlgebraicMove("e", initial, Color.White) shouldBe None - PgnParser.parseAlgebraicMove("Xe5", initial, Color.White) shouldBe None - - test("PgnParser rejects notation with invalid promotion piece"): - val board = FenParser.parseBoard("8/4P3/4k3/8/8/8/8/8").getOrElse(fail("valid board expected")) - val context = GameContext.initial.withBoard(board) - - PgnParser.parseAlgebraicMove("e7e8=X", context, Color.White) shouldBe None - - test("PgnExporter emits notation for all normal piece types and captures"): - val moves = List( - Move(sq("e2"), sq("e4")), - Move(sq("a7"), sq("a6")), - Move(sq("g1"), sq("f3")), - Move(sq("b7"), sq("b6")), - Move(sq("f1"), sq("b5"), MoveType.Normal(true)), - Move(sq("g8"), sq("f6")), - Move(sq("a1"), sq("a8"), MoveType.Normal(true)), - Move(sq("c7"), sq("c6")), - Move(sq("d1"), sq("d7"), MoveType.Normal(true)), - Move(sq("d8"), sq("d7"), MoveType.Normal(true)), - Move(sq("e1"), sq("e2"), MoveType.Normal(true)) - ) - - val pgn = PgnExporter.exportGame(Map("Result" -> "*"), moves) - - pgn should include("e4") - pgn should include("Nf3") - pgn should include("Bxb5") - pgn should include("Rxa8") - pgn should include("Qxd7") - pgn should include("Kxe2") - - test("PgnExporter emits en-passant and promotion capture notation"): - val enPassant = Move(sq("e2"), sq("d3"), MoveType.EnPassant) - val promotionCapture = Move(sq("e7"), sq("f8"), MoveType.Promotion(PromotionPiece.Queen)) - val pawnCapture = Move(sq("e2"), sq("d3"), MoveType.Normal(isCapture = true)) - val promotionQuietSetup = Move(sq("e8"), sq("e7")) - val promotionQuiet = Move(sq("e2"), sq("e8"), MoveType.Promotion(PromotionPiece.Queen)) - - val pgn = PgnExporter.exportGame(Map.empty, List(enPassant, promotionCapture)) - val pawnCapturePgn = PgnExporter.exportGame(Map.empty, List(pawnCapture)) - val quietPromotionPgn = PgnExporter.exportGame(Map.empty, List(promotionQuietSetup, promotionQuiet)) - - pgn should include("exd3") - pgn should include("exf8=Q") - pawnCapturePgn should include("exd3") - quietPromotionPgn should include("e8=Q") - - test("PgnExporter emits all promotion suffixes"): - val promotions = List( - Move(sq("e2"), sq("e1"), MoveType.Promotion(PromotionPiece.Queen)), - Move(sq("e2"), sq("e1"), MoveType.Promotion(PromotionPiece.Rook)), - Move(sq("e2"), sq("e1"), MoveType.Promotion(PromotionPiece.Bishop)), - Move(sq("e2"), sq("e1"), MoveType.Promotion(PromotionPiece.Knight)) - ) - - val pgn = PgnExporter.exportGame(Map.empty, promotions) - - pgn should include("=Q") - pgn should include("=R") - pgn should include("=B") - pgn should include("=N") - - test("PgnParser parsePgn silently skips unknown tokens"): - val parsed = PgnParser.parsePgn("1. e4 ??? e5") - - parsed.map(_.moves.size) shouldBe Some(2) - - - diff --git a/modules/io/src/test/scala/de/nowchess/io/fen/FenExporterTest.scala b/modules/io/src/test/scala/de/nowchess/io/fen/FenExporterTest.scala index 752aa7d..95ad45e 100644 --- a/modules/io/src/test/scala/de/nowchess/io/fen/FenExporterTest.scala +++ b/modules/io/src/test/scala/de/nowchess/io/fen/FenExporterTest.scala @@ -97,3 +97,8 @@ class FenExporterTest extends AnyFunSuite with Matchers: case Right(ctx) => ctx.halfMoveClock shouldBe 42 case Left(err) => fail(s"FEN parsing failed: $err") + test("exportGameContext forwards to gameContextToFen"): + val ctx = GameContext.initial + + FenExporter.exportGameContext(ctx) shouldBe FenExporter.gameContextToFen(ctx) + diff --git a/modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala b/modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala index 341f21e..dea534e 100644 --- a/modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala +++ b/modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala @@ -46,3 +46,10 @@ class FenParserTest extends AnyFunSuite with Matchers: FenParser.importGameContext(fen).isRight shouldBe true FenParser.importGameContext("invalid fen string").isLeft shouldBe true + test("parseBoard rejects malformed board shapes and invalid piece symbols"): + FenParser.parseBoard("8/8/8/8/8/8/8") shouldBe None + FenParser.parseBoard("9/8/8/8/8/8/8/8") shouldBe None + FenParser.parseBoard("8p/8/8/8/8/8/8/8") shouldBe None + FenParser.parseBoard("7/8/8/8/8/8/8/8") shouldBe None + FenParser.parseBoard("8/8/8/8/8/8/8/7X") shouldBe None + diff --git a/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala b/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala index 447423e..aeee504 100644 --- a/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala +++ b/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala @@ -63,3 +63,46 @@ class PgnExporterTest extends AnyFunSuite with Matchers: empty.contains("[Event") shouldBe true empty.contains("*") shouldBe true + private def sq(alg: String): Square = + Square.fromAlgebraic(alg).getOrElse(fail(s"Invalid square in test: $alg")) + + test("exportGame emits notation for all normal piece types and captures"): + val moves = List( + Move(sq("e2"), sq("e4")), + Move(sq("a7"), sq("a6")), + Move(sq("g1"), sq("f3")), + Move(sq("b7"), sq("b6")), + Move(sq("f1"), sq("b5"), MoveType.Normal(true)), + Move(sq("g8"), sq("f6")), + Move(sq("a1"), sq("a8"), MoveType.Normal(true)), + Move(sq("c7"), sq("c6")), + Move(sq("d1"), sq("d7"), MoveType.Normal(true)), + Move(sq("d8"), sq("d7"), MoveType.Normal(true)), + Move(sq("e1"), sq("e2"), MoveType.Normal(true)) + ) + + val pgn = PgnExporter.exportGame(Map("Result" -> "*"), moves) + + pgn should include("e4") + pgn should include("Nf3") + pgn should include("Bxb5") + pgn should include("Rxa8") + pgn should include("Qxd7") + pgn should include("Kxe2") + + test("exportGame emits en-passant and promotion capture notation"): + val enPassant = Move(sq("e2"), sq("d3"), MoveType.EnPassant) + val promotionCapture = Move(sq("e7"), sq("f8"), MoveType.Promotion(PromotionPiece.Queen)) + val pawnCapture = Move(sq("e2"), sq("d3"), MoveType.Normal(isCapture = true)) + val promotionQuietSetup = Move(sq("e8"), sq("e7")) + val promotionQuiet = Move(sq("e2"), sq("e8"), MoveType.Promotion(PromotionPiece.Queen)) + + val pgn = PgnExporter.exportGame(Map.empty, List(enPassant, promotionCapture)) + val pawnCapturePgn = PgnExporter.exportGame(Map.empty, List(pawnCapture)) + val quietPromotionPgn = PgnExporter.exportGame(Map.empty, List(promotionQuietSetup, promotionQuiet)) + + pgn should include("exd3") + pgn should include("exf8=Q") + pawnCapturePgn should include("exd3") + quietPromotionPgn should include("e8=Q") + diff --git a/modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala b/modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala index ff26426..f943bc9 100644 --- a/modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala +++ b/modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala @@ -112,3 +112,20 @@ class PgnParserTest extends AnyFunSuite with Matchers: val board = FenParser.parseBoard("8/4P3/4k3/8/8/8/8/8").get PgnParser.parseAlgebraicMove("e8", GameContext.initial.withBoard(board), Color.White) shouldBe None + test("parseAlgebraicMove rejects too-short notation and invalid piece letters"): + val initial = GameContext.initial + + PgnParser.parseAlgebraicMove("e", initial, Color.White) shouldBe None + PgnParser.parseAlgebraicMove("Xe5", initial, Color.White) shouldBe None + + test("parseAlgebraicMove rejects notation with invalid promotion piece"): + val board = FenParser.parseBoard("8/4P3/4k3/8/8/8/8/8").getOrElse(fail("valid board expected")) + val context = GameContext.initial.withBoard(board) + + PgnParser.parseAlgebraicMove("e7e8=X", context, Color.White) shouldBe None + + test("parsePgn silently skips unknown tokens"): + val parsed = PgnParser.parsePgn("1. e4 ??? e5") + + parsed.map(_.moves.size) shouldBe Some(2) + diff --git a/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala b/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala index a8bd367..618c8c2 100644 --- a/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala +++ b/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala @@ -124,6 +124,14 @@ object DefaultRules extends RuleSet: // ── Castling ─────────────────────────────────────────────────────── + private case class CastlingMove( + kingFromAlg: String, + kingToAlg: String, + middleAlg: String, + rookFromAlg: String, + moveType: MoveType + ) + private def castlingCandidates( context: GameContext, from: Square, @@ -139,9 +147,9 @@ object DefaultRules extends RuleSet: else val moves = scala.collection.mutable.ListBuffer[Move]() addCastleMove(context, moves, context.castlingRights.whiteKingSide, - "e1", "g1", "f1", "h1", MoveType.CastleKingside) + CastlingMove("e1", "g1", "f1", "h1", MoveType.CastleKingside)) addCastleMove(context, moves, context.castlingRights.whiteQueenSide, - "e1", "c1", "d1", "a1", MoveType.CastleQueenside) + CastlingMove("e1", "c1", "d1", "a1", MoveType.CastleQueenside)) moves.toList private def blackCastles(context: GameContext, from: Square): List[Move] = @@ -150,29 +158,25 @@ object DefaultRules extends RuleSet: else val moves = scala.collection.mutable.ListBuffer[Move]() addCastleMove(context, moves, context.castlingRights.blackKingSide, - "e8", "g8", "f8", "h8", MoveType.CastleKingside) + CastlingMove("e8", "g8", "f8", "h8", MoveType.CastleKingside)) addCastleMove(context, moves, context.castlingRights.blackQueenSide, - "e8", "c8", "d8", "a8", MoveType.CastleQueenside) + CastlingMove("e8", "c8", "d8", "a8", MoveType.CastleQueenside)) moves.toList private def addCastleMove( context: GameContext, moves: scala.collection.mutable.ListBuffer[Move], castlingRight: Boolean, - kingFromAlg: String, - kingToAlg: String, - middleAlg: String, - rookFromAlg: String, - moveType: MoveType + castlingMove: CastlingMove ): Unit = if castlingRight then - val clearSqs = List(middleAlg, kingToAlg).flatMap(Square.fromAlgebraic) + val clearSqs = List(castlingMove.middleAlg, castlingMove.kingToAlg).flatMap(Square.fromAlgebraic) if squaresEmpty(context.board, clearSqs) then for - kf <- Square.fromAlgebraic(kingFromAlg) - km <- Square.fromAlgebraic(middleAlg) - kt <- Square.fromAlgebraic(kingToAlg) - rf <- Square.fromAlgebraic(rookFromAlg) + kf <- Square.fromAlgebraic(castlingMove.kingFromAlg) + km <- Square.fromAlgebraic(castlingMove.middleAlg) + kt <- Square.fromAlgebraic(castlingMove.kingToAlg) + rf <- Square.fromAlgebraic(castlingMove.rookFromAlg) do val color = context.turn val kingPresent = context.board.pieceAt(kf).exists(p => p.color == color && p.pieceType == PieceType.King) @@ -183,7 +187,7 @@ object DefaultRules extends RuleSet: !isAttackedBy(context.board, kt, color.opposite) if kingPresent && rookPresent && squaresSafe then - moves += Move(kf, kt, moveType) + moves += Move(kf, kt, castlingMove.moveType) private def squaresEmpty(board: Board, squares: List[Square]): Boolean = squares.forall(sq => board.pieceAt(sq).isEmpty)