-1

In my most recent homework, we were provided a class library and told to complete unit testing to ensure the changes we made to comply with SOLID didn't break the code.

The problem I have is that all the methods are void. I may just be ignorant but I don't know how you're supposed to Unit Test Void methods.

Should I just refactor the methods to return something instead of leaving it void? I'll provide one of the methods from the code. I would appreciate some ideas to move me in the right direction. Thank you!


 public class Cart : ICart
    {
        public decimal TotalAmount { get; set; }
        public IEnumerable<OrderItem> Items { get; set; }
        public string CustomerEmail { get; set; }
    }


 public class OrderNotifyCustomer
    {

        public static void NotifyCustomer(Cart cart)
        {
            string customerEmail = cart.CustomerEmail;
            if (!String.IsNullOrEmpty(customerEmail))
            {
                using (var message = new MailMessage("orders@somewhere.com", customerEmail))
                using (var client = new SmtpClient("localhost"))
                {
                    message.Subject = "Your order placed on " + DateTime.Now.ToString();
                    message.Body = "Your order details: \n " + cart.ToString();

                    try
                    {
                        client.Send(message);
                    }
                    catch (Exception ex)
                    {
                        Logger.Error("Problem sending notification email", ex);
                    }
                }
            }
        }
    }
EricSchaefer
  • 25,272
  • 21
  • 67
  • 103
  • 1
    it may help to specify which Unit Testing framework you are using, and provide a sample of a unit test. – Brett Caswell Feb 28 '21 at 00:00
  • 1
    You should certainly not violate what is an otherwise appropriate and expected contract (in this case, a `void` method) solely for the purpose of unit testing. Instead, you should look for other indicators that your method has succeeded without changing or violating that contract. – Jeremy Caney Feb 28 '21 at 01:02
  • If refuse to strictly adhere to the SOLID principles, then your code (and indeed any) can be tested using the so-called unconstrained isolation frameworks, such as: TypeMock, JustMock, MS Fakes - all paid, and their free counterparts: Prig, Ionad.Fody, Pose, Harmony, MethodRedirect (google them). – Alexander Petrov Feb 28 '21 at 08:46

3 Answers3

1

Yes, it is possible to Unit Test void methods. Changing a void to return a value isn't going to increase your test coverage, nor account for behavior that you aren't otherwise testing (like that client.send behavior and expectation handling). A change like that will probably lead to more things to test on in the outer scope (or method that calls and handles the return).

In any scope of testing, you'll reflect on conditionality to calls/behavior and whether objects are instantiated (like SmtpClient client and MailMessage message in this sample and snippets).

Since you cannot determine behavior of client.send here, you'll probably look to add a method that you can fake/stub — refactor this method to do a call to new methods you've defined, using the types that this method instantiated in the scope of the method, respectfully.

public class OrderNotifyCustomer
{
    public static void NotifyCustomer(Cart cart)
    {
        string customerEmail = cart.CustomerEmail;
        if (!String.IsNullOrEmpty(customerEmail))
        {            
            using (var client = new SmtpClient("localhost"))
            {
                SendCustomerNotification(client, 
                    "orders@somewhere.com", 
                    customerEmail, 
                    $"Your order placed on {DateTime.Now.ToString()}", 
                    $"Your order details: \n {cart.ToString()}"
                    );
            }
        }
    }
    
    public static void SendCustomerNotification(SmtpClient client, string senderEmail, string customerEmail, string emailSubject, string emailBody)
    {
        using (var message = new MailMessage(senderEmail, customerEmail))
        {
            message.Subject = emailSubject;
            message.Body = emailBody;
            SendCustomerNotification(client, message); 
        }
    }
    
    public static void SendCustomerNotification(SmtpClient client, MailMessage mailMessage)
    {
        try
        {
            client.Send(message);
        }
        catch (Exception ex)
        {
            Logger.Error("Problem sending notification email", ex);
            throw ex; // we shouldn't suppress this
        }
    }
}

We'll now unit test NotifyCustomer by determining when/whether SendCustomerNotification is called (given the arguments you've passed and expectation).

We'll unit test SendCustomerNotification by providing it mock types of SmtpClient.


Note

You also should distinguish on the difference between integration testing and unit testing. You don't want to actual attempt client.Send; you should mock that type, suppress it's intended behavior, and provide both a scenario where it will throw an exception and won't throw when it is called.

Brett Caswell
  • 1,486
  • 1
  • 13
  • 25
0

You really should not create external dependencies like client where they are used and inject them instead. client seems to be a good candidate for mocking. In your test you could provide a mocked instance of client and formulate an expectation on the message that is passed to it. How that works depends heavily on your unit testing and mocking framework.

EricSchaefer
  • 25,272
  • 21
  • 67
  • 103
0

A "new SomethingImportant" inside a method that you want to unittest is a Red Flag. In your case that means especially that SmtpClient.

Move that actual mail sending stuff to a service outside of this method and class. Have that service implement some interface you define. Then use Dependency Injection to supply the real service for regular use.

BUT for unittesting, supply a mock implementation of that mailsender-interface that doesn't really send any mail but that you can inspect to see whether the Send method was called with expected parameters.

Hans Kesting
  • 38,117
  • 9
  • 79
  • 111