@Test(expected = BadPracticeException.class)
If you're familiar with JUnit 4 (please be familiar with it) you probably have seen or even used such form of expected exception check @Test(expected = SomeException.class)
.
Let's see why it is a bad practice.
Initial test
@Test(expected = ValidationFailedException.class)
public void invalidId() {
User user = new User();
user.setId(-1);
user.setName("bla");
user.validate();
}
// code from AndroidDevSummit Architecture Demo app, sorry Yigit 😅 (please consider making User
immutable).
Improved version of the test
@Test
public void validate_shouldThrowIfUserIdIsInvalid() {
User user = new User();
user.setId(-1);
user.setName("bla");
try {
user.validate();
fail();
} catch (ValidationFailedException expected) {
assertEquals("invalid user id", expected.getMessage());
}
}
Now you can clearly see what method is under the test can throw the exception and actually check the reason via checking the exception message.
Improvements
#######1. @Test(expected = ...) -> try-catch
.
We're isolating method under the test via precise try-catch
on the required method call. This will also protect us from false-positives if setId()
or setName()
or even constructor will throw ValidationFailedException
.
#######2. Explicit fail
.
To show our expectations to the readers and to the test runner. You can use even better method failBecauseExceptionWasNotThrown()
from AssertJ.
#######3. assertEquals("expected exception message", expected.getMessage())
To check that exception was thrown for a right reason. In our case it could also be an exception from the name validation and initial test won't see the difference!
#######4. Test name invalidId -> validate_shouldThrowIfUserIdIsInvalid
.
Such naming is especially helpful if you write and read a lot of tests. methodName_expectedBehavior
or something similar is good approach.
Addition: there is an ExpectedException
rule right in JUnit, take a look! Thanks to osi from comments.
@Test(expected = ...)
is okay for a test containing only one method call and when you definitely know that there is no sense in checking exception message.