features/steps/tdd_e2e_mock_only_coverage_steps.py

@@ -0,0 +1,257 @@
+"""Step definitions for TDD Bug #658 — E2E mock-only coverage.
+
+These steps use AST analysis to inspect the M1-M6 E2E verification helper
+files (``robot/helper_m*_e2e_verification.py``) and classify each public
+test function by its mock and CLI invocation strategy.
+
+A function is classified as "CLI-facing" if it uses ``CliRunner`` (from
+``typer.testing``) to invoke CLI commands.  The test asserts that at least
+one such CLI-facing function invokes the real ``agents`` CLI via
+``subprocess.run`` (or equivalent) without mocking the service layer.
+
+With ``@tdd_expected_fail`` the assertions are inverted: the test passes
+CI while the bug is present (all CLI-facing tests use mocks) and fails
+once the bug is fixed (signalling that the tag should be removed).
+
+Root cause
+~~~~~~~~~~
+All 21 CLI-facing tests in the M1-M6 E2E helpers use
+``unittest.mock.patch`` to replace service factory functions with
+``MagicMock`` objects and invoke the CLI via Typer's in-process
+``CliRunner``.  Zero tests invoke the actual ``agents`` CLI binary via
+``subprocess.run``.  This prevents the E2E suites from detecting DI
+wiring bugs, database issues, or process-level failures.
+"""
+
+from __future__ import annotations
+
+import ast
+from dataclasses import dataclass, field
+from pathlib import Path
+from typing import Any
+
+from behave import given, then, when
+
+_REPO_ROOT = Path(__file__).resolve().parents[2]
+
+_HELPER_GLOBS = [
+    "robot/helper_m1_e2e_verification.py",
+    "robot/helper_m2_e2e_verification.py",
+    "robot/helper_m3_e2e_verification.py",
+    "robot/helper_m4_e2e_verification.py",
+    "robot/helper_m5_e2e_verification.py",
+    "robot/helper_m6_e2e_verification.py",
+]
+
+# Patch targets that indicate service-layer mocking.
+_SERVICE_MOCK_INDICATORS: frozenset[str] = frozenset(
+    {
+        "_get_lifecycle_service",
+        "_get_registry_service",
+        "_get_namespaced_project_repo",
+        "_get_resource_link_repo",
+        "_get_resource_registry_service",
+        "_get_services",
+        "_get_service",
+        "get_container",
+        "CorrectionService",
+    }
+)
+
+
+@dataclass
+class FunctionAnalysis:
+    """Analysis result for a single function in an E2E helper file."""
+
+    name: str
+    suite: str
+    uses_cli_runner: bool = False
+    uses_mock_patch: bool = False
+    uses_subprocess_cli: bool = False
+    mock_targets: list[str] = field(default_factory=list)
+
+
+def _analyze_helper(helper_path: Path) -> list[FunctionAnalysis]:
+    """Parse a helper file and classify each top-level function."""
+    source = helper_path.read_text(encoding="utf-8")
+    tree = ast.parse(source, filename=str(helper_path))
+    suite = helper_path.stem  # e.g. "helper_m1_e2e_verification"
+
+    results: list[FunctionAnalysis] = []
+
+    for node in ast.iter_child_nodes(tree):
+        if not isinstance(node, ast.FunctionDef):
+            continue
+        # Skip private/dunder helpers and the dispatcher
+        if node.name.startswith("_"):
+            continue
+
+        fa = FunctionAnalysis(name=node.name, suite=suite)
+
+        for child in ast.walk(node):
+            if isinstance(child, ast.Attribute):
+                attr_name = child.attr
+                # Detect CliRunner.invoke or runner.invoke
+                if attr_name == "invoke" and _is_cli_runner_invoke(child):
+                    fa.uses_cli_runner = True
+                # Detect subprocess.run / subprocess.Popen for CLI
+                if attr_name in ("run", "Popen") and _is_subprocess_call(child):
+                    fa.uses_subprocess_cli = True
+
+            if isinstance(child, ast.Call) and _is_patch_call(child):
+                fa.uses_mock_patch = True
+                target = _extract_patch_target(child)
+                if target:
+                    fa.mock_targets.append(target)
+
+            if isinstance(child, ast.Constant) and isinstance(child.value, str):
+                # Detect patch target strings that reference service factories
+                for indicator in _SERVICE_MOCK_INDICATORS:
+                    if indicator in child.value:
+                        fa.uses_mock_patch = True
+
+        results.append(fa)
+
+    return results
+
+
+def _is_cli_runner_invoke(node: ast.Attribute) -> bool:
+    """Check if an attribute access looks like ``runner.invoke(...)``."""
+    return isinstance(node.value, ast.Name) and node.value.id in (
+        "runner",
+        "cli_runner",
+    )
+
+
+def _is_subprocess_call(node: ast.Attribute) -> bool:
+    """Check if an attribute access looks like ``subprocess.run(...)``."""
+    return isinstance(node.value, ast.Name) and node.value.id == "subprocess"
+
+
+def _is_patch_call(node: ast.Call) -> bool:
+    """Check if a Call node is ``patch(...)`` or ``mock.patch(...)``."""
+    func = node.func
+    if isinstance(func, ast.Attribute) and func.attr == "patch":
+        return True
+    return isinstance(func, ast.Name) and func.id == "patch"
+
+
+def _extract_patch_target(node: ast.Call) -> str | None:
+    """Extract the first string argument from a ``patch(...)`` call."""
+    if node.args and isinstance(node.args[0], ast.Constant):
+        val = node.args[0].value
+        if isinstance(val, str):
+            return val
+    return None
+
+
+# ---------------------------------------------------------------------------
+# Given steps
+# ---------------------------------------------------------------------------
+
+
+@given("the M1-M6 E2E verification helper files exist for e2e-mock-audit")
+def step_helpers_exist(context: Any) -> None:
+    """Verify that all 6 helper files exist on disk."""
+    missing: list[str] = []
+    for rel_path in _HELPER_GLOBS:
+        full = _REPO_ROOT / rel_path
+        if not full.is_file():
+            missing.append(rel_path)
+    assert not missing, f"Missing E2E helper files: {', '.join(missing)}"
+    context.e2e_mock_audit_helpers = [_REPO_ROOT / p for p in _HELPER_GLOBS]
+
+
+# ---------------------------------------------------------------------------
+# When steps
+# ---------------------------------------------------------------------------
+
+
+@when("I analyze mock and subprocess usage in M1-M6 helpers for e2e-mock-audit")
+def step_analyze_all(context: Any) -> None:
+    """Analyze all 6 helpers and store flat list of function analyses."""
+    all_functions: list[FunctionAnalysis] = []
+    for helper_path in context.e2e_mock_audit_helpers:
+        all_functions.extend(_analyze_helper(helper_path))
+    context.e2e_mock_audit_functions = all_functions
+
+
+@when("I analyze mock and subprocess usage per suite for e2e-mock-audit")
+def step_analyze_per_suite(context: Any) -> None:
+    """Analyze all 6 helpers and store per-suite results."""
+    per_suite: dict[str, list[FunctionAnalysis]] = {}
+    for helper_path in context.e2e_mock_audit_helpers:
+        suite = helper_path.stem
+        per_suite[suite] = _analyze_helper(helper_path)
+    context.e2e_mock_audit_per_suite = per_suite
+
+
+# ---------------------------------------------------------------------------
+# Then steps
+# ---------------------------------------------------------------------------
+
+
+@then(
+    "at least one CLI-facing test should use subprocess instead of "
+    "CliRunner for e2e-mock-audit"
+)
+def step_at_least_one_subprocess(context: Any) -> None:
+    """Assert that at least one CLI-facing function uses subprocess."""
+    functions: list[FunctionAnalysis] = context.e2e_mock_audit_functions
+    cli_facing = [f for f in functions if f.uses_cli_runner]
+    assert cli_facing, "No CLI-facing functions found at all"
+
+    subprocess_users = [f for f in cli_facing if f.uses_subprocess_cli]
+    assert subprocess_users, (
+        f"Bug #658 confirmed: {len(cli_facing)} CLI-facing E2E test "
+        f"functions found across M1-M6, but NONE invoke the real CLI "
+        f"via subprocess. All use Typer's in-process CliRunner.\n"
+        f"CLI-facing functions: "
+        f"{', '.join(f'{f.suite}::{f.name}' for f in cli_facing)}"
+    )
+
+
+@then(
+    "at least one CLI-facing test should not mock the service factory "
+    "for e2e-mock-audit"
+)
+def step_at_least_one_unmocked(context: Any) -> None:
+    """Assert that at least one CLI-facing function skips service mocking."""
+    functions: list[FunctionAnalysis] = context.e2e_mock_audit_functions
+    cli_facing = [f for f in functions if f.uses_cli_runner]
+    assert cli_facing, "No CLI-facing functions found at all"
+
+    unmocked = [f for f in cli_facing if not f.uses_mock_patch]
+    assert unmocked, (
+        f"Bug #658 confirmed: {len(cli_facing)} CLI-facing E2E test "
+        f"functions found across M1-M6, but ALL mock the service layer "
+        f"via unittest.mock.patch. None exercise the real DI container.\n"
+        f"Mocked functions: "
+        f"{', '.join(f'{f.suite}::{f.name}' for f in cli_facing)}"
+    )
+
+
+@then(
+    "every suite with CLI tests should have at least one unmocked test "
+    "for e2e-mock-audit"
+)
+def step_every_suite_has_unmocked(context: Any) -> None:
+    """Assert that no suite has 100% mocked CLI tests."""
+    per_suite: dict[str, list[FunctionAnalysis]] = context.e2e_mock_audit_per_suite
+
+    all_mocked_suites: list[str] = []
+    for suite, functions in per_suite.items():
+        cli_facing = [f for f in functions if f.uses_cli_runner]
+        if not cli_facing:
+            continue  # Suite has no CLI tests — skip (e.g. M5)
+        unmocked = [f for f in cli_facing if not f.uses_mock_patch]
+        if not unmocked:
+            all_mocked_suites.append(
+                f"{suite} ({len(cli_facing)} CLI tests, all mocked)"
+            )
+
+    assert not all_mocked_suites, (
+        "Bug #658 confirmed: the following suites have 100% mocked CLI "
+        "tests with no real code path coverage:\n"
+        + "\n".join(f"  - {s}" for s in all_mocked_suites)
+    )

