91

as far as I understand it, most people seem to agree that private methods should not be tested directly, but rather through whatever public methods call them. I can see their point, but I have some problems with this when I try to follow the "Three Laws of TDD", and use the "Red - green - refactor" cycle. I think it's best explained by an example:

Right now, I need a program that can read a file (containing tab-separated data) and filter out all columns that contain non-numerical data. I guess there's probably some simple tools available already to do this, but I decided to implement it from scratch myself, mostly because I figured it could be a nice and clean project for me to get some practice with TDD.

So, first, I "put the red hat on", that is, I need a test that fails. I figured, I'll need a method that finds all the non-numerical fields in a line. So I write a simple test, of course it fails to compile immediately, so I start writing the function itself, and after a couple of cycles back and forth (red/green) I have a working function and a complete test.

Next, I continue with a function, "gatherNonNumericColumns" that reads the file, one line at a time, and calls my "findNonNumericFields"-function on each line to gather up all the columns that eventually must be removed. A couple of red-green-cycles, and I'm done, having again, a working function and a complete test.

Now, I figure I should refactor. Since my method "findNonNumericFields" was designed only because I figured I would need it when implementing "gatherNonNumericColumns", it seems to me that it would be reasonable to let "findNonNumericFields" become private. However, that would break my first tests, since they would no longer have access to the method they were testing.

So, I end up with a private methods, and a suite of test that test it. Since so many people advice that private methods should not be tested, it feels like I've painted myself into a corner here. But where exactly did I fail?

I gather I could have started out at a higher level, writing a test that tests what will eventually become my public method (that is, findAndFilterOutAllNonNumericalColumns), but that feels somewhat counter to the whole point of TDD (at least according to Uncle Bob): That you should switch constantly between writing tests and production code, and that at any point in time, all your tests worked within the last minute or so. Because if I start out by writing a test for a public method, there will be several minutes (or hours, or even days in very complex cases) before I get all the details in the private methods to work so that the test testing the public method passes.

So, what to do? Is TDD (with the rapid red-green-refactor cycle) simply not compatible with private methods? Or is there a fault in my design?

  • How do I test a file reader? – gnat Apr 15 '15 at 9:18
  • 2
    Either these two pieces of functionality are different enough to be different units- in which case the private methods should probably be on their own classes- or they're the same unit in which case I don't see why you're writing tests for behaviour internal to the unit. Regarding the penultimate paragraph, I don't see the conflict. Why would you need to write an entire complex private method to pass one test case? Why not drive it out gradually via the public method, or start with it inline then extract it out? – Ben Aaronson Apr 15 '15 at 14:46
  • 26
    Why do people take idioms and cliches from programming books and blogs as actual guidelines on how to program is beyond me. – AK_ Apr 15 '15 at 14:50
  • 7
    I don't like TDD for exactly this reason: if you're in a new area then you'll be doing a lot of extra work while trying to find out how the architecture should be and how certain things work. On the other hand: if you're in an area you're already experienced with then there will be benefit to writing the tests first aside from annoying you because intellisense doesn't understand why you write non-compilable code. I'm a much bigger fan of thinking about the design, writing it and then unit testing it. – Jeroen Vannevel Apr 15 '15 at 23:05
  • 1
    "most people seem to agree that private methods should not be tested directly" - no, test a method directly if it makes sense to do so. Hide it as private if it makes sense to do so. – osa Apr 17 '15 at 22:25

15 Answers 15

43

Units

I think I can pinpoint exactly where the problem started:

I figured, I'll need a method that finds all the non-numerical fields in a line.

This should be immediately followed with asking yourself "Will that be a separate testable unit to gatherNonNumericColumns or part of the same one?"

If the answer is "yes, separate", then your course of action is simple: that method needs to be public on an appropriate class, so it can be tested as a unit. Your mentality is something like "I need to test drive out one method and I also need to test drive out another method"

From what you say though, you figured that the answer is "no, part of the same". At this point, your plan should no longer be to fully write and test findNonNumericFields then write gatherNonNumericColumns. Instead, it should be simply to write gatherNonNumericColumns. For now, findNonNumericFields should just be a likely part of the destination you have in mind when you're choosing your next red test case and doing your refactoring. This time your mentality is "I need to test drive out one method, and while I do so I should keep in mind that my finished implementation will probably include this other method".


