Making a private method public to unit test it...good idea?

Question

_{Moderator Note: There are already 39 answers posted here (some have been deleted). Before you post your answer, consider whether or not you can add something meaningful to the discussion. You're more than likely just repeating what someone else has already said.}

---

I occasionally find myself needing to make a private method in a class public just to write some unit tests for it.

Usually this would be because the method contains logic shared between other methods in the class and it's tidier to test the logic on its own, or another reason could be possible be I want to test logic used in synchronous threads without having to worry about threading problems.

Do other people find themselves doing this, because I don't really like doing it?? I personally think the bonuses outweigh the problems of making a method public which doesn't really provide any service outside of the class...

UPDATE

Thanks for answers everyone, seems to have piqued peoples' interest. I think the general consensus is testing should happen via the public API as this is the only way a class will ever be used, and I do agree with this. The couple of cases I mentioned above where I would do this above were uncommon cases and I thought the benefits of doing it was worth it.

I can however, see everyones point that it should never really happen. And when thinking about it a bit more I think changing your code to accommodate tests is a bad idea - after all I suppose testing is a support tool in a way and changing a system to 'support a support tool' if you will, is blatant bad practice.

Answer 1

Note:
This answer was originally posted for the question Is unit testing alone ever a good reason to expose private instance variables via getters? which was merged into this one, so it may be a tad specific to the usecase presented there.

As a general statement, I'm usually all for refactoring "production" code to make it easier to test. However, I don't think that would be a good call here. A good unit test (usually) shouldn't care about the class' implementation details, only about its visible behavior. Instead of exposing the internal stacks to the test, you could test that the class returns the pages in the order you expect it to after calling first() or last().

For example, consider this pseudo-code:

public class NavigationTest {     private Navigation nav;      @Before     public void setUp() {         // Set up nav so the order is page1->page2->page3 and         // we've moved back to page2         nav = ...;     }      @Test     public void testFirst() {         nav.first();          assertEquals("page1", nav.getPage());          nav.next();         assertEquals("page2", nav.getPage());          nav.next();         assertEquals("page3", nav.getPage());     }      @Test     public void testLast() {         nav.last();          assertEquals("page3", nav.getPage());          nav.previous();         assertEquals("page2", nav.getPage());          nav.previous();         assertEquals("page1", nav.getPage());     } } 

Answer 2

Personally, I'd rather unit test using the public API and I'd certainly never make the private method public just to make it easy to test.

If you really want to test the private method in isolation, in Java you could use Easymock / Powermock to allow you to do this.

You have to be pragmatic about it and you should also be aware of the reasons why things are difficult to test.

'Listen to the tests' - if it's difficult to test, is that telling you something about your design? Could you refactor to where a test for this method would be trivial and easily covered by testing through the public api?

Here's what Michael Feathers has to say in 'Working Effectively With Legacy Code"

"Many people spend a lot of time trying ot figure out how to get around this problem ... the real answer is that if you have the urge to test a private method, the method shouldn't be private; if making the method public bothers you, chances are, it is because it is part of a separate reponsibility; it should be on another class." [Working Effectively With Legacy Code (2005) by M. Feathers]