Search code examples
javaspringmavenmockitojunit4

Mockito UnfinishedStubbingException thrown on a JUnit test case which calls a void ServiceImplementationLayer method


I am using Mockito/JUnit on a Maven project for a console-based application with a DAO design pattern. My IDE is Spring Toools Suite 3.

The problem is that I am getting an UnfinishedStubbingException every time I run this particular test on my particular JUnit test, and I don't know why that is since the syntax looks correct. I am very new to unit testing and Mockito, but I assume that this is occurring because the level of abstraction from one layer to the next is confusing Mockito for some reason. Therefore, I initially tried to use a Spy versus a Mock on the service object in the test case (but then the NotAMockException is being thrown instead).

Any advice and/or recommendations how to resolve this issue will be appreciated.

Here is the stack trace:

org.mockito.exceptions.misusing.UnfinishedStubbingException: 
Unfinished stubbing detected here:
-> at com.revature.testing.BankAccountEvaluationService.testMakeDeposit_ValidUserId(BankAccountEvaluationService.java:91)

E.g. thenReturn() may be missing.
Examples of correct stubbing:
    when(mock.isOk()).thenReturn(true);
    when(mock.isOk()).thenThrow(exception);
    doThrow(exception).when(mock).someVoidMethod();
Hints:
 1. missing thenReturn()
 2. you are trying to stub a final method, which is not supported
 3: you are stubbing the behaviour of another mock inside before 'thenReturn' instruction if completed

    at com.revature.testing.BankAccountEvaluationService.testMakeDeposit_ValidUserId(BankAccountEvaluationService.java:91)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at java.lang.reflect.Method.invoke(Unknown Source)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)

Here is the sample code:

BankAccountEvaluationTest class:

@InjectMocks
private AccountServiceImpl service;

@Mock
private AccountDaoImpl daoMock;

@Before
public void setUp() {
    service = Mockito.spy(new AccountServiceImpl());
    MockitoAnnotations.initMocks(this);
}

@Test
public void testMakeDeposit_ValidUserId() {
    //setup
    Account account = Mockito.mock(Account.class);
    account.setAccountId(1);
    double amount = 20.50;
    //gives UnfinishedStubbingException -> Mockito doesn't like this because it's mocking within a mocking object
    //doNothing().when(daoMock).updateAccountBalance(account.getBalance() + amount, accountNumber); //Solution?
    
    //run
    service.makeDeposit(amount, account.getAccountId());
    
    //verify
    verify(service, times(1)).makeDeposit(amount, account.getAccountId());
}

AccountServiceImpl class:

package com.revature.serviceimpl;
import java.util.List;

import org.apache.log4j.Logger;
import com.revature.dao.AccountDao;
import com.revature.daoimpl.AccountDaoImpl;
import com.revature.model.Account;
import com.revature.service.AccountService;

public class AccountServiceImpl implements AccountService {
    private static Logger logger = Logger.getLogger(AccountServiceImpl.class);
    private AccountDao accountDao = new AccountDaoImpl();
    
    //other overridden methods from AccountService interface

    @Override
    public void makeDeposit(double addedCash, int id) {
        logger.info("Sending deposit request to the database.");
        // find the account
        Account account = accountDao.selectAccountByAccountId(id);
        System.out.println(account);
        // set new balance
        account.setBalance(account.getBalance() + addedCash);
        double myNewBalance = account.getBalance();
        logger.info("New balance: " + account.getBalance());
        logger.info("Updating account balance to account number " + id);
        // update the database
        accountDao.updateAccountBalance(myNewBalance, id);
    }
}

AccountDaoImpl class:

package com.revature.daoimpl;

import java.math.BigDecimal;
import java.sql.Connection;
import java.sql.Date;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.List;

import org.apache.log4j.Logger;

import com.revature.dao.AccountDao;
import com.revature.model.Account;
import com.revature.model.AccountStatus;
import com.revature.model.AccountType;

public class AccountDaoImpl implements AccountDao {
    private static Logger logger = Logger.getLogger(UserDaoImpl.class);

    private static String url = MY_URL;
    private static String dbUsername = MY_DATABASE_NAME;
    private static String dbPassword = MY_DATABASE_PASSWORD;

    //other overridden methods from AccountDao interface

    @Override
    public void updateAccountBalance(double balance, int id) {
        try (Connection conn = DriverManager.getConnection(url, dbUsername, dbPassword)) {

            String sql = "UPDATE accounts SET account_balance = ?  WHERE account_id = ?;";
            PreparedStatement ps = conn.prepareStatement(sql);

            ps.setDouble(1, balance);
            ps.setInt(2, id);
            ps.executeUpdate();
            logger.info("new balance is now set");
        } catch (SQLException e) {
            logger.warn("Error in SQL execution to update balance. Stack Trace: ", e);
        }
    }
}

