Testing makes me a confident developer

Main Thread July 30, 2019 • 8 min read

The truth is I don’t always test my code. I take on the risk of bugs. It’s a gamble. I don’t want to spend the time testing code early on.

But this gamble never pays off. It’s only a matter of time until something in the code breaks.

Whether this is an actual bug found by users or I find myself it's embarrassing.

Without tests, I also introduce bugs. This happens enough times and eventually I become afraid to change the code. I code scared.

It's inevitable. At some point all code becomes foreign. That point when the code no longer fits in your brain. You no longer remember the finer bits of detail.

After that point, your outlook changes. You curse the code. You think, "WTF is this?" even if you were the original author. All this builds resentment and you start to justify rewriting the code.

We jump to a rewrite over a refactor. Often because a rewrite gives us the chance to know the code again. Which in turn gives us confidence and ultimately we continue to avoid writing tests.

But this is a false confidence. It's confidence in our memory, not the code. And it doesn't last.

Testing is the only way I really feel confident about the code. Testing gives me confidence not only that the code does what I expect now, but confidence the code will continue to do so in the future.

Again I don’t test everything. But if I have an application which crosses a demand threshold, I at least test the critical parts.

Gaining confidence with tests

I want to take a look at existing piece of code without test.

I'll start by writing tests to gain confidence the code behaves as expected. Then I'll use that confidence to refactor the code.

Let’s take a look some real world code from Shift. This controller action provides the ability to rerun a previously failed Shift.

public function rerun(RerunShiftRequest $request, Order $order)
{
    $this->verifyOrderBelongsToUser($order);

    if (!$order->canRerun()) {
        return redirect()->back()->with('error', ['template' => 'partials.errors.can_not_rerun', 'data' => ['order_id' => $order->id]]);
    }

    $repository = Repository::createFromName($request->input('repository'));
    $connection = Connection::findOrFail($request->input('connection_id'));

    try {
        if ($connection->isGitLab()) {
            GitLabClient::addCollaborator($connection->access_token, $repository);
        }
    } catch (\Gitlab\Exception\RuntimeException $exception) {
        Log::error(Connection::GITLAB . ' failed to connect to: ' . $request->input('repository') . ' with code: ' . $exception->getCode());
        return redirect()->back()->withInput($request->input());
    }

    $order->update([
        'connection_id' => $request->input('connection_id'),
        'repository' => $request->input('repository'),
        'source_branch' => $request->input('source_branch'),
        'rerun_at' => now(),
    ]);

    PerformShift::dispatch($order);

    return redirect()->to('/account');
}

While not a mission critical part of the application, it provides value by making the user self sufficient and prevents a support request.

This is about 30 lines of code and contains multiple paths.

First, I like to test the happy path. This is the path where the code behaves without error or exception.

In this case, it’s where the appropriate data was passed in and the Shift was put back on the queue.

To get started, I’ll create an HTTP test and write a test case which sends the proper data and ultimattely assert it was dispatched to the queue.

That test case becomes:

/** @test */
public function rerun_updates_order_and_adds_shift_queue()
{
    $connection = factory(Connection::class)->create([
        'service' => Connection::GITHUB
    ]);
    $order = factory(Order::class)->state('held')->create([
        'connection_id' => $connection->id
    ]);

    $now = Carbon::parse($this->faker->dateTime);
    $repository = 'shift/test-repository';
    $branch = $this->faker->word;

    $queue = Queue::fake();
    Carbon::setTestNow($now);

    $response = $this->actingAs($order->user)->post(route('order.rerun', $order), [
        'connection_id' => $connection->id,
        'repository' => $repository,
        'source_branch' => $branch,
    ]);

    $response->assertRedirect('/account');

    $queue->assertPushed(PerformShift::class, function (PerformShift $job) use ($order) {
        return $job->order->is($order);
    });

    $order->refresh();

    $this->assertEquals($connection->id, $order->connection_id);
    $this->assertEquals($repository, $order->repository);
    $this->assertEquals($branch, $order->source_branch);
    $this->assertEquals($now, $order->rerun_at);
}

Next I want to test the additional paths.

There are a few exceptional, or sad paths. The one I want to focus on is when the GitLabClientException is thrown.

When this exception is thrown, an error response is returned which redirects back to the rerun form with the appropriate data.

That test case becomes:

