Improve Game integration test

.pylintrc

@@ -3,3 +3,4 @@ disable = missing-module-docstring, missing-class-docstring, missing-function-do
 
 [SIMILARITIES]
 ignore-imports=yes
+min-similarity-lines=6

azul/game.py

@@ -13,6 +13,7 @@ class Game(GameInterface):
     _boards: List[BoardInterface]
     _observable: NotifyEverybodyInterface
     _player_manager: PlayerManager
+    _finished: bool
 
     class PlayerManager:
         _player_ids: List[int]
@@ -53,14 +54,16 @@ def __init__(self, bag: StateInterface, table_area: TableAreaInterface,
         self._observable = observable
         player_ids = [entry[0] for entry in player_ids_and_boards]
         self._player_manager = Game.PlayerManager(player_ids)
+        self._finished = False
 
     def _state(self) -> str:
         state: Any = {
             "bag": json.loads(self._bag.state()),
             "table area": json.loads(self._table_area.state()),
             "boards": [json.loads(board.state()) for board in self._boards],
             "players": self._player_manager.player_ids,
-            "on turn": self._player_manager.whos_turn_it_is()[1]
+            "on turn": self._player_manager.whos_turn_it_is()[1],
+            "finished": self._finished,
         }
         return json.dumps(state)
 
@@ -69,6 +72,9 @@ def start_game(self) -> None:
         self.observable.notify_everybody(self._state())
 
     def take(self, player_id: int, source_idx: int, idx: int, destination_idx: int) -> bool:
+        if self._finished:
+            return False
+
         # check if player_id is on turn
         player_on_turn_id, board_id = self._player_manager.whos_turn_it_is()
         if player_on_turn_id != player_id:
@@ -82,15 +88,17 @@ def take(self, player_id: int, source_idx: int, idx: int, destination_idx: int)
         self._boards[board_id].put(destination_idx, taken)
         self._observable.notify_everybody(self._state())
         if self._table_area.is_round_end():
-            self._table_area.start_new_round()
-            self._player_manager.start_new_round()
             finish_round_results = [board.finish_round()
                                     for board in self._boards]
-            self._observable.notify_everybody(self._state())
             if FinishRoundResult.GAME_FINISHED in finish_round_results:
+                self._finished = True
                 for board in self._boards:
                     board.end_game()
                 self._observable.notify_everybody(self._state())
+            else:
+                self._table_area.start_new_round()
+                self._player_manager.start_new_round()
+                self._observable.notify_everybody(self._state())
         return True
 
     @property

comments

@@ -68,5 +68,8 @@ I made one test focusing on Bag refilling. I skip Board integration test that sh
 
 Game integration test
 
-I noticed that when all factories have the same tiles, we have an issue with not being able to take the starting player stone. Thus we should allow to take starting player stone from the center. As expected there were quite some issues. Some of easily fixable reasons are not using ABCs and having a lot of uncovered code. There are still plenty of outstanding errors. But I guess this is sufficient to show how it is possible to write tests that are well under control.
+I noticed that when all factories have the same tiles, we have an issue with not being able to take the starting player stone. Thus we should allow to take starting player stone from the center. As expected there were quite some issues. Some of easily fixable reasons are not using ABCs and having a lot of uncovered code. There are still plenty of outstanding errors. But I guess this is sufficient to show how it is possible to write tests that are well under control. ... I decided to finish the integration test so it really uses all classes. Doing this I found an error in the Game class. Refilling factories happened before end of turn on boards was handled. Note how little chance there is to find this issue in a Game class unit test. The best chance is that you look at the code second time and just notice it. But in that case you more or less found the error reviewing the code. After I found the issue, I need to guarantee that the test will find this issue in the future. 
+Linter did not like test length so I added some tools to make the test more compact. I should rewrite the other integration test too, but I will not :).
+
+
 

test/test_integration/test_game.py

@@ -1,9 +1,8 @@
 from __future__ import annotations
 import unittest
 import json
-from typing import Any, List
+from typing import Any, List, Tuple
 from test.utils import FakeShuffler
-from azul.simple_types import RED, BLUE, YELLOW
 from azul.interfaces import ObserverInterface
 from azul.factories import create_game
 
@@ -25,14 +24,6 @@ def setUp(self) -> None:
         self.observer = FakeObserver()
         self.observable.register_observer(self.observer)
 
-        self.shuffler.next_take = [
-            [RED, RED, RED, RED],
-            [RED, RED, RED, RED],
-            [RED, RED, RED, RED],
-            [BLUE, BLUE, BLUE, BLUE],
-            [BLUE, BLUE, BLUE, YELLOW],
-        ]
-
     def bag_state(self) -> Any:
         return json.loads(self.observer.messages[-1])["bag"]["bag"]
 
@@ -48,45 +39,73 @@ def patternline_state(self, i: int, j: int) -> Any:
     def floor_state(self, i: int) -> Any:
         return json.loads(self.observer.messages[-1])["boards"][i-1]["floor"]
 
+    def used_tiles_state(self) -> Any:
+        return json.loads(self.observer.messages[-1])["bag"]["used tiles"]
+
     def get_points(self, i: int) -> Any:
         return json.loads(self.observer.messages[-1])["boards"][i-1]["points"]
 
+    def take_from_factories(self, instructions: List[Tuple[int, int, int, int]]) -> None:
+        for player, source, index, line in instructions:
+            self.assertTrue(self.game.take(player, source, index, line))
+
     def take_from_center(self, player: int, tile: str, destination_index: int) -> bool:
         assert len(tile) == 1
         index: int = self.table_center_state().index(tile)
         return self.game.take(player, 0, index, destination_index)
 
     def test_play(self) -> None:
