Refactorizando legacy code en PHP Parte 5 – creando test sencillos

Refactoring de una aplicación PHP Legacy. Después del intenso post sobre como empezar con loo test unitarios en un refactoring en este vamos a trabajar con la clase Game.php, empezaremos a crear pequeños test de funcionalidades para ir refactorizando. Recordemos que la idea es empezar a refactorizar desde verde para intentar no introducir errores en nuestro refactor.

Creando un “Game”

Lo primero que necesitamos para poder empezar a testear es una instancia de Game, para ello lo primero que haremos sera crearnos nuestra clase específica para test (GameTest) y crear un pequeño test para crear una nueva partida de Trivial.

class GameTest extends PHPUnit_Framework_TestCase
{
    public function testCreateAGameOk()
    {
        $game = new Game();
        $this->assertInstanceOf('Game', $game);
    }
}

Parece sorprendente pero sin mayor problema hemos podido crear una instancia de Game fácilmente y nada se rompe.

Para estar seguros de que nada se rompe, podemos crear un pequeño archivo de configuración para PHPUnit en el que indiquemos la carpeta donde están los tests, algo más o menos así:

<phpunit bootstrap = "vendor/autoload.php">
    <testsuites>
        <testsuite name="LegacyCode Test Suite">
            <directory>Test</directory>
        </testsuite>
    </testsuites>
</phpunit>

Encontrando el primer método testeable

Al igual que la vez anterior, lo primero que tenemos que hacer es buscar un método para empezar. ALgo no demasiado complejo y que no necesitemos muchas precondiciones. El mejor candidato es isPlayable que indica si podemos jugar o no una partida.

Si recordamos esta aplicación legacy simula un juego del trivial, por tanto para jugar es necesario que al menos haya 2 jugadores, así que vamos a crear el tests. El primero de ellos será un test que no nos deje jugar y después crear un test para el caso negativo.

    public function testAJustCreatedNewGameIsNotPlayable()
    {
        $game = new Game();
        $this->assertFalse($game->isPlayable());
    }

    public function testCreatedNewGameIsPlayable()
    {
        $game = new Game();
        $game->add('player 1');
        $game->add('player 2');
        $this->assertTrue($game->isPlayable());
    }

No debemos dejar nada sin probar, hacer pruebas de code coverage para ver si de verdad estamos probando todas las partes es una buena práctica. Pero ojo que el código tenga un code coverage sobre 90% no significa que no se vaya a romper

Para crear el test verdadero, nos hemos basado en GameRunner para ver como se añaden jugadores, también hemos analizado el código que añade jugadores (array_push) así que podemos estar más o menos tranquilos de que nuestros tests están probando la función. Quizás pienses que utilizar add es una mala opción, todavía no lo hemos testeado, ni analizado, es cierto, pero creo que es la opción menos mala.

El siguiente paso será refactorizar los tests, teniendo en cuenta minimizar el numero de assert por test. La idea no es llegar a soluciones como esta que harán que nuestro código sea poco mantenible.

public function testIsNotPlayable()
{
    $game = new Game();
    $this->assertFalse($game->isPlayable());
    $game->add('A player');
    $this->assertFalse($game->isPlayable());
    $game->add('A player');
    $this->assertTrue($game->isPlayable());

}

Esto parece ser lo mejor, pero hará nuestros tests poco mantenibles y poco legibles. Te dejo la opción de como refactorizar los tests. ¿Hemos terminado ya con esto? Más o menos, ahora con los tests en verde ya podemos empezar ha hacer mantenible el código, intentando que si el mínimo de jugadores cambia de 2 a 4 o a 1 no haya que cambiar demasiado. Si recuerdas, algo parecido pasó en GameRunner, así que vamos allá a crear una variable estática que indique el mínimo número de jugadores:

class Game {
    static $minimumNumbersOfPlayers = 2;

    .....
    function isPlayable()
    {
        return ($this->howManyPlayers() >= Game::$minimalNumberOfPlayer);
    }

}

