I originally posted this post on my blog. It's part of the Unit Testing 101 series. I updated it for accuracy and comprehensiveness.

Let's refactor a test to make it more readable. This test is based on a real test I had to modify in one of my past client's projects.

The test was for an ASP.NET Core API controller: AccountController. This controller created, updated, and suspended user accounts.

This test checks that a configuration object has the sender, reply-to, and contact-us email addresses sent in a welcome email. If a configuration file doesn't have one of those email addresses, the controller throws an ArgumentNullException from its constructor.

This is the test. Take a moment to see what's wrong.

[TestMethod]
public Task AccountController_SenderEmailIsNull_ThrowsException()
{
    var mapper = new Mock<IMapper>();
    var logger = new Mock<ILogger<AccountController>>();
    var accountService = new Mock<IAccountService>();
    var accountPersonService = new Mock<IAccountPersonService>();
    var emailService = new Mock<IEmailService>();
    var emailConfig = new Mock<IOptions<EmailConfiguration>>();
    var httpContextAccessor = new Mock<IHttpContextAccessor>();

    emailConfig.SetupGet(options => options.Value)
        .Returns(new EmailConfiguration()
        {
            ReplyToEmail = "email@email.com",
            SupportEmail = "email@email.com"
        });

    Assert.ThrowsException<ArgumentNullException>(() =>
        new AccountController(
            mapper.Object,
            logger.Object,
            accountService.Object,
            accountPersonService.Object,
            emailService.Object,
            emailConfig.Object,
            httpContextAccessor.Object
        ));

    return Task.CompletedTask;
}

Three...two...one. Let's go.

0. Make it sync

Well, there's no async code inside the body of that test. Take a look again. But, it has a return statement, and its return type is a Task. We don't need that.

That looked weird in a unit test in the first place.

[TestMethod]
public void AccountController_SenderEmailIsNull_ThrowsException()
//     👆👆
{
    var mapper = new Mock<IMapper>();
    var logger = new Mock<ILogger<AccountController>>();
    var accountService = new Mock<IAccountService>();
    var accountPersonService = new Mock<IAccountPersonService>();
    var emailService = new Mock<IEmailService>();
    var emailConfig = new Mock<IOptions<EmailConfiguration>>();
    var httpContextAccessor = new Mock<IHttpContextAccessor>();

    emailConfig.SetupGet(options => options.Value)
        .Returns(new EmailConfiguration()
        {
            ReplyToEmail = "email@email.com",
            SupportEmail = "email@email.com"
        });

    Assert.ThrowsException<ArgumentNullException>(() =>
        new AccountController(
            mapper.Object,
            logger.Object,
            accountService.Object,
            accountPersonService.Object,
            emailService.Object,
            emailConfig.Object,
            httpContextAccessor.Object
        ));

    // return Task.CompletedTask; 👈👈👈
}

1. Use builder methods to reduce the noise

If we take a closer look, our test only cares about one object: IOptions<EmailConfiguration>. All other objects are "noise" for our test. They don't have anything to do with the scenario under test.

Let's reduce the noise with a builder method: MakeAccountController(). It will receive the only parameter the test needs: IOptions<EmailConfiguration>.

After this change, our test looks like this:

[TestMethod]
public void AccountController_SenderEmailIsNull_ThrowsException()
{
    var emailConfig = new Mock<IOptions<EmailConfiguration>>();
    emailConfig
        .SetupGet(options => options.Value)
        .Returns(new EmailConfiguration
        {
            ReplyToEmail = "email@email.com",
            SupportEmail = "email@email.com"
        });

    Assert.ThrowsException<ArgumentNullException>(() =>
        MakeController(emailConfig.Object));
        // 👆👆👆
        // We hid all the noise inside a builder
}

private static AccountController MakeController(IOptions<EmailConfiguration> emailConfig)
{
    var mapper = new Mock<IMapper>();
    var logger = new Mock<ILogger<AccountController>>();
    var accountService = new Mock<IAccountService>();
    var accountPersonService = new Mock<IAccountPersonService>();
    var emailService = new Mock<IEmailService>();
    // We don't need Mock<IOptions<EmailConfiguration>> here...
    var httpContextAccessor = new Mock<IHttpContextAccessor>();

    return new AccountController(
            mapper.Object,
            logger.Object,
            accountService.Object,
            accountPersonService.Object,
            emailService.Object,
            emailConfig,
            // 👆👆👆
            httpContextAccessor.Object);
}

After this step, our test started to look more compact and easier to read. Now it's clear this test only cares about the EmailConfiguration class.

2. Make test values obvious

Now, let's take a look at the test name. It says "sender email is null." If you're like me, when you read this test, you expect to see a variable or property SenderEmail set to null somewhere. But that's not the case.

Let's make the test scenario obvious, really obvious,

[TestMethod]
public void AccountController_SenderEmailIsNull_ThrowsException()
//                            👆👆👆👆👆
{
    var emailConfig = new Mock<IOptions<EmailConfiguration>>();
    emailConfig
        .SetupGet(options => options.Value)
        .Returns(new EmailConfiguration
        {
            SenderEmail = null,
            // 👆👆👆
            // The test value is obvious now
            ReplyToEmail = "email@email.com",
            SupportEmail = "email@email.com"
        });

    Assert.ThrowsException<ArgumentNullException>(() =>
        MakeController(emailConfig.Object));
}

After this step, when we find this test for the first time, we read "sender email is null" on the name and SenderEmail = null on the test body.

It's obvious. We don't have to guess where SenderEmail is set to null. We don't have to decode our test.

3. No more noise: Don't write mocks for IOptions

Finally, to keep reducing the noise, we don't need a mock on IOptions<EmailConfiguration>.

Let's stop using a fake object with the IOptions interface. That would introduce extra complexity or noise to keep following the analogy.

Let's use the Option.Create() method instead,

[TestMethod]
public void AccountController_SenderEmailIsNull_ThrowsException()
{
    var emailConfig = Options.Create(new EmailConfiguration
    //                👆👆👆
    {
        SenderEmail = null,
        ReplyToEmail = "email@email.com",
        SupportEmail = "email@email.com"
    });

    Assert.ThrowsException<ArgumentNullException>(() =>
        MakeController(emailConfig));
}

Voilà! That's way easier to read. Let's remember readability is one of the pillars of unit testing. To write good unit tests, let's avoid complex setup scenarios and hidden test values. Often, our tests are bloated with unneeded or complex code in the Arrange part and full of hidden test values.

Unit tests should be even more readable than production code. Don't make developers decode your tests.

Hey, there! I'm Cesar, a software engineer and lifelong learner. If you want to upgrade your unit testing skills, check my course: Mastering C# Unit Testing with Real-world Examples on Udemy. Practice with hands-on exercises and learn best practices by refactoring real-world unit tests.

🚀 Master C# Unit Testing with Real-world Examples

Happy testing!

Let's refactor a test: AccountController

0. Make it sync

1. Use builder methods to reduce the noise

2. Make test values obvious

3. No more noise: Don't write mocks for IOptions