diff --git a/skills/tdd/SKILL.md b/skills/tdd/SKILL.md index 7babfa0..3b7f09c 100644 --- a/skills/tdd/SKILL.md +++ b/skills/tdd/SKILL.md @@ -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 diff --git a/skills/tdd/references/java.md b/skills/tdd/references/java.md new file mode 100644 index 0000000..eb3a941 --- /dev/null +++ b/skills/tdd/references/java.md @@ -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 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. diff --git a/skills/tdd/references/python.md b/skills/tdd/references/python.md new file mode 100644 index 0000000..c9bed30 --- /dev/null +++ b/skills/tdd/references/python.md @@ -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. diff --git a/skills/tdd/references/rust.md b/skills/tdd/references/rust.md new file mode 100644 index 0000000..dc63059 --- /dev/null +++ b/skills/tdd/references/rust.md @@ -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. diff --git a/skills/tdd/references/typescript.md b/skills/tdd/references/typescript.md new file mode 100644 index 0000000..22e6848 --- /dev/null +++ b/skills/tdd/references/typescript.md @@ -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.