+        self.shuffler.instructions(["RRRR", "RRRR", "RRRR", "BBBB", "BBBY"])
         self.game.start_game()
         self.assertCountEqual(self.table_center_state(), "S")
         self.assertCountEqual(self.factory_state(1), "RRRR")
         self.assertTrue(self.game.take(9, 1, 1, 1))
         self.assertCountEqual(self.patternline_state(1, 1), "R")
         self.assertCountEqual(self.floor_state(1), "RRR")
-
         self.assertFalse(self.game.take(9, 2, 1, 1))   # not on turn
-
-        self.assertTrue(self.game.take(7, 5, 0, 3))
-        self.assertTrue(self.game.take(9, 3, 0, 1))    # drops
-        self.assertTrue(self.game.take(7, 4, 0, 4))
-        self.assertTrue(self.game.take(9, 2, 0, 1))    # drops
-
-        # next .take call will finish turn
-        self.shuffler.next_take = [
-            [RED, RED, RED, RED],
-            [RED, RED, RED, RED],
-            [BLUE, BLUE, BLUE, BLUE],
-            [BLUE, BLUE, BLUE, BLUE],
-            [BLUE, BLUE, BLUE, BLUE],
-        ]
+        self.take_from_factories(
+            [(7, 5, 0, 3), (9, 3, 0, 1), (7, 4, 0, 4), (9, 2, 0, 1)])
+        self.shuffler.instructions(["RRRR", "RRRR", "BBBB", "BBBB", "BBBB"])
         self.assertTrue(self.take_from_center(7, "Y", 1))
-
+        # turn end
         self.assertEqual(self.get_points(1), 0)
         self.assertEqual(self.get_points(2), 2)
+        self.assertCountEqual(self.used_tiles_state(), "RRRBBRRRRBBBRRRR")
 
-        # drops as we already have BLUE on wallline 4
         self.assertTrue(self.game.take(7, 5, 1, 4))
         self.assertCountEqual(self.floor_state(2), "BBBB")
+        self.take_from_factories(
+            [(9, 4, 0, 4), (7, 1, 0, 4), (9, 2, 0, 4), (7, 3, 0, 5)])
+        self.shuffler.instructions(["BYYY", "YYYY", "YYYY", "YYYY", "YYYY"])
+        self.game.take(9, 0, 0, 1)
+        # turn end
+
+        self.take_from_factories([(9, 1, 1, 3), (7, 2, 0, 4), (9, 3, 0, 4), (7, 4, 0, 4),
+                                  (9, 5, 0, 4)])
+        self.shuffler.instructions(["GGGG", "GGGG", "GGGG", "GGGG", "GGGG"])
+        self.take_from_center(7, "B", 1)
+        # turn end
+
+        self.take_from_factories([(7, 1, 0, 4), (9, 2, 0, 4), (7, 3, 0, 3), (9, 4, 0, 5),
+                                  (7, 5, 0, 2)])
+        self.shuffler.instructions(["LLLL", "LLLL", "LLLL", "LLLL", "LLLL"])
+        self.game.take(9, 0, 0, 2)
+        # turn end
+
+        self.take_from_factories([(9, 2, 0, 4), (7, 3, 0, 2), (9, 4, 0, 5), (7, 5, 0, 3),
+                                  (9, 1, 0, 4)])
+        # test if BLACK tiles are returned in time for the refill
+        self.shuffler.instructions(["LLLL", "LLLL", "GGGG", "GGGG", "YYYY"])
+        self.assertTrue(self.game.take(7, 0, 0, 2))
+        # turn end, refill bag
+
+        self.take_from_factories([(7, 1, 0, 4), (9, 2, 0, 4), (7, 3, 0, 3), (9, 4, 0, 5),
+                                  (7, 5, 0, 1)])
+        self.assertFalse(json.loads(self.observer.messages[-1])["finished"])
+        self.assertTrue(self.game.take(9, 0, 0, 2))
+        # turn end, game finished
+        self.assertTrue(json.loads(self.observer.messages[-1])["finished"])
+        self.assertFalse(self.game.take(9, 0, 0, 1))
 
 
 if __name__ == '__main__':

test/utils.py

@@ -1,5 +1,5 @@
 from typing import List, Tuple
-from azul.simple_types import Tile
+from azul.simple_types import Tile, RED, GREEN, BLACK, BLUE, YELLOW
 from azul.interfaces import GiveTilesInterface
 from azul.bag import Bag
 
@@ -17,6 +17,9 @@ def give(self, tiles: List[Tile]) -> None:
 class FakeShuffler(Bag.RandomTakeInterface):
     next_take: List[List[Tile]]
 
+    def __init__(self) -> None:
+        self.next_take = []
+
     def take(self, count: int, tiles: List[Tile]) -> Tuple[List[Tile], List[Tile]]:
         if not count:
             return ([], tiles)
@@ -30,3 +33,9 @@ def take(self, count: int, tiles: List[Tile]) -> Tuple[List[Tile], List[Tile]]:
             tiles.remove(tile)
 
         return we_want, tiles
+
+    def instructions(self, draws: List[str]) -> None:
+        assert not self.next_take
+        convert = {"R": RED, "G": GREEN, "B": BLUE, "L": BLACK, "Y": YELLOW}
+        for draw in draws:
+            self.next_take.append([convert[c] for c in draw])