Unit test for class converting between DTOs and entities

Question

Is this correctly written test? Tested class is responsible for converting from dto to entity class and i wanted to test it. Are these tests useful and well designed? I have a problem understanding how should look such classes.

Tested class:

@Service 
public class ConverterRegisterUserDto extends Converter<RegisterUserDto,User> {
 private final PasswordEncoder passwordEncoder;
 @Autowired
 public ConverterRegisterUserDto(PasswordEncoder passwordEncoder, ModelMapper modelMapper) {
 super(modelMapper);
 this.passwordEncoder = passwordEncoder;
 }
 @Override
 public User convert(RegisterUserDto dto) {
 User user = modelMapper.map(dto, User.class);
 user.setPassword(encodePassword(dto.getPassword()));
 return user;
 }
 private String encodePassword(String password){
 return passwordEncoder.encode(password);
 }
}

Test:

class ConverterRegisterUserDtoTest {
 private PasswordEncoder passwordEncoder;
 private ConverterRegisterUserDto converterRegisterUserDto;
 private RegisterUserDto registerUserDto;
 @BeforeEach
 void init(){
 ModelMapper modelMapper = new ModelMapper();
 passwordEncoder = new BCryptPasswordEncoder();
 converterRegisterUserDto = new ConverterRegisterUserDto(passwordEncoder, modelMapper);
 registerUserDto = getSampleRegisterUserDto();
 }
 private RegisterUserDto getSampleRegisterUserDto(){
 RegisterUserDto registerUserDto = new RegisterUserDto();
 registerUserDto.setUsername("t_username");
 registerUserDto.setPassword("t_password");
 registerUserDto.setEmail("t_email");
 registerUserDto.setClientVerifyAccountUrl("t_url");
 return registerUserDto;
 }
 @Test
 void encodePassword_correctly(){
 //WHEN
 User user = converterRegisterUserDto.convert(registerUserDto);
 //THEN
 assertNotEquals(registerUserDto.getPassword(), user.getPassword());
 assertTrue(passwordEncoder.matches(registerUserDto.getPassword(), user.getPassword()));
 }
 @Test
 void convert_fromRegisterUserDto_toUserDtoClass(){
 //WHEN
 User user = converterRegisterUserDto.convert(registerUserDto);
 //THEN
 assertEquals(User.class, user.getClass());
 }
 @Test
 void convert_fromRegisterUserDto_isMappingCorrectly(){
 //WHEN
 User user = converterRegisterUserDto.convert(registerUserDto);
 //THEN
 assertEquals(registerUserDto.getUsername(), user.getUsername());
 assertEquals(registerUserDto.getEmail(), user.getEmail());
 }
}

I am trying to understand how to write good tests. Is that a good way? Or am I totally missing an idea of testing?

Answer 1

@BeforeEach
void init(){
 ModelMapper modelMapper = new ModelMapper();
 passwordEncoder = new BCryptPasswordEncoder();

You're using actual implementation classes from your unit test. This has implications in that you're also effectively testing these classes, so you have potential side effects from changes when you upgrade the library, performance / dependency considerations. That said, it may be acceptable if you consider these to be reliable library methods with low performance impact/dependencies (nobody mocks out String for example), however if not you might want to consider mocks. I would probably mock out at least the PasswordEncoder, as this would make it easier to control the result of the encoded password.

You're also setting up these dependencies before each test. There doesn't seem to be any need to do this, the same ModelMapper and PasswordEncoder could be used for all of the tests, so I'd tend to put it in @BeforeAll, or even when initialising the fields.

@Test
void encodePassword_correctly(){

Your other tests have three parts to their names. It looks like they are 'methodName_source_result'. MethodName seems to be missing for this test name. Where possible, consistency really does make it easier for other people to understand to expect.

//THEN
assertNotEquals(registerUserDto.getPassword(), user.getPassword());

You don't really need to assert that the encoded password doesn't match the original password, if you're going to test that it matches with the password encoder as well. It just adds noise to the test.

assertTrue(passwordEncoder.matches(registerUserDto.getPassword(), user.getPassword()));

This is checking with your encoder that the returned password would match. As I've said, this is testing more than just your code, however it might be acceptable to you. I'd prefer a pair of tests that explicitly test this interaction something like this:

@Test
void convert_fromRegisterUserDto_toEncodedPassword() {
 //BEFORE
 when(passwordEncoder.encode(any(String.class)))
 .thenReturn("encodedPassword");
 //WHEN
 ConvertMapper.User user = converterRegisterUserDto.convert(registerUserDto);
 //THEN, assert encodedPassword is mapped into returned user.
 assertEquals("encodedPassword", user.getPassword());
}
@Test
void convert_delegatesPasswordEncoding_toPasswordEncoder() {
 //BEFORE
 registerUserDto.setPassword("unencodedPassword");
 //WHEN
 converterRegisterUserDto.convert(registerUserDto);
 //THEN, verify correct password was encoded
 verify(passwordEncoder).encode("unencodedPassword");
}

Which assumes a mock setup in the field/init:

privatePasswordEncoder passwordEncoder = mock(PasswordEncoder.class);
void init() {
 // default to returning emptyString
 when(passwordEncoder.encode(any(String.class))).thenReturn("")

This test seems a bit redundant:

assertEquals(User.class, user.getClass());

Do you really care if it's an actual instance of User, rather than a derived type?

assertEquals(registerUserDto.getUsername(), user.getUsername());

assertEquals always seems counter intuitive to me (I want the expected to be on the right). So I tend to use assertJ:

assertThat(user.getUsername()).isEqualTo(registerUserDto.getUsername());

However, it can also be more descriptive to use constants in your tests to be more expressive. This allows you to see at a glance where the tested information is coming from:

@Test
void convert_fromRegisterUserDto_isMappingCorrectly() {
 //Before
 registerUserDto.setUsername("UserName");
 registerUserDto.setEmail("[email protected]");
 //WHEN
 ConvertMapper.User user = converterRegisterUserDto.convert(registerUserDto);
 //THEN
 assertEquals("UserName", user.getUsername());
 assertEquals("[email protected]", user.getEmail());
}

• Toby Speight: I don't do much Java these days, but in the testing frameworks I'm familiar with for other languages, assert.equals(a, b) is a lot more informative about the values that fail, compared to assert.true(a == b), which can only report that a == b is false. Is the same true for JUnit?
• forsvarir: @TobySpeight Yes, there's not a lot worse than an assert.true comparison where you don't know what either of the values being compared were.