Commit Graph

3 Commits

Author SHA1 Message Date
freemo 8ea00f5185 fix: restore CI quality tests to passing state (#4175)
CI / unit_tests (push) Has been cancelled
CI / benchmark-publish (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / typecheck (push) Has been cancelled
CI / security (push) Has been cancelled
CI / quality (push) Has been cancelled
CI / integration_tests (push) Has been cancelled
CI / e2e_tests (push) Has been cancelled
CI / coverage (push) Has been cancelled
CI / benchmark-regression (push) Has been cancelled
CI / build (push) Has been cancelled
CI / push-validation (push) Has been cancelled
CI / status-check (push) Has been cancelled
CI / docker (push) Has been cancelled
CI / helm (push) Has been cancelled
Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
2026-04-08 11:02:14 +00:00
hurui200320 1878998b7a refactor(testing): rename tdd_bug/tdd_bug_N tags to tdd_issue/tdd_issue_N
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 25s
CI / lint (pull_request) Successful in 3m19s
CI / typecheck (pull_request) Successful in 3m56s
CI / security (pull_request) Successful in 4m6s
CI / quality (pull_request) Successful in 4m10s
CI / integration_tests (pull_request) Successful in 7m20s
CI / unit_tests (pull_request) Successful in 7m44s
CI / docker (pull_request) Successful in 1m8s
CI / e2e_tests (pull_request) Successful in 13m1s
CI / coverage (pull_request) Successful in 12m25s
CI / status-check (pull_request) Successful in 1s
CI / build (push) Successful in 26s
CI / lint (push) Successful in 3m31s
CI / quality (push) Successful in 3m40s
CI / typecheck (push) Successful in 3m57s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 4m1s
CI / integration_tests (push) Successful in 8m55s
CI / unit_tests (push) Successful in 9m14s
CI / docker (push) Successful in 9s
CI / e2e_tests (push) Successful in 11m42s
CI / coverage (push) Successful in 11m37s
CI / status-check (push) Successful in 4s
CI / benchmark-publish (push) Successful in 29m25s
CI / benchmark-regression (pull_request) Successful in 54m8s
Rename the TDD tag system from tdd_bug/tdd_bug_<N> to tdd_issue/tdd_issue_<N>
across the entire codebase. The tdd_expected_fail tag is unchanged.

The TDD expected-failure workflow is not limited to bug fixes — it applies
equally to any issue type (features, tasks, refactors). The _bug suffix was
misleading and narrowed the perceived scope. The new _issue suffix accurately
reflects that the TDD tagging system applies to any Forgejo issue.

Changes span 92 files:
- features/environment.py: validate_tdd_tags(), should_invert_result(), and
  apply_tdd_inversion() updated — regex, variables, error messages
- robot/tdd_expected_fail_listener.py: _validate_tdd_tags(), _should_invert_result(),
  start_test(), end_test() updated consistently
- 33 Behave .feature files: all @tdd_bug/@tdd_bug_<N> tags renamed
- 29 Robot .robot files: all tdd_bug/tdd_bug_<N> tags renamed
- 3 Robot fixture files renamed (tdd_bug_alone, tdd_missing_tdd_bug,
  tdd_expected_fail_missing_bug_n) with content and references updated
- Tag validation tests and helpers updated (function names, command dispatch
  keys, output strings, fixture references)
- CONTRIBUTING.md: section renamed from 'TDD Bug Test Tags' to
  'TDD Issue Test Tags', all tag references and examples updated
- noxfile.py: comment references updated
- Step definition files, mock helpers, and benchmark files: docstring
  references updated

ISSUES CLOSED: #965
2026-03-27 05:58:35 +00:00
hurui200320 774dfedc6b test: add TDD bug-capture test for #967 — plan execute phase processing (#1050)
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>
2026-03-19 06:41:26 +00:00