Top Down TDD

The software culture I operate in is strongly influenced by Extreme Programming (XP) and a big part of this is the importance of test driven development (TDD). We typically don’t approve PRs without unit tests, as a concrete example. This post will discuss using TDD to design and implement a feature in a top-down fashion. The examples are in C# and ASP.NET, but I expect them to be understandable for people using different languages and frameworks.

Bottom up design Link to heading

I am usually inclined to do bottom-up design, wherein I think about the problem at hand - typically a particular feature to implement - and break it down into parts which are the components I am confident I will need. In the case of C#, the components are often new interfaces and classes, and occasionally assemblies, or modifications of existing ones.

I then start writing tests for one of those components, implement the behavior, and then continue with the next component. Then I write a few tests that verifies the behavior of the entire feature, where I use the components to compose the solution.

This works well for small to medium issues where I can envision the entire solution, and the components needed in construction. In general, I think bottom-up works well for people, who, like me, have a lot of software design experience and thus a good feel for what parts your design will need.

Top-down design Link to heading

However, occasionally, I am faced with a problem space when initially it is unclear to me what a good solution would look like, and thus unclear what components I’d need to use to compose the solution, so I don’t really know what the first test should do.

I usually then turn to an experimental mode where I prototype one or more solutions just long enough to come up with a good enough solution to the problem, and which components I’d need, and then continue using my usual bottom-up design style.

But this situation is also where a top-down approach might be better, so when I recently implemented a feature in a web application, I decided I would sharpen my top-down design skills, by designing the feature in a top-down fashion to contrast with my usual bottom-up style, even though I had a fairly good idea of the components involved.

The use case Link to heading

The feature that needed implementing was to provide an endpoint that lets you download log messages from the application, from a known location on the file system. The log messages are stored in a single file per day, named logs-yyyymmdd.log. The spec does not say in which format the logs are, or which format the downloaded log messages should be in.

I knew the downloaded logs are not ultimately meant to be read by the client, they are to be extracted by the client, and sent to our support team to help the client with troubleshooting. We cannot assume internet connectivity to allow the support team to access the logs themselves.

Let’s assume we can just have the client download a zip archive to send. We can further assume that this implementation does not need to parse the logs.

Setup Link to heading

Adding a user-facing feature to a web application typically needs modifications to both the backend and the frontend. If we were truly doing top-down TDD, we should start with a browser UI test engine, such as Cypress, to start with a test that accesses the user interface. You might think that UI-driven tests are cumbersome to write, but I could argue that the reason for that is that we typically only write UI tests as “test-after”, not in a test-driven manner. But I’ll have to expand on TDDing the UI at a different time. This time, I started with the backend implementation.

If you want to understand the context for the tests and implementation in this post, you can reference this example from Microsoft where we have an ASP.NET Core MVC project for the controller, an test project that references the MVC project (and Microsoft.AspNetCore.Mvc.Testing), and a test class with a WebApplicationFactory from which we have gotten a HttpClient, enabling us to communicate with our ASP.NET Core MVC controller.

The first test Link to heading

I decided the first test would be to verify the existence of a route to download the logs. In a bottom-up approach, this would be a test I’d write much later in the process.

    [Test]
    public async Task there_is_a_logs_endpoint()
    {
        var response = await _httpClient.GetAsync("/api/admin/logs");

        Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK));
    }

Run the test, to ensure it does not accidentally pass. Then add a rudimentary controller to the MVC project.

[ApiController]
[Route("/api/{controller}")]
public class AdminController : ControllerBase
{
    [HttpGet("logs")]
    public IActionResult GetLogs()
    {
    	return Ok();
    }
}

and run the tests again to see them pass.

After this, it is just a matter of adding tests to drive the rest of the implementation. A.k.a Draw the rest of the effing Owl :)

Why not a more demanding first test? Link to heading

But seriously, though, why don’t we write a first test that forces most of the implementation?

This is for various reasons. First it suffers from the Ketchup Effect, in that we will get nothing, nothing and then the entire implementation at once when were almost finished with this part. This risks delaying code review feedback. Maybe it is just an hour or two of work to get your test green, in which case it would be fine, but maybe it is more than a day. In my opinion this makes the test feedback loop way too long, and we don’t really do TDD at that point, as we don’t get much design feedback from our tests if we don’t get the red/green/refactor cycle going (or, as I like to say, red/green/reflect) and landing on a state of all tests passing quite often.

