diff --git a/.pylintrc b/.pylintrc index 02900ba..ce94ab8 100644 --- a/.pylintrc +++ b/.pylintrc @@ -3,3 +3,4 @@ disable = missing-module-docstring, missing-class-docstring, missing-function-do [SIMILARITIES] ignore-imports=yes +min-similarity-lines=6 diff --git a/azul/game.py b/azul/game.py index d463c6e..41e1a26 100644 --- a/azul/game.py +++ b/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,6 +54,7 @@ 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 = { @@ -60,7 +62,8 @@ def _state(self) -> str: "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 diff --git a/comments b/comments index f5c7390..38e9be2 100644 --- a/comments +++ b/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 :). + + diff --git a/test/test_integration/test_game.py b/test/test_integration/test_game.py index 2026eb7..b08e8cb 100644 --- a/test/test_integration/test_game.py +++ b/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__': diff --git a/test/utils.py b/test/utils.py index d4886ee..2874b2b 100644 --- a/test/utils.py +++ b/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])