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:
- You are supposed to pick the most specific assertion the testing framework offers.
- You are supposed to have only one assertion in each test method.
- 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:
- The number of elements is correct when an element has been added.
- 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!
Hello Mr Noback, Your post is making me more confident about what I do for unit tests because I actually do pretty much everything you exposed in the article. Except naming private methods in a descriptive manner to make the test even more readable. Even though it's an old article, to me it's still incredibly valuable. Thanks for sharing. Take care. Have a great day. Best regards Mr Alexandre ELISÉ
Monsieur Alexandre ELISÉ
Lead web developer @aproximito
Blog : https://coderparlerpartager.fr
> And with regard toassertCount(): is that really better than assertTrue(count(...))?
I see your point but these specialized assert functions can improve readability.
Actually I don't think PHPUnit is the right tool for applying the principles you are talking about. Imo organizing your tests a la Jasmine/Rspecs & co seems to make a lot more sense here. I created an open source project which is kind of port of Jasmine for PHP https://github.com/crysalea... . There's other existing implementations but I didn't find one which was as complete as Kahlan. There's no 1.0 yet but it worth a look if you are interested to write better specs.
We should go deeper: http://pastebin.com/qQ9bPefz
Kidding of course - I'm curious what your actual point of view is: do you think this is too much, or...?
Busy anyway ;) Yeah, your last point is not explained. Example 3.2 is probably good: easier for modification, removes duplication. But why do you want to remove all technicalities in the test code? There are 2 common cases when someone would want to read tests: 1) something is broken; 2) to learn how to use your code. In both cases they'd be interested in technicalities and I wouldn't want to jump back and forth among methods. Also, "makes it a test that very clearly communicates what it is trying to prove" that's what a function name is for.
Thanks for taking the time to make something better of this! I particularly like "const KLASS". What is your hourly rate? Maybe I have something for you.
> By the way, I always use private methods in test classes for all kinds of things (see the following chapter)
The link to chapter is missing.
public function setUp()
{
$this->collection = new Collection();
}
I do this often, but I like my suggestion in this article much better ;) The reason is: it's more explicit. When you read the test method it should be clear what the starting position is, it should not be hidden inside the "setUp()" method.
setUp is part of process, it's like __construct(), you have to look at it. Descriptive method is OK, so maybe:
public function setUp()
{
$this->startWithAnEmptyCollection();
}
You've got a point there. Although I never look at setUp myself ;) Only if the test method itself is unclear.
Would you instance variable not need to be cleared between each test? If I'm not wrong if you forget or as someone new to your test does not know that you have to call a startWith* method then you will be acting on an instance from another test. I guess what I'm saying is you should probably have a tearDown() which sets the variable to null.
Other than that I really like the idea of making the tests more readable!
I think PHPUnit clears instance variables automatically, though I'm not really sure about this. It may be a good idea to clean up yourself inside the tearDown() method. At the same time I strongly believe that each test should prepare the stage itself and not be susceptible to accidental characteristics of the environment.
PHPUnit performance is better if you put $this->startWithAnEmptyCollection() in setUp() than if you put it on test.
In this case the difference can be minimal, so in a more complex case, you should take this into account.