Files
cleveragents-core/features/tool_wrapping_runtime.feature
hurui200320 2005b8ef82
CI / push-validation (push) Successful in 18s
CI / build (push) Successful in 19s
CI / helm (push) Successful in 24s
CI / lint (push) Successful in 29s
CI / security (push) Successful in 1m11s
CI / e2e_tests (push) Successful in 2m56s
CI / quality (push) Successful in 3m40s
CI / typecheck (push) Successful in 3m59s
CI / integration_tests (push) Successful in 4m3s
CI / unit_tests (push) Successful in 4m55s
CI / docker (push) Successful in 10s
CI / coverage (push) Successful in 10m44s
CI / status-check (push) Successful in 1s
CI / benchmark-regression (push) Has been skipped
CI / benchmark-publish (push) Successful in 1h13m28s
feat(tests): replace all @skip tags with proper @tdd_expected_fail tags or remove them across the entire codebase (#7221)
## Summary

Replaces all 234 bare `@skip` occurrences across 82 Behave feature files with the correct TDD issue-capture tagging system described in CONTRIBUTING.md § Bug Fix Workflow.

Previously, the noxfile ran Behave with `--tags=not @skip`, silently excluding all `@skip`-tagged scenarios from every CI run. These tests never ran, never inverted results via the `@tdd_expected_fail` mechanism, and never contributed to coverage — defeating the purpose of TDD issue-capture testing. Every `@skip` occurrence had a commented-out hint line immediately above it showing the intended proper tags (e.g., `# @tdd_issue @tdd_issue_4272 @tdd_expected_fail @skip`), confirming they were all intended for conversion.

## Changes

### Mechanical conversion (234 replacements across 82 files)
- Extracted the proper TDD tags from the comment hint above each `@skip` line, removed `@skip` from the tag set, and replaced the `@skip` line with those tags.
- Removed the now-redundant comment hint lines alongside each replacement.

### Bug-fixed scenarios — `@tdd_expected_fail` removed (84 scenarios)
- After conversion, ran `nox -s unit_tests` to identify which newly-enabled `@tdd_expected_fail` scenarios now **pass** (their referenced bugs have already been fixed). Removed `@tdd_expected_fail` from those 84 scenarios and their corresponding feature-level tags, leaving only the permanent `@tdd_issue @tdd_issue_<N>` regression-guard tags.
- Affected features include: `tdd_tool_runner_env_precedence`, `tdd_automation_profile_session_leak`, `tls_certificate_check`, `project_create_persist`, `resource_type_bootstrap_*`, and 18 others.

### Noxfile cleanup
- Removed all four `--tags=not @skip` arguments from `noxfile.py` (unit_tests and coverage sessions). With zero `@skip` tags remaining in the codebase, this filter was dead code and its presence would mislead future maintainers into thinking `@skip` is still a supported escape mechanism.

### Regression guard files
- Split the regression guards into two focused files:
  - `tdd_regression_guards_exec_env.feature` for bug #4281 (exec-env precedence)
  - `tdd_regression_guards_session_list.feature` for bug #4271 (session list summary)
- Each file carries only its own `@tdd_issue` tags at the feature level, avoiding cross-contamination via Behave tag inheritance. The `Background` step (`session-list-summary mock`) only appears in the session-list file where it is actually needed.

### Duplicate tag cleanup
- Removed duplicate `@tdd_issue @tdd_issue_4287` tag lines in `tdd_skill_add_regression.feature` (lines 20 and 29).

### Inline comment for retained `@tdd_expected_fail`
- Added inline comment in `ci_workflow_validation.feature:134` explaining why this specific #4227 scenario retains `@tdd_expected_fail` despite #4227 being closed (CI YAML does not encode threshold as a machine-readable value).

### Known edge cases — `@tdd_expected_fail` retained (closed issues, fix on master, scenarios still fail)
The following issues are **closed** and their fixes **are on master**, but the specific test assertions still fail because the fixes address other aspects of the bugs. The `@tdd_expected_fail` tags are functionally correct and must remain until the specific scenario assertions pass:
- `tdd_exec_env_resolution_precedence.feature` — bug #1080 (closed 2026-03-31). The precedence-level-2-vs-4 scenario still fails.
- `session_list_summary_dedup.feature` — bug #3046 (closed 2026-04-05). The dedup-consistency scenarios still fail.
- `actor_add_update_enforcement.feature` — bug #2609 (closed 2026-04-05). The enforcement scenarios still fail.
- `ci_workflow_validation.feature:134` — #4227 (closed 2026-04-08). The CI YAML threshold assertion still fails.

## Verification

- `grep -r "@skip" features/ --include="*.feature"` → **zero results** ✓
- `grep -n "tags=not @skip" noxfile.py` → **zero results** ✓
- `nox -s unit_tests` → **629 features passed, 0 failed** ✓ (up from ~545 before this PR)
- CI all green (coverage ≥ 97%) ✓
- Integration tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- E2E tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- `CHANGELOG.md` updated with entry for this change ✓
- `CONTRIBUTORS.md` — Rui Hu already listed ✓

## Issues Addressed

Closes #7025

Co-authored-by: CleverThis <hal9000@cleverthis.com>
Reviewed-on: #7221
Reviewed-by: HAL 9000 <HAL9000@cleverthis.com>
Reviewed-by: HAL9001 <hal9001@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
2026-04-13 04:56:01 +00:00

223 lines
12 KiB
Gherkin

Feature: Tool wrapping runtime
As the validation execution engine
I need to delegate execution to wrapped tools via wraps + transform
So that validations can reuse existing tool implementations
# ArgumentMapper
Scenario: ArgumentMapper with None mapping passes arguments through
Given an argument mapper with no mapping
When I apply the mapper to inputs {"path": "/src", "verbose": true}
Then the mapped arguments should be {"path": "/src", "verbose": true}
Scenario: ArgumentMapper with mapping translates argument names
Given an argument mapper with mapping {"test_directory": "source_dir", "coverage_enabled": true}
When I apply the mapper to inputs {"source_dir": "/tests"}
Then the mapped arguments should be {"test_directory": "/tests", "coverage_enabled": true}
Scenario: ArgumentMapper with literal values injects fixed values
Given an argument mapper with mapping {"mode": "strict", "count": 42}
When I apply the mapper to inputs {"extra": "ignored"}
Then the mapped arguments should be {"mode": "strict", "count": 42}
Scenario: ArgumentMapper rejects non-dict inputs
Given an argument mapper with no mapping
When I try to apply the mapper to non-dict input "not_a_dict"
Then a TypeError should be raised from the argument mapper
Scenario: ArgumentMapper rejects non-dict mapping at construction
When I try to create an argument mapper with a non-dict mapping
Then a TypeError should be raised from argument mapper construction
# ── TransformExecutor ─────────────────────────────────────────────
Scenario: TransformExecutor runs a valid transform function
Given a transform executor with code that checks returncode equals zero
When I execute the transform with tool output {"returncode": 0, "tests_run": 10}
Then the transform result should have passed true
And the transform result should have message "All tests passed"
Scenario: TransformExecutor handles failing transform output
Given a transform executor with code that checks returncode equals zero
When I execute the transform with tool output {"returncode": 1, "tests_run": 10}
Then the transform result should have passed false
Scenario: TransformExecutor raises on missing transform function
Given a transform executor with code that does not define a transform function
When I try to execute the transform with any output
Then a TransformExecutionError should be raised with message containing "callable"
Scenario: TransformExecutor raises on non-dict return value
Given a transform executor with code that returns a non-dict value
When I try to execute the transform with any output
Then a TransformExecutionError should be raised with message containing "dict"
Scenario: TransformExecutor raises on missing passed key in result
Given a transform executor with code that returns a dict without passed key
When I try to execute the transform with any output
Then a TransformExecutionError should be raised with message containing "passed"
Scenario: TransformExecutor raises on empty transform code
When I try to create a transform executor with empty code
Then a ValueError should be raised from transform construction
Scenario: TransformExecutor sandboxes dangerous operations
Given a transform executor with code that attempts to import os
When I try to execute the transform with any output
Then a TransformExecutionError should be raised from sandbox restriction
# ── WrappedToolExecutor ───────────────────────────────────────────
Scenario: Simple wrapping delegates to the wrapped tool
Given a tool registry with a tool "local/run-tests" that returns {"returncode": 0}
And a validation "local/tests-pass" that wraps "local/run-tests" with a simple transform
And a wrapped tool executor using the test registry
When I execute the wrapping validation with inputs {"path": "/src"}
Then the wrapped execution should succeed with passed true
And the wrapped tool "local/run-tests" should have been called
Scenario: Argument mapping translates arguments to the wrapped tool
Given a tool registry with a tool "local/run-tests" that returns {"returncode": 0}
And a validation "local/mapped-check" that wraps "local/run-tests" with argument mapping {"test_directory": "source_dir", "coverage_enabled": true}
And a wrapped tool executor using the test registry
When I execute the wrapping validation with inputs {"source_dir": "/tests"}
Then the wrapped tool "local/run-tests" should have received argument "test_directory" with value "/tests"
And the wrapped tool "local/run-tests" should have received argument "coverage_enabled" with value true
Scenario: Chained wrapping delegates through the chain
Given a tool registry with a tool "local/base-tool" that returns {"status": "ok"}
And a validation "local/mid-wrapper" that wraps "local/base-tool" with a passthrough transform
And a validation "local/outer-wrapper" that wraps "local/mid-wrapper" with a status transform
And a wrapped tool executor using the test registry
When I execute the outer wrapping validation with inputs {}
Then the wrapped execution should succeed with passed true
And the wrapped tool "local/base-tool" should have been called
Scenario: Missing wrapped tool raises WrappedToolNotFoundError
Given a validation "local/broken-wrap" that wraps "local/nonexistent" with a simple transform
And a wrapped tool executor with empty registry
When I try to execute the wrapping validation with inputs {}
Then a WrappedToolNotFoundError should be raised for "local/nonexistent"
Scenario: Circular wrapping chain raises WrappingCycleError
Given a validation "local/wrap-a" that wraps "local/wrap-b" with a simple transform
And a validation "local/wrap-b" that wraps "local/wrap-a" with a simple transform
And a wrapped tool executor using the test registry with cycle
When I try to execute the wrapping validation with cycle from "local/wrap-a"
Then a WrappingCycleError should be raised
Scenario: Execution context is inherited from wrapping validation
Given a tool registry with a tool "local/context-tool" that returns {"data": "value"}
And a validation "local/context-wrap" that wraps "local/context-tool" with a data transform
And a wrapped tool executor using the test registry
When I execute the wrapping validation with inputs {"key": "value"}
Then the wrapped tool should have received the validation inputs
Scenario: WrappedToolExecutor rejects non-Validation argument
Given a wrapped tool executor with empty registry
When I try to execute with a non-Validation object
Then a TypeError should be raised from the executor
Scenario: WrappedToolExecutor rejects validation without wraps
Given a validation "local/no-wrap" without wraps set
And a wrapped tool executor with empty registry
When I try to execute the non-wrapping validation
Then a ValueError should be raised indicating wraps is not set
# ── Additional coverage: WrappingDepthExceededError ──────────────
Scenario: WrappingDepthExceededError is raised when chain is too deep
Given a deep wrapping chain of 11 validations
And a wrapped tool executor using the deep chain registry
When I try to execute the deep chain wrapping validation
Then a WrappingDepthExceededError should be raised with depth 10
# ── Additional coverage: ArgumentMapper.mapping property ─────────
Scenario: ArgumentMapper exposes the raw mapping via property
Given an argument mapper with mapping {"x": "y"}
Then the mapper mapping property should return {"x": "y"}
Scenario: ArgumentMapper mapping property returns None for identity mapper
Given an argument mapper with no mapping
Then the mapper mapping property should be None
# ── Additional coverage: TransformExecutor type checks ───────────
Scenario: TransformExecutor rejects non-string transform code
When I try to create a transform executor with non-string code
Then a TypeError should be raised from transform code type check
Scenario: TransformExecutor rejects non-string tool name
When I try to create a transform executor with non-string tool name
Then a TypeError should be raised from transform tool name check
Scenario: TransformExecutor raises on code that throws during exec
Given a transform executor with code that raises an error during exec
When I try to execute the transform with any output
Then a TransformExecutionError should be raised with message containing "compile"
# ── Additional coverage: WrappedToolExecutor init checks ─────────
Scenario: WrappedToolExecutor rejects None tool_lookup
When I try to create a wrapped tool executor with None tool_lookup
Then a ValueError should be raised from executor construction for tool_lookup
Scenario: WrappedToolExecutor rejects None tool_executor
When I try to create a wrapped tool executor with None tool_executor
Then a ValueError should be raised from executor construction for tool_executor
Scenario: WrappedToolExecutor rejects non-callable tool_lookup
When I try to create a wrapped tool executor with non-callable tool_lookup
Then a TypeError should be raised from executor construction for tool_lookup
Scenario: WrappedToolExecutor rejects non-callable tool_executor
When I try to create a wrapped tool executor with non-callable tool_executor
Then a TypeError should be raised from executor construction for tool_executor
# ── Additional coverage: execute with non-dict inputs ────────────
Scenario: WrappedToolExecutor rejects non-dict inputs
Given a tool registry with a tool "local/some-tool" that returns {"ok": true}
And a validation "local/input-wrap" that wraps "local/some-tool" with a simple transform
And a wrapped tool executor using the test registry
When I try to execute wrapping validation with non-dict inputs
Then a TypeError should be raised for non-dict inputs
# ── Additional coverage: chain with None wraps at end ────────────
Scenario: Chain resolution handles validation with wraps set to None in chain
Given a tool registry with a tool "local/leaf" that returns {"value": 1}
And a validation "local/wrapper-no-inner" that wraps "local/leaf" with a passthrough transform
And a wrapped tool executor using the test registry
When I execute the wrapping validation with inputs {}
Then the wrapped execution should succeed with passed true
# ── Additional coverage: _execute_chain with None wraps target ────
Scenario: _execute_chain raises ValueError when leaf wraps is None
Given a wrapped tool executor with empty registry
When I directly call _execute_chain with a no-wraps leaf
Then a ValueError should be raised for missing wraps target
# ── ToolRunner coverage: execution environment and error paths ───
Scenario: ToolRunner resolve_execution_environment delegates to resolver
Given a ToolRunner with a mock registry
When I call resolve_execution_environment on the runner
Then the resolved environment should be local
@tdd_issue @tdd_issue_4292 @tdd_expected_fail
Scenario: ToolRunner execute returns error when env resolver raises ValueError
Given a ToolRunner with a value-error-raising env resolver
When I execute a tool through the runner with env error
Then the tool result should have success false
And the tool result error should contain "Execution environment error"
@tdd_issue @tdd_issue_4292 @tdd_expected_fail
Scenario: ToolRunner execute returns error for container environment
Given a ToolRunner with a container-returning env resolver
When I execute a tool through the runner with container env
Then the tool result should have success false
And the tool result error should contain "Container execution is not available"