Write the test that catches the bug

Level 5: Level 5: Judging Code You Did Not Writemedium24 mintest designassertionscode reviewregression tests

A suspicion is not a bug. The assertion that fails before the fix and passes after it is the only durable artifact of a review.

The only review comment that cannot be argued with

"I think this breaks on empty input" starts a conversation. "summarize([]) raises IndexError, here is the line" ends one. The difference is not tone, it is that the second is a fact anyone can reproduce in five seconds.

A test is that fact, written down so it keeps being checked. Everything else in a review evaporates the moment the branch merges.

A useful test has exactly two properties:

It fails against the current code.
It passes against code that satisfies the contract.

A test that passes both before and after the fix proves nothing about the fix. That sounds obvious and it is the single most common flaw in tests written under time pressure, because the natural instinct is to test the behavior you understand, which is the behavior that already works.

Check yourself

You suspect that def busiest_hour(counts): return counts.index(max(counts)) mishandles ties. Which assertion is worth adding?

assert busiest_hour([1, 5, 2]) == 1assert busiest_hour([5, 5, 2]) == 0assert busiest_hour([]) is Noneassert isinstance(busiest_hour([1, 5, 2]), int)

Arrange, act, assert

Every test has the same three parts, and keeping them visually separate is what makes a failing test readable at 2am.

def test_blank_ratings_do_not_crash():
    scores = [4, None, 5]                 # arrange: the input, built here, not shared
    summary = summarize_ratings(scores)   # act: exactly one call
    assert summary["count"] == 2          # assert: one claim per line
    assert summary["average"] == 4.5

Three rules earn their keep.

Build the input inside the test. A fixture shared between tests is a dependency between tests: if the function under review mutates its argument, test three fails because of what test one did, and you spend an hour on the wrong function.

One call in the act. If two calls are needed to reproduce, the second one belongs to a different test, or the pair is itself the thing being tested and should say so.

Name the test after the claim. test_blank_ratings_do_not_crash tells you what broke from the failure line alone. test_summarize_2 tells you to go read it.

Probing a function you cannot see inside

In a review you often have several candidate implementations and no time to read all of them carefully. A probe treats each one as a black box: call it on inputs derived from the contract, compare against the answer the contract demands, and report a verdict.

def check(fn):
    cases = [
        ("abcdefghi12", True),    # happy path: eleven characters, digit, letter
        ("abcdefgh12", True),     # exactly ten: the boundary the rule names
        ("abcdefg 12", False),    # ten characters but one of them is a space
        ("abcdefghij", False),    # no digit
        ("1234567890", False),    # no letter
    ]
    return all(fn(text) is expected for text, expected in cases)

Read the comments rather than the code. Each case exists because a specific misreading of the contract would pass every other case. That is the design method: for each rule in the contract, write the input that only that rule rejects.

Check yourself

In the probe above, what does the case ('abcdefgh12', True) exist to catch?

An implementation that forgets to check for digitsAn implementation that wrote len(text) > 10 instead of len(text) >= 10An implementation that allows spacesAn implementation that returns a truthy value instead of True

Some contracts are about what did not happen

Not every rule is visible in the return value. "Do not modify the caller's data" is a promise about the world after the call, and the only way to check it is to look.

def check(fn):
    defaults = {"theme": "light", "rows": 20}
    overrides = {"rows": 50}

    result = fn(defaults, overrides)

    if result != {"theme": "light", "rows": 50}:
        return False
    if defaults != {"theme": "light", "rows": 20}:   # the caller's dict must survive
        return False
    return True

An implementation that writes defaults.update(overrides) and returns defaults produces a perfectly correct return value. It also silently rewrites the caller's configuration, and every later read of defaults in that process sees the merged version. Only the second assertion finds it.

Check yourself

A probe builds its input dict once at module level and reuses it across four candidates. Candidate two mutates its argument. What happens?

Candidates three and four are judged against corrupted inputNothing, because Python passes dictionaries by valueCandidate two is correctly reported as broken and the rest are unaffectedPython raises a RuntimeError for mutating a shared structure

The test outlives the review

Once a bug is found, the assertion that found it goes into the suite permanently. That is what a regression test is: not a new idea, just the failing test kept after the fix landed, so the same mistake cannot return in six months when someone rewrites the function for unrelated reasons.

Interview nuance: when you find a bug in a pairing exercise, the strongest thing you can do is write the failing assertion before you write the fix. It proves the diagnosis is real, it gives you an unambiguous definition of done, and it is exactly the loop a professional works in. Saying "let me pin this with a test first" reads as senior in a way that no amount of fluent typing does.

Check yourself

You have found and fixed a bug in someone else's function. What do you do with the input that exposed it?

Mention it in the pull request description so reviewers know it was consideredKeep it as an assertion in the test suite, named after the claim it makesDelete it, since the code now passes and a passing test adds no informationReplace it with a broader test covering the whole function

Worked example (Python)

def merge_settings(defaults, overrides):
    defaults.update(overrides)      # returns the right value, wrecks the caller's dict
    return defaults


base = {"theme": "light", "rows": 20}
print(merge_settings(base, {"rows": 50}))   # {'theme': 'light', 'rows': 50}
print(base)                                 # {'theme': 'light', 'rows': 50} <- mutated

Apply

Your turn

The task this lesson builds to.

Write a function check(name) that returns True only when the candidate stored under name is a correct is_strong_password, and False otherwise.

The contract: is_strong_password(text) returns True when text is at least 10 characters long, contains at least one digit, contains at least one letter, and contains no space character. Otherwise it returns False.

The starter holds four candidates in CANDIDATES. Two of them are correct. Each rule in the contract needs its own probe case, including one input that sits exactly on the length threshold. Keep check as the last function in the file.

3 hints and 4 automated checks are waiting in the workspace.

Practice

Make it stick

A second problem on the same idea, so it survives past today.

Your app loads a dictionary of default settings once at start-up and merges each user's saved preferences on top of it. Four implementations of that merge are sitting in an open pull request, all of them produce the right settings screen in a manual test, and one reviewer has noticed that a user who changes their row count seems to change it for everyone.

Write a function check(name) that returns True only when the candidate stored under name is a correct merge_settings, and False otherwise.

The contract: merge_settings(defaults, overrides) returns a new dictionary holding every key from both, with the value from overrides winning wherever a key appears in both. Neither argument may be modified. Build fresh input dictionaries inside check, and keep check as the last function in the file.

3 hints and 4 automated checks are waiting in the workspace.