Mokona Guu Center

Test Driven Development : le début d'une partie

Publié le

À présent, sauf erreur, toutes les règles sont implémentées à l'exception de la détection d'une partie nulle. Il est temps de revenir à une session de jeu et de faire jouer deux joueurs automatiquement.

Pièce de Shôgi en bois

Les tests actuellement disponibles pour une session de jeu sont :

  • Session
    • Creates a board
    • Places the initial pieces on board
    • Ask move to active player
  • SessionFactory
    • Creates a session
    • Creates a session with the given players
    • Creates a session with one of the player active

Il me faudrait maintenant une boucle qui demande les actions des joueurs jusqu'à l'arrêt du jeu. Cela pourrait être une responsabilité de Session, mais cette classe à déjà un ask_player_move. Cela me semble étrange d'y ajouter la responsabilité d'animer la session.

Je créé donc une nouvelle série de tests sur un SessionRunner qui commence par son initialisation.

    class SessionRunnerTestCase(unittest.TestCase):
        def test_is_initialized_with_a_session(self):
            SessionRunner(mock.Mock(Session))

Qui se résous s'implement en créant la classe correspondante :

    class SessionRunner:
        """SessionRunner runs a given session by asking the players to play until the end of the game."""
        def __init__(self, session):
            pass

Petite nouveauté, l'arrivée d'une chaîne de documentation. Même s'il m'est arrivé d'en placer ici ou là, je n'en ai pas vraiment parlé. Je ne suis pas un grand fan de forcer une documentation excessive. La documentation, contrairement à du code, n'est pas exécutée. La logique derrière les tests unitaires est de pouvoir s'assurer que tout ce qui est écrit répond correctement à une certaine vérification. Si du code change, les tests doivent changer (en TDD, les tests changent d'ailleurs avant le reste).

Les commentaires, eux, ne suivent aucune règle autre qu'éventuellement syntaxique. Le code d'une classe peut changer sans que les commentaires soient mis à jour. Rien ne vérifiera que la documentation ne reflète plus exactement le fonctionnement de la classe.

Cette divergence est une maladie chronique de la sur-documentation.

Et pourtant, commenter est intéressant. Comment commenter alors ? La première forme de documentation lorsque l'on fait du TDD est assez simple : c'est l'ensemble des tests. Du code exécuté montre au lecteur la manière dont le système a été pensé. C'est normal, puisque les tests ont été pensés avant le code qui les résous.

Il reste selon moi deux autres choses à commenter dans un code :

  • les responsabilités d'une classe, sous forme d'une phrase. Si la phrase est trop longue ou trop alambiquée, la classe a probablement trop de responsabilités. C'est un bon test.
  • les parties de code non évidentes. Il s'agit là de commenter des astuces, des parties qu'un autre lecteur pourraient trouver non triviales, voire louches, des effets de bords inattendus, une explication prévenant qu'une autre solution n'est pas meilleure,... La naissance de ces commentaires à lieu généralement en revue de code : lorsque le paire à qui l'on explique le changement pose une question, a un doute sur un fonctionnement, mais qu'après explication, il accepte la solution, c'est qu'il y a de la place pour un commentaire.

C'est tout. À mon sens, toute autre documentation doit rester loin du code, tout en y étant liée (archivée au même endroit). Le but est de faciliter la lecture d'un autre programmeur (ou de soit même un mois plus tard). Trop de verbosité empêche une lecture fluide. Quelques phrases bien placées aident.

Revenons à la boucle.

Le problème qui se pose lorsque l'on veut tester une boucle qui doit s'arrêter sur une condition, c'est que si la boucle ne s'arrête jamais, on ne récupère jamais la main. Le système qui lance les tests doit donc être pourvu d'un système de timeout. Cependant les systèmes de timeout ne sont jamais très agréables : à combien définir le temps maximal ? Trop juste, et les tests échoueront dès qu'ils prendront un peu de poids. Trop long, et l'attente d'un retour d'erreur devient long lorsque l'erreur se produit.

Pour résoudre ce problème, je vais fournir à mon SessionRunner un système de signal qui pourra appeler une fonction donnée avant chaque demande de mouvement à un joueur. Cela pourra être utile plus tard pour tenir un journal des évènements du système. En attendant, cela va surtout me permettre de contrôler la boucle et d'en sortir d'urgence si je détecte un problème.

    def test_triggers_an_event_before_asking_for_move():
        ask_player_callback = mock.Mock()

        session = mock.Mock(Session)
        runner = SessionRunner(session)
        runner.set_on_before_ask_player(ask_player_callback)

        runner.launch_game()

        ask_player_callback.assert_called_once_with(session)
        session.ask_player_move.assert_called_once_with()

Il y a un petit peu trop de choses testées ici, à cause de la boucle qui est un fonctionnement opaque. J'installe d'abord le système avec la Session, le SessionRunner auquel j'associe une fonction de callback (mockée). Je lance ensuite la boucle et je vérifie que la callback ainsi que la session ont été appelées.

Je ne teste pas que la callback est appelée avant la session. Je pourrais, mais c'est un peu trop complexe pour mon test qui est déjà bien fourni.

Tout cela m'amène à écrire les fonctions set_on_before_ask_player() et launch_game() de SessionRunner.

    class SessionRunner:
        """SessionRunner runs a given session by asking the players to play until the end of the game."""
        def __init__(self, session):
            self.on_before_ask_player = None

        def set_on_before_ask_player(self, callback):
            self.on_before_ask_player = callback

        def launch_game(self):
            pass

L'erreur suivante est que la callback n'a pas été appelée. Je l'appelle donc. Puis la session mockée entre en erreur à son tour puisque ask_player_move n'est pas appelée.

Le tout donne :

    class SessionRunner:
        """SessionRunner runs a given session by asking the players to play until the end of the game."""
        def __init__(self, session):
            self.on_before_ask_player = None
            self.session = session

        def set_on_before_ask_player(self, callback):
            self.on_before_ask_player = callback

        def launch_game(self):
            if self.on_before_ask_player is not None:
                self.on_before_ask_player(self.session)
            self.session.ask_player_move()

Je remanie immédiatement le code des tests pour mettre dans un setUp la duplication de l'initialisation. Ce qui, on en a maintenant l'habitude, fait disparaître le premier test qui n'était là que pour l'initialisation.

Je peux maintenant écrire un test pour vérifier que la boucle s'arrête bien lorsque le jeu est terminé. Du moins, un début de test :

    def test_runs_a_loop_until_game_is_finished(self):
        ask_player_callback = mock.Mock()
        self.runner.set_on_before_ask_player(ask_player_callback)

        self.runner.launch_game()

Quoi tester ? Je dois mettre à un moment la session dans un état 'terminé'. Je dois aussi vérifier que le SessionRunner a bien tourné tant que la session n'était pas terminée.

Grâce à la callback installé juste au test précédent, je vais pouvoir contrôler la session via son objet mocké. Disons qu'au bout de 5 appels, la session passe dans un état terminé.

J'écris donc cela :

    def test_runs_a_loop_until_game_is_finished(self):
        def ask_player_callback_side_effect(session):
            if session.get_move_count() == 5:
                session.is_finished = mock.Mock(return_value=True)
            return mock.DEFAULT

        session.is_finished = mock.Mock(return_value=False)

        ask_player_callback = mock.Mock(side_effect=ask_player_callback_side_effect)
        self.runner.set_on_before_ask_player(ask_player_callback)

        self.runner.launch_game()

        self.assertEqual(5, self.session.get_move_count())

J'introduis l'existence d'une méthode is_finished() sur la Session. Méthode qui n'existe pas actuellement. Comme je veux que ce soit relevé comme une erreur, je change légèrement le setUp en précisant un spec_set. Un MockObject avec un spec_set plutôt qu'un spec (l'argument par défaut) lancera une erreur s'il reçoit une demande sur quelque chose (méthode ou attribut) qui n'existe pas dans l'objet mocké.

