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());
}