concrete5/concrete5 — pull request #1665 by circmark (2014-03-14 22:07:25+00:00)
Title: a few improvements including data providers to allow for one assert per ...

From the looks of it unit tests are not much of a priority but I wish they were. To help out I have refactored the TextHelperTest class to show how to write better tests.

The main issue with this test is that each method had several asserts. This is a problem. What if you have 5 asserts and the 3rd fails? The last two asserts will not be run. You should still be able to figure out which assert failed but you have no idea if the last two would have passed. If you fix your code so that the 3rd assert passes but the 4th or 5th fail you have no way of knowing if they would have failed before fixing the 3rd assert or if your fix broke it. If you have one assert per method you will have a better idea of what does and doesn't work, assuming there are no fatal errors since they will cause phpunit to crash.

In this case it was a simple matter of using data providers (http://phpunit.de/manual/3.7/en/writing-tests-for-phpunit.html#writing-tests-for-phpunit.data-providers) to allow one assert per method without having to create several methods to test the same thing. If you have a failure you can see which input from the data provider failed and either fix the data or fix the code depending on where the problem lies.

Another issue was using the Loader class in the setUp() method to provide the helper class. There are a few problems with this. First, we are not testing the loader and the tests can fail if the loader is broken. Second, the loader will always provide the same instance of the helper class but we need each test to stand on its own which means that each test needs its own instance of the helper. In this case the helper has no properties and therefore no state so it's not such a big issue, but this can change.

I also moved the for loop that tests the "regular expression is too large" error into its own method since it is testing something specific.

I added a new test to ensure that class TextHelper correctly extends class Concrete5_Helper_Text.

Lastly, I unset the helper class in the tearDown() method. This isn't really necessary since the setUp() method will overwrite it with a new instance but it always makes me feel better knowing that it is absolutely destroyed after each test.