test(session): add TDD failing tests for session create DI error
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 18s
CI / security (pull_request) Successful in 36s
CI / typecheck (pull_request) Successful in 38s
CI / unit_tests (pull_request) Successful in 2m42s
CI / integration_tests (pull_request) Successful in 3m13s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 6m15s
CI / benchmark-regression (pull_request) Successful in 34m51s
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 18s
CI / security (pull_request) Successful in 36s
CI / typecheck (pull_request) Successful in 38s
CI / unit_tests (pull_request) Successful in 2m42s
CI / integration_tests (pull_request) Successful in 3m13s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 6m15s
CI / benchmark-regression (pull_request) Successful in 34m51s
Implement TDD bug-capture tests for bug #570 where `agents session create` fails because `_get_session_service()` calls `container.db()` which does not exist on the DI Container class (AttributeError). Same root cause as bug #554. Behave BDD scenarios tagged @tdd_bug @tdd_bug_570 @tdd_expected_fail exercise the real DI path (no mocks). Includes Robot Framework integration smoke tests with self-inverting helper and ASV benchmark baseline. ISSUES CLOSED: #631
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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(),
|
||||
)
|
||||
@@ -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"])
|
||||
@@ -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)
|
||||
]
|
||||
|
||||
+36
-14
@@ -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_<N>`` **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_<N>`` 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_<N>`` 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_<N> 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"):
|
||||
|
||||
@@ -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="<infrastructure-test>",
|
||||
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="<infrastructure-test>",
|
||||
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}"
|
||||
)
|
||||
@@ -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"])
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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"
|
||||
@@ -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
|
||||
+1
-1
@@ -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
|
||||
|
||||
@@ -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()
|
||||
@@ -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)
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user