Keeping a short cycle

Doing the above should not lead to the problems you describe in your penultimate paragraph:

Because if I start out by writing a test for a public method, there will be several minutes (or hours, or even days in very complex cases) before I get all the details in the private methods to work so that the test testing the public method passes.

At no point does this technique require you to write a red test which will only turn green when you implement the entirety of findNonNumericFields from scratch. Much more likely, findNonNumericFields will start as some code in-line in the public method you're testing, which will be built up over the course of several cycles and eventually extracted during a refactoring.


Roadmap

To give an approximate roadmap for this particular example, I don't know the exact test cases you used, but say you were writing gatherNonNumericColumns as your public method. Then most likely the test cases would be the same as the ones you wrote for findNonNumericFields, each one using a table with only one row. When that one-row scenario was fully implemented, and you wanted to write a test to force you to extract out the method, you'd write a two-row case which would require you to add your iteration.

  • 2
    I think this is the answer right here. Adopting TDD in an OOP environment, I've often found myself having a hard time overcoming my own bottom-up instincts. Yes, functions should be small, but that's after refactoring. Before, they can be huge monoliths. +1 – João Mendes Apr 16 '15 at 9:26
  • 2
    @JoãoMendes Well, I'm not sure you should get to the state of a huge monolith before refactoring, especially on very short RGR cycles. But yeah, within a testable unit, working bottom-up can lead to the problems the OP describes. – Ben Aaronson Apr 16 '15 at 14:22
  • 1
    OK, I think I understand where it went wrong now. Thanks a lot to all of you (marked this one as the answer, but most of the other answers are equally helpful as well) – Henrik Berg Apr 17 '15 at 13:19
66

A lot of people think that unit testing is method-based; it's not. It should be based around the smallest unit that makes sense. For most things this means the class is what you should be testing as a whole entity. Not individual methods on it.

Now obviously you will be calling methods on the class, but you should be thinking of the tests as applying to the black box object you have, so you should be able to see that whatever logical operations your class provides; these are the things you need to test. If your class is so large that the logical operation is too complex, then you have a design issue that should be fixed first.

A class with a thousand methods may appear testable, but if you only test each method individually you're not really testing the class. Some classes might require to be in a certain state before a method is called, for example a network class that needs a connection set up before sending data. The send data method cannot be considered independently of the whole class.

So you should see that private methods are irrelevant to the testing. If you cannot exercise your private methods by calling the public interface of your class, then those private methods are useless and are not going to be used anyway.

I think many people try to turn private methods into testable units because it appears easy to run tests for them, but this takes the test granularity too far. Martin Fowler says

Although I start with the notion of the unit being a class, I often take a bunch of closely related classes and treat them as a single unit

which makes a lot of sense for an object-oriented system, objects being designed to be units. If you want to test individual methods, perhaps you should be creating a procedural system like C, or a class comprised entirely of static functions instead.

  • 14
    To me it looks like this answer completely ignores the TDD approach in the OP's question. It is just a repetition of the "don't test private methods" mantra, but it does not explain how TDD - which is actually method based - might work with a non-method based unit test approach. – Doc Brown Apr 15 '15 at 12:58
  • 6
    @DocBrown no, it answers him completely in saying "don't over-granualise" your units and make life hard for yourself. TDD is not method based, it is unit based where a unit is whatever make sense. If you have a C library, then yes each unit will be a function. If you have an class the unit is an object. As Fowler says, sometimes a unit is several tightly-related classes. I think many people consider unit testing to be method-by-method simply because some dumb tools generate stubs based on methods. – gbjbaanb Apr 15 '15 at 13:01
  • 3
    @gbjbaanb: try to to suggest a test which allows the OP to implement his "gather non-numerical fields in a line" by using pure TDD first, without having the public interface of the class he intends to write already written. – Doc Brown Apr 15 '15 at 13:35
  • 8
    I've got to agree with @DocBrown here. The questioner's problem isn't that he desires more testing granularity than he can achieve without testing private methods. It's that he's tried to follow a strict TDD approach and- without planning as such- that's led him to hit a wall where he suddenly finds he has a bunch of tests for what should be a private method. This answer doesn't help with that. It's a good answer to some question, just not this one. – Ben Aaronson Apr 15 '15 at 15:03
  • 7
    @Matthew: His mistake is that he wrote the function in the first place. Ideally, he should have written the public method as spaghetti code then refactor it out into a private function in the refactor cycle - not mark it as private in the refactor cycle. – slebetman Apr 16 '15 at 3:45
