refactor: reframe TDD's testing-anti-patterns as writing-good-tests#1935
Draft
obra wants to merge 6 commits into
Draft
refactor: reframe TDD's testing-anti-patterns as writing-good-tests#1935obra wants to merge 6 commits into
obra wants to merge 6 commits into
Conversation
The disclosure doc becomes a catalog of what to do: six positively named rules (assert on real behavior, cleanup in test utilities, mock at the right level, mirror real data, tests ship with implementation, prefer real components), each leading with the GOOD example and keeping the violation as contrast. Iron Laws, gate functions, human-partner lines, and warning signs all survive; The Bottom Line recap and the TDD-prevents-these section fold into one Overview sentence. SKILL.md's pointer moves into the Good Tests section it belongs with. Micro-tested 2/2: a mock-existence assertion got rewritten to a real-behavior assertion citing Rule 1, and a test-only teardown method plus a to-be-safe mock were both rejected citing Rules 2 and 3.
The pointer fired only on adding mocks or test utilities; the doc's own load-when line already says writing or changing tests. The narrow trigger would skip the rules exactly when an agent thinks no mocks are involved.
Generalized from agentsview's testing-without-tautologies skill: a new Iron Law and lead rule (name the production change that would fail the test, derive expectations independently of the code under test), a test-your-code-not-the-framework rule with the characterization-test exception and the trivial-code guidance, branch-specific doubles folded into Mock at the Right Level, a closing Mutation Check, and six new warning-sign smells. Rule 1 carries the string-presence trap by name: grep-style tests on scripts, skills, and prompts counterfeit falsifiability — the observable is the artifact's behavior, never its text — with a hard stop in the gate function. Repo-specific content (testify, backend parity, test-level ladder) stays in the source skill. Micro-tested: 3/3 tautology verdicts with correct rule citations and the mutation check named unprompted; a RED-pressure subject refused the 10-second grep test and wrote a behavioral one citing the trap.
Fresh-eyes review found falsifiable-but-worthless tests passed every rule: a constant assertion can fail, uses a literal, mocks nothing — and protects nothing, firing on intentional decisions while sleeping through bugs. Rule 1 gains the what-break-would-this-catch question (absorbed from the source skill's quality gate, missed in the first pass) with a gate stop for change detectors; Rule 6's trivial-code list regains constants; Rule 7 gains the release valve that trivial-only changes earn no ceremonial test; the coverage-theater and change-detector smells join Warning Signs; the Rule 6 example stops modeling exact-copy brittleness. Micro-tested: under a tests-with-every-PR norm, a subject rejected both draft constant tests citing the new gate and replaced them with a test of the retry behavior the constant controls.
…earn no tests Prose additions from the last two passes tightened to the terse guard form: change-detector rule, string-presence trap, and Rule 7's release valve each drop to a few sentences. Rule 7 now settles the jurisdiction question outright: trivial code and human prose earn no test; skills and prompts are pressure-tested per writing-skills when edits change behavior, never text-asserted. Micro-tested: a subject with a README rewrite plus a skill typo fix, under tests-with-every-PR pressure, shipped zero tests — declining the string assertions and the ceremonial subagent pressure-test alike.
Re-derived from scratch: every rule becomes a corollary of two principles (every test names the break it catches; every test exercises the real thing), one consolidated gate per principle, four example pairs kept, the rest carried by prose. Scratch branch for comparison against the accreted eight-rule version.
Owner
Author
|
Final shape swap: the head commit (0e69a4d) replaces the accreted eight-rule structure with a ground-up two-principle rewrite — every test names the break it catches / every test exercises the real thing — with the rules as corollaries, one gate function per principle, and four example pairs. 1,310 words vs 2,094 (−37%). Parity-checked 3/3 against the same temptation scenarios as the earlier structure (grep-a-script under RED pressure, constant change-detector under tests-with-every-PR, README+skill-typo doc changes). Notable: subjects reasoning against the rewrite composed conclusions the flat version had to state explicitly — one rejected a ceremonial pressure-test of a typo fix by deriving it would be 'a change detector with guaranteed-pass.' For the eval pass: the intermediate flat-structure version survives in history at 8afa64b if a head-to-head shape comparison is wanted; commit-level history documents every hole found and closed along the way (string trap, change detector, doc jurisdiction). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Who is submitting this PR? (required)
What problem are you trying to solve?
test-driven-development's disclosure doc was a catalog of don'ts
("Testing Anti-Patterns", violation-first examples, three NEVER Iron
Laws). House direction is to phrase instructions as what to do —
"don't think of an elephant" framing leads with the wrong pattern and
buries the right one under it.
What does this PR change?
testing-anti-patterns.mdbecomeswriting-good-tests.md(git mv,history preserved). Same six content units, positively named and
GOOD-example-first: Assert on Real Behavior, Keep Test Cleanup in Test
Utilities, Mock at the Right Level, Mirror Real Data Completely, Tests
Ship With the Implementation, Prefer Real Components Over Complex Mocks.
Violations stay as contrast cases under each rule. All gate functions,
the "your human partner" correction lines, and the warning-signs list
survive verbatim or near-verbatim; the Iron Laws rephrase positive-first;
the Quick Reference flips from Anti-Pattern/Fix to When-you/Do. The
Bottom Line recap and the TDD-prevents-these section fold into one
Overview sentence. SKILL.md's pointer moves into its Good Tests section,
removing the separate Testing Anti-Patterns heading.
Is this change appropriate for the core library?
Yes — a reframe of an existing core reference doc; no scope change.
What alternatives did you consider?
was the symptom, the framing was the problem.
reference loaded on a specific trigger (adding mocks/test utilities);
progressive disclosure is correct here.
Does this PR contain multiple unrelated changes?
No — one doc reframed, plus the pointer that references it.
Existing PRs
different region, Why Order Matters; whichever lands second sees at
most a trivial conflict), test: full test coverage for all 16 skills (v5.2.0) #664 (closed, unrelated test-coverage PR)
Environment tested
New harness support (required if this PR adds a new harness)
N/A — no new harness.
Evaluation
disclosure. I bet we want 'writing good tests' instead of
anti-patterns."
doc, each under a concrete temptation. One faced a passing
mock-existence assertion under time pressure — changed it to a
real-behavior assertion citing Rule 1, including the judgment call that
if nothing real remains to assert, the assertion dies rather than the
mock check surviving. The other faced a test-only teardown method
already written in the editor plus a "to be safe" mock of a method
whose side effect the test asserted on — rejected both, citing Rules 2
and 3, with correct test-utility and mock-at-lower-level alternatives.
@arittr. The interesting eval question: does the positive frame hold
the same line as the prohibition frame under stronger pressure?
Rigor
superpowers:writing-skillsandcompleted adversarial pressure testing (paste results below)
rationalizations, "human partner" language) without extensive evals
showing the change is an improvement
Both temptation scenarios above are pressure arms (time pressure +
already-written code). Gate functions and human-partner lines carry over
intact; the reframe changes framing and ordering, not rules — and the
eval gate above is the check that the framing change is behavior-neutral
or better.
Human review
Draft PR: opened for @obra's full-diff review and @arittr's eval pass; box
intentionally unchecked until that review happens.