0

I have below piece of code in my spring boot app, which validates email addresses

class EmailValidation {

    public static void validate(List<String> s){
        try {
            for (String address : s) {
                if (s == null || s.indexOf("@") < 0) {  
                    throw new InvalidEmailAddressException("Email address is invalid ");
                }
                new InternetAddress(s);
            }
        } catch(AddressException e){
            LOGGER.Error("Please validate email addresses");
        }
        catch(InvalidEmailAddressesException e){
            LOGGER.error(e.getMessage());
        }
    }

    class InvalidEmailAddressException extends Exception {

        public InvalidEmailAddressException(String message) {
            super(message)
        }
    }
}

I want to write a Junit test which will verify that that InvalidEmailAddressesException was thrown and CAUGHT. How can I do that in JUnit?

Nicholas K
  • 15,148
  • 7
  • 31
  • 57
Jeena
  • 17
  • 1
  • 4
  • Why do you need such test? If the exception is thrown and there is a `catch` for that exception, it will get caught. There is no way that it could bypass that catch block. What are you trying to accomplish? – BackSlash Nov 23 '18 at 16:07
  • Well, for *starters*, you don't use exceptions for non-exceptional conditions in the first place. Someone entering an invalid email is not exceptional behaviour. You're using exceptions as part of normal control flow, which they are not intended for. – Michael Nov 23 '18 at 16:08
  • The comments above already hint at that but what you're trying to test is internal java behaviour, e.g. that when throwing an exception that exception is caught. Numerous tests have been written about that :) Also there' some issues with the snippet you pasted but I'm assuming those are copy-paste errors :) – Thanos Nov 23 '18 at 16:10
  • You shouldn't test internal implementation details. You test behaviour. In this case, the only thing you can test is that an error was logged. Why does your method create a `InternetAddress` and do nothing with it, anyway? – Michael Nov 23 '18 at 16:10

3 Answers3

0

In general I agree with the comments that such a test is probably unnecessary.

However, if I wanted to test something like that I would test the two cases separately and that requires a small modification to your code.

Firstly I would construct a method that only throws the exception if there is one.

public static void checkAddresses(List<String> s) throws AddressException, InvalidEmailAddressException {
    for (String address : s) {
         if (s == null || s.indexOf("@") < 0) {  
             throw new InvalidEmailAddressException("Email address is invalid ");
         }
         new InternetAddress(s);
    }
}

then I would use it in your code like that:

class EmailValidation {

    public static void validate(List<String> s){
         try {
             checkAddresses(s); // a wrapper method that throws the expected exceptions
         } catch(AddressException e){
             LOGGER.Error("Please validate email addresses");
         }
         catch(InvalidEmailAddressesException e){
             LOGGER.error(e.getMessage());
         }
     }

     // add checkAddresses here or somewhere appropriately

     class InvalidEmailAddressException extends Exception {

         public InvalidEmailAddressException(String message) {
             super(message)
         }
     }

}

Then, I would write separate tests for checkAddresses that tests both if an exception is expected or not and separate tests for validate, (possibly with the same input that was given to checkAddresses) that should pass if an exception isn't thrown.

Also, if you would like to verify your logs may be you could try something like that.

Master_ex
  • 789
  • 6
  • 12
0

Indeed using java Exception for common cause is considered a bad practice, and as @Michael said, Exceptions must be exceptional, because

  • they break flow control
  • they are slow (more details here How slow are Java exceptions?)
  • they do not mix with functional paradigm (where Java is in part going to with the addition of lamda-expressions

However, creating a custom object for wrapping validation data is a good thing and InvalidEmailAddressException can be turned into CheckedEmail:

import java.util.List;
import java.util.stream.Collectors;

public class EmailValidator {

    public List<CheckedEmail> validate(List<String> emailAddresses) {
        return emailAddresses.stream().map(this::validate).collect(Collectors.toList());
    }

    public CheckedEmail validate(String emailAddress) {
        String[] emailParts = emailAddress.toString().split( "@", 3 );
        final boolean valid;
        if ( emailParts.length != 2 ) {
            valid = false;
        } else {
            // More validation can go here using one or more regex
            valid = true;
        }
        return new CheckedEmail(emailAddress, valid);
    }

    public static final class CheckedEmail {
        private final String emailAddress;
        private final boolean valid;

        private CheckedEmail(String emailAddress, boolean valid) {
            this.emailAddress = emailAddress;
            this.valid = valid;
        }

        public String getEmailAddress() {
            return emailAddress;
        }

        public boolean isValid() {
            return valid;
        }
    }
}

This in turn can be tested quite easily (and improved with a parameterized test):

import static org.assertj.core.api.Assertions.assertThat;

import java.util.Arrays;
import java.util.List;

import org.junit.Test;

public class EmailValidatorTest {

    private final EmailValidator emailValidator = new EmailValidator();

    @Test
    public void invalid_email() {
        EmailValidator.CheckedEmail checkedEmail = emailValidator.validate("missing.an.at.symbol");

        assertThat(checkedEmail.isValid()).isFalse();
    }

    @Test
    public void valid_email() {
        EmailValidator.CheckedEmail checkedEmail = emailValidator.validate("at.symbol@present");

        assertThat(checkedEmail.isValid()).isTrue();
    }

    @Test
    public void multiple_email_addresses() {
        List<String> emailAddresses = Arrays.asList("missing.an.at.symbol", "at.symbol@present");

        List<EmailValidator.CheckedEmail> checkedEmails = emailValidator.validate(emailAddresses);

        assertThat(checkedEmails)
                .extracting(ce -> ce.getEmailAddress() + " " + ce.isValid())
                .containsExactly(
                        "missing.an.at.symbol false",
                        "at.symbol@present true");
    }
}

If somewhere the point is just to log this, then:

List<EmailValidator.CheckedEmail> checkedEmails = emailValidator.validate(emailAddresses);

checkedEmails.stream()
        .filter(ce -> !ce.isValid())
        .map(ce -> String.format("Email address [%s] is invalid", ce.getEmailAddress()))
        .forEach(logger::error);

Hope this helps !

Loïc Le Doyen
  • 975
  • 7
  • 16
0

Don't approach testing that way. You should test only the specified behaviour of your code, not its implementation details.

If the method you are testing delegates to a method that throws a checked exception, and the method you are testing does not also declare that it throws that checked exception, the compiler will enforce that the method catches the exception. So in that case a unit test is unnecessary.

If the method you are testing delegates to a method that throws an unchecked exception, consult the specification of the method to determine whether it is acceptable for the method under test to also throw (propagate) that exception. If it is not acceptable for it to propagate the exception, then you should create a test case that causes the the method delegated to to throw that unchecked exception. If the method propagates the exception, the test case will fail. How to do that? That depends on the method being delegated to, but in most cases you will need to use Dependency Injection to supply a mock object that throws the exception.

Raedwald
  • 46,613
  • 43
  • 151
  • 237