Sunday, 19 April 2015

PHP: unit testing private methods (a bit of reflection)

G'day:
Yes, I still do PHP as well as Lucee. Well, indeed, I don't do Lucee, I just seem to "invest" my time blogging about it. I am very much doing PHP all day every day again now.

Recently I revisited the notion of unit testing private methods. I realise a lot of people poopoo this notion as one is only supposed to unit test the interface, so only public methods. However when doing TDD, one needs to maintain one's private methods too, so this means one needs to test the behaviour of the change before making it.


Some argue these days that one should not have private methods: having them suggests bad class composition and a private method should be a public method of another class. I can see where they're coming from, but I also suspect the person deciding this was a Java developer and revels in the theory of OO put into practice, rather than... you know... getting shit done. I am in the "getting shit done" camp. I see no benefit in making classes for the sake of it, and I also like the idea of having code as close as possible to where it'll be used. So private helper methods have their place. And they need to be tested whilst being developed.

Others make the very good point that what actually needs testing is the nature of the impact on the result of the public method is what's important, so a private method ought to be tested "by proxy": call the public method, give it inputs that will invoke the to-be-developed new behaviour, and don't care - at test level - where the code happens to be. I have come to appreciate this idea, and it's how we approach a lot of our testing now.

One hitch with this is sometimes it's difficult to only hit the new functionality without also having to horse around appeasing other bits of functionality on the way through. Mocking makes this a lot easier, but not everything can be mocked, and even mocking stuff can be time and effort consuming. I guess if our code was perfect we'd not have this issue so much, but it's not (we're not), so we need to deal with that reality.

Sometimes it's just bloody easier to test a private method.

Back in my CFML days this was piss-easy. MockBox comes with a method makePublic(), which simply makes a private method public. It achieves this by injecting a proxy into the object under test which is public, and this wraps a call to the private method. One then calls that proxy in one's test.

Using makePublic() is as easy as:

mockbox.makePublic(object, "methodName");

Then continue to call methodName as per usual in one's tests. Cool.

This ain't so easy in PHP. One cannot just inject proxy functions into one's methods. However one can use reflection to make changes to an object's existing methods, including its access restrictions.

So what I set out to do was to make my own makePublic() method.

I arrived at this:

private function makePublic($object, $method){
    $reflectedMethod = new \ReflectionMethod($object, $method);
    $reflectedMethod->setAccessible(true);
    return $reflectedMethod;        
}

(it's private as it's just inline in my test class, the entire code for which can be seen here: SomeClassTest.php SomeClass.php).

This is simple enough. However actually using it is inelegant compared to the MockBox approach:

/**
@covers isIndexedArray
*/
function testIsIndexedArray(){
    $testValue = [53,59,61];
    $exposedMethod = $this->makePublic($this->testObject, 'isIndexedArray');
    $actual = $exposedMethod->invoke($this->testObject, $testValue);
    $this->assertTrue($actual);
}

That's no so clear.

So I decided to abandon that approach, instead coming up with this method:

private function executePrivateMethod($object, $method, $arguments){
    $reflectedMethod = new \ReflectionMethod($object, $method);
    $reflectedMethod->setAccessible(true);

    $result = $reflectedMethod->invoke($object, ...$arguments);
    return $result;        
}

This does the same thing, but it looks far more understandable in the calling code:

/**
@covers isIndexedArray
*/

function testIsIndexedArray_withAssociativeArray(){
    $testValue = ['a'=>67,'b'=>71,'c'=>73];

    $actual = $this->executePrivateMethod($this->testObject, 'isIndexedArray', [$testValue]);
    $this->assertFalse($actual);
}

It's really clear what executePrivateMethod does, I reckon.

I'mm going to show this to the lads at work tomorrow, and see what they think. Note: my position in general will be to test via the public interface, but in those situations where this is going to be a lot of work, or make the testing unclear, hopefully we can use this instead.

Thoughts?

--
Adam