Unit Testing and good use of matchers

Question

I been having discussion with colleagues recently about good use of matchers. Consider the following sample:

@RunWith(MockitoJUnitRunner.class)
public class LoginRestServiceImplTest {
 @Mock
 private LoginService loginService;
 @InjectMocks
 private LoginRestService loginRestService = new LoginRestServiceImpl();
 private LoginRequest loginRequest;
 private LoginResponse loginResponse;
 @Before
 public void setUp() throws Exception {
 loginRequest = new LoginRequest("Username","Password");
 }
 @Test
 public void thatUserCanLoginWhenUserAndPassCorrect() {
 // Given 
 loginResponse = new LoginResponse();
 loginResponse.setSuccess(true);
 when(loginService.login(anyString(), anyString())).thenReturn(loginResponse);
 // When 
 LoginResponse result = loginRestService.login(loginRequest);
 // Then
 assertTrue("User Can login", result.isSuccess());
 }
 @Test
 public void thatUserCantLoginWhenPasswordIncorrect() {
 // Given 
 loginResponse = new LoginResponse();
 loginResponse.setSuccess(false);
 loginResponse.setError(INCORRECT_PASSWORD);
 when(loginService.login(anyString(), anyString())).thenReturn(loginResponse);
 // When 
 LoginResponse result = loginRestService.login(loginRequest);
 // Then
 assertFalse("User Can't login", result.isSuccess());
 assertEqual("Error Code is Incorrect Password", INCORRECT_PASSWORD, loginResponse.getError());
 }
}

Note this is just a sample. Normally we would use a spring MVC for unit testing a REST service. But is the use of anyString() valid. Does it mean we are being too broad with the mocking criteria?

Is there anything else that is wrong with this style?

• Caridorc: Note this is just a sample. we review real, working code, I fear this is off-topic... Sorry
• npmaster: Valid point, I just omitted the inclusion of the spring MVC stuff to make the code more shorter to read. I can edit it with the original code if that preferred

Answer 1

Well, what you do not actually test is: Are the login parameters passed to the LoginService correctly? In your example, you are simply assuming that the next login will fail or succeed, but you do that completely ignoring the actual parameters.

Especially for the successfull test I would use the actual expected parameters instead of anyString. For the not successfull test you could do the same, return a result with false if the login data is the expected value (otherwise it will be null).

This way you don't just test the way the LoginRestService handles the responses from the LoginService, but also that the LoginRestService gives the correct parameters to the LoginService in the first place, eliminating one possible cause of errors.

• npmaster: Thanks, I haven't thought about the fact we are also verifying that the LoginRestService was sending the correct parameters.