Overcoming test writer’s block

Reason

If you’ve ever stared blankly at a legacy codebase with zero percent test coverage, trying to figure out how to get started writing tests for that one static class which every other piece of code seems to be referencing, the following can help you take a step towards your first green bar.

Code — writing & rewriting

When writing tests, keep in mind that things might have to get a little worse before they can get better: adding method or constructor overloads to a class that’s already bloated, exposing fields or methods as public purely for test purposes etc.

Unit tests aren’t the only kind of software tests you can write. Depending on the code you’re working on, integration and/or end-to-end tests might be easier to get started with and can have tremendous value. Make sure any external resources being used when writing these kind of tests return consistent results, to avoid flaky tests.

The aim of a test suite isn’t 100% coverage — it’s minimizing the amount of code you don’t have faith in. In other words, your goal should be to write tests until you reach a level of confidence in the system under test that outweighs your fear of change, not necessarily to write tests for every nook and cranny of the codebase!

What characterizes code that’s difficult to test?

In my current daily work I find a class or method difficult to test when it…

  • contains static references, since they’re difficult to mock using non-commercial frameworks (DateTime.Now being a classic example).

  • instantiates dependencies, since it usually means you can’t inject a mock during testing.

  • fetches data from external sources like databases, webservices and files, since it requires the existance/availability of said sources during testing.

  • has lots of side effects and/or responsibilities, as it makes testing only what’s relevant to your current task difficult.

Based on the above, the following summarizes my usual workflow when rewriting C# code to improve testability.

Rewriting a method

This basically boils down to applying the introduce parameter refactoring until the code is testable:

  1. Create an overload of the method whose logic you’d like to test (see “Example – Step 2”).

  2. Pick a static reference / dependency instantiation / external resource access result (e.g. DateTime.Now / new Employee() / IList<Employee> employeesFromDb) in the method and pass it from the original method to the overloaded version as a parameter (see “Example – Step 2” and “Example – Step 3”).

  3. Split the method into smaller pieces when the number of parameters grows too large or use a parameter object — a long list of parameters is often an indication of a method having too much responsibility (see “Example – Step 5”).

  4. Repeat steps 2 and 3 until you’re able to write tests which suit the level of abstraction you’re aiming for (unit, integration, end-to-end).

This should leave the original method doing little more than calling 1–n extracted/overloaded methods and passing in parameters to these.
This in turn leaves you free to write tests by simply passing in POCOs/mocks/primitives to the subparts of the method, and doesn’t require rewriting other parts of the application, as the original method signature is left intact and still has the same output for a given input.

Rewriting a class

Rewriting a non-static class follows much the same refactoring flow as outlined for methods, i.e. by extracting constructor parameters:

  1. Move static references and new‘ed dependencies to overloaded constructors and use constructor chaining (see “Example – Step 1” and “Example – Step 4”).

  2. When confronted with God objects, refactoring the methods within it should open up the opportunity to extract classes; if so, inject them in an overloaded constructor and use constructor chaining.

This should leave the original constructor(s) doing little more than passing parameters to an overloaded constructor chain, which allows you to mock dependencies during testing while leaving the rest of the codebase untouched.

Example

The following code came up during a recent Mob programming session; the goal was to reproduce and fix a bug reg. character encoding in the subject of certain autogenerated emails. The bug was tracked down to the method SendMessage in the class EmailMessageChannel shown below.

Even though the code is fairly well written (e.g. the class as a whole does one thing), the amount of static references and dependency instantiations was perplexing to some of the team when it came to writing unit tests.

The code shown below is the untouched class, highlighting both the SendMessage method and the pain points which made our driver stop in his tracks.

using System;
using System.Linq;
using System.Net.Mail;
using System.Reflection;
using System.Text;
using FormIntegrator.Cache;
using FormIntegrator.Mailers.GenericMailer.SubstitutionProviders;
using log4net;

namespace FormIntegrator.Mailers.GenericMailer
{
  public class EmailMessageChannel : IMessageChannel
  {
    private readonly ILog _logger = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
    private EmailMessageChannelConfig _specificConfig;

    public EmailMessageChannelConfig SpecificConfig
    {
      get
      {
        if (_specificConfig == null)
        {
          _specificConfig = Config as EmailMessageChannelConfig;
          ValidateConfiguration();
        }
        return _specificConfig;
      }
    }

    public IMessageChannelConfig Config { get; set; }