The reason I say red/green/reflect instead of is that you should only refactor if, upon reflection, you need to. Don’t feel forced to rearrange the code just because the recipe says so. But do take a second to reflect on the code and the design, and where it is headed, and refactor the code, or the design, if it didn’t turn out as neat as planned.

Second the tests would stay red most of the time, and this can hide other issues. If you are doing trunk-based development, you cannot do a PR until you’ve made all tests green, and this drives the Ketchup Effect. It is much easier to detect a deviation from the entire test suite being green, than from “currently some tests are red”. If you realize you need to add component A before you can complete your current component, then the failing tests for X and the unfinished state of X may become something of a distraction or hindrance. This is certainly the case if it would take many days to implement the feature to the point where the test passes, and I think this is misapplying top-down TDD - I’d certainly switch to bottom-up mode long before that.

Check the content type Link to heading

We can check the Content Type, which I expect to be application/octet-stream. There seems to be several possible choices for a zip archive so I just picked one.

    [Test]
    public async Task logs_has_expected_content_type()
    {
        var response = await _httpClient.GetAsync("/api/admin/logs");

        Assert.That(response.Content.Headers.ContentType?.ToString(), Is.EqualTo("application/octet-stream").IgnoreCase);
    }

The diff for the implementation is just replacing the Ok response with a Content response.

-	return Ok();
+	return Content("fake log content", "application/octet-stream");

Check the file name Link to heading

We should also be providing a file name to the user. We also need to allow download of logs from a different day than today. Knowing that the specification states there’s a log file per day, we can write a test to pass a date using a query parameter, and check that this date is reflected in the returned file name. This information is found in the Content Disposition header.

Let’s make this a data driven test, to verify that we are not just hard coding the file name.

    [TestCase("20221221")]
    [TestCase("20010102")]
    public async Task logs_has_expected_attachment(string dateFilter)
    {
        var response = await _httpClient.GetAsync($"/api/admin/logs?q={dateFilter}");

        Assert.That(response.Content.Headers.ContentDisposition?.FileName, Is.EqualTo($"logs-{dateFilter}.zip"));
    }

To make this pass, first we need add the query parameter to the controller GetLogs method. In ASP.NET MVC this is done with a FromQuery attribute.

-    public IActionResult GetLogs()
+    public IActionResult GetLogs([FromQuery(Name = "q")] string? dateFilter)

Further, there’s a File result helper in ASP.NET MVC we can use. To make the tests pass, it is sufficient to provide an empty file for now, and use the query parameter to help name the returned file.

-	return Content("fake log content", "application/octet-stream");
+	return File(Array.Empty<byte>(), "application/octet-stream", $"logs-{dateFilter}.zip");

Verify the date parameter Link to heading

Let’s check the dateFilter passed in is actually parseable as a date. If it isn’t, we can signal failure by returning BadRequest. This is also a good candidate for a data driven test. instead of copy-pasting the same test and just making minor adjustments manually to check various inputs.

    [TestCase("20221101", HttpStatusCode.OK)]
    [TestCase("12345678", HttpStatusCode.BadRequest)]
    [TestCase("humbug", HttpStatusCode.BadRequest)]
    public async Task logs_query_param_must_be_valid_date(string dateFilter, HttpStatusCode expected)
    {
        var response = await _httpClient.GetAsync($"/api/admin/logs?q={dateFilter}");

        Assert.That(response.StatusCode, Is.EqualTo(expected));
    }

