[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[jfw] Private methods should not be unit-tested



Private methods should not be unit-tested. Code quality would improve if we delete those tests.

It sounds weird and I understand it is felt as a waste to delete work you've done and lowering the code coverage, but I'll explain. Testing is not an improvement of code robustness in itself. It is done to test if the results are the same when you change the code. Change is the only constant in software development, that is why testing is important. Testing should therefore be done to an interface, not to an implementation. If you test an implementation, you are making the code more rigid, less flexible: for an other implementation could be an improvement of the code. Testing the implementation prevents change. Private methods are only necessary as part of the implementation.

In a way we have made ourselves prisoners of the "code coverage" measurement. It is easy: we just look at a number and are more satisfied if it is in the direction of 100% and sad if it is in the direction of zero. Colors in the tools we use support that: the more 100% = the more green = good and happy. Zero = red = wrong. The tools and measurements get a life on their own. We almost forget that the tools there are to help us, not the other way around.

I mainly came to this conclusion after watching "The Magic Tricks of Testing" presentation by Ruby star Sandi Metz; see for instance https://www.youtube.com/watch?v=qPfQM4w4I04 and https://www.youtube.com/watch?v=URSWYvyc42M
She points out that we should only have assertions for incoming queries and commands, asserting the result of an incoming query and the direct public side effects of an incoming command. For the outgoing commands we should only test the expectation that they are sent. We should ignore all tests of messages sent to self (they test the implementation , not the interface) and outgoing query messages (they are redundant). Testing private methods can be usefull during development (certainly if you follow a TDD road), but should be deleted afterwards. Or, funny moment in Sandi's presentation: be marked as "delete these tests if they fail".

Some other arguments against testing too much can be found in a recent paper by James Coplien ('Cope') "Why Most Unit Testing is Waste" : http://www.rbcs-us.com/documents/Why-Most-Unit-Testing-is-Waste.pdf

During the coming GSOC (Google Summer of Code) several students apply to work on more Unit Tests for Joomla. The main measurement for success is "code coverage". I hope we will get some better criteria in. Maybe we can even improve some code by having less testing code.  

I looked for tested private methods in the Joomla Framework and fortunately found not much matching that criterium:

Private methods that are currently tested in the Joomla Framework (only in 4 test-files):

The private methods Joomla\Archive\Zip::readZipInfo and Joomla\Archive\Zip::getFileData are marked as covered in the test of the calling Joomla\Archive\Zip::extractCustom , which could be a way to get more coverage.

This is the other way around, marking the private methods as
$this->markTestSkipped('This method is tested via requesting classes.');

In order to preserve our "good feeling" about code coverage, we could adopt a standard way of marking private methods as covered. Or just take the code coverage as measure with a grain of salt, in favour of better judgement for the need and desirability of tests.


--
Framework source code: https://github.com/joomla/joomla-framework
Visit http://developer.joomla.org for more information about developing with Joomla!
---
You received this message because you are subscribed to the Google Groups "Joomla! Framework Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to joomla-dev-framework+unsubscribe AT googlegroups.com.
Visit this group at http://groups.google.com/group/joomla-dev-framework.