Test Driven Development : la prise du Lion !
Publié le
Au tout début de mon voyage dans l'implémentation des règles du Dôbutsu Shôgi en utilisant le Test Driven Development, j'avais créé une fonction capture_lion, qui m'avait permis de démarrer.
Cette fonction était une fonction d'attente, et il est temps d'implémenter la condition de victoire de capture du lion correctement.
def test_has_a_winner_if_player_1_lion_got_captured(self):
self.game.drop(PIECE_LION_P1, (1, 0))
self.game.drop(PIECE_PAWN_P2, (1, 1))
self.game.capture((1, 1), (1, 0))
self.assertTrue(self.game.has_winner())
Bien entendu, je peux faire passer le test en appelant capture_lion() ou en changeant la valeur de win_condition_happened. Mais aucune de ses options n'est intéressante. Ça n'avancerait à rien.
J'en écrit une version un peu plus intéressante.
def test_has_a_winner_if_player_1_lion_got_captured(self):
self.game.drop(PIECE_LION_P1, (1, 0))
self.game.drop(PIECE_PAWN_P2, (1, 1))
self.game.capture((1, 1), (1, 0))
if self.tray.add_piece.called:
self.game.win_condition_happened = True
self.assertTrue(self.game.has_winner())
Ainsi que la version pour l'autre joueur (je ne copie pas ici la version où le test ne passe pas).
def test_has_a_winner_if_player_2_lion_got_captured(self):
self.game.drop(PIECE_LION_P2, (1, BOARD_MAXIMAL_Y))
self.game.drop(PIECE_PAWN_P1, (1, BOARD_MAXIMAL_Y - 1))
self.game.capture((1, BOARD_MAXIMAL_Y - 1), (1, BOARD_MAXIMAL_Y))
if self.tray.add_piece.called:
self.game.win_condition_happened = True
self.assertTrue(self.game.has_winner())
Et je remonte la duplication dans capture().
def capture(self, source, destination):
source_piece = self.get_piece_at(source)
dest_piece = self.get_piece_at(destination)
if (source_piece != PIECE_NONE and dest_piece != PIECE_NONE and
get_piece_controller(source_piece) != get_piece_controller(dest_piece)):
self.pieces_on_board[source_piece] = destination
self.pieces_on_board[dest_piece] = None
self.win_condition_happened = True
self.tray.add_piece(dest_piece)
Ce qui me permet ensuite d'enfin supprimer capture_lion() et le seul test qui l'appel.
Voilà, le lion est mort ce soir.
Retournement de situation
Lors de l'article précédent, je faisais remarquer que lors de l'envoi à la réserve, la pièce ne changeait pas de contrôleur. Or, lorsqu'une pièce est capturée, elle passe sous le contrôle du joueur ayant provoqué la capture.
Deux choix se présentent : est-ce que ShogiGame envoie la pièce dont le contrôle a été changé à la réserve, ou bien est-ce que la réserve en recevant la pièce provoque ce changement ?
Pour répondre à cette question, je me pose la question des responsabilités. Tray a pour responsabilité de garder une trace des pièces dans la réserve. C'est une responsabilité simple, bien définie.
ShogiGame a pour responsabilité de gérer les règles du jeu.
Le choix semble assez simple : c'est un test qui doit être écrit pour ShogiGame. Cela a amènera probablement une implémentation dans ShogiGame.
Le test existe déjà, c'est celui-ci :
def test_signals_the_captured_piece_to_the_tray(self):
list_of_pieces = (
(PIECE_ELEPHANT_P2, (0, 0)),
(PIECE_PAWN_P1, (1, 1))
)
self._place_source_and_destination_pieces_from_list_then_capture(list_of_pieces)
self.tray.add_piece.assert_called_once_with(PIECE_PAWN_P1)
Qui doit être modifié :
def test_signals_the_captured_piece_to_the_tray(self):
list_of_pieces = [
(PIECE_ELEPHANT_P2, (0, 0)),
(PIECE_PAWN_P1, (1, 1))
]
self._place_source_and_destination_pieces_from_list_then_capture(list_of_pieces)
self.tray.add_piece.assert_called_once_with(PIECE_PAWN_P2)
Pour faire passer le test, je change la ligne qui appelle add_piece() dans capture() par :
self.tray.add_piece((dest_piece[0], [0, 2, 1][dest_piece[1]]))
C'est un peu barbare, mais ça passe. Il manque une fonction utilitaire pour changer le contrôle d'une pièce.
J'ai déjà une fonction qui traite les pièces, c'est :
def get_piece_controller((piece, controller_index)):
return controller_index
J'en ai besoin d'une autre. C'est peut-être le signe qu'il me faut un concept de pièces séparé. Dans le cas contraire, je peux créer la nouvelle fonction dans doubutsugame_tests.py et voir comment elle évolue. La fonction ne sera alors testée qu'indirectement. S'il commence à y avoir plusieurs fonctions qui manipulent des pièces, il faut peut-être songer à les tester unitairement.
C'est le choix que je fais. Puisqu'il y a déjà un fichier pieces.py, je créé un fichier pieces_tests.py. J'y place un premier test que je fais passer de manière simple :
import unittest
from pieces import PIECE_PAWN_P1, PIECE_LION_P1
def get_piece_controller(piece):
return 1
class PieceTestCase(unittest.TestCase):
def test_has_a_controller(self):
self.assertEqual(get_piece_controller(PIECE_PAWN_P1), get_piece_controller(PIECE_LION_P1))
Ajouter un second test me permet de déplacer l'implémentation réelle déjà présente aux côtés de ShogiGame dans ce nouveau fichier.
def get_piece_controller((piece, controller_index)):
return controller_index
class PieceTestCase(unittest.TestCase):
def test_has_a_controller(self):
self.assertEqual(get_piece_controller(PIECE_PAWN_P1), get_piece_controller(PIECE_LION_P1))
def test_can_have_a_different_controller(self):
self.assertNotEqual(get_piece_controller(PIECE_PAWN_P2), get_piece_controller(PIECE_PAWN_P1))
Puisque les tests de pieces_tests.py passent et pour enlever la duplication d'entre les deux fichiers, je déplace la fonction get_piece_controller dans pieces.py et je l'importe pour utilisation dans doubutsugame_tests.py, où j'en retire le code.
J'ajoute un test pour la nouvelle fonctionnalité dont j'ai besoin.
def test_can_change_from_controller(self):
piece = get_same_piece_controlled_by_opponent(PIECE_PAWN_P1)
self.assertEqual(PIECE_PAWN_P2, piece)
Si je copie-colle le code de changement de contrôleur directement, cela donne quelque chose comme ça.
def get_same_piece_controlled_by_opponent(piece):
opponent = [0, 2, 1][piece[1]]
return (piece[0], opponent)
C'est un peu moche. Je remanie puisque les tests passent.
def _get_opponent_index(player_index):
return [0, 2, 1][player_index]
def get_same_piece_controlled_by_opponent(piece):
controller = get_piece_controller(piece)
return (piece[0], _get_opponent_index(controller))
C'est un peu mieux. Je ne suis pas entièrement satisfait, mais ça fera l'affaire jusqu'à nouveau besoin futur. L'essentiel est que les détails d'implémentation de la construction d'une pièce sont limités à pieces.py.
J'y déplace d'ailleurs les fonctions créées. Puis je change l'implémentation de capture() avec la nouvelle fonction.
def capture(self, source, destination):
source_piece = self.get_piece_at(source)
dest_piece = self.get_piece_at(destination)
if (source_piece != PIECE_NONE and dest_piece != PIECE_NONE and
get_piece_controller(source_piece) != get_piece_controller(dest_piece)):
self.pieces_on_board[source_piece] = destination
self.pieces_on_board[dest_piece] = None
self.win_condition_happened = True
self.tray.add_piece(get_same_piece_controlled_by_opponent(dest_piece))
Il reste encore quelques détails d'implémentations de pieces dans les tests de ShogiGame. Il s'agit des trois fonctions de test suivantes :
def test_has_a_piece_controlled_by_player_1(self):
self.assertEqual(1, get_piece_controller(PIECE_PAWN_P1))
def test_has_a_piece_controlled_by_player_2(self):
self.assertEqual(2, get_piece_controller(PIECE_ELEPHANT_P2))
def test_has_no_controller_on_empty_places(self):
self.assertEqual(0, get_piece_controller(PIECE_NONE))
Au passage, je change le nom des tests pour qu'ils correspondent à la sémantique des pièces de jeu.
def test_can_be_controlled_by_player_1(self):
self.assertEqual(1, get_piece_controller(PIECE_PAWN_P1))
def test_can_be_piece_controlled_by_player_2(self):
self.assertEqual(2, get_piece_controller(PIECE_ELEPHANT_P2))
def test_has_a_special_none_value(self):
self.assertEqual(0, get_piece_controller(PIECE_NONE))
Comme j'ai enlevé des tests pour ShogiGame, et même si je suis persuadé que je n'ai pas régressé dans ma couverture de code puisque la fonction testée a été déplacée aussi, je vérifie en passant un coup de coverage.
$ coverage run --omit="/usr/*" -m unittest doubutsugame_tests
$ coverage report -m
Name Stmts Miss Cover Missing
--------------------------------------------------
doubutsugame_tests 186 0 100%
pieces 13 0 100%
tray_tests 31 19 39% 7, 10, 14-17, 22, 25, 28-29, 32-36, 39-44
--------------------------------------------------
TOTAL 230 19 92%
Vous pouvez remarquer l'utilisation de l'option --omit pour éviter de mesurer la couverture de l'import mock, qui n'aurait aucun intérêt.
tray_tests n'est pas complètement couvert à travers le lancement des tests de ShogiGame. Ce n'est pas anormal, puisqu'on utilise un Mock Object.
Pour être certain que l'intégralité du code est toujours couvert, je lance tous les tests.
$ coverage run --omit="/usr/*" -m unittest doubutsugame_tests tray_tests pieces_tests
$ coverage report -m
Name Stmts Miss Cover Missing
--------------------------------------------------
doubutsugame_tests 186 0 100%
pieces 13 0 100%
pieces_tests 17 0 100%
tray_tests 31 0 100%
--------------------------------------------------
TOTAL 247 0 100%
La couverture est complète.
Je vois double !
À présent que les pièces peuvent changer de contrôleur, il peut se passer quelque chose qui n'est actuellement pas prévu : il peut y avoir deux pièces de même type et même contrôleur sur le tablier.
Ce n'est pas géré car l'implémentation des pièces sur le tablier est un tableau dont la clé est la pièce. Si deux pièces sont identiques, alors elles utilisent la même clé, ce qui n'est pas possible.
Je pourrai ajouter un test au niveau de ShogiGame, quelque chose comme test_can_have_identical_pieces_on_board(). Mais je trouve qu'il commence vraiment à y avoir trop de tests sur ShogiGame qui concernent la gestion du tablier d'un point de vue technique. La responsabilité principale de ShogiGame est une interface vers le jeu, qui gère les règles : ce que l'on peut faire (déplacement, capture, parachutage) et l'état du jeu (est-ce qu'il y a un vainqueur).
Je vais donc extraire les responsabilités du tablier vers une nouvelle classe. Je pourrai plus facilement y ajouter des tests techniques, comme savoir si la classe peut gérer d'avoir des pièces identiques à des endroits différents.
Quels outils pour cela ?
Les tests bien entendu. J'en ai déjà quelques uns qui concernent le tablier, comme par exemple :
def test_has_a_dropped_pawn_on_its_board(self):
pass
def test_gives_invalid_location_for_piece_not_on_board(self):
pass
def test_ignores_a_dropped_piece_outside_the_board(self):
pass
def test_has_a_dropped_elephant_on_its_board(self):
pass
En outil, j'ai aussi les Mock Objects. Et bien entendu, les principes du TDD.
Pour commencer, je vais donc extraire une classe Board directement dans le fichier doubutsugame_tests.py, je vais la tester via une classe de test correspondante. Ceci afin de manipuler le fichier rapidement. Ensuite, j'extraierai la classe et ses tests dans leurs propres fichiers.
class Board:
pass
class BoardTestCase(unittest.TestCase):
pass
La classe Board aura pour responsabilité de tenir à jour l'état des pièces sur le tablier. Je cherche donc dans les tests excitants des tests qui correspondent à cette responsabilité.
Premier candidat : test_has_a_dropped_pawn_on_its_board(). Déplacé dans Board, cela donne :
class BoardTestCase(unittest.TestCase):
def setUp(self):
self.board = Board()
def test_has_a_dropped_pawn_on_its_board(self):
location = (0, 0)
self.board.drop(PIECE_PAWN_P1, location)
self.assertEqual(PIECE_PAWN_P1, self.board.get_piece_at(location))
J'ai remplacé game par board, une instance de la classe Board. Pour faire passer le test, je pourrai copier/coller directement le contenu des fonctions drop et get_piece_at, en enlevant la partie concernant tray. Cependant, ça serait aller trop vite !
En effet, j'écrirai beaucoup plus de code que nécessaire et donc mes tests suivants fonctionneraient probablement du premier coup. Ce n'est pas possible en TDD.
Petit rappel sur l'intérêt de faire échouer les tests en TDD : en faisant échouer les tests en premier lieu on s'assure de deux choses. Tout d'abord que le test ajouté à un intérêt et n'est pas déjà traité. Ensuite que lors d'une future régression, l'échec du test soit correctement signalé.
Si un test passe du premier coup, sans besoin de nouveau code d'implémentation, cela signifie que ce test est inutile, et qu'il n'est pas certain qu'en cas d'erreur, il reporte une quelconque erreur.
Je vais donc copier/coller de l'implémentation de ShogiGame la partie minimale pour que le test passe.
Techniquement, uniquement faire renvoyer PIECE_PAWN_P1 par get_piece_at() serait suffisant à faire passer le test. Mais on a déjà fait ce chemin là à la première implémentation. On peut se permettre d'aller un peu plus vite.
class Board:
def __init__(self):
self.pieces_on_board = {}
def drop(self, piece, location):
self.pieces_on_board[piece] = location
def get_piece_at_location(self, location):
pieces_at_location = [p[0] for p in self.pieces_on_board.items() if p[1] == location]
if len(pieces_at_location) == 1:
return pieces_at_location[0]
return None
def get_piece_at(self, location):
piece = self.get_piece_at_location(location)
return piece or PIECE_NONE
Un petit test de couverture de code montre qu'en fait, le return None de get_piece_at_location() n'est pas testé. Je remplace les trois dernières lignes par return pieces_at_location0. Je relance les tests. C'est bon.
Mais du coup il me faut un nouveau test, car ce code était là pour quelque chose.
def test_returns_piece_none_if_no_piece_on_location(self):
location = (0, 0)
self.assertEqual(PIECE_NONE, self.board.get_piece_at(location))
Échec. Je remets les lignes. Ça passe. Et la couverture est complète.
Bon, mais maintenant, il y a de la duplication. Et les tests passent. Je suis donc en phase de refactor et je vais éliminer la duplication entre les classes.
J'ai un choix à faire. Soit je donne à ShogiGame la responsabilité de création d'une instance de Board, soit je lui passe à la création. Le second choix me permettrait d'utiliser un Mock Object.
Un Mock Object serait peut être un peu compliqué à programmer avec le nombre de tests sur ShogiGame qui concernent le tablier. Cependant, laisser la responsabilité à ShogiGame de la création du Board est limitant.
Je transforme donc ShogiGame et ShogiGameTestCase comme ceci :
class ShogiGame:
def __init__(self, board, tray):
self.win_condition_happened = False
self.pieces_on_board = {}
self.board = board
self.tray = tray
class ShogiGameTestCase(unittest.TestCase):
def setUp(self):
self.tray = mock.Mock(tray.Tray)
self.tray.get_piece.side_effect = lambda x: x
board = Board()
self.game = ShogiGame(board, self.tray)
L'idée maintenant serait de transformer toutes les manipulations de pieces_on_board vers Board. Or il y a encore quelques accès directs à ce membre.
Un petit tour des besoins montre qu'il y du placement de pièce, un drop() au sens Board, mais aussi de la suppression de pièce, qui n'existe pas encore. Il y a aussi get_piece_location(), qui peut être déplacée dans Board.
Je vais remplacer les utilisations restantes par des appels à deux fonctions internes : _place_on_board() et _remove_from_board().
class ShogiGame:
def place_on_board(self, piece, location):
self.pieces_on_board[piece] = location
def remove_from_board(self, piece):
self.pieces_on_board[piece] = None
Je fais une pause dans la phase refactor et pour créer le get_piece_location() dans Board. Puis j'ajoute un test pour avoir un remove_from_board() dans Board.
Sauf que je ne veux pas qu'on spécifie la pièce à enlever, mais l'emplacement de la pièce à enlever. En effet, le but final est de pouvoir avoir des pièces identiques sur le tablier.
def test_can_remove_a_piece(self):
location = (0, 0)
self.board.drop(PIECE_PAWN_P1, location)
self.board.remove_piece(location)
self.assertEqual(PIECE_NONE, self.board.get_piece_at(location))
Mais comme pour le moment, les pièces restent les clés du tableau associatif, j'implémente simplement comme ceci :
def remove_piece(self, location):
piece = self.get_piece_at_location(location)
self.pieces_on_board[piece] = None
Puis j'utilise drop et remove_piece dans place_on_board et remove_from_board. Je remplace tous les get_piece_at_location() par un appel à board.
Lors de mon refactor, je fais une erreur. En effet, le nouveau remove_piece() fonctionne avec un emplacement, mais capture() fonctionne toujours avec une piece. Je dois donc changer l'appel de remove_from_board() et le placer avant le place_on_board() :
def capture(self, source, destination):
source_piece = self.get_piece_at(source)
dest_piece = self.get_piece_at(destination)
if (source_piece != PIECE_NONE and dest_piece != PIECE_NONE and
get_piece_controller(source_piece) != get_piece_controller(dest_piece)):
self.remove_from_board(destination)
self.place_on_board(source_piece, destination)
self.win_condition_happened = True
self.tray.add_piece(get_same_piece_controlled_by_opponent(dest_piece))
Le nom drop() sur Board ne me plait pas. Je le remplace par place_piece().
Piece je supprime place_on_board() et remove_from_board() en remplaçant leurs appels par des appels directs aux méthodes de board.
Et je peux aussi enlever le test dupliqué test_has_a_dropped_pawn_on_its_board des tests de ShogiGame.
Je fais le même type de travail avec :
test_ignores_a_dropped_piece_outside_the_board()test_ignores_a_new_piece_if_a_piece_is_already_at_the_given_location
J'ai aussi retiré test_has_a_dropped_elephant_on_its_board() qui est un doublon historique avec le placement du pion. Lors du déplacement, aucune erreur ne s'étant produit, cela signifiait que le code était déjà correct.
Et enfin, j'extrais Board dans board.py et ses tests dans board_tests.py.
Je passe un petit test de couverture de code pour vérifier. Je suis toujours à 100% et j'ai extrais la gestion des pièces sur le tablier.
La prochaine fois, je pourrais donc modifier plus simplement le fonctionnement du tablier pour gérer le fait d'avoir deux pièces identiques.
Un bug !
En recherchant un peu la duplication existante encore après l'existant, je suis tombé sur cette partie :
def capture(self, source, destination):
source_piece = self.get_piece_at(source)
dest_piece = self.get_piece_at(destination)
if (source_piece != PIECE_NONE and dest_piece != PIECE_NONE and
get_piece_controller(source_piece) != get_piece_controller(dest_piece)):
self.board.remove_piece(destination)
self.board.place_piece(source_piece, destination)
self.win_condition_happened = True
self.tray.add_piece(get_same_piece_controlled_by_opponent(dest_piece))
win_condition_happened qui passe à True dès qu'une capture est réussie ? J'ai bien l'impression qu'il manque un test. Cette ligne a été écrite pour faire passer les conditions de victoire de capture de lion.
Cependant, n'importe quelle capture provoque une victoire.
J'ajoute une condition au test de capture normal :
def test_allows_an_elephant_to_capture_a_pawn(self):
list_of_pieces = (
(PIECE_ELEPHANT_P2, (0, 0)),
(PIECE_PAWN_P1, (1, 1))
)
self._place_source_and_destination_pieces_from_list_then_capture(list_of_pieces)
((source_piece, source), (dest_piece, destination)) = list_of_pieces
self.assertEqual(PIECE_NONE, self.game.get_piece_at(source))
self.assertEqual(source_piece, self.game.get_piece_at(destination))
self.assertFalse(self.game.has_winner())
Qui ne passe pas. Et que je peux faire passer comme ceci :
def capture(self, source, destination):
source_piece = self.get_piece_at(source)
dest_piece = self.get_piece_at(destination)
if (source_piece != PIECE_NONE and dest_piece != PIECE_NONE and
get_piece_controller(source_piece) != get_piece_controller(dest_piece)):
self.board.remove_piece(destination)
self.board.place_piece(source_piece, destination)
if dest_piece[0] == 'L':
self.win_condition_happened = True
self.tray.add_piece(get_same_piece_controlled_by_opponent(dest_piece))
Cependant, j'accède ici au fonctionnement interne des pièces.
J'ajoute donc un nouveau test aux pieces :
def test_can_be_a_lion(self):
self.assertTrue(is_a_lion(PIECE_LION_P1))
self.assertTrue(is_a_lion(PIECE_LION_P2))
self.assertFalse(is_a_lion(PIECE_PAWN_P1))
J'implémente la fonction is_a_lion() avec le code de capture et j'y remplace ce code par l'utilisation de la fonction.
Cette façon de résoudre des bugs est intéressante. J'ai déjà eu l'occasion de l'utiliser. Ayant trouvé une situation buggé, rien de mieux que d'écrire le test qui révèle le problème et l'envoyer au programmeur qui peut analyser le problème.
C'est souvent beaucoup plus efficace que d'expliquer en long et en large par mail l'environnement et les conditions du bug.