PHP & Symfony About PHP and Symfony2 development

A better PHP testing experience Part I: Moving away from assertion-centric unit testing

Posted on by Matthias Noback

In the introduction article of this series I quickly mentioned that I think unit testing often focuses too much on assertions. The historic reason for this is that in introductory articles and workshops it is often said that:

  1. You are supposed to pick the most specific assertion the testing framework offers.
  2. You are supposed to have only one assertion in each test method.
  3. You are supposed to write the assertion first, since that is the goal you are working towards.

I used to preach these things myself too (yes, "development with tests" often comes with a lot of preaching). But now I don't follow these rules anymore. I will shortly explain my reasons. But before I do, let's take a step back and consider something that is known as the Test framework in a tweet, by Mathias Verraes. It looks like this:

function it($m,$p){echo ($p?'✔︎':'✘')." It $m\n"; if(!$p){$GLOBALS['f']=1;}}function done(){if(@$GLOBALS['f'])die(1);}

A usage example:

it("should sum two numbers", 1+1==2);
it("should display an X for a failing test", 1+1==3);
done();

The result would look like this:

✔︎ It should sum two numbers
✘ It should display an X for a failing test

This is what unit testing all boils down to:

Unit testing is: saying something about a small unit of code and then get some feedback telling you if what you said is or is not the case.

1. Something is the case, or not

When you start using a framework like PHPUnit, you first have to familiarize yourself with everything it has to offer. You need to know which specific types of assertions are available and how you can use them. You need to know how to write your test methods in order for them to be picked up by the test runner, etc. This sometimes makes your view cloudy as to what you are actually trying to achieve with those tests.