    public void SendMessage(Recipient recipient)
    {
      #region Send Email

      //Get email body
      var rawEmailBody =
        WebCache.Get("FormIntegrator.Mailers.GenericMailer.RawEmailBody." + SpecificConfig.BodyWebText + "." + Task.Current.Ident,
          GetRawEmailBody, 5);

      var finalEmailBody = GenericMailerHelper.SubstituteText(rawEmailBody,
        new SubsitutionParams {CPR = recipient.RecipientId, Recipient = recipient});

      //Construct mail
      var mailer = new SmtpClient();
      mailer.DeliveryFormat = ResolveSmtpDeliveryFormat(recipient.Address);

      var mm = new MailMessage();
      mm.Sender = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
      mm.From = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
      mm.To.Add(new MailAddress(recipient.Address));
      mm.Subject = SpecificConfig.Subject;
      mm.BodyEncoding = Encoding.UTF8;
      mm.Body = finalEmailBody;
      mm.IsBodyHtml = SpecificConfig.IsHtml;
      _logger.Debug("email: " + recipient.Address);

      if (((GenericMailerTask) Task.Current).Test)
      {
        mailer.DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory;
        mailer.PickupDirectoryLocation = ((GenericMailerTask) Task.Current).TestOutputDir;
      }

      mailer.Send(mm);

      #endregion
    }

    private void ValidateConfiguration()
    {
      if (SpecificConfig == null)
      {
        throw new ArgumentException("EmailChannelConfig must be specified", "EmailChannelConfig");
      }
      if (string.IsNullOrWhiteSpace(SpecificConfig.Subject))
      {
        throw new ArgumentException("Cannot be null", "EmailChannelConfig.Subject");
      }
      if (string.IsNullOrWhiteSpace(SpecificConfig.BodyWebText))
      {
        throw new ArgumentException("Cannot be null", "EmailChannelConfig.BodyWebText");
      }
    }

    private string GetRawEmailBody()
    {
      return ContextHelper.GetWebText(SpecificConfig.BodyWebText, Task.Current.Ident);
    }

    private SmtpDeliveryFormat ResolveSmtpDeliveryFormat(string emailAddress)
    {
      var useInternationalFormat = DoesLocalPartOfEmailAddressContainNonAsciiCharacters(emailAddress);
      return useInternationalFormat ? SmtpDeliveryFormat.International : SmtpDeliveryFormat.SevenBit;
    }

    private bool DoesLocalPartOfEmailAddressContainNonAsciiCharacters(string email)
    {
      var localPart = email.Split('@').First();
      var isLocalPartAsciiOnly = IsAscii(localPart);
      return !isLocalPartAsciiOnly;
    }

    private bool IsAscii(string value)
    {
      return Encoding.UTF8.GetByteCount(value) == value.Length;
    }
  }
}

Using the refactoring workflow described earlier, the pain points can be remedied in a few simple steps, as shown in the following.

Step 1 — Remove dependency instantiation (LogManager)

To avoid instantiating _logger during tests we’ll simply constructor inject it.
This enables us to mock an ILog instance during testing by using the overloaded constructor.
The parameterless constructor still creates an object in the same state as previously, hence the rest of the application is blissfully ignorant of our change.

[...]

public class EmailMessageChannel : IMessageChannel
{
  private readonly ILog _logger;
  private EmailMessageChannelConfig _specificConfig;

  public EmailMessageChannel()
    : this(LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType))
  {
  }

  public EmailMessageChannel(ILog log)
  {
    _logger = log;
  }

  [...]
}

Step 2 — Remove static method call (WebCache & ContextHelper)

We’ll add an overload of the SendMessage method and pass in rawEmailBody, which cuts out WebCache.Get and ContextHelper.GetWebText from the list of things we need to deal with when calling the overload in a unit test.
As was the case for Step 1, the application as a whole is unaffected by our change.

[...]

public class EmailMessageChannel : IMessageChannel
{
  [...]

  public void SendMessage(Recipient recipient)
  {
    //Get email body
    var rawEmailBody =
      WebCache.Get("FormIntegrator.Mailers.GenericMailer.RawEmailBody." + SpecificConfig.BodyWebText + "." + Task.Current.Ident,
        GetRawEmailBody, 5);

    SendMessage(recipient, rawEmailBody);
  }

  public void SendMessage(Recipient recipient, string rawEmailBody)
  {
    var finalEmailBody = GenericMailerHelper.SubstituteText(rawEmailBody, new SubsitutionParams {CPR = recipient.RecipientId, Recipient = recipient});

    //Construct mail
    var mailer = new SmtpClient();
    mailer.DeliveryFormat = ResolveSmtpDeliveryFormat(recipient.Address);

    var mm = new MailMessage();
    mm.Sender = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mm.From = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mm.To.Add(new MailAddress(recipient.Address));
    mm.Subject = SpecificConfig.Subject;
    mm.BodyEncoding = Encoding.UTF8;
    mm.Body = finalEmailBody;
    mm.IsBodyHtml = SpecificConfig.IsHtml;
    _logger.Debug("email: " + recipient.Address);

    if (((GenericMailerTask) Task.Current).Test)
    {
      mailer.DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory;
      mailer.PickupDirectoryLocation = ((GenericMailerTask) Task.Current).TestOutputDir;
    }

    mailer.Send(mm);
  }

