16

I've been doing TDD for a while now, I feel pretty good about it, I love my test suites and all. However I've noticed that lately I've been doing a lot of mock call verification. For example I'd have a Service that will have a Repository injected - in my unit test I'd pass a mock of the Repository and verify that it was called within the method that I'm testing. I'd then check if the results returned are correct (in another test). This definitely "feels" wrong, since my unit tests are now tightly coupled to the implementation details. I've heard that you should test "behavior", however in a lot of the situations that's ... emm - not possible? If you have a void method for example, you usually test side-effects. I mean it's easy to go ahead and show some simple code-kata's where this can be demonstrated, but IMHO it doesn't reflect very well to the real world programs that we write. Is what I'm doing wrong? Is this type of testing sort of an anti-pattern? I'd appreciate your opinion on this.

3
  • 2
    Short answer: yes. There are very interesting questions about this topic already somewhere here. Your unit tests should not be fragile and depend heavily on your implementation. This is why higher level tests are for (integration, etc.). Here: programmers.stackexchange.com/questions/198453/…
    – Kemoda
    Jul 4, 2013 at 6:46
  • @Kemoda I'd appreciate it if you can link me to a discussion or some further material on this, I'd very much like to improve my techniques. Jul 4, 2013 at 6:48
  • 1
    you have this for instance programmers.stackexchange.com/questions/198453/… i ll find other links later
    – Kemoda
    Jul 4, 2013 at 6:51

5 Answers 5

9

Well, you should be trying to test inputs and outputs. You should be verifying externally visible behavior. The "promises" or "contract" that your class makes.

At the same time sometimes there's no better way to test a method than to do what you said.

I do think that it makes your test more brittle, so you should avoid tests that rely on implementation details if you can, but it's not an all-or-nothing deal. It's OK sometimes, the worst thing that happens is you change the implementation and have to update the test.

1
  • 5
    Well, that's not really the worst thing that could happen? If we ignore the slippery slope potential of planes falling from the sky, a reasonably common bad outcome of testing implementation details is artificial "implementation lock-in" and heavy impedance: "Oh, that change broke a test ... wait, why are we testing for this? Is this implementation detail a requirement? I better ask around ... "
    – svidgen
    Jun 29, 2020 at 16:33
4

The purpose of a test is to restrict the possible productive implementations. Make sure that you only put restrictions on the implementation that you actually need. Typically this is what your program should do, and not how it does it.

So if for example your service adds something to the repository, you should test that the new entry is contained in the repository afterwards, and not that the add action is triggered.

For this to work, you need to be able to use the repository implementation (tested elsewhere) in the test of the service. I found that using the real implementation of a collaborator is generally a good approach – because it is really the best implementation around.


"So but what if using the real implementations in the test is expensive (e.g. because they require resources which are complicated to set up)? I need to use mocks in this case, right?"

In any case you'd probably want one integration test which tests that the real implementations work together. Make sure that this one integration test is all that is needed to test your service. Or in other words: If a service plugs together a lot of collaborators (and is hence potentially hard to test), make sure that it doesn't contain any logic. If it does, and you'd need multiple (integration) tests, you need to change the structure of your code, e.g. by isolating the logic and hence making it more testable.

Using mocks in this case eases the pain of testing a piece of badly isolated logic, and hence hides an architectual problem. So don't use mocks to test badly structured code, but fix the structure instead.

1
  • 2
    I see what you're saying. This topic is a bit confusing as to "how much testing is too much testing", let's say I have an "aggregate service" that basically is a facade and just "glues" together bunch of other services/repositories/components what kind of tests do you write for it ? All I can think of is call verification. I hope I'm making sense. Jul 4, 2013 at 8:54
4

My thoughts re: 'aggregate services'.

Call verification will do this, but won't provide much value. You're just checking your wiring.

There are 3, non-exclusive, other ways to this:

  1. Extend the tests you have for each individual service so that it checks the higher level behaviour. For instance, if your hitting an in-memory database in your unit tests of the service, take it up a level so you're testing the service against an actual db. The service layer is higher up the abstraction tree, and so should your test.

  2. Use code generation to create the service directly from the aggregated services.

  3. Use some sort of reflection or dynamic language to do the same thing. For example, in Java, it may be possible to use a groovy interface, which passes the call on directly.

There are probably other ways to do this, but just checking wiring has very low pay back, and will hard wire you in to this implementation.

2

Call verification testing is an anti-pattern and a waste of time.

Why call a method? There are two reasons:

  • The method returns a value.
  • The method has a side effect.

That's it. A method might do both of those or only one, but if a method doesn't do either of those, then it doesn't do anything at all.

Now consider a line like this:

mockContext.Verify(x => x.SaveChanges(), Times.Once());

What does this test?

  • It doesn't test any value the method might return.
  • It doesn't test any side effect the method might produce.

No, the only thing it tests is whether the method was called, but it says absolutely nothing about why the method was called, or whether the method even did anything when it was called. The method could have done the wrong thing, or even nothing, and this test has nothing to say about it. The entire SaveChanges method body could be commented out or deleted and this "test" would still pass!

Call verification "testing" adds pointless, tightly-coupled "tests" that don't test anything.

6
  • 1
    How do you test side effects, other than by verifying method calls on other objects? IMO calling a method on another object is a side effect, and if that's the point of the method under test, that behaviour is worth testing.
    – PMah
    Mar 29 at 15:39
  • 1
    I wrote this question almost 9 years ago, and I couldn't agree more with you. Since then I've largely ditched TDD (I still use it for some problems that lend themselves nicely for it - not all do!). I don't chase 100% test coverage, but rather try to unit test only what makes sense. An integration test is slower but much more valuable in most scenarios. I guess my point is - do unit tests where it makes sense, do integration tests where it makes sense ("It depends!") - but don't waste your time with call verification testing! Apr 11 at 19:57
  • @PMah Isn't the whole point of a side effect that it has an effect? So, then, the way you test for side effects is to see whether that effect took place. All call verification testing verifies is that the side effect ought to have taken place. But maybe the method is empty. Maybe we're in a configuration such as Debug where the method is excluded due to conditional compilation.
    – Kyralessa
    Jun 26 at 11:39
  • @Kyralessa Sure, but sometimes it's difficult in a unit test to check for the effect, e.g. if the effect is "some data gets saved to a database", or "an external API call is made". In unit testing a business logic class, the database access / API client is usually mocked, so the test becomes "make sure we asked the database layer to save the data" or "make sure we asked the API client to make the call", which is a call verification.
    – PMah
    Jun 26 at 21:10
  • @PMah That shouldn't be a unit test, then. It should be an integration test, either automated or written out as manual steps.
    – Kyralessa
    Jun 27 at 8:54
-2

Basically all the big answers here are correct. I've been throughout the same questions and, unfortunately haven't found something really clear. But then I realized that the answers where always there. I've read Roy Osherove in his wonderful The Art Of Unit Testing book, and also Kent Beck's TDD book, along with some others and lots and lots of reading and streaming about the subject. It's been a while since I saw my first test. So, in order to answer and enforce what was already said here: test the behavior, not the way you write the code. So validate that GetAll returns to a list of objects and that GetById returns the object when found or null (object.empty?). This way you'll mock whatever is inside your implementation and then test how your method behaves (does it respect its contract?).

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