hurui200320
774dfedc6b
CI / lint (push) Successful in 16s
CI / build (push) Successful in 19s
CI / quality (push) Successful in 31s
CI / typecheck (push) Successful in 47s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 1m16s
CI / unit_tests (push) Successful in 3m18s
CI / docker (push) Successful in 9s
CI / e2e_tests (push) Successful in 3m49s
CI / integration_tests (push) Successful in 3m55s
CI / coverage (push) Successful in 6m53s
CI / benchmark-publish (push) Has been cancelled
## Summary This PR adds TDD bug-capture tests for bug #967 — `plan execute` only transitions state without running strategize or execute phase processing. ### Motivation Bug #967 describes that the `plan execute` CLI command originally only called `service.execute_plan(plan_id)`, which is a state transition only (Strategize/COMPLETE → Execute/QUEUED). When a plan was in Strategize/QUEUED state (immediately after `plan use`), the command failed because `execute_plan()` requires Strategize/COMPLETE. The CLI should detect the plan's current phase and run `PlanExecutor.run_strategize()` before transitioning. Per the project's TDD Bug Fix Workflow (`CONTRIBUTING.md`), the first step in fixing any bug is to write a test that captures the buggy behavior. Since the fix for #967 is already present in the codebase (the CLI handler already orchestrates properly), the `@tdd_expected_fail` tags have been removed and these tests serve as **permanent regression guards** ensuring the fix is never reverted. ### Design Approach **All `@tdd_expected_fail` scenarios were rewritten to exercise the CLI orchestration layer** — the actual code path affected by bug #967. This satisfies AC4: "The test is specific enough that it will pass normally (without the tag) only when the bug is genuinely fixed." - **Scenarios 1, 2, 4** (previously `@tdd_expected_fail`): Use Typer's `CliRunner` with mocked services to invoke the `plan execute` CLI command handler directly. This tests the orchestration logic in `plan.py` — the exact code that was buggy. - **Scenario 3** (positive control): Uses real `PlanLifecycleService` (in-memory) and `PlanExecutor` (stub actors) to demonstrate that proper service-level orchestration works. - **Robot tests**: Replicate the CLI orchestration logic using real services to verify at the integration level. ### Changes #### Behave Unit Tests - `features/tdd_plan_execute_phase_processing.feature` — 4 scenarios - `features/steps/tdd_plan_execute_phase_processing_steps.py` — Step definitions using CliRunner and mocked services **Scenarios:** 1. **CLI execute command handles plan in Strategize/QUEUED state** — Invokes `plan execute` via CliRunner on a QUEUED plan. Verifies the CLI succeeds and the plan reaches Execute phase. 2. **CLI execute command orchestrates full lifecycle for QUEUED plan** — Verifies `run_strategize()` and `run_execute()` are both called by the CLI handler. 3. **Positive control — proper orchestration transitions QUEUED plan to Execute** — Demonstrates that `run_strategize()` → `execute_plan()` works correctly at the service level. Always passes. 4. **CLI auto-discovery finds plans in Strategize/QUEUED state** — Invokes `plan execute` with no plan_id. Verifies the auto-discovery filter includes QUEUED plans. #### Robot Integration Tests - `robot/tdd_plan_execute_phase_processing.robot` — 4 test cases matching the Behave scenarios - `robot/helper_tdd_plan_execute_phase_processing.py` — Helper script replicating CLI orchestration logic #### CHANGELOG - `CHANGELOG.md` — Added entry under "## Unreleased" describing the new tests. ### Quality Gates - `nox -e lint`: ✅ passed - `nox -e typecheck`: ✅ passed (0 errors) - `nox -e unit_tests`: ✅ passed (391 features, 11177 scenarios, 0 failures) - `nox -e integration_tests`: ✅ passed (1572 tests, 0 failures) - `nox -e e2e_tests`: ✅ passed (16 tests, 0 failures) - `nox -e coverage_report`: ✅ 97% coverage ### Review Cycle 2 Fixes - **Critical #1**: Rewrote `@tdd_expected_fail` scenarios to exercise the CLI orchestration layer via CliRunner instead of testing service/executor APIs that are correct by design. Removed `@tdd_expected_fail` tags since the bug fix is already in the codebase. - **Major #2**: Added CHANGELOG entry. - **Major #3**: Rebased onto current `master`. - **Minor #4**: Added `ProcessingState.QUEUED` assertion to positive control scenario. - **Minor #5**: Narrowed exception handler from bare `except Exception` to `except (PlanError, PlanNotReadyError)`. - **Minor #7**: Changed Robot suite to use `Setup Test Environment With Database Isolation`. - **Minor #8**: Added `on_timeout=kill` to all Robot `Run Process` calls. - **Minor #12**: Added docstring to `_fail()` helper. - **Minor #13**: Removed redundant `Settings()` instantiation. ### Known Limitations - The `@tdd_expected_fail` tags were removed because the bug fix for #967 is already in the codebase. If this PR is merged before the #967 fix PR, the tags would need to be re-added. However, the CHANGELOG and existing CLI code confirm the fix is already on `master`. Closes #977 Reviewed-on: #1050 Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
61 lines
3.3 KiB
Plaintext
61 lines
3.3 KiB
Plaintext
*** Settings ***
|
|
Documentation TDD Bug #967 — plan execute only transitions state, does not run
|
|
... strategize or execute phase processing. Integration smoke tests
|
|
... exercising the CLI orchestration layer via real
|
|
... PlanLifecycleService (in-memory) and PlanExecutor (stub actors)
|
|
... to verify that a plan in Strategize/QUEUED state can be taken
|
|
... through to Execute phase. The bug fix is already in the
|
|
... codebase; these tests serve as permanent regression guards.
|
|
Resource ${CURDIR}/common.resource
|
|
Suite Setup Setup Test Environment With Database Isolation
|
|
Suite Teardown Cleanup Test Environment
|
|
|
|
*** Variables ***
|
|
${HELPER} ${CURDIR}/helper_tdd_plan_execute_phase_processing.py
|
|
|
|
*** Test Cases ***
|
|
TDD CLI Execute Plan From Strategize QUEUED State
|
|
[Documentation] Verify that the CLI execute_plan handler correctly
|
|
... orchestrates run_strategize before transitioning when
|
|
... a plan is in Strategize/QUEUED state.
|
|
... Bug #967: originally the CLI only called service.execute_plan().
|
|
[Tags] tdd_bug tdd_bug_967
|
|
${result}= Run Process ${PYTHON} ${HELPER} cli-execute-from-queued cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
|
Log ${result.stdout}
|
|
Log ${result.stderr}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} tdd-cli-execute-from-queued-ok
|
|
|
|
TDD CLI Execute Orchestrates Full Lifecycle
|
|
[Documentation] Verify that the CLI execute_plan handler calls both
|
|
... run_strategize and run_execute for a QUEUED plan.
|
|
... Bug #967: the CLI skipped PlanExecutor orchestration.
|
|
[Tags] tdd_bug tdd_bug_967
|
|
${result}= Run Process ${PYTHON} ${HELPER} cli-full-orchestration cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
|
Log ${result.stdout}
|
|
Log ${result.stderr}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} tdd-cli-full-orchestration-ok
|
|
|
|
TDD Positive Control Proper Orchestration
|
|
[Documentation] Positive control: verify that the proper service-level
|
|
... orchestration path (run_strategize then execute_plan)
|
|
... works correctly.
|
|
[Tags] tdd_bug tdd_bug_967
|
|
${result}= Run Process ${PYTHON} ${HELPER} proper-orchestration cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
|
Log ${result.stdout}
|
|
Log ${result.stderr}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} tdd-proper-orchestration-ok
|
|
|
|
TDD CLI Auto Discovery Includes QUEUED Plans
|
|
[Documentation] Verify that the CLI auto-discovery filter includes plans
|
|
... in Strategize/QUEUED state, not just Strategize/COMPLETE.
|
|
... Bug #967: the original filter excluded QUEUED plans.
|
|
[Tags] tdd_bug tdd_bug_967
|
|
${result}= Run Process ${PYTHON} ${HELPER} cli-auto-discovery cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
|
Log ${result.stdout}
|
|
Log ${result.stderr}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} tdd-cli-auto-discovery-ok
|