To make this pass, we need to add a check in the Controller. We also don’t want the current thread culture on the server to affect the outcome, so we use InvariantCulture. I didn’t actually write a test for this detail, as it is a bit cumbersome to set up a test where I can vary the culture of the server response thread at will.

     public IActionResult GetLogs([FromQuery(Name = "q")] string dateFilter)
     {
+        if (!DateTime.TryParseExact(dateFilter, "yyyyMMdd", System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.None, out var date))
+            return BadRequest("q was not a yyyyMMdd date");
+

Check that we get a zip archive Link to heading

Now, I want to verify that we actually return a proper zip archive.

    [Test]
    public async Task logs_returns_zip_archive()
    {
        var response = await _httpClient.GetAsync("/api/admin/logs?q=20221101");

        Assert.DoesNotThrow(() => _ = new ZipArchive(response.Content.ReadAsStream()));
    }

To make this pass, let’s make the controller return an empty ZipArchive:

    public IActionResult GetLogs([FromQuery(Name = "q")] string dateFilter)
    {
        if (!DateTime.TryParseExact(dateFilter, "yyyyMMdd", System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.None, out var date)
            return BadRequest("q was not a yyyyMMdd date");

        // write the archive to a memory stream
        var stream = new MemoryStream();
        using (var archive = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true))
        {
            // TODO: write log content to archive
        }
        // then rewind the stream to the beginning
        stream.Seek(0, SeekOrigin.Begin);

        return File(stream, "application/octet-stream", $"logs-{dateFilter}.zip");
    }

Check the single file entry in the zip archive Link to heading

To detect that this implementation isn’t doing what we want, we can check that there’s a single file entry in the ZipArchive with the expected name:

    [Test]
    public async Task logs_returns_zip_archive_with_the_expected_single_entry()
    {
        var response = await _httpClient.GetAsync("/api/admin/logs?q=20221101");
        var entries = new ZipArchive(response.Content.ReadAsStream()).Entries;

        Assert.That(entries.Single().Name, Is.EqualTo("logs-20221101.log"));
    }

And then provide an implementation that adds such an (empty) file entry to the ZipArchive.

         using (var archive = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true))
         {
-            // TODO: write log content to archive
+            var entryName = $"logs-{dateFilter}.log";
+            var entry = archive.CreateEntry(entryName);
+            // TODO: write log content to entry
         }

Now I had trouble coming up with further tests that didn’t include the log contents, so I think it is time to test the reading of the logs in our tests.

Verify the log contents Link to heading

Let’s write a failing test that verifies that if the logs contain some known log data, this ends up in our downloaded ZipArchive.

    [Test]
    public async Task logs_returns_the_expected_log_contents()
    {
        var response = await _httpClient.GetAsync("/api/admin/logs?q=20221101");
        var archive = new ZipArchive(response.Content.ReadAsStream());

        using var streamReader = new StreamReader(archive.Entries.Single().Open());

        var lines = new List<string?>();
        while(!streamReader.EndOfStream)
        {
            lines.Add(await streamReader.ReadLineAsync());
        }

        Assert.That(lines, Is.EquivalentTo(new []{"SOME", "LOG", "ENTRIES"}));
    }

This will fail because we are just returning an empty zip file entry. There are several ways to make this new test pass

• We could take actual logs that the application previously wrote to file, include them in the test project, and have the controller read the logs from there, and check that the first few lines match.
• We could manually create a log file with these entries, and have the controller read that.
• We could add a log reader constructor parameter to the controller, which would enable us to inject a fake log reader component in our tests, and can provide us with a known set of log entries.

The log reader constructor parameter is a great fit for Dependency Injection, which is already heavily used in a typical ASP.NET project, and I think this is the better option here.

Having the tests read actual log files would provide some evidence that we can handle the real log format, but we would probably have to do a manual copy of a real log file into the test project, and then remember to keep the log file up-to-date if the log format ever changed, to keep that benefit. It would be better to provide an integration test that started the application, and then read some of the real logs just written, but I’ll leave that out of this post.

Adding a Logs Reader abstraction Link to heading

Let’s rewire things so we are able to inject a fake logs reader implementation during testing, that will pretend to read those lines from the logs. To be able to use DI, we add an interface ILogsReader, which can be empty for now.

To configure DI for tests, we can add an empty test class FakeLogsReader, implementing ILogsReader, which we leave empty for now, and add this registration to test DI, much like as described in the integration test example from Microsoft I mentioned earlier:

	services.AddSingleton<ILogsReader, FakeLogsReader>();

If we add an ILogsReader constructor parameter in our controller, it will be provided with a FakeLogsReader during testing.

+    private readonly ILogsReader _logsReader;
+
-    public AdminController()
+    public AdminController(ILogsReader logsReader)
+    {
+        _logsReader = logsReader;
     }