  [...]
}

Step 3 — Remove static method call (GenericMailHelper)

We’ll move the call to GenericMailHelper.SubstituteText to the initial method and pass in finalEmailBody instead of rawEmailBody.

[...]

public class EmailMessageChannel : IMessageChannel
{
  [...]

  public void SendMessage(Recipient recipient)
  {
    //Get email body
    var rawEmailBody =
      WebCache.Get("FormIntegrator.Mailers.GenericMailer.RawEmailBody." + SpecificConfig.BodyWebText + "." + Task.Current.Ident,
        GetRawEmailBody, 5);
    var finalEmailBody = GenericMailerHelper.SubstituteText(rawEmailBody, new SubsitutionParams { CPR = recipient.RecipientId, Recipient = recipient });

    SendMessage(recipient, finalEmailBody);
  }

  public void SendMessage(Recipient recipient, string finalEmailBody)
  {
    //Construct mail
    var mailer = new SmtpClient();
    mailer.DeliveryFormat = ResolveSmtpDeliveryFormat(recipient.Address);

    var mm = new MailMessage();
    mm.Sender = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mm.From = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mm.To.Add(new MailAddress(recipient.Address));
    mm.Subject = SpecificConfig.Subject;
    mm.BodyEncoding = Encoding.UTF8;
    mm.Body = finalEmailBody;
    mm.IsBodyHtml = SpecificConfig.IsHtml;
    _logger.Debug("email: " + recipient.Address);

    if (((GenericMailerTask) Task.Current).Test)
    {
      mailer.DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory;
      mailer.PickupDirectoryLocation = ((GenericMailerTask) Task.Current).TestOutputDir;
    }

    mailer.Send(mm);
  }

  [...]
}

Step 4.1 — Remove static object reference (Task.Current)

References to Task.Current are moved to the original method, passing in isTest and testOuputDirectory as parameters to the overload.
These properties toggle saving output to a directory rather than sending via SMTP, presumably to do manual verification of message content as the codebase didn’t contain any automated tests.

[...]

public class EmailMessageChannel : IMessageChannel
{
  [...]

  public void SendMessage(Recipient recipient)
  {
    //Get email body
    var rawEmailBody =
      WebCache.Get("FormIntegrator.Mailers.GenericMailer.RawEmailBody." + SpecificConfig.BodyWebText + "." + Task.Current.Ident,
        GetRawEmailBody, 5);
    var finalEmailBody = GenericMailerHelper.SubstituteText(rawEmailBody, new SubsitutionParams { CPR = recipient.RecipientId, Recipient = recipient });
    var isTest = ((GenericMailerTask) Task.Current).Test;
    var testOutputDirectory = ((GenericMailerTask) Task.Current).TestOutputDir;
    SendMessage(recipient, finalEmailBody, isTest, testOutputDirectory);
  }

  public void SendMessage(Recipient recipient, string finalEmailBody, bool isTest, string testOutputDirectory)
  {
    //Construct mail
    var mailer = new SmtpClient();
    mailer.DeliveryFormat = ResolveSmtpDeliveryFormat(recipient.Address);

    var mm = new MailMessage();
    mm.Sender = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mm.From = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mm.To.Add(new MailAddress(recipient.Address));
    mm.Subject = SpecificConfig.Subject;
    mm.BodyEncoding = Encoding.UTF8;
    mm.Body = finalEmailBody;
    mm.IsBodyHtml = SpecificConfig.IsHtml;
    _logger.Debug("email: " + recipient.Address);

    if (isTest)
    {
      mailer.DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory;
      mailer.PickupDirectoryLocation = testOutputDirectory;
    }

    mailer.Send(mm);
  }

  [...]
}

Step 4.2 — Remove static object reference (Task.Current)

Taking the refactoring a step further, all references to the static property Task.Current are replaced with a constructor injected object instance instead.
The task object seems vital enough to the class that it shouldn’t be possible to create an EmailMessageChannel instance without it, hence constructor injection seemed more appropriate in the long run than just adding a parameter to the SendMessage method.

[...]

public class EmailMessageChannel : IMessageChannel
{
  private readonly ILog _logger;
  private readonly GenericMailerTask _task;
  private EmailMessageChannelConfig _specificConfig;

