Page 1 of 1
PHPUnit and aggregating class
Posted: Wed Nov 03, 2010 4:58 pm
by VladSun
I'm wondering why PHPUinit does not use aggregate() (and alternatives) in order to access protected methods. It's like implementing on the fly (dynamic generated) interface "Testable". While I'm not a fan of private (it's like a "final"), protected keyword seems pretty much meaningful.
Re: PHPUnit and aggregating class
Posted: Wed Nov 03, 2010 5:18 pm
by Weirdan
VladSun wrote:I'm wondering why PHPUinit does not use aggregate() (and alternatives) in order to access protected methods. It's like implementing on the fly (dynamic generated) interface "Testable". While I'm not a fan of private (it's like a "final"), protected keyword seems pretty much meaningful.
To start with, testing non-public interface is considered bad practice. Another point would be that you don't need non-standard extensions to access non-public methods / members since you can alway use ReflectionMethod::setAccessible()
Re: PHPUnit and aggregating class
Posted: Wed Nov 03, 2010 6:04 pm
by VladSun
Weirdan wrote:To start with, testing non-public interface is considered bad practice.
And everybody tells not to use protected/private because it's not testable and to use public instead ... in order to *make* it testable ... Grhhh... I think exposing the non-public interface to the outside world would be even a worse practice.
Weirdan wrote:Another point would be that you don't need non-standard extensions to access non-public methods / members since you can alway use ReflectionMethod::setAccessible()
VladSun wrote:... (and alternatives) ...
Re: PHPUnit and aggregating class
Posted: Wed Nov 03, 2010 6:12 pm
by Eran
Not everything needs to be tested. All the protected methods should be used by the public methods - by testing the public methods you are also testing that the protected methods work. If the tests covering the public methods don't cover all the protected methods - it might mean you have some unused code you can get rid of.
You should be careful of double coverage - if you have tests testing public methods and other tests that test protected methods used by the previously tested public methods, one change could cause multiple tests to fail, making it harder to localize the problem.
Re: PHPUnit and aggregating class
Posted: Wed Nov 03, 2010 6:19 pm
by VladSun
VladSun wrote:And everybody tells not to use protected/private because it's not testable and to use public instead ... in order to *make* it testable ...
I've seen such advice many times here... Why? I suppose a "method" (it doesnt' matter whether it's a public/private/protected) is still a "unit"...
Also, public methods are often used by other public methods ... so, it's not a reason not to test protected/private methods.
Re: PHPUnit and aggregating class
Posted: Wed Nov 03, 2010 6:28 pm
by Eran
Private / protected methods are specifically meant for internal use in the class - meaning they are not meant to be used by itself. Public methods are supposed to be used independently, and so they should be tested, even though they might be used by other public methods - that is not necessarily the case.
If you are testing private / protected methods, you might get false positives - tests failing even though behavior hasn't changed. When refactoring internal private / protected methods, the end result should be that the public methods of the class remains unchanged. What is the purpose of maintaining tests on protected methods? its only purpose is to serve the public methods of the class, it doesn't matter if the internal implementation changes as long as the (public) behavior stays the same. In fact, it might be just what the refactoring set out to achieve. Don't test implementation, test behavior
Re: PHPUnit and aggregating class
Posted: Thu Nov 04, 2010 7:01 am
by Jenk
As Eran says, you don't test implementation you test behaviour. The only way to access this behaviour is through the objects public methods.
In the ideal world, the class won't even exist until you have at least the first part of your test. You then do the simplest (even if messy) possible thing to pass that test, then progress to the next test. Once you have a meaningful set of tests, and they pass, you refactor. Then, and only from then, is when you should see private/protected methods appear in the form of extractions/abstractions.
Re: PHPUnit and aggregating class
Posted: Thu Nov 04, 2010 9:21 am
by VladSun
So, why I've seen so many posts arguing that protected/private are bad modifiers because no tests can be done for *these* methods?
Re: PHPUnit and aggregating class
Posted: Thu Nov 04, 2010 6:49 pm
by Jenk
Because typically developers will write them upfront, instead of letting the refactorings emerge as they should do, or even worse can't be bothered to test them and 'hide' them as private. I still avoid them as they are just a ballache when I revist a class and need to override them.
Re: PHPUnit and aggregating class
Posted: Fri Nov 05, 2010 11:52 am
by josh
You test the features you care about. When building from in out, I test the implementation, so that when you add layers ontop of that code later, you have good error handling in the tools you've made yourself. I think of my system like a command line. When I have a job there are tools like grep, and cat and I can pipe these tools together.
I break my tasks down into tools, and code from a low level out. There are no protected methods anymore. For example on my 'import' screen it had to enumerate ranges of years. The enumeration part used to be a protected method. But from a user standpoint, why shouldn't I also be able to put year ranges in the control panel, not just the import? Indeed enumerating ranges of years is 1 responsibility, so 1 class. Now both the import & control panel classes can call into the 'enumeration' class.
As the system grows, enumeration could become more or less complex, involving many responsibilities, or it could be something the users never use and you take it out. Or maybe it grows past 1 responsibility and ends up becoming a whole framework of it's own.
The reason you are tempted to make it protected is to 'flag' it as deviating from the responsibility of the class it is currently living on. Thats my opinion at least.
Re: PHPUnit and aggregating class
Posted: Fri Nov 05, 2010 2:10 pm
by Jenk
josh wrote:The reason you are tempted to make it protected is to 'flag' it as deviating from the responsibility of the class it is currently living on. Thats my opinion at least.
That's a very good phrase.