51

The fact that your data-gathering methods are complex enough to merit tests and separate enough from your primary goal to be methods of their own rather than part of some loop points to the solution: make these methods not private, but members of some other class that provides gathering/filtering/tabulating functionality.

Then you write tests for the dumb data-munging aspects of the helper class (e.g. "distinguishing numbers from characters") in one place, and tests for your primary goal (e.g. "getting the sales figures") in another place, and you don't have to repeat basic filtering tests in the tests for your normal business logic.

Quite generally, if your class that does one thing contains extensive code for doing another thing that is required for, but separate from, its primary purpose, that code should live in another class and be called via public methods. It shouldn't be hidden in private corners of a class that only accidentally contains that code. This improves testability and understandability at the same time.

  • Yes, I agree with you. But I have a problem with your first statement, both the "complex enough" part and the "separate enough" part. Regarding "complex enough": I'm trying to do a rapid red-green-cycle, which means that I can only code for at most a minute or so at a time before switching to testing (or the other way round). That means that my tests will be very fine-grained indeed. I figured that was one of the advantages with TDD, but maybe I've overdone it, so that it becomes a disadvantage. – Henrik Berg Apr 15 '15 at 9:08
  • Regarding "separate enough": I've learned (again from unclebob) that functions should be small, and that they should be smaller than that. So basically I try to make 3-4 line functions. So more or less all functionality is separated into methods of their own, no matter how small and simple. – Henrik Berg Apr 15 '15 at 9:09
  • Anyway, I feel like the data-munging aspects (e.g., findNonNumericFields) should really be private. And if I separate it into another class, I will have to make it public anyway, so I don't quite see the point in that. – Henrik Berg Apr 15 '15 at 9:13
  • 6
    @HenrikBerg think why you have objects in the first place - they're not convenient ways to group functions, but are self-contained units that make complex systems easier to work with. Hence, you should be thinking of testing the class as a thing. – gbjbaanb Apr 15 '15 at 9:26
  • @gbjbaanb I would argue that they're both one and the same. – RubberDuck Apr 17 '15 at 11:37
29

Personally, I feel you went to far into the implementation mindset when you wrote the tests. You assumed you would need certain methods. But do you really need them to do what the class is supposed to do? Would the class fail if someone came along and refactored them internally? If you were using the class (and that should be the mindset of the tester in my opinion), you could really care less if there is an explicit method to check for numbers.

You should test the public interface of a class. The private implementation is private for a reason. It's not part of the public interface because it's not needed and can change. It's an implementation detail.

If you write tests against the public interface, you will never actually get the problem you ran into. Either you can create test cases for the public interface that cover your private methods (great) or you cannot. In that case, it might be time to think hard about the private methods and maybe scrap them alltogether if they cannot be reached anyway.

  • 1
    "Implementation details" are things like "did i use a XOR or temporary variable to swap ints between variables". Protected/private methods have contracts, like anything else. They take input, work with it, and produce some output, under certain constraints. Anything with a contract ultimately should be tested - not necessarily for those consuming your library, but for those maintaining it and modifying it after you. Just because it's not "public" doesn't mean it isn't part of an API. – Knetic Apr 17 '15 at 23:25
11

You don't do TDD based on what you expect the class will do internally.

Your test cases should be based on what the class/functionality/program has to do to the external world. In your example, will the user ever be calling your reader class with to find all the non-numerical fields in a line?

If the answer is "no" then it's a bad test to write in the first place. You want to write the test on functionality at a class/interface level -- not the "what will the class method have to implement to get this to work" level, which is what your test is.

The flow of TDD is:

  • red (what is the class/object/function/etc doing to the external world)
  • green (write the minimal code to make this external world function work)
  • refactor (what is the better code to make this work)

