docs(tdd): make the spine language-neutral, add four language references
The discipline in tdd was already language-agnostic; the checklist under it was not. A Python user inherited a mandatory `go test -race` box they can never tick and never got told that `mypy --strict` belongs in the red gate. Per #18's own comment and writing-skills' Multi-Language Dilution warning, the main SKILL.md keeps ONE canonical inline example (Go, the default stack) and the other four languages live in reference files: python, typescript, rust, java. The RED run drove every change. A subagent given a Python task with the Go-only skill reported, in its own words: - "Everything from L169 to L244 is Go-only, and the verification checklist at L326 silently inherits that Go-ness — which is why a Python user ends up with a mandatory box (-race) they can never tick and a required gate (mypy --strict) that isn't listed at all." - "For a brand-new function, the first RED is always a collection-time ImportError... The only clean resolutions are a stub that raises NotImplementedError (production code before a red test?) or accepting ImportError as a legitimate red. The skill picks neither, and this is the single biggest gap." It now picks: that red is legitimate, and writing a stub to satisfy the runner is the Iron Law with extra steps. - The Go table-driven exemplar branches on a `wantErr` column inside the loop. Translated literally that is a conditional assertion folding two behaviours into one test — which this skill's own quality table forbids. The agent caught itself mid-translation; both SKILL.md and python.md now say not to port it. Also: the checklist names the static-analysis gate per language instead of assuming `go vet`, and says to strike the concurrency box deliberately where no equivalent exists — a box that cannot be ticked teaches people to skip checklists. Refs #18 Bump-Type: minor
This commit is contained in:
+37
-6
@@ -100,13 +100,21 @@ go test -run TestRetryOperation_RetriesThreeTimes ./...
|
||||
```
|
||||
|
||||
Confirm:
|
||||
- Test fails (not compilation errors)
|
||||
- Failure message is expected
|
||||
- Fails because feature is missing (not typos)
|
||||
- Test fails, and fails because the feature is missing
|
||||
- The failure message is the one you expected
|
||||
- It is not failing on a typo in the test itself
|
||||
|
||||
**Test passes?** You're testing existing behavior. Fix test.
|
||||
|
||||
**Compilation errors?** Fix errors, re-run until it fails correctly.
|
||||
**Failing because the thing does not exist yet?** That is a legitimate red — a Go
|
||||
compile error for an undefined function, a Python `ImportError` for a module you have
|
||||
not written, a TypeScript "cannot find module". Read the message, confirm it names the
|
||||
unit you are about to build, and proceed to GREEN. **Do not write a stub to make the
|
||||
runner happier.** A stub is production code written before a failing test, which is the
|
||||
Iron Law with extra steps.
|
||||
|
||||
**Failing on a typo or a broken import elsewhere?** Different thing. Fix it and re-run
|
||||
until the failure is the one above.
|
||||
|
||||
### GREEN - Minimal Code
|
||||
|
||||
@@ -166,6 +174,23 @@ Keep tests green. Don't add behavior.
|
||||
|
||||
Next failing test for next behavior.
|
||||
|
||||
## Other languages
|
||||
|
||||
The discipline above is language-neutral; the examples below it are Go, because Go is
|
||||
this fleet's default stack and one good example beats five mediocre ones.
|
||||
|
||||
For anything else, the per-language conventions live in reference files — load the one
|
||||
you need rather than translating Go idioms by eye. Translation is where the mistakes
|
||||
happen: the Go table-driven exemplar below branches on a `wantErr` column inside the
|
||||
loop, which is idiomatic Go and an anti-pattern in pytest, where it produces a
|
||||
conditional assertion and folds two behaviours into one test.
|
||||
|
||||
- `references/python.md` — pytest, `@pytest.mark.parametrize`, `mypy --strict` in the
|
||||
RED gate, `pytest.raises(..., match=)`, fixtures vs helpers
|
||||
- `references/typescript.md` — vitest/jest, `it.each`, `expect(...).rejects`
|
||||
- `references/rust.md` — `#[cfg(test)]` modules, table-driven via arrays, `#[should_panic]`
|
||||
- `references/java.md` — JUnit 5, `@ParameterizedTest`, `assertThrows`
|
||||
|
||||
## Go-Specific TDD Notes
|
||||
|
||||
### Table-Driven Tests (Preferred Pattern)
|
||||
@@ -329,8 +354,14 @@ Before marking work complete:
|
||||
- [ ] Watched each test fail before implementing
|
||||
- [ ] Each test failed for expected reason (feature missing, not typo)
|
||||
- [ ] Wrote minimal code to pass each test
|
||||
- [ ] All tests pass: `go test ./...`
|
||||
- [ ] Race detector clean: `go test -race ./...`
|
||||
- [ ] All tests pass
|
||||
- [ ] The language's static gate is clean — `go vet`, `mypy --strict`, `tsc --noEmit`,
|
||||
`cargo clippy`. A red type-check is a red build; it does not wait for GREEN.
|
||||
- [ ] The language's concurrency gate is clean **where one exists and applies** —
|
||||
`go test -race`, `cargo test` under Miri for unsafe code. There is no equivalent
|
||||
for a pure Python function, and a checklist box that can never be ticked teaches
|
||||
people to skip checklists. Tick it or strike it deliberately; do not leave it
|
||||
hanging.
|
||||
- [ ] Tests use real code (mocks only if crossing a system boundary)
|
||||
- [ ] Edge cases and errors covered
|
||||
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
# TDD in Java
|
||||
|
||||
The Iron Law and Red-Green-Refactor in `SKILL.md` apply unchanged. This file replaces
|
||||
the Go-Specific TDD Notes for Java. JUnit 5 (Jupiter) throughout.
|
||||
|
||||
## The first red is a compile error, and that is correct
|
||||
|
||||
`cannot find symbol: method splitVat` is a legitimate red. Do not stub the method with
|
||||
`throw new UnsupportedOperationException()` to make the module compile — that is
|
||||
production code before a failing test.
|
||||
|
||||
## Table-driven: `@ParameterizedTest`
|
||||
|
||||
```java
|
||||
@ParameterizedTest(name = "{0}")
|
||||
@MethodSource("vatCases")
|
||||
void roundsHalfToEven(String name, long amountMinor, BigDecimal rate, long net, long vat) {
|
||||
assertEquals(new Split(net, vat), VatSplitter.split(amountMinor, rate));
|
||||
}
|
||||
|
||||
static Stream<Arguments> vatCases() {
|
||||
return Stream.of(
|
||||
Arguments.of("tie rounds down to even", 14L, new BigDecimal("0.12"), 12L, 2L),
|
||||
Arguments.of("tie rounds up to even", 42L, new BigDecimal("0.12"), 38L, 4L),
|
||||
Arguments.of("exact division", 12500L, new BigDecimal("0.25"), 10000L, 2500L)
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
`name = "{0}"` puts the case label in the report — the `t.Run` analogue. `@CsvSource` is
|
||||
terser for primitives; `@MethodSource` is the one to use as soon as a case carries a
|
||||
`BigDecimal` or a domain object.
|
||||
|
||||
## Error cases get their own test
|
||||
|
||||
```java
|
||||
@Test
|
||||
void rejectsNegativeAmount() {
|
||||
var e = assertThrows(IllegalArgumentException.class,
|
||||
() -> VatSplitter.split(-1, new BigDecimal("0.25")));
|
||||
assertTrue(e.getMessage().contains("non-negative"));
|
||||
}
|
||||
```
|
||||
|
||||
Assert on the message, not only the type — `IllegalArgumentException` is thrown by half
|
||||
the JDK, and a test that accepts any of them will keep passing for the wrong reason.
|
||||
|
||||
## Money is `BigDecimal` or `long` minor units, never `double`
|
||||
|
||||
`0.1 + 0.2 != 0.3` is not a rounding curiosity in a payments system, it is a defect.
|
||||
Where a rounding mode matters, state it: `setScale(0, RoundingMode.HALF_EVEN)`. Banker's
|
||||
rounding is the default expectation in Swedish accounting and is *not* Java's default.
|
||||
|
||||
## Assertions
|
||||
|
||||
Prefer AssertJ (`assertThat(x).isEqualTo(y)`) where the project already has it: the
|
||||
failure messages name the actual value. Where it does not, plain JUnit assertions are
|
||||
fine — do not add a dependency for aesthetics.
|
||||
|
||||
`assertAll` is the closest analogue to Go's soft `assert`: it reports every failure in
|
||||
the group instead of stopping at the first.
|
||||
|
||||
## Mocks
|
||||
|
||||
Mockito is standard, and easy to overuse. Per `SKILL.md`, mock only across a system
|
||||
boundary. A mocked class you own tests the mock; a `@SpringBootTest` that mocks the
|
||||
repository tests neither the wiring nor the query.
|
||||
|
||||
## Gates
|
||||
|
||||
```bash
|
||||
./gradlew test # the failing test
|
||||
./gradlew check # static gate: compiler warnings, spotless, errorprone
|
||||
```
|
||||
|
||||
Java has no `-race`. For concurrent code the honest substitutes are a stress test with
|
||||
`CountDownLatch` to force the interleaving, and jcstress for lock-free work. A pure
|
||||
function should strike that checklist box deliberately.
|
||||
@@ -0,0 +1,109 @@
|
||||
# TDD in Python
|
||||
|
||||
The Iron Law, Red-Green-Refactor and the rationalization table in `SKILL.md` apply
|
||||
unchanged. This file replaces the Go-Specific TDD Notes for Python work.
|
||||
|
||||
Canonical stack (`~/dev/.context/AGENT.md`): pytest + pytest-cov, `mypy --strict`, ruff,
|
||||
uv, src-layout. Not restated here — that table has one home.
|
||||
|
||||
## The first red is an ImportError, and that is correct
|
||||
|
||||
For a function that does not exist yet, the first run fails at collection:
|
||||
|
||||
```
|
||||
E ModuleNotFoundError: No module named 'invoicing.vat'
|
||||
```
|
||||
|
||||
That is a legitimate red. Read it, confirm it names the module you are about to write,
|
||||
and go to GREEN. **Do not create a stub raising `NotImplementedError` to make pytest
|
||||
collect.** A stub is production code written before a failing test.
|
||||
|
||||
## Table-driven: `parametrize`, one behaviour per function
|
||||
|
||||
```python
|
||||
@pytest.mark.parametrize(
|
||||
("amount_minor", "rate", "expected"),
|
||||
[
|
||||
pytest.param(14, Decimal("0.12"), (12, 2), id="tie-rounds-down-to-even"),
|
||||
pytest.param(42, Decimal("0.12"), (38, 4), id="tie-rounds-up-to-even"),
|
||||
pytest.param(12500, Decimal("0.25"), (10000, 2500), id="exact-division"),
|
||||
pytest.param(0, Decimal("0.25"), (0, 0), id="zero-is-not-an-error"),
|
||||
],
|
||||
)
|
||||
def test_split_vat_rounds_half_to_even(
|
||||
amount_minor: int, rate: Decimal, expected: tuple[int, int]
|
||||
) -> None:
|
||||
assert split_vat(amount_minor, rate) == expected
|
||||
```
|
||||
|
||||
`id=` is the analogue of Go's `t.Run` name: it is what appears in the failure output and
|
||||
what you pass to `pytest -k`.
|
||||
|
||||
**Do not port Go's `wantErr` column.** The Go exemplar branches inside the loop —
|
||||
`if tt.wantErr { assert.Error } else { assert.NoError }`. Translated literally that
|
||||
gives a conditional assertion and folds two behaviours into one test, which `SKILL.md`'s
|
||||
own quality table forbids ("One thing. 'and' in name? Split it."). Error cases get
|
||||
their own function:
|
||||
|
||||
```python
|
||||
def test_split_vat_rejects_negative_amount() -> None:
|
||||
with pytest.raises(ValueError, match="amount_minor must be non-negative"):
|
||||
split_vat(-1, Decimal("0.25"))
|
||||
```
|
||||
|
||||
`match=` pins the message, not just the type. Go's `assert.Error` does not, and the
|
||||
stricter version is available here for free — use it.
|
||||
|
||||
## `mypy --strict` is part of RED, not a chore after GREEN
|
||||
|
||||
A red type-check is a red build.
|
||||
|
||||
```bash
|
||||
uv run pytest tests/test_vat.py -x -q # the failing test
|
||||
uv run mypy --strict src tests # same gate, same phase
|
||||
```
|
||||
|
||||
Under `--strict` every test function needs `-> None`, or it is an untyped def and the
|
||||
type-checker rejects the file — including the file that holds your failing test.
|
||||
|
||||
## Layout and imports
|
||||
|
||||
- `src/` layout, tests in a sibling `tests/`. This is the *opposite* of Go's
|
||||
same-directory convention, and it buys black-box testing structurally: `from
|
||||
invoicing.vat import split_vat` can only resolve against the installed package, so
|
||||
the preference Go has to state explicitly is enforced here by the import system.
|
||||
- No `__init__.py` in `tests/` unless you need shared test packages; pytest's rootdir
|
||||
handling copes, and an accidental package shadows names.
|
||||
- `conftest.py` holds fixtures shared across a directory. A fixture in a single test
|
||||
file that only one test uses should be a plain function call instead — fixtures are
|
||||
indirection, and indirection is only worth it at reuse.
|
||||
|
||||
## No `require` vs `assert`
|
||||
|
||||
Every pytest `assert` is fatal to its test; there is no continue-on-failure form. Where
|
||||
Go would use `assert` to collect several soft failures, either split the test or use
|
||||
`pytest.approx`/a single structural comparison:
|
||||
|
||||
```python
|
||||
assert result == Invoice(id=InvoiceId("42"), net=10000, vat=2500) # one assertion, whole shape
|
||||
```
|
||||
|
||||
Comparing whole values beats three field assertions: the failure output shows the entire
|
||||
diff rather than the first field that differed.
|
||||
|
||||
## Fixtures vs helpers
|
||||
|
||||
- **Helper function** — when the setup is cheap and local. It reads top-to-bottom.
|
||||
- **Fixture** — when the setup is shared, or needs teardown, or needs a scope
|
||||
(`session`, `module`) because it is expensive.
|
||||
- Set `__tracebackhide__ = True` in an assertion helper so failures point at the caller,
|
||||
the way `t.Helper()` does in Go.
|
||||
|
||||
## Coverage is not the concurrency gate
|
||||
|
||||
`SKILL.md`'s checklist asks for the language's concurrency gate. Python has no `-race`
|
||||
equivalent, and pretending `pytest --cov` fills that slot is dishonest — coverage says
|
||||
which lines ran, not whether two threads can interleave badly. For a pure function,
|
||||
strike the box deliberately. For genuinely concurrent code (`asyncio`, threads), the
|
||||
substitute is a test that runs the interleaving you fear, plus `pytest-timeout` so a
|
||||
deadlock fails rather than hangs.
|
||||
@@ -0,0 +1,86 @@
|
||||
# TDD in Rust
|
||||
|
||||
The Iron Law and Red-Green-Refactor in `SKILL.md` apply unchanged. This file replaces
|
||||
the Go-Specific TDD Notes for Rust.
|
||||
|
||||
## The first red is a compile error, and that is correct
|
||||
|
||||
```
|
||||
error[E0425]: cannot find function `split_vat` in this scope
|
||||
```
|
||||
|
||||
Legitimate — the compiler is the test runner's first phase. Read it, confirm it names
|
||||
the item you are about to write, and go to GREEN. Do not write a `todo!()` stub first;
|
||||
that is production code before a failing test. (`todo!()` is fine *during* GREEN as a
|
||||
branch you have not reached yet.)
|
||||
|
||||
## Where tests live
|
||||
|
||||
- **Unit tests** in the same file, in `mod tests` behind `#[cfg(test)]`. They can reach
|
||||
private items — the white-box option Go spells `package foo`.
|
||||
- **Integration tests** in `tests/`, which link the crate as an external consumer and see
|
||||
only the public API. This is the black-box default `SKILL.md` prefers; use it unless
|
||||
the behaviour under test is genuinely private.
|
||||
|
||||
```rust
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn tie_rounds_down_to_even() {
|
||||
assert_eq!(split_vat(14, dec!(0.12)).unwrap(), (12, 2));
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Table-driven: an array of cases
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn rounds_half_to_even() {
|
||||
let cases = [
|
||||
("tie rounds down", 14u64, dec!(0.12), (12, 2)),
|
||||
("tie rounds up", 42, dec!(0.12), (38, 4)),
|
||||
("exact division", 12500, dec!(0.25), (10000, 2500)),
|
||||
];
|
||||
for (name, amount, rate, want) in cases {
|
||||
assert_eq!(split_vat(amount, rate).unwrap(), want, "case: {name}");
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The `"case: {name}"` message is the `t.Run` analogue — without it a failure names a line
|
||||
number and leaves you counting rows. If you want each case to be its own reported test,
|
||||
use `rstest`'s `#[case]`; that is a dependency, so justify it before adding it.
|
||||
|
||||
## Error cases
|
||||
|
||||
`Result` makes the Go `wantErr` column unnecessary — assert on the variant:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn rejects_negative_amount() {
|
||||
let err = split_vat_signed(-1, dec!(0.25)).unwrap_err();
|
||||
assert!(matches!(err, VatError::NegativeAmount));
|
||||
}
|
||||
```
|
||||
|
||||
`#[should_panic(expected = "...")]` is for genuine panics only — invariant violations,
|
||||
not expected error paths. A function whose contract includes "rejects negative input"
|
||||
should return `Err`, not panic, and a test that accepts a panic hides that design choice.
|
||||
|
||||
## Gates
|
||||
|
||||
```bash
|
||||
cargo test # the failing test
|
||||
cargo clippy -- -D warnings # static gate, same phase as RED
|
||||
cargo fmt --check
|
||||
```
|
||||
|
||||
`cargo test` compiles in debug, so integer overflow panics rather than wrapping — a
|
||||
release build will not. Where overflow matters, test the checked path explicitly
|
||||
(`checked_mul`), or you are testing debug-only behaviour.
|
||||
|
||||
For unsafe code, `cargo +nightly miri test` is the nearest thing to Go's `-race`.
|
||||
Safe-only crates have no equivalent and should strike that checklist box deliberately.
|
||||
@@ -0,0 +1,85 @@
|
||||
# TDD in TypeScript
|
||||
|
||||
The Iron Law and Red-Green-Refactor in `SKILL.md` apply unchanged. This file replaces
|
||||
the Go-Specific TDD Notes for TypeScript.
|
||||
|
||||
Vitest is the default; Jest differs only in imports and a few matcher names, both noted
|
||||
where it matters.
|
||||
|
||||
## The first red is a resolution error, and that is correct
|
||||
|
||||
```
|
||||
Error: Failed to resolve import "./vat" from "src/vat.test.ts"
|
||||
```
|
||||
|
||||
Legitimate. Do not create an empty module or a stub throwing `new Error("not
|
||||
implemented")` to get past collection — that is production code before a failing test.
|
||||
|
||||
## Table-driven: `it.each`
|
||||
|
||||
```ts
|
||||
describe('splitVat', () => {
|
||||
it.each([
|
||||
{ amountMinor: 14, rate: 0.12, expected: [12, 2], name: 'tie rounds down to even' },
|
||||
{ amountMinor: 42, rate: 0.12, expected: [38, 4], name: 'tie rounds up to even' },
|
||||
{ amountMinor: 12500, rate: 0.25, expected: [10000, 2500], name: 'exact division' },
|
||||
])('$name', ({ amountMinor, rate, expected }) => {
|
||||
expect(splitVat(amountMinor, rate)).toEqual(expected)
|
||||
})
|
||||
})
|
||||
```
|
||||
|
||||
`$name` interpolates into the test title, which is the `t.Run` analogue. Prefer the
|
||||
object form over the array form — a positional tuple with five entries is unreadable at
|
||||
the point of failure.
|
||||
|
||||
**Do not port Go's `wantErr` column.** Error cases get their own test:
|
||||
|
||||
```ts
|
||||
it('rejects a negative amount', () => {
|
||||
expect(() => splitVat(-1, 0.25)).toThrow(/must be non-negative/)
|
||||
})
|
||||
|
||||
it('rejects a negative amount (async)', async () => {
|
||||
await expect(splitVat(-1, 0.25)).rejects.toThrow(/must be non-negative/)
|
||||
})
|
||||
```
|
||||
|
||||
The `await` on the async form is load-bearing. Without it the assertion returns a
|
||||
promise nobody checks, the test passes, and it will keep passing after you break the
|
||||
code. This is the single most common false-green in a TypeScript suite.
|
||||
|
||||
## `tsc --noEmit` is part of RED
|
||||
|
||||
A red type-check is a red build:
|
||||
|
||||
```bash
|
||||
npx vitest run src/vat.test.ts # the failing test
|
||||
npx tsc --noEmit # same gate, same phase
|
||||
```
|
||||
|
||||
Vitest transpiles without type-checking, so a suite can be green while the code does not
|
||||
compile. `tsc --noEmit` is not optional here — it is the half of the gate the runner
|
||||
does not perform.
|
||||
|
||||
## Equality matchers are not interchangeable
|
||||
|
||||
- `toBe` — `Object.is`. For primitives and identity.
|
||||
- `toEqual` — structural, and **ignores `undefined` properties**: `{a: 1, b: undefined}`
|
||||
equals `{a: 1}`. That is a real hole when the bug you are hunting is a missing field.
|
||||
- `toStrictEqual` — structural, respects `undefined`, and checks class identity. Default
|
||||
to this one; reach for `toEqual` deliberately.
|
||||
|
||||
## Mocks
|
||||
|
||||
`vi.mock` (Jest: `jest.mock`) is hoisted above imports, so a factory referencing an
|
||||
outer variable throws at runtime. Use `vi.hoisted` when the mock needs shared state.
|
||||
Per `SKILL.md`, mock only across a system boundary — a mocked module you own is a test
|
||||
of your mock.
|
||||
|
||||
## Async and fake timers
|
||||
|
||||
- `vi.useFakeTimers()` needs a matching `vi.useRealTimers()` in teardown, or the next
|
||||
file inherits frozen time and fails somewhere unrelated.
|
||||
- Prefer condition polling (`await vi.waitFor(...)`) over a fixed `setTimeout` — an
|
||||
arbitrary wait is a flake with a schedule.
|
||||
Reference in New Issue
Block a user