Le test échoue donc tout d'abord par absence de cette fonction is_finished(), que j'ajoute donc à Session. Pour son implémentation, j'envoie une exception pour éviter d'oublier de l'implémenter lorsque le moment sera venu.

La méthode manquante suivante est get_move_count, que j'implémente de la même manière.

    class Session:
        def is_finished(self):
            raise exceptions.NotImplementedError("Session.is_finished() is not yet implemented")

        def get_move_count(self):
            raise exceptions.NotImplementedError("Session.get_move_count() is not yet implemented")

Le test ne passe pas cependant, avec un message d'erreur plutôt cryptique :

    AssertionError: 5 != <Mock name='mock.get_move_count()' id='154179276'>

La valeur 5 est comparée à un object. Forcément, c'est différent. L'objet en question est un objet mocké automatiquement géré par le framework de mock par auto_spec et la présence de la méthode dans Session.

Je programme donc le compteur de session à travers l'appel de ask_player_callback_side_effect. Puisqu'un test me dit que j'appelle bien cette fonction à chaque fois, c'est correct.

    def test_runs_a_loop_until_game_is_finished(self):
        def ask_player_callback_side_effect(session):
            session.get_move_count.return_value += 1
            if session.get_move_count() == 5:
                session.is_finished.return_value = True
            return mock.DEFAULT

        self.session.is_finished.return_value = False
        self.session.get_move_count.return_value = 0

        ask_player_callback = mock.Mock(side_effect=ask_player_callback_side_effect)
        self.runner.set_on_before_ask_player(ask_player_callback)

        self.runner.launch_game()

        self.assertEqual(5, self.session.get_move_count())