It is NOT to do "because I will need X in the future as a private method, let me implement it and test it first." If you find yourself doing this, you are doing the "red" stage incorrectly. This appears to be your issue here.

If you find yourself frequently writing tests for methods which become private methods you are doing one of a few things:

  • Not correctly understanding your interface/public level use cases well enough to write a test for them
  • Dramatically changing your design and refactoring away several tests (which may be a good thing, depending on whether that functionality is tested in newer tests)
9

You are encountering a common misconception with testing in general.

Most people who are new to testing start out thinking this way:

  • write a test for function F
  • implement F
  • write a test for function G
  • implement G using a call to F
  • write a test for a function H
  • implement H using a call to G

and so on.

The problem here is that you in fact have no unit test for the function H. The test that is supposed to test H is actually testing H, G and F at the same time.

To solve this you have to realize that testable units must never depend on each other, but rather on their interfaces. In your case, where the units are simple functions the interfaces are just their call signature. You must therefore implement G in such a way, that it can be used with any function having the same signature as F.

How exactly this can be done depends on your programming language. In many languages you can pass functions (or pointers to them) as arguments to other functions. This will enable you to test each function in isolation.

  • 3
    I wish I could up vote this many more times. I would just sum it up as, you haven't architected your solution correctly. – Justin Ohms Apr 16 '15 at 20:52
  • In a language like C, this makes sense, but for OO languages where the unit should generally be a class (with public and private methods) then you should be testing the class, not every private method in isolation. Isolating your classes, yes. Isolating the methods in each class, no. – gbjbaanb Jun 2 '15 at 12:58
8

The tests that you write during Test Driven Development are supposed to make sure that a class correctly implements its public API, while simultaneously making sure that that public API is easy to test and use.

You can by all means use private methods to implement that API, but there is no need to create tests through TDD – the functionality will be tested because the public API will work correctly.

Now suppose your private methods are complicated enough that they merit standalone tests – but they make no sense as part of your original class's public API. Well, this probably means that they should actually be public methods on some other class – one that your original class takes advantage of in its own implementation.

By only testing the public API, you are making it far easier to modify the implementation details in future. Unhelpful tests will only annoy you later when they need to be rewritten to support some elegant refactoring you have just discovered.

4

I think the right answer is the conclusion you came to about starting with the public methods. You would start by writing a test that calls that method. It would fail so you create a method with that name that does nothing. Then you maybe right a test that checks for a return value.

(I'm not entirely clear as to what your function does. Does it return a string with the file contents with the non-numeric values stripped out?)

If your method returns a string then you check for that return value. So you just continue to build it up.

I think anything that happens in a private method should be in the public method at some point during your process, and then only moved into the private method as part of a refactoring step. Refactoring doesn't require having failing tests, as far as I know. You only need failing tests when adding functionality. You just need to run your tests after the refactor to make sure they all pass.

3

it feels like I've painted myself into a corner here. But where exactly did I fail?

There's an old adage.

When you fail to plan, you plan to fail.

People seem to think that when you TDD, you just sit down, write tests, and the design will just magically happen. This isn't true. You need to have a high level plan going in. I've found that I get my best results from TDD when I design the interface (public API) first. Personally, I create an actual interface that defines the class first.

gasp I wrote some "code" before writing any tests! Well, no. I didn't. I wrote a contract to be followed, a design. I suspect you could get similar results by jotting down a UML diagram on graph paper. The point is, you must have a plan. TDD isn't a license to go willy nilly hacking at a piece of code.

I really feel like "Test First" is a misnomer. Design First then test.

Of course, please follow the advice others have given about extracting more classes out of your code. If you strongly feel the need to test the internals of a class, extract those internals into an easily tested unit and inject it.

2

Remember that tests can be refactored too! If you make a method private, you're reducing the public API, and hence it's perfectly acceptable to throw away some corresponding tests for that "lost functionality" (AKA reduced complexity).

Others have said your private method will either be called as part of your other API tests, or it will be unreachable and hence deletable. In fact, things are more fine-grained if we think about execution paths.

For example, if we have a public method which performs division we might want to test the path which results in division-by-zero. If we make the method private, we get a choice: either we can consider the division-by-zero path, or we can eliminate that path by considering how it's called by the other methods.

In this way, we may throw away some tests (eg. divide-by-zero) and refactor the others in terms of the remaining public API. Of course, in an ideal world the existing tests take care of all those remaining paths, but reality is always a compromise ;)

  • 1
    While the other answers is correct in that the private method shouldn't have been written in the red cycle, humans make mistakes. And when you've gone down the path of the mistake far enough this is the appropriate solution. – slebetman Apr 16 '15 at 3:46
