Thursday, 13 August 2015

@VisibleForTesting

I recently encountered the following annotation in the source code at work:
@VisibleForTesting
It is one I had not seen before. Below is an example of what it looked like.
@VisibleForTesting
void updateTaxation(TaxationData data) 
{
    ...
}
The annotation is part of the com.google.common.annotations package.

It turns out that it is a Marker annotation, simply to communicate to (other) software designers that the access level modifier has been relaxed, to make the code more easy to test.

In the example above, the access level modifier is "default", while it should/could have been "private".

I am still not convinced this is a good thing, because you changed production code to fix a testing problem.

We could work around this issue by using Reflection to access private methods/members, but Reflection is very brittle, especially when dealing with living code. So, that doesn't appeal to me either.

The best way, is to either:
  • test the private method, by using the public api that uses it
  • if this not enough, you can move the private method into a Strategy object1, and test the Strategy object in isolation

References

[1] Wikipedia - Strategy Pattern
https://en.wikipedia.org/wiki/Strategy_pattern
[2] StackOverflow -annotation to make a private method public only for test classes
http://stackoverflow.com/questions/6913325/annotation-to-make-a-private-method-public-only-for-test-classes

3 comments:

  1. Do I understand @VisibleForTesting is just an indicator for other devs and we will have to manually change the access level in production code. I

    ReplyDelete
    Replies
    1. I agree, it is an abomination! Well-thought-out code is always verifiable without the need for such a thing. Thanks for this post :D

      Delete
  2. I totally agree with you. Having said that, I did read on the docs for the annotation the following ...

    ==========================================================================
    You can optionally specify what the visibility should have been if not for testing; this allows tools to catch unintended access from within production code.

    Example:

    @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
    public String printDiagnostics() { ... }

    If not specified, the intended visibility is assumed to be private.
    ==========================================================================

    So its assumed the method you mark public should be private, unless you use the 'otherwise' clause. In theory, your IDE instrumentation should pick up any production code calling the method annotated as such, but you gotta hope your IDE is good at what it does.

    Otherwise, yeah, reflection is the way to go. While brittle, if your class changes, you should be reviewing your test class anyway, to update any of its test methods to match the changes to the class its testing, so you would catch any broken reflection calls in your test. Besides, we all run our unit tests before checking in our code changes, right? >:p

    ReplyDelete