Some things to consider before using the @VisibleForTesting annotation

Image by Arek Socha from Pixabay

Recently a coworker and I had a small exchange on a pull request about the Android annotation @VisibleForTesting. For those who aren’t familiar with this annotation this is what the Android docs have to say about it:

Denotes that the class, method or field has its visibility relaxed, so that it is more widely visible than otherwise necessary to make code testable. 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. — Android Developer Documentation

It prompted me to take a bit of a deeper dive into this annotation and its use. If you read the description of the annotation it is clear what it does. Since it’s just technical documentation it doesn’t describe the use of the annotation however. In this post you’ll read about some things that you should consider before opening up your private functions for testing by using the @VisibleForTesting annotation.

It is always good to have some practical examples to go with discussions like this. Below you’ll find the example I’ll use to demonstrate some of the ideas that are discussed in this post. The code comprises of a simple VendingMachine class that has a public function buyDrink and several private functions. The class has simple requirements:

• It should return a Drink when enough money is inserted
• It should throw an exception when the amount inserted is not greater than the price of the requested drink
• It should be able to determine the price of a drink, including sales tax

The implications of @VisibleForTesting

As the documentation describes the @VisibleForTesting annotation allows you to change a functions visibility modifier to be more relaxed so it’s easier to test. Adding this annotation to your function enables you are to easily test private functions while still having compile time verification that these functions aren’t being used outside of their class.

Traditionally if you would make a function public without using this annotation it would break the design of your class. External dependencies might accidentally call your private functions and use the class in an unintended way. The @VisibleForTesting annotation fixes this problem by throwing a Lint warning (which can be treated as an error depending on your configuration) when the function in question is being used by a different class.

While at first sight it might seem like this is a great tool to make testing your private functions easier and safer, using the annotation has design implications that should be considered. The biggest design implication is the fact that you are actually about to test your private functions. There are valid scenarios that might bring you to this decision, but before you add the annotation to your function you should consider why you might not want to test your private functions.

Testing the public contract should already test your private functions

When designing a class and implementing its functionality, picking whether functions should be private or public is a very important step. You are basically defining a contract between the system and its user. In the case of the VendingMachine we have a clearly defined contract: the user can get a drink if he inserts the right amount of money and his desired drink type. In order to test this class we should be able to test all of its functionality through the public contract: buyDrink. After all, a client will never directly call calculateTax or calculateDrinkPrice. So if you aren’t able to test the result of these functions through buyDrink, why are the functions even in the same class?

As an example, in order to test all functionality within the VendingMachine class we can set up a test that verifies whether the calculations have been made properly. We know that a ‘Water’ drink costs 2.15 including sales tax. So we can set up a test like this:

With this test we’re only testing the contract, or what the user of the class would actually use. But we still confirm that the private implementation is working correctly. So even though we don’t specifically test our private functions, we’re still making sure that the logic is covered with test by testing the public contract.

Changes in the system

Aside from the fact that it shouldn’t be necessary to test the private functions, because you are able to test it through the public contract, there are actually situations where it could be detrimental to only specifically test the private functions. Let’s assume that we are testing the calculateTax function with the @VisibleForTesting annotation. We have some test data that we use to verify that the algorithm is working, something along these lines:

We figure this function contains the bulk of complicated logic for the class and we call it a day. Most of our (complicated) logic is tested, so this should be fine right?

Now as it often happens, people make mistakes and someone introduces some new logic to the buyDrink function that isn’t working as intended. Someone was under the assumption that water shouldn’t be taxed and created this change in the buyDrink function of the VendingMachine class:

The tests that cover our public contract, the ones we wrote in the previous example, break. The contract is violated, the tests that cover the price of a water are returning a value that isn’t valid anymore. Our tests that cover the private functions wouldn’t have caught this issue. They are of course still passing since the implementation of the calculateTax function hasn’t changed.

We can take one lesson from this: we should at the very least cover our public contract with tests to account for changes to the system itself. We can’t rely on just testing the private implementation.

Changes to the implementation

Assuming we now use both tests, public and private, there is another good example we can make that shows that most of the times private tests could create more work rather than save us time. Let’s imagine a situation where the implementation of the calculateTax function changes, but the outcome of the system as a whole stays the same.

