Refactorizando legacy code en PHP Parte 4 – como empezar con test unitarios en un refactoring

Continuamos con la serie refactoring PHP legacy code. en el post de hoy empezaremos a hacer test unitarios que serán la base de todos los cambios y refactorizaciones futuras. Extraeremos funcionalidades, pequeñas piezas de código que podremos probar con test unitarios.Aunque parezca que solo son unos simples test, no debemos olvidar que estamos trabajando con código “feo”, sin coding estándar. Como siempre iremos dando pequeños pasos para tropezar.

Test unitarios con PHPUnit

Ya hemos hablado en el blog de test unitarios puedes encontrarlo aquí así que no vamos a dar una introducción a los tests unitarios. Además como ya tenemos composer con phpunit instalado solo tenemos que empezar.

Los test unitarios deben ser independientes

Buscando funciones aisladas

Si el código lo permite (como este caso) es recomendable empezar a escribir test de código que podamos probar. Esto nos será útil para comprender la lógica de funciones pequeñas y para ir cubriendo poco a poco el código a testear.

Analizando nuestros archivos, podemos descartar de momento el método run de GameRunner.php porque no tiene ninguna lógica y aunque podemos probarlo no tiene sentido gastar demasiado tiempo en el todavía. Vamos a por un trabajo más fácil para coger un poco de práctica antes. ¿Recordáis el método isCorrectAnswer() que refactorizamos en el anterior post? Vamos a por él.

Lo primero es centrarnos solo en esta función, por lo que de momento vamos a marcar como skipped el test de GameRunnerTest.php y vamos a crear un test específico para testear la función is isCorrectAnswer, pero tenemos un problema la funcion rand() necesitamos hacer algo para controlarla 😉 Así que lo primero será ir a la documentación y hacer una pequeña prueba ¿que pasa si min y max son iguales?

    function testCanFindCorrectAnswer()
    {
        for ($i = 0; $i < 10; $i++) {
            var_dump(rand($i,$i));
        }
    }

¡Mola! Ya tenemos una manera de forzar números aleatorios como nosotros queramos.

Dependencia de Inyección

Según la wikipedia es un patrón de diseño orientado a objetos, en el que se suministran objetos a una clase en lugar de ser la propia clase quien cree el objeto. Para nuestro caso lo que hacemos es pasar como parámetros min y max. Así que creamos un test para los casos de pregunta correcta

function testCanFindCorrectAnswer()
    {
        for ($i = 0; $i < 7; $i++) {
            $this->assertTrue(isCorrectAnswer($i,$i));
        }
    }

Con el test ya hecho ahora refactorizamos el método

function isCorrectAnswer( $minAnswerId = 0,$maxAnswerId = 9)
{
    $wrongAnswerId = 7;
    return rand($minAnswerId, $maxAnswerId) != $wrongAnswerId;
}

Como vemos el test no es exhaustivo, hay casos que no estamos probando como el de respuesta falsa, así que vamos a crear un test para ese caso:

    function testCanFindWrongAnswer()
    {
        $this->assertFalse(isCorrectAnswer(7,7));
    }

Ya esta algo mejor, aun así todavía hay casos que se nos escapan. Podemos crear otro bucle para crear el resto de casos, pero entramos en un terreno un tanto farragoso… tenemos tests pero estos no son legibles ni mantenibles, además tampoco nos ayudan a entender el código. Una solución podría ser crear un customAssert que nos ayude en este caso.

    function testCanFindCorrectAnswer()
    {
        $correctAnswerId = [0, 1, 2, 3, 4, 5, 6, 8, 9];
        $this->assertAnswersAreCorrectFor($correctAnswerId);
    }

    function testCanFindWrongAnswer()
    {
        $this->assertFalse(isCorrectAnswer(7, 7));
    }

    protected  function assertAnswersAreCorrectFor($correctAnswerIDs) 
    {
        foreach ($correctAnswerIDs as $id) {
            $this->assertTrue(isCorrectAnswer($id, $id));
        }
    }

Con estos tests podemos entender mejor el concepto ya que nuestro test es simple y dejamos la lógica enmascarada en una función protected fácil de entender. Otra ventaja clara es que si hay cambios en la aplicación podemos modificar los tests de manera sencilla.

