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?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 3, 2017 at 13:48
\$\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.

answered Jul 3, 2017 at 14:42
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.