/** @test */
public function rerun_redirect_and_does_not_rerun_for_a_non_rerunnable_shift()
{
    $order = factory(Order::class)->state('ran_twice')->create();

    $repository = 'shift/test-repository';
    $branch = $this->faker->word;

    $queue = Queue::fake();

    $response = $this->from('/rerun')->actingAs($order->user)->post(route('order.rerun', $order), [
        'connection_id' => $order->connection_id,
        'repository' => $repository,
        'source_branch' => $branch,
    ]);

    $response->assertRedirect('/rerun');
    $response->assertSessionHas('error', [
            'template' => 'partials.errors.can_not_rerun',
            'data' => ['order_id' => $order->id]
        ]
    );

    $queue->assertNothingPushed();
}

There's additional sad path for when:

  1. Invalid data was sent.
  2. The user doesn’t own the Shift.
  3. The Shift is not eligible for a rerun.

I’m not going to worry so much about the incorrect data being sent.

Validation can take a while to test properly and doesn’t provide much additional confidence. Often, I'll use an alternative approach for testing validation in Laravel.

I also won't focus on the user not owning the Shift. While this is a small security measure, I'll save that for another time.

The path which is custom to this feature is eligibility to rerun logic. So I want to test this.

In order to do so I will manipulate the Order data to met the condition for canRerun.

This test case becomes:

/** @test */
public function rerun_does_not_rerun_and_redirects_back_when_there_is_a_gitlab_client_exception()
{
    $connection = factory(Connection::class)->create([
        'service' => Connection::GITLAB
    ]);
    $order = factory(Order::class)->state('held')->create([
        'connection_id' => $connection->id
    ]);

    $repository = 'shift/test-repository';
    $branch = $this->faker->word;

    $gitlab_client = $this->mock(GitLabClient::class);
    $gitlab_client->shouldReceive('addCollaborator')
        ->with($connection->access_token, Mockery::type(Repository::class))
        ->andThrow(GitLabClientException::class);

    $queue = Queue::fake();

    $response = $this->from('/rerun')->actingAs($order->user)->post(route('order.rerun', $order), [
        'connection_id' => $connection->id,
        'repository' => $repository,
        'source_branch' => $branch,
    ]);

    $response->assertRedirect('/rerun');
    $response->assertSessionHas('_old_input', [
        'connection_id' => $connection->id,
        'repository' => $repository,
        'source_branch' => $branch,
    ]);

    $queue->assertNothingPushed();
}

Refactoring with tests

Now I have tests which give me confidences this code is behaving as expected.

Armed with the confidence of these tests, I now have the ability to truly refactor the existing code.

First, I don’t like the try/catch block. I’ve never been an exceptional programmer 😅. try/catch blocks appear dense to me. Furthermore, Laravel offers a better way.

Starting in Laravel 5.5, the framework will automatically call the render() or report() method defined on a custom exception.

As such, I can move the code for generating the response to the GitLabClientException and allow the framework to catch the exception and return the response.

namespace App\Exceptions;

use App\Models\Connection;
use Illuminate\Support\Facades\Log;

class GitLabClientException extends \Exception
{
    public function render($request)
    {
        Log::error(Connection::GITLAB . ' failed to connect to: ' . $request->input('repository') . ' with code: ' . $this->getCode());
        return redirect()->back()->withInput($request->input());
    }
}

Now with the framework handling the exception, I have not only reduced the lines of code, but made it less complex and more readable.

-    try {
     if ($connection->isGitLab()) {
         resolve(GitLabClient::class)->addCollaborator($connection->access_token, $repository);
     }
-    } catch (GitLabClientException $exception) {
-        Log::error(Connection::GITLAB . ' failed to connect to: ' . $request->input('repository') . ' with code: ' . $exception->getCode());
-        return redirect()->back()->withInput($request->input());
-    }

To confirm everything behaves as expected, I simply run the tests to see everything passes.

Passing PHPUnit test results

In 272 milliseconds, tests afford me the confidence I have successfully refactored the code. Something I might have been reluctant to do without tests, or introduced bugs during the refactor.

Truly Refactoring

Refactoring is defined by Martin Fowler as:

a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior.

Many developers set out to refactor code, but really change code.

Having tests help verify the behavior was not changed. This in turn provides confidence you are truly refactoring the code.

I don’t want to do anything to lose this confidence.

As such when refactoring or adding tests, I make it a point to only alter one set of code at a time.

When starting to test an existing implementation I only wrote code for the tests. Once I has the tests, I only altered the implementation code then confirmed it with the tests.

Together the tests and the code create a balance. The tests confirm the code and the code confirms the tests. It's a harmonious equation, where only one side should be changed at a time.


Need to test your Laravel applications? Check out the Test Generator Shift to quickly generate tests for an existing codebase and the Confident Laravel video course for a step-by-step guide from no tests to a confidently tested Laravel application.