Rewrote Behave unit tests and Robot integration tests to exercise the
CLI orchestration layer — the actual code path affected by bug #967.
Tests use CliRunner (Behave) and replicated CLI logic (Robot) to verify
that the plan execute command correctly handles plans in Strategize/QUEUED
state by running PlanExecutor.run_strategize() before transitioning.
Bug #967: The plan execute CLI command originally only called
service.execute_plan(plan_id), which is a state transition only
(Strategize/COMPLETE → Execute/QUEUED). It did not construct a
PlanExecutor or call run_strategize() / run_execute(). The fix added
phase-aware orchestration to the CLI handler. Since the fix is already
in the codebase, the @tdd_expected_fail tags have been removed and
these tests serve as permanent regression guards.
Four Behave scenarios:
- CLI execute command handles plan in Strategize/QUEUED state
- CLI execute command orchestrates full lifecycle for QUEUED plan
- Positive control: proper service-level orchestration works
- CLI auto-discovery finds plans in Strategize/QUEUED state
Four Robot integration tests mirror the Behave scenarios via a helper
script that replicates the CLI orchestration logic using real
PlanLifecycleService (in-memory) and PlanExecutor (stub actors).
Key changes from review cycle 2:
- Critical: Rewrote @tdd_expected_fail scenarios to test CLI orchestration
layer via CliRunner, not service/executor APIs (review finding #1)
- Critical: Removed @tdd_expected_fail tags since bug fix is already in
codebase — tests now serve as regression guards
- Major: Added CHANGELOG.md entry (review finding #2)
- Major: Rebased onto current master (review finding #3)
- Minor: Added state assertion (ProcessingState.QUEUED) to positive control
scenario (review finding #4)
- Minor: Narrowed exception handler from bare Exception to
(PlanError, PlanNotReadyError) (review finding #5)
- Minor: Robot suite uses Setup Test Environment With Database Isolation
(review finding #7)
- Minor: Added on_timeout=kill to all Robot Run Process calls (review #8)
- Minor: Added docstring to _fail() helper (review finding #12)
- Minor: Removed redundant Settings() instantiation (review finding #13)
- Nit: Removed @mock_only tag concern by keeping it only on the feature
level where it belongs
ISSUES CLOSED: #977
hurui200320