Let's say we've been tasked with returning 400 when GET /users/<userId> is called with a negative userId.
The requirement can be turned into a test that hits the endpoint with a negative userId and checks that a 400 is returned:
@Test
public void getUser_InvalidUserId_400() {
expect().statusCode(400).when().get("/users/-1");
}A ubiquitous style of implementation may look something like this: (you may or may not like it, but it's ubiquitous)
public class UserResource {
@Inject
private UserService userService;
@GET
@Path("/users/{userId}")
public Response getUser(@PathParam("userId") final Long userId) {
try {
return Response.ok(userService.findById(userId)).build();
} catch (final UserException e) {
return Response.status(e.getErrorCode()).build();
}
}
}public UserService {
@Inject
private UserDao userDao;
public User findById(final Long userId) throws UserException {
if ((userId == null) || (userId <= 0)) {
throw new UserException("Invalid arguments", 400);
}
return userDao.findById(userId);
}
}Everyone knows a test hitting the endpoint is not enough- more tests are required. A ubiquitous style of additional test may look something like this:
public class UserResourceTest {
@InjectMocks
private UserResource userResource = new UserResource();
@Mock
private UserService userService;
@Test
public void getUser_InvalidParams_400() throws UserException {
doThrow(new UserException(INVALID_ARGUMENTS, 400)).when(userService).findById(-1L);
assertThat(userResource.getUser(-1L).getStatus(), is(equalTo(400)));
}
}Let's say the negative userId check is removed from the UserService. (by mistake or whatever)
The test that hits the endpoint will fail, because it will no longer get a 400 when GET /users/<userId> is called with a negative userId. This is exactly what we want out of a test!
The additional test however will pretend the negative userId check is still in the service and pass. This is literally the exact opposite of what we want out of a test!
Or, instead, let's say the UserResource is refactored to use a NewBetterUserService. (the NewBetterUserService still throws an exception on negative userId)
The test that hits the endpoint will pass, because it will still get a 400 when GET /users/<userId> is called with a negative userId. This is exactly what we want out of a test!
The additional test however will fail because it expects the UserResource to call the (old) UserService. This is literally the exact opposite of what we want out of a test!
Maybe there's a UserServiceTest that would fail if the negative userId check is removed from the UserService:
public class UserServiceTest {
@Test
public void findById_InvalidParams_400() throws UserException {
expectedException.expect(UserException.class);
expectedException.expectMessage("Invalid arguments");
assertThat(expectedException.errorCode(), is(equalTo(400)));
userService.findById(-1L);
}
}It's irrelevant. The UserResourceTest test is still an anti-test, because it fails on refactoring. And even if there is a UserServiceTest that fails if the negative userId check is removed from the UserService, the UserResourceTest doesn't fail if there is no such UserServiceTest, or if there is but it's incorrectly implemented etc. It just pretends the negative userId check is in the UserService and passes.
If you were absolutely adamant about testing the implementation (which you shouldn't be, because implementation tests fail on refactoring, making them anti-tests), the "correct" way of testing that would be:
public class UserResourceTest {
private UserResource userResource = new UserResource(new UserService()); //not a mock
@Test
public void getUser_InvalidParams_400() throws UserException {
assertThat(userResource.getUser(-1L).getStatus(), is(equalTo(400)));
}
}Because now, if the negative userId check is removed from the UserService the test will fail. But since it also fails on refactoring, it's still an anti-test.
Anti-tests like the ones described are worse than useless- they are positively harmful.
The test that hits the endpoint is commonly referred to as a "integration test", and the other test is commonly referred to as a "unit test". (I even used that terminology myself in the original post, see the gist commit history)
Actually, Kent Beck (the originator of test-driven development) defines a unit test as "a test that runs in isolation from other tests". This is very different from the definition of a unit test as "a test that tests a class/method in isolation from other classes/methods". The test that hits the endpoint satisfies the original definition.
But it doesn't really matter if you want to call a given test an "integration" test or a "unit" test. The point is the test fails when something breaks and passes when something is improved. If it does the opposite, it's an anti-test.
- http://googletesting.blogspot.com.au/2013/08/testing-on-toilet-test-behavior-not.html
- http://codebetter.com/iancooper/2011/10/06/avoid-testing-implementation-details-test-behaviours/
There are other problems with the example, such as eg
- Primitive Obsession: using a Long to represent
userIdand using procedural inline checks in the service to check for negativeuserId. Instead, create a classUserIdthat encapsulates and enforces its own checks. - Hidden Dependencies:
UserResourcehas a hidden dependency onUserServiceandUserServicehas a hidden dependency onUserDao. Dependency Injection frameworks like Spring encourage such hidden dependencies, and the use of mocks in unit tests. - The service shouldn't know about HTTP error codes. (it's just a simplification in order to keep the example short)