Damir's Corner.

Refactor tests to make them more readable

The key role of tests is to ensure that the software under test behaves correctly. You first add tests to verify that you have correctly implemented a feature. The same tests then serve as a safety net so that you don't inadvertently break existing functionality.

But there's also value in test code being easy to read and understand. When a test breaks you want to quickly see what it is doing and what failed. And that is not going to happen if the test code is long and unstructured. And the only way to achieve that is to treat your test code the same way as your production code: as its complexity is growing you should refactor it.

Let's take a look at a test from my previous blog post:

[Test]
public async Task SlowResponseAfterPointOfNoReturnTest()
{
    SetUpApiResponses(TimeSpan.FromSeconds(0.6), TimeSpan.FromSeconds(0.6));

    using var httpClient = _factory.CreateClient();

    var stopwatch = Stopwatch.StartNew();
    var response = await httpClient.PostAsync(
        "/submissions",
        new StringContent(string.Empty)
    );
    stopwatch.Stop();
    response.EnsureSuccessStatusCode();
    var submissionFromCreate = await response
        .Content
        .ReadFromJsonAsync<Submission>();
    Assert.That(stopwatch.ElapsedMilliseconds, Is.InRange(1000, 1100));
    Assert.That(submissionFromCreate, Is.Not.Null);
    Assert.That(submissionFromCreate.Id, Is.Not.EqualTo(Guid.Empty));
    Assert.That(submissionFromCreate.Phase1CompletedAt, Is.Not.Null);
    Assert.That(submissionFromCreate.Phase2CompletedAt, Is.Null);

    await Task.Delay(TimeSpan.FromSeconds(1));

    var submissionFromGet = await httpClient.GetFromJsonAsync<Submission>(
        $"/submissions/{submissionFromCreate.Id}"
    );
    Assert.That(submissionFromGet, Is.Not.Null);
    Assert.That(submissionFromGet.Id, Is.EqualTo(submissionFromCreate.Id));
    Assert.That(submissionFromGet.Phase1CompletedAt, Is.Not.Null);
    Assert.That(submissionFromGet.Phase2CompletedAt, Is.Not.Null);
}

The test is rather short but there is still some effort required to fully understand what is happening. The code in between the assertion blocks has many responsibilities: it initializes the test case, it makes REST calls, it measures time. This makes it difficult to see what is the actual test scenario.

With some refactoring this can be significantly improved:

[Test]
public async Task SlowResponseAfterPointOfNoReturnTest()
{
    var scenario = new SubmissionScenario(
        _application,
        TimeSpan.FromSeconds(0.6),
        TimeSpan.FromSeconds(0.6)
    );

    var submissionFromCreate = await scenario.CreateSubmissionAsync();
    Assert.That(scenario.LastResponseTimeMilliseconds, Is.InRange(1000, 1100));
    Assert.That(submissionFromCreate, Is.Not.Null);
    Assert.That(submissionFromCreate.Id, Is.Not.EqualTo(Guid.Empty));
    Assert.That(submissionFromCreate.Phase1CompletedAt, Is.Not.Null);
    Assert.That(submissionFromCreate.Phase2CompletedAt, Is.Null);

    await Task.Delay(TimeSpan.FromSeconds(1));

    var submissionFromGet = await scenario.GetSubmissionAsync();
    Assert.That(submissionFromGet, Is.Not.Null);
    Assert.That(submissionFromGet.Id, Is.EqualTo(submissionFromCreate.Id));
    Assert.That(submissionFromGet.Phase1CompletedAt, Is.Not.Null);
    Assert.That(submissionFromGet.Phase2CompletedAt, Is.Not.Null);
}

I have moved the low-level code for making REST calls and measuring response times into separate clearly-named methods. It's now much easier to see that the test only creates a submission and retrieves it after a short delay.

The new helper methods hide all the low-level implementation details:

public async Task<Submission?> CreateSubmissionAsync()
{
    var stopwatch = Stopwatch.StartNew();
    var response = await _httpClient.PostAsync(
        "/submissions",
        new StringContent(string.Empty)
    );
    stopwatch.Stop();
    LastResponseStatusCode = response.StatusCode;
    LastResponseTimeMilliseconds = stopwatch.ElapsedMilliseconds;
    if (!response.IsSuccessStatusCode)
    {
        return null;
    }
    var submission = await response.Content.ReadFromJsonAsync<Submission>();
    if (submission != null)
    {
        _lastSubmissionId = submission.Id;
    }
    return submission;
}

public async Task<Submission?> GetSubmissionAsync()
{
    return await _httpClient.GetFromJsonAsync<Submission>(
        $"/submissions/{_lastSubmissionId}"
    );
}

The methods return only REST responses and store all the other details as local state for easy access from assertions. Even the submission ID is stored locally so that you don't need pass it from one method to the other.

I've also moved all the setup code from the test class:

• The setup method to the same scenario class that hosts the two methods above:

  public class SubmissionScenario : IDisposable
  {
      private readonly WebApplicationUnderTest _application;
      private readonly HttpClient _httpClient;
  
      public SubmissionScenario(
          WebApplicationUnderTest application,
          TimeSpan phase1Delay,
          TimeSpan phase2Delay
      )
      {
          _application = application;
          _httpClient = _application.Factory.CreateClient();
          SetUpApiResponses(phase1Delay, phase2Delay);
      }
  
      // ...
  
      public void Dispose()
      {
          _httpClient.Dispose();
      }
  }
  

• And the code for starting the web application under test to a separate class so that it could be used from different scenario classes:

  public class WebApplicationUnderTest : IDisposable
  {
      public WebApplicationFactory<Program> Factory { get; }
      public WireMockServer Server { get; }
  
      public WebApplicationUnderTest()
      {
          Server = WireMockServer.Start();
          Factory = new WebApplicationFactory<Program>()
              .WithWebHostBuilder(builder =>
              {
                  builder.ConfigureAppConfiguration(
                      (_, config) =>
                      {
                          config.AddInMemoryCollection(
                              new Dictionary<string, string?> {
                                  ["BaseAddress"] = Server.Url
                              }
                          );
                      }
                  );
              });
      }
  
      public void Dispose()
      {
          Factory.Dispose();
          Server.Dispose();
      }
  }
  

This way the test class is only responsible for the test cases it contains. And it's now much easier to share the helper code between multiple test classes if needed without resorting to inheritance and deriving all test classes from a common base class.

I suggest you clone the test project from my GitHub repository and look at it in your favorite IDE. It should make it much easier to see how the code is structured. You can also check how I improved the other tests in the project. And even inspect the exact changes I made to the original unstructured tests since those are still available in the repository as a separate commit.

With some minimum refactoring effort I managed to make the tests much more readable. Of course, further improvements could be made:

• The long blocks of assertions which repeat a lot across all tests could be moved to custom assertions.
• A separate builder class for the scenario would make the initialization code even easier to understand, especially as the number of parameter grows.