Here is a long function that reads files, calls an API and writes a report. Refactor it so you can test it.

“My plan: pin today’s behavior with one test, split deciding from doing, then pass in the clock and the price client. Five small commits, and the test passes after each one.

Here’s the function, trimmed to its shape. It reads every CSV in a folder, prices the SKUs with one API call, and writes one total per SKU under a dated header.”

API_URL = "https://pricing.example.com/v1/prices"

def generate_report(input_dir, out_path):
    orders = []
    for name in os.listdir(input_dir):
        if name.endswith(".csv"):
            path = os.path.join(input_dir, name)
            with open(path, newline="") as f:
                for row in csv.DictReader(f):
                    orders.append((row["sku"], int(row["qty"])))
    skus = sorted({sku for sku, _ in orders})
    # no timeout, and the status is never checked
    resp = requests.get(API_URL, params={"skus": ",".join(skus)})
    prices = {s: Decimal(p) for s, p in resp.json().items()}
    totals = {}
    for sku, qty in orders:
        # KeyError if the API did not price this SKU
        amount = prices[sku] * qty
        totals[sku] = totals.get(sku, Decimal(0)) + amount
    lines = ["Sales report " + date.today().isoformat()]
    lines += [f"{s},{totals[s]:.2f}" for s in sorted(totals)]
    open(out_path, "w").write("\n".join(lines) + "\n")

“Before I change anything I want a test that tells me I haven’t changed behavior. The function calls requests.get and date.today() inside its body, so I replace the HTTP call with a fake and skip the date line. A later step makes the date a parameter, and from then on the unit tests control it.

The fake stands in for requests.get: it returns the saved prices whatever it is asked, like a server that always answers.”

def fake_get(prices_file: Path):
    data = json.loads(prices_file.read_text())
    def get(url, params=None, **kwargs):
        return SimpleNamespace(json=lambda: data)
    return get

def test_report_matches_current_output(tmp_path, monkeypatch):
    shutil.copytree(FIXTURES / "orders", tmp_path / "in")
    fake = fake_get(FIXTURES / "prices.json")
    monkeypatch.setattr(legacy.requests, "get", fake)
    legacy.generate_report(tmp_path / "in", tmp_path / "out.txt")
    got = (tmp_path / "out.txt").read_text().splitlines()
    want = (FIXTURES / "expected.txt").read_text().splitlines()
    # line 0 holds today's date, the one line that varies
    assert got[1:] == want[1:]

“expected.txt is what the current code produces on the fixture: I ran it once and saved it, and I read it before trusting it. The fixture has a SKU repeated across two files, a zero quantity and a header-only file, so the test covers every path that still produces a report. The paths that crash get their own tests later.

Now I split deciding from doing. Each piece that touches the outside world gets its own small function, and the arithmetic moves into one pure function that takes plain values.”

@dataclass(frozen=True)
class Order:
    sku: str
    qty: int

class PriceClient(Protocol):
    def prices(self, skus: list[str]) -> dict[str, Decimal]: ...

# doing: moved out of generate_report, unchanged
class HttpPriceClient:
    def __init__(self, url: str):
        self.url = url

    def prices(self, skus: list[str]) -> dict[str, Decimal]:
        params = {"skus": ",".join(skus)}
        resp = requests.get(self.url, params=params)
        return {s: Decimal(p) for s, p in resp.json().items()}

# doing: reads the files
def read_orders(folder: Path) -> list[Order]:
    orders = []
    for path in sorted(folder.glob("*.csv")):
        with path.open(newline="") as f:
            for r in csv.DictReader(f):
                orders.append(Order(r["sku"], int(r["qty"])))
    return orders

# deciding: pure, no files, no network, no clock
def build_report(
    orders: list[Order],
    prices: dict[str, Decimal],
    today: date,
) -> str:
    totals: dict[str, Decimal] = {}
    for o in orders:
        amount = prices[o.sku] * o.qty
        totals[o.sku] = totals.get(o.sku, Decimal(0)) + amount
    lines = [f"Sales report {today.isoformat()}"]
    for sku, total in sorted(totals.items()):
        lines.append(f"{sku},{total:.2f}")
    return "\n".join(lines) + "\n"

