TDD for Bug Fixes and Legacy Code: Red Test First, Characterization and Sprouting New Code

Version note: The code in this post was built and run with .NET 10 and xUnit.net v3 4.0.3.

Predict First

Bookstore's CheckoutService, which calculates the checkout step, has been in production since 2019 without a single test. A bug report comes in, and before touching it you record what the class does today with three tests. All three are green. Then you write a red test for the bug, make the fix, and the bug test turns green. But one of the earlier three tests turns red.

Is that bad news? Should you roll back the fix, update the test, or delete the test? The answer depends on what kind of test it is. Halfway through the post we see this red with its real output.

TDD That Starts with a Bug, and the Right Test Level

TDD follows the same order when fixing a bug as when writing new code: red test first, then the fix. The "Reproducing the Bug" post in the Debugging and Problem Solving series explained why: a test you never saw red doesn't prove it catches the bug, and a test written after the fix is shaped by the fixed code. Here we continue from that post and talk about where this test sits in the TDD flow.

The bug report: "With the SAVE50 coupon a 320 cart goes down to 270, but the checkout page shows free shipping. The rule is that shipping is free when the amount after the discount is 300 or more." The report came from the checkout page, but that doesn't mean the test should be written on the checkout page. The question when choosing the test level is: at what level does the defect live, and what is the lowest level that shows it? Here the shipping decision is made in the backend, inside CheckoutService.Quote. An E2E (end-to-end) test starting from the browser would catch the same defect, but it takes seconds, doesn't show the cause when it breaks, and adds a permanent load to the suite just for this bug. A unit-level test shows the defect exactly where it is.

Choosing the level well has a side effect too: after the fix, the test becomes permanent regression protection. If the same defect comes back tomorrow in another refactoring, it turns red within milliseconds.

A Class with No Tests: CheckoutService

The definition in Michael Feathers's Working Effectively with Legacy Code (2004) frames this post: legacy code is code without tests, regardless of its age. Because it has no tests, changing it is scary; changing it needs tests, and writing tests often needs changing it. Our legacy class:

public class CheckoutService
{
    private readonly ICouponStore _coupons;

    public CheckoutService(ICouponStore coupons)
    {
        _coupons = coupons;
    }

    public CheckoutQuote Quote(List<OrderLine> lines, string? couponCode, string city)
    {
        decimal subtotal = 0;
        foreach (OrderLine line in lines)
        {
            subtotal += line.UnitPrice * line.Quantity;
        }

        decimal shipping = 29.90m;
        if (subtotal >= 300)
        {
            shipping = 0;
        }
        if (city == "Istanbul" || city == "Ankara" || city == "Izmir")
        {
            shipping = shipping * 0.8m;
        }

        decimal discount = 0;
        if (!string.IsNullOrEmpty(couponCode))
        {
            StoredCoupon? coupon = _coupons.Find(couponCode);
            if (coupon != null && coupon.ExpiresOn >= DateTime.Today)
            {
                if (coupon.IsPercentage)
                {
                    discount = Math.Round(subtotal * coupon.Value / 100, 2);
                }
                else
                {
                    discount = coupon.Value;
                }
            }
        }

        return new CheckoutQuote(subtotal, discount, shipping, subtotal - discount + shipping);
    }
}

This class is a lucky legacy example: the coupon store comes through the constructor, so we can pass a stub in tests. In real life the class often news up its own dependency or calls a static method, and you first have to open a seam. The details of dependency-breaking techniques are in the upcoming Refactoring series; in this post we deal with the case where a test could have been written but never was.

Characterization Tests: What Does the Code Do Today?

Before touching the bug, we record the class's current behavior. Feathers calls this a characterization test, and the technique is deliberately odd: you write something you know is wrong as the expected value, run the test and copy the actual value from the failure message into the expected value.

[Fact]
public void Small_cart_in_Istanbul()
{
    CheckoutService service = new(new CouponStoreStub());

    CheckoutQuote quote = service.Quote(Cart(45.50m, 2), couponCode: null, city: "Istanbul");

    Assert.Equal(new CheckoutQuote(0m, 0m, 0m, 0m), quote);
}

failed Bookstore.Checkout.Tests.CheckoutServiceCharacterizationTests.Small_cart_in_Istanbul (8ms)
  Expected: CheckoutQuote { Subtotal = 0, Discount = 0, Shipping = 0, Total = 0 }
  Actual:   CheckoutQuote { Subtotal = 91.00, Discount = 0, Shipping = 23.920, Total = 114.920 }

