Resolviendo bugs con TDD
En nuestro proyecto de lista de tareas hemos desarrollado lo que podríamos llamar el happy path de la aplicación. Es decir, hemos supuesto que al crear una tarea la consumidora del API no cometería errores como intentar crear una tarea sin nombre. Otro supuesto es que al marcar una tarea como completada se usarán id de tareas que existan en la lista.
Esto significa que la aplicación puede fallar si no se cumple alguno de estos supuestos. ¿Es esto un bug? En cierto sentido sí, aunque también podemos argumentar que son prestaciones no implementadas.
Cuando desarrollamos usando TDD podemos prevenir muchos defectos debidos a implementaciones con errores. Por ejemplo, imagina que no hemos implementado un método Task::markComplete y no tenemos tests cuya ejecución implique esa llamada. El resultado será un bug.
Obviamente, si no hemos escrito un test para verificar un comportamiento específico, como podría ser impedir que se puedan crear tareas sin descripción, ese defecto acabará apareciendo.
Se dice que ni el testing ni el TDD nos libran de que un software tenga defectos. Sin embargo, creo que podemos estar muy seguras de que usando TDD los defectos aparecerán en aquellas partes que no estén cubiertas por un test. Considerados así, por tanto, los defectos son más bien circunstancias no previstas o casos no implementados todavía. Esto es interesante y bastante liberador, porque en cierto modo, hace que los defectos del software sean bastante previsibles y manejables, motivados por la falta de definición o, simplemente, por falta de información en el momento del desarrollo.
Así que vamos a ver cómo trabajaríamos en el caso de que se reporta un bug sobre nuestro proyecto To-do list.
Defectos en To-do list
Después de un tiempo utilizando nuestra API para generar listas de tareas, vemos que hay algunos defectos. Uno de ellos es que podemos introducir tareas sin descripción, lo que no tiene mucho sentido, ya que no nos sirve para saber qué tenemos que hacer.
Este bug es debido a que no controlamos en ningún momento que efectivamente recibimos la descripción de la tarea, es decir, estamos confiando en que el input es siempre correcto. En ese caso, caben dos posibilidades: que el payload de la petición al endpoint del POST esté completamente vacía o que el campo task venga vacío.
En esta ocasión, el comportamiento del sistema no está especificado para estas circunstancias y no está expresado en tests. En otros casos, el bug es algún tipo de error no cubierto por los tests actuales.
Así que nuestra primera aproximación será crear un test que ponga de manifiesto el bug.
Ahora bien, ¿en dónde nos interesa poner ese test? Vamos a intentar pensar un poco en esto.
Por un lado, los endpoints afectados deberían devolver una respuesta 400 (Bad Request) porque en este caso, lo que ocurre es que la request está mal construída y el endpoint no la entiende.
Según esto, tendría sentido añadir un test de aceptación. Sin embargo, también tenemos tests unitarios del controlador, que son mucho más rápidos y también nos permitirían comprobar que la respuesta tiene el código adecuado.
Por otro lado, debemos tomar en consideración qué componente se responsabiliza de validar qué cuestiones.
Así por ejemplo, si la request no trae ninguna payload o la estructura no incluye los campos requeridos, tiene sentido que el controlador sea el responsable de verificarlo y fallar en cuanto lo detecta. El test tiene sentido a nivel del controlador.
En cambio, si la request tiene la estructura correcta y se pueden encontrar los campos requeridos, puede que la validación de sus valores corresponda a capas más internas. Así, por ejemplo, si el valor de task en la payload es una cadena vacía el controlador puede intentar pasarlo al caso de uso y que sea el constructor de Task quien valide si ese valor es aceptable o no. El test, en este caso, sería en el nivel del caso de uso AddTaskHandler.
Esto abre entonces una nueva problemática para el controlador y es gestionar los errores o excepciones que vengan del caso de uso para devolver la respuesta de error adecuada.
Como se puede ver, se plantean un montón de circunstancias que nos obligan a intervenir en distintos niveles de la aplicación.
Un principio que podríamos tratar de seguir es que si falla algo en el nivel de aceptación debería tener un reflejo en el nivel unitario. El primer fallo nos indica que la aplicación tiene un defecto, mientras que el fallo en el nivel unitario nos indicaría el componente que falla.
Así que vamos a ir por partes, atacando cada uno de los problemas.
Payload inválida
El supuesto que vamos a tratar de resolver es enviar una petición al endpoint que esté mal formada o vacía. En cualquier caso no incluye el campo task.
Empezaremos por el test de aceptación y vamos a intentar reproducir el error lanzando una petición a la API con una payload inválida.
1 namespace App\Tests\Katas\TodoList;
2
3 use Symfony\Bundle\FrameworkBundle\Client;
4 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
5 use Symfony\Component\HttpFoundation\Response;
6
7 class TodoListAcceptanceTest extends WebTestCase
8 {
9 private Client $client;
10
11 // ...
12
13 /** @test */
14 public function asUserITryToAddTaskWithInvalidPayload(): void
15 {
16 $this->client->request(
17 'POST',
18 '/api/todo',
19 [],
20 [],
21 ['CONTENT-TYPE' => 'json/application'],
22 json_encode(['bad payload'], JSON_THROW_ON_ERROR)
23 );
24
25 $response = $this->client->getResponse();
26
27 self::assertEquals(400, $response->getStatusCode());
28
29 $body = json_decode($response->getContent(), true);
30
31 self::assertEquals('Invalid payload', $body['error']);
32 }
33
34 // ...
35 }
El test falla porque el endpoint devuelve un error 500 en lugar del error 400 que sería deseable en este caso. ¿Qué podemos hacer ahora?
Pues nos moveremos al nivel del controlador, para ver qué podemos hacer allí al respecto. Y tendremos que escribir otro test caracterizando la misma situación.
1 namespace App\Tests\TodoList\Infrastructure\EntryPoint\Api;
2
3 use App\TodoList\Application\AddTaskHandler;
4 use App\TodoList\Application\GetTaskListHandler;
5 use App\TodoList\Application\MarkTaskCompletedHandler;
6 use App\TodoList\Application\TaskListTransformer;
7 use App\TodoList\Infrastructure\EntryPoint\Api\TodoListController;
8 use PHPUnit\Framework\TestCase;
9 use Symfony\Component\HttpFoundation\Request;
10
11 class TodoListControllerTest extends TestCase
12 {
13 private const TASK_DESCRIPTION = 'Task Description';
14 private const COMPLETED_TASK_ID = 1;
15 private AddTaskHandler $addTaskHandler;
16 private TodoListController $todoListController;
17 private GetTaskListHandler $getTaskListHandler;
18 private TaskListTransformer $taskListTransformer;
19 private MarkTaskCompletedHandler $markTaskCompletedHandler;
20
21 protected function setUp(): void
22 {
23 $this->addTaskHandler = $this->createMock(AddTaskHandler::class\
24 );
25 $this->getTaskListHandler = $this->createMock(GetTaskListHandle\
26 r::class);
27 $this->taskListTransformer = $this->createMock(TaskListTransfor\
28 mer::class);
29 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCom\
30 pletedHandler::class);
31 $this->todoListController = new TodoListController(
32 $this->addTaskHandler,
33 $this->getTaskListHandler,
34 $this->taskListTransformer,
35 $this->markTaskCompletedHandler
36 );
37 }
38
39
40 // ...
41
42 /** @test */
43 public function shouldFailWithBadRequestIfInvalidPayload(): void
44 {
45 $request = new Request(
46 [],
47 [],
48 [],
49 [],
50 [],
51 ['CONTENT-TYPE' => 'json/application'],
52 json_encode(['invalid payload'], JSON_THROW_ON_ERROR)
53 );
54
55 $response = $this->todoListController->addTask($request);
56
57 self::assertEquals(400, $response->getStatusCode());
58
59 $body = json_decode($response->getContent(), true);
60
61 self::assertEquals('Invalid payload', $body['error']);
62 }
63
64 }
Fíjate que he eliminado que se haga alguna expectativa sobre el mock del caso de uso. En mi opinión se trata de un test que no aportaría nada en este caso y contribuye a acoplarnos a la implementación. Es cierto que eso ocurre en todos los test del controlador que tenemos hasta ahora, pero no hay razón para aumentar el acoplamiento.
Al ejecutar el test falla porque ya no encuentra el índice task cuando lo necesita en TodoListController:
1 public function addTask(Request $request): Response
2 {
3 $payload = $this->obtainPayload($request);
4
5 $this->addTaskHandler->execute($payload['task']);
6
7 return new JsonResponse('', Response::HTTP_CREATED);
8 }
Esto forma parte de la lógica del controlador por lo que no vamos a necesitar profundizar más. Si lo arreglamos aquí, habremos resuelto el problema.
Parece bastante claro que tenemos que chequear si la payload tiene la estructura correcta y responder en consecuencia:
1 namespace App\TodoList\Infrastructure\EntryPoint\Api;
2
3
4 use App\TodoList\Application\AddTaskHandler;
5 use App\TodoList\Application\GetTaskListHandler;
6 use App\TodoList\Application\MarkTaskCompletedHandler;
7 use App\TodoList\Application\TaskListTransformer;
8 use Symfony\Component\HttpFoundation\JsonResponse;
9 use Symfony\Component\HttpFoundation\Request;
10 use Symfony\Component\HttpFoundation\Response;
11
12 class TodoListController
13 {
14 private AddTaskHandler $addTaskHandler;
15 private GetTaskListHandler $getTaskListHandler;
16 private TaskListTransformer $taskListTransformer;
17 private MarkTaskCompletedHandler $markTaskCompletedHandler;
18
19 public function __construct(
20 AddTaskHandler $addTaskHandler,
21 GetTaskListHandler $getTaskListHandler,
22 TaskListTransformer $taskListTransformer,
23 MarkTaskCompletedHandler $markTaskCompletedHandler
24 ) {
25 $this->addTaskHandler = $addTaskHandler;
26 $this->getTaskListHandler = $getTaskListHandler;
27 $this->taskListTransformer = $taskListTransformer;
28 $this->markTaskCompletedHandler = $markTaskCompletedHandler;
29 }
30
31 public function addTask(Request $request): Response
32 {
33 $payload = $this->obtainPayload($request);
34
35 if (!isset($payload['task'])) {
36 return new JsonResponse(['error' => 'Invalid payload'], Res\
37 ponse::HTTP_BAD_REQUEST);
38 }
39
40 $this->addTaskHandler->execute($payload['task']);
41
42 return new JsonResponse('', Response::HTTP_CREATED);
43 }
44
45 # ...
46 }
Con el test del controlador pasando, volvemos al de aceptación. Los ejecutamos todos y vemos que también pasan perfectamente.
Y con ello hemos resuelto el bug.
Valores de negocio no válidos
Una cosa es que el payload sea incorrecto estructuralmente, validación de la que se tiene que ocupar el controlador. Sin embargo, dado un payload estructuralmente válido, en el que el controlador pueda encontrar los valores que necesita, ¿qué ocurre si esos valores no son aceptables según las reglas de negocio?
Lo que ocurre es que la responsabilidad de detectar el problema está en los objetos de dominio, los cuales lanzarán excepciones que tienen que ir subiendo hasta que algún componente pueda gestionarlas.
Por ejemplo, en el caso de Task, esperamos que tenga alguna descripción. No ha sido definido en las historias de usuario, pero es algo que damos por hecho. Puede ocurrir, entonces, que la payload nos traiga un campo task con alguno de estos valores:
null- Un tipo de dato que no sea
string - Un string vacío o demasiado corto
- Un string de longitud suficiente
Los dos primeros puntos son particularmente técnicos. A nivel del controlador podríamos validar que task en un string y fallar si no es así.
Sin embargo, los dos últimos apuntan a reglas que tiene que definir negocio. Es decir, ¿consideraríamos aceptable una tarea con una descripción de dos caracteres? Es una decisión de negocio. Supongamos que nos dicen que nos basta con un carácter para admitir una cadena como descripción válida de una tarea, así que simplemente tendremos que controlar que no es una cadena vacía.
Esta regla, en todo caso, tendría que estar en el dominio, porque se trata de una regla de negocio.
Es más discutible la ubicación de las otras dos restricciones, que se pueden resumir en que el sistema no puede aceptar task si no es un string.
Pero lo mejor es ver esto partiendo de un test, así que vamos a introducir uno. En este caso, para comprobar qué pasa si task es null:
1 namespace App\Tests\Katas\TodoList;
2
3 use Symfony\Bundle\FrameworkBundle\Client;
4 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
5 use Symfony\Component\HttpFoundation\Response;
6
7 class TodoListAcceptanceTest extends WebTestCase
8 {
9 private Client $client;
10
11 # ...
12
13 /** @test */
14 public function asUserITryToAddTaskWithABadTaskDescription(): void
15 {
16 $this->client->request(
17 'POST',
18 '/api/todo',
19 [],
20 [],
21 ['CONTENT-TYPE' => 'json/application'],
22 json_encode(['task' => null], JSON_THROW_ON_ERROR)
23 );
24
25 $response = $this->client->getResponse();
26
27 self::assertEquals(400, $response->getStatusCode());
28
29 $body = json_decode($response->getContent(), true);
30
31 self::assertEquals('Invalid payload', $body['error']);
32 }
33
34 # ...
35 }
Es muy interesante comprobar que este test pasa. Posiblemente, el arreglo del bug anterior ha evitado que nos encontremos con este.
¿Merece la pena dejar este test aquí? Yo diría que no, ya que no hemos tenido que añadir nada en el código. Lo aprovecharé para probar qué ocurriría si enviamos un dato que no sea string, como un número.
1 namespace App\Tests\Katas\TodoList;
2
3 use Symfony\Bundle\FrameworkBundle\Client;
4 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
5 use Symfony\Component\HttpFoundation\Response;
6
7 class TodoListAcceptanceTest extends WebTestCase
8 {
9 private Client $client;
10
11 # ...
12
13 /** @test */
14 public function asUserITryToAddTaskWithABadTaskDescription(): void
15 {
16 $this->client->request(
17 'POST',
18 '/api/todo',
19 [],
20 [],
21 ['CONTENT-TYPE' => 'json/application'],
22 json_encode(['task' => 12345], JSON_THROW_ON_ERROR)
23 );
24
25 $response = $this->client->getResponse();
26
27 self::assertEquals(400, $response->getStatusCode());
28
29 $body = json_decode($response->getContent(), true);
30
31 self::assertEquals('Invalid payload', $body['error']);
32 }
33
34 # ...
35 }
Este test falla por el siguiente motivo:
1 TypeError : Argument 1 passed to App\TodoList\Application\AddTaskHandle\
2 r::execute() must be of the type string, int given
Es decir. AddTaskHandler espera que pasemos un string en execute, por lo que nunca va a admitir un dato que no lo sea. Esto nos plantea un problema interesante y es si preferiríamos forzar el tipo string para la descripción de modo que si viene, como es el caso, un número lo convierta y siga adelante.
En este ejemplo vamos a suponer que no queremos eso, que el tipo ha de ser string sí o sí.
Como hemos visto, el fallo se ha producido también en el controlador, al intentar invocar el caso de uso. Por tanto, nos vamos de nuevo al test del controlador.
1 namespace App\Tests\TodoList\Infrastructure\EntryPoint\Api;
2
3 use App\TodoList\Application\AddTaskHandler;
4 use App\TodoList\Application\GetTaskListHandler;
5 use App\TodoList\Application\MarkTaskCompletedHandler;
6 use App\TodoList\Application\TaskListTransformer;
7 use App\TodoList\Infrastructure\EntryPoint\Api\TodoListController;
8 use PHPUnit\Framework\TestCase;
9 use Symfony\Component\HttpFoundation\Request;
10
11 class TodoListControllerTest extends TestCase
12 {
13 private const TASK_DESCRIPTION = 'Task Description';
14 private const COMPLETED_TASK_ID = 1;
15 private AddTaskHandler $addTaskHandler;
16 private TodoListController $todoListController;
17 private GetTaskListHandler $getTaskListHandler;
18 private TaskListTransformer $taskListTransformer;
19 private MarkTaskCompletedHandler $markTaskCompletedHandler;
20
21 protected function setUp(): void
22 {
23 $this->addTaskHandler = $this->createMock(AddTaskHandler::class\
24 );
25 $this->getTaskListHandler = $this->createMock(GetTaskListHandle\
26 r::class);
27 $this->taskListTransformer = $this->createMock(TaskListTransfor\
28 mer::class);
29 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCom\
30 pletedHandler::class);
31 $this->todoListController = new TodoListController(
32 $this->addTaskHandler,
33 $this->getTaskListHandler,
34 $this->taskListTransformer,
35 $this->markTaskCompletedHandler
36 );
37 }
38
39 # ...
40
41 /** @test */
42 public function shouldFailWithBadRequestIfInvalidTaskField(): void
43 {
44 $request = new Request(
45 [],
46 [],
47 [],
48 [],
49 [],
50 ['CONTENT-TYPE' => 'json/application'],
51 json_encode(['task' => 12345], JSON_THROW_ON_ERROR)
52 );
53
54 $response = $this->todoListController->addTask($request);
55
56 self::assertEquals(400, $response->getStatusCode());
57
58 $body = json_decode($response->getContent(), true);
59
60 self::assertEquals('Invalid payload', $body['error']);
61 }
62 }
El test del controlador falla de la misma forma que el de aceptación. A continuación implementamos lo necesario para hacerlo pasar.
1 namespace App\TodoList\Infrastructure\EntryPoint\Api;
2
3
4 use App\TodoList\Application\AddTaskHandler;
5 use App\TodoList\Application\GetTaskListHandler;
6 use App\TodoList\Application\MarkTaskCompletedHandler;
7 use App\TodoList\Application\TaskListTransformer;
8 use Symfony\Component\HttpFoundation\JsonResponse;
9 use Symfony\Component\HttpFoundation\Request;
10 use Symfony\Component\HttpFoundation\Response;
11 use function is_string;
12
13 class TodoListController
14 {
15 private AddTaskHandler $addTaskHandler;
16 private GetTaskListHandler $getTaskListHandler;
17 private TaskListTransformer $taskListTransformer;
18 private MarkTaskCompletedHandler $markTaskCompletedHandler;
19
20 public function __construct(
21 AddTaskHandler $addTaskHandler,
22 GetTaskListHandler $getTaskListHandler,
23 TaskListTransformer $taskListTransformer,
24 MarkTaskCompletedHandler $markTaskCompletedHandler
25 ) {
26 $this->addTaskHandler = $addTaskHandler;
27 $this->getTaskListHandler = $getTaskListHandler;
28 $this->taskListTransformer = $taskListTransformer;
29 $this->markTaskCompletedHandler = $markTaskCompletedHandler;
30 }
31
32 public function addTask(Request $request): Response
33 {
34 $payload = $this->obtainPayload($request);
35
36 if (!isset($payload['task'])) {
37 return new JsonResponse(['error' => 'Invalid payload'], Res\
38 ponse::HTTP_BAD_REQUEST);
39 }
40
41 if (!is_string($payload['task'])) {
42 return new JsonResponse(['error' => 'Invalid payload'], Res\
43 ponse::HTTP_BAD_REQUEST);
44 }
45
46 $this->addTaskHandler->execute($payload['task']);
47
48 return new JsonResponse('', Response::HTTP_CREATED);
49 }
50
51 # ...
52 }
Este cambio hace pasar el test, y es de esperar que también el de aceptación, así que lo comprobamos. Recuerda que es muy importante pasar todos los tests, no solo el específico para el caso, ya que hay que asegurarse de que los cambios introducidos no alteran el comportamiento actual del sistema.
El test de aceptación pasa también. Ahora tenemos que pensar un par de cosas.
Por un lado, el código que hemos introducido es un poco feo y nos distrae del propósito del controlador. Necesitamos hacer un refactor que despeje un poco las cosas.
Esta es una posible solución:
1 namespace App\TodoList\Infrastructure\EntryPoint\Api;
2
3
4 use App\TodoList\Application\AddTaskHandler;
5 use App\TodoList\Application\GetTaskListHandler;
6 use App\TodoList\Application\MarkTaskCompletedHandler;
7 use App\TodoList\Application\TaskListTransformer;
8 use Symfony\Component\HttpFoundation\JsonResponse;
9 use Symfony\Component\HttpFoundation\Request;
10 use Symfony\Component\HttpFoundation\Response;
11 use function is_string;
12
13 class TodoListController
14 {
15 private AddTaskHandler $addTaskHandler;
16 private GetTaskListHandler $getTaskListHandler;
17 private TaskListTransformer $taskListTransformer;
18 private MarkTaskCompletedHandler $markTaskCompletedHandler;
19
20 public function __construct(
21 AddTaskHandler $addTaskHandler,
22 GetTaskListHandler $getTaskListHandler,
23 TaskListTransformer $taskListTransformer,
24 MarkTaskCompletedHandler $markTaskCompletedHandler
25 ) {
26 $this->addTaskHandler = $addTaskHandler;
27 $this->getTaskListHandler = $getTaskListHandler;
28 $this->taskListTransformer = $taskListTransformer;
29 $this->markTaskCompletedHandler = $markTaskCompletedHandler;
30 }
31
32 public function addTask(Request $request): Response
33 {
34 $payload = $this->obtainPayload($request);
35
36 if (!$this->isValidPayload($payload)) {
37 return new JsonResponse(['error' => 'Invalid payload'], Res\
38 ponse::HTTP_BAD_REQUEST);
39 }
40
41 $this->addTaskHandler->execute($payload['task']);
42
43 return new JsonResponse('', Response::HTTP_CREATED);
44 }
45
46 # ...
47
48 private function isValidPayload(array $payload): bool
49 {
50 return isset($payload['task']) && is_string($payload['task']);
51 }
52 }
Es una primera aproximación. Podríamos avanzar más en ella, pero de momento es suficiente.
La otra cuestión es la siguiente. Los tests de aceptación que hemos añadido nos han servido para reproducir los bugs y guiarnos en la solución. Sin embargo, en el nivel unitario del controlador hemos hecho prácticamente el mismo test. Es más, el test de aceptación verifica exactamente el comportamiento que el test del controlador, ya que es este último el que tiene toda la responsabilidad sobre ese comportamiento.
Esta duplicación no siempre es útil. De hecho, para negocio, a quien interesa el test de aceptación, no le preocupa demasiado el tipo de problemas técnicos que estamos verificando. En cambio, en el nivel unitario este tipo de detalles es más relevante.
Así que en caso de tener tests en el nivel de aceptación que son idénticos a otros en el nivel unitario, es preferible eliminar los de aceptación si no aportan valor a negocio, teniendo cubierta la misma circunstancia en los unitarios.
Así que nosotros vamos a eliminar esos tests antes de continuar.
Garantizando reglas de dominio
Nuestro siguiente test verificará que una descripción vacía, aunque sea un string, no generará una tarea nueva. Lo pondremos en el test de aceptación:
1 namespace App\Tests\Katas\TodoList;
2
3 use Symfony\Bundle\FrameworkBundle\Client;
4 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
5 use Symfony\Component\HttpFoundation\Response;
6
7 class TodoListAcceptanceTest extends WebTestCase
8 {
9 private Client $client;
10
11 # ...
12
13 /** @test */
14 public function asUserITryToAddTaskWithAnEmptyTaskDescription(): vo\
15 id
16 {
17 $this->client->request(
18 'POST',
19 '/api/todo',
20 [],
21 [],
22 ['CONTENT-TYPE' => 'json/application'],
23 json_encode(['task' => ''], JSON_THROW_ON_ERROR)
24 );
25
26 $response = $this->client->getResponse();
27
28 self::assertEquals(400, $response->getStatusCode());
29
30 $body = json_decode($response->getContent(), true);
31
32 self::assertEquals('Task description should not be empty', $bod\
33 y['error']);
34 }
35
36 # ...
37 }
Este test ya indica una violación de una regla de negocio. Este es el error que genera:
1 Failed asserting that 201 matches expected 400.
2 Expected :400
3 Actual :201
Nos indica que se estarían creando tareas con la descripción vacía como si fuesen válidas. Tenemos que profundizar en la aplicación para ver dónde deberíamos controlar el error.
Así que vamos al controlador. Pero este no tiene que saber nada de las reglas del negocio, ya que está en la capa de Infraestructura. Sin embargo, tiene que encargarse de dar la respuesta HTTP adecuada y eso solo es posible si el caso de uso le comunica de alguna forma que ha habido problemas.
La mejor forma que tiene de hacerlo es lanzando una excepción que el controlador capturará devolviendo una respuesta adecuada.
De este modo, el test del controlador podría quedar así:
1 namespace App\Tests\TodoList\Infrastructure\EntryPoint\Api;
2
3 use App\TodoList\Application\AddTaskHandler;
4 use App\TodoList\Application\GetTaskListHandler;
5 use App\TodoList\Application\MarkTaskCompletedHandler;
6 use App\TodoList\Application\TaskListTransformer;
7 use App\TodoList\Infrastructure\EntryPoint\Api\TodoListController;
8 use InvalidArgumentException;
9 use PHPUnit\Framework\TestCase;
10 use Symfony\Component\HttpFoundation\Request;
11
12 class TodoListControllerTest extends TestCase
13 {
14 private const TASK_DESCRIPTION = 'Task Description';
15 private const COMPLETED_TASK_ID = 1;
16 private AddTaskHandler $addTaskHandler;
17 private TodoListController $todoListController;
18 private GetTaskListHandler $getTaskListHandler;
19 private TaskListTransformer $taskListTransformer;
20 private MarkTaskCompletedHandler $markTaskCompletedHandler;
21
22 protected function setUp(): void
23 {
24 $this->addTaskHandler = $this->createMock(AddTaskHandler::class\
25 );
26 $this->getTaskListHandler = $this->createMock(GetTaskListHandle\
27 r::class);
28 $this->taskListTransformer = $this->createMock(TaskListTransfor\
29 mer::class);
30 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCom\
31 pletedHandler::class);
32 $this->todoListController = new TodoListController(
33 $this->addTaskHandler,
34 $this->getTaskListHandler,
35 $this->taskListTransformer,
36 $this->markTaskCompletedHandler
37 );
38 }
39
40 # ...
41
42 /** @test */
43 public function shouldFailWithBadRequestIfTaskDescriptionIsEmpty():\
44 void
45 {
46 $exceptionMessage = 'Task description should not be empty';
47 $exception = new InvalidArgumentException($exceptionMessage);
48
49 $this->addTaskHandler
50 ->method('execute')
51 ->willThrowException($exception)
52 ->with('');
53
54
55 $request = new Request(
56 [],
57 [],
58 [],
59 [],
60 [],
61 ['CONTENT-TYPE' => 'json/application'],
62 json_encode(['task' => ''], JSON_THROW_ON_ERROR)
63 );
64
65 $response = $this->todoListController->addTask($request);
66
67 self::assertEquals(400, $response->getStatusCode());
68
69 $body = json_decode($response->getContent(), true);
70
71 self::assertEquals($exceptionMessage, $body['error']);
72 }
73 }
Como se puede ver, simulamos que el caso de uso lanza una excepción y el test falla porque no se captura y, por tanto, no se devuelve una respuesta adecuada. Para no complicar la solución no voy a crear una excepción de dominio, cosa que haría en un proyecto real.
Hagamos pasar el test.
1 namespace App\TodoList\Infrastructure\EntryPoint\Api;
2
3
4 use App\TodoList\Application\AddTaskHandler;
5 use App\TodoList\Application\GetTaskListHandler;
6 use App\TodoList\Application\MarkTaskCompletedHandler;
7 use App\TodoList\Application\TaskListTransformer;
8 use InvalidArgumentException;
9 use Symfony\Component\HttpFoundation\JsonResponse;
10 use Symfony\Component\HttpFoundation\Request;
11 use Symfony\Component\HttpFoundation\Response;
12 use function is_string;
13
14 class TodoListController
15 {
16 private AddTaskHandler $addTaskHandler;
17 private GetTaskListHandler $getTaskListHandler;
18 private TaskListTransformer $taskListTransformer;
19 private MarkTaskCompletedHandler $markTaskCompletedHandler;
20
21 public function __construct(
22 AddTaskHandler $addTaskHandler,
23 GetTaskListHandler $getTaskListHandler,
24 TaskListTransformer $taskListTransformer,
25 MarkTaskCompletedHandler $markTaskCompletedHandler
26 ) {
27 $this->addTaskHandler = $addTaskHandler;
28 $this->getTaskListHandler = $getTaskListHandler;
29 $this->taskListTransformer = $taskListTransformer;
30 $this->markTaskCompletedHandler = $markTaskCompletedHandler;
31 }
32
33 public function addTask(Request $request): Response
34 {
35 $payload = $this->obtainPayload($request);
36
37 if (!$this->isValidPayload($payload)) {
38 return new JsonResponse(['error' => 'Invalid payload'], Res\
39 ponse::HTTP_BAD_REQUEST);
40 }
41
42 try {
43 $this->addTaskHandler->execute($payload['task']);
44 } catch (InvalidArgumentException $invalidTaskDescription) {
45 return new JsonResponse(['error' => $invalidTaskDescription\
46 ->getMessage()], Response::HTTP_BAD_REQUEST);
47 }
48
49 return new JsonResponse('', Response::HTTP_CREATED);
50 }
51
52 # ...
53 }
El test unitario del controlador ha pasado. Sin embargo, el test de aceptación no lo hace. Esto es porque el caso de uso no lo hemos tocado todavía. Tenemos que descender un poco más y hacer un test que pueda fallar para que nos diga qué implementar.
Pero primero, examinemos el código:
1 namespace App\TodoList\Application;
2
3 use App\TodoList\Domain\Task;
4 use App\TodoList\Domain\TaskRepository;
5
6 class AddTaskHandler
7 {
8 /** @var TaskRepository */
9 private TaskRepository $taskRepository;
10
11 public function __construct(TaskRepository $taskRepository)
12 {
13 $this->taskRepository = $taskRepository;
14 }
15
16 public function execute(string $taskDescription): void
17 {
18 $id = $this->taskRepository->nextIdentity();
19
20 $task = new Task($id, $taskDescription);
21
22 $this->taskRepository->store($task);
23 }
24 }
Como se puede ver, el punto en que se debería lanzar la excepción es cuando se crea una tarea (Task), pero no hay razón para que sea el caso de uso quien verifique que $taskDescription tiene una longitud suficiente.
En su lugar, tiene sentido que esta lógica esté en Task. Al fin y al cabo, el caso de uso no es un lugar para aplicar reglas de negocio, sino para coordinar objetos de dominio que son quienes tienen la responsabilidad de mantenerlas.
Así que tendríamos que ir un poco más adentro y modificar el test de Task para que garantice que siempre se construye de forma consistente, con una descripción que tiene al menos un carácter. En caso de que la descripción esté vacía, lanzaremos la excepción.
1 namespace App\Tests\TodoList\Domain;
2
3 use App\TodoList\Domain\Task;
4 use PHPUnit\Framework\TestCase;
5 use InvalidArgumentException;
6
7 class TaskTest extends TestCase
8 {
9 /** @test */
10 public function shouldNotAllowEmptyDescription(): void
11 {
12 $this->expectException(InvalidArgumentException::class);
13
14 new Task(1, '');
15 }
16
17 /** @test */
18 public function shouldProvideRepresentation(): void
19 {
20 $expected = '[ ] 1. Task Description';
21 $task = new Task(1, 'Task Description');
22
23 $representation = $task->representedAs('[:check] :id. :descript\
24 ion');
25
26 self::assertEquals($expected, $representation);
27 }
28
29 /** @test */
30 public function shouldMarkTaskCompleted(): void
31 {
32 $expected = '[√] 1. Task Description';
33 $task = new Task(1, 'Task Description');
34 $task->markCompleted();
35
36 $representation = $task->representedAs('[:check] :id. :descript\
37 ion');
38
39 self::assertEquals($expected, $representation);
40 }
41 }
Ahora quedaría implementarlo.
1 namespace App\TodoList\Domain;
2
3 use InvalidArgumentException;
4
5 class Task
6 {
7 private int $id;
8 private string $description;
9 private bool $completed;
10
11 public function __construct(int $id, string $description)
12 {
13 $this->id = $id;
14
15 if ($description === '') {
16 $exceptionMessage = 'Task description should not be empty';
17 throw new InvalidArgumentException($exceptionMessage);
18 }
19
20 $this->description = $description;
21 $this->completed = false;
22 }
23
24 public function id(): int
25 {
26 return $this->id;
27 }
28
29 public function representedAs(string $format): string
30 {
31 $values = [
32 ':check' => $this->completed ? '√' : ' ',
33 ':id' => $this->id,
34 ':description' => $this->description
35 ];
36 return strtr($format, $values);
37
38 }
39
40 public function markCompleted(): void
41 {
42 $this->completed = true;
43 }
44 }
Esta implementación es suficiente para que pase el test unitario. Veamos si también pasan los tests en los demás niveles. Así es, todos los tests unitarios siguen pasando y también lo hace el test de aceptación.
El test de aceptación que acabamos de introducir lo dejaremos ahí porque tiene significado de negocio. Además, el test del controlador en este caso solo verifica que este es capaz de gestionar la excepción lanzada por la capa de dominio, mientras que el test de Task verifica que esta se tiene que construir con una descripción adecuada.
Tareas no encontradas
Otro defecto de nuestra aplicación tiene que ver con intentar marcar como completada una tarea que no existe. Actualmente, el endpoint devolverá un error 500, cuando lo correcto sería un 404 indicando que el recurso que se quiere modificar no existe.
El siguiente test de aceptación lo pone de manifiesto:
1 namespace App\Tests\Katas\TodoList;
2
3 use Symfony\Bundle\FrameworkBundle\Client;
4 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
5 use Symfony\Component\HttpFoundation\Response;
6
7 class TodoListAcceptanceTest extends WebTestCase
8 {
9 private Client $client;
10
11 # ...
12
13 /** @test */
14 public function asUserITryToMarkNotExistentTasksAsCompleted(): void
15 {
16 $this->givenIHaveAddedTasks(
17 [
18 'Write a test that fails',
19 'Write code to make the test pass',
20 ]
21 );
22
23 $response = $this->apiMarkTaskCompleted(3);
24
25 self::assertEquals(404, $response->getStatusCode());
26 }
27
28 # ...
29 }
El resultado es:
1 Failed asserting that 500 matches expected 404.
Además de que hay un error en:
1 "Undefined offset: 3" at /application/src/TodoList/Infrastructure/Persi\
2 stence/FileTaskRepository.php
El error de base se produce en el repositorio. Sin embargo, vamos a proceder sistemáticamente. Como hemos visto en el ejemplo anterior, el error puede manifestarse de distintas maneras en las diferentes capas o niveles de la aplicación, por lo que tenemos que ir paso por paso, decidir si ese error tiene que manifestarse de algún modo e implementar el comportamiento necesario.
El controlador, como ya hemos visto anteriormente, es responsable de interpretar el problema y expresarlo con un error 404 en la respuesta. Por tanto, espera que el caso de uso le comunique eso con una excepción. En la práctica, significa que ese controlador tiene que reaccionar a una excepción determinada que lanzará o relanzará el caso de uso.
Así que expresamos eso en un test.
1 namespace App\Tests\TodoList\Infrastructure\EntryPoint\Api;
2
3 use App\TodoList\Application\AddTaskHandler;
4 use App\TodoList\Application\GetTaskListHandler;
5 use App\TodoList\Application\MarkTaskCompletedHandler;
6 use App\TodoList\Application\TaskListTransformer;
7 use App\TodoList\Infrastructure\EntryPoint\Api\TodoListController;
8 use InvalidArgumentException;
9 use PHPUnit\Framework\TestCase;
10 use OutOfBoundsException;
11 use Symfony\Component\HttpFoundation\Request;
12
13 class TodoListControllerTest extends TestCase
14 {
15 private const TASK_DESCRIPTION = 'Task Description';
16 private const COMPLETED_TASK_ID = 1;
17 private AddTaskHandler $addTaskHandler;
18 private TodoListController $todoListController;
19 private GetTaskListHandler $getTaskListHandler;
20 private TaskListTransformer $taskListTransformer;
21 private MarkTaskCompletedHandler $markTaskCompletedHandler;
22
23 protected function setUp(): void
24 {
25 $this->addTaskHandler = $this->createMock(AddTaskHandler::class\
26 );
27 $this->getTaskListHandler = $this->createMock(GetTaskListHandle\
28 r::class);
29 $this->taskListTransformer = $this->createMock(TaskListTransfor\
30 mer::class);
31 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCom\
32 pletedHandler::class);
33 $this->todoListController = new TodoListController(
34 $this->addTaskHandler,
35 $this->getTaskListHandler,
36 $this->taskListTransformer,
37 $this->markTaskCompletedHandler
38 );
39 }
40
41 # ...
42
43 /** @test */
44 public function shouldFailWithNotFoundIdCompletingNotExistentTask()\
45 : void
46 {
47 $exceptionMessage = 'Task 3 doesn\'t exist';
48 $exception = new OutOfBoundsException($exceptionMessage);
49
50 $this->markTaskCompletedHandler
51 ->method('execute')
52 ->willThrowException($exception);
53
54 $request = new Request(
55 [],
56 [],
57 [],
58 [],
59 [],
60 ['CONTENT-TYPE' => 'json/application'],
61 json_encode(['completed' => true], JSON_THROW_ON_ERROR)
62 );
63
64 $response = $this->todoListController->markTaskCompleted(self::\
65 COMPLETED_TASK_ID, $request);
66
67 self::assertEquals(400, $response->getStatusCode());
68
69 $body = json_decode($response->getContent(), true);
70
71 self::assertEquals($exceptionMessage, $body['error']);
72 }
73
74 }
Como era de esperar, el test del controlador fallará porque no se captura la excepción simulada en el mock. Nos toca implementar el código para hacerlo:
1 namespace App\TodoList\Infrastructure\EntryPoint\Api;
2
3 use App\TodoList\Application\AddTaskHandler;
4 use App\TodoList\Application\GetTaskListHandler;
5 use App\TodoList\Application\MarkTaskCompletedHandler;
6 use App\TodoList\Application\TaskListTransformer;
7 use InvalidArgumentException;
8 use OutOfBoundsException;
9 use Symfony\Component\HttpFoundation\JsonResponse;
10 use Symfony\Component\HttpFoundation\Request;
11 use Symfony\Component\HttpFoundation\Response;
12 use function is_string;
13
14 class TodoListController
15 {
16 private AddTaskHandler $addTaskHandler;
17 private GetTaskListHandler $getTaskListHandler;
18 private TaskListTransformer $taskListTransformer;
19 private MarkTaskCompletedHandler $markTaskCompletedHandler;
20
21 public function __construct(
22 AddTaskHandler $addTaskHandler,
23 GetTaskListHandler $getTaskListHandler,
24 TaskListTransformer $taskListTransformer,
25 MarkTaskCompletedHandler $markTaskCompletedHandler
26 ) {
27 $this->addTaskHandler = $addTaskHandler;
28 $this->getTaskListHandler = $getTaskListHandler;
29 $this->taskListTransformer = $taskListTransformer;
30 $this->markTaskCompletedHandler = $markTaskCompletedHandler;
31 }
32
33 # ...
34
35 public function markTaskCompleted(int $taskId, Request $request): R\
36 esponse
37 {
38 $payload = $this->obtainPayload($request);
39
40 try {
41 $this->markTaskCompletedHandler->execute($taskId, $payload[\
42 'completed']);
43 } catch (OutOfBoundsException $taskNotFound) {
44 return new JsonResponse(['error' => $taskNotFound->getMessa\
45 ge()], Response::HTTP_NOT_FOUND);
46 }
47
48 return new JsonResponse('', Response::HTTP_OK);
49 }
50
51 # ...
52 }
Dado que ahora el test pasa en el controlador, volvemos al nivel de aceptación. Este nivel todavía falla porque, de hecho, no se lanza realmente ninguna excepción en el flujo de esta acción.
Necesitamos ir un poco más adentro.
Si examinamos la capa de Aplicación, el caso de uso tiene poco que hacer. Al igual que en el problema anterior, su trabajo es delegar en objetos de dominio, por lo que son estos los que deben fallar. Como he señalado antes, estoy usando excepciones genéricas, pero en proyectos reales usaríamos también excepciones del dominio en distintos niveles. Por ejemplo, una TaskNotFound que perfectamente podría extender de OutOfBoundsException.
Así que no haremos nada en el caso de uso, pero observamos que el responsable de decir que una tarea no existe será el repositorio. La primera línea del método execute es clara.
1 namespace App\TodoList\Application;
2
3
4 use App\TodoList\Domain\TaskRepository;
5
6 class MarkTaskCompletedHandler
7 {
8 private TaskRepository $taskRepository;
9
10 public function __construct(TaskRepository $taskRepository)
11 {
12 $this->taskRepository = $taskRepository;
13 }
14
15 public function execute(int $taskId, bool $completed): void
16 {
17 $task = $this->taskRepository->retrieve($taskId);
18
19 if ($completed) {
20 $task->markCompleted();
21 }
22
23 $this->taskRepository->store($task);
24 }
25 }
No merece la pena escribir un test que simule que el repositorio lanza una excepción y el caso de uso no hace nada. Si el caso de uso capturase excepciones que vienen de un nivel más interno para relanzarlas como una excepción distinta, una técnica que podríamos denominar de anidado de excepciones, entonces sí lo haríamos para verificar eso.
Por tanto, nos vamos al nivel del repositorio y hacemos un test:
1 namespace App\Tests\TodoList\Infrastructure\Persistence;
2
3 use App\Lib\FileStorageEngine;
4 use App\TodoList\Domain\Task;
5 use App\TodoList\Domain\TaskRepository;
6 use App\TodoList\Infrastructure\Persistence\FileTaskRepository;
7 use PHPUnit\Framework\TestCase;
8 use Symfony\Component\DependencyInjection\Exception\OutOfBoundsExceptio\
9 n;
10
11 class FileTaskRepositoryTest extends TestCase
12 {
13 private FileStorageEngine $fileStorageEngine;
14 private TaskRepository $taskRepository;
15
16 public function setUp(): void
17 {
18 $this->fileStorageEngine = $this->createMock(FileStorageEngine:\
19 :class);
20 $this->taskRepository = new FileTaskRepository($this->fileStora\
21 geEngine);
22 }
23
24 # ...
25
26 /** @test */
27 public function shouldFailIfTaskNotFound(): void
28 {
29 $storedTasks = [
30 1 => new Task(1, 'Write a test that fails'),
31 2 => new Task(2, 'Write code to make the test pass'),
32 ];
33
34 $this->fileStorageEngine
35 ->method('loadObjects')
36 ->willReturn(
37 $storedTasks
38 );
39
40 $this->expectException(OutOfBoundsException::class);
41 $this->taskRepository->retrieve(3);
42 }
43 }
El test falla, puesto que no se lanza la excepción. Así que vamos al código de producción:
1 namespace App\TodoList\Infrastructure\Persistence;
2
3
4 use App\Lib\FileStorageEngine;
5 use App\TodoList\Domain\Task;
6 use App\TodoList\Domain\TaskRepository;
7 use OutOfBoundsException;
8
9 class FileTaskRepository implements TaskRepository
10 {
11 private FileStorageEngine $fileStorageEngine;
12
13 public function __construct(FileStorageEngine $fileStorageEngine)
14 {
15 $this->fileStorageEngine = $fileStorageEngine;
16 }
17
18 # ...
19
20 public function retrieve(int $taskId): Task
21 {
22 $tasks = $this->findAll();
23
24 if (!isset($tasks[$taskId])) {
25 throw new OutOfBoundsException(
26 sprintf('Task %s doesn\'t exist', $taskId)
27 );
28 }
29
30 return $tasks[$taskId];
31 }
32
33 # ...
34 }
Con esta sencilla solución hacemos pasar el test. Ejecutamos de nuevo el test de aceptación para ver si el problema está resuelto.
Resolviendo defectos
Una conclusión interesante sobre lo que acabamos de hacer es que, en realidad, resolver bugs no es más que implementar un comportamiento inexistente en el software. En este contexto, prefiero usar el término defecto en lugar de bug.
Es más, se podría argumentar que este capítulo más que tratar de resolución de bugs, trata de añadir características al software que se habían dejado atrás de manera consciente o no. En la vida real, este tipo de cosas se suele reportar como bug, aunque nosotros sepamos que es una feature no desarrollada todavía o que no tuvimos en cuenta en su momento.
De hecho, al desarrollar el software usando TDD normalmente evitamos el tipo de defectos que se asocia habitualmente con bugs, como podría ser un problema de tipado o algún despiste en el código que el lenguaje, por la razón que sea, permita que pase desapercibido.
En cualquier caso, el procedimiento es más o menos el siguiente:
- Lo primero es reproducir el bug mediante un test en el nivel más externo posible. Lo más seguro es que se manifieste en el test de aceptación y es lo esperable si es un problema detectado por las usuarias. Pero eso puede depender del contexto también.
- Seguidamente, vamos al siguiente nivel de la aplicación, intentando reproducir el mismo bug con un test unitario. Habrá niveles en que esto no sea posible y el test pase. Como hemos visto en el último ejemplo, la manifestación del bug en cada nivel puede ser diferente o puede no darse incluso. Si no podemos demostrar el bug en ese nivel, nos vamos al siguiente.
- En cada nivel, implementamos código para hacer que el test que demuestra el bug pase. Una vez que los tests de ese nivel están en verde, volvemos al test de aceptación.
- Si el test de aceptación continúa sin pasar, tendremos que ir un nivel más adentro en la aplicación, crear un test que describa el bug y resolverlo. Después de eso, volvemos al nivel de aceptación hasta que el test vuelva a pasar.
- En el momento en que el test de aceptación pase, es que ya está resuelto el defecto.