2
\$\begingroup\$

I have a @Service to send verification emails. Here is the code of the service:

package my.package.service.impl;

//imports

@Service
public class EmailSenderServiceImpl implements EmailSenderService {

    private final static Logger LOGGER = LoggerFactory.getLogger( EmailSenderServiceImpl.class );

    @Autowired
    EmailVerificationTokenService emailVerificationService;

    @Autowired
    EmailService emailService;

    @Override
    public boolean sendVerificationEmail( final String appUrl, final Locale locale, final UserDto user ) {
        final String token = UUID.randomUUID().toString();
        emailVerificationService.insertToken( user.getIdUser(), token );

        final String body = createEmailBody( appUrl, locale, user, token );

        final String to = user.getEmail();

        final String subject = MessageResourceUtility.getMessage( "e.verify.email.subject", null, locale );

        return sendEmail( body, to, subject );
    }

    private boolean sendEmail( final String body, final String to, final String subject ) {
        try {
            if( emailService.sendEmail( to, subject, body ) ) {
                return true;
            } else {
                LOGGER.debug( "Email NOT sent" );
                return false;
            }
        } catch( final MessagingException e ) {
            LOGGER.error( "Error sending email" );
            return false;
        }
    }

    private String createEmailBody( String appUrl, final Locale locale, final UserDto user, final String token ) {
        appUrl = appUrl.substring( 0, appUrl.lastIndexOf( "/" ) );
        appUrl += "/accountVerification?token=" + token;
        appUrl += "&username=" + user.getUserName();

        final Object[] array = new Object[] { user.getName(), user.getSurname(), appUrl };

        final String body = MessageResourceUtility.getMessage( "e.verify.email", array, locale );
        return body;
    }
}

And here is the test class:

package my.package.app.service;

//imports

@RunWith( PowerMockRunner.class )
@PrepareForTest( MessageResourceUtility.class )
public class EmailSenderServiceTest {

    private static final String URL = "https://www.ysi.si/register/";

    @InjectMocks
    EmailSenderService emailSenderService = new EmailSenderServiceImpl();

    @Mock
    EmailService emailService;

    @Mock
    EmailVerificationTokenService emailVerificationTokenService;

    Locale locale;

    UserDto user;

    @Before
    public void setUp() throws Exception {
        MockitoAnnotations.initMocks( this );
        user = new UserDto();

        when( emailService.sendEmail( Matchers.anyString(), Matchers.anyString(), Matchers.anyString() ) ).thenReturn( true );
        mockStatic( MessageResourceUtility.class );
    }

    @Test
    public void shouldCreateValidationToken() throws Exception {
        emailSenderService.sendVerificationEmail( URL, locale, user );

        verify( emailVerificationTokenService, times( 1 ) ).insertToken( Matchers.anyInt(), Matchers.anyString() );
    }

    @Test
    public void shouldReturnFalseWhenError() throws Exception {
        when( emailService.sendEmail( Matchers.anyString(), Matchers.anyString(), Matchers.anyString() ) ).thenThrow( new MessagingException() );

        final boolean returnValue = emailSenderService.sendVerificationEmail( URL, locale, user );

        assertEquals( "Returns false when error", false, returnValue );
    }

    @Test
    public void shouldReturnFalseWhenEmailNotSent() throws Exception {
        when( emailService.sendEmail( Matchers.anyString(), Matchers.anyString(), Matchers.anyString() ) ).thenReturn( false );

        final boolean returnValue = emailSenderService.sendVerificationEmail( URL, locale, user );

        assertEquals( "Returns false when email not sent", false, returnValue );
    }

    @Test
    public void shouldSendAnEmailWithTheToken() throws Exception {
        final boolean returnValue = emailSenderService.sendVerificationEmail( URL, locale, user );

        verify( emailService, times( 1 ) ).sendEmail( Matchers.anyString(), Matchers.anyString(), Matchers.anyString() );
        assertEquals( "Returns true when success", true, returnValue );
    }

}

I have the feeling I am not unit testing in the proper way. I feel like the test code is too coupled to production code. Am I right?

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

Thanks for sharing the code!

As far as I see you're running your tests through the public interface of the class under test and you verify its results and the communication with its dependencies. That exactly like it should be.

There are just a few issues:

avoid PowerMock

IMHO having to use PowerMock(-ito) is a surrender to bad design. In your case the Problem is the MessageResourceUtility cause the problem.

There is no such rule as that classes providing utility methods must declare them static.

avoid verify with any* matchers

Test cases should be as explicit as possible. That means that you should verify the parameters of the methods called against concrete values by any chance.

Eg. you could verfy that the ID given by the user is passed to the dependency by the cut. Cause here is similar as above: The dependency to UUID class is hidden by the static access. Problem here is that UUID is provided by the JVM so that you cannot simply convert it to an "instance-able" utility class. You should encapsulate that in a facade class which you can pass in as dependency.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.