test: add TDD bug-capture test for #1038 — validation add --required flag #1133
No reviewers
Labels
No labels
auto/needs-reevaluation
controller-managed
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!1133
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "tdd/m5-validation-required-flag"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Adds TDD bug-capture tests for bug #1038 — the
agents validation addcommand is missing--required/--informationalflags that the specification describes at line 22334 and in numerous walkthrough examples.Behave BDD Tests
Four Behave BDD scenarios verify the correct expected behavior:
--requiredflag is accepted — invokesvalidation add --config <file> --requiredand asserts exit code 0 and mode"required"in output.--informationalflag is accepted — same pattern with--informational.--requiredoverrides YAML config mode — creates a YAML config withmode: informational, invokes with--required, and asserts the CLI output and service layer both reflectmode: "required".--informationaloverrides YAML config mode — creates a YAML config withmode: required, invokes with--informational, and asserts the CLI output and service layer both reflectmode: "informational".Robot Framework Integration Tests
Four Robot Framework integration tests exercise the same CLI paths as the Behave scenarios via a subprocess helper script:
--requiredis recognised by the CLI.--informationalis recognised.--requiredoverrides a YAML config withmode: informational.--informationaloverrides a YAML config withmode: required.The helper script (
robot/helper_tdd_validation_required_flag.py) usestyper.testing.CliRunnerwith mocked services, following the same pattern as other TDD Robot helpers (e.g.,helper_tdd_plan_apply_yes_flag.py).Tags
All scenarios (Behave and Robot) are tagged
@tdd_bug @tdd_bug_1038 @tdd_expected_fail. Because the bug is still present, the underlying assertions fail (proving the bug exists) and the@tdd_expected_failtag inverts the result so CI passes. Once bug #1038 is fixed, the tag must be removed.Spec Contradiction Note
Rui Hu's investigation (issue #1038 comment #70755) found that the formal CLI reference (specification.md lines 9279–9290) does NOT include
--required/--informationalflags — they appear only in walkthrough examples and specification.md line 22334. Additionally, specification.md line 30761 states: "For entity registration commands (actor add,skill add,tool add,validation add, …), the YAML configuration file is the sole source of truth — the--configfile fully defines the entity and no CLI override flags are accepted." The resolution may be to add the flags to the CLI OR to clean up the spec. This is documented in both the feature file header and step definition module docstring.Key Design Decisions
modekey — matches whatValidation.from_config()actually reads (config.get("mode", "required")). The specification's Validation Configuration schema (specification.md line 34109) nests mode under avalidationkey, but the currentfrom_config()reads a flat top-levelmodekey. A comment in the step definitions documents this discrepancy.side_effect = lambda v: v— theregister_toolmock returns the actual Validation object passed by the CLI, so the CLI output reflects real CLI processing rather than hard-coded mock configuration. This prevents false positives where the mock masks an incomplete fix.f"mode: {expected_mode}"prefix match) ANDregister_toolcall args to prevent the mock from masking an incomplete fix.typer.testing.CliRunnerwith mocked_get_tool_registry_service, matching the pattern fromhelper_tdd_plan_apply_yes_flag.py. Reports real outcome (exit 0 + sentinel on bug fixed, exit 1 on bug present) and relies ontdd_expected_fail_listenerfor result inversion.--requiredoverriding informational YAML config, and--informationaloverriding required YAML config.--requiredand--informationalare passed simultaneously is deferred to the bug-fix PR for #1038.Closes #1102
2b7f239bea6fff40a2e76fff40a2e7982f46c35cReview: REQUEST CHANGES
The Behave tests are excellent — 4 well-designed scenarios covering
--required,--informational, and override-mode edge cases. The TDD tags are all correct (@tdd_bug @tdd_bug_1038 @tdd_expected_fail), step definitions are fully implemented with real assertions (not placeholders), and the feature file has a thorough 30-line header comment documenting the spec contradiction. This is high-quality test work.Blocking Issue
Missing Robot Framework integration tests. This is the only TDD bug-capture PR in the current batch that does not include a
.robotfile + helper script. Per CONTRIBUTING.md §BDD Framework: integration tests use Robot Framework underrobot/. Per §Testing Philosophy: "Every coding task must include or update tests at multiple levels: unit tests, integration tests, and performance benchmarks."All other recent TDD PRs (e.g., #1148, #1144, #1110, #1113, #1109, #1130) include both Behave and Robot tests. This PR should follow the same pattern for consistency and to satisfy the multi-level testing mandate.
Action Item
robot/tdd_validation_required_flag.robot) and corresponding helper script that exercises thevalidation add --requiredCLI path via subprocess, with matching TDD tags.Everything else about this PR is solid — once the Robot tests are added, this is ready to approve.
Self-QA status: QA process was started but interrupted before completing. The PR has not yet been reviewed by the self-QA loop. Manual review recommended.
982f46c35cd81f783917b11d41386e54c748f5c4Review Response
@freemo — thanks for the thorough review and the good catch on the missing Robot tests. You're right that this was the only TDD bug-capture PR without
.robotcoverage.Addressed: Missing Robot Framework integration tests ✅
Added
robot/tdd_validation_required_flag.robotandrobot/helper_tdd_validation_required_flag.pywith four test cases exercising the same CLI paths as the Behave scenarios:--requiredflag is acceptedcheck-required--informationalflag is acceptedcheck-informational--requiredoverrides YAML config modecheck-required-overrides-config--informationaloverrides YAML config modecheck-informational-overrides-configAll test cases are tagged
tdd_bug,tdd_bug_1038,tdd_expected_fail. The helper follows the established pattern fromhelper_tdd_plan_apply_yes_flag.py— reports the real outcome and relies ontdd_expected_fail_listenerfor result inversion.All 4 new Robot tests pass via the expected-fail mechanism (underlying assertions fail correctly, confirming the bug exists).
Quality Gates
lint: ✅ |typecheck: ✅ |unit_tests: ✅ |integration_tests: ✅ (new tests pass; pre-existing failures only) |coverage: 98%Branch rebased onto latest
origin/masterand force-pushed.Review: test: add TDD bug-capture test for #1038 — validation add --required flag
Approved. Clean TDD bug-capture test with proper conventional commit format,
@tdd_expected_failtags, and issue reference.54c748f5c455fd37c54aNew commits pushed, approval review dismissed automatically according to repository settings
45bcbc457e1507425240