Time and time again we're told about the red-green-refactor cycle of TDD, but people rarely emphasise that the actual test code is just as much a candidate for refactoring as the production code. Depending on the kind of goals you have for your test suite, there are a multitude of refactorings you can apply to your test code and here I'm going to cover some basics for removing duplication.

Before I start, don't get hung up on some of the terminology I use in this post (or any of my posts). I rarely use most of the terms day to day as they confuse people or are simply unnecessary, but feel like it's the right thing to do when I'm writing, particularly if I want to link to something on the subject.

Refactoring fixture setup

One of the easiest ways to get started removing duplication in your test code is to simply extract methods, much like you would with your production code. We usually see this in the form of a setup method. In the following example, we start with two tests that create the same transient fixtures in the tests arrangement phase. This kind of setup is called Inline Setup and keeps most of the information a reader needs to know about the test right there in the test. This can be very desirable, but can become a nuisance to maintain if a lot of tests require the exact same setup, especially if that set up is not all that simple.

/** @test */
public function newly_registered_user_should_be_active()
{
    $user = User::register(
        UserId::generate(), 
        new DateOfBirth("30 years ago"), 
        new NationalInsuranceNumber("QQ 12 34 56 A")
    );

    $this->assertTrue($user->isActive());
}

/** @test */
public function newly_registered_user_should_not_be_premium()
{
    $user = User::register(
        UserId::generate(), 
        new DateOfBirth("30 years ago"), 
        new NationalInsuranceNumber("QQ 12 34 56 A")
    );

    $this->assertFalse($user->isPremium());
}

Extracting that fixture arrangement to the test class's setUp method allows us to remove the duplicate code. Depending on the test's purpose, this could make them easier to read and understand, despite hiding a little bit of information. This kind of setup is called Implicit Setup.

public function setup()
{
    $this->user = User::register(
        UserId::generate(), 
        new DateOfBirth("30 years ago"), 
        new NationalInsuranceNumber("QQ 12 34 56 A")
    );
}

/** @test */
public function newly_registered_user_should_be_active()
{
    $this->assertTrue($this->user->isActive());
}

/** @test */
public function newly_registered_user_should_not_be_premium()
{
    $this->assertFalse($this->user->isPremium());
}

This isn't to say you should always carry out this refactoring. A lot of developers prefer Inline Setup, as all of the information they need to understand the test is contained within the test method. Implicit setup could hide details about the test and it can also lead to some poor practices, where the setUp method is used to set up fixtures that are only relevant to one or a few tests, rather than all of the tests. This is the reason I have named the instance variable in this example user, rather than newlyRegisteredUser, as I would hope that a user created in the setup method would be used for all tests. In doing so, I've blurred the lines a little. The two test cases require $this->user to be a newly registered user, if the setup changes at some point to accomodate other tests, our original tests may become fragile or ineffective.

One compromise is to use Delegated Setup. While this still hides some information, it makes the tests more readable, remains explicit and allows for more flexible refactoring in the future. Care should be taken to name these methods appropriately, conveying the right kind of information.

/** @test */
public function newly_registered_user_should_be_active()
{
    $this->assertFalse($this->newlyRegisteredUser()->isActive());
}

/** @test */
public function newly_registered_user_should_not_be_premium()
{
    $this->assertFalse($this->newlyRegisteredUser()->isPremium());
}

private function newlyRegisteredUser()
{
    return User::register(
        UserId::generate(), 
        new DateOfBirth("30 years ago"), 
        new NationalInsuranceNumber("QQ 12 34 56 A")
    );
}

Extracting custom assertions and verifications

/** @test */
public function test_freemium_user_can_login()
{
    $this->repo->add(User::register("dave@example.com", "password123"));

    $response = $this->client->post('/login_check', [
        'email' => $email, 
        'password' => $password
    ]);

    $this->assertEquals(200, $response->getStatusCode());
    $this->assertContains('Login Successful', $response->getContent());
}

/** @test */
public function test_premium_user_can_login()
{
    $user = User::register("dave@example.com", "password123");
    $user->upgrade();
    $this->repo->add($user);

    $response = $this->client->post('/login_check', [
        'email' => $email, 
        'password' => $password
    ]);

    $this->assertEquals(200, $response->getStatusCode());
    $this->assertContains('Login Successful', $response->getContent());
}

If you start to see code duplication during the act and assert steps, you might want to introduce custom assertions and/or verifications. Taking the example above, we have the same assertion in both tests.

/** @test */
public function test_freemium_user_can_login()
{
    $this->repo->add(User::register("dave@example.com", "password123"));

    $response = $this->client->post('/login_check', [
        'email' => $email, 
        'password' => $password
    ]);

    $this->assertSuccessfulLogin($response);
}

/** @test */
public function test_premium_user_can_login()
{
    $user = User::register("dave@example.com", "password123");
    $user->upgrade();
    $this->repo->add($user);

    $response = $this->client->post('/login_check', [
        'email' => $email, 
        'password' => $password
    ]);

    $this->assertSuccessfulLogin($response);
}

private function assertSuccessfulLogin(Response $response)
{
    $this->assertEquals(200, $response->getStatusCode());
    $this->assertContains('Login Successful', $response->getContent());
}

A further step could be extracting the verification out itself. Again, I might not bother for these tests, but for the sake of an example:

/** @test */
public function test_freemium_user_can_login()
{
    $this->repo->add(User::register("dave@example.com", "password123"));

    $this->verifyUserCanLogin("dave@example.com", "password123");
}

/** @test */
public function test_premium_user_can_login()
{
    $user = User::register("dave@example.com", "password123");
    $user->upgrade();
    $this->repo->add($user);

    $this->verifyUserCanLogin("dave@example.com", "password123");
}

private function verifyUserCanLogin($email, $password)
{
    $response = $this->client->post('/login_check', [
        'email' => $email, 
        'password' => $password
    ]);

    $this->assertEquals(200, $response->getStatusCode());
    $this->assertContains('Login Successful', $response->getContent());
}

How much is too much?

You can definitely refactor your test code too much. Extracting methods usually improves the maintainability of your tests, but a few poor names and you might regress things. When we extract code for reuse, we are essentially hiding details from the main test case. This is great for when you want to glance over the tests, but when you have a failing test that you want to examine, we may have actually created a couple of hoops to jump through. People often talk about keeping your tests DAMP, as opposed to being completely DRY. It's not about being one or the other, but finding a compromise between the two that keeps your tests maintainable and readable.

In the above examples, I've kept the extracted code reasonable close (all the code is in the same file) and I've created descriptive names for the extracted methods. I think this would be a reasonable refactoring, that hasn't left the code too DRY. However, there are a lot of considerations to make while trying to balance DAMP and DRY, mostly around what you are trying to achieve with your test suite, and I'll talk about them more thoroughly in later posts.

Refactoring Risks

One thing worth mentioning, there are risks involved in refactoring test code, as most people don't write tests for their tests.

I've heard people say refactoring without test coverage, isn't really refactoring, you're just changing things.

You might say that simply running the tests and seeing the green bar is enough to verify your refactoring was successful, but this would be missing a critical step from the TDD process. We'd essentially be missing the red part of the cycle. Yes, we might have verified our test still passes, but have we re-verified that our test has the potential to fail? This would of course require extra work and you would have to make some sort of judgement as to it's worth. If it was a large refactoring on a critical portion of the code, you might want to verify that the tests can in fact fail.

With regards to writing custom assertions, it is indeed quite possible to write tests for your custom assertions, especially if you go all in and write assertions in the form of your chosen testing library, where writing tests for the assertions and constraints is recommended. See this post by Matthias Noback for an example of writing a proper custom assertion in PHPUnit.

Someone on the C2 Wiki mentioned using Mutation Testing as a possible means to providing a safety net while refactoring test code, it's defintely got potential, but it's not something I've put in to practice.

This is the first post in a series about refactoring test code, be sure to take a look at the next article on Object Mothers.

davedevelopment.co.uk

Introduction to Reducing Duplication in Test Code

Refactoring fixture setup

Extracting custom assertions and verifications

How much is too much?

Refactoring Risks

Refactoring fixture setup

Extracting custom assertions and verifications

How much is too much?

Refactoring Risks

Subscribe to news and updates