À présent, le test échoue car SessionRunner n'a pas encore de boucle. La méthode fait un seul appel à la session puis sort. Il est donc temps de l'implémenter.

J'ai ai aussi profité (suite à un test qui échoue) pour corriger la programmation du mock de is_finished() en n'agissant que sur l'attribut return_value.

Je remarque au passage que test_triggers_an_event_before_asking_for_move se met à échouer. En effet, self.session.is_finished() n'est pas programmé dans ce test. La boucle n'est jamais exécutée et donc la fonction de callback n'est jamais appelée.

    def test_triggers_an_event_before_asking_for_move(self):
        def ask_player_callback_side_effect(session):
            self.session.is_finished.return_value = True
            return mock.DEFAULT

        self.session.is_finished.return_value = False

        ask_player_callback = mock.Mock(side_effect=ask_player_callback_side_effect)
        self.runner.set_on_before_ask_player(ask_player_callback)

        self.runner.launch_game()

        ask_player_callback.assert_called_once_with(self.session)
        self.session.ask_player_move.assert_called_once_with()

Et voilà.

Mais ça fait beaucoup de duplication. Voici une proposition :

    class SessionRunnerTestCase(unittest.TestCase):
        def setUp(self):
            self.session = mock.Mock(spec_set=Session)
            self.runner = SessionRunner(self.session)
            self.session.is_finished.return_value = False

        def _run_loop_with_side_effect(self, side_effect):
            ask_player_callback = mock.Mock(side_effect=side_effect)
            self.runner.set_on_before_ask_player(ask_player_callback)

            self.runner.launch_game()

            return ask_player_callback

        def test_triggers_an_event_before_asking_for_move(self):
            def ask_player_callback_side_effect(session):
                self.session.is_finished.return_value = True
                return mock.DEFAULT

            ask_player_callback = self._run_loop_with_side_effect(ask_player_callback_side_effect)

            ask_player_callback.assert_called_once_with(self.session)
            self.session.ask_player_move.assert_called_once_with()

        def test_runs_a_loop_until_game_is_finished(self):
            def ask_player_callback_side_effect(session):
                self.session.get_move_count.return_value += 1
                if session.get_move_count() == 5:
                    self.session.is_finished.return_value = True
                return mock.DEFAULT

            self.session.get_move_count.return_value = 0

            self._run_loop_with_side_effect(ask_player_callback_side_effect)

            self.assertEqual(5, self.session.get_move_count())