Lo mismo pasa con los tests hay que hay que refactorizarlos tanto test para el caso positivo como negativo. Después de darle un par de vueltas a los test para refactorizarlos y eliminar todo el código repetido obtenemos algo parecido a esto:

    public function testAJustCreatedNewGameIsNotPlayable()
    {
        $this->assertFalse($this->_game->isPlayable());
        $this->addInsufficientPlayers();
        $this->assertFalse($this->_game->isPlayable());
    }

    public function testCreatedNewGameIsPlayable()
    {
        $this->addEnoughPlayers();
        $this->assertTrue($this->_game->isPlayable());
    }

    protected function addEnoughPlayers()
    {
        $this->addALotOfPlayers(Game::$minimalNumberOfPlayer);
    }

    protected function addInsufficientPlayers()
    {
        $this->addALotOfPlayers(Game::$minimalNumberOfPlayer - 1);
    }

    protected function addALotOfPlayers($numberOfPlayers)
    {
        for ($i = 0; $i < $numberOfPlayers; $i++) {
            $this->_game->add('a Player');
        }
    }

Podemos dar por concluido el trabajo pasa isPlayable, ahora solo nos queda buscar el siguiente método a testear.

No debemos olvidar ir dando pequeños pasos y “commiteando” a menudo para poder volver atrás si lo necesitamos Elimina Control + Z de la mente.

Buscando el siguiente método a testear

Si analizamos el código de Game podemos escoger cualquier método, pero lo mejor sería seguir con el método add, es un método que utilizamos en los tests, así que debemos estar seguros de que funciona bien para no estropear el trabajo que llevamos hecho hasta ahora. ¿Como sería el test?

    public function testCanAddANewPlayer()
    {
        $this->assertEquals(0,count($this->_game->players));
        $this->_game->add('a Player');
        $this->assertEquals(1,count($this->_game->players));
    }

Ya hemos hablado de test, pero un buena idea a la hora de hacer test es estructurarlos pensando en algo así: given, when, then. Con esta mnemotecnica podemos contar una historia con cada test: Dada una partida con 0 jugadores, cuando añado un jugador a la partida entonces tengo un juego con 1 jugador. ¿Hemos terminado? ¿Solo hay que tener en cuenta el número de jugadores o hay más elementos?

Cada vez que añadimos un jugador a la partida, también se añade un elemento a places y a purses por lo que es necesario testear también estas cosas.

    public function testCanAddANewPlayer()
    {
        $this->assertEquals(0, count($this->_game->players));
        $this->_game->add('a Player');
        $this->assertEquals(1, count($this->_game->players));
        $this->assertEquals(0, $this->_game->places[1]);
        $this->assertEquals(0, $this->_game->purses[1]);
        $this->assertFalse($this->_game->inPenaltyBox[1]);
    }

Esto esta algo mejor, probamos todo lo relacionado con añadir un nuevo jugador. Quizás este test esté un poco acoplado al código de Game… y sea mejorable. Os propongo que intentéis mejorarlo 🙂 Aunque como siempre, podemos refactorizar un poco más los tests para intentar que el acoplamiento afecte lo menos posible a los tests.

    public function testCanAddANewPlayer()
    {
        $this->assertEquals(0, count($this->_game->players));
        $this->_game->add('a Player');
        $this->assertEquals(1, count($this->_game->players));
        $this->assertSetDefaultParametersForPlayer(1);
    }

    protected function assertSetDefaultParametersForPlayer($playerId)
    {
        $this->assertEquals(0, $this->_game->places[$playerId]);
        $this->assertEquals(0, $this->_game->purses[$playerId]);
        $this->assertFalse($this->_game->inPenaltyBox[$playerId]);
    }

Teniendo los tests ahora podemos refactorizar el método add, además mirando los tests ya tenemos como hacer el refactor 🙂

    function add($playerName)
    {
        array_push($this->players, $playerName);
        $this->setDefaultParameterForPlayer($this->howManyPlayers()]);

        echoln($playerName . " was added");
        echoln("They are player number " . count($this->players));

        return true;
    }

    protected function setDefaultParameterForPlayer($playerId)
    {
        $this->places[$playerId]= 0;
        $this->purses[$playerId] = 0;
        $this->inPenaltyBox[$playerId] = false;
    }

