Add BDD specification skill and bridge toolkit features to tests - #54
Conversation
Introduce a bdd-specification skill that enforces Living Documentation through Behaviour-Driven Development: a language-agnostic Gherkin feature spec bridged into unit tests via a strict scenario-to-test naming convention and Given/When/Then anchors, for target languages with or without a native BDD framework. Wire it as the strict default for any new or changed observable behaviour from architecture-impact (request/analysis time) and implement-issue-workflow (implementation time). It is relaxed only with an explicit, recorded human waiver; pure refactorings keep the existing specs green. Add a bdd-writing-guide reference, a prompt template, README and prompt-index entries, and regenerate the agent adapters. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Dogfood the bdd-specification skill on the toolkit's own executable features. Add Gherkin living documentation under features/ for the metamodel validator, the documentation generators, the agent adapter generator, and the validation CLI. Bridge the specifications into tests with the strict scenario-to-test naming convention and Given/When/Then anchors: - adapt the existing validator/generator unit tests and add the missing validator-rule cases (duplicate ids, relation shape and keys, unknown relation type, front-matter errors, missing docs target, generated skip, report status); - add a CLI integration test driving the script via a subprocess; - add node:test coverage for the adapter generator (build, --check, staleness, adapter_expose) against an isolated workspace. Wire the new CLI and JS test steps plus Node setup into the validate workflow, and document the test layout in the README. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Review feedbackThe overall direction is strong: the scope is correctly limited to observable behaviour, the human-waiver rule prevents agents from silently bypassing the contract, existing test frameworks are preferred over introducing BDD dependencies by default, and the additional CLI/generator tests are a real improvement. I would still adjust three conceptual points before merging. 1. Do not couple one Gherkin scenario to exactly one unit testThe current contract says that every scenario has exactly one corresponding test and that behaviour tests trace back to scenarios. This mixes two different abstraction levels:
Otherwise, the feature files risk becoming technical test catalogues written in Gherkin syntax rather than useful Living Documentation. Suggested rule:
A scenario should also be allowed to map to a test class, nested test group, or several supporting tests rather than exactly one test method. The corresponding wording in 2. Either validate the traceability or weaken the claim of strict couplingThe PR calls the This means a scenario can be renamed, added, or removed while all tests and CI remain green. In that case, the documentation is not technically kept alive by the build. Two viable approaches: Pragmatic approachWeaken the claim from strict/executable coupling to reviewer-verifiable traceability:
Strict approachAdd a lightweight dependency-free consistency check. Stable IDs would be more robust than scenario titles, for example: @scenario-id(PAYMENT-003)
Scenario: Payment is rejected when the credit limit is exceededand in the test code: @Scenario("PAYMENT-003")Names are useful for readability, but poor identifiers: improving the wording of a scenario should not silently break traceability. 3. arc42 Chapter 6 should not be the default home for every scenarioChapter 6 is a good location when a scenario explains runtime collaboration between architectural building blocks. But not every Gherkin scenario is an architecture-relevant runtime scenario. A more differentiated rule would be:
Otherwise, Chapter 6 may gradually become a collection of validation and acceptance details rather than a focused Runtime View. ConclusionThe implementation and CI changes look solid, but I recommend adjusting the contract semantics before merging:
With those changes, the skill should provide strong BDD guidance without turning Living Documentation into a costly naming convention. |
Address three conceptual review points on PR #54: 1. Decouple scenarios from unit tests. A scenario now maps to at least one automated verification (prefer acceptance/component/use-case), may map to a test class/group/several tests, and supporting unit tests need no separate scenario. Removes the exactly-one-test rule from the skill, the writing guide, and the wiring skills so feature files stay Living Documentation rather than Gherkin test catalogues. 2. Weaken the coupling claim to reviewer-verifiable traceability rather than strict executable coupling, since no CI check enforces the link. Recommend a stable scenario id (for example @scenario-id(...)) as a more robust identifier than the title. No new dependency or checker. 3. Choose the arc42 location by architectural relevance instead of defaulting every scenario to Chapter 6: Chapter 6 for runtime collaboration, Chapter 10 for quality behaviour, requirement-oriented sections for functional acceptance; non-architectural scenarios may stay in features/. Docs-only change; validator, CLI, and adapter tests remain green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
Danke für das Review — alle drei Punkte sind in 1. Szenario ↔ Unit-Test entkoppelt. Neue Regel im Skill, im 3. arc42-Ort nach architektonischer Relevanz. Kapitel 6 ist nicht mehr Default. Neue Differenzierung: Ch6 für Runtime-Kollaboration zwischen Bausteinen, Ch10 für qualitätsbezogenes Verhalten, anforderungsorientierte Sektionen/Referenzen für funktionale Akzeptanz — und nicht-architekturrelevante Szenarien dürfen in 2. Traceability — Entscheidung nötig. Ich habe den pragmatischen Pfad gewählt: der Bridge ist jetzt explizit als reviewer-verifizierbare Konvention beschrieben, nicht als build-erzwungene/ausführbare Kopplung (Skill, README und Feature-Header entsprechend umformuliert). Zusätzlich empfiehlt das Skill jetzt stabile Szenario-IDs (z. B. Bewusst nicht hinzugefügt: der maschinenprüfbare CI-Check aus deinem „strict approach" — das hätte stabile IDs in allen 👉 Wenn du für dieses Repo doch die strikte, maschinenprüfbare Variante möchtest, rüste ich sie gern nach: |
Kleine RestbeobachtungIm Writing Guide steht in der Einleitung noch:
Direkt darunter wird diese Kopplung allerdings korrekt relativiert und als reviewer-verifizierbare Konvention erklärt. FazitApproved / mergebar. Die Nacharbeit trifft das Feedback sehr gut und macht den Skill wesentlich praxistauglicher, ohne den Anspruch an Living Documentation weichzuspülen. |
Replace "strictly coupled to the tests" with "clearly traceable to the tests" in the writing guide intro so the opening matches the reviewer-verifiable traceability rule described below it. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Address three conceptual review points on PR #54: 1. Decouple scenarios from unit tests. A scenario now maps to at least one automated verification (prefer acceptance/component/use-case), may map to a test class/group/several tests, and supporting unit tests need no separate scenario. Removes the exactly-one-test rule from the skill, the writing guide, and the wiring skills so feature files stay Living Documentation rather than Gherkin test catalogues. 2. Weaken the coupling claim to reviewer-verifiable traceability rather than strict executable coupling, since no CI check enforces the link. Recommend a stable scenario id (for example @scenario-id(...)) as a more robust identifier than the title. No new dependency or checker. 3. Choose the arc42 location by architectural relevance instead of defaulting every scenario to Chapter 6: Chapter 6 for runtime collaboration, Chapter 10 for quality behaviour, requirement-oriented sections for functional acceptance; non-architectural scenarios may stay in features/. Docs-only change; validator, CLI, and adapter tests remain green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Summary
Introduces a new
bdd-specificationskill that enforces Living Documentation through Behaviour-Driven Development, wires it as the strict default across the feature/implementation skills, and dogfoods it by specifying and testing the toolkit's own executable features.1. New skill:
bdd-specification.featurespec per behaviour, bridged into unit tests via a strict scenario-to-test naming convention andGiven/When/Thenanchors — for target languages with or without a native BDD framework.skills/references/bdd-writing-guide.md), prompt template, README/prompt-index entries, regenerated agent adapters.2. Strict-default wiring
architecture-impactinvokes it at request/analysis time;implement-issue-workflowinvokes it at the latest at implementation time..featureplacement: co-locate for the runner, surface into arc42 docs viainclude::.3. BDD test specifications for the toolkit's features
features/for the metamodel validator, the documentation generators, the agent adapter generator, and the validation CLI.test/validate_metamodel_test.rb: existing 15 tests adapted to the bridge + 12 new validator-rule cases (27 tests).test/validate_metamodel_cli_test.rb(new): CLI behaviour via subprocess (4 tests).test/build-agent-adapters.test.mjs(new,node:test, no dependency): build,--check, staleness,adapter_expose, against an isolated workspace (4 tests)..github/workflows/validate.yml; README test docs.Verification
ruby -Itest test/validate_metamodel_test.rbruby -Itest test/validate_metamodel_cli_test.rbnode --test test/build-agent-adapters.test.mjsruby scripts/validate-metamodel.rbnode scripts/check-agent-adapters.jsWorking tree stays clean after all test runs.
Out of scope
Container-based render scripts (
build.sh,scripts/render-presentation.sh) remain untested — they need Docker/Podman; tracked as a follow-up in the README.🤖 Generated with Claude Code