Also, the implementation of GetLogs in the controller needs to be changed to utilize our new reader. Let’s imagine we have a ReadLogEntries method that returns a list of log entries, this would help us implement the following:

             var entry = archive.CreateEntry(entryName);
-            // TODO: write log content to entry
+            using (var writer = new StreamWriter(entry.Open()))
+            {
+                foreach (var logEntry in _logsReader.ReadLogEntries(date))
+                    writer.Write(logEntry + "\n");
+            }

For this to compile, we need to add ReadLogEntries to the ILogsReader interface.

public interface ILogsReader
{
	IEnumerable<string> ReadLogEntries(DateTime date);
}

and we’ll need to add an implementation of this method to our implementations of ILogsReader.

We can make FakeLogsReader provide the expected entries directly.

public class FakeLogsReader : ILogsReader
{
    public IEnumerable<string> ReadLogEntries(DateTime date) => new [] {
    	"SOME", "LOG", "ENTRIES"
    };
}

Now the test should pass.

Improvements to the tests Link to heading

A more sophisticated approach, and one that I would go for in a production setting, would be to allow each test to tell the FakeLogsReader what log contents should be returned for the following request, but this would require some plumbing to set up DI-overrides per test, and care that you are not affecting other tests if a singleton FakeLogsReader is used from many tests in parallel.

Also worth mentioning is that there’s some duplication creeping up in the tests, and I’d refactor the tests to use some helper functions, for example one that does the request, and unpacks the returned ZipArchive, and one that builds the logs request URL. Removing duplication is something most developers naturally do in production code, but it is also important to avoid duplication in test code. Don’t go complaining TDD “slows you down” when you needed to change the same thing in 50 different tests, just because you didn’t care about the duplication when you wrote the tests.

However, I’ve left this cleanup out of this post.

Adding a Logs Packager abstraction Link to heading

At this point, when all tests were passing, I decided to refactor the controller, to move the business logic for packaging log entries into a ZipArchive out of the body of GetLogs.

In general, controllers should be very thin layers on top of your business logic, and you should put a minimum amount of business logic in them. As the project grows, and you need to verify more complex interactions in the business logic, it becomes increasingly complex and time-consuming to verify via the controllers (both in effort, and time to run the tests).

What we could be testing using a method call, is instead using a simulated browser, doing routing, parameter binding, authentication and authorization, serialization and deserialization, just to verify the details of the behavior. To be clear, we want to test those things, but we don’t need to include them in each and every test. Stuffing the controller full of business logic also makes it harder to re-use the business logic elsewhere in a non-HTTP context.

To this end I added a new interface ILogsPackager

public interface ILogsPackager
{
    Stream GetLogArchive(DateTime date);
}

and an implementing class, with our implementation so far.

The LogsPackager below is not a test class or “fake” implementation, as we don’t need a test version of the packaging itself. As it does not depend on external things apart from the ILogsReader, we only need the real implementation of the packager. This implementation is then easily tested in isolation by providing it with a fake ILogsReader.

public class LogsPackager : ILogsPackager
{
    private readonly ILogsReader _logsReader;

    public LogsPackager(ILogsReader logsReader)
    {
        _logsReader = logsReader;
    }

    public Stream GetLogArchive(DateTime date)
    {
        // write the archive to a memory stream
		var stream = new MemoryStream();
        using (var archive = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true))
        {
            var entryName = $"logs-{date:yyyyMMdd}.log";
            var entry = archive.CreateEntry(entryName);
            using (var writer = new StreamWriter(entry.Open()))
            {
                foreach (var logEntry in _logsReader.ReadLogEntries(date))
                    writer.Write(logEntry + "\n");
            }
        }
        // then rewind the stream to the beginning
        stream.Seek(0, SeekOrigin.Begin);
        return stream;
    }
}

and a refactoring of the controller to depend on ILogsPackager, instead of directly on the ILogsReader

-    private readonly ILogsReader _logsReader;
+    private readonly ILogsPackager _logsPackager;

-    public AdminController(ILogsReader logsReader)
+    public AdminController(ILogsPackager logsPackager)
     {
-        _logsReader = logsReader;
+        _logsPackager = logsPackager;
         _settings = settings;
     }

