Breaking Dependencies in Legacy Code: Sprout, Wrap, Extract Interface and Static Dependencies

Version note: The code in this post was built and run with C# 14 and .NET 10 (SDK 10.0.402), with tests on xUnit.net v3 4.0.3 and Microsoft.Extensions.TimeProvider.Testing 10.11.0; database access is for SQL Server through Microsoft.Data.SqlClient 7.1.2.

Guess First: Can You Write a Test for This Job?

Partner bookshops send order files to Bookstore. Every five minutes OrderImportJob looks at an inbox folder, reads each CSV file (the separator here is a semicolon), writes the rows to the dbo.ImportedOrders table in SQL Server, moves the file to the done folder and sends a summary email to the operations team:

public class OrderImportJob : BackgroundService
{
    private readonly string _inbox;
    private readonly string _connectionString;

    public OrderImportJob(IConfiguration configuration)
    {
        _inbox = configuration["Import:Inbox"]!;
        _connectionString = configuration.GetConnectionString("Bookstore")!;
    }

    protected override async Task ExecuteAsync(CancellationToken stoppingToken)
    {
        while (!stoppingToken.IsCancellationRequested)
        {
            foreach (string file in Directory.GetFiles(_inbox, "*.csv"))
                ImportFile(file);

            await Task.Delay(TimeSpan.FromMinutes(5), stoppingToken);
        }
    }

    public void ImportFile(string path)
    {
        int imported = 0;
        int failed = 0;
        using SqlConnection connection = new(_connectionString);
        connection.Open();

        // Header row: OrderNo;CustomerEmail;Isbn;Quantity;UnitPrice;OrderedAt
        foreach (string line in File.ReadLines(path).Skip(1))
        {
            string[] parts = line.Split(';');
            try
            {
                using SqlCommand command = new(
                    "INSERT INTO dbo.ImportedOrders (OrderNo, CustomerEmail, Isbn, Quantity, UnitPrice, OrderedAt) " +
                    "VALUES (@orderNo, @email, @isbn, @quantity, @unitPrice, @orderedAt)", connection);
                command.Parameters.AddWithValue("@orderNo", parts[0]);
                // ... the other parameters
                command.ExecuteNonQuery();
                imported++;
            }
            catch (Exception)
            {
                failed++;
            }
        }

        string archive = Path.Combine(_inbox, "done", DateTime.Now.ToString("yyyyMMdd-HHmmss") + "-" + Path.GetFileName(path));
        File.Move(path, archive);
        SmtpHelper.Send("ops@bookstore.example", "Order import finished",
            $"{Path.GetFileName(path)}: {imported} imported, {failed} failed");
    }
}

The new request: rows whose order date is more than 30 days old should not be imported, and they should be listed separately in the summary email. The rule is simple, but we want to add it with tests. What will the first test you write for ImportFile get stuck on? How many outside-world dependencies can you count?

The Answer: The First Test Goes to SQL Server

I tried it. A test that writes a one-line CSV to a temporary folder and calls ImportFile got stuck on the connection before reaching the first row:

failed Bookstore.Import.Tests.FirstAttemptTests.ImportFile_imports_a_valid_row (4s 566ms)
  Xunit.MicrosoftTestingPlatform.XunitException: Microsoft.Data.SqlClient.SqlException : A network-related or instance-specific error occurred while establishing a connection to SQL Server. ...

A test that waits four and a half seconds and then fails. Had the connection succeeded, things wouldn't have been better: the test would write a row to a real table, move the file and try to reach a real SMTP server through SmtpHelper. The dependency list is long: new SqlConnection, the static SmtpHelper.Send, DateTime.Now, the file system and IConfiguration. None of them comes through the constructor; all of them are born inside the method.

Michael Feathers' definition of legacy code in Working Effectively with Legacy Code fits this situation exactly: code without tests. The book's real contribution is naming the small, safe techniques used while making untested code testable. In this post we apply some of those techniques, following the needs of our own job rather than the order of the book.

Change Point and Test Point

Instead of trying to make everything testable at once, we start with two questions. Where will we make the change? Inside the row loop, before the row is written to the database. Where can we observe the result of the change? In the rows written and in the body of the email. The first question gives the change point, the second the test point. Breaking dependencies is done only to remove the obstacles between these two points; the rest of the job can stay as it is for now.

