NYCPHP Meetup

[Gary A. Mort]

On 11/15/2013 10:58 AM, Robert Stoll wrote:
> I even think it is ok to have some degree of code duplication in test 
> classes just to make the test case very obvious. Each test case should 
> ideally just cover one aspect (method) and if you start having a lot 
> of test cases for just one method, then you should probably refactor 
> your method. 

One test per method would be a good reason to avoid inheritance. Most 
open source projects that I've reviewed that have unit tests have one 
test per class, not per method.

With one test per class, I find that in real code there is a LOT of code 
duplication when it comes to child classes. A moderately complex 
hiearchy of classes, with many children coming from one parent, session 
handling is a good example where you may end up with different sessions 
for many different backends - memcache, file, mysql, mongo, etc.

The tests for storing/retrieving a key are duplicated over and over in 
each of those tests.  Expanding session storage to support lazy loading 
of session data means making the same changes to each class used for 
storing session data.

I know of at least one project[Joomla!] where I have found bugs in some 
of the less utilized session storage classes because a change was made 
to a method in the parent class and was implemented in the more commonly 
used storage classes - the tests were updated in the parent test class 
and the commonly used child classes - but not in the less commonly used 
ones.

Since the less commonly used classes still pass their unchanged unit 
tests, and the modified classes pass their modified unit tests - the 
code seems to be working.   If the unit tests from the parent class had 
been applied to the child class then it would have been immediately 
obvious that something had been broken.

I'm not sure inheritance of tests classes is "the answer" mind you... 
It's just an "easy" way to catch /some/ changes like this.  A better 
"answer" is that designing unit tests based on "class" is the wrong 
approach.  As you mentioned, it could be better for it to be functional. 
Ie you need a SessionSave test and a SessionGet test, and then for each 
session storage class, you should call create an object of those classes 
and call their tests.   Then if for some reason for a specific storage 
engine the return values and functionality is different, you 
conspicuously write a new test just for that class/method combination.

Also keep in mind, this is NOT specific to Joomla! - I see the same 
testing structure used over and over - it is even the structure 
recommended/taught on the PHPUnit website.  Each class has it's own unit 
test class which has duplicated tests for all the methods.  I am sure 
other projects have the same issues, I just only know of the ones where 
I've fixed bugs in edge cases[having the PHP XCache plugin installed and 
not disabling the XCache password setting for operating on the entire 
list of cache objects, for example].

I know that both popular PHP classes for email[PHPMail and 
Swiftmail/Symfony] have odd little edge cases where they pass the unit 
tests but don't work for example - so I know it's a widespread 
occurrence.  I'm mainly curious on if this is a conscious choice - or if 
it is done simply because that is how it is documented on the PHPUnit 
website.

 From the replies here my initial conclusion is:
1) It is done this way simply because that is the way the examples were 
written['this way' being one mega test class per class]

2) The preferred method of writing test cases is to test by 
functionality, not by class.  Each method/function should have at least 
one test - not each class.