diff --git a/CHANGELOG.md b/CHANGELOG.md index bcb635ecb..0f5222c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ## Unreleased +- Added TDD-style failing Behave BDD tests for the session list DI container + missing `db` provider bug. Three scenarios exercise `session list`, + `_get_session_service()`, and `session list --format json` through the real + DI path. Includes Robot Framework smoke tests and ASV benchmarks. Tests + are intentionally failing (``@tdd_expected_fail``) until the bug fix for + #554 is applied. (#631) +- Added TDD-style failing Behave BDD tests for the session create DI container + missing `db` provider bug. Three scenarios exercise `session create`, + `session create --actor`, and `session create --format json` through the + real DI path. Includes Robot Framework smoke tests and ASV benchmarks. + Tests are intentionally failing (``@tdd_expected_fail``) until the bug fix + for #570 is applied. (#630) - Added `RepoIndexingService` for repository file indexing with incremental refresh, extension-based language detection, SHA-256 content hashing, and token estimation. Supports policy enforcement via include/exclude globs, diff --git a/benchmarks/_session_bench_common.py b/benchmarks/_session_bench_common.py new file mode 100644 index 000000000..c109e0996 --- /dev/null +++ b/benchmarks/_session_bench_common.py @@ -0,0 +1,45 @@ +"""Shared helpers for TDD session DI ASV benchmarks. + +Extracted from ``tdd_session_create_di_bench.py`` and +``tdd_session_list_di_bench.py`` to eliminate duplication (review finding F9). +""" + +from __future__ import annotations + +import sys +from datetime import datetime +from pathlib import Path + +# Ensure the local *source* tree is importable even when ASV has an +# older build of the package installed. This is a permanent mutation of +# ``sys.path`` — acceptable because ASV runs each benchmark suite in its +# own isolated process. +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from typer.testing import CliRunner # noqa: E402 +from ulid import ULID # noqa: E402 + +from cleveragents.domain.models.core.session import ( # noqa: E402 + Session, + SessionTokenUsage, +) + +runner = CliRunner() + + +def mock_session( + session_id: str | None = None, + actor_name: str | None = None, +) -> Session: + """Create a ``Session`` instance with sensible defaults for benchmarks.""" + return Session( + session_id=session_id or str(ULID()), + actor_name=actor_name, + namespace="local", + messages=[], + token_usage=SessionTokenUsage(), + created_at=datetime.now(), + updated_at=datetime.now(), + ) diff --git a/benchmarks/tdd_session_create_di_bench.py b/benchmarks/tdd_session_create_di_bench.py new file mode 100644 index 000000000..49e23ddc9 --- /dev/null +++ b/benchmarks/tdd_session_create_di_bench.py @@ -0,0 +1,39 @@ +"""ASV benchmarks for TDD Bug #570 — session create CLI throughput. + +Measures the performance of the session create CLI command path to establish +a baseline before and after the DI bug fix. Uses a mocked service so the +benchmark isolates CLI/rendering overhead from database I/O. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from benchmarks._session_bench_common import mock_session, runner + +from cleveragents.cli.commands import session as session_mod # noqa: E402 +from cleveragents.cli.commands.session import app as session_app # noqa: E402 + + +class TDDSessionCreateDISuite: + """Benchmark session create command throughput (TDD bug #570 baseline).""" + + def setup(self) -> None: + self._svc = MagicMock() + self._svc.create.return_value = mock_session() + session_mod._service = self._svc + + def teardown(self) -> None: + session_mod._service = None + + def time_create_default(self) -> None: + """Benchmark create with defaults.""" + runner.invoke(session_app, ["create"]) + + def time_create_with_actor(self) -> None: + """Benchmark create with actor flag.""" + runner.invoke(session_app, ["create", "--actor", "openai/gpt-4"]) + + def time_create_json(self) -> None: + """Benchmark create with JSON format.""" + runner.invoke(session_app, ["create", "--format", "json"]) diff --git a/benchmarks/tdd_session_list_di_bench.py b/benchmarks/tdd_session_list_di_bench.py index c323ee83b..fb296a1ca 100644 --- a/benchmarks/tdd_session_list_di_bench.py +++ b/benchmarks/tdd_session_list_di_bench.py @@ -7,43 +7,12 @@ benchmark isolates CLI/rendering overhead from database I/O. from __future__ import annotations -import sys -from datetime import datetime -from pathlib import Path from unittest.mock import MagicMock -# Ensure the local *source* tree is importable even when ASV has an -# older build of the package installed. -_SRC = str(Path(__file__).resolve().parents[1] / "src") -if _SRC not in sys.path: - sys.path.insert(0, _SRC) - -from typer.testing import CliRunner # noqa: E402 -from ulid import ULID # noqa: E402 +from benchmarks._session_bench_common import mock_session, runner from cleveragents.cli.commands import session as session_mod # noqa: E402 from cleveragents.cli.commands.session import app as session_app # noqa: E402 -from cleveragents.domain.models.core.session import ( # noqa: E402 - Session, - SessionTokenUsage, -) - -_runner = CliRunner() - - -def _mock_session( - session_id: str | None = None, - actor_name: str | None = None, -) -> Session: - return Session( - session_id=session_id or str(ULID()), - actor_name=actor_name, - namespace="local", - messages=[], - token_usage=SessionTokenUsage(), - created_at=datetime.now(), - updated_at=datetime.now(), - ) class TDDSessionListDISuite: @@ -51,8 +20,13 @@ class TDDSessionListDISuite: def setup(self) -> None: self._svc = MagicMock() + # Use deterministic, unique session IDs for reproducible benchmarks. self._svc.list.return_value = [ - _mock_session(actor_name=f"openai/gpt-{i}") for i in range(50) + mock_session( + session_id=f"01HXYZ{i:020d}", + actor_name=f"openai/gpt-{i}", + ) + for i in range(50) ] session_mod._service = self._svc @@ -61,17 +35,21 @@ class TDDSessionListDISuite: def time_list_rich(self) -> None: """Benchmark listing sessions with default rich format.""" - _runner.invoke(session_app, ["list"]) + runner.invoke(session_app, ["list"]) def time_list_json(self) -> None: """Benchmark listing sessions with JSON format.""" - _runner.invoke(session_app, ["list", "--format", "json"]) + runner.invoke(session_app, ["list", "--format", "json"]) def time_list_empty(self) -> None: """Benchmark listing with no sessions.""" self._svc.list.return_value = [] - _runner.invoke(session_app, ["list"]) - # Restore for next benchmark + runner.invoke(session_app, ["list"]) + # Restore for next benchmark (deterministic IDs). self._svc.list.return_value = [ - _mock_session(actor_name=f"openai/gpt-{i}") for i in range(50) + mock_session( + session_id=f"01HXYZ{i:020d}", + actor_name=f"openai/gpt-{i}", + ) + for i in range(50) ] diff --git a/features/environment.py b/features/environment.py index a18a95400..7d06891c7 100644 --- a/features/environment.py +++ b/features/environment.py @@ -1,6 +1,7 @@ """Behave environment setup for feature tests.""" import contextlib +import logging import os import re import shutil @@ -12,6 +13,8 @@ from typing import Any from behave.model import Scenario from behave.model_core import Status +_tdd_logger = logging.getLogger("behave.tdd_expected_fail") + LANGSMITH_ENV_VARS = [ "CLEVERAGENTS_LANGSMITH_ENABLED", "CLEVERAGENTS_LANGSMITH_PROJECT", @@ -316,29 +319,37 @@ def before_scenario(context, scenario): _TDD_BUG_N_RE = re.compile(r"^tdd_bug_\d+$") -def _handle_tdd_expected_fail(scenario: Scenario) -> None: - """Invert pass/fail for scenarios tagged ``@tdd_expected_fail``. +def handle_tdd_expected_fail(scenario: Scenario) -> None: + """Validate TDD tags and invert pass/fail for ``@tdd_expected_fail``. - When a TDD bug-capture test carries ``@tdd_expected_fail``, the test is - *expected* to fail because the bug it captures is still present. This - hook inverts the status so CI stays green: + Tag validation (per CONTRIBUTING.md — unconditional): + * Any scenario with ``@tdd_bug_`` **must** also carry ``@tdd_bug``. + Missing ``@tdd_bug`` causes the scenario to fail unconditionally. + + Status inversion (only when ``@tdd_expected_fail`` is present): + + * ``@tdd_expected_fail`` additionally requires ``@tdd_bug`` **and** at + least one ``@tdd_bug_`` tag. * **failed → passed** — the bug still triggers, which is expected. * **passed → failed** — the bug was fixed but the tag was not removed; this is an error that must be caught. - - Tag validation (per CONTRIBUTING.md): - - * ``@tdd_expected_fail`` requires ``@tdd_bug`` **and** at least one - ``@tdd_bug_`` tag. Missing companion tags cause the scenario to - fail unconditionally. """ tags = set(scenario.effective_tags) + # --- unconditional tag validation (F5) -------------------------------- + has_tdd_bug_n = any(_TDD_BUG_N_RE.match(t) for t in tags) + if has_tdd_bug_n and "tdd_bug" not in tags: + scenario.set_status(Status.failed) + sys.stderr.write( + f"TDD TAG ERROR: {scenario.name!r} — @tdd_bug_ requires @tdd_bug tag\n" + ) + return + if "tdd_expected_fail" not in tags: return - # --- tag validation --------------------------------------------------- + # --- @tdd_expected_fail tag validation -------------------------------- if "tdd_bug" not in tags: scenario.set_status(Status.failed) sys.stderr.write( @@ -347,7 +358,7 @@ def _handle_tdd_expected_fail(scenario: Scenario) -> None: ) return - if not any(_TDD_BUG_N_RE.match(t) for t in tags): + if not has_tdd_bug_n: scenario.set_status(Status.failed) sys.stderr.write( f"TDD TAG ERROR: {scenario.name!r} — " @@ -357,6 +368,17 @@ def _handle_tdd_expected_fail(scenario: Scenario) -> None: # --- status inversion ------------------------------------------------- if scenario.status == Status.failed: + # Log original failure details before inverting so CI logs show what + # actually failed (N3 review finding). + for step in scenario.steps: + if step.status == Status.failed: + _tdd_logger.info( + "TDD inversion: %s — step %r failed: %s", + scenario.name, + step.name, + step.error_message or "(no message)", + ) + # Bug still present — expected. Mark scenario and its failed/skipped # steps as passed so that summary counts are accurate. scenario.clear_status() @@ -378,7 +400,7 @@ def after_scenario(context, scenario): """Clean up after each scenario.""" # Handle TDD expected-fail inversion BEFORE cleanup (status is already set # by step execution; cleanup does not change it). - _handle_tdd_expected_fail(scenario) + handle_tdd_expected_fail(scenario) # Return to original directory first if hasattr(context, "original_cwd"): diff --git a/features/steps/tdd_expected_fail_infrastructure_steps.py b/features/steps/tdd_expected_fail_infrastructure_steps.py new file mode 100644 index 000000000..8bbf6df62 --- /dev/null +++ b/features/steps/tdd_expected_fail_infrastructure_steps.py @@ -0,0 +1,83 @@ +"""Step definitions for the TDD expected-fail handler infrastructure tests. + +These scenarios exercise ``_handle_tdd_expected_fail`` directly using real +Behave ``Scenario`` and ``Step`` objects (not the running test's own scenario) +to verify both scenario-level and **step-level** status inversion. +""" + +from __future__ import annotations + +from behave import given, then, when +from behave.model import Scenario, Step +from behave.model_core import Status +from behave.runner import Context + +from features.environment import handle_tdd_expected_fail + +_STATUS_MAP: dict[str, Status] = { + "failed": Status.failed, + "passed": Status.passed, + "skipped": Status.skipped, + "untested": Status.untested, +} + + +@given('a mock scenario tagged "{tags}" with status "{status}"') +def step_mock_scenario(context: Context, tags: str, status: str) -> None: + """Build a real ``Scenario`` object with the requested tags and status.""" + tag_list = [t.lstrip("@") for t in tags.split()] + scenario = Scenario( + filename="", + line=1, + keyword="Scenario", + name="mock-tdd-scenario", + tags=tag_list, + ) + # Force the desired starting status. + scenario.clear_status() + scenario.set_status(_STATUS_MAP[status]) + context.mock_scenario = scenario + context.mock_steps = {} + + +@given('the mock scenario has a step "{step_name}" with status "{status}"') +def step_add_mock_step(context: Context, step_name: str, status: str) -> None: + """Add a real ``Step`` to the mock scenario with the requested status.""" + step = Step( + filename="", + line=1, + keyword="Given", + step_type="given", + name=step_name, + ) + step.status = _STATUS_MAP[status] + # Provide an error_message for failed steps so logging (N3) can use it. + if status == "failed": + step.error_message = f"simulated failure in {step_name!r}" + context.mock_scenario.steps.append(step) + context.mock_steps[step_name] = step + + +@when("the TDD expected-fail handler processes the scenario") +def step_run_handler(context: Context) -> None: + """Invoke ``_handle_tdd_expected_fail`` on the mock scenario.""" + handle_tdd_expected_fail(context.mock_scenario) + + +@then('the scenario status should be "{expected}"') +def step_check_scenario_status(context: Context, expected: str) -> None: + """Assert the scenario has the expected status after handler processing.""" + actual = context.mock_scenario.status + assert actual == _STATUS_MAP[expected], ( + f"Expected scenario status {expected!r}, got {actual!r}" + ) + + +@then('the step "{step_name}" should have status "{expected}"') +def step_check_step_status(context: Context, step_name: str, expected: str) -> None: + """Assert a named step has the expected status after handler processing.""" + step = context.mock_steps[step_name] + actual = step.status + assert actual == _STATUS_MAP[expected], ( + f"Expected step {step_name!r} status {expected!r}, got {actual!r}" + ) diff --git a/features/steps/tdd_session_create_di_steps.py b/features/steps/tdd_session_create_di_steps.py new file mode 100644 index 000000000..ac90e82fb --- /dev/null +++ b/features/steps/tdd_session_create_di_steps.py @@ -0,0 +1,35 @@ +"""Step definitions for TDD Bug #570 — session create DI error. + +These steps exercise the *real* DI path in ``_get_session_service()`` without +mocking, so the ``container.db()`` ``AttributeError`` is triggered. The +``@tdd_expected_fail`` tag on the scenarios inverts the result. +""" + +from __future__ import annotations + +from behave import when +from behave.runner import Context + +from cleveragents.cli.commands.session import app as session_app + +# The "Given a CLI runner using the real session DI path" step and the +# parameterised exit/JSON assertion steps live in tdd_session_shared_steps.py +# (Behave loads all steps globally). + + +@when("I invoke the session create command") +def step_invoke_create(context: Context) -> None: + """Invoke ``session create`` through the real CLI app.""" + context.result = context.runner.invoke(session_app, ["create"]) + + +@when('I invoke the session create command with actor "{actor}"') +def step_invoke_create_with_actor(context: Context, actor: str) -> None: + """Invoke ``session create --actor`` through the real CLI app.""" + context.result = context.runner.invoke(session_app, ["create", "--actor", actor]) + + +@when("I invoke the session create command with format json") +def step_invoke_create_json(context: Context) -> None: + """Invoke ``session create --format json`` through the real CLI app.""" + context.result = context.runner.invoke(session_app, ["create", "--format", "json"]) diff --git a/features/steps/tdd_session_list_di_steps.py b/features/steps/tdd_session_list_di_steps.py index 06e6a017b..8fd506344 100644 --- a/features/steps/tdd_session_list_di_steps.py +++ b/features/steps/tdd_session_list_di_steps.py @@ -9,52 +9,16 @@ mocking, so the ``container.db()`` ``AttributeError`` is triggered. The from __future__ import annotations -import contextlib -import json -import os -import tempfile - -from behave import given, then, when +from behave import then, when from behave.runner import Context -from typer.testing import CliRunner from cleveragents.cli.commands import session as session_mod from cleveragents.cli.commands.session import app as session_app +from cleveragents.domain.models.core.session import SessionService - -@given("a CLI runner using the real session DI path") -def step_real_di_runner(context: Context) -> None: - """Set up a CLI runner that does NOT mock the session service. - - By ensuring ``session_mod._service`` is ``None``, the CLI will call - ``_get_session_service()`` which hits the real DI container and - triggers the ``container.db()`` bug. - """ - context.runner = CliRunner() - - # Ensure we go through the real DI path — no mock service. - session_mod._service = None - - # Provide a database URL so the container can be constructed (the bug - # triggers before the URL is actually used). - fd, db_path = tempfile.mkstemp(suffix=".db") - os.close(fd) - context._tdd_db_path = db_path - os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{db_path}" - - def _cleanup() -> None: - session_mod._service = None - os.environ.pop("CLEVERAGENTS_DATABASE_URL", None) - try: - from cleveragents.application.container import reset_container - - reset_container() - except ImportError: - pass - with contextlib.suppress(OSError): - os.unlink(db_path) - - context.add_cleanup(_cleanup) +# The "Given a CLI runner using the real session DI path" step and the +# parameterised exit/JSON assertion steps live in tdd_session_shared_steps.py +# (Behave loads all steps globally). @when("I invoke the session list command") @@ -80,34 +44,12 @@ def step_invoke_list_json(context: Context) -> None: context.result = context.runner.invoke(session_app, ["list", "--format", "json"]) -@then("the session list command should exit successfully") -def step_list_exits_ok(context: Context) -> None: - """Assert the command exits with code 0.""" - assert context.result.exit_code == 0, ( - f"Expected exit code 0, got {context.result.exit_code}.\n" - f"Output:\n{context.result.output}" - ) - - @then("the session service should be a valid SessionService instance") def step_service_is_valid(context: Context) -> None: """Assert that ``_get_session_service()`` returned a usable service.""" - from cleveragents.domain.models.core.session import SessionService - assert context.service_error is None, ( f"_get_session_service() raised {context.service_error!r}" ) assert isinstance(context.session_service, SessionService), ( f"Expected SessionService, got {type(context.session_service)}" ) - - -@then("the session list output should be valid JSON") -def step_list_output_json(context: Context) -> None: - """Assert the output is parseable JSON.""" - try: - json.loads(context.result.output) - except json.JSONDecodeError as exc: - raise AssertionError( - f"Output is not valid JSON:\n{context.result.output}" - ) from exc diff --git a/features/steps/tdd_session_shared_steps.py b/features/steps/tdd_session_shared_steps.py new file mode 100644 index 000000000..89b08158e --- /dev/null +++ b/features/steps/tdd_session_shared_steps.py @@ -0,0 +1,75 @@ +"""Shared step definitions for TDD session DI bug tests. + +Steps in this module are used by both ``tdd_session_create_di.feature`` and +``tdd_session_list_di.feature``. Behave loads all step files globally, so +placing shared steps here avoids duplication and satisfies the CONTRIBUTING.md +rule that feature-specific steps live in matching ``*_steps.py`` files while +shared steps live in clearly named reusable modules. +""" + +from __future__ import annotations + +import json +import os +import tempfile +from pathlib import Path + +from behave import given, then +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.application.container import reset_container +from cleveragents.cli.commands import session as session_mod +from cleveragents.config.settings import Settings + + +@given("a CLI runner using the real session DI path") +def step_real_di_runner(context: Context) -> None: + """Set up a CLI runner that does NOT mock the session service. + + By ensuring ``session_mod._service`` is ``None``, the CLI will call + ``_get_session_service()`` which hits the real DI container and + triggers the ``container.db()`` bug. + """ + context.runner = CliRunner() + + # Ensure we go through the real DI path — no mock service. + session_mod._service = None + + # Provide a database URL so the container can be constructed (the bug + # triggers before the URL is actually used). + fd, db_path = tempfile.mkstemp(suffix=".db") + os.close(fd) + context._tdd_db_path = db_path + os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{db_path}" + + def _cleanup() -> None: + session_mod._service = None + os.environ.pop("CLEVERAGENTS_DATABASE_URL", None) + reset_container() + # Reset the Settings singleton so stale database URLs from this + # scenario do not leak into subsequent scenarios. + Settings._instance = None + Path(db_path).unlink(missing_ok=True) + + context.add_cleanup(_cleanup) + + +@then("the session {subcommand} command should exit successfully") +def step_session_exits_ok(context: Context, subcommand: str) -> None: + """Assert the command exits with code 0.""" + assert context.result.exit_code == 0, ( + f"Expected exit code 0, got {context.result.exit_code}.\n" + f"Output:\n{context.result.output}" + ) + + +@then("the session {subcommand} output should be valid JSON") +def step_session_output_json(context: Context, subcommand: str) -> None: + """Assert the output is parseable JSON.""" + try: + json.loads(context.result.output) + except json.JSONDecodeError as exc: + raise AssertionError( + f"Output is not valid JSON:\n{context.result.output}" + ) from exc diff --git a/features/tdd_expected_fail_infrastructure.feature b/features/tdd_expected_fail_infrastructure.feature new file mode 100644 index 000000000..9729104dd --- /dev/null +++ b/features/tdd_expected_fail_infrastructure.feature @@ -0,0 +1,38 @@ +@infrastructure +Feature: TDD expected-fail handler infrastructure + Verify that the ``_handle_tdd_expected_fail`` hook in ``environment.py`` + correctly inverts scenario AND step-level status for TDD bug-capture tests. + + Scenario: Handler inverts a failed scenario with failed and skipped steps to passed + Given a mock scenario tagged "@tdd_expected_fail @tdd_bug @tdd_bug_999" with status "failed" + And the mock scenario has a step "broken step" with status "failed" + And the mock scenario has a step "skipped step" with status "skipped" + When the TDD expected-fail handler processes the scenario + Then the scenario status should be "passed" + And the step "broken step" should have status "passed" + And the step "skipped step" should have status "passed" + + Scenario: Handler fails a passed scenario that still carries @tdd_expected_fail + Given a mock scenario tagged "@tdd_expected_fail @tdd_bug @tdd_bug_999" with status "passed" + When the TDD expected-fail handler processes the scenario + Then the scenario status should be "failed" + + Scenario: Handler rejects @tdd_expected_fail without @tdd_bug + Given a mock scenario tagged "@tdd_expected_fail" with status "failed" + When the TDD expected-fail handler processes the scenario + Then the scenario status should be "failed" + + Scenario: Handler rejects @tdd_expected_fail without @tdd_bug_N + Given a mock scenario tagged "@tdd_expected_fail @tdd_bug" with status "failed" + When the TDD expected-fail handler processes the scenario + Then the scenario status should be "failed" + + Scenario: Handler rejects @tdd_bug_N without @tdd_bug unconditionally + Given a mock scenario tagged "@tdd_bug_999" with status "passed" + When the TDD expected-fail handler processes the scenario + Then the scenario status should be "failed" + + Scenario: Handler ignores scenarios without @tdd_expected_fail + Given a mock scenario tagged "@tdd_bug @tdd_bug_999" with status "failed" + When the TDD expected-fail handler processes the scenario + Then the scenario status should be "failed" diff --git a/features/tdd_session_create_di.feature b/features/tdd_session_create_di.feature new file mode 100644 index 000000000..83b598acc --- /dev/null +++ b/features/tdd_session_create_di.feature @@ -0,0 +1,29 @@ +@tdd_bug @tdd_bug_570 +Feature: TDD Bug #570 — session create DI container missing db provider + As a developer + I want to verify that `agents session create` fails due to the + DI container missing a `db` provider + So that the bug is captured and will be caught by a regression test + + The root cause is shared with bug #554: `_get_session_service()` in + session.py calls `container.db()`, but the Container class has no `db` + provider, causing an AttributeError at runtime. + + @tdd_expected_fail + Scenario: Session create command succeeds via DI container + Given a CLI runner using the real session DI path + When I invoke the session create command + Then the session create command should exit successfully + + @tdd_expected_fail + Scenario: Session create with actor succeeds via DI container + Given a CLI runner using the real session DI path + When I invoke the session create command with actor "openai/gpt-4" + Then the session create command should exit successfully + + @tdd_expected_fail + Scenario: Session create command produces structured output via DI + Given a CLI runner using the real session DI path + When I invoke the session create command with format json + Then the session create command should exit successfully + And the session create output should be valid JSON diff --git a/noxfile.py b/noxfile.py index 8c140e0f2..72e47daf4 100644 --- a/noxfile.py +++ b/noxfile.py @@ -252,7 +252,7 @@ def _has_failures(total): def _no_scenarios_ran(total): - """Return True when the runner collected zero scenario results. + """Return True when no scenario reached a terminal state. This catches runner-level crashes (e.g. ``before_all`` failure) that prevent any scenario from executing. Without this guard the summary diff --git a/robot/helper_tdd_session_create_di.py b/robot/helper_tdd_session_create_di.py new file mode 100644 index 000000000..696058316 --- /dev/null +++ b/robot/helper_tdd_session_create_di.py @@ -0,0 +1,118 @@ +"""Helper script for tdd_session_create_di.robot smoke tests. + +Each subcommand exercises the real DI path (no mocks) to reproduce bug #570. +The helper reports the **real** outcome: it exits 0 and prints the sentinel +when the operation succeeds (bug is fixed), and exits 1 when the bug is +still present. The ``tdd_expected_fail_listener`` on the Robot side handles +pass/fail inversion while the bug remains open. +""" + +from __future__ import annotations + +import sys +from collections.abc import Callable +from pathlib import Path + +# Ensure local source tree AND robot/ directory are importable. +_ROOT = Path(__file__).resolve().parents[1] +_SRC = str(_ROOT / "src") +_ROBOT = str(_ROOT / "robot") +for _p in (_SRC, _ROBOT): + if _p not in sys.path: + sys.path.insert(0, _p) + +from helper_tdd_session_di_common import runner, setup_real_di, teardown # noqa: E402 + +from cleveragents.cli.commands.session import app as session_app # noqa: E402 + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def create_di_error() -> None: + """Invoke ``session create`` through the real DI path. + + Exits 0 with sentinel when the command succeeds (bug fixed). + Exits 1 when the command fails (bug still present). + """ + db_path = setup_real_di() + try: + result = runner.invoke(session_app, ["create"]) + if result.exit_code == 0: + print("tdd-session-create-di-error-ok") + else: + print( + f"session create failed with exit code {result.exit_code}", + file=sys.stderr, + ) + sys.exit(1) + finally: + teardown(db_path) + + +def create_actor() -> None: + """Invoke ``session create --actor`` through the real DI path. + + Exits 0 with sentinel when the command succeeds (bug fixed). + Exits 1 when the command fails (bug still present). + """ + db_path = setup_real_di() + try: + result = runner.invoke( + session_app, + ["create", "--actor", "openai/gpt-4"], + ) + if result.exit_code == 0: + print("tdd-session-create-actor-ok") + else: + print( + f"session create --actor failed with exit code {result.exit_code}", + file=sys.stderr, + ) + sys.exit(1) + finally: + teardown(db_path) + + +def create_json() -> None: + """Invoke ``session create --format json`` through the real DI path. + + Exits 0 with sentinel when the command succeeds (bug fixed). + Exits 1 when the command fails (bug still present). + """ + db_path = setup_real_di() + try: + result = runner.invoke(session_app, ["create", "--format", "json"]) + if result.exit_code == 0: + print("tdd-session-create-json-ok") + else: + print( + "session create --format json failed " + f"with exit code {result.exit_code}", + file=sys.stderr, + ) + sys.exit(1) + finally: + teardown(db_path) + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "create-di-error": create_di_error, + "create-actor": create_actor, + "create-json": create_json, +} + +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/helper_tdd_session_di_common.py b/robot/helper_tdd_session_di_common.py new file mode 100644 index 000000000..6fa42d8d7 --- /dev/null +++ b/robot/helper_tdd_session_di_common.py @@ -0,0 +1,49 @@ +"""Shared setup/teardown for TDD session DI Robot helpers. + +Extracted from ``helper_tdd_session_create_di.py`` and +``helper_tdd_session_list_di.py`` to eliminate duplication (review finding F8). +""" + +from __future__ import annotations + +import os +import sys +import tempfile +from pathlib import Path + +# Ensure local source tree is importable +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from typer.testing import CliRunner # noqa: E402 + +from cleveragents.application.container import reset_container # noqa: E402 +from cleveragents.cli.commands import session as session_mod # noqa: E402 +from cleveragents.config.settings import Settings # noqa: E402 + +runner = CliRunner() + + +def setup_real_di() -> str: + """Prepare the environment for real DI resolution. + + Returns the path to a temporary database file (caller must clean up). + """ + session_mod._service = None + # Reset any stale container from a prior test. + reset_container() + fd, db_path = tempfile.mkstemp(suffix=".db") + os.close(fd) + os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{db_path}" + return db_path + + +def teardown(db_path: str) -> None: + """Clean up after a test.""" + session_mod._service = None + os.environ.pop("CLEVERAGENTS_DATABASE_URL", None) + reset_container() + # Reset the Settings singleton so stale database URLs do not leak. + Settings._instance = None + Path(db_path).unlink(missing_ok=True) diff --git a/robot/helper_tdd_session_list_di.py b/robot/helper_tdd_session_list_di.py index 68e89f6d1..0ff06e721 100644 --- a/robot/helper_tdd_session_list_di.py +++ b/robot/helper_tdd_session_list_di.py @@ -9,52 +9,23 @@ pass/fail inversion while the bug remains open. from __future__ import annotations -import contextlib -import os import sys -import tempfile from collections.abc import Callable from pathlib import Path -# Ensure local source tree is importable -_SRC = str(Path(__file__).resolve().parents[1] / "src") -if _SRC not in sys.path: - sys.path.insert(0, _SRC) +# Ensure local source tree AND robot/ directory are importable. +_ROOT = Path(__file__).resolve().parents[1] +_SRC = str(_ROOT / "src") +_ROBOT = str(_ROOT / "robot") +for _p in (_SRC, _ROBOT): + if _p not in sys.path: + sys.path.insert(0, _p) -from typer.testing import CliRunner # noqa: E402 +from helper_tdd_session_di_common import runner, setup_real_di, teardown # noqa: E402 from cleveragents.cli.commands import session as session_mod # noqa: E402 from cleveragents.cli.commands.session import app as session_app # noqa: E402 -runner = CliRunner() - - -def _setup_real_di() -> str: - """Prepare the environment for real DI resolution. - - Returns the path to a temporary database file (caller must clean up). - """ - session_mod._service = None - fd, db_path = tempfile.mkstemp(suffix=".db") - os.close(fd) - os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{db_path}" - return db_path - - -def _teardown(db_path: str) -> None: - """Clean up after a test.""" - session_mod._service = None - os.environ.pop("CLEVERAGENTS_DATABASE_URL", None) - try: - from cleveragents.application.container import reset_container - - reset_container() - except ImportError: - pass - with contextlib.suppress(OSError): - os.unlink(db_path) - - # --------------------------------------------------------------------------- # Subcommands # --------------------------------------------------------------------------- @@ -66,7 +37,7 @@ def list_di_error() -> None: Exits 0 with sentinel when the command succeeds (bug fixed). Exits 1 when the command fails (bug still present). """ - db_path = _setup_real_di() + db_path = setup_real_di() try: result = runner.invoke(session_app, ["list"]) if result.exit_code == 0: @@ -78,7 +49,7 @@ def list_di_error() -> None: ) sys.exit(1) finally: - _teardown(db_path) + teardown(db_path) def service_resolution() -> None: @@ -87,7 +58,7 @@ def service_resolution() -> None: Exits 0 with sentinel when the service resolves (bug fixed). Exits 1 when resolution raises AttributeError (bug still present). """ - db_path = _setup_real_di() + db_path = setup_real_di() try: try: session_mod._get_session_service() @@ -100,7 +71,7 @@ def service_resolution() -> None: print("tdd-session-list-service-resolution-ok") finally: - _teardown(db_path) + teardown(db_path) def list_json() -> None: @@ -109,7 +80,7 @@ def list_json() -> None: Exits 0 with sentinel when the command succeeds (bug fixed). Exits 1 when the command fails (bug still present). """ - db_path = _setup_real_di() + db_path = setup_real_di() try: result = runner.invoke(session_app, ["list", "--format", "json"]) if result.exit_code == 0: @@ -121,7 +92,7 @@ def list_json() -> None: ) sys.exit(1) finally: - _teardown(db_path) + teardown(db_path) # --------------------------------------------------------------------------- @@ -136,7 +107,10 @@ _COMMANDS: dict[str, Callable[[], None]] = { 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) + 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_session_create_di.robot b/robot/tdd_session_create_di.robot new file mode 100644 index 000000000..86a4130b0 --- /dev/null +++ b/robot/tdd_session_create_di.robot @@ -0,0 +1,38 @@ +*** Settings *** +Documentation TDD Bug #570 — session create DI container missing db provider +... Integration smoke tests verifying that the session create command +... fails due to the DI container lacking a ``db`` provider. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tdd_session_create_di.py + +*** Test Cases *** +TDD Session Create DI Error Via CLI + [Documentation] Verify that ``session create`` triggers the DI db error + [Tags] tdd_bug tdd_bug_570 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} create-di-error cwd=${WORKSPACE} timeout=30s + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-session-create-di-error-ok + +TDD Session Create With Actor DI Error + [Documentation] Verify that ``session create --actor`` triggers the DI db error + [Tags] tdd_bug tdd_bug_570 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} create-actor cwd=${WORKSPACE} timeout=30s + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-session-create-actor-ok + +TDD Session Create DI JSON Output + [Documentation] Verify that ``session create --format json`` fails due to DI db error + [Tags] tdd_bug tdd_bug_570 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} create-json cwd=${WORKSPACE} timeout=30s + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-session-create-json-ok diff --git a/robot/tdd_session_list_di.robot b/robot/tdd_session_list_di.robot index 9675aeae9..a4f1b6a20 100644 --- a/robot/tdd_session_list_di.robot +++ b/robot/tdd_session_list_di.robot @@ -14,7 +14,7 @@ ${HELPER} ${CURDIR}/helper_tdd_session_list_di.py TDD Session List DI Error Via CLI [Documentation] Verify that ``session list`` succeeds via the real DI path [Tags] tdd_bug tdd_bug_554 tdd_expected_fail - ${result}= Run Process ${PYTHON} ${HELPER} list-di-error cwd=${WORKSPACE} + ${result}= Run Process ${PYTHON} ${HELPER} list-di-error cwd=${WORKSPACE} timeout=30s Log ${result.stdout} Log ${result.stderr} Should Be Equal As Integers ${result.rc} 0 @@ -23,7 +23,7 @@ TDD Session List DI Error Via CLI TDD Session List DI Service Resolution [Documentation] Verify that ``_get_session_service()`` resolves a valid service [Tags] tdd_bug tdd_bug_554 tdd_expected_fail - ${result}= Run Process ${PYTHON} ${HELPER} service-resolution cwd=${WORKSPACE} + ${result}= Run Process ${PYTHON} ${HELPER} service-resolution cwd=${WORKSPACE} timeout=30s Log ${result.stdout} Log ${result.stderr} Should Be Equal As Integers ${result.rc} 0 @@ -32,7 +32,7 @@ TDD Session List DI Service Resolution TDD Session List DI JSON Output [Documentation] Verify that ``session list --format json`` succeeds via the real DI path [Tags] tdd_bug tdd_bug_554 tdd_expected_fail - ${result}= Run Process ${PYTHON} ${HELPER} list-json cwd=${WORKSPACE} + ${result}= Run Process ${PYTHON} ${HELPER} list-json cwd=${WORKSPACE} timeout=30s Log ${result.stdout} Log ${result.stderr} Should Be Equal As Integers ${result.rc} 0