The tool for removing obstacles is the seam: a place where you can change behavior without editing the code at that point. A constructor parameter, an interface, a virtual method are examples. The concept itself was covered in the Unit Test series post "Testable Design: Dependency Injection, Seams, Functional Core and Humble Object"; here we look at how we open seams step by step in a legacy class.

Sprout: Growing New Behavior Somewhere Else

If we write the new rule inside the job, we first have to make the whole job testable in order to test it. The faster route is to grow the rule in a pure place independent of the job and connect it to the job with one line. Feathers calls this sprout; we saw the test-first side of it in the TDD series post "TDD for Bug Fixes and Legacy Code: Red Test First, Characterization and Sprouting New Code". First the rule:

public static class ImportRules
{
    public static readonly TimeSpan MaxAge = TimeSpan.FromDays(30);

    public static bool IsStale(DateTime orderedAt, DateTime now) => now - orderedAt > MaxAge;
}

[Fact]
public void An_order_from_exactly_30_days_ago_is_not_stale() =>
    Assert.False(ImportRules.IsStale(Now.AddDays(-30), Now));

[Fact]
public void An_order_older_than_30_days_is_stale() =>
    Assert.True(ImportRules.IsStale(Now.AddDays(-30).AddMinutes(-1), Now));

Then the summary. A small class that collects the counters and the stale orders and produces the email body; with no stale rows, the body is identical to the old one, character for character:

public sealed class ImportSummary(string fileName)
{
    private readonly List<string> _stale = [];

    public int Imported { get; private set; }
    public int Failed { get; private set; }
    public IReadOnlyList<string> Stale => _stale;

    public void RecordImported() => Imported++;
    public void RecordFailed() => Failed++;
    public void RecordStale(string orderNo) => _stale.Add(orderNo);

    public string MailBody() => _stale.Count == 0
        ? $"{fileName}: {Imported} imported, {Failed} failed"
        : $"{fileName}: {Imported} imported, {Failed} failed, {_stale.Count} skipped as stale ({string.Join(", ", _stale)})";
}

The two classes have four tests and the job hasn't been touched yet. The wiring commit changes a few lines in the job:

+        ImportSummary summary = new(Path.GetFileName(path));
+            if (DateTime.TryParse(parts[5], CultureInfo.InvariantCulture, out DateTime orderedAt)
+                && ImportRules.IsStale(orderedAt, DateTime.Now))
+            {
+                summary.RecordStale(parts[0]);
+                continue;
+            }
-                imported++;
+                summary.RecordImported();
-                failed++;
+                summary.RecordFailed();
+        SmtpHelper.Send("ops@bookstore.example", "Order import finished", summary.MailBody());

No test exercises these lines, and DateTime.Now is still there. This is a deliberate trade-off: the rule is tested, and its connection to the job is small and readable by eye. But "readable by eye" is not a guarantee; shortly we'll see that one of these lines carries a bug.

Parameterizing the Constructor and TimeProvider

To test the job as a whole, we need to be able to supply time, email and the database from outside. The first step is time. TimeProvider, which came with .NET 8, is the standard way to abstract the clock. We don't change the existing constructor; we add a new one and make the old one forward to it:

public OrderImportJob(IConfiguration configuration)
    : this(configuration, TimeProvider.System)
{
}

internal OrderImportJob(IConfiguration configuration, TimeProvider time)
{
    _inbox = configuration["Import:Inbox"]!;
    _connectionString = configuration.GetConnectionString("Bookstore")!;
    _time = time;
}

The DateTime.Now calls become _time.GetLocalNow(); the wait in the loop is tied to the same clock through the Task.Delay(TimeSpan.FromMinutes(5), _time, stoppingToken) overload, so in a test it can be advanced without really waiting five minutes. The new constructor is internal; it is opened to the test project with <InternalsVisibleTo Include="Bookstore.Import.Tests" />. The production wiring didn't change: Microsoft's DI container only looks at public constructors. I verified with a small program that the job still resolves in a host set up with AddHostedService<OrderImportJob>(); the output of GetServices<IHostedService>().OfType<OrderImportJob>().Count() is 1.

