From 8373e32f346d0445467021877927839bfc678f64 Mon Sep 17 00:00:00 2001 From: Harald Hoyer Date: Thu, 7 May 2026 13:00:54 +0200 Subject: [PATCH] fix(opencode): forbid RED-state references in test names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow run produced test names like move_enemies_following_path_ panics_on_todo, path_types_randomly_assigned, and spawn_enemies_ special_stage_panics_on_todo. The first and third leak the stub-first RED mechanic into the test name; once @make's body pass turns them GREEN, the name lies. The middle one is too vague to describe a contract. Adds a Test Naming subsection to @test's Test Philosophy stating the TDD survival principle — the name describes the contract under test, not the current state, and must remain accurate after the body pass. Bans ..._panics_on_todo / ..._fails_red / ..._stub_works / generic placeholders / vague verbs / implementation-detail leakage. Requires action + observable outcome and shows bad-to-good rewrites of the three names from this run. --- config/opencode/agents/test.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/config/opencode/agents/test.md b/config/opencode/agents/test.md index 3c4506f..54f7113 100644 --- a/config/opencode/agents/test.md +++ b/config/opencode/agents/test.md @@ -175,6 +175,27 @@ Rust: - Use `assert_eq!`, `assert_ne!`, `assert!` with informative messages. - Use existing test helpers from the crate's `tests/common/` module when present. +### Test Naming + +In TDD, tests are *specifications*. The test name describes the **contract under test**, not the test machinery or the current RED state. The same name must be valid both before the body pass (RED) and after it (GREEN). If a name wouldn't survive the body pass, rename now. + +**Forbidden naming patterns:** +- Anything referencing the stub mechanic: `..._panics_on_todo`, `..._fails_red`, `..._stub_works`, `..._not_yet_implemented`. These describe the RED state, which disappears once `@make` fills in the body. +- Generic placeholders: `test_works`, `it_does_the_thing`, `basic_test`. +- Vague verbs without an outcome: `..._handles_input`, `..._processes_data` — handles or processes how, with what observable result? +- Implementation-detail names that leak internals: `..._calls_query_get_mut_three_times`, `..._uses_hashmap`. + +**Required form: action + observable outcome.** Examples: + +| Bad | Good | +|---|---| +| `move_enemies_following_path_panics_on_todo` | `move_enemies_advances_position_along_path` | +| `path_types_randomly_assigned` | `spawn_in_special_stage_assigns_one_of_three_pattern_types` | +| `spawn_enemies_special_stage_panics_on_todo` | `spawn_enemies_in_special_stage_attaches_flight_pattern_component` | +| `weaving_test` | `weave_enemies_removes_weaving_component_after_duration` | + +The name should read like a sentence: "[subject] [verb] [observable outcome under condition]". When you can't write such a sentence, the test is testing too much (split it) or testing the wrong thing (revisit the spec). + ### Devshell wrapping If the project has a `flake.nix` with a `devShells.default`, wrap every test/lint command with `nix develop -c …` (e.g. `nix develop -c cargo test`, `nix develop -c uv run pytest`). The devshell guarantees the right toolchain is on PATH.