views:

130

answers:

4

I am looking at the Unit Tests I wrote for an email service (unsing SMTP) and I am wondering if one test inparticular is suffient. Here is a snippet of my email service:

[PluginFamily("EmailService")]
public interface IEmailService
{
    Boolean SendEmail( string toAddress, string fromAddress, string bccAddress,  string ccAddress, string subject,
                       string body, bool html );
}
[Pluggable("EmailService")]
public class EmailService : IEmailService
{
    private IConfigurationReader _configReader;
    public EmailService(IConfigurationReader configurationReader)
    {
        _configReader = configurationReader;
    }
    public bool SendEmail( string toAddress, string fromAddress, string bccAddress, string ccAddress, string subject, string body, bool isHtml )
    {
        MailMessage email = new MailMessage();

        try
        {
            if (_configReader.TestMode)
            {
                toAddress = _configReader.TestEmailAddress;
            }
        }

        //send email here
     }
}

I am mocking IConfigurationReader (basically a wrapper for ConfigurationManager) and setting the test mode = true to test if I can send an email in my "test mode" in my unit test. So my unit test looks something like this (This is one of my unit tests on the method...I have 100% code coverage):

    [Test]
    public void Validate_Send_Email_In_Test_Mode()
    {
        bool result;
        MockRepository mockRepository = new MockRepository();
        var mockConfigReader = mockRepository.StrictMock<IConfigurationReader>();


        using (mockRepository.Record())
        {

            SetupResult.For(mockConfigReader.TestMode).Return(true);
            SetupResult.For(mockConfigReader.TestEmailAddress).Return("[email protected]");
            SetupResult.For(mockConfigReader.EmailContentLocation).Return("test");
            SetupResult.For( mockConfigReader.SmtpHost ).Return( "test.mail.com" );
        }

        ObjectFactory.InjectStub(typeof(IConfigurationReader), mockConfigReader);
        emailService = ObjectFactory.GetInstance<IEmailService>();

        using (mockRepository.Playback())
        {
            result = emailService.SendEmail( "[email protected]",
                                                  "[email protected]", "", "",
                                                  "this is a unit test - config in test mode", "body of unit test", true );

        }

        Assert.That( result, Is.True );
        ObjectFactory.ResetDefaults();

    }

Is this a sufficient enough of a unit test? What can I do to improve it?

I am concerned that just checking that my method is returing a true is not a sufficient enough of a unit test.

+2  A: 

Improve your code coverage (all of the types)

Allen
A lot of people will say code coverage isn't a real great sign of good tests, but it definitely goes a long way, especially if your code coverage is at 2%.If you're new to code coverage, get to 98% and don't worry about throwing every last exception.Just my 2 cents.
Joshua Belden
Allen - I have 100% coverage on the email service. I guess I'm looking to see if my unit test is sufficient enough...
Miyagi Coder
100% function coverage and a decently high branch coverage should be a good indication :)
workmad3
+1  A: 

I would recommend that you also include negative tests that are "successful" when various possible error conditions are met. For example, pass in an invalid email address and verify that the proper error code and/or exception is returned.

You may also want to mock up an actual SMTP server. While I didn't do much searching, I found this SMTP server mock site. I used a method similar to this for my Java project's email plugin testing. That way you don't care if it's in "test mode" or production. The only difference is the server/port combination in your configuration. This has the added benefit of ensuring that you are not just testing the "test code".

Paul Kuykendall
+1  A: 

I would say that this part:

    {
        if (_configReader.TestMode)
        {
            toAddress = _configReader.TestEmailAddress;
        }
    }

Is a code smell. You should be passing in the toe-mail address and making sure that the service gets called with what you passed in as a parameter to the method, and use the mock to ensure that it was sent along to the e-mail api correctly. In otherwords, mock the e-mail api, don't use the Mock as a test subject in and of itself.

It might help you to experiment with writing this the other way around, test first, where the production code only gets written if you can make a test that justifies writing it.

Yishai
+1  A: 

Can sending email fail? If it can, you have at least two cases to test, and your single test is not sufficient.

Since you have a boolean return value, that would suggest to me that you're expecting one of two return codes, so again you're not testing all possibilities.

Not to mention that, when sending email, the return value is the least important of the results of the function: did the email get sent?

ijw