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
## 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>
223 lines
12 KiB
Gherkin
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"
|