HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Using Mockito to verify that an object is saved

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
verifymockitothatusingobjectsaved

Problem

We've been reviewing some legacy code and have found differences in the preferred approach to how to write tests using Mockito.

Both the following tests pass but which one is better and why?

@Test
public void verifyObjectSaved() throws PersistQueueException {
    final UserTokenVo userTokenVo = new UserTokenVo();

    String token = vodafoneSubscriptionService.processTokenRequest(userTokenVo);

    Mockito.verify(userTokenDao).savePayload(isA(String.class), eq(userTokenVo));

    assertTrue(StringUtils.isNotBlank(token));
}


and

@Test
public void verifyObjectSaved() throws PersistQueueException {
    final UserTokenVo userTokenVo = new UserTokenVo();

    doAnswer(new Answer() {
        public Object answer(InvocationOnMock invocation) {
            return "completed";
        }
    }).when(userTokenDao).savePayload(any(String.class), eq(userTokenVo));

    String token = vodafoneSubscriptionService.processTokenRequest(userTokenVo);

    assertTrue(StringUtils.isNotBlank(token));

    verify(userTokenDao, times(1)).savePayload(any(String.class), eq(userTokenVo));
}


Here's the production code it tests — it's not very complicated:

public String processTokenRequest(UserTokenVo userTokenVo) {
    String token;

    try {
        token = new String(TokenFactory.getToken()); 
        if (token != null) {
            userTokenDao.savePayload(token, userTokenVo);
        }
    } catch (Exception e) {
        LOGGER.error("failed to save token");
        token = null;
    }
    return token;
}

Solution

Looking at your production code, my positive test case would check the getToken() and savePayload() methods were invoked; and the same string returned from getToken() was also returned by processTokenRequest().

Some points:

  • I wouldn't use the any matcher with verify(). Explicitly state what you want.



  • No need to return value from a method when you don't use the returned value i.e. doAnswer for savePayload() is unnecessary.



  • As mentioned, times(1) is the default for verify() and used in your first test.



  • Tests shouldn't throw exceptions.



  • Static imports often make short code easy to read.



  • Is isNotBlank() enough?

Context

StackExchange Code Review Q#87171, answer score: 2

Revisions (0)

No revisions yet.