11

I was reading this answer about testing private methods and it mentioned several possibilities:

  • extract methods as public to another class
  • make them public
  • separate the env of test and production so that methods are only public in the test env

I am currently coding in Java and I was wondering what does the community think about a kind of intermediary possibility provided by Java and maybe some other languages. With Java you can make your methods Protected or Package access so it is neither private not public but something between the two that make it testable by JUnit.

The Google Guava library even provide an annotation just for that called "VisibleForTesting" which doesn't do anything apart from explicitly saying that it is protected or public for testing purpose.

Since there is even annotations dedicated to this usage surely it is an accepted strategy among part of the community at least?

Is it an acceptable strategy or should I stick to the 3 options mentioned above and consider it the same as the "make public" strategy?

6
  • 3
    Just to be sure, please review all of the answers for this earlier question, and if you still want to keep this question open, please explain the particular reasons.
    – rwong
    Commented Aug 7, 2015 at 10:01
  • 1
    i am really interested in what people have to say about the use of Protected instead of private, in the question you mention, some people use it but nobody gives any pros and cons of that.
    – sam
    Commented Aug 7, 2015 at 10:09
  • Subjective questions can be asked on this site. To help keep this question in the "good subjective" category, please review the guidelines and techniques regarding the curation of opinion-based technical discussions.
    – rwong
    Commented Aug 7, 2015 at 10:17
  • 1
    possible duplicate of Protected variable & protected method in the era of TDD
    – gnat
    Commented Aug 7, 2015 at 10:27
  • 1
    @gnat thanks for the links but they are mostly centered on the protected variables, not methods
    – sam
    Commented Aug 7, 2015 at 10:36

1 Answer 1

21

Generally, one would better be judicious about using protected access at all. The reasons for that are laid out in answers to prior questions over here: Why is Clean Code suggesting avoiding protected variables?

As for using it the way you think of here - weakening access limitation because it feels like more comfortable to test - it looks like a terribly bad idea.

The "right option" when you test functionality covered by private method is to leave its modifier alone and figure testing approach based on assumption that method is and will stay private.

There could be many cases and reasons when one needs to redesign code for testability. Various difficulties in writing unit tests often serve as good indication for such a need. But, "oh, that method is private" is not one of those indicative difficulties.

I wrote an explanation for this in an answer to prior question as a side note because that other question and answer were focused on different matters. But for the question you asked, it seems to apply fully and directly, so I will simply copy that part over here.


<rant "I think I heard enough whining. Guess it's about time to say loud and clear...">

Private methods are beneficial to unit testing.

Note below assumes that you are familiar with code coverage. If not, take a time to learn, since it's quite useful to those interested in unit testing and in testing at all.

All right, so I've got that private method and unit tests, and coverage analysis telling me that there's a gap, my private method isn't covered by tests. Now...

What do I gain from keeping it private

Since method is private, the only way to proceed is to study the code to learn how it is used through non-private API. Typically, such a study reveals that reason for the gap is that particular usage scenario is missing in tests.

    void nonPrivateMethod(boolean condition) {
        if (condition) {
            privateMethod();
        }
        // other code...
    }
    
    // unit tests don't invoke nonPrivateMethod(true)
    //   => privateMethod isn't covered.

For the sake of completeness, other (less frequent) reasons for such coverage gaps could be bugs in specification / design. I won't dive deep into these here, to keep things simple; suffice to say that if you weaken access limitation "just to make method testable", you'll miss a chance to learn that these bugs exist at all.

Fine, to fix the gap, I add a unit test for missing scenario, repeat coverage analysis and verify that gap is gone. What do I have now? I've got as new unit test for specific usage of non-private API.

  1. New test ensures that expected behavior for this usage won't change without a notice since if it changes, test will fail.

  2. An outside reader may look into this test and learn how it is supposed to use and behave (here, outside reader includes my future self, since I tend to forget the code a month or two after I'm done with it).

  3. New test is tolerant to refactoring (do I refactor private methods? you bet!) Whatever I do to privateMethod, I'll always want to test nonPrivateMethod(true). No matter what I do to privateMethod, there will be no need to modify test because method isn't directly invoked.

Not bad? You bet.

What do I loose from weakening access limitation

Now imagine that instead of above, I simply weaken access limitation. I skip the study of the code that uses the method and proceed straight with test that invokes my exPrivateMethod. Great? Not!

  1. Do I gain a test for specific usage of non-private API mentioned above? No: there was no test for nonPrivateMethod(true) before, and there is no such test now.

  2. Do outside readers get a chance to better understand usage of the class? No. "- Hey what's the purpose of the method tested here? - Forget it, it's strictly for internal use. - Oops."

  3. Is it tolerant to refactoring? No way: whatever I change in exPrivateMethod, will likely break the test. Rename, merge into some other method, change arguments and test will just stop compiling. Headache? You bet!


Summing up, sticking with private method brings me a useful, reliable enhancement in unit tests. In contrast, weakening access limitations "for testability" only gives me an obscure, hard to understand piece of test code, which is additionally at permanent risk of being broken by any minor refactoring; frankly what I get looks suspiciously like technical debt.

</rant>

5
  • 1
    Thanks Gnat sounds like a very good answer to my question, I am only waiting a bit if I will get a different opinion
    – sam
    Commented Aug 7, 2015 at 11:15
  • I think this is a strawman. When I'm testing for coverage (and I often do), then I don't use function coverage except as a progress indicator – statement coverage, branch coverage, and input bounds testing are far more useful. All of those would have told me that branch was untested. And if we're already doing white-box coverage driven tests, explicitly testing private methods does not seem to be a problem: smaller code units are way easier to verify. When a refactoring breaks the test – that's an useful warning, but I can freely adapt the test since it only covers implementation details.
    – amon
    Commented Aug 7, 2015 at 11:47
  • @amon re-check the question asked. Smaller code units have nothing to do with it - it's solely about changing access modifier, not about restructuring / splitting the code
    – gnat
    Commented Aug 7, 2015 at 12:02
  • ...as for coverage analysis, it looks like you over-complicate things. Tools I used (more precisely, tools used by QA folks against my code - 'cause it's their job to verify quality) simply told me what lines of code were executed by the tests and what weren't. It's that simple - you just go over lines of code that are reported as gaps and figure what tests to add / modify to make them executed
    – gnat
    Commented Aug 7, 2015 at 12:26
  • related discussion: Is it a bad practice to modify code strictly for testing purposes
    – gnat
    Commented Mar 21, 2016 at 12:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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