From 6806ef3620050b21ff5895e9dff861fb5a1e0768 Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Thu, 12 Mar 2026 18:35:54 +0000 Subject: [PATCH] test(e2e): TDD failing tests for E2E mock-only coverage (bug #658) Add Behave and Robot Framework TDD tests that use AST analysis to inspect the M1-M6 E2E verification helper files and prove that all 21 CLI-facing test functions use unittest.mock.patch + CliRunner instead of exercising the real CLI via subprocess invocation. The tests verify three properties that currently fail (proving #658): - At least one CLI-facing test uses subprocess instead of CliRunner - At least one CLI-facing test exercises the real service layer - Every suite with CLI tests has at least one unmocked test Tagged @tdd_expected_fail so the tests pass CI while the bug is present. The @tdd_expected_fail tag inverts the result: the tests pass because the underlying assertions fail (confirming the bug). Once #658 is fixed, the tag is removed and the tests run normally. Files added: - features/tdd_e2e_mock_only_coverage.feature (3 scenarios) - features/steps/tdd_e2e_mock_only_coverage_steps.py - robot/tdd_e2e_mock_only_coverage.robot (3 test cases) - robot/helper_tdd_e2e_mock_only_coverage.py ISSUES CLOSED: #697 --- .../steps/tdd_e2e_mock_only_coverage_steps.py | 257 ++++++++++++++++++ features/tdd_e2e_mock_only_coverage.feature | 37 +++ robot/helper_tdd_e2e_mock_only_coverage.py | 254 +++++++++++++++++ robot/tdd_e2e_mock_only_coverage.robot | 44 +++ 4 files changed, 592 insertions(+) create mode 100644 features/steps/tdd_e2e_mock_only_coverage_steps.py create mode 100644 features/tdd_e2e_mock_only_coverage.feature create mode 100644 robot/helper_tdd_e2e_mock_only_coverage.py create mode 100644 robot/tdd_e2e_mock_only_coverage.robot diff --git a/features/steps/tdd_e2e_mock_only_coverage_steps.py b/features/steps/tdd_e2e_mock_only_coverage_steps.py new file mode 100644 index 000000000..75ef2b07b --- /dev/null +++ b/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) + ) diff --git a/features/tdd_e2e_mock_only_coverage.feature b/features/tdd_e2e_mock_only_coverage.feature new file mode 100644 index 000000000..34a2a0e67 --- /dev/null +++ b/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 diff --git a/robot/helper_tdd_e2e_mock_only_coverage.py b/robot/helper_tdd_e2e_mock_only_coverage.py new file mode 100644 index 000000000..d695f3339 --- /dev/null +++ b/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() diff --git a/robot/tdd_e2e_mock_only_coverage.robot b/robot/tdd_e2e_mock_only_coverage.robot new file mode 100644 index 000000000..e911207b6 --- /dev/null +++ b/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 -- 2.52.0