Let's refactor a test: Payment report
02 Aug 2021 #tutorial #csharpLet’s refactor a test to follow our unit testing best practices. This test is based on a real test I had to modify in one of my client’s projects.
Imagine this test belongs to a payment gateway. The system takes payments on behalf of partners. At the end of every month, the partners get the collected payments discounting any fees. This process is called a payout.
Partners can generate a report with all the transactions associated with a payout. This report can show dates at a different timezone if the user wants it.
This is the test we’re going to refactor. This test is for the GetPayoutDetailsAsync() method. This method finds the payouts in a date range. Then, it shows all transactions related to those payouts. This method feeds a report in Microsoft Excel or any other spreadsheet software.
[TestMethod]
public async Task GetPayoutDetailsAsync_HappyPath_SuccessWithoutTimezone()
{
var account = TestAccount;
var payouts = TestPayouts;
var balanceTransactions = TestBalanceTransactions;
var payments = TestPayments;
var request = new PayoutRequest
{
PageSize = 10,
AccountId = "AnyAccountId",
DateRange = new DateRange
{
StartDate = DateTime.Now.AddDays(-15),
EndDate = DateTime.Now
}
};
var builder = new TypeBuilder<PayoutDetailsService>()
.WithAccount(account)
.WithPayouts(payouts)
.WithBalanceTransactions(balanceTransactions)
.WithPayments(payments);
var service = builder.Build();
var result = await service.GetPayoutDetailsAsync(request);
Assert.IsTrue(result.Any());
builder.GetMock<IAccountService>().Verify(s => s.GetAsync(It.IsAny<string>()), Times.Once);
builder.GetMock<IPayoutWrapper>()
.Verify(s => s.GetPayoutsByDateRangeAsync(It.IsAny<string>(), It.IsAny<DateRange>()), Times.Once);
builder.GetMock<IBalanceTransactionWrapper>()
.Verify(
s => s.GetBalanceTransactionsByPayoutsAsync(It.IsAny<string>(), It.IsAny<string>(),
It.IsAny<CancellationToken>()), Times.Once);
}
This test uses automocking with TypeBuilder. This TypeBuilder<T> creates an instance of a class with its dependencies replaced by fakes using Moq.
Also, this test uses the Builder pattern to create fakes with some test values before building a new instance. This test relies on object mothers to create input values for the stubs.
1. Separate the Arrange/Act/Assert parts
Let’s start by grouping related code to follow the Arrange/Act/Assert (AAA) principle.
To achieve this, let’s declare variables near their first use and inline the ones used in a single place.
[TestMethod]
public async Task GetPayoutDetailsAsync_HappyPath_SuccessWithoutTimezone()
{
// Notice we inlined all input variables
var builder = new TypeBuilder<PayoutDetailsService>()
.WithAccount(TestAccount)
.WithPayouts(TestPayouts)
.WithBalanceTransactions(TestBalanceTransactions)
.WithPayments(TestPayments);
var service = builder.Build();
// Notice we moved the request variable near its first use
var request = new PayoutRequest
{
PageSize = 10,
AccountId = "AnyAccountId",
DateRange = new DateRange
{
StartDate = DateTime.Now.AddDays(-15),
EndDate = DateTime.Now
}
};
var result = await service.GetPayoutDetailsAsync(request);
Assert.IsTrue(result.Any());
builder.GetMock<IAccountService>().Verify(s => s.GetAsync(It.IsAny<string>()), Times.Once);
builder.GetMock<IPayoutWrapper>()
.Verify(s => s.GetPayoutsByDateRangeAsync(It.IsAny<string>(), It.IsAny<DateRange>()), Times.Once);
builder.GetMock<IBalanceTransactionWrapper>()
.Verify(
s => s.GetBalanceTransactionsByPayoutsAsync(It.IsAny<string>(), It.IsAny<string>(),
It.IsAny<CancellationToken>()), Times.Once);
}
Notice we inlined all input variables and move the request variable closer to the GetPayoutDetailsAsync() method where it’s used.
Remember, declare variables near their first use.
2. Show the scenario under test and the expected result
Now, let’s look at the test name.
[TestMethod]
public async Task GetPayoutDetailsAsync_HappyPath_SuccessWithoutTimezone()
{
// Notice the test name...
// The rest of the test remains the same,
// but not for too long
}
It states the GetPayoutDetailsAsync() method should work without a timezone. That’s the scenario of our test.
Let’s follow the “UnitOfWork_Scenario_ExpectedResult” naming convention to show the scenario under test in the middle part of the test name.
Also, let’s avoid the filler word “Success”. In this test, success means the method returns the details without showing the transactions in another timezone. We learned to avoid filler words in our test names when we learned the 4 common mistakes when writing tests.
Let’s rename our test.
[TestMethod]
public async Task GetPayoutDetailsAsync_NoTimeZone_ReturnsDetails()
{
// Test body remains the same...
}
After this refactoring, it’s a good idea to add another test passing a timezone and checking that the found transactions are in the same timezone.
3. Make test value obvious
In the previous refactor, we renamed our test to show it works without a timezone.
Anyone reading this test should expect a variable named timezone assigned to null or a method WithoutTimeZone() in a builder. Let’s make the test value explicit.
[TestMethod]
public async Task GetPayoutDetailsAsync_NoTimeZone_ReturnsDetails()
{
var builder = new TypeBuilder<PayoutDetailsService>()
.WithAccount(TestAccount)
.WithPayouts(TestPayouts)
.WithBalanceTransactions(TestBalanceTransactions)
.WithPayments(TestPayments);
var service = builder.Build();
var request = new PayoutRequest
{
PageSize = 10,
AccountId = "AnyAccountId",
DateRange = new DateRange
{
StartDate = DateTime.Now.AddDays(-15),
EndDate = DateTime.Now
},
// Notice we explicitly set no timezone
TimeZone = null
// ^^^^^
};
var result = await service.GetPayoutDetailsAsync(request);
Assert.IsTrue(result.Any());
builder.GetMock<IAccountService>().Verify(s => s.GetAsync(It.IsAny<string>()), Times.Once);
builder.GetMock<IPayoutWrapper>()
.Verify(s => s.GetPayoutsByDateRangeAsync(It.IsAny<string>(), It.IsAny<DateRange>()), Times.Once);
builder.GetMock<IBalanceTransactionWrapper>()
.Verify(
s => s.GetBalanceTransactionsByPayoutsAsync(It.IsAny<string>(), It.IsAny<string>(),
It.IsAny<CancellationToken>()), Times.Once);
}
If we have more than one test without a timezone, we can use a constant NoTimeZome or an object mother for the PayoutRequest, something like NoTimeZonePayoutRequest.
4. Remove over-specification
For our last refactor, let’s remove those Verify() calls. We don’t need them. We don’t need to assert on stubs.
If any of the stubs weren’t in place, probably we will get a NullReferenceException somewhere in our code. Those extra verifications make our test harder to maintain.
[TestMethod]
public async Task GetPayoutDetailsAsync_NoTimeZone_ReturnsDetails()
{
var builder = new TypeBuilder<PayoutDetailsService>()
.WithAccount(TestAccount)
.WithPayouts(TestPayouts)
.WithBalanceTransactions(TestBalanceTransactions)
.WithPayments(TestPayments);
var service = builder.Build();
var request = new PayoutRequest
{
PageSize = 10,
AccountId = "AnyAccountId",
DateRange = new DateRange
{
StartDate = DateTime.Now.AddDays(-15),
EndDate = DateTime.Now
},
TimeZone = null
};
var result = await service.GetPayoutDetailsAsync(request);
Assert.IsTrue(result.Any());
// We stopped verifying on stubs
}
Voilà! That looks better! Unit tests got our back when changing our code. It’s better to keep them clean too. They are our safety net.
If you’re new to unit testing, read Unit Testing 101, 4 common mistakes when writing your first tests and 4 test naming conventions.
For more advanced content on unit testing, check what are fakes in unit testing and don’t miss the rest of my Unit Testing 101 series.
Happy testing!