Je dois avouer qu'il y a un détail que je n'aime pas : la valeur de retour dans _run_loop_with_side_effet(). De manière générale, j'essaie toujours d'avoir soit une commande (un appel qui a un effet de bord, une modification d'état), soit une fonction (un appel qui renvoie un résultat, sans effet de bord). Ici, suivre ce principe amènerait à trop de code, alors que j'essaie de simplifier la suite de tests.

Si vous trouvez mieux, je suis preneur.

Choisir son mouvement

Bon, j'ai de quoi lancer une partie. À présent, il faut qu'un joueur puisse un mouvement. J'avais déjà parlé des joueurs lors du début de leur implémentation. Je n'avais pas beaucoup de choses à tester. À présent qu'il ne reste que ça (à quelques détails près), il faut que je m'y mette.

Avec quelle stratégie ?

Ce que je voudrais faire est d'avoir deux joueurs suivant un scénario d'une partie prédéfinie et vérifier que l'état de la partie se déroule comme elle le devrait.

Pour cela, je dois vérifier qu'un joueur est capable de renseigner ces choix. Je dois aussi veiller à ce que les joueurs puissent vérifier l'état du tablier comme devra le faire une vraie implémentation de joueur plus tard.

Pour cette vérification, je veux faire un premier pas avec une Session et un premier appel au premier joueur.

Ici, je sors du principe de test unitaire. J'ai en effet besoin de d'une vraie session et de deux vrais joueurs. Les joueurs ne peuvent pas être des mock objects car même si le scénario ressemble à une programmation d'objet, le code va agir sur des objets passés en paramètre et/ou appeler des fonctions (j'imagine...). Cela pourrait se faire à coup de side_effect mais cela deviendrait complexe pour une utilisation assez minimale, voire absente, de ce qu'offre le framework de mock.

Puisqu'il va me falloir une session, c'est le moment d’extraire Session dans un nouveau fichier session.py.

Cette opération s'accompagne d'un nettoyage des imports et de lancement de l'intégralité des tests pour vérifier que tout se passe bien.

Note : pour vérifier la validité des imports facilement sans y aller par essai/erreur, je vous conseille fortement l'utilisation d'un 'linter' (Flake8 par exemple) qui vous donnera des indications sur des erreurs évidentes (pour un ordinateur) dans votre code source. Couplé a un éditeur de texte qui lancera le 'linter' automatiquement en continue ou à la sauvegarde du fichier, vous gagnerez beaucoup de temps.

À présent, où placer ces tests. Pas dans pieces_tests.py, qui n'a actuellement pas d'import de Session et aucune raison d'en ajouter.

session_tests.py serait un meilleur endroit, mais pour le moment, je n'ai mis dans les fichiers post fixés _tests.py que des tests unitaires.

J'opte pour session_functests.py. C'est purement personnel, et c'est même la première fois que j'opère cette différence. Je veux voir ce que cela donne. Il est tout à fait possible de mettre les tests dans session_tests.py, à la condition qu'ils n'encombrent pas le fichier avec des dépendances inutiles pour les tests unitaires.

On y va ?

    import unittest
    import mock
    from session import Session
    from player import Player

    class Scenario1_Player1(Player):
        pass

    class GameSessionTestCase(unittest.TestCase):
        def test_scenario_1(self):
            player_1 = Scenario1_Player1()
            player_2 = mock.Mock(Player)
            session = Session([player_1, player_2])
            session.ask_player_move()

Premier problème, le test signale que ask_move() du joueur attend un paramètre. En effet, Player est défini comme ceci :

    class Player:
        def ask_move(self, effector):
            pass

Cet effector vient des premiers tests sur Player qui vérifiaient qu'un joueur avait un moyen d'effectuer des actions sur le jeu. Les tests n'étaient pas bien poussés, mais l'idée était là. Je vais la conserver.

Ceci dit, comment se fait-il que ask_player_move() n'ait jusqu'à maintenant pas créé d'erreur ? Probablement car j'utilisais un MockObject. Et si les MockObjects du framework move vérifient la validité du nom des fonctions appelées, ils ne vérifient pas les arguments à moins que cela soit demandé explicitement.

Or le test sur ask_move est :

    def test_ask_move_to_active_player(self):
        self.session.ask_player_move()
        self.player_1.ask_move.assert_called_once_with()
        self.session.ask_player_move()
        self.player_2.ask_move.assert_called_once_with()

Il n'y a pas de tests sur l'argument. Et cet argument n'existe de toute façon actuellement pas. Je vais donc le créer au niveau du dispatcher pour faire passer le test actuel du scénario 1.

    class PlayerDispatcher:
        def ask_player_move(self):
            self.active_and_opponent[0].ask_move(None)
            self.active_and_opponent = self.active_and_opponent[1], self.active_and_opponent[0]