I think you just want to describe the behavior of some unit of code (be it a function, an object or an object's method). Behavior is not usually something linear; based on some choices, the outcome will be different (just like in real life). So you have to test various "execution paths" of your code. You want to test different groups of input values and verify that the resulting behavior is as you had specified.

Why you don't have to use all those highly specialized assertions

If unit testing is "verifying the resulting behavior of running a piece of code", then unit testing can be quite simple: you only need to make sure that something is the case. And "being the case" is a simple boolean value. In the end this means that you should be able to write all your unit tests using boolean assertions, which is exactly the assumption of the "Test framework in a tweet".

Of course, it's very useful that PHPUnit offers some other assertions, but basically you don't need many of them. Besides, they often have some specific behavior that you should know about (which you may only find out when you're trying to debug some test). I think that in most cases you should write assertions yourself, with the exception of assertEquals() and assertSame() which are highly useful (you wouldn't want to write that code yourself again and again).

So I really don't think you need all the highly specialized PHPUnit assertions, like assertXmlStringNotEqualsXmlFile() or even assertCount(). I feel that it's best to be more explicit in your test cases: what does it mean in your specific situation for two XML strings to be "equal"? Which elements and attributes are important, which don't? And with regard to assertCount(): is that really better than assertTrue(count(...))?

What about failure messages?

Some people say you really should use those specialized assertions because they throw more specific failure messages when something is wrong. I'd say there is a very simple way to solve this, which is to supply your own failure messages. Which is a best practice anyway, since it is often quite uninformative if you just get a message like "Failed asserting that actual size 1 matches expected size 0." You really want information that is more specific to your test scenario:

$this->assertTrue(count($elements) === 0, 'No elements should have been added');

Now the failure message speaks the language of the domain of your test!

2.Only one assertion for each test?

Another "rule" that goes around in testing tutorials is that each of your test methods should contain just one assertion. I don't think this is true. But before you can let go of this rule, you need to know why the rule exists at all.

Consider the following test:

public function testAddAndRemoveElements()
{
    $element = 'some element';
    $collection = new Collection();

    $collection->add($element);
    $this->assertSame(1, count($collection));

    $collection->remove($element);
    $this->assertSame(0, count($collection));
}

I've made it easy for you to spot the problem, though in reality test smells like these are often better hidden. This one test method tests two behaviors of an object:

  1. The number of elements is correct when an element has been added.
  2. The number of elements is correct when an element has been removed.

These two behaviors are tested in one test method, which basically makes the unit of code that is being tested too large. Having multiple assertions in one test method here is a sign that the granularity of your tests is not good.

Why is granularity important? Well, if you introduced a bug which breaks the first behavior, the second behavior may still be fine, but you won't know it because the test runner does not execute the remaining statements of this test method. And, the other way around, if the second behavior is broken, this entire test fails, even though the first behavior remains unchanged.

So, multiple assertions in a test are definitely something to keep an eye on, but in many cases they are allowed and even reasonable. For instance when the outcome of running the code has several aspects which can not be verified using one assertion:

// Arrange, Act and...

$this->assertTrue($container->hasDefinition('logger');
$this->assertSame('FileLogger', $container->getDefinition('logger')->getClass());
$this->assertEquals(array('dev.log'), $container->getDefinition('logger')->getArguments());

This demonstrates the fact that the "one assertion per test" is not an absolute rule: one (complex) behavior is tested here (not two or three), and it would not make sense to have three test methods for verifying each of these outcomes. Anyway, that would require all the code above the assertions to be repeated in each of those test methods.

However, there are much better options than just adding many assertions in a row to verify some complex result of running a particular unit of code.These assertions are really not readable. In a single glance you should be able to see what is going on here, and this is definitely not the case. You would need to read these lines very carefully and still, you would probably need some experience with the Subject Under Test (SUT) to understand what is going on.

It should be easy to create another similar test, without copy and pasting (and possibly modifying) these assertions. It should also be easy to fix bugs in this test, without needing to dive too deep in the test code itself. The person who is going to fix the bug should only need to be looking at the test for a short while and then dive in the production code itself.

Group assertions

The easiest way you can fix the maintainability issue is to group assertions in a private method. For instance the above example can be rewritten like this:

public function testFileLoggerService()
{
    $this->containerHasServiceWithClassAndArguments(
        'logger',
        'FileLogger'
        array('dev.log')
    );
}

private function containerHasServiceWithClassAndArguments(
    ContainerInterface $container,
    $serviceId,
    $expectedClass,
    array $expectedArguments
) {
    $this->assertTrue($container->hasDefinition($serviceId);
    $this->assertSame($expectedClass, $container->getDefinition($serviceId)->getClass());
    $this->assertEquals($expectedArguments, $container->getDefinition($serviceId)->getArguments());
}

By the way, I always use private methods in test classes for all kinds of things (see the following chapter), which is why I don't understand why some people believe that your test class should not have any private methods and even define a PHP_CodeSniffer sniff for it!

Write a custom assertion

Although grouping assertions is a big step forwards, it can still be tempting to copy/paste the "grouped assertion methods" to other test classes. In that case it pays off to group the assertions in a custom assertion class. This allows you to reuse the same assertion (or "constraint" in PHPUnit terms) in other test classes and even to redistribute them in reusable (open source) packages. You can read all about creating custom assertions in an older article of mine.

3. Write assertions first and then work towards them?

This is what the average test method looks like:

public function testSizeOfTheCollectionReflectsNewElements()
{
    // Arrange
    $collection = new Collection();

    // Act
    $collection->add('element');

    // Assert
    $this->assertSame(1, count($collection);
}

I've heard many people say that they start with the assertion (Assert) at the end of the test method and then work towards the beginning of the test method, making function calls (Act) and preparing input values (Arrange). I used to do it like this myself; it feels a bit like documentation-driven development, or API-design first.

It's a good thing to use classes and functions in your test as if they already exist and are fully implemented. This helps you define a good API for your code: an interface which is easy to use and understand. However focusing on assertions is not necessary at all. In the above example, designing the API is more about the "Act" part of the test anyway.

Describe what you are doing

Instead of following the "Arrange Act Assert" (AAA) cycle in your code, it's more useful to actually describe what is going on and what you are trying to prove about your SUT. Reading the code in the test above makes sense if you read it carefully, but it is not an easy read, which means someone may misunderstand the test when they need to fix a bug, or modify it in the wrong way, which causes it to loose value.

This is what I mean with "describing what is going on":

  • $collection = new Collection() tries to describe: "start with an empty collection"
  • $collection->add('element') tries to describe: "add any element to the collection"
  • $this->assertSame(1, count($collection); tries to describe: "the collection has one element"

Rephrasing these small pieces of test code could result in something like this:

public function testSizeOfTheCollectionReflectsNewElements()
{
    $collection = $this->emptyCollection();

    $collection->add($this->anyElement());

    $this->collectionHasOneElement($collection);
}

private function emptyCollection()
{
    return new Collection();
}

private function anyElement()
{
    return 'any element';
}

private function collectionHasOneElement($collection)
{
    $this->assertSame(1, count($collection);
}

There are many other ways you can do this, but rewriting the original test has already greatly improved its quality: it speaks for itself (which makes it better maintainable).

Introduce instance variables for the SUT

One interesting way in which you can refactor the code above is by introducing an instance variable for the SUT (i.e. the collection object), like this:

private $collection;

public function testSizeOfTheCollectionReflectsNewElements()
{
    $this->startWithAnEmptyCollection();

    $this->addAnyElementToTheCollection();

    $this->theSizeOfTheCollectionIs(1);
}

private function startWithAnEmptyCollection()
{
    $this->collection = new Collection();
}

private function addAnyElementToTheCollection()
{
    $this->collection->add('any element');
}

private function theSizeOfTheCollectionIs($size)
{
    $this->assertSame($size, count($this->collection);
}

Now there are almost no technicalities left in the code of the actual test method, which makes it a test that very clearly communicates what it is trying to prove.

Conclusion

I started this article by mentioning some general assertion-centric truths about unit testing. Then I tried to refute each of them by showing some other approaches to the same old problems. Combined these reflect my current view on unit tests. I wish I had learned these practices earlier on!

There's more to discuss, in particular communication between objects (using mocks, stubs, etc.). This will be the subject of the next article.

Please let me know if you have some other good ideas about all of this or any objections to my personal approach described in this article!

Categories: PHP Testing

Tags: PHPUnit unit testing

Comments: Comments