As you might know, I've recently taken on a different role as a PHP developer. My employer are shifting our code base from CFML to PHP for various reasons ("So long, and thanks for all the CF"). One facet of this is we're moving off a venerable, well-established CFML code base to a new code base in the infancy of its existence (some work on it had been done before we picked up the project). In a different language.
And we've got from having about 4000 unit tests to zero.
A coupla of us are having a quick exploratory look at PHPUnit to see what it can do, and whether there's any "gotchas" we need to be aware of when testing in PHP. So far the findings seem to be that unit testing in CFML via MXUnit and TestBox - especially in conjunction with MockBox - is a bit easier than the hoops we need to jump through in PHP to achieve the same ends. This is mostly down to CFML's objects being far more dynamic than PHP's, so injecting testing helper methods and exposing non-public methods for testing is much easier.
The challenges that PHP has thrown at us has caused us to revisit our TDD and unit testing strategy. Not necessarily to revise it, but to revisit it and see if it does need revision.
Our existing policy is that all code is written via a TDD & "Clean Code" approach:
- the need for a piece of functionality is identified;
- a failing test is written to test a facet of the functionality;
- code is written to pass the test;
- repeat from 2 until the functionality is complete;
- refactor if necessary.
This is applied both to new functionality as well as maintenance of existing functionality. The TDD side of things drives the code design, and also demonstrates that downstream changes don't have adverse effects on earlier requirements. The usual sort of thing.
On new work, this will mean creating a new public method (and the class it needs to go in, as required), and implementing the requirement. So all the tests are on that public method. When we refactor the code, those tests all still work, which is fine.
As a result of the refactoring we might end up with some of the code moving out into public methods of other classes, or - as often - private methods of the same class.
The newly refactored public methods go through the same TDD approach as the initial one, although this is quite often just a re-homing of the tests which were previously testing the unfactored functionality from the original public method. And the original public method's tests are selectively updated to remove any tests which are now the domain of the new methods, and mocks are used in lieu of calling these factored-out methods in the remaining tests of the original function.
And traditionally we have done exactly the same thing with the refactoring that was simply moved into private methods.
Perhaps I can demonstrate this with some pseudocode. At the end of our first TDD round, and before refactoring, we have this:
class SomeClass
public method someMethod
(1) some logic to derive first part of result
(1) some logic to derive first part of result
(1) some logic to derive first part of result
(2) depending on something
(2) we take this approach
(2) we take this approach
(2) we take this approach
(3) else
(3) or we take this other approach
(3) or we take this other approach
(3) or we take this other approach
/depending
(4) a number of times
(4) do some functionality better homed elsewhere
(4) do some functionality better homed elsewhere
(4) do some functionality better homed elsewhere
/times
(5) massaging the remaining stuff
(5) massaging the remaining stuff
(5) massaging the remaining stuff
(1) and return it
/method
/class
class SomeClassTest
test (1) (later updated to accommodate 5)
test (2)
test (3)
test (4) (0 times)
test (4) (1 time)
test (4) (more times)
/class
Here the logic has been built sequentially as indicated with the numeric indexes, with obviously the test being written first. I was gonna display the code that way too, but it didn't make much sense.
After refactoring we might have this:
class SomeClass
method someMethod
(1) some logic to derive first part of result
(1) some logic to derive first part of result
(1) some logic to derive first part of result
(2) depending on something
(2)(6) call helperA()
(3) else
(3)(7) call helperB()
/depending
(8) call FirstNewClass.publicMethod
(9) call SecondNewClass.publicMethod
(1) and return it
/method
private method helperA
(2)(6) we take this approach
(2)(6) we take this approach
(2)(6) we take this approach
/method
private method helperB
(3)(7) or we take this other approach
(3)(7) or we take this other approach
(3)(7) or we take this other approach
/method
/class
class FirstNewClass
public method publicMethod
(4)(8) a number of times
(4)(8) do some functionality now homed elsewhere
(4)(8) do some functionality now homed elsewhere
(4)(8) do some functionality now homed elsewhere
/times
/method
/class
class SecondNewClass
public method publicMethod
(5)(9) massaging the remaining stuff
(5)(9) massaging the remaining stuff
(5)(9) massaging the remaining stuff
/method
/class
class SomeClassTest
test (1)
test (2)
test (3)
/class
class FirstNewClassTest
test (4)(8) (0 times)
test (4)(8) (1 time)
test (4)(8) (more times)
/class
class SecondNewClassTest
test (9)
/class
This is all basically just moving code around the place. Note that we do not have specific tests for
helperA() and helperB() because the existing tests for (2) and (3) still cover them.I don't think anyone would find anything here too controversial, and this is all fine.
However, and this is where there's a divergence in opinions, and I think most opinions here are wrong, and based on convenient use of bad logic.
TDD is a developmental approach right? One designs one's logic via deciding what it needs to do, and "proving" it does it via tests. With TDD, I think it's the dev process that is the important thing, and that we get unit test coverage as a result is a secondary side-effect. A good one, but a side-effect nevertheless. TDD is not a way of writing tests, it's a way of designing our code, basically.
OK, so the life of this software continues. And requirements change, and as a result - continuing this example - the realisation of that requirement is thus:
(2)(6) we take this approach
(2)(6) we take this approach
(2)(6) we take this approach
(10) but depending on something
(10) we also now have to do this
/but
And where's this logic going? into
helperA(), which is a private method.Following TDD - which is an approach to development, and "public" or "private" is neither here nor there - we test for "
(10) but depending on something" before we implement it:class SomeClassTest
test (1)
test (2)
test (3)
test (10) - happy
test (10) - unhappy
/class
But the dogmatic position is "don't test private methods". The problem here is that this dogma is a unit testing dogma, not a TDD dogma.
The general argument I see on the net when the subject of testing private methods when using TDD is that people conveniently ignore the "when using TDD", and just parrot back "you don't unit test private methods, you unit test the public interface". This is a logical non sequitur.
So, in actual fact, the position on TDD vis-a-vis private methods should be along these lines:
- Do not refactor into private methods unless the public method has test coverage for the side effects of the code targetted for refactoring (this is a unit testing consideration).
- Happily refactor the code into private methods, relying on existing test coverage (again, a unit testing consideration).
- Existing code that needs revision should have test coverage for the revisions before the revisions are made (this is a TDD consideration, and where the code happens to be located is immaterial).
My problem is that there there's a wealth of discussion on this out there (Google: "tdd private methods"), and the general message is contrary to this (but, as I suggested, backed-up with faulty logic). If this was a one-on-one situation I'd be confident of my understanding of things. But as I kinda feel like Cnut vs The Tide, I wonder if I'm missing something here?
What are your thoughts?
--
Adam
