Now to talk about Functions. This could arguably be more controversial than the Naming and Classes standards.
There will be times where you raise your hand and cry out, “but that violates DRY principles!” or “the xUnit pattern book says this!” I assure you, I will provide further argument in later articles as none of these ideas exist in a vacuum.
Now let’s get to it!
No private methods in test files
Most of you will find yourself doing private methods to make your tests DRY (Don’t Repeat Yourself), but just remember that when things get too dry, they become brittle and break.
Sometimes it is more prudent to copy/paste that line or two into the test method itself. Honestly, it is a lot simpler to read the test in a single block than it is to jump up and down in the test file to track down what is hiding in a private method.
private methods are also cesspools for growing logic. If you read the previous articles about testing, you should know that tests are no place for logic, and that all logic should be considered part of the test framework or production code so that it is tested before use.
Let’s learn from past uses of private methods in unit test cases. Typically, you are either creating a custom assert, a custom setup, or you are encoding business logic.
In the case of a custom assert, please pull it out into the testing framework, or rather as an extension to the framework. I’ll write more about the best ways to do this later, but for now, just pull the asserts out into a class that lives with code that needs to be tested. You can also look at the Etsy PHPUnit Extensions wiki on Asserts and Constraints. Trust me, you will find that others might later need to use your custom assertions for new tests.
In the case of custom setup, we need to look at the problem a little harder. First, could you have used the inherited setUp() function to implement the set up. If yes, then please use that instead. If the answer is no because you have some instances of the SUT (subject under test) that need to be set up one way and some another, then you need to go back to an earlier consideration: can you just copy/paste. If copy/paste is a no go, and you couldn’t even offload some of the set up cost to setUp, then there is a problem with the construction of your SUT. You need to fix your production code because it is far too complicated to obtain an instance of this object. Maybe you can refactor your constructor into something simpler. You might have primitive envy, and could encapsulate commonly used together variables into objects. Another option is, you just might need a factory. While going through the exercise of creating a factory, you should learn more about the patterns for constructing your object. In your test, you can now use the factory to create your object for you, since it is a nicely tested piece of code. Maybe even some of your tests will be moved over to testing the object creation in the factory instead of cluttering the testing of the behaviors of a created object.
In the case where you are encoding business logic, you should seriously ask yourself why this business logic needs to be reconstructed in a test. There should be something in your production code that encapsulates the business logic.
Really, you are trying to test business logic. You don’t want to copy paste it in your test so you put that business logic in a private method, in your test code.
Ok, Mr. Fancy Pants, how is that business logic going to be implemented in production? If you have a method somewhere in your production code that does that business logic, then call that in the test and not the private method that copy/paste-d into the test code. If that business logic is hiding in a private method in your production, maybe you are being overzealous with protection levels. If that business logic is no where to be found in your production code, then why are you testing it. If that business logic is copy/paste-d all around your code base, then why was your test case so much more special than your production code.
It may seem mean taking out the usage of private methods from unit tests, but hopefully you now better understand how the presence of a private method in test code indicates a smell.
Applicable Sniffs
PHPUnitStandard.Testing.NoPrivateMethods
protected methods are reserved for overriding parent methods
If you look though the Test_Case implementations in PHPUnit, you should notice that the methods intended to be overridden for the most part are marked protected. This include setUp and tearDown.
The test case will run just fine if you override it with a public modifier, but it is an indicator to the reader of the test that you are overriding parent behavior. Also, the next criteria that I will present will not allow for methods that are not tests or providers to be marked public.
Just to double-check that you are not lost, these methods cannot be private either because that would definitely not override the function of higher visibility. Also, we do this as a whitelist check so that you do not squeak around the NoPrivateMethods sniff by upgrading to protected.
Applicable Sniffs
PHPUnitStandard.Testing.AllowedFunctionOverride
Only provider and test methods are allowed to be public
By provider, I mean a method you pass to @dataProvider in the phpdoc to provide data to a test method. See: PHPUnit DataProviders
By test, I mean a method that encapsulates a single test in a test case. The methods that begin with test or are annotated in the phpdoc with @test are test methods.
Both of these function types must be public for PHPUnit to see them. When PHPUnit cannot see a provider method, it will error out, but when PHPUnit cannot see a test function, it will silently not execute it.
Tests that are not executed will atrophy and die, but worse, they will confuse those who read the test. Most people will not notice that the modifier on a test function is not public, and they will read the test and assume that it has been executing and is valid. Readers of your test will then try to complete work based on these faulty assumptions and waste their time.
If you intend for you test not to execute then please mark it skipped, incomplete, or delete it.
Never be afraid to delete code when there is version control. It’s one of the many great reasons for version control.
Applicable Sniffs
PHPUnitStandard.Testing.TestOrProviderFunctionsOnly
PHPUnitStandard.Testing.TestOrProviderIsPublic
The final piece of the coding standard, as presented here, is Control Structures. This probably needs no separate article beyond this and the previous two. You should know by now that there should never be logic in you tests.