This two-constructor structure is a temporary ugliness. Once all dependencies are registered in DI, a single public constructor will remain; but that isn't work we have to do today.

Moving a Static Call Behind a Thin Wrapper

SmtpHelper.Send is static; we can't get in between. Other places in the code base call it too, so instead of changing SmtpHelper we put a thin instance layer in front of it:

public interface IMailSender
{
    void Send(string to, string subject, string body);
}

// Other code using SmtpHelper stays as it is; this class is only a thin bridge.
public sealed class SmtpMailSender : IMailSender
{
    public void Send(string to, string subject, string body) => SmtpHelper.Send(to, subject, body);
}

The job now takes an IMailSender; the public constructor gives it new SmtpMailSender(). The wrapper has no logic inside, so it doesn't need a test. Removing the static helper entirely is done later as separate work, by moving every caller to the same wrapper.

Extract Interface: When the Connection Opens Is Behavior Too

The last obstacle is SqlConnection. The first interface that comes to mind is a one-method IOrderWriter.Insert(order). But in the old code the connection is opened once per file, before any row is processed. If the connection can't be opened, the exception leaves the job, the file isn't moved, no email goes out; on the next round the file is retried. If Insert opened its own connection on every call, with the database down every row's exception would land in the loop's catch, the file would be archived as "0 imported, 3 failed" and the orders would be lost. When the connection opens is part of the behavior; the interface has to preserve that order:

public interface IOrderStore
{
    // The connection opens here: if it can't, the exception is thrown before any row in the file is processed.
    IOrderBatch OpenBatch();
}

public interface IOrderBatch : IDisposable
{
    void Insert(ImportedOrder order);
}

SqlOrderStore carries the SQL code from the job over unchanged. The job's ImportFile now starts with using IOrderBatch batch = _store.OpenBatch();. Two small interfaces, two real needs; anything more is unnecessary for now.

Here Come the Tests: Fakes and FakeTimeProvider

All the seams are open. The test sets up the job with an in-memory store, a mail sender that records sent emails and FakeTimeProvider from the Microsoft.Extensions.TimeProvider.Testing package:

public OrderImportJobTests()
{
    _time.SetLocalTimeZone(TimeZoneInfo.Utc);
    _time.SetUtcNow(new DateTimeOffset(2026, 11, 14, 8, 0, 0, TimeSpan.Zero));
}

[Fact]
public void ImportFile_imports_valid_rows_and_reports_the_rest()
{
    string csv = WriteCsv(
        "A-1001;ada@example.com;9780441013593;1;450.00;2026-11-10T09:30:00",
        "A-0950;alan@example.com;9780141439518;2;180.00;2026-09-01T12:00:00",
        "A-1002;grace@example.com;9780062316097;x;300.00;2026-11-12T15:45:00");

    Job().ImportFile(csv);

    Assert.Equal(["A-1001"], _store.Orders.Select(o => o.OrderNo));
    Assert.Equal("orders.csv: 1 imported, 1 failed, 1 skipped as stale (A-0950)", _mail.Sent.Single().Body);
    Assert.True(File.Exists(Path.Combine(_inbox, "done", "20261114-080000-orders.csv")));
}

The first run is red, but not for the reason I expected:

System.IO.DirectoryNotFoundException : Could not find a part of the path '.../inbox.../done/20261114-080000-orders.csv'.

The job doesn't create the done folder itself; since the folder already exists in production, this never showed up. This is a classic characterization moment: before touching the code, you learn one of its assumptions you didn't know about. For now we don't change the behavior; we create the folder in the test setup and record the assumption in a comment. The test is green.

Then I added a test for a row with missing columns. The old code blew up on such a row the moment it accessed parts[3], inside the try block, and counted it as "failed":

failed Bookstore.Import.Tests.OrderImportJobTests.A_row_with_missing_columns_counts_as_failed (0ms)
  Xunit.MicrosoftTestingPlatform.XunitException: System.IndexOutOfRangeException : Index was outside the bounds of the array.
    at Bookstore.Import.OrderImportJob.ImportFile(String path) in src/Bookstore.Import/OrderImportJob.cs:51

Line 51 is the line where I wired in the sprout: it accesses parts[5] outside the try. A single row with missing columns now stops the processing of the whole file; in the old behavior only that row was counted as "failed". The one line without a test turned out to be the line carrying the bug. The fix, in its own commit, adds a parts.Length > 5 check to the condition.

