On Aug 10, 2009, at 5:27 PM, Andrew Shafer wrote: > > These tests look to be redundant now, so if you can push a new fix > with them removed, +1 > > Removing tests is easy enough, but I want to understand your motive. > > I don't consider them redundant per se, as they definitely test > different things. > > I'm familiar with the argument some people make that you shouldn't > test private methods, which those test do, but that is not the only > position. > > I resent the patch without the tests and updated the ticket.
It's functionally a private method, which means that it exists for convenience rather than behaviour, which means you should be able to test its behaviour from the outside. Especially given such a simple method with entirely transient results, testing the behaviour of the containing method seems to make a lot more sense, since that's what you really care about. If you're not testing all of the behaviours from the containing method, then you're opening yourself up to bugs anyway because the private method might pass its tests but still result in incorrect external behaviours. -- God loved the birds and invented trees. Man loved the birds and invented cages. -- Jacques Deval --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
