Michał Mielec
Michał Mielec

Reputation: 1651

TDD dilemma: Testing behavior instead of testing state VS Tests should be unaware of implementation

I am trying to implement my Spring website using TDD technique.

There are some TDD rules:

  1. Test behaviour instead of state.
  2. Tests shouldn't depends on implementation.

I created UsersService empty class which depends on crud UsersRepository. Now, I am trying to write test for signing up new users, but I don't know how to do this properly.

@Test
public void signUp_shouldCheckIfUserExistsBeforeSign() throws ServiceException {
    // given
    User user = new User();
    user.setEmail(EMAIL);
    when(usersRepository.save(user)).thenReturn(user);
    when(usersRepository.exists(anyString())).thenReturn(Boolean.FALSE);

    // when
    usersService.signUp(user);

    // then
    thrown.expect(UserAlreadyExistsServiceException.class);
    usersService.signUp(user);
}

This code tests behaviour but also enforce me to implement my service using exists() method instead of findByEmail() for example.

How should this test looks like?

Upvotes: 1

Views: 1257

Answers (2)

aro_tech
aro_tech

Reputation: 1143

Your test seems to reflect some confusion about behavior and implementation. It seems like you expect a state change when you call signUp() the first time, but because you're using mocks I don't think that will happen, so don't call signUp() twice if you're using mocks (and the expect() should be before signUp(), I believe). If you weren't using mocks, it would be a valid test to call signUp() twice, with no implementation dependency, but you're (wisely, IMHO) using mocks to avoid slow, database-dependent tests for easily mockable dependencies, so call signUp() just one time and let the mocks simulate the state. It makes sense to mock your storage interface when testing your service behavior.

As for your 2 testing rules, you can't use mocks without some notion of implementation (I prefer to think of it as "interactions" - especially if you mock interfaces rather than concrete classes). You seem to have a modular design, so I wouldn't worry about simulating an obvious interaction. If you change your mind later about the interaction (whether you should retrieve a user object instead of the boolean existence check), you change your test - no big deal, IMHO. Having unit tests should make you LESS afraid to change your code. It's true that mocks can make tests more brittle if the interactions need to be changed. The flip side of that is you think more about those interactions before coding them, which is good, but don't get stuck.

Your dilemma about whether to retrieve the user by email or to check its existence with a boolean exists() call sounds like a case of YAGNI to me. If you don't know what you're going to do with the User object you retrieve besides checking whether it's null, go with the boolean. If you change your mind later, you may have a few broken tests to (easily) fix, but you'll have a clearer idea of how things should work.

So your test might look like this if you decide to stick with exists():

@Test
public void signUp_shouldCheckIfUserExistsBeforeSign() throws ServiceException {
    // given
    User user = new User();
    user.setEmail(EMAIL);
    when(usersRepository.exists(anyString())).thenReturn(Boolean.FALSE);
    thrown.expect(UserAlreadyExistsServiceException.class);

    // when
    usersService.signUp(user);

    // then - no validation because of expected exception
}

BTW, (this is a side issue, and there are lots of different approaches to expecting exceptions in tests which are covered elsewhere on StackOverflow) it would be nice to be able to put the expect() call in the "then" section, but it has to be before signUp(). You could alternately (if you don't want to call expect() in the "given" section) use the expected parameter of @Test instead of calling expect(). Apparently JUnit 5 will allow wrapping the throwing call in an expectation call that returns the exception thrown or fails if none is thrown.

Upvotes: 2

Mike Stockdale
Mike Stockdale

Reputation: 5266

Testing behavior is good, but the production code will need to exhibit that behavior, which will affect the implementation to some degree.

Focus the test on a single behavior:

@Test
public void signUpFailsIfUserEmailAlreadyExists() throws ServiceException {
    // given
    User user = new User();
    user.setEmail(EMAIL);
    when(usersRepository.emailExists(EMAIL)).thenReturn(Boolean.TRUE);

    // when
    usersService.signUp(user);

    // then
    thrown.expect(UserAlreadyExistsServiceException.class);
}

Upvotes: 0

Related Questions