Wrap: Adding Around the Old Flow Without Touching It

The second half of the request: if there are stale orders, a separate email goes to the sales team. This is behavior added at the end of the existing flow, not inside it. With the wrap technique the body of the old method moves into a new private method, and the old method's name calls it together with the new behavior:

public void ImportFile(string path)
{
    ImportSummary summary = ImportAndArchive(path);

    // New behavior: added around the old flow without touching it.
    if (summary.Stale.Count > 0)
        _mail.Send("sales@bookstore.example", "Stale orders skipped", string.Join(Environment.NewLine, summary.Stale));
}

private ImportSummary ImportAndArchive(string path)
{
    // ... the old body, ending with: return summary;
}

Sprout grows new behavior in a new place and adds a call to the old code; wrap sets the old behavior aside as it is and places the new behavior before or after it. What they share is not mixing new code and old code on the same lines. The whole journey is ten commits:

$ git log --oneline
e78aa99 feat: report stale orders to sales (wrap ImportFile)
7001c6a fix: stale check no longer throws on rows with missing columns
33aeaf0 test: OrderImportJob with in-memory store, recorded mail and FakeTimeProvider
9b6cd47 refactor: extract IOrderStore from OrderImportJob
6d5493a refactor: send mail through IMailSender instead of static SmtpHelper
269c727 refactor: parameterize OrderImportJob with TimeProvider
7063b57 feat: skip orders older than 30 days in OrderImportJob
c5df862 feat: sprout ImportSummary (not wired yet)
20ef4d0 feat: sprout ImportRules.IsStale (not wired yet)
76bc3f4 legacy: OrderImportJob

There are seven tests and they run in under a second. None of them touches the database, an SMTP server or the real clock. SqlOrderStore itself is still untested; the right tool for it is an integration test against a real SQL Server, the subject of the upcoming Integration and E2E Testing series.

Common Pitfalls

Making everything public to be able to test it. Had we made the new constructor public, the entry point opened for tests would become part of the production API: another developer could call it directly, and since the DI container picks the longest public constructor whose parameters it can resolve, a service registration added later could silently change which constructor is used. internal plus InternalsVisibleTo opens the test seam without leaking it into the production API. The same goes for private methods: the urge to make a private method public in order to test it is usually a sign that it should be sprouted into its own class.

Building a big interface hierarchy as the first step. Adding IOrderRepository, IUnitOfWork, IImportService, IFileSystem and IClock on the same day changes the architecture before the tests and produces a big diff. Here we opened three seams for the three obstacles between the test point and the change point; we didn't touch the file system, because testing with temporary folders is cheap enough.

Losing the timing when extracting an interface. The order of side effects, like opening the connection once per file, determines the shape of the interface. An Insert that opened its own connection per row would turn a database outage into an "orders are invalid" message.

Not testing the wiring lines because they are "simple anyway". The value of sprout is testing the new rule in isolation, but the line connecting the rule to the old code is code too. The only bug in this post appeared exactly there, and it was caught only once the job became testable as a whole.

Taking the ugly intermediate step for a permanent one. Two constructors, a bridge wrapping SmtpHelper, InternalsVisibleTo: these are temporary prices paid to make legacy code testable. If they aren't written down in an issue, they become permanent.

A Small Experiment

★☆☆ Find a class in your own code base that uses DateTime.Now or DateTime.UtcNow. Add an internal constructor that takes a TimeProvider, forward the old one to it with TimeProvider.System, and write a date-dependent test with FakeTimeProvider.

★★☆ Pick a class that calls a static helper (email, logging, cache). First write down the change point and the test point, then open seams only for the obstacles between the two. How many seams did you need, and how many did you leave closed?

★★★ Instead of this post's IOrderStore, design a one-method IOrderWriter that opens a connection on every call, and write a test showing how the behavior changes when the database is down. Then run that test against the IOrderStore design.

In this post we broke dependencies from inside a single class. In the next post we move to a bigger change: replacing LegacyShippingClient, which works through synchronous, static calls, with a modern client based on a typed HttpClient. The change won't fit in one commit; we'll plan it with the Mikado Method and carry it out on trunk with branch by abstraction.