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

[jfw] Re: Private methods should not be unit-tested


I get the arguments, but I am having trouble to accept this white-black rule  public -> test, private -> don't.
Could't think of some "real world" example, I came up with "school" like example. Consider following pseudo code.

Interface Sorter
 array public function sort(array)

Now I would write "interface" test, which would take array (maybe random generated) use sorter and check output if is sorted.
Then I would make run all implementations of this interface trough this test.

But also if I were implementing merge sort:
Class MergeSorter implements Sorter
 array public function sort(array)
 array private function merge(array, array)

Personally I would test merge function, because the interface test would tell me "Fail, MergeSorter doesn't work". But I would have no Idea what is broken, and have to go trough entire class, read again, debug it etc.
And making merge method public doesn't make much sense to me, because it is Implementation specific to MergeSorter.
Am I wrong?

Sure, there is argument I should run tests after small changes, but I don't have to be the guy who wrote it. 
I might get to some already written code (pull request maybe) I want to merge. But between the time someone made PR and I merge it I made other changes so there came up a bug. With this interface only test I will have to find what is wrong manually, which will cost me more time.

Actually I always thought that good tests (enough quality and enough quality) is important for OSS, because it saves time (and nerves) of the guy who is in charge.

(please keep in mind this is just example I was able to quickly came up from top of my head)

On Friday, 14 March 2014 22:38:20 UTC+1, Herman Peeren wrote:
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):
  • Joomla\Application\Cli\ColorProcessor::replaceColors
  • Joomla\Cache\File::checkFilePath
  • Joomla\Cache\File::fetchStreamUri
  • Joomla\Cache\File::isExpired
  • Joomla\Oauth1\Client::_generateRequestToken
  • Joomla\String\Inflector::addRule
  • Joomla\String\Inflector::getCachedPlural
  • Joomla\String\Inflector::getCachedSingular
  • Joomla\String\Inflector::matchRegexRule
  • Joomla\String\Inflector::setCache

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.');
  • Joomla\Linkedin\Groups::_likeUnlike
  • Joomla\Linkedin\Groups::_followUnfollow
  • Joomla\Linkedin\Stream_likeUnlike

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.