  public EmailMessageChannel()
    : this(LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType), (GenericMailerTask) Task.Current)
  {
  }

  public EmailMessageChannel(ILog log, GenericMailerTask task)
  {
    if (log == null)
    {
      throw new ArgumentNullException(nameof(log));
    }
    if (task == null)
    {
      throw new ArgumentNullException(nameof(task));
    }
    _logger = log;
    _task = task;
  }

  [...]

  public void SendMessage(Recipient recipient)
  {
    var rawEmailBody = WebCache.Get("FormIntegrator.Mailers.GenericMailer.RawEmailBody." + SpecificConfig.BodyWebText + "." + _task.Ident, 
      GetRawEmailBody, 5);
    var finalEmailBody = GenericMailerHelper.SubstituteText(rawEmailBody,
      new SubsitutionParams {CPR = recipient.RecipientId, Recipient = recipient});
    var isTest = _task.Test;
    var testOutputDirectory = _task.TestOutputDir;
    SendMessage(recipient, finalEmailBody, isTest, testOutputDirectory);
  }

  private string GetRawEmailBody()
  {
    return ContextHelper.GetWebText(SpecificConfig.BodyWebText, _task.Ident);
  }

  [...]
}

Mission complete

At this point the overloaded constructor of EmailMessageChannel and overload of SendMessage only relied on abstractions, POCOs and primitives, and were easy to write automated tests for.

But as coding boyscouts it’s our obligation to leave the proverbial campsite cleaner than we found it; since step 4.1 it’s clear that the method SendMessage does two things, depending on the parameter isTest — a case of the flag argument antipattern.
To clarify what each logical branch does we’ll split our overload of SendMessage in two.

Step 5.1 — Refactor flag argument

The first step towards removing the flag argument is extracting the mail message creational logic to a separate method, as we’ll be calling it from two different methods shortly.
A pleasant side effect is that the SendMessage method becomes even more focused and readable.

[...]

public class EmailMessageChannel : IMessageChannel
{
  [...]

  public void SendMessage(Recipient recipient, string finalEmailBody, bool isTest, string testOutputDirectory)
  {
    var mailer = new SmtpClient();
    mailer.DeliveryFormat = ResolveSmtpDeliveryFormat(recipient.Address);

    var mailMessage = CreateMailMessage(recipient, finalEmailBody);

    if (isTest)
    {
      mailer.DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory;
      mailer.PickupDirectoryLocation = testOutputDirectory;
    }

    mailer.Send(mailMessage);
  }

  private MailMessage CreateMailMessage(Recipient recipient, string finalEmailBody)
  {
    var mailMessage = new MailMessage();
    mailMessage.Sender = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mailMessage.From = new MailAddress(recipient.Domain.OutgoingMailAddress, recipient.Domain.NameLong);
    mailMessage.To.Add(new MailAddress(recipient.Address));
    mailMessage.Subject = SpecificConfig.Subject;
    mailMessage.BodyEncoding = Encoding.UTF8;
    mailMessage.Body = finalEmailBody;
    mailMessage.IsBodyHtml = SpecificConfig.IsHtml;
    _logger.Debug("email: " + recipient.Address);
    return mailMessage;
  }

  [...]
}

Step 5.2 — Refactor flag argument

The overload of SendMessage can now be split in two with minimal code duplication.
The extracted method SaveMessage clearly conveys which effect Task.Current.Test == true actually has, thus improving maintainability and adherence to the principle of least astonishment.

[...]

public class EmailMessageChannel : IMessageChannel
{
  [...]

  public void SendMessage(Recipient recipient)
  {
    var cacheKey = "FormIntegrator.Mailers.GenericMailer.RawEmailBody." + SpecificConfig.BodyWebText + "." + _task.Ident;
    var rawEmailBody = WebCache.Get(cacheKey, GetRawEmailBody, 5);
    var subsitutionParams = new SubsitutionParams {CPR = recipient.RecipientId, Recipient = recipient};
    var finalEmailBody = GenericMailerHelper.SubstituteText(rawEmailBody, subsitutionParams);
    var shouldSaveMessageToDisk = _task.Test;
    if (shouldSaveMessageToDisk)
    {
      SaveMessage(recipient, finalEmailBody, new DirectoryInfo(_task.TestOutputDir));
    }
    else
    {
      SendMessage(recipient, finalEmailBody);
    }
  }

  public void SaveMessage(Recipient recipient, string finalEmailBody, DirectoryInfo directory)
  {
    var mailMessage = CreateMailMessage(recipient, finalEmailBody);
    var mailer = new SmtpClient
    {
      DeliveryFormat = ResolveSmtpDeliveryFormat(recipient.Address),
      DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory,
      PickupDirectoryLocation = directory.FullName
    };
    mailer.Send(mailMessage);
  }

  public void SendMessage(Recipient recipient, string finalEmailBody)
  {
    var mailMessage = CreateMailMessage(recipient, finalEmailBody);
    var mailer = new SmtpClient
    {
      DeliveryFormat = ResolveSmtpDeliveryFormat(recipient.Address)
    };
    mailer.Send(mailMessage);
  }

  [...]
}

If you’ve made it this far, thanks for reading and happy coding🙂