ADR-300: Standardize TDD practice rules and add logging to coverage checklist - #51
Conversation
- test-driven-development: add Practice rules section (AAA structure, red-must-fail-for-the-right-reason, frozen-Arrange/Act-after-first-green, and the default <Component>_<Scenario>_<ExpectedResult> naming convention, deferring to developer-standards when the target project documents its own). - code-change-expectations: add a fourth self-review checklist item requiring logging differentiation by branch/condition to be asserted, unless the member is branch-free.
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis-claude
left a comment
jodavis-claude
left a comment
There was a problem hiding this comment.
Reviewed against ADR-300's exit criteria. Both SKILL.md files are updated correctly and the required rules/checklist item are present with clearly citable headings for ADR-301/302 to reference. One documentation-scope issue flagged inline; otherwise looks solid. Dry run for exit criterion 3 is documented in the PR description as required by the spec.
Carve out Gherkin E2E/API scenarios (step 1) from the new Practice rules section so AAA structure and the naming convention aren't misread as applying to feature files, which follow write-e2e-test's own conventions instead. Addresses PR #51 review comment.
jodavis-claude
left a comment
jodavis-claude
left a comment
There was a problem hiding this comment.
Sign-off review for ADR-300.
Prior review thread: the one open Priority 4 (documentation) comment on test-driven-development/SKILL.md — about the Practice rules intro reading as if it applied to Gherkin E2E scenarios too — has been addressed satisfactorily in commit 7098b2f. The intro now explicitly scopes the rules to unit tests written in step 2 and carves out E2E/API scenarios to follow write-e2e-test's own conventions. Thread resolved.
Scan of files modified since last review pass: only plugins/dev-team/skills/test-driven-development/SKILL.md changed in the fix commit (5 insertions, 3 deletions, intro paragraph only). No new issues found — the change is minimal, accurate, and consistent with the rest of the document's style.
Exit criteria check:
- test-driven-development/SKILL.md states AAA structure, red-must-fail-for-the-right-reason, frozen-Arrange/Act, and the naming convention (deferring to developer-standards) — confirmed, each as its own citable heading.
- code-change-expectations/SKILL.md's checklist includes the logging-differentiation-by-branch item, with the branch-free exemption — confirmed.
- Dry run demonstrating a frozen-Arrange/Act violation is documented in the Implementation Summary and correctly flags the synthetic violation.
No blocking issues. Approving sign-off.
…estration skill Adds the sub-agent pair that drives one Testable component through the structural-then-behavioral red/green TDD ping-pong loop, plus the implement-tdd skill Developer uses to orchestrate them turn by turn through to done + commit. Post-done tdd-refactorer wiring is explicitly out of scope (ADR-302). Proceeds referencing test-driven-development's anticipated Practice-rule headings (AAA structure, Red must fail for the right reason, Arrange/Act frozen after first green, Naming convention) by name, per ADR-300 (PR #51, still open) landing those headings unchanged.
3 participants
Work item
ADR-300 — Fold the AAA / red-for-the-right-reason / frozen-Arrange-Act / naming-convention TDD practice rules into
test-driven-development, and add logging as a fourth coverage-checklist item incode-change-expectations, so both skills carry these rules as standardized dev-team conventions rather than leaving them to be reinvented by future tasks (ADR-301/302) that will reference them by name.Changes
plugins/dev-team/skills/test-driven-development/SKILL.md— added a new "Practice rules" section after the existing three numbered steps, with four citable subsections: "AAA structure," "Red must fail for the right reason," "Arrange and Act are frozen after first green," and "Naming convention" (defaults to<Component>_<Scenario>_<ExpectedResult>, deferring to thedeveloper-standardsskill when the target project documents its own convention).plugins/dev-team/skills/code-change-expectations/SKILL.md— added a fourth bullet to the "Self-review before reporting" checklist: logging that differs by branch or condition must be asserted like any other observable behavior, unless the member is branch-free.No other files were touched —
agents/developer.md,agents/researcher.md,plugin.json, and the newimplement-direct/implement-tdd/tdd-*files are explicitly out of scope for this task (later ADRs) and were not created or modified.Design decisions
###heading for ADR-301/302 to reference by name without disrupting the existing numbered flow of steps 1-3.test-driven-development), rather than requiring the violation-detection logic to live specifically insidecode-change-expectations's checklist.Dry run performed (exit criterion 3)
Constructed a synthetic before/after test diff simulating a violation of the new "Arrange and Act are frozen after first green" rule (an existing test's Act was rewritten in place to cover a different scenario instead of adding a new test method). Reviewing the diff against the new rule correctly flags it as a violation.