Con estos tests podemos darle un poco más de cariño a la función, así si definiremos como constante el ID de una pregunta incorrecta por si en un futuro cambiamos dicho ID.

const WRONG_ANSWER_ID = 7;

function isCorrectAnswer($minAnswerId = 0,$maxAnswerId = 9)
{
    return rand($minAnswerId, $maxAnswerId) != WRONG_ANSWER_ID;
}
`

y también modificamos el test:

    function testCanFindWrongAnswer()
    {
        $this->assertFalse(isCorrectAnswer(WRONG_ANSWER_ID, WRONG_ANSWER_ID));
    }

Siguiendo la misma filosofía, vamos a refactorizar el test para los casos correctos. Para ello vamos a extraer a una función el array de Id así:

    function testCanFindCorrectAnswer()
    {
        $this->assertAnswersAreCorrectFor($this->getGoodAnswerId(););
    }

    protected function getGoodAnswerId()
    {
        return [0, 1, 2, 3, 4, 5, 6, 8, 9];
    }

Esta función podría estar buen, pero se puede mejorar un poco más:

    protected function getGoodAnswerId()
    {
        return array_diff(range(0,9), [WRONG_ANSWER_ID]);
    }

Podemos pensar que esto es suficiente, pero podríamos refinar mucho más. Por ejemplo cambiando minAnswerId y maxAnswerIdcomo constantes para que sea posible modificar el rango de preguntas. Solo tenemos que seguir los mismo pasos que para WRONG_ANSWER_ID

A por el método Run

Todavía no hemos terminado de refactorizar la clase, ahora vamos a ir poco a poco refactorizando el método run utilizando sobre todo las refactorizaciones automáticas de PHPStorm para no romper demasiado.

function run()
{
    $notAWinner;

    $aGame = new Game();

    $aGame->add("Chet");
    $aGame->add("Pat");
    $aGame->add("Sue");

    do {
        $dice = rand(0, 5) + 1;
        $aGame->roll($dice);

        if (!isCorrectAnswer()) {
            $notAWinner = $aGame->wrongAnswer();
        } else {
            $notAWinner = $aGame->wasCorrectlyAnswered();
        }

    } while ($notAWinner);
}

Lo primero será refactorizar el if para ello vamos a extraerlo a una función que nos devuelva true o false dependiendo de si ha habido o no un ganador.

function getNotAwinner($aGame)
{
    if (!isCorrectAnswer()) {
        $notAWinner = $aGame->wrongAnswer();

        return $notAWinner;
    } else {
        $notAWinner = $aGame->wasCorrectlyAnswered();

        return $notAWinner;
    }
}

Parece que vamos bien, ahora podemos crear un test con el que ir probando la función para refactorizarla. Si tenemos dudas sobre si hemos extraído bien o no siempre podemos ejecutar el test master que marcamos antes como skipped.

Nuestro test se llamará : testWhenCorrectAnswerIsProviderItCanTellIfThereIsNoWinnerpuede ser un poco largo, pero debemos tener en cuenta que dentro de 6 meses no tendremos ni idea del código que hemos escrito así que debemos intentar ser lo más explícitos posibles con el namming.
Aquí llega nuestro 1º problema, necesitamos tener un Game para que todo esto funcione.

Podemos utilizar cualquier técnica para conseguir objetos falsos, Mocking, Stubbing y Fakking. Lo más fácil sería crear una clase falsa GameFake y crear los métodos necesarios vamos a dar un pasito más y ha utilizar Mockery o PHPUnit para mockear.

    public function testWhenCorrectAnswerIsProviderItCanTellIfThereIsNoWinner()
    {
        $mock = \Mockery::mock('Game');
        $mock->shouldReceive('wrongAnswer');
        $mock->shouldReceive('wasCorrectlyAnswered');

        getNotAwinner($mock);
    }

    public function tearDown()
    {
        Mockery::close();
    }

Con esto ya tenemos los tests “funcionando”, ahora tenemos que preocuparnos de que valores queremos que devuelvan los métodos para poder hacer un assert. En un primer momento podemos hacer que wasCorrectlyAnswered devuelvuelva siempre cierto, así podemos crear un assert.

public function testWhenCorrectAnswerIsProviderItCanTellIfThereIsNoWinner()
    {
        $mockGame = \Mockery::mock('Game');
        $mockGame->shouldReceive('wrongAnswer');
            ->andReturn(true);
        $mockGame->shouldReceive('wasCorrectlyAnswered')
            ->andReturn(true);

        $this->assertTrue(getNotAwinner($mockGame));
    }

De la misma manera podemos crear un test que sea testWhenAWrongAnswerIsProvidedItCanTellIfThereIsNoWinner con un mock que siempre devuelva falso.

    public function testWhenAWrongAnswerIsProvidedItCanTellIfThereIsNoWinner()
    {
        $mockGame = \Mockery::mock('Game');
        $mockGame->shouldReceive('wrongAnswer')
            ->andReturn(false);
        $mockGame->shouldReceive('wasCorrectlyAnswered')
            ->andReturn(false);


        $this->assertFalse(getNotAwinner($mockGame));
    }
```language

