Skip to content

RED can pass for the wrong reason, and inverts on refactors #2146

Description

@kylekellogg

Repo: obra/superpowers - Skill: skills/test-driven-development/SKILL.md - Version seen: 6.2.0

Two related gaps in ### Verify RED - Watch It Fail. Both share a root: RED is a proxy for
the property that actually matters - that the test can distinguish correct from incorrect
- and
in two common situations the proxy and the property come apart. In one the test goes RED without
having that power; in the other it must not go RED at all.

Filing as an issue rather than a PR because the second one argues an existing Red Flag is wrong in
a specific case, which seems worth discussing before wording.


1. An extraction makes RED pass for the wrong reason

The skill says to confirm the test "Fails because feature missing (not typos)". A missing export
satisfies that reading while proving nothing.

Scenario. A bug lives in code with no test harness - inline in a component, a closure, a
handler. The natural TDD move is to extract the logic into a pure function and test it. If you
write the extracted function correctly from the start, the RED run fails with
X is not exported from Y.

That failure proves the test file can resolve an import. It says nothing about whether the
assertions can detect the defect. The test then ships as a regression test that was never shown to
regress - and it is precisely the tests written during bug fixes that are expected to carry
detection power.

Proposed addition to ### Verify RED - Watch It Fail:

Extracting to make code testable? A missing export is not a meaningful RED. Transcribe the
existing (buggy) logic verbatim into the new function first and run the tests: they must fail on
the behaviour, with the wrong value visible in the assertion diff. Then apply the
correction and watch them pass. Two runs, both meaningful - the first proves detection, the
second proves the fix.

The existing "Fails because feature missing (not typos)" bullet could also read
"...(not typos, and not a missing import or export)".

Encountered as: extracting a currency-rounding expression out of a Svelte component into a
shared helper. Written correctly first, RED was dollarsToCents is not exported. Transcribed
buggy-first instead, RED was expected 1999, received 1998 - which is the failure that proves
the test would catch a regression.


2. The "test passes immediately" Red Flag inverts on behaviour-preserving refactors

### Verify RED - Watch It Fail says:

Test passes? You're testing existing behavior. Fix test.

and ## Red Flags - STOP and Start Over lists "Test passes immediately" under
"All of these mean: Delete code. Start over with TDD."

For a characterization test guarding a refactor, testing existing behaviour is the entire
point. Extracting duplicated logic into a shared helper wants a test that pins current behaviour
before the extraction - and that test passing immediately is the correct and required outcome. A
characterization test going RED would mean the refactor had already broken something.

Followed literally, the rule pushes toward one of two bad outcomes: skip the test (leaving the
extraction unguarded - which is typically how the duplicated call sites drifted apart in the first
place), or contrive a failure corresponding to no real defect.

What replaces RED here: the test's discriminating power is unproven precisely because it never
failed. Mutation supplies the missing proof - deliberately break the extracted unit and
confirm the characterization test fails.

Proposed addition to ## Red-Green-Refactor, as a short branch:

Behaviour-preserving change? RED inverts. Write the characterization test first and confirm
it passes against the unchanged source - that is the correct outcome, not a red flag. Then
refactor and confirm it still passes. Because the test never went RED, its discriminating power
is unproven: replace that proof with a mutation. Break the extracted unit deliberately,
confirm the test fails, restore it.

"Test passes immediately" is a red flag when you are adding behaviour and the expected
outcome when you are preserving it. The distinction is which of the two you are doing.

The Red Flags entry could become "Test passes immediately (when adding behaviour - see the
refactor branch)"
so the list stays scannable without contradicting the branch.

Encountered as: consolidating two call sites that had silently diverged (Math.floor vs
Math.round on the same money conversion). The characterization test had to pass against both
originals to be worth anything.


Why this generalises

The skill's own framing is that RED proves the test is real. That is true whenever the correct
behaviour does not yet exist. When it already exists - extraction, refactor, characterization -
RED either measures the wrong thing or cannot happen. Any rule phrased in terms of RED needs a
stated equivalent for behaviour-preserving change, and mutation is that equivalent.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions