What makes a good Unit test?

According to Clean Code, the three things that make a clean unit test are readability, readability, and readability.

Readability in tests is probably even more important than in production code. So what makes a test readable? The same standards that make other code readable: clarity, simplicity, and density of expression.

Clean tests will contain a lot of your domain language. That means it will not focus on the technical terms of your testing framework, but rather on the business case the code is implementing.

F.I.R.S.T.

Clean tests should follow this acronym.

Fast

Fast means fast. Tests should run fast because you want to run them as frequently as possible. If your tests are slow, you will not run them often enough and the feedback loop will be longer. Lately, I have been working on a codebase that has around 5000 tests and performs around 8300 assertions in less than 3 seconds. I think that’s a good example of fast tests.

Independent

Tests should not depend on each other. It should be clear why a test fails and where you should go to fix the problem. If tests depend on each other, one failing test can cause a lot of depending tests to fail which makes diagnosis more difficult.

Repeatable

You should be able to run your tests in all environment. In your production environment, in your QA environment, and on your local machine. They should not depend on a network connection. If your tests are not repeatable, you will have an excuse for when they fail. Plus you will not be able to run them when the environment is not available.

Self-Validating

A self-validating test is a test with a boolean output. It either fails or passes. You should know it directly. You should not have to go through a log file to see which tests failed.

Timely

Tests should be written just before the production code. If you write your tests after the production code, the change that you will write code that’s hard to test is much bigger. Writing your tests before the production code always results in code that’s easy to test. And code that’s easy to test is also often clean, clear and simple.

How to test protected and private methods

Testing public methods with any testing framework is easy because we have access to the public interface of the test subject. Unfortunately, we don’t have access to the protected and private methods of our test subject. They are hidden to us (the client) for a reason. They are implementation details the class does not want to leak to potential clients.

Generally speaking, it is a bad idea to test private and protected methods. They are not part of the public interface and they are usually executed via the public methods of a class. So when we test the public methods, they implicitly test private and protected methods.

In TDD we are testing the behavior of a class, not the specific methods. So when we make sure we test all the things a class can do, we can rest assured that the private and protected methods are tested.

If you find yourself having large private methods which need their own test, there is probably something wrong with your design. Always aim for designing small classes that do one thing.

If you still think this is nonsense and you really want to test those methods, you can take a look here on how to do that with PHPUnit.

 

How to handle time in your tests?

Handling time in Unit Tests

Often we face scenario’s where a process we are modelling is depending on time. Take for example an expiring coupon. A coupon is no longer valid when it has expired. In PHP we could implement this as follows:

class Coupon
{
    /**
     * @var \DateTimeImmutable
     */
    private $expiryDate;

    public function __construct(\DateTimeImmutable $expiryDate)
    {
        $this->expiryDate = $expiryDate;
    }

    public function isValid(): bool
    {
        $diffInSeconds = $this->expiryDate->getTimestamp() - time();
        return $diffInSeconds > 0;
    }
}

class CouponTest extends TestCase
{
    public function testANotExpiredCouponIsValid(): void
    {
        $coupon = new Coupon(new \DateTimeImmutable('2018-10-28T07:14:49+02:00'));
        self::assertTrue($coupon->isValid());
    }

    public function testAnExpiredCouponIsNotValid(): void
    {
        $coupon = new Coupon(new \DateTimeImmutable('2017-10-28T07:14:49+02:00'));
        self::assertFalse($coupon->isValid());
    }
}

The problem

When we run the test for a valid coupon someday after 2018-10-28 (the date it will expire), it will actually fail because the coupon has actually expired. That’s not what we want because we need to test coupons with an expiry date in the future. So we need a way to fix that.

One way to resolve this issue would be to just change our test and pass in a date that’s 500 years in the future. It will fail again in 500 years but it will not be our problem anymore.

A better way to resolve this is to actually pass the current time as an argument to the isValid() method. Look at the new implementation:

class Coupon
{
    /**
     * @var \DateTimeImmutable
     */
    private $expiryDate;

    public function __construct(\DateTimeImmutable $expiryDate)
    {
        $this->expiryDate = $expiryDate;
    }

    public function isValid(\DateTimeImmutable $now): bool
    {
        $diffInSeconds = $this->expiryDate->getTimestamp() - $now->getTimestamp();
        return $diffInSeconds > 0;
    }
}

class CouponTest extends TestCase
{
    public function testANotExpiredCouponIsValid(): void
    {
        $coupon = new Coupon(new \DateTimeImmutable('2018-10-28T07:14:49+02:00'));
        self::assertTrue($coupon->isValid(new \DateTimeImmutable('2018-10-01T00:00:00+02:00')));
    }

    public function testAnExpiredCouponIsNotValid(): void
    {
        $coupon = new Coupon(new \DateTimeImmutable('2017-10-28T07:14:49+02:00'));
        self::assertFalse($coupon->isValid(new \DateTimeImmutable('2018-10-01T00:00:00+02:00')));
    }
}

That’s better. Now we have frozen the current time and are no longer depend on the real system type. Our tests will always pass.

How to test code that needs sleep?

The problem

Let’s say we have an API that knows a rate-limit of 1 request per second. When we have to make multiple requests, we need to build in some timeout mechanism. This mechanism would check if the last request was shorter than 1 second ago, otherwise it delays the request for 1 second.

One way to achieve this in PHP is:

class Api
{
    private $lastTime = 0;
    /**
     * @var \GuzzleHttp\Client
     */
    private $client;

    public function __construct(\GuzzleHttp\ClientInterface $client)
    {
        $this->client = $client;
    }

    public function doRequest(): void
    {
        $now = time();
        if ($now === $this->lastTime) {
            sleep(1);
        }
        $this->lastTime = $now;
        $this->client->get('https://some.api');
    }
}

How to test it?

To verify the behaviour of this class, we can mock the http client and do assertions on it. We are going to do a request twice and we are going to assert that the api is called twice. It would look something like this:

class ApiTest extends TestCase
{
    public function testCallsTheApi(): void
    {
        $httpClient = $this->createMock(\GuzzleHttp\ClientInterface::class);
        $httpClient->expects($this->exactly(2))
            ->method('get');

        $api = new Api($httpClient);
        $api->doRequest();
        $api->doRequest();
    }
}

Now we have a few problems. First of all we just slowed down our tests. Our unit tests are supposed to be fast, but now we waste 1 second. Second of all we are not actually asserting the second request came at least 1 second after the other. We could assert this by measuring the execution time, but this would still keep our tests slow.

A solution

To make our tests run fast and actually assert that our sleep function is called, we could do the following:

class Api
{
    private $lastTime = 0;
    /**
     * @var \GuzzleHttp\Client
     */
    private $client;
    /**
     * @var callable
     */
    private $sleep;

    public function __construct(\GuzzleHttp\ClientInterface $client, callable $sleep)
    {
        $this->client = $client;
        $this->sleep = $sleep;
    }

    public function doRequest(): void
    {
        $now = time();
        if ($now === $this->lastTime) {
            \call_user_func($this->sleep, 1);
        }

        $this->lastTime = $now;
        $this->client->get('https://some.api');
    }
}

class ApiTest extends TestCase
{
    public function testCallsTheApi(): void
    {
        $httpClient = $this->createMock(\GuzzleHttp\ClientInterface::class);
        $httpClient->expects($this->exactly(2))
            ->method('get');

        $sleepCalled = false;
        $api = new Api($httpClient, function (int $sec) use (&$sleepCalled)  {
            $sleepCalled = true;
            self::assertEquals(1, $sec);
        });
        $api->doRequest();
        $api->doRequest();

        self::assertTrue($sleepCalled);
    }
}

So what changed? We made the sleep function configurable via the constructor of the Api class. In our test we pass a spy function that asserts the callback is called and it’s called with the correct amount of seconds. This test now runs fast and is actually testing the needed timeout logic.

In our production code we would instantiate the Api class like this:

new Api(new Client(), function (int $sec) {
    sleep($sec);
});

Conclusion

There are for sure other ways to handle this case, but this is surely an elegant solution. If you have idea’s or thoughts about another solution, please let me know in the comments.