diff --git a/skills/planning-a-feature/SKILL.md b/skills/planning-a-feature/SKILL.md index 4c37101..b68e488 100644 --- a/skills/planning-a-feature/SKILL.md +++ b/skills/planning-a-feature/SKILL.md @@ -92,8 +92,8 @@ Each contract names the wire / signature / layout the consumer can write code ag **Realization strategy.** A contract row is conceptual; for parallel work to actually compile, the interface has to exist as code or as data before either side starts. Pick one per row and put it in the Realization column: -- **Pre-merge stub PR** — file a tiny scaffold PR that exports the symbol the consumers need (Go interface or function signature with `panic("unimplemented")` body, TypeScript type, HTTP path constant). It targets the feature branch (in multi-PR features) or main (in single-PR features) and merges BEFORE the implementation PRs branch off. Producer and consumers then all branch from the post-stub state and import the real symbol. Best default for code-shaped contracts (Go signatures, TS types); costs one trivial extra PR; payoff is both sides compile from day one. Reference the stub PR's number in the Realization column once it's open. -- **Stub-on-producer-branch** — the producer's own PR opens with just the interface + panic-bodies; consumers branch from the producer's branch (not main) and rebase as the producer fills in the body. Avoids the extra PR but couples consumers to the producer's branch lifetime — rebase pain when the interface evolves. Use only when the interface and one implementation are inherently coupled (shared unexported package, sibling files in one module). +- **Pre-merge stub PR** — file a tiny scaffold PR that exports the symbol the consumers need: the type or function signature with a not-yet-implemented body, the shared type declaration, the route/path constant. It targets the feature branch (in multi-PR features) or main (in single-PR features) and merges BEFORE the implementation PRs branch off. Producer and consumers then all branch from the post-stub state and import the real symbol. Best default for code-shaped contracts; costs one trivial extra PR; payoff is both sides compile from day one. Reference the stub PR's number in the Realization column once it's open. +- **Stub-on-producer-branch** — the producer's own PR opens with just the signature + not-yet-implemented bodies; consumers branch from the producer's branch (not main) and rebase as the producer fills in the body. Avoids the extra PR but couples consumers to the producer's branch lifetime — rebase pain when the interface evolves. Use only when the interface and one implementation are inherently coupled (a shared private/internal module, sibling files in one package). - **Data-only** — when the contract is a path layout, file format, wire protocol, or env-var name, no code stub is needed. Consumers write against the contract row directly — strings and paths are just strings and paths. Mark the row `data-only`. Sequential work doesn't need contracts. Only document them where two workers genuinely run in parallel. diff --git a/skills/testing-a-feature/SKILL.md b/skills/testing-a-feature/SKILL.md index a279972..dde78f8 100644 --- a/skills/testing-a-feature/SKILL.md +++ b/skills/testing-a-feature/SKILL.md @@ -45,7 +45,7 @@ A clean docstring should already enumerate the surface's promises. If reading it For each behavior the contract promises, ask: - **Happy path** — the documented success case. Always test. -- **Boundary inputs** — zero, one, many; empty string, single char, max length; nil vs empty slice. +- **Boundary inputs** — zero, one, many; empty string, single char, max length; null/absent vs empty collection. - **Error paths** — every error the contract names. Every error the contract _doesn't_ name but the code clearly can return (then either name it in the docstring or make the code not return it). - **Invariants** — what should never happen, regardless of input. Concurrency safety, idempotency, atomicity. - **Failure recovery** — partial failure mid-call, retry semantics, what state survives. @@ -54,7 +54,7 @@ Don't test for behavior the contract doesn't promise. If you're tempted to, the ### 4. Write one test per edge case -Each test name reads as a contract statement: `TestStore_PersistsMultipleSessionsAcrossRestart`, `TestGetChannelID_MissingScopeIsActionable`. The name describes the intent being verified, not the function being called. Each test's assertion is what the contract promises, not what the implementation happens to do today. +Each test name reads as a contract statement — "persists multiple sessions across restart", "missing scope is actionable" — written in whatever naming convention the project already uses. The name describes the intent being verified, not the function being called. Each test's assertion is what the contract promises, not what the implementation happens to do today. ### 5. When tempted to rewrite a test @@ -70,10 +70,10 @@ Rewriting a test "to match" a new implementation when the contract is unchanged Apply per surface, per change: - [ ] **Happy path** — documented success case. -- [ ] **Empty / zero / nil inputs** — what does the contract say happens? If it doesn't say, fix the contract. +- [ ] **Empty / zero / absent inputs** — what does the contract say happens? If it doesn't say, fix the contract. - [ ] **Boundary values** — first / last / single-element / off-by-one neighbors. - [ ] **Error paths named in the contract** — each one triggered and asserted. -- [ ] **Concurrency** — race detector clean (Go: `-race`); concurrent writers if the contract promises safety. +- [ ] **Concurrency** — clean under whatever race/thread detector the toolchain provides; concurrent writers if the contract promises safety. - [ ] **Idempotency** — does calling twice produce the same result the contract promises? - [ ] **Persistence** — does on-disk / on-network state survive restart, if the contract says so? - [ ] **Partial failure** — what state survives a mid-call error? @@ -93,7 +93,7 @@ Apply per surface, per change: | ---------------------------------------------------------------- | -------------------------------------------------------------------------------------------------- | | "The function is too small to test" | If it's worth writing, its contract is worth asserting. | | "The test is fragile, let me loosen the assertion" | Fragile = coupled to implementation. Tighten the contract or rewrite the test against intent. | -| "I'll skip the nil case, the caller will always pass non-nil" | The contract should say "never call with nil" then. If it doesn't, test the nil case. | +| "I'll skip the absent case, the caller will always pass a value" | The contract should say "never call with an absent value" then. If it doesn't, test the absent case. | | "The docstring is wrong but the implementation is right" | Fix the docstring (or the implementation, if the docstring is the source of truth) before testing. | | "Rewriting the test to match the new code is faster" | And weakens the suite silently. Verify the contract changed first. | | "Edge cases can wait for the next PR" | They get forgotten. List them now even if you don't write them all. |