The output already gives us the first interesting finding: shipping with the big city discount applied is 23.920, with three decimal places. Because decimal multiplication preserves scale, 29.90 × 0.8 looks like that. As a value it equals 23.92, but if it is turned into a string somewhere and printed on the screen or an invoice, the difference shows. We note it and don't fix it: the purpose of a characterization test is not to judge behavior but to record it. We copy the actual value and add two more scenarios the same way:

Assert.Equal(new CheckoutQuote(91.00m, 0m, 23.920m, 114.920m), quote);   // Small_cart_in_Istanbul
Assert.Equal(new CheckoutQuote(320m, 0m, 0m, 320m), quote);              // Cart_over_300_in_Bursa
Assert.Equal(new CheckoutQuote(320m, 50m, 0m, 270m), quote);             // Fixed_coupon_on_a_320_cart_in_Bursa

Three tests green. The third is notable: it recorded exactly the situation in the bug report, zero shipping. A characterization test can't know that; it writes down what the code does, not what it should do.

This technique is exactly what Kent Beck lists as a mistake in Canon TDD: copying the actual value into the expected value. There's no contradiction, because the two tests answer different questions. A TDD test writes a specification and derives the expected value from the behavior; copying destroys the test's double-checking value. A characterization test takes a measurement; taking the expected value from the code is precisely its purpose.

Fixing the Bug: Red Test, Then Code

Now the TDD test for the bug. It lives in a separate class, because it is a specification: it says what the behavior should be.

[Fact]
public void Free_shipping_threshold_applies_to_the_discounted_subtotal()
{
    // Bug #482: SAVE50 brings a 320 cart down to 270 but shipping shows as free
    StoredCoupon save50 = new() { Code = "SAVE50", Value = 50m, ExpiresOn = DateTime.Today.AddDays(30) };
    CheckoutService service = new(new CouponStoreStub(save50));
    List<OrderLine> lines = [new OrderLine { Isbn = "978-0321125217", UnitPrice = 80m, Quantity = 4 }];

    CheckoutQuote quote = service.Quote(lines, couponCode: "SAVE50", city: "Bursa");

    Assert.Equal(29.90m, quote.Shipping);
    Assert.Equal(299.90m, quote.Total);
}

failed Bookstore.Checkout.Tests.CheckoutServiceTests.Free_shipping_threshold_applies_to_the_discounted_subtotal (17ms)
  Expected: 29.90
  Actual:   0

The red reproduces the bug report exactly. The fix is small: move the coupon block above the shipping block and compare the threshold with the amount after the discount (if (subtotal - discount >= 300)). Running the tests brings up the picture from "Predict First":

failed Bookstore.Checkout.Tests.CheckoutServiceCharacterizationTests.Fixed_coupon_on_a_320_cart_in_Bursa (7ms)
  Expected: CheckoutQuote { Subtotal = 320, Discount = 50, Shipping = 0, Total = 270 }
  Actual:   CheckoutQuote { Subtotal = 320, Discount = 50, Shipping = 29.90, Total = 299.90 }
  total: 4
  failed: 1
  succeeded: 3

The bug test is green, two characterization tests are green, and the characterization test that recorded the bug is red. That is exactly what we expected, and it's good news: the red says the fix changed behavior exactly where the bug was and nowhere else. That test's job is done; the bug test now carries the same scenario with the correct expectation. We delete the red characterization test.

What matters here is making this decision consciously. When a characterization test turns red there are two possibilities: either it recorded a behavior you changed on purpose (delete it or update it with the new behavior), or it caught a behavior you broke by accident (review the fix). The way to tell them apart is to check whether the red matches the scenario in the bug report.

Sprout: Growing the New Rule Test-First Somewhere Else

The same week a new request arrives: Bookstore Club members will pay half price for shipping. You could write this rule as one more if in the middle of Quote, but then the new rule would also be born inside a method that is hard to test. Feathers's sprout method and sprout class techniques suggest another path: grow the new behavior in a separate method or class written test-first, and connect it to the old code with a single line.

public sealed class ClubShippingTests
{
    public static TheoryData<bool, decimal, decimal> Cases => new()
    {
        { false, 29.90m, 29.90m },
        { true, 29.90m, 14.95m },
        { true, 23.92m, 11.96m },
        { true, 0m, 0m },
    };

