views:

197

answers:

6

I'm trying to get into unit testing, but there's one thing bothering me.

I have a php class which I want to unit test. It takes some parameters, and then spits out HTML. The problem is that the main functionality is calculating some values and conditions, and these I want to test. But I have put this in a private method, because normally, nobody needs to know about this method. But this way I am not possible to unit test the class because I have no means of testing the result of the method.

I have found this article about the subject. The conclusion of the article is using reflection to test the private methods.

How do you stand against this subject?

+6  A: 

You should have the logic in its own class and then unit test that class, so you don't have to reach through the html in order to test the logic.

As a rule:

You should never test private methods. The private methods exists in order to make the public methods pass their tests.

If you can delete the private methods without breaking the public methods, you don't need the private methods and can delete them.

If you can't delete the private methods without breaking the public methods, then the private methods are being tested.

If you follow the practice of TDD, it would be hard to get into this situation because every line of code is written to make unit tests pass. There should be no "stray" code within your class.

Tormod
Putting this method in a different class makes no sense to me, because it's the main thing the class does. But the output is not a simple value, but HTML, which isn't really testable in this sense. The method in question is normally called from the constructor.
Ikke
It seems to me that the HTML rendering and testable logic are two different responsibilities. Can you describe what the logic, which you wish to place under test, does?
Tormod
It is for a paginator. It calculates checks if it needs to show next and previous links and that sort of stuff.
Ikke
+2  A: 

I agree with Tormod; private methods should not be tested. Separating the logic from the presentation is a good idea in general and would allow you to test the logic separately from the presentation. Also, writing tests for the logic is a really good way of catching subtle cases where the logic and presentation isn't properly separated.

(Using reflection to test private methods sounds like a really bad idea to me.)

JesperE
Well the presentation is not really handled by this class. This class passes it's data to a view, and returns the view. Extracting the logic from this class would add unnecessary complexity imo.
Ikke
A: 

Calculating values and conditions is your business logic. The business logic is much more stable than the visual representation. You should test against a well defined interface. This will safe you changes to your tests that would be necessary if you test your code through the GUI and the GUI changes. (You will be able to add other clients as well.)

If you want to test the rendering (and you should) do it separately.

Here's a nice article why integration test won't work. (Your test is not really a unit test. It tests two aspects of the application.)

With this setup there won't be private methods to test.

Thomas Jung
I do want to test the business logic. The rendering is allready somewhere else (in a view).
Ikke
I thought that it already renders HTML.
Thomas Jung
It invokes the rendering, and returns the output.
Ikke
+1  A: 

Unit testing is about improving the probability of correctness of execution.

Encapsulation is about minimising the number of potential dependencies with the highest change propagation probability.

Unit testing is about runtime; encapsulation is about source code.

They're practically orthogonal: neither should influence the other. Making a private method public just to test it is not a good idea: that's unit testing unnecessarily degrading your encapsulation.

Copy your entire source code to a test directory and then remove any and all instances of the modifier, "private." Then write your tests towards this, deprivatised directory. This decouples unit testing from encapsulation concerns.

Automate this copying, deprivatising and unit test running with a script such as the below.

Regards,

Ed.

!/bin/bash

rm -rf code-copy

echo Creating code-copy ...

mkdir code-copy

cp -r ../www code-copy/

for i in find code-copy -name "*php" -follow; do

sed -i 's/private/public/g' $i

done

php run_tests.php

Ed Kirwan
That's an interesting solution.
Ikke
A: 

The usual solution to test the private methods is to extract them into a new class and to delegate. This is what Tormod suggested but you commented that this does not make any sense to you.

What you could also do, is to make the method public but to relay on some sort of naming convention to mark its privacy : for instance privateGetNumberOfPages(), or _getNumberOfPages(). This would be moral encapsulation: this won't prevent anyone to invoke the method, but no one could say he didn't know it was private.

That way you can invoke the method in your unit test but document (not enforce) it is a private method. This works well in some teams, but not in all.

Another possibility, albeit not the best one design-wise, is to make the method protected and to have the testcase class inheriting from the tested class so that the test can invoke the method and the encapsulation is enforced. I'm not sure this is possible in PHP.

philippe
A: 

I find that if I want to test a private method, then it ends up being complex enough to warrant moving to a new class and becoming public. I usually follow these steps: make the method public, test it thoroughly, notice the duplication this creates in the tests, refactor to push the newly-public method out to a new class.

J. B. Rainsberger