Te llega un email, es otro ticket de ese envío de notificaciones que hiciste hace tiempo, la solución que hiciste tiene otro bug,.. ¿te suena? O abrir un pedazo de código, empezar a leerlo y que te entren sudores fríos porque no entiendes nada o porque lo que vemos allí no quieres tocarlo ni con un palo… y lo peor es que haces un git blame y eres tu mismo el que escribiste eso en un «día de inspiración». Quizás esos pedazos de código puedan ser un buen ejemplo de anti-patrón. Un anti-patrón es una serie de técnicas que nos conducen a una mala solución para un problema. Lo mejor que puedes hacer con los anti-patrones es conocerlos para así puedas evitarlos en el futuro.
Hoy me gustaría escribirte una pequeña recopilación de anti-patrones de tests. Hace unos años ya escribí sobre «patrones test utilizando PHPUnit» y sobre «patrones de test, mejorando la arquitectura en PHP» así que vamos allá.
Este post se basa en los ejemplos de https://github.com/sarven/unit-testing-tips, en las respuestas StackOverflow sobre unit testing antipatterns, en el post «Unit Testing Anti-Patterns, Full List» y en el libro «Unit Testing Principles, Practices, and Patterns» Aquí comentaré algunos anti-patrones y su solución
Testing private methods
Hay varios anti-patrones asociados a usar métodos privados en los tests, tenemos por un lado «anal probe«, es un test que tiene que utilizar «triquiñuelas» para realizar su tarea: acceder a un método privado, extender una clase para acceder a parámetros protected. Por otro lado tenemos «The Inspector«, este anti-patrón viola la encapsulación con el objetivo de llegar a un porcentaje de cobertura mucho más alto.
Nos daremos cuenta de que tenemos uno de estos anti-patrones cuando cualquier intento de refactorización rompe la prueba existente y requiere que cualquier cambio se refleje en la prueba unitaria.
Vamos con un ejemplo en el que tenemos una clase Order y queremos probar su método getTotal()
final class Order
{
/**
* @param OrderItem[] $items
* @param int $transportCost
*/
public function __construct(private array $items, private int $transportCost) {}
public function getTotal(): int
{
return $this->getItemsTotal() + $this->transportCost;
}
private function getItemsTotal(): int
{
return array_reduce(
array_map(fn (OrderItem $item) => $item->getTotal(), $this->items),
fn (int $sum, int $total) => $sum += $total,
0
);
}
}
final class OrderItem
{
public function __construct(private int $total) {}
public function getTotal(): int
{
return $this->total;
}
}
Malos tests
Una mala aproximación sería intentar probar todos los métodos tanto públicos como privados así:
final class InvalidTest extends TestCase
{
/**
* @test
* @dataProvider ordersDataProvider
*/
public function get_total_returns_a_total_cost_of_a_whole_order(Order $order, int $expectedTotal): void
{
self::assertEquals($expectedTotal, $order->getTotal());
}
/**
* @test
* @dataProvider orderItemsDataProvider
*/
public function get_items_total_returns_a_total_cost_of_all_items(Order $order, int $expectedTotal): void
{
self::assertEquals($expectedTotal, $this->invokePrivateMethodGetItemsTotal($order));
}
public function ordersDataProvider(): array
{
return [
[new Order([new OrderItem(20), new OrderItem(20), new OrderItem(20)], 15), 75],
[new Order([new OrderItem(20), new OrderItem(30), new OrderItem(40)], 0), 90],
[new Order([new OrderItem(99), new OrderItem(99), new OrderItem(99)], 9), 306]
];
}
public function orderItemsDataProvider(): array
{
return [
[new Order([new OrderItem(20), new OrderItem(20), new OrderItem(20)], 15), 60],
[new Order([new OrderItem(20), new OrderItem(30), new OrderItem(40)], 0), 90],
[new Order([new OrderItem(99), new OrderItem(99), new OrderItem(99)], 9), 297]
];
}
private function invokePrivateMethodGetItemsTotal(Order &$order): int
{
$reflection = new \ReflectionClass(get_class($order));
$method = $reflection->getMethod('getItemsTotal');
$method->setAccessible(true);
return $method->invokeArgs($order, []);
}
}
Como se ha comentado arriba que los test dependan de los métodos privados es una mala idea. ¿Qué es lo que tendríamos que hace? ¿Cómo se podría probar todo esto sin romper la encapsulación? Sencillo, no usar reflexión. Aunque puede que haya casos en los que usar la reflexión esté justificado como puede ser trabajar con código legacy, esto solo debería ser un estado intermedio para proveernos de seguridad hasta llegar a un estado sin reflexión.
Un buen test podría ser
final class ValidTest extends TestCase
{
/**
* @test
* @dataProvider ordersDataProvider
*/
public function get_total_returns_a_total_cost_of_a_whole_order(Order $order, int $expectedTotal): void
{
self::assertEquals($expectedTotal, $order->getTotal());
}
public function ordersDataProvider(): array
{
return [
[new Order([new OrderItem(20), new OrderItem(20), new OrderItem(20)], 15), 75],
[new Order([new OrderItem(20), new OrderItem(30), new OrderItem(40)], 0), 90],
[new Order([new OrderItem(99), new OrderItem(99), new OrderItem(99)], 9), 306]
];
}
}
Trivial Test
No tiene nada que ver con el juego de mesa, sino con test que no aportan ningún valor, o su único valor es aumentar la cobertura, por ejemplo probar los setter y getter:
final class Customer
{
public function __construct(private string $name) {}
public function getName(): string
{
return $this->name;
}
public function setName(string $name): void
{
$this->name = $name;
}
}
///////
final class CustomerTest extends TestCase
{
public function testSetName(): void
{
$customer = new Customer('Pepe');
$customer->setName('Manolo');
self::assertEquals('Manolo', $customer->getName());
}
}
Happy path
Aunque antes se dice que no hay que testearlo todo, sí que es necesario que los test sean robustos y prueben casos extraños y no solo el happy path. Los valores límite es algo a tener en cuenta cuando estamos testeando.
final class Factorial
{
public function __construct(){}
public function calculate(int $number): int
{
$factorial = 1;
for ($x=$number; $x>=1; $x--) {
$factorial = $factorial * $x;
}
return $factorial;
}
}
Un mal test sería alguno que solo probase los casos típicos
final class FactorialTest extends TestCase
{
/**
* @test
* @dataProvider factorialData
*/
public function testFactorial(int $number, int expected): void
{
$result = (new Factorial)->calculate($number);
self::assertEquals($expected, $result);
}
public function factorialData(): array
{
return [
[1,1],
[2,2],
[3,5],
];
}
}
¿Qué pasa con los casos límite?¿Cómo los pruebas? Podrías poner más casos de test para tener cubiertos más casos, el inconveniente de esto es que hace los test mucho menos legibles y al final acabamos con un batiburrilo de casos.
Lo mejor es hacer los test los más legibles posible y no tratar a los test como ciudadanos de segunda clase. Por ejemplo cuando usamos «dataproviders» tener un método para los casos positivos y otro para los negativos
final class ExampleTest extends TestCase
{
/**
* @test
* @dataProvider getInvalidEmails
*/
public function detects_an_invalid_email_address(string $email): void
{
$sut = new EmailValidator();
$result = $sut->isValid($email);
self::assertEquals(false, $result);
}
/**
* @test
* @dataProvider getValidEmails
*/
public function detects_an_valid_email_address(string $email): void
{
$sut = new EmailValidator();
$result = $sut->isValid($email);
self::assertEquals(true, $result);
}
public function getInvalidEmails(): array
{
return [
['test'],
['test@'],
['test@test'],
//...
];
}
public function getValidEmails(): array
{
return [
['test@test.com'],
['test123@test.com'],
['Test123@test.com'],
//...
];
}
}
Slow Poke
Una prueba unitaria que funciona realmente lenta. Si tienes tiempo para ir al baño, fumar o desesperarte mientras que un test unitario se está ejecutando eso es un mal síntoma. La idea de los test unitarios es que nos proporcionen de un feedback rápido mientras estamos desarrollando. Si los test son lentos, cada vez los ejecutamos menos y eso al final pasa factura.
Mockery
A veces, utilizar Mocks puede ser útil y beneficioso. Pero otras veces… puedes perderte con los mocks y no tener claro qué se está probando. Si un test tienes que interaccionar con stubs y el sujeto de test no se está probando directamente, sino que se prueban los datos devueltos por los stubs puede ser síntoma de que es frágil.
Aquí un mal ejemplo:
final class TestExample extends TestCase
{
/**
* @test
*/
public function sends_all_notifications(): void
{
$message1 = new Message();
$message2 = new Message();
$messageRepository = $this->createMock(MessageRepositoryInterface::class);
$messageRepository->method('getAll')->willReturn([$message1, $message2]);
$mailer = $this->createMock(MailerInterface::class);
$sut = new NotificationService($mailer, $messageRepository);
$messageRepository->expects(self::once())->method('getAll');
$mailer->expects(self::exactly(2))->method('send')
->withConsecutive([self::equalTo($message1)], [self::equalTo($message2)]);
$sut->send();
}
}
El problema que tenemos aquí es que estamos probando el estado interno del mock con esta línea. Para mí un test correcto sería el mismo pero tan solo eliminado esa linea, ya que es donde se chequea la implementación que se ha hecho y no el comportamiento.
$messageRepository->expects(self::once())->method('getAll');
Generous Leftovers (aka Chain Gang, Wet Floor)
Uno de los test crea datos y los guarda en algún sitio (p.e. en la base de datos) y luego otros test reutilizan esos datos. Es el típico caso donde estamos probando un CRUD, el primer test crea los datos, el siguiente los lee,… Una manera de identificar este antipatrón es el uso de depends
/** @depends testPutSlots */
public function testGetSlots(): void
{
//....
}
Siempre podemos llevar estas dependencias a la inicialización de los test, como puede ser el método setup o en el peor de los casos al método setUpBeforeClass.
Una solución para evitar tener dependencias ocultas es ejecutar los test en un orden random, para ello solo tenemos que poner la directiva executionOrder="random" en nuestro fichero phpunit.xml
Local Hero (aka Hidden Dependency, Operating System Evangelist, Wait and See, Environmental Vandal)
Es una variante del anterior, es un test que depende de algo instalado en tu entorno, pero que falla en el resto. El caso típico es que hemos instalado una nueva dependencia como puede ser RabbitMQ o Redis y la usamos en algunos test. Otra opción es la típica de puertos distintos, donde cada desarrollador tiene montado el entorno de una manera. Para ello la solución más sencilla sería que todo el mundo utilizase algo como Docker en local y que ese Docker reprodujese (lo más fielmente posible) el entorno de producción.
Enumerator (aka Test With No Name)
Son una serie de unit test donde cada uno de ellos tiene un nombre que solo es una numeración. El nombre del test es solo el método que prueba.
Aquí un mal ejemplo de clase de test
final class TestExample extends TestCase
{
public function testSendNotification1(): void
{
//....
}
public function testSendNotification2(): void
{
//....
}
public function testSendNotificationBUG431(): void
{
//....
}
}
El resultado que vemos de estos test es que no sabemos qué se está probando, tenemos que leer el test para entenderlo. La solución es escribir tests con mejores nombres.
- El nombre debe describir el comportamiento, no la implementación
- Utilizar nombres sin palabras técnicas. Debería ser legible para una persona que no sea programadora.
¿Por qué es importante dar buenos nombres a los tests?
Si tenemos una una lógica de dominio compleja, esta lógica tiene que ser clara para todas las personas involucradas, por lo que si escribimos test con buenos nombres sin tecnicismos es sencillo hablar un lenguaje común. Algunos ejemplos que deberíamos evitar son devuelve null, lanza una excepción,… ya que este tipo de información no tiene nada que ver con el dominio.
Free Ride (aka Piggyback)
Esto pasa cuando vamos con prisas o peor aún cuando nos puede la pereza… lo que hacemos es añadir un nuevo assert en otro test en lugar de escribir un nuevo test. La típica justificación es que nuestra test suit es lenta y añadir un nuevo caso de test la hará aun más lenta.
Excessive Setup (aka Mother Hen)
Es un test que requiere mucho esfuerzo de configuración para empezar a probar. A veces se utilizan cientos de linea de código, con otros objetos involucrados, lo que puede impedir (aka llenar de ruido) lo que realmente se está probando.
Una prueba que requiere mucho trabajo para configurar para incluso comenzar a probar. A veces, se utilizan varios cientos de líneas de código para configurar el entorno para una prueba, con varios objetos involucrados, lo que puede dificultar la determinación real de lo que se está probando debido al «ruido» de toda la configuración.
Unas veces puede ser debido a que necesitamos tener guardados en Base de datos una cierta cantidad de objetos, otras puede ser debido a exceso de mocks haciendo que nuestros test sean frágiles
Un mal ejemplo de excesivo setup debido a demasiado mock
final class TestUserRepository extends TestCase
{
public function testGetUserNameByEmail(): void
{
$email = 'test@test.com';
$connection = $this->createMock(Connection::class);
$queryBuilder = $this->createMock(QueryBuilder::class);
$result = $this->createMock(ResultStatement::class);
$userRepository = new UserRepository($connection);
$connection
->expects($this->once())
->method('createQueryBuilder')
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('from')
->with('user', 'u')
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('where')
->with('u.email = :email')
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('setParameter')
->with('email', $email)
->willReturn($queryBuilder);
$queryBuilder
->expects($this->once())
->method('execute')
->willReturn($result);
$result
->expects($this->once())
->method('fetch')
->willReturn(['email' => $email]);
$result = $userRepository->getUserNameByEmail($email);
self::assertEquals(['email' => $email], $result);
}
}
Probar repositorios de esa manera conduce a pruebas frágiles y después la refactorización será muy difícil. Para probar repositorios, sería mejor apoyarnos en pruebas de integración.
Otra opción para mejorar este antipatrón es apoyarnos en el patrón Object Mothers, este patrón nos permite crear objetos que puedan ser reutilizables en varias pruebas
Aquí un ejemplo de Object Mother
final class SubscriptionMother
{
public static function new(): Subscription
{
return new Subscription();
}
public static function activated(): Subscription
{
$subscription = new Subscription();
$subscription->activate();
return $subscription;
}
public static function deactivated(): Subscription
{
$subscription = self::activated();
$subscription->deactivate();
return $subscription;
}
}
///////////////
final class ExampleTest
{
public function example_test_with_activated_subscription(): void
{
$activatedSubscription = SubscriptionMother::activated();
// do something
// check something
}
}
Success Against All Odds (aka The Liar, Evergreen Test)
Es un test que no valida ningún comportamiento y siempre es verde. El problema con estos test es que no sirven para nada, ya que podríamos borrar parte del código de producción y ese test seguiría fallando. Normalmente estos test se dan cuando se escriben después de la implementación, por lo que el autor del test no está seguro de lo que está probando.
La regla de oro en este caso es siempre comprobar que el test falla antes de ponerlo en verde, desconfía de los test hasta que los veas fallar
Conclusiones
Una de las cosas que te habrás dado cuenta es que existen multitud de anti-patrones de tests, muchos de estos anti-patrones podrían atribuirse a una implementación un poco defectuosa o mejor dicho a una implementación que no tuvo en cuenta a los tests desde el principio. Por ejemplo: test que siempre están en verde, clases difíciles de instanciar, test que necesitan mucho setup,… todo esto son señales de advertencia. Lo que puedo decir en este sentido es:
- Escribir nuestro software con un enfoque top-down de modo que por un lado solo implementamos lo que necesitamos y por otro se hace más sencillo comprender los de abajo
- Reconocer cuando un componente tiene varias responsabilidades (la S de SOLID) para así poder dividirlo
En resumen si es complicado, lo estás haciendo mal. La solución ideal suele ser sencilla; solo necesita ser perseverante e iterar hasta lograrlo.
Jesús L.C. - Apuntes de un aprendiz 