a refactoring of the GetLogs method in the controller

    public IActionResult GetLogs([FromQuery(Name = "q")] string dateFilter)
    {
        if (!DateTime.TryParseExact(dateFilter, "yyyyMMdd", System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.None, out var date)
            return BadRequest("q was not a yyyyMMdd date");

		var stream = _logsPackager.GetLogArchive(date);

        IActionResult logData = File(stream, "application/octet-stream", $"logs-{dateFilter}.zip");
        return Task.FromResult(logData);
    }

And, after adding a registration for ILogsPackager to DI, the tests should pass after this refactoring.

What have we done? Link to heading

At this point maybe it is helpful to look at this class diagram displaying the relationship between the components we’ve created so far. A couple of interfaces, and classes implementing them, or depending on them, and Dependency Injection composing these together.

Start testing on a lower level Link to heading

We can now add some tests for the ILogsPackager as well. We can just copy the previous test for the controller, and remove some of the browser setup and response handling, manually create a LogsPackager and just call GetLogArchive directly. This is essentially the same test, but it avoids the entire HTTP request/response plumbing and thus runs much much faster.

    [Test]
    public async Task packager_returns_the_expected_log_contents()
    {
        var logsReader = new FakeLogsReader();
        var packager = new LogsPackager(logsReader);
        using var archive = packager.GetLogArchive("logs", new Date(2022, 11, 01));
        using var streamReader = new StreamReader(archive.Entries.Single().Open());

        var lines = new List<string?>();
        while(!streamReader.EndOfStream)
        {
            lines.Add(await streamReader.ReadLineAsync());
        }

        Assert.That(lines, Is.EquivalentTo(new []{"SOME", "LOG", "ENTRIES"}));
    }

Is the corresponding test from the controller tests now redundant? I don’t think so, since the above test does not cover testing that the parts work well together with the controller and that the dependencies can be automatically composed via DI.

What’s left? Link to heading

To complete the feature, we would need to add unit tests for a production ILogsReader implementation, to verify it can read log entries from some configured location in the file system. I’d be fairly confident that this implementation will work fine with the LogsPackager, but maybe I’d write an integration test to verify all production components work fine together. For the actual customer case that this post is based on, I added a simple UI to enter a date, and a button to download the corresponding logs, with tests of course.

Conclusion Link to heading

For me, this was an interesting TDD experiment, working top-down instead of my normal bottom-up style. I will use it more in the future, especially when I don’t have a clear picture of which parts are needed to solve the problem, or how they best fit together. In such cases, I think the top-down style helps guide the implementation, and once a cohesive part has formed, it can be refactored into one or more components, which may then be broken down into further parts when new tests forces me to complete more of the implementation.

I think one of the pitfalls of using top-down TDD could be to lead someone to put all business logic in the “first place it fits”, which in this example would be the HTTP controller (generally a bad idea as explained above) and it would take some discipline and end up with less composable code - whereas in bottom-up style I think you’d more naturally end up with a good separation of controller and business logic, as you’d basically be forced to start unit testing the LogsReader or LogsPackager without involving the HTTP layer.

We might get tempted to add a lot of implementation tests early, when we are limited to our costly HTTP integration tests like the controller tests in this post

Note that here I’m using “integration test” I mean “component integration tests” that runs in-process, here simulating the HTTP request/response cycle without actually doing cross process/host network communication. I am not talking about the system integration tests that you’d write to start a whole system, composed with production components as far as possible, which typically includes real network communication between services, and between the test runner and the system you are testing. The line is somewhat blurred by features like ASP.NET WebApplicationFactory enabling in-process integration tests with test or production components.

Tests to check the exact details of the business logic, to get 100% code coverage so to speak, are possible to write as either (component) integration or unit tests, but are best left as unit tests, simply due to the fact that “pure” unit tests are at least 10 times faster to run. When you have thousands of tests, it will make a big difference in test run-time.

There’s of course an inverse to this, which is that when doing bottom-up TDD, you might think your component is so well unit-tested that you don’t put much effort into either component or system integration tests. The number of times I’ve just made a small change, and mistakenly thought that it “couldn’t possibly affect anything else”, finding the components not actually working together as well as I thought, tells me this is an actual downside of focusing too much on the individual parts. Either extreme is to be avoided. Me personally, I think I’ll keep mostly doing bottom-up, but will try to remember top-down when I’m unsure about the direction to go.