06130212ed
CI / build (push) Successful in 17s
CI / security (push) Successful in 1m7s
CI / lint (push) Successful in 3m18s
CI / quality (push) Successful in 3m40s
CI / typecheck (push) Successful in 3m48s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Has started running
CI / docker (push) Blocked by required conditions
CI / status-check (push) Blocked by required conditions
CI / integration_tests (push) Has started running
CI / e2e_tests (push) Has started running
CI / coverage (push) Has started running
CI / benchmark-publish (push) Has started running
## Summary E2E test for Workflow Example 5 — database schema migration with safety nets using the **review** automation profile. Exercises the full spec-aligned workflow: - **Custom resource type registration** via `resource type add --config` (postgres-db type with `transaction_rollback` sandbox strategy, `--host`, `--port`, `--database`, `--schema` CLI args with flat `type`/`default` fields per `ResourceTypeArgument` schema) - **Custom resource instantiation** — attempts `resource add` with the custom type to exercise mixed resource types, followed by `project link-resource` to link DB resource to the project - **Custom skill creation** with spec-aligned database tools: `local/query_db` (read-only), `local/execute_migration` (writes, checkpointable), `local/backfill_column` (writes, checkpointable) — registered via `skill add --config`, with namespaced tool reference names per `SkillToolRefSchema` validation - **Action creation** with `automation_profile: review`, `reusable: true`, `state: available`, spec invariants, and typed `arguments` section (`table_name`, `column_name`, `column_type`, `backfill_source` — all required per spec, using `arguments` field per `ActionConfigSchema`) - **Plan use** with `--arg` flags exercising parameterized action invocation including `backfill_source=audit_log`, plus **explicit `--automation-profile review`** flag (action-to-plan profile propagation is not yet wired in `PlanLifecycleService.use_action`) - **Phased child plan verification** via `plan tree --format json` with `decision_count >= 2` hard assertion on framework decisions plus WARN tiers for LLM decomposition quality (`< 3`, `< 5`) - **Plan phase assertion** — hard assertion that phase is populated after execute - **Checkpoint-based rollback** with hard assertions: `rc=0` on rollback success, `rc!=0` on fake checkpoint, None guard for JSON null checkpoint IDs, re-execute with Traceback/INTERNAL checks on success and explanatory comment on failure path - **Plan diff** with hard `rc=0` assertion and content-signal verification - **Migration content verification** — baseline SHA saved before apply, diff against baseline (not `HEAD~1`), WARN-level check on migration keywords (`last_login`, `schema`, `migration`, `column`, `alter`) — flexible per LLM non-determinism - **Commit count** assertion `>= 2` (fixture baseline: Create Temp Git Repo + DB fixture commit), WARN if no additional commits from lifecycle-apply - **Backfill evidence** WARN-level check in plan tree/execution output (`backfill`, `batch`, `populate`, `last_login`) with explanatory comment noting tree covers decomposition plan - **Combined AC #6 gate** — if *both* migration content *and* backfill evidence are absent, explicit WARN visibility for CI debugging - **Terminal state assertion** after `lifecycle-apply` — `plan status` call verifies phase/processing_state reflects terminal or apply-progress outcome - **Automation profile fallback verification** — if `plan use` output omits `automation_profile`, falls back to `plan status` for secondary verification (hard assertion always runs) - **Traceback and INTERNAL checks** on all CLI commands (resource add, project create, resource type add, skill add, action create, plan use, strategize, execute, plan tree, plan status, plan diff, plan rollback, re-execute after rollback, lifecycle-apply) including custom resource error paths - **Dynamic actor selection** — detects available API keys (Anthropic/OpenAI) at suite setup - **Skip If No LLM Keys** guard for graceful CI degradation - **Test-level teardown** with diagnostic logging for both plan status and plan tree on failure - **30-minute timeout** covering worst-case rollback+re-execute path - **Force Tags** for consistency with `m6_acceptance.robot` - **Timeout parameters** (`timeout=60s on_timeout=kill`) on all local `Run Process` git commands - **Sequential section numbering** (1 through 15) for readability Closes #751 ISSUES CLOSED: #751 ## Approach Follows the patterns established by `m6_acceptance.robot` and `m2_acceptance.robot`: - `WF05 Suite Setup` initialises the workspace, generates a unique run suffix, and detects available LLM API keys - `Safe Parse Json Field` from `common_e2e.resource` for JSON field extraction with None guards for JSON null values - All CLI commands use `--format json` for predictable, parseable output - `expected_rc=None` with explicit `Should Be Equal As Integers` for detailed failure messages - Hard assertions on infrastructure/framework behavior (CLI commands, phase transitions, tool registration) - WARN-level assertions on LLM-dependent output (decision decomposition, migration content, backfill evidence, commit count) — per ticket requirement "output validation is flexible" - Traceback and INTERNAL checks on all CLI commands following `m2_acceptance.robot` pattern - Baseline SHA approach for post-apply diff verification eliminates false positives from fixture commits ## Bug Fix: LifecyclePlanRepository.update() UNIQUE Constraint Violation **Root cause**: `LifecyclePlanRepository.update()` called `clear()` on child relationship collections (project_links, arguments, invariants) followed by `append()` with new items, but only flushed at the end. SQLAlchemy's default operation ordering can emit INSERTs before DELETEs within the same flush, causing `UNIQUE constraint failed: plan_arguments.plan_id, plan_arguments.name` when plans have arguments. **Fix**: Group all three `clear()` calls together and flush them before appending new rows. This ensures the DELETEs are committed before any INSERTs, preventing the UNIQUE constraint violation. **Impact**: This was a latent bug affecting ALL plans with arguments when `update()` is called. Previously undetected because existing E2E tests (M1, M2, M5, M6) create plans without `--arg` flags. ## Review Fixes (addressing medium findings from @CoreRasurae review) | # | Finding | Fix | |---|---------|-----| | **BUG-1** | No regression test for UNIQUE constraint fix | Added targeted BDD scenario in `repositories_coverage_boost.feature` — creates plan with argument `x=v1`, updates to `x=v2`, asserts no `IntegrityError` | | **TEST-1** | AC #4 weakened — fragile string counting | Replaced raw `count('"decision_id"')` with proper JSON parsing via `json.loads()`, recursive tree walking for decision counting, structural `children_key_count` and `child_link_count` verification | | **TEST-2** | AC #5 conditionally tested | Added explicit WARN log when no checkpoint_id is present ("AC #5 visibility"); fake checkpoint test now runs unconditionally (moved outside IF/ELSE) with Traceback/INTERNAL checks | | **TEST-3** | No terminal state assertion after lifecycle-apply | Added `plan status` call after apply with phase/processing_state extraction; hard assertion on terminal state or apply-phase progress | | **TEST-4** | AC #6 migration/backfill WARN-only | Added combined gate (`has_ac6_evidence`): if *both* migration and backfill evidence are absent, explicit WARN for CI visibility. WARN-only is intentional per ticket AC "output validation is flexible" | | **TEST-5** | Automation profile silently skipped | Added fallback to `plan status --format json` when `plan use` output omits `automation_profile`; hard assertion (`Should Be Equal As Strings review`) now always executes | | **TEST-8** | Missing Traceback/INTERNAL on custom resource error paths | Added Traceback/INTERNAL checks inside both `resource add` and `project link-resource` ELSE branches with `NoSuchOption` guard | ## Quality Gates - `nox -e lint` ✅ - `nox -e typecheck` ✅ (0 errors) - `nox -e unit_tests` ✅ (471 features, 12,422 scenarios, 0 failures) - `nox -e integration_tests` ✅ (1,727 tests, 0 failures) - `nox -e e2e_tests` ✅ (42 tests, 42 passed, 0 failed) - `nox -e coverage_report` ✅ (98%, meets threshold) ## Manual Verification ### Prerequisites - `OPENAI_API_KEY` or `ANTHROPIC_API_KEY` environment variable set ### Commands ```bash nox -e e2e_tests # Or run just this suite: python -m robot --outputdir build/reports/robot --include E2E robot/e2e/wf05_db_migration.robot ``` Reviewed-on: #816 Reviewed-by: Luis Mendes <luis.mendes@cleverthis.com> Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
149 lines
7.6 KiB
Gherkin
149 lines
7.6 KiB
Gherkin
@phase1 @domain @repository @coverage_boost
|
||
Feature: Repository coverage boost for actions, plans, and resources
|
||
As a developer ensuring high test coverage
|
||
I want to exercise uncovered repository branches
|
||
So that the persistence layer is thoroughly tested
|
||
|
||
Background:
|
||
Given a fresh in-memory database with full lifecycle schema
|
||
And an action repository using the session factory
|
||
And a lifecycle plan repository using the session factory
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# ActionRepository.update – non-existent action raises DatabaseError
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@action_update @error_handling
|
||
Scenario: Updating a non-existent action raises DatabaseError with "not found for update"
|
||
Given a valid action object named "local/ghost-action"
|
||
When the action is updated without being persisted first
|
||
Then a DatabaseError mentioning "not found for update" should be raised
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# ActionRepository.update – with arguments (including default_value) and invariants
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@action_update
|
||
Scenario: Updating an action replaces its child arguments and invariants
|
||
Given a valid action object named "local/updatable-with-children"
|
||
And the action has been persisted in the database
|
||
When the action arguments are replaced with new arguments including defaults
|
||
And the action invariants are set to new invariant texts
|
||
And the action is updated via the repository
|
||
Then the update should succeed without error
|
||
And retrieving the action should show the new arguments
|
||
And retrieving the action should show the new invariants
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# LifecyclePlanRepository.get_by_name
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@plan_read
|
||
Scenario: A lifecycle plan is retrieved by its namespaced name
|
||
Given a valid action object named "local/plan-action-lookup"
|
||
And the action has been persisted in the database
|
||
And a lifecycle plan domain object linked to "local/plan-action-lookup"
|
||
And the lifecycle plan has been persisted in the database
|
||
When the lifecycle plan is looked up by namespaced name
|
||
Then the returned plan should match the original plan identity
|
||
|
||
@plan_read
|
||
Scenario: Looking up a non-existent plan by name returns nothing
|
||
When a lifecycle plan is looked up by name "local/nonexistent-plan"
|
||
Then no lifecycle plan should be returned
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# LifecyclePlanRepository.update – PlanNotFoundError
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@plan_update @error_handling
|
||
Scenario: Updating a non-existent lifecycle plan raises PlanNotFoundError
|
||
Given a valid action object named "local/plan-action-missing"
|
||
And the action has been persisted in the database
|
||
And a lifecycle plan domain object linked to "local/plan-action-missing"
|
||
When the lifecycle plan is updated without being persisted first
|
||
Then a PlanNotFoundError should be raised for the repository boost
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# LifecyclePlanRepository.update – with project_links, arguments, invariants
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@plan_update
|
||
Scenario: Updating a plan replaces project links, arguments, and invariants
|
||
Given a valid action object named "local/plan-action-update"
|
||
And the action has been persisted in the database
|
||
And a lifecycle plan domain object linked to "local/plan-action-update"
|
||
And the lifecycle plan has been persisted in the database
|
||
When the plan is updated with project links, arguments, and invariants
|
||
Then the plan update should succeed without error
|
||
And retrieving the plan should show the new project links
|
||
And retrieving the plan should show the new plan arguments
|
||
And retrieving the plan should show the new plan invariants
|
||
|
||
@plan_update
|
||
Scenario: Updating a plan argument with the same name does not hit UNIQUE constraints
|
||
Given a valid action object named "local/plan-action-arg-regression"
|
||
And the action has been persisted in the database
|
||
And a lifecycle plan domain object linked to "local/plan-action-arg-regression"
|
||
And the lifecycle plan has argument "x" set to "v1"
|
||
And the lifecycle plan has been persisted in the database
|
||
When the lifecycle plan argument "x" is updated to "v2"
|
||
Then the plan update should succeed without error
|
||
And retrieving the plan should show argument "x" value "v2"
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# LifecyclePlanRepository.list_plans filtered by phase
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@plan_list
|
||
Scenario: Listing plans filtered by phase returns only matching plans
|
||
Given a valid action object named "local/plan-action-phase"
|
||
And the action has been persisted in the database
|
||
And lifecycle plans in different phases linked to "local/plan-action-phase"
|
||
When plans are listed filtered by phase "strategize"
|
||
Then only the strategize-phase plans should be returned
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# ResourceTypeRepository – create, get, list_types
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@resource_type
|
||
Scenario: A resource type is created and retrieved by name
|
||
Given a resource type repository using the session factory
|
||
And a valid resource type domain object named "test/my-git-checkout"
|
||
When the resource type is created through the repository
|
||
Then the resource type should be retrievable by name "test/my-git-checkout"
|
||
|
||
@resource_type @error_handling
|
||
Scenario: Creating a duplicate resource type raises DuplicateResourceTypeError
|
||
Given a resource type repository using the session factory
|
||
And a valid resource type domain object named "test/dup-type"
|
||
And the resource type has been persisted in the database
|
||
When the same resource type is created again
|
||
Then a DuplicateResourceTypeError should be raised
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# ResourceRepository – create, get, get_by_name, list_resources by type
|
||
# ---------------------------------------------------------------------------
|
||
|
||
@resource
|
||
Scenario: A resource is created and retrieved by ID and name
|
||
Given a resource type repository using the session factory
|
||
And a resource repository using the session factory
|
||
And a valid resource type domain object named "test/res-type-a"
|
||
And the resource type has been persisted in the database
|
||
And a valid resource domain object of type "test/res-type-a"
|
||
When the resource is created through the repository
|
||
Then the resource should be retrievable by its ID
|
||
And the resource should be retrievable by its namespaced name
|
||
|
||
@resource
|
||
Scenario: Resources are listed by type name
|
||
Given a resource type repository using the session factory
|
||
And a resource repository using the session factory
|
||
And a valid resource type domain object named "test/list-type"
|
||
And the resource type has been persisted in the database
|
||
And multiple resources of type "test/list-type" have been created
|
||
When resources are listed by type "test/list-type"
|
||
Then all resources of that type should be returned
|