features/tdd_e2e_mock_only_coverage.feature

@@ -0,0 +1,37 @@
+@tdd_bug @tdd_bug_658 @tdd_expected_fail
+Feature: TDD Bug #658 — E2E verification suites use mocks instead of real system
+  As a developer
+  I want to verify that M1-M6 E2E verification suites exercise at least one
+  real (non-mocked) CLI code path via subprocess invocation
+  So that integration failures in DI wiring, database, and process-level
+  behavior are detectable by the E2E test suites
+
+  The root cause is that all 21 CLI-facing tests in the M1-M6 E2E helper
+  files use ``unittest.mock.patch`` to replace service factories with
+  ``MagicMock`` objects and invoke the CLI via Typer's in-process
+  ``CliRunner`` instead of ``subprocess.run``.  This means:
+
+  - DI wiring bugs (e.g. the ``container.db()`` AttributeError in #554,
+    #570) are invisible to the E2E suites.
+  - Database schema/migration issues are invisible.
+  - Process-level behavior (exit codes, environment variables, Rich
+    console routing) is never tested.
+
+  The 35 remaining tests are pure domain/service-level tests that never
+  invoke the CLI at all — they are legitimate unit/integration tests but
+  should not be labeled "E2E verification."
+
+  Background:
+    Given the M1-M6 E2E verification helper files exist for e2e-mock-audit
+
+  Scenario: At least one CLI-facing E2E test invokes the real CLI via subprocess
+    When I analyze mock and subprocess usage in M1-M6 helpers for e2e-mock-audit
+    Then at least one CLI-facing test should use subprocess instead of CliRunner for e2e-mock-audit
+
+  Scenario: At least one CLI-facing E2E test exercises the real service layer
+    When I analyze mock and subprocess usage in M1-M6 helpers for e2e-mock-audit
+    Then at least one CLI-facing test should not mock the service factory for e2e-mock-audit
+
+  Scenario: No E2E suite has 100% mocked CLI tests
+    When I analyze mock and subprocess usage per suite for e2e-mock-audit
+    Then every suite with CLI tests should have at least one unmocked test for e2e-mock-audit

robot/helper_tdd_e2e_mock_only_coverage.py

@@ -0,0 +1,254 @@
+"""Helper script for tdd_e2e_mock_only_coverage.robot smoke tests.
+
+Each subcommand uses AST analysis to inspect the M1-M6 E2E verification
+helper files and classify each public test function by its mock and CLI
+invocation strategy.  The helper reports the **real** outcome: it exits 0
+and prints the sentinel when the assertion holds (bug is fixed), and exits
+1 when the bug is still present.  The ``tdd_expected_fail`` tag on the
+Robot side handles pass/fail inversion while the bug remains open.
+
+Root cause (Bug #658)
+~~~~~~~~~~~~~~~~~~~~~
+All 21 CLI-facing tests in ``robot/helper_m*_e2e_verification.py`` use
+``unittest.mock.patch`` to replace service factories with ``MagicMock``
+objects and invoke the CLI via Typer's in-process ``CliRunner``.  Zero
+tests invoke the actual ``agents`` CLI binary via ``subprocess.run``.
+"""
+
+from __future__ import annotations
+
+import ast
+import sys
+from collections.abc import Callable
+from dataclasses import dataclass, field
+from pathlib import Path
+
+_REPO_ROOT = Path(__file__).resolve().parents[1]
+
+_HELPER_FILES = [
+    _REPO_ROOT / f"robot/helper_m{n}_e2e_verification.py" for n in range(1, 7)
+]
+
+# Patch targets that indicate service-layer mocking.
+_SERVICE_MOCK_INDICATORS: frozenset[str] = frozenset(
+    {
+        "_get_lifecycle_service",
+        "_get_registry_service",
+        "_get_namespaced_project_repo",
+        "_get_resource_link_repo",
+        "_get_resource_registry_service",
+        "_get_services",
+        "_get_service",
+        "get_container",
+        "CorrectionService",
+    }
+)
+
+
+@dataclass
+class FunctionAnalysis:
+    """Analysis result for a single function in an E2E helper file."""
+
+    name: str
+    suite: str
+    uses_cli_runner: bool = False
+    uses_mock_patch: bool = False
+    uses_subprocess_cli: bool = False
+    mock_targets: list[str] = field(default_factory=list)
+
+
+def _analyze_helper(helper_path: Path) -> list[FunctionAnalysis]:
+    """Parse a helper file and classify each top-level function."""
+    source = helper_path.read_text(encoding="utf-8")
+    tree = ast.parse(source, filename=str(helper_path))
+    suite = helper_path.stem
+
+    results: list[FunctionAnalysis] = []
+
+    for node in ast.iter_child_nodes(tree):
+        if not isinstance(node, ast.FunctionDef):
+            continue
+        if node.name.startswith("_"):
+            continue
+
+        fa = FunctionAnalysis(name=node.name, suite=suite)
+
+        for child in ast.walk(node):
+            if isinstance(child, ast.Attribute):
+                attr_name = child.attr
+                if attr_name == "invoke" and _is_cli_runner_invoke(child):
+                    fa.uses_cli_runner = True
+                if attr_name in ("run", "Popen") and _is_subprocess_call(child):
+                    fa.uses_subprocess_cli = True
+
+            if isinstance(child, ast.Call) and _is_patch_call(child):
+                fa.uses_mock_patch = True
+                target = _extract_patch_target(child)
+                if target:
+                    fa.mock_targets.append(target)
+
+            if isinstance(child, ast.Constant) and isinstance(child.value, str):
+                for indicator in _SERVICE_MOCK_INDICATORS:
+                    if indicator in child.value:
+                        fa.uses_mock_patch = True
+
+        results.append(fa)
+
+    return results
+
+
+def _is_cli_runner_invoke(node: ast.Attribute) -> bool:
+    """Check if attribute access is ``runner.invoke(...)``."""
+    return isinstance(node.value, ast.Name) and node.value.id in (
+        "runner",
+        "cli_runner",
+    )
+
+
+def _is_subprocess_call(node: ast.Attribute) -> bool:
+    """Check if attribute access is ``subprocess.run(...)``."""
+    return isinstance(node.value, ast.Name) and node.value.id == "subprocess"
+
+
+def _is_patch_call(node: ast.Call) -> bool:
+    """Check if a Call node is ``patch(...)`` or ``mock.patch(...)``."""
+    func = node.func
+    if isinstance(func, ast.Attribute) and func.attr == "patch":
+        return True
+    return isinstance(func, ast.Name) and func.id == "patch"
+
+
+def _extract_patch_target(node: ast.Call) -> str | None:
+    """Extract the first string argument from a ``patch(...)`` call."""
+    if node.args and isinstance(node.args[0], ast.Constant):
+        val = node.args[0].value
+        if isinstance(val, str):
+            return val
+    return None
+
+
+def _get_all_functions() -> list[FunctionAnalysis]:
+    """Analyze all M1-M6 helpers and return flat function list."""
+    all_functions: list[FunctionAnalysis] = []
+    for helper_path in _HELPER_FILES:
+        if not helper_path.is_file():
+            print(
+                f"ERROR: missing helper file {helper_path}",
+                file=sys.stderr,
+            )
+            sys.exit(2)
+        all_functions.extend(_analyze_helper(helper_path))
+    return all_functions
+
+
+# ---------------------------------------------------------------------------
+# Subcommands
+# ---------------------------------------------------------------------------
+
+
+def check_subprocess_usage() -> None:
+    """Assert at least one CLI-facing test uses subprocess.
+
+    Exits 0 with sentinel when the assertion holds (bug fixed).
+    Exits 1 when the bug is still present.
+    """
+    functions = _get_all_functions()
+    cli_facing = [f for f in functions if f.uses_cli_runner]
+    if not cli_facing:
+        print("ERROR: no CLI-facing functions found", file=sys.stderr)
+        sys.exit(2)
+
+    subprocess_users = [f for f in cli_facing if f.uses_subprocess_cli]
+    if not subprocess_users:
+        print(
+            f"Bug #658 present: {len(cli_facing)} CLI-facing E2E test "
+            f"functions, but NONE use subprocess. All use CliRunner.",
+            file=sys.stderr,
+        )
+        sys.exit(1)
+
+    print("tdd-e2e-subprocess-check-ok")
+
+
+def check_unmocked_services() -> None:
+    """Assert at least one CLI-facing test skips service mocking.
+
+    Exits 0 with sentinel when the assertion holds (bug fixed).
+    Exits 1 when the bug is still present.
+    """
+    functions = _get_all_functions()
+    cli_facing = [f for f in functions if f.uses_cli_runner]
+    if not cli_facing:
+        print("ERROR: no CLI-facing functions found", file=sys.stderr)
+        sys.exit(2)
+
+    unmocked = [f for f in cli_facing if not f.uses_mock_patch]
+    if not unmocked:
+        print(
+            f"Bug #658 present: {len(cli_facing)} CLI-facing E2E test "
+            f"functions, but ALL mock the service layer.",
+            file=sys.stderr,
+        )
+        sys.exit(1)
+
+    print("tdd-e2e-unmocked-services-ok")
+
+
+def check_per_suite_coverage() -> None:
+    """Assert no suite has 100% mocked CLI tests.
+
+    Exits 0 with sentinel when the assertion holds (bug fixed).
+    Exits 1 when the bug is still present.
+    """
+    per_suite: dict[str, list[FunctionAnalysis]] = {}
+    for helper_path in _HELPER_FILES:
+        if not helper_path.is_file():
+            print(
+                f"ERROR: missing helper file {helper_path}",
+                file=sys.stderr,
+            )
+            sys.exit(2)
+        suite = helper_path.stem
+        per_suite[suite] = _analyze_helper(helper_path)
+
+    all_mocked_suites: list[str] = []
+    for suite, functions in per_suite.items():
+        cli_facing = [f for f in functions if f.uses_cli_runner]
+        if not cli_facing:
+            continue
+        unmocked = [f for f in cli_facing if not f.uses_mock_patch]
+        if not unmocked:
+            all_mocked_suites.append(
+                f"{suite} ({len(cli_facing)} CLI tests, all mocked)"
+            )
+
+    if all_mocked_suites:
+        print(
+            "Bug #658 present: suites with 100% mocked CLI tests:\n"
+            + "\n".join(f"  - {s}" for s in all_mocked_suites),
+            file=sys.stderr,
+        )
+        sys.exit(1)
+
+    print("tdd-e2e-per-suite-coverage-ok")
+
+
+# ---------------------------------------------------------------------------
+# Dispatcher
+# ---------------------------------------------------------------------------
+
+_COMMANDS: dict[str, Callable[[], None]] = {
+    "check-subprocess-usage": check_subprocess_usage,
+    "check-unmocked-services": check_unmocked_services,
+    "check-per-suite-coverage": check_per_suite_coverage,
+}
+
+if __name__ == "__main__":
+    if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
+        print(
+            f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>",
+            file=sys.stderr,
+        )
+        sys.exit(1)
+    cmd = _COMMANDS[sys.argv[1]]
+    cmd()

robot/tdd_e2e_mock_only_coverage.robot

@@ -0,0 +1,44 @@
+*** Settings ***
+Documentation    TDD Bug #658 — E2E verification suites use mocks instead of real system
+...              Integration tests verifying that at least one CLI-facing E2E test
+...              function exercises the real CLI via subprocess invocation without
+...              mocking the service layer. Bug #658 documents that all 21 CLI-facing
+...              tests in the M1-M6 helpers use unittest.mock.patch + CliRunner.
+Resource         ${CURDIR}/common.resource
+Suite Setup      Setup Test Environment
+Suite Teardown   Cleanup Test Environment
+
+*** Variables ***
+${HELPER}    ${CURDIR}/helper_tdd_e2e_mock_only_coverage.py
+
+*** Test Cases ***
+TDD At Least One CLI Test Uses Subprocess
+    [Documentation]    Verify that at least one CLI-facing E2E test invokes
+    ...                the real ``agents`` CLI via subprocess instead of
+    ...                Typer's in-process CliRunner.
+    [Tags]    tdd_bug    tdd_bug_658    tdd_expected_fail
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    check-subprocess-usage    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    tdd-e2e-subprocess-check-ok
+
+TDD At Least One CLI Test Skips Service Mocking
+    [Documentation]    Verify that at least one CLI-facing E2E test exercises
+    ...                the real service layer without mocking service factories.
+    [Tags]    tdd_bug    tdd_bug_658    tdd_expected_fail
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    check-unmocked-services    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    tdd-e2e-unmocked-services-ok
+
+TDD No Suite Has 100 Percent Mocked CLI Tests
+    [Documentation]    Verify that every E2E suite with CLI tests has at
+    ...                least one test that exercises real code paths.
+    [Tags]    tdd_bug    tdd_bug_658    tdd_expected_fail
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    check-per-suite-coverage    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    tdd-e2e-per-suite-coverage-ok