Llegados a este punto, tenemos un pequeño problema, nuestro método llama a isCurrentAnswerCorrect y esto es algo malo, ya que su salida depende de valores aleatorios :S. No estamos testeando todos los casos por lo que es necesario refactorizar un poco todo esto antes de continuar.

function run()
{
    $notAWinner;

    $aGame = new Game();

    $aGame->add("Chet");
    $aGame->add("Pat");
    $aGame->add("Sue");

    do {
        $dice = rand(0, 5) + 1;
        $aGame->roll($dice);

        $notAWinner = getNotAwinner($aGame,isCorrectAnswer());

    } while ($notAWinner);
}

function getNotAwinner($aGame, $isCorrectAnswer)
{
    if (!$isCorrectAnswer) {
        $notAWinner = $aGame->wrongAnswer();

        return $notAWinner;
    } else {
        $notAWinner = $aGame->wasCorrectlyAnswered();

        return $notAWinner;

Ahora tenemos algo más de control sobre el método getNotAWinner por lo que podemos refactorizar nuestros test así:

    public function testWhenCorrectAnswerIsProviderItCanTellIfThereIsNoWinner()
    {
        $mockGame = \Mockery::mock('Game');
        $mockGame->shouldReceive('wrongAnswer')
            ->andReturn(true);
        $mockGame->shouldReceive('wasCorrectlyAnswered')
            ->andReturn(true);

        $this->assertTrue(getNotAwinner($mockGame, true));
    }

Estamos pasando valores “a fuego” como vulgarmente se dice a los métodos a testear, además estamos mockeando métodos que no se están utilizando. en una segunda iteración podemos limpiar un poco los métodos así:

    public function testWhenCorrectAnswerIsProviderItCanTellIfThereIsNoWinner()
    {
        $isCorrectAnswer = true;

        $mockGame = \Mockery::mock('Game');
        $mockGame->shouldReceive('wasCorrectlyAnswered')
            ->andReturn($isCorrectAnswer);

        $this->assertTrue(getNotAwinner($mockGame, $isCorrectAnswer));
    }

Ahora vamos a darle una última pasada al método. Como dijimos anteriormente lo mejor es pensar en positivo e intentar que las variables estén “inline” siempre que el código siga siendo legible.

function run()
{
    $aGame = new Game();

    $aGame->add("Chet");
    $aGame->add("Pat");
    $aGame->add("Sue");

    do {
        $dice = rand(0, 5) + 1;
        $aGame->roll($dice);

    } while (didSomeoneWin($aGame, isCorrectAnswer()));
}

function didSomeoneWin($aGame, $isCorrectAnswer)
{
    if ($isCorrectAnswer) {
        return !$aGame->wrongAnswer();
    } else {
        return !$aGame->wasCorrectlyAnswered();
    }
}

Al fin hemos eliminado esa variable $notAWinner que PHPStorm nos estaba marcando en rojo 🙂 Pero ojo, para ser coherentes con el cambio a positivo tenemos que cambiar los tests.

Conclusiones

Ha quedado un post un poco largo, pero hemos empezado a utilizar test unitarios, mockear, a ir paso a paso creando funciones pequeñas.

Este post se ha basado en http://code.tutsplus.com/tutorials/refactoring-legacy-code-part-4-our-first-unit-tests–cms-21146