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

4 Answers 4

up vote 7 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

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

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

Firstly, I'm wondering what is difficult to test about the example function you wrote? As far as I can see, you can simply pass in various inputs and check to make sure the correct boolean value is returned. What am I missing?

As for spies, the kind of so-called "white-box" testing that uses spies and mocks is orders of magnitude more work to write, not only because there is so much more test-code to write, but any time the implementation is changed, you must also change the tests (even if the interface remains the same). And this kind of testing is also less reliable than black-box testing, because you need to ensure that all that extra test code is correct, and while you can trust that black-box unit tests will fail if they don't match the interface, you can't trust that about code overusing mocks because sometimes the test doesn't even test much real code - only mocks. If the mocks are incorrect, the likelyhood is that your tests will succeed, but your code is still broken.

Anyone that has experience with white-box testing can tell you they're a pain in the ass to write and maintain. Coupled with the fact that they're less reliable, white-box testing is simply far inferior in most cases.

share|improve this answer
    
Thanks for the note. The example function is orders of magnitude simpler than anything you have to write in a complex algorithm. Actually, the question turns out to be more like: is it problematic to test algorithms with spies in several parts. This is not stateful code, all state is separated out into input arguments. The issue is the fact that I want to test the complex function in the example without having to provide sane parameters for the sub functions. –  allprog Jun 21 at 19:31
    
With the dawn of functional programming in Java 8 this has become slightly more elegant but still keeping the functionality in a single class can be better choice in the case of algorithms rather than extracting out the different (alone not useful) parts into "use once" classes just because of testability. In this respect spies do the same as mocks but without having to blow up the coherent code visually. Actually, the same setup code is used as with mocks. I like to stay away from extremes, every type of test may be appropriate in specific places. Testing somehow is lot better than nohow. :) –  allprog Jun 21 at 19:49
    
"I want to test the complex function .. without having to provide sane parameters for the sub functions" - I don't get what you mean there. Which sub functions? Are you talking about the internal functions being used by the 'complex function'? –  B T Jun 23 at 22:24
    
That's what spying is useful for in my case. The internal functions are quite complex to control. Not because of the code but because they implement something logically complex. Moving stuff in a different class is a natural option but those functions alone are not useful at all. Therefore, keeping the class together and controlling it by spy functionality turned out to be a better option. Has worked flawlessly for almost a year, and could easily withstand model changes. I haven't used this pattern ever since, just thought good to mention that it is viable in some cases. –  allprog Jun 24 at 6:51

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.