Pest-Driven Development: Minimum Viable Routing

I'm blogging along with the laracasts series Pest Driven Laravel created by Christoph Rumpel as I work on a project I hope to open source. I'm just documenting how I'm working, the resources I'm using, the problems I run into and some cool things I'm reminded of as I develop.

Now that I am passing a test when I hit the 'home' route, I'm going to add all the other routes that are already part of the app. tests/Feature/ExampleTest.php doesn't say a lot about what's happening when I run my pest command, so I'm going to rename the file and add all my tests for routes there with the format:

    it ('gives back successful response for the <page_name> page', function() {
        get(route(<page_name>)->assertStatus(200);
    });

I am putting this in bold and a quote because it is important: You Must End Your Test File Name With the Word Test or it will not be executed.

A habit I've been getting into since I've been working with laravel is searching for the name of a file or variable I'd like to rename and check out where else it appears in the app. I'm trying not to look at too much source code right now, but I do also check out the first few functions if the naming conventions are unfamiliar to me or the syntax looks interesting. So when I look for the whole path, I see it's in vendor/pestphp/pest/src/Plugins/init.pub. This looks like it gets run during install and there's a lot more files in the tests/Feature directory than are listed here.

Little story here: when I was on the smoker's deck at Laracon meandering in to a conversation about event sourcing that I think may have been immortalized in this episode of no plans to merge, I met Filip (@ganyicz) and told everyone I'm new to Laravel. Zuzana had just given her talk "The Curse of Knowledge" and we were all thinking about it, so he didn't want to say "just read the documentation". I know this because he said "Well, I'm not going to say 'just read the documentation anymore". So basically the first advice he gave me was that Laravel source code is easy to find, written very well and maybe even better than the documentation. He has since showed me how to source dive from inside the ide and a bunch of other useful things.

This is a habit I have really enjoyed so far and it has definitely improved my ability to read and write code. Hacks to the game right there. Sometimes, like this time, I don't find any necessary takeaways but I become aware of something I wouldn't have seen otherwise.

Making a new pest file is pretty easy with php artisan make:test and prompts. When I'm prompted for a test type I select Feature (Pest) and name it tests/Feature/SuccessfulGetResponseTest.php. I also uncommented Illuminate\Foundation\Testing\RefreshDatabase::class in the uses method of tests/Pest.php. When I run pest I get 10 failed tests:

This is wonderful news because while I did import my routes\web.php file, I have not updated my Http\Controllers\RouteController.php since it was created with empty methods and this code should fail. This feels like TDD is actually happening, and I have clearly a clearly defined goal: get the TaskController to work so this feature test suite passes.

Getting the Test Suite to Pass

I'm writing this as I work, so I am currently fighting every impulse I have to completely rewrite the entire application. I absolutely hate the code that's running now and the user interface makes me shudder. But this is not a development goal, it is a copy and paste goal.

Debugging when you're copying and pasting code is mostly soul-crushing. I hear that other people adhere to this methodology but I come from a different philosophy of iterative development and I'm really struggling to even copy and paste my own code.

One Eternity Later

It finally occurred to me that the component structure I was using before is too complicated. I wanted to just move the previous blade component structures that compose my views, but it's not even code I want in my completed app. It was only when I took a step back that I realized that a few of the errors I was getting had changed to 302s, which actually is the desired behavior from the app.

So I reorganize the tests I've written and sort them by what I'm looking for:

// expect 302 Routes
it('gives back successful response for the tasks.board page', function () {
    get(route('tasks.board'))->assertStatus(302);
});
it('gives back successful response for the tasks.index page', function () {
    get(route('tasks.index'))->assertStatus(302);
});
// ... 2 more tests of the same format 

// expect 200 routes
it('gives back successful response for the home page', function () {
    get(route('home'))->assertStatus(200);
});

// expect 200 routes with variables
it('gives back successful response for the tasks.display page', function () {
    get(route('tasks.display'))->assertStatus(200);
});
it('gives back successful response for the tasks.confirmCreate page', function () {
    get(route('tasks.confirmCreate'))->assertStatus(200);
});
// ... 5 more tests of the same format

And when I run pest again I see that 5/10 tests are passing.

Viewing Expected Data On Routes

I made some assumptions about what I should be testing for that really complicated things. The next part of the course includes a test that made me think that writing a test for tasks.display should look something like this:

it('gives back successful response for the tasks.display page', function () {

    // create tasks using the factory() model
    Task::factory()->create(['task_description' => 'Test Task 1', 'sheets_id' => 1, 'public' => false]);
    Task::factory()->create(['task_description' => 'Test Task 2', 'sheets_id' => 2, 'public' => false]);
    Task::factory()->create(['task_description' => 'Test Task 3', 'sheets_id' => 3, 'public' => false]);

    // assert that the tasks are visible on the page
    get(route('tasks.display'))->assertSeeText(['Test Task 1', 'Test Task 2', 'Test Task 3']);
});

In order to get this output, I needed to start with a visible route. It took a LOT of time to clean up my existing/broken components so I could actually view this route, and digging through my messy code and sorting through what I needed to keep and get rid of just to pass variables through nested blade (the php templating language I'm using with laravel and jetstream) components had me absolutely philosophizing about trees, completely unaware of the forest.

At one point during this process, something happened with git that duplicated every file in my vendor folder and broke all of my tests except the example unit test that confirms that true==true by attempting to run each migration twice every time I ran a test. Since I was distracted by refactoring components and I thought most of the changes I'd made were in related to either components or pest, I thought that the issues with the database were related to the Illuminate\Foundation\Testing\RefreshDatabase::class. Then, when I tried to refresh the database with php artisan migrate:fresh and php artisan migrate:refresh and they both failed, I started going down the rabbit hole of sqlite and testing environments and yada yada yada. I rarely open my vendor folder (or its equivalent in other frameworks) or even think about it, so it was a pretty frustrating thing to debug. By this point in the forest metaphor I was basically staring at a leaf in a magnifying glass. Rough 24 hours.

So I finally had a visual confirmation of the component and exactly one vendor folder, and I started to refactor my code for the tasks.display route:

it('gives back successful response for the tasks.display page', function () {
    // create tasks
    for ($x = 0; $x <= 10; $x++) {
        $task = Task::factory()->create(['task_description' => "Test Task $x", 'sheets_id' => "$x", 'public' => false]);
    }
    $tasks = Task::all();
    $descriptions = [];
    foreach ($tasks as $task) {
        array_push($descriptions, $task->task_description);
    };
    get(route('tasks.display'), ['tasks' => $tasks])->assertSeeText($descriptions);
});

^ definitely don't do this, it does not pass and is cringe

Before I realized that (1) this is a really bad test to start with and (2) this is almost definitely a really bad way to think about this endpoint.

It's a bad test to start with because my tasks.display should only list upcoming tasks a user already assigned to themselves. tasks.show should display all the tasks with filters and pagination and the like and tasks.board shows a volunteer dashboard that emphasizes upcoming unassigned tasks and other tasks marked as important. Plus, I was too busy considering trees and leaves and the like to really consider what text I should be checking for and designing how the route should work. Woof.

It's almost definitely a really bad way to think about this endpoint because if I can figure out how to test simpler endpoints (ones that have tasks that should be visible to everyone, and then ones that have tasks that should be visible to everyone with some tasks being filtered out), I'll have a much better sense of how I want to go about presenting tasks a user has signed up for.

So I realized all of the tests here should be rewritten to just check if a page loads or not. I'm going to add other behavior to other test files later. I finally wrote a passing test I feel ok with for tasks.display because I thought this through: when I build out this method in the controller, I don't want to pass anything at all; I can get the user from the $request object

it('gives back successful response for the tasks.display page', function () {
    get(route('tasks.display'))->assertStatus(200);
});

This fails with a 302. And then I renamed the route from '/tasks/user/{user}' to '/tasks/user' in app/routes/web.php and rewrote the display() function in my app/Http/TaskController.php to:

public function display(Request $request)
    {
        $user = $request->user();
        $userTasks = Task::all(); // change this when assignment is working
        return view('tasks.display', ['tasks' => $userTasks, 'user' => $user]);
    }

But I'm not sure why I'm getting a 302. At least I've gotten to a level where I realized that a lot of the tests I considered to be passing actually aren't because I'm getting 302s instead of 200s and I don't have that many redirects in my routes. So I intentionally changed the controller to pass ['tasks' => $userTasks, 'user' => $user] and checked it out in the network part of the console. 500. And THEN the lightbulb went off-- I had already added authentication middleware on all the methods I'm getting 302s for, but I'm not writing tests for authenticated users. I guess I could spin it in a more positive way and say I'm already doing some work for testing my security groups, or hope someone else completely new learns from my mistakes. It's pretty frusterating right now. A 302 (redirect) is still information -- it's not a 404 (not found) or any other kind of error, and that was the goal.

10 Passing Tests

The final version of the test that tasks.display is working turned out to be:

it('gives back successful redirect response for unauthenticated users for the tasks.display page', function () {
    get(route('tasks.display'))->assertStatus(302);
});

Generally, the format for the tests on protected routes is:

it('gives back successful redirect response for unauthenticated users for the <route name> page', function () {
    $task = Task::factory()->create(['task_description' => 'Test Task 1', 'sheets_id' => 1, 'public' => false]);
    get(route('<route name>', ['task' => $task]))->assertStatus(302);
});

With the one exception of 'tasks.confirmStore'. This route gets a 405 (Method Not Allowed) because it should only be accessed by a redirect after a task is stored by a user.

What I learned

Since I published the first blog post, I've heard opinions from a few people who said they've tried TDD but couldn't stick to it and some more people that said they've never tried it but have been meaning to get around to it. I've heard from exactly one person who says he uses TDD as a methodology, and that was the instructor of the course. In the course recording. Before I started writing this. There's definitely problems with sampling here, but maybe the most important takeway I've had is that this is absolutely not required.

I started writing this post about 48 hours ago, and I was hoping to have it published at least yesterday. Completing it has been a milestone that kept me focused on TDD, and TDD has been a practice that allowed me to solve a pretty overwhelming bug. Normally I would have completely given up by this point, or started greenfield. So I can confidently say at this point that this is a methodology I really like for projects, especially if you're like me and just like liking things so much that you can get distracted when the next bug or endpoint calls. But it's a very cognitively challenging practice to implement these tests if you don't start out with them and let the test suite grow with the app. Too many spinning plates.

My buddy Tyler told me when he writes tests, he usually looks for access to a resource and validation rules and very little else. And then there's people who don't write any tests.

I'm making a commitment to myself to:

1. Finish the entire course
2. Add the types of testing described in the course to the project until I meet the (limited) goals I made in the first post in this series. Then I'll decide what to do as the project expands.