2

There are times when a private method could be made a public method of another class.

For example, you might have private methods that are not thread-safe and leave the class in an temporary state. These methods can be moved to a separate class which is held privately by your first class. So if your class is a Queue, you could have an InternalQueue class which has public methods, and the Queue class hold the InternalQueue instance privately. This lets you test the internal queue, and it also makes clear what the individual operations on the InternalQueue is.

(This is most obvious when you imagine there was no List class, and if you tried to implement the List functions as private methods in class that uses them.)

  • 2
    "There are times when a private method could be made a public method of another class." I cannot stress that enough. Sometimes, a private method is simply another class screaming for its own identity. – user22815 Apr 17 '15 at 3:34
0

I wonder why your language only has two levels of privacy, fully public and completely private.

Can you arrange your non-public methods be package-accessible or something like that? Then put your tests in the same package and enjoy testing the inner workings which are not a part of the public interface. Your build system will exclude tests when building a release binary.

Of course sometimes you need to have truly private methods, not accessible to anything but the defining class. I hope all such methods are very small. In general, keeping the methods small (e.g. below 20 lines) helps a lot: testing, maintenance and just understanding the code becomes easier.

  • 3
    Changing a method access modifier just to run tests is a situation where a tail wiggles a dog. I think testing inner parts of a unit is just making it harder to refactor later. Testing public interface, on the contrary, is great because it works as a "contract" for a unit. – scriptin Apr 15 '15 at 15:19
  • You don't need to change the access level of a method. What I'm trying to say is that you have an intermediate access level which allows to write certain code, including tests, easier, without making a public contract. Of course you have to test the public interface, but additionally it's sometimes beneficial to test some of the inner workings in isolation. – 9000 Apr 15 '15 at 15:35
0

I occasionally have bumped private methods to protected to allow finer grained testing (tighter than the exposed public API). This should be the (hopefully very rare) exception rather than the rule, but it can be helpful in certain specific cases that you may run across. Also, that's something you would not want to consider at all when building a public API, more of "cheat" that one can use on internal use software in those rare situations.

0

I've experienced this and felt your pain.

My solution was to:

stop treating tests like building a monolith.

Remember that when you have written a set of tests, lets say 5, to drive out some functionality, you don't have to keep all of these tests around, especially when this becomes part of something else.

For example I often have:

  • low level test 1
  • code to meet it
  • low level test 2
  • code to meet it
  • low level test 3
  • code to meet it
  • low level test 4
  • code to meet it
  • low level test 5
  • code to meet it

so then I have

  • low level test 1
  • low level test 2
  • low level test 3
  • low level test 4
  • low level test 5

However if I now add higher level function(s) that call it, that have a lot of tests, I might be able to now reduce those low level tests to just be:

  • low level test 1
  • low level test 5

The devil is in the details and the ability to do it will depend on the circumstances.

-2

Does the sun revolve around the earth or the earth around the sun? According to Einstein the answer is yes, or both as both models differ only by the point of view, likewise encapsulation and test driven development are only in conflict because we think they are. We sit here like Galileo and the pope, hurling insults at one another: fool, don't you see that private methods need testing too; heretic, don't break encapsulation! Likewise when we recognize that truth is grander than we thought can we try something like encapsulate the tests for the private interfaces so that the tests for the public interfaces don't break encapsulation.

Try this: add two methods, one that has no input but justs returns the number of private tests and one that takes a test number as a parameter and returns pass/fail.

  • 1
    The insults chosen were used by Galileo and The Pope, not by any answer to this question. – hildred Apr 18 '15 at 14:54

protected by Community Apr 15 '15 at 22:04

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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