4

I have a method below which is pretty straightforward. It calls another method that soft deletes an API key and then calls another method to create a new one and returns it.

Test is below also which just checks that the two methods were called correctly. But still getting 0 invocation error on both methods though. What causes this problem?

AuthApiKeyPair updateApiKeyPair(AuthApiKeyPair apiKeyPair, Boolean createNewKey) {

    AuthApiKeyPair newKeyPair

    if (createNewKey) {
        deleteApiKeyPair(apiKeyPair)

        //The key will be created with the same info as the previous key.
        newKeyPair = createApiKeyPair(apiKeyPair.label, apiKeyPair.accountMode, apiKeyPair.source)
    }

    newKeyPair
}

TEST:

def "should soft delete key pair and create new one"() {
    setup:
    AuthApiKeyPair apiKeyPair = AuthApiKeyPair.build(acquirerId: 123, source: PaymentSource.BOARDING_API, label: 'Boarding API key')

    when:
    service.updateApiKeyPair(apiKeyPair, true)

    then:
    1 * service.deleteApiKeyPair(apiKeyPair)
    1 * service.createApiKeyPair(apiKeyPair.label, apiKeyPair.accountMode, apiKeyPair.source)
}
Szymon Stepniak
  • 40,216
  • 10
  • 104
  • 131
kennanwho
  • 171
  • 1
  • 3
  • 14
  • 1
    You want to know why an error occurs, but neither have you posted the error message nor the definition of the crucial element `service`. Information hiding makes this issue a quiz show, the ones you are expecting help from can only speculate. So please update the question, providing at least the definition of `service`. – kriegaex Jun 24 '18 at 07:27

1 Answers1

8

If you think about your test you will realize that in best case scenario it tests Spock's mocking mechanism and not your business code. You haven't shown us full class with your test specification, however basing on your scenario we can assume that service in your test is simply a mock. If this is true, then you can't expect these two invocations:

then:
1 * service.deleteApiKeyPair(apiKeyPair)
1 * service.createApiKeyPair(apiKeyPair.label, apiKeyPair.accountMode, apiKeyPair.source)

to happen. Simply because mocked class does not execute real methods.

I would strongly suggest you testing a real class and not what kind of invocations specific method causes, but what are the expected (and deterministic) results of invoking specific method(s) instead. You could execute service.updateApiKeyPair(apiKeyPair, true) on a real object in when clause and then you could check if the new API key pair was created (and persisted in storage you use) and if the old pair does not exist anymore. Such test has at least a few benefits over checking invocations only:

  • you can change implementation of service.updateApiKeyPair() at any time and as long as it produces the same results your test is still useful (because the test does not limit your implementation like the invocation test does),
  • you test actual behavior and not mocking library - there is this anecdote saying that Mockito is most tested library ever in millions of projects.

Of course it may require some design changes. I'm guessing that your service class uses some injected DAO or repository that persists API key pairs. Consider providing an in-memory implementation of such DAO for your test - a class that instead of persisting objects in a real database stores all objects in the internal ConcurrentMap<K,V>. Thanks to this you can still run your test as a unit test and you can test if updating API key pair with createNewKey parameter set to true does exactly what you expect. Alternatively you could write an integration test with H2 database replacement, but it only makes your test bootstrap much longer. The choice is yours.

There is one rule worth remembering - if your class/component/functionality etc. is hard to unit test, it means that there were made a bad design choices.

Alternative: Spock's Spy objects

There is one thing I mention in the end on purpose. Spock supports so called "spy" objects, that behave like a real objects, but they allow you to stub some parts and treat this object like a mock for e.g. invocation counting. But even Spock authors caution developers about using this feature:

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

Source: http://spockframework.org/spock/docs/1.0/interaction_based_testing.html#spies

I don't know if Grails has an annotation for a test to create Spy instead of a Mock, but you can always follow official documentation and create plain Spock unit test that instantiates your service as a Spy and then you can try counting invocations. I wouldn't suggest doing this though, just mentioning this for the record.

Szymon Stepniak
  • 40,216
  • 10
  • 104
  • 131
  • 1
    This is a very good answer, I just did not want to write a similar for yet another question which does not even provide a full test or application class. The answer is actually too good end extensive for the little effort the OP put into his question. Anyway, big compliment to Szymon. I do wonder why people are so keen on testing the internal implementation of each trivial class by checking interactions instead of trying to do good functional testing first. Interactions should only be tested where necessary for checking the correct implementation of e.g. a design pattern like observer. – kriegaex Jun 24 '18 at 11:30
  • 1
    Thanks @kriegaex for kind words! I think doing interaction based tests is one of the milestones in learning writing good tests journey. I have been there, I did the same and I have learned that there are better ways to test the code. If OP did a TDD cycle he wouldn't even think about testing interactions and he would focus on testing results and corner cases, no matter what the internal implementation is. Making these mistakes is acceptable if they are a side effect of the learning process. And this is what SO is about - to teach and learn from each other. Best! – Szymon Stepniak Jun 24 '18 at 12:14