0

Lets say I have a class with two public methods and one private method.
The private method is used by both public methods and unit tests that are written against either of the public methods, could cover all lines of the private method.
Also the private method is complex and to cover all its behaviors many test cases are needed.

enter image description here

I could now:

Test the private method with only one of the public methods and leave it out for the other one.

Disadvantages:

  • No clear answer, which of the public methods should be used
  • If I chose public method 1 to cover the private method, method 2 is not covered at all. So if method 1 changes in some point in future and is not covering all lines of the private method anymore (but method 2 still does) this test gap could not be noticed.

Write double tests - for each public method the same tests

Disadvantage:

  • To hard to maintain the tests

Make the private method public

Disadvantage:

  • Hurting the encapsulation

How do you treat situations like this?

6 Answers6

7

You do not test private methods.

You test public methods.

Since you are supposed to test both public methods and your private code is 100% covered when you do, there is no problem or question.

It seems your questions premise is based on "I'm not done yet, what do I do". You continue. Test both public methods.

nvoigt
  • 9,230
  • 3
  • 30
  • 31
4

Although I agree with nvoight's answer that you test the public methods, it's worth expanding on a few things.

When you test public methods, you should do so based on their contract. Given a specific state of the object and inputs, calling the method will result in output and/or changes to the object's state. Your test sets up the object's state, provides the necessary inputs, calls the method, and then makes assertions about the output and the object's state.

When establishing equivalence partitioning, you may want to consider the implementation details of the method under test and any private methods called.

The tests for the two public methods may overlap, but there's nothing wrong with that. I'd argue that overlapping test coverage is a good thing.

The duplication of testing makes the test cases more resilient to refactoring. Since refactoring, by definition, involves changing the internal implementation details without changing the behaviors of the system, you should be able to perform many refactoring operations without touching the test cases. If you only cover the details when testing one public method, you will likely lose test coverage during refactoring, which will cause you to lose confidence that your refactoring preserves behavior.

If you consider your unit tests part of your codebase's documentation, you should enumerate the behaviors of your public methods as test cases, regardless of whether those behaviors are supported within the public method or an abstracted private method. This will make the definition of each method more explicit for future developers.

If you want to reduce duplication in test code, I'd look more at setup and teardown rather than the testing of behaviors.

Thomas Owens
  • 85,641
  • 18
  • 207
  • 307
1

I don't want to duplicate all those tests on both public methods. David Mason

Do those two public methods have the same API? If they don't, an adapter can make them the same. Then all the tests care about is behavior. Not what's hiding behind the API.

Oh sure their implementations can be wildly different but if they offer the same behavior you can certainly reuse those tests. It just has nothing to do with if they used that private method or not. All we care about is behavior. Test this way and you're free to change or swap out that private method with anything that gives you the same behavior.

One of the neatest tricks with testing is abstract test classes that allow tests to not know exactly what they are testing. An abstract factory can let the children of the abstract test class decide what it will construct to be tested. Now all your tests sit in an abstract class that knows exactly what behavior they want and have no idea what's providing it.

public abstract class FooTest {
    abstract Foo fooFactory();
    // Your reusable tests which now use fooFactory() to get their foo.
}

public class FooByMethod1Test extends FooTest { @Override Foo fooFactory(){ return new FooByMethod1(); } }

