Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am working on a project where class internal calls are usual but the results are many times simple values. Example (not real code):

public boolean findError(Set<Thing1> set1, Set<Thing2> set2) {
  if (!checkFirstCondition(set1, set2)) {
    return false;
  }
  if (!checkSecondCondition(set1, set2)) {
    return false;
  }
  return true;
}

Writing unit tests for this type of code is really hard as I just want to test the condition system and not the implementation of the actual conditions. (I do that in separate tests.) In fact it would be better if I passed functions that implement the conditions and in tests I simply provide some mock. The issue with this approach is the noisiness: we use generics a lot.

A working solution; however, is to make the tested object a spy and mock out the calls to the internal functions.

systemUnderTest = Mockito.spy(systemUnderTest);
doReturn(true).when(systemUnderTest).checkFirstCondition(....);

The concern here is that the SUT's implementation is effectively changed and it may be problematic to keep the tests in sync with the implementation. Is this true? Is there best practice to avoid this havoc of internal method calls?

Note that we are talking about parts of an algorithm, so breaking it out to several classes may not be a desired decision.

share|improve this question
add comment

3 Answers

up vote 6 down vote accepted

Unit tests should treat the classes they test as black boxes. The only thing which matters is that its public methods behave the way it is expected. How the class achieves this through internal state and private methods does not matter.

When you feel that it is impossible to create meaningful tests this way, it is a sign that your classes are too powerful and do too much. You should consider to move some of their functionality to separate classes which can be tested separately.

share|improve this answer
    
I grasped the idea of unit testing long ago and have written a bunch of them successfully. It's just deceiving that something looks simple on paper, it looks worse in code, and finally I'm faced with something that has a really simple interface but requires me to mock half of the world around the inputs. –  allprog Sep 25 '13 at 13:08
    
@allprog When you need to do lots of mocking, it seems like you have too many dependencies between your classes. Did you try to reduce the coupling between them? –  Philipp Sep 25 '13 at 13:40
    
@allprog if you are in that situation, the class design is to blame. –  itsbruce Sep 25 '13 at 13:57
    
It's the data model that causes the headache. It has to adhere to ORM rules and many other requirements. With pure business logic and stateless code it's much easier to get unit tests right. –  allprog Sep 25 '13 at 14:03
    
Unit tests do not necessarily need to handle the SUT as backbox. This is why they are called unit tests. With mocking the dependencies I can influence the environment and to know what I have to mock, I have to know some of the internals as well. But of course this does not mean that the SUT should be changed in any way. Spying, however, allows to do some changes. –  allprog Sep 26 '13 at 7:49
show 1 more comment

If both findError() and checkFirstCondition() etc. are public methods of your class, then findError() is effectively a facade for functionality that is already available from the same API. There's nothing wrong with that, but it means that you have to write tests for it that are very similar to already existing tests. This duplication simply reflects the duplication in your public interface. That is no reason for treating this method any differently from others.

share|improve this answer
    
The internal methods are made public just because they need to be testable and I don't want to subclass the SUT or include the unit tests in the SUT class as a static inner class. But I get your point. However, I couldn't find good guide lines to avoid these kind of situations. Tutorials always stuck on the basic level that has nothing to do with real software. Otherwise, the reason for spying is exactly to avoid duplication of test code and make the tests unit scoped. –  allprog Sep 25 '13 at 11:16
2  
I disagree that the helper methods need to be public for proper unit testing. If the contract of a method states that it checks for various conditions, then there is nothing wrong with writing several tests against the same public method, one for each "subcontract". The point of unit tests is to achieve coverage of all code, not to achieve a superficial coverage of public methods via a 1:1 method-test correspondence. –  Kilian Foth Sep 25 '13 at 11:28
    
Using only the public API for testing is many times significantly more complex than testing the internal pieces one by one. I don't argue, I get that this approach is not the best and it has its hindsight that my question shows. The biggest issue is that functions are not composable in Java and the workarounds are extremely terse. But there seems to be no other solution for real unit testing. –  allprog Sep 25 '13 at 11:42
add comment

Unit tests should test the contract; it is the only important thing, for them. Testing anything that isn't part of the contract is not only a waste of time, it is a potential source of error. Any time you see a developer changing the tests when he changes an implementation detail, alarm bells should ring; that developer may be (whether intentionally or not) hiding his mistakes. Deliberately testing implementation detail forces this bad habit, making it more likely that errors will be masked.

The internal calls are an implementation detail and should only be of interest in measuring performance. Which is not usually the job of unit tests.

share|improve this answer
    
Sounds great. But in reality, the "string" I have to type in and call it code is in a language that knows very little about functions. In theory I can easily describe a problem and make substitutions here and there to simplify it. In code I have to add a lot of syntactic noise to achieve this flexibility that turns me down from using it. If method a contains a call to method b in the same class, then tests of a must include tests of b. And there is no way to change this as long as b is not passed to a as parameter But there is no other solution, I see. –  allprog Sep 25 '13 at 12:29
    
If b is a part of the public interface, it should be tested anyway. If it isn't, it need not be tested. If you made it public just because you wanted to test it, you did wrong. –  itsbruce Sep 25 '13 at 13:59
    
See my comment to @Philip's answer. I haven't mentioned yet but the data model is the source of evil. Pure, stateless code is a piece of cake. –  allprog Sep 25 '13 at 14:05
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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