diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index f6542dd0..e7d44ce8 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -61,6 +61,7 @@ Go through each changed file and check for violations. Flag only actual problems - [ ] New functionality has tests (prefer integration tests) - [ ] Bug fixes have an integration test that reproduces the bug (fails before fix, passes after) +- [ ] E2E/integration tests assert observable behavior only (CLI output, exit codes, files, requests received by mocks) — not internal details; external systems may be mocked at the boundary - [ ] Interactive tests use PTY (`github.com/creack/pty`) - [ ] No unchecked errors outside of test files diff --git a/CLAUDE.md b/CLAUDE.md index 075638ba..2da62fab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -232,6 +232,9 @@ When drafting Slack messages, PR descriptions, review replies, release notes, or # Testing +- **TDD is mandatory for every feature and bug fix (red → green → refactor).** First write an end-to-end integration test that reproduces the bug or specifies the new behavior, and run it to confirm it fails for the expected reason. Only then implement the change, and run the test again to confirm it passes. Never write the implementation first and backfill the test. +- E2E tests must assert only **observable behavior** — what a user sees through the CLI (exit codes, output, files written, requests the emulator/wrapped tool receives) — never internal details (function calls, internal state, log internals). If it's unclear what the observable behavior is, ask the user before writing the test. +- To keep e2e tests faithful, it's fine (and encouraged) to provide observable mocks of *external* systems — a mock auth/license endpoint, a fake `aws`/`az` binary on `PATH`, a fake browser opener. These mocks define the boundary between lstk and the outside world; do not mock lstk's own internals in e2e tests. Existing examples: `fakeBrowserOpener` in `test/integration/login_test.go`, mock platform-API servers. - Prefer integration tests to cover most cases. Use unit tests when integration tests are not practical. - **When fixing a bug, always add an integration test** that fails before the fix and passes after. This prevents regressions and documents the exact scenario that was broken. - Integration tests that run the CLI binary with Bubble Tea must use a PTY (`github.com/creack/pty`) since Bubble Tea requires a terminal. Use `pty.Start(cmd)` instead of `cmd.CombinedOutput()`, read output with `io.Copy()`, and send keystrokes by writing to the PTY (e.g., `ptmx.Write([]byte("\r"))` for Enter).