[FIXED] My service test fails when I have an if statement in the class under test SpringBoot

Issue

I am writing tests from my springboot application. The class has a method getUserById which returns Optional<User>. This methos has an if statement that will check whether an row was returned from repository before sending a response. Problem: With the if statement in place, my test always throws the error in the if statement. when I remove the if statement, the test passes. What am I missing?

This is my UserServiceImpl (Class under test)

@Service
@RequiredArgsConstructor
@Transactional
@Slf4j
public class UserServiceImpl implements UserService, UserDetailsService {
    @Autowired
    private final UserRepository userRepository;
    private final PasswordEncoder passwordEncoder;

    @Override
    public List<User> getUsers() {
        log.info("Fetching users");
        return userRepository.findAll();
    }

    @Override
    public Optional<User> getUserById(Long id) {
        log.info("Fetching user id: {}", id);
        Optional<User> user = userRepository.findById(id);
        if (!user.isPresent()) {
            throw new ResourceNotFoundException(MessageUtil.ERROR_USER_NOTFOUND);
        }
        return user;
    }
}

This is my UserServiceImplTest (test class)

@RunWith(SpringRunner.class)
@SpringBootTest
class UserServiceImplTest {

    @MockBean
    private UserRepository userRepositoryTest;

    @InjectMocks
    private UserServiceImpl userServiceTest;

    @Mock
    private PasswordEncoder passwordEncoder;

    private List<User> userSet;
    private User user1;
    private User user2;

    @BeforeEach
    void setUp() {
        userServiceTest = new UserServiceImpl(userRepositoryTest, passwordEncoder);

        Set<ApplicationUserRole> roles = new HashSet<>();
        roles.add(ApplicationUserRole.TEST_USER);
        userSet = new ArrayList<>();

        user1 = User.builder().nickname("test-nickname")
                .id(1L)
                .username("254701234567")
                .roles(roles)
                .password("password")
                .build();

        user2 = User.builder().nickname("test2-nickname2")
                .id(2L)
                .username("254701234589")
                .roles(roles)
                .password("password")
                .build();

        userSet.add(user1);
        userSet.add(user2);

        userSet.stream().forEach(user -> {
            userServiceTest.saveUser(user);
        });
    }

    @AfterEach
    void tearDown() {
    }

    @Test
    void testGetUsers() {
        when(userServiceTest.getUsers()).thenReturn(userSet);
        assertEquals(2, userServiceTest.getUsers().size());
        verify(userRepositoryTest).findAll();
    }

    @Test
    void testGetUserById() {
        when(userServiceTest.getUserById(user1.getId())).thenReturn(Optional.ofNullable(user1));
        assertEquals(1, user1.getId());
        verify(userRepositoryTest).findById(user1.getId());
    }

    @Test
    void testSaveUser() {
        when(userServiceTest.saveUser(user1)).thenReturn(user1);
        assertEquals(1L, user1.getId());
        verify(userRepositoryTest).save(user1);
    }

    @Test
    void updateUser() {
        user1.setNickname("nickname-update");
        when(userServiceTest.saveUser(user1)).thenReturn(user1);
        assertEquals("nickname-update", user1.getNickname());
        verify(userRepositoryTest).save(user1);
    }

}

NOTE: Other tests work just fine

Solution

None of your tests set up the repository mock. You are trying to mock the service method instead, which will implicitly call the real method while mocking. But the service method is never called to assert correct behavior. In other words: your service's behavior is never exercised by the test, because the return value of the method calls is overwritten.

Example:

@Test
void testGetUsers() {
    // repository is never mocked
    //   vvvvvvvvvvvvvvvvvvvvvvvvvv--- this calls the service method
    when(userServiceTest.getUsers()).thenReturn(userSet);
    //                               ^^^^^^^^^^^^^^^^^^^--- this overwrites the return value of the service method
    assertEquals(2, userServiceTest.getUsers().size()); // this uses the overwritten return value
    verify(userRepositoryTest).findAll();
}

To fix, you need to mock the repository (not the service) and then call the real service. It is also quite useless to assert the user's id, because the user is set up by the test, not in the classes under test.

@Test
void testGetUserById() {
    // arrange
    when(userRepositoryTest.getUserById(user1.getId())
            .thenReturn(Optional.ofNullable(user1));
    // act
    Optional<User> userById = userServiceTest.getUserById(user1.getId());
    // assert
    assertEquals(1, user1.orElseThrow().getId());
    verify(userRepositoryTest).findById(user1.getId());
}

I'd also question your usage of verify at the end of test. You are testing implementation details here. You should only be interested in the return value of your service, not which methods of the repository it is calling. Especially since you are mocking those methods anyway, so you already know with which arguments they are called; otherwise the mock would not return the configured return value in the first place.

A side-question is why your if-condition is always true and the exception always thrown. Again: incorrect/missing setup of your mocks.

@Test
void testGetUserById() {
         vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv--- calls real method, but the repository mock is not set up
    when(userServiceTest.getUserById(user1.getId()))
            .thenReturn(Optional.ofNullable(user1));
    assertEquals(1, user1.getId());
    verify(userRepositoryTest).findById(user1.getId());
}

Answered By - knittl
Answer Checked By - David Marino (JavaFixing Volunteer)