Let's refactor a test: AccountController
Tags: csharp, tutorial, dotnet, unittest
Published: 2024-03-11 05:00:00

_I originally posted this post on [my blog](https://canro91.github.io/2020/11/02/UnitTestingTips/). It's part of the [Unit Testing 101](https://canro91.github.io/UnitTesting) 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.

```csharp
[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.

```csharp
[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:

```csharp
[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,

```csharp
[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<T>

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,

```csharp
[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](https://www.udemy.com/course/mastering-csharp-unit-testing-with-real-world-examples/?referralCode=8456B1B78E2EDE923174) on Udemy. Practice with hands-on exercises and learn best practices by refactoring real-world unit tests._

{%cta https://www.udemy.com/course/mastering-csharp-unit-testing-with-real-world-examples/?referralCode=8456B1B78E2EDE923174 %} 🚀 Master C# Unit Testing with Real-world Examples {% endcta %}

_Happy testing!_