# doing: wires the pieces together
def run(folder: Path, out: Path,
        client: PriceClient, today: date) -> None:
    orders = read_orders(folder)
    prices = client.prices(sorted({o.sku for o in orders}))
    out.write_text(build_report(orders, prices, today))

# old signature kept, so no caller changes
def generate_report(input_dir, out_path):
    client = HttpPriceClient(API_URL)
    run(Path(input_dir), Path(out_path), client, date.today())

“The characterization test still passes after each of those moves. The original read files in os.listdir order; sorting them changes nothing here, because the report is keyed and sorted by SKU. If output order had depended on file order, I’d keep the old order and note it.

Now the core gets real unit tests, with no network. They cover a normal report, no orders, a malformed row and a missing price. The last two pin crashes that are really bugs.”

def test_totals_by_sku():
    report = build_report(
        [Order("b", 2), Order("a", 1), Order("b", 1)],
        {"a": Decimal("2.50"), "b": Decimal("1.10")},
        date(2026, 3, 2),
    )
    assert report == "Sales report 2026-03-02\na,2.50\nb,3.30\n"

def test_no_orders_gives_header_only():
    report = build_report([], {}, date(2026, 3, 2))
    assert report == "Sales report 2026-03-02\n"

def test_malformed_qty_raises_today(tmp_path):
    (tmp_path / "a.csv").write_text("sku,qty\nx,two\n")
    with pytest.raises(ValueError):
        read_orders(tmp_path)

def test_missing_price_raises_today():
    with pytest.raises(KeyError):
        build_report([Order("x", 1)], {}, date(2026, 3, 2))

@pytest.mark.xfail(strict=True, reason="unpriced SKU is fatal")
def test_unpriced_sku_listed_not_fatal():
    report = build_report([Order("x", 1)], {}, date(2026, 3, 2))
    assert "x,unpriced" in report

“Two of these pin bugs. A SKU the pricing API doesn’t know crashes the whole report, and so does one malformed row in any file. The raising tests pin today’s crashes; the xfail records what the report should do, and flips to a failure the day someone fixes it without updating the tests. I’d raise both with whoever owns the report and fix them in separate commits, probably by listing unpriced SKUs and skipped rows at the bottom.

The same goes for the bugs a reviewer of this code will find next. The code never checks the HTTP status, so a server error fails wherever its body happens to break the parsing: a JSON error, a Decimal error, or a KeyError on the first SKU, never a clear ‘the pricing API is down’. And a CSV saved from Excel can start with a byte-order mark, so the first header reads as \ufeffsku instead of sku, and row["sku"] raises KeyError. That one gets a failing test now, and its fix, opening the file with encoding="utf-8-sig", gets its own commit.”

@pytest.mark.xfail(strict=True, reason="BOM breaks the header")
def test_excel_bom_header(tmp_path):
    (tmp_path / "a.csv").write_bytes(b"\xef\xbb\xbfsku,qty\nx,1\n")
    assert read_orders(tmp_path) == [Order("x", 1)]

“The timeout goes the same way: retries, the timeout and a status check belong in HttpPriceClient, tested by handing it a fake session that times out twice and then answers. That change waits until the characterization test fakes the client instead of requests.get, or the test would start calling the real API.

The original already parsed prices as Decimal, so I kept it. Had it used floats, switching would change rounding, so that would go on the bug list too, not into this refactor.

In real code I’d commit after each step: characterization test, extract read_orders, extract build_report, inject client and date, add unit tests. Each commit is small enough to review and revert on its own.”

If they ask where the date goes: “It’s a parameter of run and build_report, and only the old wrapper calls date.today(), so a test passes any date it likes. Before that step, I couldn’t pin it by setting datetime.date.today: that raises a TypeError, because date is a built-in type. Patching the module’s own name works, monkeypatch.setattr(legacy, "date", FixedDate) with a subclass whose today() returns a fixed date, but a parameter is simpler and needs no patching at all.”