Skip to content

fix: add mandatory test generation to quick-dev workflow#2212

Open
don-petry wants to merge 2 commits intobmad-code-org:mainfrom
don-petry:fix/2208-quick-dev-add-tests
Open

fix: add mandatory test generation to quick-dev workflow#2212
don-petry wants to merge 2 commits intobmad-code-org:mainfrom
don-petry:fix/2208-quick-dev-add-tests

Conversation

@don-petry
Copy link
Copy Markdown

Summary

  • Adds a mandatory Tests section to step-03-implement.md requiring test generation for every new or modified behavior, with convention discovery, test writing, and test execution substeps
  • Adds a matching Tests section to step-oneshot.md for the one-shot route
  • Updates the Acceptance auditor in step-04-review.md to flag missing test coverage as a bad_spec finding
  • Strengthens the spec-template.md task comment to mark test tasks as REQUIRED

Closes #2208

Root Cause

The quick-dev workflow had zero test-related instructions in any step. The bmad-dev-story workflow has dedicated TDD steps (5-8) with explicit test authoring and validation, but quick-dev had nothing comparable. Agents followed the workflow faithfully -- they just were never told to write tests.

Approach

Rather than adding full TDD ceremony (which would conflict with quick-dev's lightweight philosophy), the fix adds a focused Tests section between implementation and self-check that:

  1. Discovers the project's testing conventions
  2. Requires tests for happy paths, edge cases, and regressions
  3. Requires running the test suite before proceeding
  4. Only allows skipping if the project has no test infrastructure at all

The review step's acceptance auditor now also checks for test coverage, creating a safety net even if the implementation step's instructions are somehow bypassed.

Test plan

  • Run quick-dev workflow on a project with existing test infrastructure -- verify tests are generated
  • Run quick-dev one-shot route -- verify tests are generated
  • Run quick-dev on a project with no test infrastructure -- verify graceful skip with note
  • Verify step-04 review flags missing tests as bad_spec

🤖 Generated with Claude Code

The quick-dev workflow never produced tests because no step required them.
This adds a mandatory Tests section to step-03 (implement) and step-oneshot,
adds test coverage verification to the step-04 acceptance auditor, and
strengthens the spec template to require test tasks in the plan.

Closes bmad-code-org#2208

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 4, 2026 23:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the bmad-quick-dev workflow documentation to make test generation and execution mandatory for any new or modified behavior, and adds a review safeguard to flag missing test coverage.

Changes:

  • Adds a mandatory Tests section to step-03-implement.md and step-oneshot.md (discover conventions → write tests → run tests; only skip when no test infrastructure exists).
  • Updates step-04-review.md so the Acceptance auditor flags missing tests for changed behavior as a bad_spec finding (with the same “no infra” exception).
  • Strengthens spec-template.md guidance to require test tasks for every new/modified behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/bmm-skills/4-implementation/bmad-quick-dev/step-03-implement.md Introduces a mandatory test step and adds a self-check gate requiring tests before proceeding.
src/bmm-skills/4-implementation/bmad-quick-dev/step-oneshot.md Adds a mandatory test step to the one-shot route before review/present.
src/bmm-skills/4-implementation/bmad-quick-dev/step-04-review.md Extends acceptance auditing to explicitly verify test coverage for changed behavior.
src/bmm-skills/4-implementation/bmad-quick-dev/spec-template.md Makes test-related tasks explicitly required in the template guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +37 to +41
1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure from existing tests. If no existing tests exist, use the project's language-idiomatic defaults.
2. **Write tests.** Cover:
- Happy-path behavior for each new or changed feature.
- Edge cases and error scenarios from the I/O & Edge-Case Matrix (if present in the spec).
- Regressions — any behavior that could break due to the change.
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “Discover conventions” step only looks at existing tests; if a repo has test infrastructure (e.g., package.json scripts / deps, pyproject.toml, go test, etc.) but currently no test files, falling back to “language-idiomatic defaults” can pick the wrong framework/layout. Consider explicitly instructing the agent to infer the test runner and conventions from build/test scripts, dependency manifests, and config files (and only then fall back to language defaults).

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +27
**This is mandatory, not optional.** After implementation, write tests for every new or modified behavior. Follow the project's testing conventions discovered from `{project_context}`, CLAUDE.md, or the existing test suite.

1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure from existing tests. If no existing tests exist, use the project's language-idiomatic defaults.
2. **Write tests.** Cover happy-path behavior, edge cases, and regressions.
3. **Run tests.** All new and existing tests must pass. Fix failures before proceeding.

If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests.
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conventions-discovery instruction relies on “existing tests”; for repos that have a test runner configured but no test files yet, falling back to “language-idiomatic defaults” can lead to generating tests that don’t match the configured tooling. Consider adding guidance to infer the framework and naming/layout from test scripts, dependencies, and config files before defaulting.

Suggested change
**This is mandatory, not optional.** After implementation, write tests for every new or modified behavior. Follow the project's testing conventions discovered from `{project_context}`, CLAUDE.md, or the existing test suite.
1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure from existing tests. If no existing tests exist, use the project's language-idiomatic defaults.
2. **Write tests.** Cover happy-path behavior, edge cases, and regressions.
3. **Run tests.** All new and existing tests must pass. Fix failures before proceeding.
If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests.
**This is mandatory, not optional.** After implementation, write tests for every new or modified behavior. Follow the project's testing conventions discovered from `{project_context}`, CLAUDE.md, the existing test suite, and any configured test tooling.
1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure in this order:
- First, inspect existing tests.
- If existing tests are missing or insufficient, inspect the repo's configured test tooling: test scripts, dependencies/devDependencies, and test config files (for example `package.json`, `pytest.ini`, `pyproject.toml`, `jest.config.*`, `vitest.config.*`, `mocha` config, `rspec` config, `go test` conventions, etc.).
- Only if neither existing tests nor configured tooling establish conventions should you fall back to the project's language-idiomatic defaults.
2. **Write tests.** Cover happy-path behavior, edge cases, and regressions.
3. **Run tests.** All new and existing tests must pass. Fix failures before proceeding.
If the project has no test infrastructure at all (no existing tests, no test framework, no relevant test config, no test directory, and no test scripts), skip — but this is the only acceptable reason to skip tests.

Copilot uses AI. Check for mistakes.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

📝 Walkthrough

Walkthrough

These changes enforce mandatory test requirements across the Quick Dev workflow by updating four guidance documents: the spec template now requires test tasks, the implementation step includes a dedicated Tests section, the review step validates test coverage, and the oneshot workflow adds explicit testing requirements.

Changes

Cohort / File(s) Summary
Quick Dev Workflow Test Requirements
src/bmm-skills/4-implementation/bmad-quick-dev/spec-template.md, src/bmm-skills/4-implementation/bmad-quick-dev/step-03-implement.md, src/bmm-skills/4-implementation/bmad-quick-dev/step-04-review.md, src/bmm-skills/4-implementation/bmad-quick-dev/step-oneshot.md
Updated guidance across spec template and workflow steps to enforce mandatory test coverage for every newly added or modified behavior. Adds explicit "Tests" sections, requires test discovery, happy-path/edge-case/regression coverage, and treating missing test coverage as a review finding. Exceptions allowed only when projects lack test infrastructure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • #2121: Modifies step-oneshot.md with spec-trace generation and spec_file behavior updates alongside the current PR's test requirement additions
  • #2109: Updates the same bmad-quick-dev spec-template and workflow files in parallel with test requirement enforcement changes
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding mandatory test generation instructions to the quick-dev workflow across multiple steps.
Description check ✅ Passed The description is clearly related to the changeset, detailing what was added (Tests sections, updated acceptance auditor, strengthened template) and why (addressing the root cause that quick-dev had no test instructions).
Linked Issues check ✅ Passed All code changes directly address issue #2208 objectives: adding mandatory test generation instructions to quick-dev workflow (step-03-implement.md, step-oneshot.md), updating review auditor (step-04-review.md), and strengthening template guidance (spec-template.md) to ensure tests are required for new/modified behavior.
Out of Scope Changes check ✅ Passed All changes are directly within scope: four documentation files updated to add test requirements and enforcement across the quick-dev workflow steps, with no unrelated modifications present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/bmm-skills/4-implementation/bmad-quick-dev/step-oneshot.md`:
- Line 27: The guidance allows skipping tests when "no test infrastructure at
all" but doesn’t require documenting why; update the step text that currently
reads "If the project has no test infrastructure at all (no test framework, no
test directory, no test scripts), skip — but this is the only acceptable reason
to skip tests." to mandate an explicit skip reason note: require the reviewer to
add a short recorded rationale (e.g., "skip reason: no test infra – describe
what was checked") whenever skipping for this reason, and state where that note
should be placed (PR description or checklist entry) so traceability is
enforced.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: df0b46f7-706b-4465-859c-ab931cfc073b

📥 Commits

Reviewing files that changed from the base of the PR and between 783601b and 9ff99ec.

📒 Files selected for processing (4)
  • src/bmm-skills/4-implementation/bmad-quick-dev/spec-template.md
  • src/bmm-skills/4-implementation/bmad-quick-dev/step-03-implement.md
  • src/bmm-skills/4-implementation/bmad-quick-dev/step-04-review.md
  • src/bmm-skills/4-implementation/bmad-quick-dev/step-oneshot.md

2. **Write tests.** Cover happy-path behavior, edge cases, and regressions.
3. **Run tests.** All new and existing tests must pass. Fix failures before proceeding.

If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add an explicit “skip reason note” requirement when no test infrastructure exists.

Line 27 allows skipping, but it does not require recording the skip rationale. That leaves weak traceability for review and conflicts with the “graceful skip with note” objective.

Suggested wording update
-If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests.
+If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests. Record this skip reason explicitly in the final summary output.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests.
If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests. Record this skip reason explicitly in the final summary output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/bmm-skills/4-implementation/bmad-quick-dev/step-oneshot.md` at line 27,
The guidance allows skipping tests when "no test infrastructure at all" but
doesn’t require documenting why; update the step text that currently reads "If
the project has no test infrastructure at all (no test framework, no test
directory, no test scripts), skip — but this is the only acceptable reason to
skip tests." to mandate an explicit skip reason note: require the reviewer to
add a short recorded rationale (e.g., "skip reason: no test infra – describe
what was checked") whenever skipping for this reason, and state where that note
should be placed (PR description or checklist entry) so traceability is
enforced.

@alexeyv
Copy link
Copy Markdown
Collaborator

alexeyv commented Apr 5, 2026

There is any number of contexts in which quick-dev is used and there is no automated testing.

…ntation

Address PR bmad-code-org#2212 review feedback:
- Expand "Discover conventions" to check configured test tooling (test
  scripts, dependency manifests, config files) before falling back to
  language-idiomatic defaults, so repos with test infrastructure but no
  test files yet pick the correct framework.
- Require explicit skip reason in final summary output when skipping
  tests due to missing infrastructure, improving traceability.
- Apply consistently to both step-03-implement.md and step-oneshot.md.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@don-petry
Copy link
Copy Markdown
Author

All review comments (Copilot, CodeRabbit, and human feedback) predate the fix commit 33bd6b5 (2026-04-05T03:23:37Z). That commit addressed:

  • Convention discovery (Copilot): Expanded "Discover conventions" to check configured test tooling (test scripts, dependency manifests, config files) before falling back to language-idiomatic defaults.
  • Skip reason traceability (CodeRabbit): Added requirement to record skip reason explicitly in the final summary output.
  • Graceful skip for no-test-infra contexts (alexeyv): The skip clause now covers repos with no test infrastructure at all, which handles the concern about quick-dev contexts without automated testing.

No new review comments remain to address. All feedback has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Quick Dev flow doesn't add new tests at all despite CLAUDE.md/rules being extremely explicit about it

3 participants