Solution

  • It seems that there is a some confusion about mocking going on here.

    With the code you have, you would look to write a test for your AccountServiceImpl but mocking out the AccountDao. Instead of using the real DAO which talks to a database, you use a mock DAO which you can set up to return certain values when certain methods are called. You can also query the mock to find out how many times it was called and with what values.

    Mocking domain objects

    In your test it seems you've also chosen to mock out the Account class. You haven't included your Account class in your question but I'm guessing it just has getters and setters. If so, I wouldn't bother mocking it out. Instead of using a mock account, use a real one. Replace the line

            Account account = Mockito.mock(Account.class);
    

    with

            Account account = new Account();
    

    That way you can call the getters and setters on account and they will behave as you expect. If you insist on using a mock account, you would have to use Mockito to implement the getters and setters:

            when(account.getId()).thenReturn(...)        
            when(account.getBalance()).thenReturn(...)
            // ...
    

    You also call account.setId() on your mock account. This will do nothing because you haven't told Mockito what to do when you call .setId() on a mock account. Calling account.setId() on a real account will set the ID.

    You don't have to mock out every single class other than the one being tested. Whether you do depends on what these other classes do. If another class has some complicated logic, or communicates with databases, filesystems, networks, etc., then it can be useful to mock it out, so that your test doesn't have to worry about either the complicated logic or the communication with external systems. In this case, however, let's not bother mocking Account because it doesn't do anything complicated.

    Mock setup

    When creating a mock, all void methods will do nothing and all other methods will return false, zero or null as appropriate. If you want them to do something else, you need to set them up.

    Your service uses your DAO to load an account from the database. You need to set up the mock accountDao to return account when given its ID. Do this by adding the following line to your test before the line that calls service.makeDeposit(...):

            when(daoMock.selectAccountByAccountId(1)).thenReturn(account);
    

    But what about the updateAccountBalance() method? By default, that will do nothing, and you appear to be attempting to set it up to do nothing, which it already does. You can delete the line that attempts to set up this method, as it would achieve nothing. Later on, we will look at verifying this method, i.e. asserting that it was called.

    Mocking inside of mocking

    You were getting an error with this line:

            doNothing().when(daoMock).updateAccountBalance(account.getBalance() + amount, accountNumber)
    

    When setting up one mock, you can't call a method on another mock. Why would you need to? If it's a mock then you should already have set up the mock method to return a certain value, so just use that value instead. In other words, instead of writing

            // if account is a mock...
            when(account.getBalance()).thenReturn(10.00);
            doNothing().when(daoMock).updateAccountBalance(account.getBalance() + amount, accountNumber)
    

    just write

            // if account is a mock...
            when(account.getBalance()).thenReturn(10.00);
            doNothing().when(daoMock).updateAccountBalance(10.00 + amount, accountNumber)
    

    In this line, you are attempting to set up daoMock and are calling account.getBalance(). If account is also mock, this will cause the problem.

    The reason this causes a problem is due to the way Mockito works. Mockito can't see your source code, all it sees is calls to its own static methods and calls to mocks. The line

            doNothing().when(daoMock).updateAccountBalance(account.getBalance() + amount, accountNumber)
    

    causes the following sequence of interactions:

    1. Static Mockito method doNothing() called,
    2. when() method of whatever object doNothing() called,
    3. account.getBalance() method called,
    4. updateAccountBalance() method of the mock DAO called. (We can't call a method until we've evaluated all of the arguments.)

    To Mockito, the first three steps of this are indistinguishable from the following:

            doNothing().when(daoMock);
            when(account.getBalance()).thenReturn(...);
    

    In this case, it is clear that we haven't finished setting up daoMock. We would expect an exception in this case.

    Mockito's syntax leads to clear and expressive mocking, but if care isn't taken you can sometimes end up in situations like this.

    We've already decided to get rid of the line that caused this problem, so this section is more for information and understanding than anything else.

    Verification

    Next, we look at the following lines:

            //run
            service.makeDeposit(amount, account.getAccountId());
    
            //verify
            verify(service, times(1)).makeDeposit(amount, account.getAccountId());
    

    What does this do? You call the makeDeposit method, and then you verify that you called the makeDeposit method. There's really no need to make this verification: you can plainly see that this method is called three lines above.

    Generally, you don't verify methods on the class being tested. Instead, you verify methods on the mocks that your class called. What you want to do instead is to verify that the relevant method on the mock DAO got called with the expected values:

            verify(daoMock, times(1)).updateAccountBalance(account.getBalance() + amount, account.getAccountId());
    

    You can also get rid of the call to Mockito.spy(...). I've never used spies myself, and I don't see the need for them here. Replace the line

            service = Mockito.spy(new AccountServiceImpl());
    

    with

            service = new AccountServiceImpl();
    

    In conclusion

    There's a fair amount here, and hopefully at least some of it makes sense and gives you a better understanding of what's going on. I made the changes above to your test and I got it to pass.