De momento nos olvidaremos del método echon, ya lo afrontaremos más adelante. De la misma manera, para llegar a este punto hemos dado un par de pasitos. Primero extrayendo el método y luego sacando factor común a howManyPlayers().

A por el siguiente método

La idea sería ir a por howManyPlayers()pero es demasiado simple, ahí no hay mucho que testear. roll()es demasiado complejo todavía, no tenemos mucho código testeado para asegurarlo. askQuestion() solo depende de echon así que mejor no tocarlo todavía, creo que un buen reto podría ser didPlayerWin(), ya que sigue la misma filosofía que otros métodos que ya hemos refactorizado. Extraer una variable.

Lo primero el test:

    public function testPlayerWinsWithTheCorrectNumberOfScore()
    {
        $this->_game->currentPlayer = 0;
        $this->_game->purses[0] = Game::$numberOfScoreToWin;
        $this->assertTrue($this->_game->didPlayerWin());
    }

El test falla :S ¿Por qué? porque solo nos hemos fijado en el nombre del método. Como podemos ver en el return se niega la condición, por lo que el método quizás debería llamarse didPlayerNotWin() y por tanto deberíamoscambiar el nombre del test y el assert. Así que después de los cambios tenemos este test:

     public function testPlayerNotWinsWithTheCorrectNumberOfScore()
    {
        $this->_game->currentPlayer = 0;
        $this->_game->purses[0] = Game::$numberOfScoreToWin;
        $this->assertFalse($this->_game->didNotPlayerWin());
    }

Nos queda extraer la variable numberOfScoreToWin.

A por el siguiente método

Vamos a elegir wrongAnswer() ya que tiene poco acoplamiento con echon y su salida es un boolean. A por el test:

    function testWhenAPlayerEntersAWrongAnswerItIsSentToThePenaltyBox() {
        $this->_game->add('A player');
        $this->_game->currentPlayer = 0;
        $this->_game->wrongAnswer();
        $this->assertTrue($this->_game->inPenaltyBox[0]);
    }

Solo hemos testeado la mitad, no entramos dentro del if. Aunque el método de test parezca sencillo en realidad llegar hasta él nos ha costado un poco, hay bastante acoplamiento entre capa de presentación y lógica. El test no está completo, entramos en el if por pura suerte ya que el método shoudResetCurrentPlayer() tiene en cuenta al número de jugadores y en este caso solo hay 1. Necesitamos un par de test uno que entre en el if (el que tenemos) y otro que no .

    public function testWhenAPlayerEntersAWrongAnswerItIsSentToThePenaltyBox()
    {
        $this->_game->add('A player');
        $this->_game->currentPlayer = 0;
        $this->_game->wrongAnswer();
        $this->assertTrue($this->_game->inPenaltyBox[0]);
        $this->assertEquals(0, $this->_game->currentPlayer);
    }

    public function testCurrentPlayerIsNotResetAfterWrongAnswerIfOtherPlayersDidNotYetPlay()
    {
        $this->addALotOfPlayers(2);
        $this->_game->currentPlayer = 0;
        $this->_game->wrongAnswer();
        $this->assertEquals(1, $this->_game->currentPlayer);
    }

Tenemos este 2ºtest para probar el caso de que jugadores todavía no jugaron.

Ya tenemos los tests, pero el método wrongAnswer esta bien de momento, no necesitamos refactorizarlo. Hemos creado los tests para tener una buena cobertura (dormir tranquilos) cuando entremos en refactorizaciones más profundas.

Conclusiones

La cobertura de nuestros tests va aumentando, ya tenemos bastantes tests unitarios que se ejecutan muy rápido y además hemos refactorizado unas pocas funciones. Cada vez entendemos un poco mejor como funciona la clase Game y como podemos mejorarla a futuro. En próximos tutoriales nos adentraremos en refactorizar los métodos más complejos, esto solo ha sido un entrenamiento.
Podrás encontrar todo el código en mi cuenta de github (https://github.com/jeslopcru/LegacyCode) cada post está etiquetado con un tag.

Este post está basado en http://code.tutsplus.com/tutorials/refactoring-legacy-code-part-5-games-testable-methods–cms-21213