0

How do I go about unit testing a private method that gets called in a loop like this one:

    // Calls MethodToUnitTest in a loop
    public static string Generate(params int[] values)
    {
        if (values.Any(i => i == 0)) // return empty string if any value contains 0 
            return "";

        return string.Join(", ", values.Select(MethodToUnitTest)); // return a comma delimited string
    }

    // Called multiple times by Generate
    private static string MethodToUnitTest(int value)
    {
        if (value % 2 == 0)
            return "Divisible by 2";

        if (value % 3 == 0)
            return "Divisible by 3";

        return value.ToString();
    }

I can see two options:

  1. Change the visibility of MethodToUnitTest to public (and possibly even move it to another class) then write unit test codes that exercise it.

    Con: The output of MethodToUnitTest is only for Generate so there is no need to make it public or move it to another class. Furthermore, MethodToUnitTest is just a simple function.

  2. Test MethodToUnitTest by calling Generate: Con: Just feels unnatural.

Sample tests

Assert.AreEqual("Divisible by 2", Generate(2));

Assert.AreEqual("Divisible by 3", Generate(3));

Assert.AreEqual("Divisible by 2, Divisible by 3", Generate(2, 3));

// ...etc

Which solution is better? At what point do you break down a method into multiple individually testable methods?

  • 3
    "Con: Just feels unnatural." No. It doesn't. You are expecting Generate to return string as you have written in your sample tests. if Generate delegates some part of that behavior to private method is irrelevant to you. – Euphoric Oct 25 '17 at 10:51
  • @Euphoric it feels unnatural to me because Generate is just calling MethodToUnitTest in a loop then joins the returned strings. MethodToUnitTest has a non-trivial logic that I can't test directly. – UnitTestingN00b Oct 27 '17 at 06:31

2 Answers2

9

Let me save you a lot of headaches down the road: avoid testing private methods. There are several reasons for this including:

  • Fragile implementation
  • Making the smallest change breaks many unit tests
  • Freezing your implementation for fear of all the rework you have to do

Think about what is important to work correctly. What are the interfaces and contracts that those interfaces dictate or imply. That is what developers will be using. From a more philosophical standpoint, think of it this way:

  • Should the user of the class care whether there is a private method called MethodToUnitTest? It's not part of any interface they can use.
  • Should the implementer of the class be free to change the implementation drastically while still supporting the exposed interface? Absolutely, that's basically what the Liskov Substitution Principle is about (although technically that's talking about substituting whole classes).
  • Should someone maintaining this class have to change 3-4 unit tests because the internal implementation needs to change? Now you are just creating work.

The solution is to only test the interface, or the methods that consumers of that class are expected to use. You test the contracts of that interface so that you are confident it works well. You add unit tests as requirements change. If you do that you will be well on your way to having a test infrastructure that works for you instead of against you.

If you insist on testing private methods, I predict that all those tests will be commented out, disabled, or simply removed within a year (assuming you didn't abandon the project altogether).

Berin Loritsch
  • 45,784
  • 7
  • 87
  • 160
  • I've heard this viewpoint a lot, and I understand why it's promoted, but I still want to have my cake and eat it too. I'm a bottom-up coder in those cases where unit testing has the most value; how can I build up my logic in small methods, unit testing each one, without polluting the API by making those methods public? – Robert Harvey Oct 25 '17 at 15:01
  • 1
    +1. Also, suggestion to read: http://blog.cleancoder.com/uncle-bob/2017/03/03/TDD-Harms-Architecture.html – Emerson Cardoso Oct 25 '17 at 16:46
  • @RobertHarvey Besides language-specific solutions you can always move the method to another class and compose it privately in the original one. That is proposed in the question itself. – Stop harming Monica Oct 25 '17 at 20:21
  • @RobertHarvey, I go from the bottom up as well, but I treat each class as if it has its own contract and reason to live. I can get near 100% coverage as well. When classes work together I then test that one class is using the other according to the contracts--i.e. mocking. Typically I start with no private methods, but create them when it makes the code more readable. – Berin Loritsch Oct 25 '17 at 22:37
  • I understand that I shouldn't be testing private methods. My question really is when should I break down a method into multiple testable methods. Imagine a JSON parser that has only one public method called `JSON.Parse()`. It would be unwise to do all tests through that interface. Is it worth it to make `MethodToUnitTest` in my sample code directly testable? – UnitTestingN00b Oct 27 '17 at 20:18
  • @UnitTestingN00b, At some point you'll be creating support classes as well. Most JSON parsers split the work to other classes. At that point, you can test each part individually. But I still stand by my statement. Private methods are implementation details that **need** to be free to change. – Berin Loritsch Oct 30 '17 at 12:44
  • @BerinLoritsch Sorry for bothering you again. I have always known that I shouldn't test private methods. My question is when do you decide to turn private methods into public (or internal for c#, or move into a supporting class). Sorry for not wording my question correctly. – UnitTestingN00b Oct 31 '17 at 22:58
  • @UnitTestingN00b, You have to have a reason to change an interface that is centered around the consumer of that interface. I still consider `internal` methods to be private (assembly/package private), particularly since unit test code is in a completely different assembly and can't access it. Turning a private method public really has to have a good reason for it. Perhaps if your JSON parser only provided a parameter to read a file you could make a great argument for just passing in a stream. But it should solve a problem that users of your class need to solve. – Berin Loritsch Nov 01 '17 at 12:40
0

How do I go about unit testing a private method

You don't!

Unittests verify public observable behavior (where "public" does not imply public visibility of the tested interface...) which is the return values and the communication with dependencies.


I can see two options:

Another option is to move that method into a separate class that becomes a dependency of your code under test and could be replaces with a mock on which you can verify the calls of this method.


At what point do you break down a method into multiple individually testable methods?

This boils down to the question What is a unit? I like the answer of Roy Osherove in The Art of Unittesting:

A Unit is all the code that has the same reason to change.

So a private method becomes a "unit" when it appears to have its own "context" which somehow differs from the context if the other methods.

Another reason to make it a separate unit is that it is hard to test.

Timothy Truckle
  • 2,336
  • 9
  • 12