Lets say that instead of taking in the drink price and returning the price including the sales tax, the calculateTax function now returns a TaxRate and the price can simply be multiplied by the rate, like this:

In this scenario our contract based tests are still working fine. The price of the drinks hasn’t changed. The tests we wrote for the calculateTax function are totally obsolete, since we now return a TaxRate instead of a price including sales tax. In this example we would need to make an effort to refactor the tests that cover the implementation details, creating more work and generating a false-negative in our test suite.

So now we’re at a point where we can conclude a couple of things:

• By testing the public contract, you are already testing the private functions. Testing the contract is more practical because that is what the client ends up using.
• You should always test the public contract. Tests that cover private implementation details don’t detect changes to the system as a whole.
• Testing both the public contract and private functions can create more work in the long haul. Tightly coupling tests to implementation details creates extra refactor work when the implementation changes.

What if testing the private implementation through the public API is too complex?

Now even though we have gathered some points on why it’s best not to test your private functions, in the real world things are sometimes complicated. It isn’t always easy to test all private functionality of a class through its public contract. Sometimes a piece of code is complex and has many dependencies. Sometimes the input goes through a lot of possible transformations before resulting in the output, making it hard to cover all possible branches of logic.

These are all valid points. Sometimes it is actually more convenient to test a private function. But instead of jumping the gun and starting to write tests for private functionality with all the disadvantages we have mentioned before, we should stop and think about why the private functionality is so difficult to test. Whenever you find yourself in a position where you have to mock a lot of different inputs to assert a lot of different possible outputs it is probably a good time to rethink the design of your class. Mocking a lot of different dependencies with numerous amount of outputs creates tests that are hard to understand. Instead of bending over backwards to get all the logic tested, we can ask ourselves: is the function we are trying to test really an implementation detail of the class, or should it be its own entity?

For example, let’s imagine that the calculateTax function has three dependencies, which it requires to calculate the right tax amount. It depends on a TaxRepository, a class responsible for getting the latest tax rates based on your region. A RegionRepository, a class that is responsible for getting the users’ current region and a TaxExemptStatusRepository that checks if the user has any tax exempt statuses.

If we wanted to test all of the logic inside the calculateTax function through the public contract of buyDrink we would need an entire swath of tests that mock all kinds of output from these dependencies.

The real solution to this problem isn’t to start testing the calculateTax function directly inside the VendingMachine class, but to extract it to its own separate class: TaxCalculator. This way you define a new public contract and make the VendingMachine a consumer of this contract. Whenever something is difficult to test through the public contract, it might be a sign that you are breaking the Single Responsibility Principle. In this examples case the VendingMachine shouldn’t be responsible for calculating the tax.

Now even though I think this is the preferred solution, I can also imagine situations where extracting the logic to a separate class just doesn’t make sense. Things are often not as simple as the VendingMachine example. Whether it’s too much work and you are limited by serious time constraints, or you are dealing with legacy code that you’d rather not touch, I think there are valid cases for testing the private functions of a class. We shouldn’t treat design principles as a silver bullet for every possible problem.

Conclusion

Whenever you feel the urge to test a private function by using the @VisibleForTesting annotation, think about the following things:

• You should preferably test the public contract of a class. Protecting you from changes to the contract itself and relieving you of refactoring work because of rigid tests that are tied to the implementation.
• If testing the private functions through the public contract is difficult, ask yourself why that is. Is there a flaw in the design of the class? Does it break the Single Responsibility Principle? Do you have to mock too many dependencies and this will make your tests unclear? Consider extracting the logic to a separate class with a separate contract rather than testing the private functions of this specific class.

If you’ve considered all of the above, but you don’t see a practical way of extracting the logic or you may just not want to refactor a piece of legacy code. Then by all means use @VisibleForTesting as a tool to help you cover that piece of code with unit tests anyway. Don’t let the tools dictate your design, choose them if they are necessary. But make sure you’ve considered all the implications this one annotation has.

This was my first Medium post, hoped you liked it. If you want to see more content about writing quality Android apps or software in general hit the follow button!