Cela fait passer le test du scénario 1, mais test_ask_move_to_active_player ne passe plus. Mais attention, c'est le test qui est faux, pas la modification que j'ai faite. En effet, le test était incohérent en vérifiant que l'appel se faisait sans paramètres. C'est une erreur que j'ai faite à ce moment.

Je reconnais l'erreur et je modifie le test en conséquence. Au niveau de la session, je cherche juste à savoir si æsk_move a été appelée sur un joueur. Je ne cherche pas à vérifier les arguments ; ce test n'a pas les moyens de le vérifier.

    def test_ask_move_to_active_player(self):
        self.session.ask_player_move()
        self.assertEqual(1, self.player_1.ask_move.call_count)
        self.session.ask_player_move()
        self.assertEqual(1, self.player_2.ask_move.call_count)

Tout passe, je peux revenir sur mon scénario fonctionnel.

    class GameSessionTestCase(unittest.TestCase):
        def test_scenario_1(self):
            player_1 = Scenario1_Player1()
            player_2 = mock.Mock(Player)
            session = Session([player_1, player_2])
            session.ask_player_move()

            where_the_giraffe_was = (0, 0)
            self.assertEqual(PIECE_NONE, session.board.get_piece_at(where_the_giraffe_was))

J'imagine donc que le joueur 1 a avancé sa giraffe d'une case. L'avantage est que c'est un mouvement qui n'entraine pas de prise (j'aurais pu aussi partir sur un scenario où c'est le lion qui bouge).

Évidemment, le test échoue. Il échoue avec une erreur pas très explicative :

    AssertionError: <pieces.Piece instance at 0xa07dcac> != <pieces.Piece instance at 0xa07df6c>

J'avais déjà eu ce soucis avec les tests de session_tests.py. Et j'avais ajouté une fonction spécialisée pour écrire un message d'erreur plus explicite.

Si je le copie dans cette nouvelle classe de test, j'obtiens :

    AssertionError: Piece('', 0) != Piece('G', 1)

Mais du coup, j'ai du code dupliqué. J'extrais donc la partie d'initialisation de la fonction dans une classe de test dont j'hériterai sur les tests qui ont besoin d'une spécialisation de l'affichage.

    class SessionTests(unittest.TestCase):
        def __init__(self, methodName='runTest'):
            unittest.TestCase.__init__(self, methodName)
            self.addTypeEqualityFunc(type(Piece(GIRAFFE, P1)), 'assertEqualObject')

        def assertEqualObject(self, first, second, msg=None):
            if not first == second:
                message = "%s != %s" % (str(first), str(second))
                raise self.failureException(message)

Et je place cette classe dans un fichier test_utils.py.

Maintenant que j'ai un affichage d'erreur sympathique, je peux continuer.

J'aimerais pouvoir résoudre le test comme ceci :

    class Scenario1_Player1(Player):
        def ask_move(self, effector):
            effector.move((0, 0), (0, 1))

Mais effector est en fait None, puisque cette partie n'est pas du tout implémentée.

Je bricole un Effector rapidement histoire que l'erreur ne soit pas un objet None.

    class Effector:
        def move(self, source, destination):
            pass

    class PlayerDispatcher:
        def ask_player_move(self):
            self.active_and_opponent[0].ask_move(Effector())
            self.active_and_opponent = self.active_and_opponent[1], self.active_and_opponent[0]

À présent, je remonte la pile des appels. Pour le moment, c'est le PlayerDispatcher qui créé l'effecteur à envoyer au joueur. Mais le PlayerDispatcher n'est qu'une classe d'aide à la classe qui a vraiment les informations : Session. Je remonte donc l'Effector dans session.py et j'ajoute un argument à ask_player_move.

Cependant, cela brise les tests de PlayerDispatcher qui ne connaissent pas cet argument. Puisque je brise des tests déjà écrit, c'est qu'il est temps de se poser des questions.

Vraiment.

Quitter le clavier, se rafraîchir, marcher un peu. C'est important. Briser le 'flow' intentionnellement peut parfois être bénéfique.

