How do I deal with the fact that I am forced to make helper functions public for testing purposes?

Question

I've encountered several scenarios that require me to mock certain helper methods because they call outside resources. As a result, I'm forced to convert all my helper methods from private into public. Is this a code smell/bad practice?

Answer 1

Call outside resources through a public API customized to your apps needs. Doing so makes your dependence on outside resources explicit. This provides flexibility when those outside resources need to change.

That said, private methods rarely need a unit test written against them. Obtain code coverage by calling the private method through the public method that uses it.

Therefore, it's entirely reasonable to write a public method that calls a private method that calls a public API to access an outside resource. Do your mocking at the public boundaries.

Answer 2

Is this a code smell/bad practice?

Yes. The urge to test is good, but you're trying to test the wrong thing.

require me to mock certain helper methods because they call outside resources. As a result, I'm forced to convert all my helper methods from private into public.

You don't mock the helper methods, you mock the outside resources¹.

From the perspective outside of a class, you don't know (nor should you care) that the class uses private submethods or not. That's what "private" means: not for you to know or worry about.

From the perspective of testing, which is still outside of the class that is to be tested, you only care about public methods, and you are only able/allowed to mock dependencies that are publically settable/injectable (which should be all dependencies as far as I'm aware, though I'm not 100% sure if valid exceptions to this rule exist).

If you're writing tests and think to yourself "I should really test this helper method", then you're muddying the line between that class' public contract and the rest of its implementation.

---

It's hard to know without a code example in your question, but what I suspect is that your outside resource has been hardcoded into that private method (which would explain why you say you need to make it public to test it).

The correct solution in that case is to inject this outside resource as a dependency, most commonly done as a constructor parameter (though a method parameter can also be valid in some particular cases).

---

¹ Note that when you say "outside resources", I assume you mean other classes in your codebase, not hardcoded access to other servers/services. If you do mean the latter, then that access logic should always be wrapped in a class of its own in your codebase, and that wrapper class should be the dependency used by your original class (at which point your "outside resource" has become the former interpretation, i.e. another class in your codebase).

Answer 3

I'm forced to convert all my helper methods from private into public. Is this a code smell/bad practice?

Yes; it is actually suggesting two different concerns.

One of the motivations for adopting TDD is the idea that creating a design that is easy to test will also give your design a lot of properties that are desirable for long term maintenance and code health: modularity, cohesion, and so on.

For example, we want designs where the complicated logic is easy to test. "Outside resources" are often not easy to test, because they are difficult or expensive to control.

So we often end up with three broad categories of elements in our design:

• complicated logic (brancing) that depends only on arguments and local in memory data structures.
• management of outside resources that is "so simple there are obviously no deficiencies"
• elements that orchestrate the communication between other elements

Some resources to consider

• Parnas: On the Criteria to be Used in Decomposing Systems into Modules
• Gary Bernhardt: Boundaries
• Gary Bernhardt: Functional Core, Imperative Shell
• Cory Benfield: Building Protocol Libraries the Right Way

Some notes:

Ideally, a "management of outside resources" element is small. But I'll admit that I haven't always been able to do that; some kinds of things, like queries over HTTP tend to stretch the "obviously no deficiencies" constraint. But a focused element of this sort tends to be stable, which is almost as good. You do a bit of extra work to certify the element once, then you leave it alone.

"Mocks", and other forms of test doubles, usually show up when we are testing "orchestration". The idea here being that we achieve "easy to test" by measuring a system that uses inert substitutes for the elements that manage outside resources.

Answer 4

As much as I agree with the other answers, I like to see it from another point of view.

When you're writing tests, you're switching your hat from programmer to tester (very often). As the tester, you don't know much else than the specification the programmer let behind him. So you don't know anything about the private methods.

That means when, as a programmer, you write the code connecting to an external API in a private method, you're actually hiding this to the testing guy.

And oh boy how that poor testing guy will be disappointed and deceived. How could he expect those tests to take 30 seconds each, or fail miserably even though he followed the arrangement path you wrote in the specification.

So I like to see it as : If something else than the subject under test can fail during the test, make it obvious and make sure you can swap it with something safe. Then it's up to the tester to pick whether he judges it's safe enough to use an in-memory database for example, or tests will be too fragile if he queries an external API for real.