Stub and Verify?

Several times lately I came across Spock tests, which conceptually looked as follows:

    private static final BigDecimal ANY_AMOUNT = new BigDecimal("100.00")
    
    Payment payment = aSamplePayment()
    Client client = aSampleClient()
    
    PaymentCreator paymentCreator = Mock(PaymentCreator)
    PaymentRepository repository = Mock(PaymentRepository)

    @Subject
    Payments payments = new Payments(paymentCreator, repository)

    def "should create outgoing payment for loan"() {
        when:
            Payment actualPayment = payments.saveOutgoingFor(client, ANY_AMOUNT)
        then:
            1 * paymentCreator.createOutgoing(client, ANY_AMOUNT) >> payment
            1 * repository.save(payment)
            actualPayment == payment
    }

Spock allows you to stub and verify collaborator at once. See section 'Combining Mocking and Stubbing'. It is followed by sections which describe couple of discouraged solutions to testing, like partial stubs or stubbing static methods. In my opinion above section should also have this sidenote:

(Think twice before using this feature. It might be better to change the design of the code under specification.)

Design smell!

Consider following implementation under test:

 Payment saveOutgoingFor(Client client, BigDecimal amount) {
        Payment payment = paymentCreator.createOutgoing(client, amount);
        paymentRepository.save(payment);
        return payment;
    }

What it does is creating new payment and saving it in some kind of storage. Actually, the paymentRepository.save() method could return stored payment, so that we could get rid of last line. Such design can be explained like the following "do something with the payment, return it, do something else, and return it". Two actions are being done by two collaborating services. Actually we can cut it to "do something andThen do something else". Rings a bell? It is just function composition and you can read why dependency injection in most cases is just function composition here. What we care about is the result of function composition, which is being done inside one method. We don't care about partial steps and about their implementation. We are interested in the result, to be more precise we check expected state.

I bolded state not without a reason. Classical TDD user would tend to use stubs and check state, mockist TDD user likes mocks and behaviours. More about that you can read in famous Fowler's article. So what is actually wrong in checking partial steps in such scenarios? Well, it leaks implementation details. Now, we force the implementation to actually use PaymentCreator in a fixed way, contrary to stub, which can be desribed as "if you use one of my methods, I'll return this fixture". Both classical and mockists TDD enthusiasts have their rights, but in my opinion those two concepts should not be combined.

I express this opinion to one of my friends and he replied: "But what if your first step does something very important that you want to check. Plus it returns value that is used in next lines. Let's say it sends an email". That is a fair question and for simplicity reasons let's assume that mail sender sends information about payment being created to the client and actually really returns something that we care of later (for example a result indicating mail failure/success). The approach showed at the beginning of this article solves that issue, but...

Test isolation

You want your tests to fail for one reason. That is one of the most ground rules when writing tests. That means that we don't want to check how the payment looks like and if client was informed in the same test. To me, the tests should look like:

def "should create outgoing payment for loan"() {
        given:
            paymentCreator.createOutgoing(client, ANY_AMOUNT) >> payment
        expect:
            payments.saveOutgoingFor(client, ANY_AMOUNT) == payment
    }

    def "should store the payment"() {
        given:
            paymentCreator.createOutgoing(client, ANY_AMOUNT) >> payment
        when:
            payments.saveOutgoingFor(client, ANY_AMOUNT)
        then:
            1 * repository.save(payment)
    }

    def "should inform the client"() {
        when:
            payments.saveOutgoingFor(client, ANY_AMOUNT)
        then:
            1 * mailSender.informClient()
    }

That way every test fail only for one reason. Not more. It is easier to refractor code, it stops rigidity. Rigidity means that the software is difficult to change. A small change causes a cascade of subsequent changes. In that case, small change breaks unrelated tests.

My personal rule of thumb

I (try to) practise TDD all the time. I faced descirbed problems several times, and I came up with my rule when to use mocks and when go for stubs. I am a big fan of CQRS idea and I truly love it how it fits into above considerations. So when my unit test needs to check whether some action was perfomed (command was sent) I choose mocks and verify behaviours. When i rely on my collaborators to return something (queries) I always go for stubs. I don't think that "veryfing what you stubbed" is a good idea.

Written on November 13, 2015
comments powered by Disqus