    [Theory]
    [MemberData(nameof(Cases))]
    public void Club_members_pay_half_the_shipping_fee(bool isClubMember, decimal shipping, decimal expected)
    {
        decimal fee = ClubShipping.Apply(shipping, isClubMember);

        Assert.Equal(expected, fee);
    }
}

The test is independent of the legacy class. No setup, no stub; four data rows state all the edges of the rule: a non-member is unaffected, free shipping stays free, big-city discounted shipping is halved too. After the compiler's red (CS0103: The name 'ClubShipping' does not exist), the implementation is one line:

public static class ClubShipping
{
    public static decimal Apply(decimal shipping, bool isClubMember) =>
        isClubMember ? Math.Round(shipping / 2, 2, MidpointRounding.AwayFromZero) : shipping;
}

Now the connection. First we write the test that asks for the connection at the Quote level: a 91 cart in Istanbul, a club member, shipping should be 11.96. After adding an isClubMember parameter with a default of false to Quote, the red says the connection is missing:

failed Bookstore.Checkout.Tests.CheckoutServiceTests.Club_members_get_half_price_shipping_at_checkout (7ms)
  Expected: 11.96
  Actual:   23.920

The single line added to the legacy method, right after the big city block:

shipping = ClubShipping.Apply(shipping, isClubMember);

Test run summary: Passed!
  total: 8
  failed: 0
  succeeded: 8
  skipped: 0
  duration: 246ms

Because the parameter has a default value, existing callers didn't change and the characterization tests stayed green. We didn't get the whole legacy class under test and didn't rewrite any of it; but the two places we touched, the bug and the new rule, are now protected by tests. The tested part of the code grows a little with every change.

Characterization Tests and TDD Tests Side by Side

Keeping the two kinds in separate classes makes this difference visible. When a test in CheckoutServiceCharacterizationTests turns red you ask "did I change this on purpose?"; a red in CheckoutServiceTests is a regression, plain and simple.

In Production

The bug test's name and trail. Put the behavior in the test name and the bug number in a comment. A name like "Bug482Test" tells nobody anything six months later; "Free_shipping_threshold_applies_to_the_discounted_subtotal" states the rule. The bug number shows the trail to whoever asks why the test exists.

Time-dependent legacy code. CheckoutService checks coupon expiry with DateTime.Today. In the characterization tests we went around that dependency by setting the coupon's expiry to DateTime.Today.AddDays(30). When the expiry rule itself has to be tested, you need to open a seam with TimeProvider as in the second post; that is a refactoring step and should first be protected by characterization tests.

Maintaining characterization tests. These tests know a lot: the code's output with all its quirks. As the code gets cleaner and specification tests multiply, characterization tests should be reduced; otherwise every deliberate change in behavior requires updating dozens of tests. Write in the commit message which characterization test was replaced by which specification test.

Common Pitfalls

Writing the test after the fix in a bug fix. If the test has never been red, you have no proof it catches the bug. Roll back the fix, watch the test turn red, apply the fix again. This check takes a minute.

Believing you can't touch legacy code until all of it is under test. For a 2,000-line class the plan "let's get it all under test first" usually never starts. Characterization tests around the behavior you will touch, a sprout for new behavior; the tested area grows with every change.

Mistaking a characterization test for a specification. The value you recorded may be a bug; the third test in this post was exactly that. Blindly putting a red characterization test "back the way it was" can mean undoing a fix you made on purpose.

Leaving the sprout as hard to test as the legacy code. If the new class takes the legacy class's dependencies (the database, DateTime.Now), the point of the sprout is lost. Give the sprout only the values it needs; in this post ClubShipping takes only an amount and a bool.

A Small Experiment

★☆☆ What would have happened if you had made the fix before writing the bug test? Write the bug test against the fixed code and see it green; then roll back the fix. Did the test turn red? Why does the order of these two steps matter?

★★☆ Add a characterization test for Quote: a percentage coupon (BOOK10), Izmir, 3 books at 12.45. Do you see another quirk in the coupon discount or the shipping in the output? Would you write what you see into a specification test or onto a notes list?

★★★ Plan moving the whole shipping calculation into a sprout class: first record the shipping behavior broadly with characterization tests, then write a ShippingFee class test-first, and in the last step replace the shipping block in Quote with a single call. Which characterization tests became unnecessary after this?

In the last post of the series we step back and look at the big picture: the "Is TDD Dead?" debate, what research says, where TDD doesn't help, and TDD with AI coding agents.