Je reviens sur le test initial : vérifier que la giraffe a bougé. Je suis sensé alors écrire un code minimal qui doit faire passer le test. Or ça fait plusieurs modification que je fais et le tests ne passe toujours pas. Pire, j'ai brisé des tests déjà existant, je m'éloigne donc de l'objectif.

Cependant, j'y ai gagné des indices :

  • Il est temps de s'occuper de cette notion d'Effector que j'avais introduit pendant la construction de Player
  • L'action du joueur doit remonter jusqu'à la Session pour être intéressante.

Attends, j'ai pas lâché le pion!

Donc je remets la giraffe à sa place : je désactive les tests du scénario fonctionnel 1.

Voici mes nouveaux tests pour PlayerDispatcher :

    class PlayerDispatcherTestCase(unittest.TestCase):
        def setUp(self):
            self.player_1 = mock.Mock(Player)
            self.player_2 = mock.Mock(Player)
            self.dispatcher = PlayerDispatcher(self.player_1, self.player_2)

        def test_asks_first_player_for_move_first(self):
            self.dispatcher.ask_player_move(effector=1)

            self.player_1.ask_move.assert_called_once_with(1)
            self.assertFalse(self.player_2.ask_move.called)

        def test_asks_second_player_for_move_after_first(self):
            self.dispatcher.ask_player_move(effector=1)
            self.dispatcher.ask_player_move(effector=2)

            self.player_2.ask_move.assert_called_once_with(2)
            self.assertTrue(self.player_2.ask_move.called)

        def test_asks_first_player_for_move_after_second(self):
            self.dispatcher.ask_player_move(effector=None)
            self.dispatcher.ask_player_move(effector=None)
            self.dispatcher.ask_player_move(effector=None)

            self.assertEqual(2, self.player_1.ask_move.call_count)
            self.assertEqual(1, self.player_2.ask_move.call_count)

Qui se résolvent bien évidemment en ajoutant un paramètre effector transmit à l'objet joueur actif.

Petite note : j'ai nommé le paramètre systématiquement à l'appel (effector=1 par exemple) alors que ça n'est pas obligatoire puisque c'est un paramètre positionnel. Cependant, j'utilise effector sans utiliser un objet qui est vraiment du type attendu. J'utilise le fait que Python est typé dynamiquement pour y passer une valeur simple à vérifier (un entier).

Un appel qui ressemblerait à self.dispatcher.ask_player_move(1) n'aurait pas vraiment de sens. En précisant que c'est le paramètre effector que j'assigne, je signale que c'est volontaire.

Alors cette giraffe ?

Je réactive mon test de déplacement de Giraffe. Et je peux à présent écrire un code qui fait passer le test.

    class Effector:
        def __init__(self, board):
            self._board = board

        def move(self, source, destination):
            self._board.remove_piece(source)


    class Session:
        def ask_player_move(self):
            self._dispatcher.ask_player_move(Effector(self.board))

J'ajoute un second test à mon déplacement de giraffe

    class GameSessionTestCase(SessionTests):
        def test_scenario_1(self):
            player_1 = Scenario1_Player1()
            player_2 = mock.Mock(Player)
            session = Session([player_1, player_2])
            session.ask_player_move()

            where_the_giraffe_was = (0, 0)
            where_the_giraffe_goes = (0, 1)
            self.assertEqual(PIECE_NONE, session.board.get_piece_at(where_the_giraffe_was))
            self.assertEqual(Piece(GIRAFFE, P1), session.board.get_piece_at(where_the_giraffe_goes))

Que je peux résoudre très simplement :

    class Effector:
        def __init__(self, board):
            self._board = board

        def move(self, source, destination):
            self._board.remove_piece(source)
            self._board.place_piece(Piece(GIRAFFE, P1), destination)

Et je peux améliorer move comme ceci :

    def move(self, source, destination):
        piece = self._board.get_piece_at(source)
        self._board.remove_piece(source)
        self._board.place_piece(piece, destination)

Mais je suis en train de réimplémenter teleport_piece qui existe déjà dans ShogiGame. Sans toutes les règles du jeu.

Session ne devrait pas passer un Board à Effector mais un ShogiGame. Pour le moment Session ne créé pas de ShogiGame, cela sera probablement pour le prochain épisode.

À la prochaine.