2026-09-05 · 8 min read

Python code review exercise: passing tests that cannot catch a wrong rate

I have seen this bug in more than one codebase, and it shows up roughly the same way every time. A billing constant changes, the tests stay green, and invoices quietly go wrong. Nobody suspects the test suite because the suite is passing. The root cause is always embarrassing: the tests import the rate from the module under test and rebuild their expected values with the same formula. Both sides of every assertion move together, so the tests literally cannot fail no matter what the rate is set to. The change passes review ("looks good, tests pass"), merges without friction, and only surfaces weeks later when someone compares the numbers by hand and thinks to open the test file.

This exercise is built around that same pattern, scaled down to a single function: a VAT calculation with a parametrized pytest suite where every test passes, the code is clean, the Decimal arithmetic is careful, and the entire suite is worthless for catching the one mistake it was written to prevent. It is a Python code review exercise for developers who want to practice spotting tautological test oracles, the kind of test that checks the code against itself instead of against the spec. The 20% rate in this exercise is a teaching scenario, not tax advice.

The requirements

The PR description says:

• Add the invoice_total(net) helper: gross total = net plus the standard 20% VAT, rounded to whole cents. Inputs are non-negative Decimal amounts with two decimal places.
• Add unit tests that pin the 20% rate independently of the implementation: if someone edits the rate in billing.py to a wrong value, the suite must fail.
• Cover a zero net amount.
• Standard pytest setup; use Decimal throughout (no floats).

Read the second requirement twice. It does not just say "test the function." It says the tests must independently pin the rate so that a wrong edit to VAT_RATE breaks the build. That is the contract this PR needs to satisfy.

The PR

The implementation is fine. Clean Decimal arithmetic, explicit rounding mode, a named constant for the rate. Nothing to object to here. Now the tests:

Five tests, all green. Parametrized over four amounts plus a zero edge case. Careful Decimal quantization in the expected-value computation. No float noise. Looks thorough.

Walkthrough starts here. If you haven't looked at the diff yourself yet, scroll back up.

Five tests, zero protection

Look at line 5 of the test file:

from app.billing import CENT, VAT_RATE, invoice_total

The test imports VAT_RATE and CENT from the module it is testing. Then on line 13 it rebuilds the expected value using the exact same formula the implementation uses:

expected = (net * (Decimal("1") + VAT_RATE)).quantize(CENT, rounding=ROUND_HALF_UP)

Compare that to lines 9-10 of billing.py:

gross = net * (Decimal("1") + VAT_RATE)
return gross.quantize(CENT, rounding=ROUND_HALF_UP)

They are the same computation, character for character, drawing from the same constants. The assertion on line 14 is not checking the function against the spec. It is checking the function against a second copy of itself. Both sides of the == are downstream of VAT_RATE, so both sides move together whenever VAT_RATE changes. The test is a mirror: the code looks at its own reflection and confirms it sees a face.

The mutation that proves it

Open billing.py and change VAT_RATE from Decimal("0.20") to Decimal("0.02"). That is a 2% VAT rate, obviously wrong, the kind of typo that a copy-paste or a careless config edit could introduce. Now run the suite:

$ pytest tests/test_billing.py -v
test_invoice_total_applies_standard_vat[10.00]    PASSED
test_invoice_total_applies_standard_vat[19.99]    PASSED
test_invoice_total_applies_standard_vat[0.01]     PASSED
test_invoice_total_applies_standard_vat[123.45]   PASSED
test_invoice_total_zero_net                        PASSED

All green. Five out of five. The function now computes 2% VAT on every invoice, and the test suite has absolutely nothing to say about it, because the test recomputed its expected values from the same wrong constant. invoice_total(Decimal("10.00")) returns Decimal("10.20") instead of Decimal("12.00"), and the test expects Decimal("10.20") too, because it did the same multiplication with the same wrong rate.

The second requirement explicitly said: "if someone edits the rate in billing.py to a wrong value, the suite must fail." This suite does not satisfy that requirement. It satisfies a weaker, useless requirement: "the function's output must match the function's own formula." That is always true by construction.

Why the zero case does not save you

You might notice that test_invoice_total_zero_net uses a hardcoded expected value: Decimal("0.00"). That is technically an independent oracle. But zero times any rate is zero, so this test passes for every rate including negative ones. It pins nothing. It is the one input where the tautology is invisible because all rates agree on the answer.

The comment you would leave

Verdict: request changes

The implementation is correct. The tests are not. They do not satisfy the stated requirement of pinning the rate independently, and the proof is a one-line mutation that leaves the entire suite green. This is not a style issue or a nitpick; it is a test suite that provides zero protection against the exact class of bug it was written to catch.

The fix

Remove the VAT_RATE and CENT imports. Parametrize over (net, gross) tuples with the expected gross totals hardcoded as literals you computed by hand (or by calculator, once, outside the test):

Now each expected value is a number that a human derived from the requirement "20% VAT, rounded to cents." The test file no longer imports VAT_RATE or CENT. Only one side of the assertion depends on the code.

The check that distinguishes the two versions

Apply the same mutation: change VAT_RATE in billing.py to Decimal("0.02"). Run the fixed tests:

$ pytest tests/test_billing.py -v
test_invoice_total_applies_standard_vat[10.00-12.00]    FAILED
test_invoice_total_applies_standard_vat[19.99-23.99]    FAILED
test_invoice_total_applies_standard_vat[0.01-0.01]      PASSED
test_invoice_total_applies_standard_vat[123.45-148.14]  FAILED
test_invoice_total_zero_net                              PASSED

Three failures. 10.00 now returns 10.20 instead of 12.00. 19.99 returns 20.39 instead of 23.99. 123.45 returns 125.92 instead of 148.14. The build breaks, the wrong rate does not ship, and someone asks why VAT_RATE is 0.02. The two cases that still pass (0.01 and 0.00) are too small for the rounding difference to surface, which is fine: three out of four non-trivial cases catch the mutation, and that is all you need.

What makes this hard to spot

This PR does not look like a mistake. It looks like someone who knows what they are doing. The Decimal arithmetic is correct. The rounding mode is explicit. The parametrization covers four different amounts plus the edge case. The test names are descriptive. If you skim the diff and pattern-match against "what good pytest code looks like," everything checks out. The problem is entirely in the relationship between the test and the spec, not in the shape of the code, and that relationship is invisible unless you trace where expected comes from and ask yourself what mutation would break it.

This is also why automated code review tools tend to miss it. A linter sees correct Python. A coverage tool sees every branch hit. An AI reviewer sees clean structure, descriptive naming, and careful Decimal handling. None of them ask "if the rate were wrong, would this test tell me?" because that is a question about the purpose of the test, not its syntax.

The habit that catches this every time

When you are reviewing a test, trace the expected value back to its source. If it is a literal, ask whether that literal came from the spec or from running the function once and copying the output (that is a different smell, but at least it pins a specific value). If it is computed, ask whether the computation imports anything from the module under test. If it does, ask the question that kills every tautological oracle: "what change to the thing this test claims to protect would actually make this test fail?" If the answer is "nothing" or "only a change to the formula, not the constant," the test is a mirror, and the mirror needs to be replaced with a window that looks at the spec.