|
| 1 | +# BDD Testing — Mandatory Guide |
| 2 | + |
| 3 | +> **Audience:** every contributor — human and AI assistant. |
| 4 | +> **Status:** binding. No exceptions without a written waiver in the PR body |
| 5 | +> approved by a maintainer. |
| 6 | +
|
| 7 | +--- |
| 8 | + |
| 9 | +## TL;DR |
| 10 | + |
| 11 | +1. **Every new feature, behaviour change, or bug fix MUST ship with at least one |
| 12 | + BDD scenario** (Gherkin `.feature` file + step definitions). |
| 13 | +2. **PRs cannot be merged until the `bdd-tests` CI job is green.** The check is |
| 14 | + a required status on `main`. |
| 15 | +3. Feature files live under `test/bdd/features/`. Step definitions live under |
| 16 | + `test/bdd/steps/` (or in-package under `internal/<pkg>/` when the function |
| 17 | + under test is unexported — see `internal/router/copy_options_bdd_test.go`). |
| 18 | +4. Run locally: `go test -v -count=1 -run BDD ./test/bdd/... ./internal/router/...` |
| 19 | +5. CI publishes every run's BDD artifacts (Gherkin output, JUnit, coverage) as |
| 20 | + GitHub Actions workflow artifacts on the `bdd-tests` job. |
| 21 | + |
| 22 | +--- |
| 23 | + |
| 24 | +## The rule |
| 25 | + |
| 26 | +**No BDD, no merge.** |
| 27 | + |
| 28 | +A PR is mergeable only if **all** of the following are true: |
| 29 | + |
| 30 | +| # | Requirement | Enforcement | |
| 31 | +|---|---|---| |
| 32 | +| 1 | At least one new `.feature` scenario covers the change | Reviewer + checklist | |
| 33 | +| 2 | New scenarios cover happy path AND at least one invalid / edge case | Reviewer + checklist | |
| 34 | +| 3 | All existing scenarios still pass | CI: `bdd-tests` job | |
| 35 | +| 4 | All new scenarios pass | CI: `bdd-tests` job | |
| 36 | +| 5 | New step definitions are reviewed (no `Skip`, no `// TODO`) | Reviewer | |
| 37 | +| 6 | CHANGELOG entry references the new scenario in `Tests` or relevant section | `docs-quality-reviewer` | |
| 38 | + |
| 39 | +### Exceptions (the only ones) |
| 40 | + |
| 41 | +You may skip BDD only for these — and you must call it out explicitly in the |
| 42 | +PR description under `### BDD waiver`: |
| 43 | + |
| 44 | +- Pure formatting (`gofmt`, comment typos, file rename with no logic change). |
| 45 | +- Pure dependency bump with no source change (e.g. `go.mod` only). |
| 46 | +- CI / workflow changes that have no runtime effect on the binary. |
| 47 | + |
| 48 | +If you are unsure, **write the test**. The cost of writing a Gherkin scenario |
| 49 | +is lower than the cost of a regression that ships. |
| 50 | + |
| 51 | +--- |
| 52 | + |
| 53 | +## Why BDD here |
| 54 | + |
| 55 | +CQLAI is a CLI tool used interactively against live Cassandra clusters. A unit |
| 56 | +test that says "function X returns Y" does not prove "user can run COPY FROM |
| 57 | +PARQUET against a list<text> column without breaking." BDD scenarios encode |
| 58 | +the second statement directly. |
| 59 | + |
| 60 | +Concrete examples of what BDD has caught in this repo: |
| 61 | + |
| 62 | +- `internal/router/copy_from_parquet_collection_test.go` — list vs set literal |
| 63 | + selection from destination schema (issue #81). |
| 64 | +- `test/bdd/features/copy-options.feature` — silent option drops in COPY |
| 65 | + WITH-clause parser. |
| 66 | +- `test/bdd/features/command-validation.feature` — destructive commands not |
| 67 | + guarded by confirmation. |
| 68 | + |
| 69 | +--- |
| 70 | + |
| 71 | +## Where BDD code lives |
| 72 | + |
| 73 | +``` |
| 74 | +test/ |
| 75 | +├── bdd/ |
| 76 | +│ ├── features/ # Gherkin .feature files (one per behaviour area) |
| 77 | +│ │ ├── ai-command-parser.feature |
| 78 | +│ │ ├── command-validation.feature |
| 79 | +│ │ ├── copy-options.feature |
| 80 | +│ │ ├── cql-splitter.feature |
| 81 | +│ │ ├── save-command.feature |
| 82 | +│ │ └── ssl-flags.feature |
| 83 | +│ └── steps/ # Step definitions (Go, godog) |
| 84 | +│ ├── ai_steps_test.go |
| 85 | +│ ├── save_steps_test.go |
| 86 | +│ ├── splitter_steps_test.go |
| 87 | +│ ├── ssl_flags_steps_test.go |
| 88 | +│ └── validation_steps_test.go |
| 89 | +└── integration/ # +build integration — godog steps that need a live Cassandra |
| 90 | +``` |
| 91 | + |
| 92 | +In-package BDD (for unexported functions): co-locate the step definitions |
| 93 | +with the code under test: |
| 94 | + |
| 95 | +``` |
| 96 | +internal/router/ |
| 97 | +├── copy_options_bdd_test.go # godog suite calling parseCopyOptions directly |
| 98 | +``` |
| 99 | + |
| 100 | +Use `_bdd_test.go` suffix so it is picked up by `go test` but kept distinct |
| 101 | +from unit tests. |
| 102 | + |
| 103 | +--- |
| 104 | + |
| 105 | +## Writing a scenario |
| 106 | + |
| 107 | +### 1. Pick the right file |
| 108 | + |
| 109 | +- New behaviour in an existing area → append to the existing `.feature`. |
| 110 | +- New area → create `test/bdd/features/<kebab-name>.feature`. |
| 111 | + |
| 112 | +### 2. Write the Gherkin first, before the code |
| 113 | + |
| 114 | +This is non-negotiable. The scenario IS the spec. Acceptance criteria from the |
| 115 | +issue map 1:1 to scenarios. |
| 116 | + |
| 117 | +Skeleton: |
| 118 | + |
| 119 | +```gherkin |
| 120 | +Feature: <one-line capability statement> |
| 121 | + As a <user role> |
| 122 | + I want <observable behaviour> |
| 123 | + So that <business reason> |
| 124 | +
|
| 125 | + Scenario: <one short, declarative sentence> |
| 126 | + Given <starting state> |
| 127 | + When <single action> |
| 128 | + Then <single observable outcome> |
| 129 | + And <optional further outcome> |
| 130 | +``` |
| 131 | + |
| 132 | +Rules (enforced by `engineering-agents:bdd-guidelines`): |
| 133 | + |
| 134 | +| Rule | Why | |
| 135 | +|---|---| |
| 136 | +| One `When` per scenario | A scenario describes one action | |
| 137 | +| `Then` asserts observable behaviour, not implementation | Reviewers should not need to read source to grade the assertion | |
| 138 | +| No persisted state between scenarios | Each scenario runs in isolation | |
| 139 | +| Scenario titles use business language, not function names | Findable in failure output | |
| 140 | +| Cover happy path AND at least one negative / edge case | Bugs hide on the edges | |
| 141 | +| Use `Scenario Outline` + `Examples` for parameterised cases | DRY | |
| 142 | +| No `Background` larger than 3 steps | Keeps scenarios self-contained | |
| 143 | + |
| 144 | +### 3. Implement step definitions |
| 145 | + |
| 146 | +```go |
| 147 | +// test/bdd/steps/<area>_steps_test.go |
| 148 | +package steps_test |
| 149 | + |
| 150 | +import ( |
| 151 | + "context" |
| 152 | + "testing" |
| 153 | + |
| 154 | + "github.com/cucumber/godog" |
| 155 | +) |
| 156 | + |
| 157 | +type myWorld struct { |
| 158 | + // per-scenario state — never package-level globals |
| 159 | +} |
| 160 | + |
| 161 | +func (w *myWorld) iDoX() error { /* ... */ } |
| 162 | + |
| 163 | +func (w *myWorld) yIsObserved() error { /* ... */ } |
| 164 | + |
| 165 | +func InitializeMyScenario(ctx *godog.ScenarioContext) { |
| 166 | + w := &myWorld{} |
| 167 | + ctx.Before(func(_ context.Context, _ *godog.Scenario) (context.Context, error) { |
| 168 | + *w = myWorld{} // reset |
| 169 | + return nil, nil |
| 170 | + }) |
| 171 | + ctx.Step(`^I do X$`, w.iDoX) |
| 172 | + ctx.Step(`^Y is observed$`, w.yIsObserved) |
| 173 | +} |
| 174 | + |
| 175 | +func TestBDDMyArea(t *testing.T) { |
| 176 | + suite := godog.TestSuite{ |
| 177 | + ScenarioInitializer: InitializeMyScenario, |
| 178 | + Options: &godog.Options{ |
| 179 | + Format: "pretty", |
| 180 | + Paths: []string{"../features/<file>.feature"}, |
| 181 | + TestingT: t, |
| 182 | + }, |
| 183 | + } |
| 184 | + if suite.Run() != 0 { |
| 185 | + t.Fatal("BDD suite failed") |
| 186 | + } |
| 187 | +} |
| 188 | +``` |
| 189 | + |
| 190 | +Step rules: |
| 191 | + |
| 192 | +- One `world` struct per suite. Reset in `Before`. No globals. |
| 193 | +- Steps return `error`, not `t.Fatal` — godog formats errors nicely. |
| 194 | +- Use real production code paths. **Never** mock the function you are |
| 195 | + validating. Mock only the boundary (network, filesystem) if needed. |
| 196 | +- Regex captures: prefer named patterns in plain English. Avoid clever |
| 197 | + optional groups — duplicate the step instead. |
| 198 | + |
| 199 | +### 4. Run locally |
| 200 | + |
| 201 | +Pure (no Cassandra needed): |
| 202 | + |
| 203 | +```bash |
| 204 | +go test -v -count=1 -run BDD ./test/bdd/... ./internal/router/... |
| 205 | +``` |
| 206 | + |
| 207 | +Single feature: |
| 208 | + |
| 209 | +```bash |
| 210 | +go test -v -count=1 -run TestBDDCopyOptions ./internal/router/ |
| 211 | +``` |
| 212 | + |
| 213 | +Integration (needs Cassandra on `127.0.0.1:9042`, e.g. |
| 214 | +`docker run -p 9042:9042 cassandra:5.0`): |
| 215 | + |
| 216 | +```bash |
| 217 | +go test -v -tags integration -timeout 10m -count=1 ./test/integration/... |
| 218 | +``` |
| 219 | + |
| 220 | +### 5. Verify CI |
| 221 | + |
| 222 | +The `bdd-tests` job in `.github/workflows/ci.yml` runs on every push. It must |
| 223 | +be green before merge. CI also runs the integration suite against the matrix |
| 224 | +of Cassandra versions — your scenario must pass against **all** matrix |
| 225 | +versions if it touches CQL behaviour. |
| 226 | + |
| 227 | +--- |
| 228 | + |
| 229 | +## Accessing previous test runs |
| 230 | + |
| 231 | +CI publishes BDD + integration artifacts (Gherkin reports, JUnit XML, raw `go |
| 232 | +test` output, coverage profiles) as GitHub Actions workflow artifacts on the |
| 233 | +`bdd-tests` job. Use these when triaging flakes, comparing runs, or auditing |
| 234 | +what scenarios actually ran on a given commit. |
| 235 | + |
| 236 | +Fetch via `gh`: |
| 237 | + |
| 238 | +```bash |
| 239 | +# List runs for a commit |
| 240 | +gh run list --repo axonops/cqlai --commit <sha> |
| 241 | + |
| 242 | +# Download all artifacts for a run |
| 243 | +gh run download <run-id> --repo axonops/cqlai --dir ./ci-artifacts/ |
| 244 | + |
| 245 | +# Download artifacts for a PR's head commit |
| 246 | +SHA=$(gh pr view 82 --repo axonops/cqlai --json headRefOid -q .headRefOid) |
| 247 | +RUN=$(gh run list --repo axonops/cqlai --commit "$SHA" --json databaseId -q '.[0].databaseId') |
| 248 | +gh run download "$RUN" --repo axonops/cqlai --dir ./ci-artifacts/ |
| 249 | +``` |
| 250 | + |
| 251 | +Retention is governed by the repo's GitHub Actions artifact retention policy. |
| 252 | + |
| 253 | +### Pretty HTML report |
| 254 | + |
| 255 | +The Cucumber JSON output is consumable by any Cucumber HTML reporter. After |
| 256 | +downloading the artifact: |
| 257 | + |
| 258 | +```bash |
| 259 | +npx cucumber-html-reporter \ |
| 260 | + --cucumber-json ./ci-artifacts/bdd-tests/godog-cucumber.json \ |
| 261 | + --output report.html |
| 262 | +open report.html |
| 263 | +``` |
| 264 | + |
| 265 | +--- |
| 266 | + |
| 267 | +## Pre-merge checklist (copy into your PR description) |
| 268 | + |
| 269 | +```markdown |
| 270 | +### BDD compliance |
| 271 | + |
| 272 | +- [ ] Added or updated `.feature` file(s): <path> |
| 273 | +- [ ] Scenarios cover happy path AND at least one invalid / edge case |
| 274 | +- [ ] Step definitions added or updated: <path> |
| 275 | +- [ ] `go test -v -count=1 -run BDD ./test/bdd/... ./internal/router/...` passes locally |
| 276 | +- [ ] CI `bdd-tests` job is green |
| 277 | +- [ ] CHANGELOG entry mentions the new scenario(s) |
| 278 | +- [ ] (If CQL-affecting) integration suite green on every Cassandra matrix version |
| 279 | +``` |
| 280 | + |
| 281 | +If you set `### BDD waiver`, justify it in one paragraph and tag a maintainer. |
| 282 | + |
| 283 | +--- |
| 284 | + |
| 285 | +## Troubleshooting |
| 286 | + |
| 287 | +| Symptom | Cause | Fix | |
| 288 | +|---|---|---| |
| 289 | +| `pending step` in godog output | Regex in `ctx.Step(...)` does not match the Gherkin sentence | Print godog's suggested snippet, paste the new step | |
| 290 | +| Scenario passes locally, fails in CI | Hidden state leaks between scenarios | Reset `world` in `Before`. Never use package-level vars | |
| 291 | +| `bdd-tests` job times out | Suite calls a real network in a step | Mock the network boundary only. Production code stays untouched | |
| 292 | +| Cassandra integration scenario passes on 5.0, fails on 2.1 | CQL grammar / system table differs | Gate the scenario with a tag and version-skip in step setup | |
| 293 | +| Want to add a step but it duplicates an existing one | You probably don't | Reuse the existing step. Rename for clarity if needed | |
| 294 | + |
| 295 | +--- |
| 296 | + |
| 297 | +## For AI assistants |
| 298 | + |
| 299 | +If you are an AI (Claude, Copilot, Cursor, etc.) implementing a feature in |
| 300 | +this repo: |
| 301 | + |
| 302 | +1. **Before writing implementation code, write the Gherkin.** The user can |
| 303 | + correct the spec faster than the implementation. |
| 304 | +2. Always run `engineering-agents:bdd-guidelines` skill in parallel with the |
| 305 | + stack-specific skill (e.g. `python-bootstrap:pytest-bdd-tests`, |
| 306 | + `go-bootstrap:godog-bdd-tests`). |
| 307 | +3. **Never** mark a task complete with `Skip` or `t.Fatal("TODO")` in a step. |
| 308 | + The CI gate will reject it. |
| 309 | +4. If you cannot satisfy a scenario, stop and ask. Do not delete the scenario |
| 310 | + to make CI green. |
| 311 | +5. When triaging a CI failure, download the workflow artifact first |
| 312 | + (`gh run download ...`), do not guess from the GitHub Actions log |
| 313 | + summary. |
| 314 | + |
| 315 | +--- |
| 316 | + |
| 317 | +## References |
| 318 | + |
| 319 | +- `engineering-agents:bdd-guidelines` skill — generic BDD rules (load first). |
| 320 | +- `go-bootstrap:godog-bdd-tests` skill — Go/godog specifics. |
| 321 | +- [Cucumber Gherkin reference](https://cucumber.io/docs/gherkin/reference/). |
| 322 | +- [godog README](https://github.com/cucumber/godog). |
| 323 | +- CHANGELOG.md — every PR appends a `Tests` entry referencing the new scenario. |
0 commit comments