In my application I have quite a few service classes that act as a facade and delegate most calls to one or more underlying manager classes. I've read very different opinions on how to test such facades. Some people say it's not necessary to test them at all, others say one should test whether the integration is working. I've implemented two test methods for the delegating method below. One test is checking whether the underlying manager method has been called exactly once, the other test checks whether the passed data is equal (or in this case: the same).
I'd love to hear opinions from experienced devs whether testing such methods makes sense and if it does, which method of testing is better.
When performing Unit Testing it is useful to consider what the 'unit' is that you're testing. Perhaps in this example where the ApplicationService performs trivial method delegation it might make sense to consider the ApplicationManager and the ApplicationService as a single logical 'unit' and test all of its functionality through the ApplicationService interface.
I can see a point for your first test, testGetApplicationsListCalled(), but I don't see a point in going through all the gyrations of the other test. Your first test already checks that the call was made. The second test basically checks whether your *mock* object returns what you expect it to return, which obviously it should. That fact is probably masked by all the hand-waving, twisting, and turning that you have to do with the mocking framework that you're using. As a reality check, did you even see that test fail first? If you did, what made it fail? Was it a bug in the production code or something that you did wrong with the mock setup?
As an aside, I just don't get why people would stick with the old JUnit 3 naming convention for tests even when they're obviously using JUnit 4. To me, the redundancy of @Test public void testBlahBlahBlah is just so grating to my clean code sensibilities and I can't imagine why it's not to others. I see this all the time and I just shake my head every time I see it. (Stepping off soapbox)
(Edit: Stepping back up on soapbox again)
This is how I would write that:
That is much easier to read in the JUnit results report. I treat my tests as detailed design specifications and I treat the test names as subheadings for that spec.
Sorry to digress from your original question but I just want to make a point about what I consider is the primary use of testing: to encourage discussions about design and to drive, clarify, correct, and justify design decisions.
I said that I would write that first test this way:
I want to clarify that: That's how I would have initially written it.
I would then have a discussion with my programming partners, which is usually my whole team of 3 to 5 developers. We would ask each other questions like the following:
Is this test name high-level enough? (It's not - it reveals the method that we're delegating to in the facade)
Does it leak any implementation details that it shouldn't? (It does - see last answer)
How can we make this test name more robust and resilient to changes in the underlying implementation? (Hmmm...)
Maybe we'll try this instead:
With this refactoring, we remove the implementation detail that leaks information about the method that was delegated to. We still want to make sure the test name establishes a Facade-Delegate relationship between the ApplicationService and the ApplicationManager class though, so that would be a detail we purposely leak and deem it to be appropriate at this level.
That's the kind of value that testing really brings to the table on my teams.
and thanks for your input. I really like your approach of naming tests after the methods' public API. I wasn't aware there was a "JUnit 4 naming convention". According to https://dzone.com/articles/7-popular-unit-test-naming there's many conventions, which is why I stick to the conventions used at my company. I think consistency is more important here.
I'm all for consistency most of the time but in the case of test names, I favor what makes sense to me over consistency. I think it is more important for test names to be expressive rather to follow an outdated naming convention. In fact I like expressive tests so much I'm starting to move more of testing efforts over to Spock because Spock tests in Groovy make it so much easier for me to write expressive tests. But that's just me; you do what you need to do.
You never said if you saw that second test fail before you made it pass or if you're going to keep it or not. Do you see why it's a pointless test?