public class FooByMethod2Test extends FooTest { @Override Foo fooFactory(){ return new FooAdapter(new FooByMethod2()); // If API different, adapt } }

// As many other FooByMethod classes as you like...

Done that way you could run any number of methods through these same tests. It follows an old principle: Separate use from Construction. Follow that and you'll be solving your problem with good old polymorphism.

If the API is different, but the behavior is the same, you'll want to wrap one of these in an adapter to make the API the same.

If that's not useful to you then I don't know how those tests could ever be reused.

Test both public methods. But don't think that means you must duplicate the tests. If the behavior is the same exploit that. Only duplicate when you expect the tests to change independently.

The reason we don't want the private method tested is because that locks us into structural decisions that keep us from turning the private method into something else that does the same thing. That makes refactoring harder not easier. Test abstractions. Not structures.

For that mater, you shouldn't test all public methods for the same reason. Some are hidden behind some API that doesn't care if the entire object that public method hangs off of even exists. So long as the API provides the needed behavior who cares? I explained this better here.

candied_orange
  • 119,268
1

The private method is used by both public methods and unit tests that are written against either of the public methods, could cover all lines of the private method.

In "test driven developement" world, this is a perfectly normal situation to be in. We were working on public method 2, we noticed that the logic/requirements duplicated some part of public method 1, we performed an "extract method" refactoring on public method 1 to produce private method, then we refactored public method 2 to delegate some of its work to private method. At each change along the way, we're running our existing suite of programmer tests to ensure that things are working as expected.

This is all fine.


But if you decide that private method is complicated code that needs to be checked in isolation, the TDD answer is that the complexity here is a "code smell" indicating that you can improve your design by arranging your code so that this complicated logic can be tested in isolation.

This can include both exposing the method so that it can be activated directly by the test harness (see Martin Fowler's essay on Public vs Published Interfaces), or by arranging that private method delegate its work to some other method/class that which includes as part of its API the affordances you need to test the logic well

def complexPrivateMethod():
  self.z = MoreEasilyTestedThing.publicMethod(self.a, self.b, self.c)

Written this way complextPrivateMethod is no "so simple that there are obviously no deficiencies", so you ignore concerns about testing it directly.

There is, of course, some tension here: done poorly, introducing artifacts into your design leads to "test driven design damage", where the resulting code (though tested) is harder to understand / harder to change over time.

Done well... the position of the TDD thought leaders was always that if you made good design choices, the refactored-to-be-more-easily-tested code would actually end up being a better (easier to understand / easier to change over time) design.


If I chose public method 1 to cover the private method, method 2 is not covered at all. So if method 1 changes in some point in future and is not covering all lines of the private method anymore (but method 2 still does) this test gap could not be noticed.

One thing to recognize is that design frequently means trade-offs; we make choices that give us some valuable benefit, but in doing so also introduces a cost/risk that is acceptable.

Here, you've introduced two hypotheticals - that the needs of public method 1 change in a very particular way, and that change causes a "necessary" test to be decommissioned (and not, instead, replaced in the public method 2 suite), and then some well meaning programmer comes along and makes a breaking change to the shared code.

Could that all happen? yes, there is a chance. Does the cost/benefit ratio justify doing a bunch of work now to offset that chance later? Maybe....


A pattern that you can sometimes apply is to use the principles of data-driven testing, where each combination of inputs is paired with an expected result of some scenario that evaluates public method 1, and also an expected result of some scenario that evaluates public method 2.

Essentially, the data table gives you a way of documenting that the expectations of the tests are "the same", without introducing an undue coding burden.

VoiceOfUnreason
  • 34,589
  • 2
  • 44
  • 83
0

There are two typical ways that I approach the scenario you describe.

  1. Arrange (either in an official way, or using a trick) that the private function is accessible to the test cases. That takes away the whole question of ensuring full coverage without duplication due to the two public access points.

  2. Choose one of the public functions to exercise the functionality of the private function as well. The choice can be fairly arbitrary. Then test the other public functions only to the extent that you verify the private function gets called and any functionality not yet covered by tests through the other public functions.

    To reduce the risk that a later change to method 1 reduces the test coverage of the private method (e.g. some functionality becomes inaccessible through method 1), add a comment next to the test case (or encode it in the test case name) that the test is covering common functionality of methods 1 and 2. The aim here is to let a developer think twice before removing a test case because it is no longer applicable to method 1.

-1

Such an inner complexity - the apparent need to test an inner private method with more than one usage - propably indicates that a separation of concerns is needed.

Make a class that does the work of the inner method, now more or less a public method. Expect that the now 2 classes split fields, establish a separation of concerns.

With two classes unit testing becomes easy. The new low-level class can be kept private. In modular java place it in a non-exported package.

My assumption here is, that the test need stems from states, complexity. (The extracted class could also play to be a base class for the old class.)

If the inner method does really much, the bulk work, then the public API of the original class seems not okay, like incomplete methods, parameters and results.

Joop Eggen
  • 2,629