Fixing bugs with TDD
In our to-do list project, we’ve developed what we could call the application’s happy path. That is, we’ve assumed that, when trying to create a task, the consumer of the API wouldn’t make mistakes such as trying to create a task without a name. Another assumption is that, when marking a task as completed, the system will only use ids from existing tasks on the list.
This means that the application might fail if any of these assumptions isn’t met. Is this a bug? In a sense, yes, but we could also argue that they’re just features that haven’t been implemented yet.
When we develop using TDD we can prevent many defects due to faulty implementations. For example, imagine that we haven’t implemented a Task::markComplete method, and we don’t have any test that tries to call it. The result will be a bug.
Of course, if we haven’t written a test to verify a specific behavior, as it could be preventing the creation of tasks without description, the bug will end up surfacing.
It’s said that neither testing nor TDD rid us of every fault in a piece of software. Nevertheless, I think that we can rest assured that by using TDD, the defects will appear in those parts that aren’t covered by a test. If we look at them through this lens, therefore, bugs are actually unforeseen circumstances and unimplemented cases. This is as interesting as it is liberating, because in a certain sense, it makes the software flaws quite predictable and manageable, motivated by a lack of definition, or simply by a lack of information at the time of development.
So, let’s see how we would proceed in the case that a bug about our to-do list project was reported.
To-do list bugs
After some time using our API to generate task lists, we find a few defects. One of them is that we’re able to enter tasks that don’t have a description, which doesn’t make much sense, as it’s not very useful if we want to know what we have to do.
This bug is due to the fact that we’re not controlling at any point that we’re actually receiving the description of the task, that is, we’re trusting that the input is always correct. In that case, there’s two possibilities: that the payload of the request to the POST endpoint is completely empty, or that the task field is blank.
In this occasion, the system doesn’t have any specified behavior for these circumstances, nor is it expressed in the tests. In other cases, the bug is some kind of error not covered by the current tests.
So our first approach will be to create a test that exposes the bug.
Now then, where do we want to put that test? Let’s try and think a little about this.
On the one hand, the affected endpoints should return a 400 response (Bad Request) because in this case, that happens is that the request is badly constructed and the endpoint doesn’t understand it.
According to this, it would make sense to add an acceptance test. However, we also have the controller’s unit tests, which are much faster and would also allow us to verify that the response has the correct code.
On the other hand, we have to take into consideration which component is responsible of validating which questions.
So, for example, if the request doesn’t have any payload or the structure doesn’t include the required fields, it makes sense for the controller to be responsible of verifying it and failing as soon as it detects it. The test makes sense at the controller level.
However, if the request has the correct structure and the required fields can be found, the validation of its values may correspond to more inner layers. Thus, for example, if the value of task in the payload is an empty string, the controller may try to pass it to the use case, and let the Task constructor validate whether that value is acceptable or not. The test, in this case, would be at the AddTaskHandler use case level.
This, however, opens up a new predicament for the controller, and that is handling the errors or exceptions that come from the use case in order to return the appropriate error response.
As you can see, a bunch of circumstances arise and force us to intervene at different levels of the application.
One principle that we could try following is that if something goes wrong at the acceptance level, it should be reflected at the unit level. The first error tells us that the application has a defect, while the error at the unit level tells us which one is the component that’s failing.
So, let’s go bit by bit, tackling each of the problems.
Invalid payload
The case that we’ll be trying to handle and solve is sending an empty or badly formed request to the endpoint. In any case, it doesn’t include the task field.
We’ll start with the acceptance test, and we’ll try to reproduce the error by launching a request to the API with an invalid payload.
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 }
The test fails because the endpoint returns a 500 error instead of the 400 error that would be desireable in this case. What can we do now?
Well, we’ll move to the controller level to see what can we do there. And we’ll have to write another test characterizing the same situation.
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 $this->getTaskListHandler = $this->createMock(GetTaskListHandler::class);
25 $this->taskListTransformer = $this->createMock(TaskListTransformer::class);
26 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCompletedHandler\
27 ::class);
28 $this->todoListController = new TodoListController(
29 $this->addTaskHandler,
30 $this->getTaskListHandler,
31 $this->taskListTransformer,
32 $this->markTaskCompletedHandler
33 );
34 }
35
36
37 // ...
38
39 /** @test */
40 public function shouldFailWithBadRequestIfInvalidPayload(): void
41 {
42 $request = new Request(
43 [],
44 [],
45 [],
46 [],
47 [],
48 ['CONTENT-TYPE' => 'json/application'],
49 json_encode(['invalid payload'], JSON_THROW_ON_ERROR)
50 );
51
52 $response = $this->todoListController->addTask($request);
53
54 self::assertEquals(400, $response->getStatusCode());
55
56 $body = json_decode($response->getContent(), true);
57
58 self::assertEquals('Invalid payload', $body['error']);
59 }
60
61 }
Note that I’ve removed the expectations about the mock of the use case. In my opinion, it’s a test that wouldn’t add anything in this case, and contributes to the coupling to the implementation. It’s true that the same happens in all of the controller tests that we have so far, but there’s no reason to deliberately increase the coupling if we can avoid it.
When we run the test, it fails because it can no longer find the task index when it needs it in 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 }
This is part of the controller’s logic, so we’re not going to need to go any deeper. If we fix it here, we’ll have solved the problem.
It seems pretty clear that we have to check is the payload has the correct structure, and respond consequently:
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'], Response::HTTP_B\
37 AD_REQUEST);
38 }
39
40 $this->addTaskHandler->execute($payload['task']);
41
42 return new JsonResponse('', Response::HTTP_CREATED);
43 }
44
45 # ...
46 }
Having passed the controller test, we return to the acceptance one. We run them all, and we check that they also pass perfectly.
And with that, we’ve solved the bug.
Invalid business values
One thing is that the payload is incorrect structurally, a validation that corresponds to the controller. However, given a structurally valid payload in which the controller is able to find all of the fields it needs, what happens if the values aren’t acceptable according to the business rules?
What happens is that the responsibility of detecting the problem lies in the domain objects, which will throw exceptions that should bubble up the application until some component is able to handle them.
For example, in the case of Task, we expect it to have a description. It hasn’t been defined in the user stories, but it’s something that we just assume. It may happen, then, that the payload comes with a task field containing any of these values:
null- A data type that isn’t
string - An empty or too short string
- A string of sufficient length
The first two points are particularly technical. At the controller level, we could validate that task is a string and fail if it isn’t.
However, the last two point to rules that must be defined by the business. That is to say, would we consider acceptable a task with a two-character description? It’s a business decision. Let’s assume that we’re told that one character is enough to accept a string as a valid description, so we’ll only have to control that it’s not an empty string.
This rule, in any case, belongs to the domain because it’s a business rule.
The location of the previous two restrictions is more open to debate, which may be summarizes as “the system cannot accept task if it’s not a string”.
But it’s best to see this from the point of view of a test, so let’s introduce one. In this case, to see what happens if task is 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 }
It’s very interesting to check that this test passes. Probably, fixing the previous bug has prevented us from running into this one.
Is it worth it to leave this test here? I’d say no, as we haven’t had to add anything to the code. I’ll make use of it by trying to see what would happen if we sent non-string data, like a number.
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 }
This test fails for the following reason:
1 TypeError : Argument 1 passed to App\TodoList\Application\AddTaskHandler::execute() \
2 must be of the type string, int given
That is. AddTaskHandler expects us to pass a string in execute, so it’s never going to admit a value that isn’t one. This poses an interesting problem: are we interested in forcing the string type for the description, so that if a number comes -as is the case-, it converts it and moves on?
In this example we’re going to assume that we don’t want that, that the type must be string no matter what.
As we’ve seen, the failure has also been produced in the controller, when trying to invoke the use case. Thereby, we go back to the controller 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 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 $this->getTaskListHandler = $this->createMock(GetTaskListHandler::class);
25 $this->taskListTransformer = $this->createMock(TaskListTransformer::class);
26 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCompletedHandler\
27 ::class);
28 $this->todoListController = new TodoListController(
29 $this->addTaskHandler,
30 $this->getTaskListHandler,
31 $this->taskListTransformer,
32 $this->markTaskCompletedHandler
33 );
34 }
35
36 # ...
37
38 /** @test */
39 public function shouldFailWithBadRequestIfInvalidTaskField(): void
40 {
41 $request = new Request(
42 [],
43 [],
44 [],
45 [],
46 [],
47 ['CONTENT-TYPE' => 'json/application'],
48 json_encode(['task' => 12345], JSON_THROW_ON_ERROR)
49 );
50
51 $response = $this->todoListController->addTask($request);
52
53 self::assertEquals(400, $response->getStatusCode());
54
55 $body = json_decode($response->getContent(), true);
56
57 self::assertEquals('Invalid payload', $body['error']);
58 }
59 }
The controller test fails in the same way as the acceptance test. Next, we implement what’s necessary to make it pass.
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'], Response::HTTP_B\
38 AD_REQUEST);
39 }
40
41 if (!is_string($payload['task'])) {
42 return new JsonResponse(['error' => 'Invalid payload'], Response::HTTP_B\
43 AD_REQUEST);
44 }
45
46 $this->addTaskHandler->execute($payload['task']);
47
48 return new JsonResponse('', Response::HTTP_CREATED);
49 }
50
51 # ...
52 }
This change makes the test pass, and it’s to be expected that it also makes the acceptance one pass, so we check it. Remember: it’s very important to pass all the tests every time, not only the one specific to the case, as we have to make sure that the changes that we introduce don’t alter the current behavior of the system.
The acceptance test also passes. Now we have to think about a couple of things.
On the one hand, the code that we’ve introduced is a little ugly, and it distracts us from the controller’s purpose. We need to refactor and clear things up a bit.
This is one possible solution:
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'], Response::HTTP_B\
38 AD_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 }
It’s a first approach. We could advance it further, but it’s enough for now.
The other issue is the following. The acceptance tests that we’ve added have been useful to reproduce the bugs and guide us towards their solution. However, at the controller’s unit level, we’ve made practically the same test. What’s more, the acceptance test verifies exactly the same behavior as the controller one, as it’s over the latter where all of the responsibility about this behavior lies.
This duplication isn’t always useful. In fact, the business, which is only interested in the acceptance test, is not very worried about the kind of technical issues that we’re verifying. On the other hand, at the unit level, this kind of detail is more relevant.
So, in case of having tests at the acceptance level that are identical to others at the unit level, it’s preferable to delete the acceptance ones if they’re not bringing in any business value, having covered the same circumstance in the unit ones.
So we’re going to delete those tests before continuing.
Guaranteeing business rules
Our next test will verify that an empty description, even if it’s a string, will not generate a new task. We’ll put it in the acceptance test:
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(): void
15 {
16 $this->client->request(
17 'POST',
18 '/api/todo',
19 [],
20 [],
21 ['CONTENT-TYPE' => 'json/application'],
22 json_encode(['task' => ''], 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('Task description should not be empty', $body['error']);
32 }
33
34 # ...
35 }
This test already indicates the violation of a business rule. This is the error that generates:
1 Failed asserting that 201 matches expected 400.
2 Expected :400
3 Actual :201
It tells us that tasks with empty description would be created as if they were valid. We have to go deeper into the application to see where we should be controlling the error.
So we go to the controller. But it mustn’t know anything about the business rules, as it lives in the Infrastructure layer. Nevertheless, it has to give the appropriate HTTP response, and that’s only possible is the use case somehow communicates to it that there’s been a problem.
The best way for it to do so is to throw an exception that the controller will capture, returning an adequate response.
This way, the controller test might end up like this:
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 $this->getTaskListHandler = $this->createMock(GetTaskListHandler::class);
26 $this->taskListTransformer = $this->createMock(TaskListTransformer::class);
27 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCompletedHandler\
28 ::class);
29 $this->todoListController = new TodoListController(
30 $this->addTaskHandler,
31 $this->getTaskListHandler,
32 $this->taskListTransformer,
33 $this->markTaskCompletedHandler
34 );
35 }
36
37 # ...
38
39 /** @test */
40 public function shouldFailWithBadRequestIfTaskDescriptionIsEmpty(): void
41 {
42 $exceptionMessage = 'Task description should not be empty';
43 $exception = new InvalidArgumentException($exceptionMessage);
44
45 $this->addTaskHandler
46 ->method('execute')
47 ->willThrowException($exception)
48 ->with('');
49
50
51 $request = new Request(
52 [],
53 [],
54 [],
55 [],
56 [],
57 ['CONTENT-TYPE' => 'json/application'],
58 json_encode(['task' => ''], JSON_THROW_ON_ERROR)
59 );
60
61 $response = $this->todoListController->addTask($request);
62
63 self::assertEquals(400, $response->getStatusCode());
64
65 $body = json_decode($response->getContent(), true);
66
67 self::assertEquals($exceptionMessage, $body['error']);
68 }
69 }
As you can see we simulate the use case throwing an exception, and the test fails because it doesn’t get captured, and therefore an adequate response isn’t returned. To not complicate the solution too much, I’m not going to create a domain exception (but it’s something that I would do in a real project).
Let’s pass this 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'], Response::HTTP_B\
39 AD_REQUEST);
40 }
41
42 try {
43 $this->addTaskHandler->execute($payload['task']);
44 } catch (InvalidArgumentException $invalidTaskDescription) {
45 return new JsonResponse(['error' => $invalidTaskDescription->getMessage(\
46 )], Response::HTTP_BAD_REQUEST);
47 }
48
49 return new JsonResponse('', Response::HTTP_CREATED);
50 }
51
52 # ...
53 }
The controller unit test has passed. However, the acceptance test has not. This is because we haven’t touched the use case yet. We have to go down a bit further and write a test that can fail to tell us what to implement.
But first, let’s examine the code:
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 }
As it can be seen, the point at which the exception should be thrown, is when a Task is created, but there’s no reason for the use case to be the one who verifies that $taskDescription has a sufficient length.
Instead, it makes more sense that this logic is in Task. After all, the use case is not the place to apply business rules, but rather to coordinate domain objects, which are the ones that have the responsibility to maintain them.
So, we’d have to dive a little deeper, and modify the Task test to guarantee that it’s always constructed consistently, with a description at least one character long. In the case that the description is empty, we’ll throw the exception.
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. :description');
24
25 self::assertEquals($expected, $representation);
26 }
27
28 /** @test */
29 public function shouldMarkTaskCompleted(): void
30 {
31 $expected = '[√] 1. Task Description';
32 $task = new Task(1, 'Task Description');
33 $task->markCompleted();
34
35 $representation = $task->representedAs('[:check] :id. :description');
36
37 self::assertEquals($expected, $representation);
38 }
39 }
All that’s left is to implement it.
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 }
This implementation is enough to pass the unit test. Let’s see if the test at the other level also pass. Indeed, all of the unit tests keep passing, and so does the acceptance one.
We’ll leave the acceptance test that we’ve just introduced, as it carries business meaning. Moreover, in this case the controller test only verifies that it’s able to handle the exception thrown by the domain layer, while the Task test verifies that it has to be constructed with a valid description.
Not found tasks
Another of our app’s defects has to do with trying to mark as completed an inexistent task. Currently, the endpoint will return a 500 error, when the correct one would be a 404 indicating that the resource that we’re trying to modify doesn’t exist.
The following acceptance test puts a spotlight on it:
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 }
The result is:
1 Failed asserting that 500 matches expected 404.
Apart from an error in:
1 "Undefined offset: 3" at /application/src/TodoList/Infrastructure/Persistence/FileTa\
2 skRepository.php
The base error occurs in the repository. However, we’re going to proceed systematically. As we’ve seen in the last example, the error can manifest itself in several manners in the different layers or levels of the application, so we have to go step by step, decide if that error has to be manifested in some way, and implement the necessary behavior.
The controller, as seen previously, is responsible of interpreting the problem and express it as a 404 error in the response. Therefore, it expects the use case to communicate it with an exception. In practice, it means that the controller has to react to a specific exception that will be thrown or rethrown by the use case.
So we express this with a 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 $this->getTaskListHandler = $this->createMock(GetTaskListHandler::class);
27 $this->taskListTransformer = $this->createMock(TaskListTransformer::class);
28 $this->markTaskCompletedHandler = $this->createMock(MarkTaskCompletedHandler\
29 ::class);
30 $this->todoListController = new TodoListController(
31 $this->addTaskHandler,
32 $this->getTaskListHandler,
33 $this->taskListTransformer,
34 $this->markTaskCompletedHandler
35 );
36 }
37
38 # ...
39
40 /** @test */
41 public function shouldFailWithNotFoundIdCompletingNotExistentTask(): void
42 {
43 $exceptionMessage = 'Task 3 doesn\'t exist';
44 $exception = new OutOfBoundsException($exceptionMessage);
45
46 $this->markTaskCompletedHandler
47 ->method('execute')
48 ->willThrowException($exception);
49
50 $request = new Request(
51 [],
52 [],
53 [],
54 [],
55 [],
56 ['CONTENT-TYPE' => 'json/application'],
57 json_encode(['completed' => true], JSON_THROW_ON_ERROR)
58 );
59
60 $response = $this->todoListController->markTaskCompleted(self::COMPLETED_TAS\
61 K_ID, $request);
62
63 self::assertEquals(400, $response->getStatusCode());
64
65 $body = json_decode($response->getContent(), true);
66
67 self::assertEquals($exceptionMessage, $body['error']);
68 }
69
70 }
As expected, the controller test will fail because the exception simulated in the mock isn’t being captured. It’s time to implement the code to do so:
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): Response
36 {
37 $payload = $this->obtainPayload($request);
38
39 try {
40 $this->markTaskCompletedHandler->execute($taskId, $payload['completed']);
41 } catch (OutOfBoundsException $taskNotFound) {
42 return new JsonResponse(['error' => $taskNotFound->getMessage()], Respon\
43 se::HTTP_NOT_FOUND);
44 }
45
46 return new JsonResponse('', Response::HTTP_OK);
47 }
48
49 # ...
50 }
Given that the test is now passing at the controller level, we go back to the acceptance test. This level still fails because, in fact, no exception is being actually thrown in this action’s flow.
We need to go a bit deeper.
If we examine the Application layer, the use case doesn’t have much to do. As in the previous problem, its job is to delegate domain objects, so it’s those the ones who should fail. As I’ve pointed out before I’m using generic exceptions, but in real projects we’d be also using domain exceptions at different levels. For example, a TaskNotFound, that could perfectly be extending OutOfBoundsException.
So we won’t do anything to the use case, but we observe that the responsible of saying whether a task exists or not will be the repository. The first line of the execute method is clear.
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 }
It’s not worth it to write a test to simulate that the repository throws an exception and the use case does nothing. If the use case was capturing exceptions from a more inner level to rethrow them as a different exception -a technique that we could call exception nesting, then we would do it to verify that.
Since we’re not doing that, we go to the repository level and we write a 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\OutOfBoundsException;
9
10 class FileTaskRepositoryTest extends TestCase
11 {
12 private FileStorageEngine $fileStorageEngine;
13 private TaskRepository $taskRepository;
14
15 public function setUp(): void
16 {
17 $this->fileStorageEngine = $this->createMock(FileStorageEngine::class);
18 $this->taskRepository = new FileTaskRepository($this->fileStorageEngine);
19 }
20
21 # ...
22
23 /** @test */
24 public function shouldFailIfTaskNotFound(): void
25 {
26 $storedTasks = [
27 1 => new Task(1, 'Write a test that fails'),
28 2 => new Task(2, 'Write code to make the test pass'),
29 ];
30
31 $this->fileStorageEngine
32 ->method('loadObjects')
33 ->willReturn(
34 $storedTasks
35 );
36
37 $this->expectException(OutOfBoundsException::class);
38 $this->taskRepository->retrieve(3);
39 }
40 }
The test fails, as the exception is not being thrown. So we go to the production code:
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 }
With this simple solution we pass the test. We rerun the acceptance test to check that the problem is solved.
Solving defects
An interesting conclusion about what we just did is that, in reality, solving bugs is nothing more that implementing an inexistent behavior in the software. In this context, I prefer using the term defect instead of bug.
What’s more, it could be argued that this chapter, more than being about fixing bugs, is about adding features to the software that had been left behind either consciously or unconsciously. In real life, this kind of thing is usually reported as a bug, although we know that it’s a feature that hasn’t been developed yet, or one that we hadn’t taken into account before.
In fact, by developing software using TDD, we normally prevent the kinds of defects that are usually associated to bugs, as it might be a typing problem, or some slip in the code that the language, for some reason, allows to go undetected.
In any case, the procedure is more or less the following:
- The first thing to do is reproduce the bug through a test at the most external level possible. Most likely it will manifest itself in the acceptance test, and it’s what’s expected if it’s a problem that can be detected by the users. But it may depend on the context.
- Next, we go to the next level of the application, trying to reproduce the same bug with a unit test. There will be levels where this isn’t possible and the test passes. As we’ve seen in the last example, the manifestation of the bug may be different in each level, or it may not happen at all. If we can’t demonstrate the bug at that level, we advance to the next one.
- In each level, we implement code to pass the test that showcases the bug. Once that level’s tests are green, we return to the acceptance test.
- If the acceptance test continues to fail, we’ll have to go to a deeper level in the application, create a test that describes the bug, and solve it. After this, we go back to the acceptance level until the test passes again.
- The moment the acceptance test passes, the defect is fixed.