0

This question already has an answer here:

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?

marked as duplicate by Kilian Foth, David Arno, amon, Robert Harvey, gnat Oct 26 '17 at 7:29

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 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 6:31
8

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).

  • 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: blog.cleancoder.com/uncle-bob/2017/03/03/… – 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. – Goyo 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
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.

  • is not wrong. You don't test private methods. – Berin Loritsch Oct 25 '17 at 12:55

Not the answer you're looking for? Browse other questions tagged or ask your own question.