From 06bbe48a9c059009e047ea0c1880850c3d153d68 Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Mon, 9 Mar 2026 21:15:46 +0000 Subject: [PATCH 1/3] test(session): add TDD failing tests for session list DI error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement TDD bug-capture tests for bug #554 where `agents session list` fails because `_get_session_service()` calls `container.db()` which does not exist on the DI Container class (AttributeError). Behave BDD scenarios tagged @tdd_bug @tdd_bug_554 @tdd_expected_fail exercise the real DI path (no mocks) and assert correct behavior. The @tdd_expected_fail handler in environment.py inverts failed→passed while the bug is present, keeping CI green. Also adds: - @tdd_expected_fail infrastructure in features/environment.py (tag validation + status inversion in after_scenario hook) - behave-parallel exit logic fix to use summary-based failure detection (compatible with TDD status inversion) - Robot Framework integration smoke tests with self-inverting helper - ASV benchmark baseline for session list command throughput ISSUES CLOSED: #630 --- benchmarks/tdd_session_list_di_bench.py | 77 ++++++++++ features/environment.py | 73 ++++++++++ features/steps/tdd_session_list_di_steps.py | 113 +++++++++++++++ features/tdd_session_list_di.feature | 29 ++++ noxfile.py | 38 ++++- robot/helper_tdd_session_list_di.py | 147 ++++++++++++++++++++ robot/tdd_session_list_di.robot | 40 ++++++ 7 files changed, 512 insertions(+), 5 deletions(-) create mode 100644 benchmarks/tdd_session_list_di_bench.py create mode 100644 features/steps/tdd_session_list_di_steps.py create mode 100644 features/tdd_session_list_di.feature create mode 100644 robot/helper_tdd_session_list_di.py create mode 100644 robot/tdd_session_list_di.robot diff --git a/benchmarks/tdd_session_list_di_bench.py b/benchmarks/tdd_session_list_di_bench.py new file mode 100644 index 000000000..c323ee83b --- /dev/null +++ b/benchmarks/tdd_session_list_di_bench.py @@ -0,0 +1,77 @@ +"""ASV benchmarks for TDD Bug #554 — session list CLI throughput. + +Measures the performance of the session list 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 + +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 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: + """Benchmark session list command throughput (TDD bug #554 baseline).""" + + def setup(self) -> None: + self._svc = MagicMock() + self._svc.list.return_value = [ + _mock_session(actor_name=f"openai/gpt-{i}") for i in range(50) + ] + session_mod._service = self._svc + + def teardown(self) -> None: + session_mod._service = None + + def time_list_rich(self) -> None: + """Benchmark listing sessions with default rich format.""" + _runner.invoke(session_app, ["list"]) + + def time_list_json(self) -> None: + """Benchmark listing sessions with JSON format.""" + _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 + self._svc.list.return_value = [ + _mock_session(actor_name=f"openai/gpt-{i}") for i in range(50) + ] diff --git a/features/environment.py b/features/environment.py index e71e026f8..a18a95400 100644 --- a/features/environment.py +++ b/features/environment.py @@ -2,12 +2,16 @@ import contextlib import os +import re import shutil import sys import tempfile from pathlib import Path from typing import Any +from behave.model import Scenario +from behave.model_core import Status + LANGSMITH_ENV_VARS = [ "CLEVERAGENTS_LANGSMITH_ENABLED", "CLEVERAGENTS_LANGSMITH_PROJECT", @@ -305,8 +309,77 @@ def before_scenario(context, scenario): pass # Container not needed for all tests +# --------------------------------------------------------------------------- +# TDD expected-fail tag handler (see CONTRIBUTING.md §TDD Bug Test Tags) +# --------------------------------------------------------------------------- + +_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``. + + 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: + + * **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) + + if "tdd_expected_fail" not in tags: + return + + # --- tag validation --------------------------------------------------- + if "tdd_bug" not in tags: + scenario.set_status(Status.failed) + sys.stderr.write( + f"TDD TAG ERROR: {scenario.name!r} — " + "@tdd_expected_fail requires @tdd_bug tag\n" + ) + return + + if not any(_TDD_BUG_N_RE.match(t) for t in tags): + scenario.set_status(Status.failed) + sys.stderr.write( + f"TDD TAG ERROR: {scenario.name!r} — " + "@tdd_expected_fail requires at least one @tdd_bug_ tag\n" + ) + return + + # --- status inversion ------------------------------------------------- + if scenario.status == Status.failed: + # Bug still present — expected. Mark scenario and its failed/skipped + # steps as passed so that summary counts are accurate. + scenario.clear_status() + scenario.set_status(Status.passed) + for step in scenario.steps: + if step.status in (Status.failed, Status.skipped): + step.status = Status.passed + elif scenario.status == Status.passed: + # Bug was fixed but @tdd_expected_fail was not removed — error. + scenario.set_status(Status.failed) + sys.stderr.write( + f"TDD TAG ERROR: {scenario.name!r} — " + "scenario passed but still carries @tdd_expected_fail; " + "remove the tag now that the bug is fixed\n" + ) + + 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) + # Return to original directory first if hasattr(context, "original_cwd"): os.chdir(context.original_cwd) diff --git a/features/steps/tdd_session_list_di_steps.py b/features/steps/tdd_session_list_di_steps.py new file mode 100644 index 000000000..06e6a017b --- /dev/null +++ b/features/steps/tdd_session_list_di_steps.py @@ -0,0 +1,113 @@ +"""Step definitions for TDD Bug #554 — session list 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: these tests +**pass** CI while the bug is present and will **fail** once the bug is fixed +(signalling that the tag should be removed). +""" + +from __future__ import annotations + +import contextlib +import json +import os +import tempfile + +from behave import given, 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 + + +@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) + + +@when("I invoke the session list command") +def step_invoke_list(context: Context) -> None: + """Invoke ``session list`` through the real CLI app.""" + context.result = context.runner.invoke(session_app, ["list"]) + + +@when("I request the session service from the DI container") +def step_request_service(context: Context) -> None: + """Call ``_get_session_service()`` directly to test the DI wiring.""" + try: + context.session_service = session_mod._get_session_service() + context.service_error = None + except AttributeError as exc: + context.session_service = None + context.service_error = exc + + +@when("I invoke the session list command with format json") +def step_invoke_list_json(context: Context) -> None: + """Invoke ``session list --format json`` through the real CLI app.""" + 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/tdd_session_list_di.feature b/features/tdd_session_list_di.feature new file mode 100644 index 000000000..a9aa42131 --- /dev/null +++ b/features/tdd_session_list_di.feature @@ -0,0 +1,29 @@ +@tdd_bug @tdd_bug_554 +Feature: TDD Bug #554 — session list DI container missing db provider + As a developer + I want to verify that `agents session list` 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 that `_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 list command succeeds via DI container + Given a CLI runner using the real session DI path + When I invoke the session list command + Then the session list command should exit successfully + + @tdd_expected_fail + Scenario: Session list DI path resolves a SessionService + Given a CLI runner using the real session DI path + When I request the session service from the DI container + Then the session service should be a valid SessionService instance + + @tdd_expected_fail + Scenario: Session list command produces structured output via DI + Given a CLI runner using the real session DI path + When I invoke the session list command with format json + Then the session list command should exit successfully + And the session list output should be valid JSON diff --git a/noxfile.py b/noxfile.py index 8489678f6..fb395e3a7 100644 --- a/noxfile.py +++ b/noxfile.py @@ -251,6 +251,18 @@ def _has_failures(total): ) +def _no_scenarios_ran(total): + """Return True when the runner collected zero scenario results. + + This catches runner-level crashes (e.g. ``before_all`` failure) that + prevent any scenario from executing. Without this guard the summary + would contain all-zero counters, ``_has_failures()`` would return + ``False``, and CI would silently pass a broken suite. + """ + s = total["scenarios"] + return s["passed"] + s["failed"] + s["errors"] + s["skipped"] == 0 + + # --------------------------------------------------------------------------- # Feature discovery # --------------------------------------------------------------------------- @@ -371,7 +383,7 @@ def main(argv=None): if processes <= 1 or coverage_mode or len(feature_paths) == 1: # ---- sequential in-process mode ---- - failed, total = _run_features_inprocess(feature_paths, other_args) + _, total = _run_features_inprocess(feature_paths, other_args) else: # ---- parallel in-process mode (multiprocessing fork) ---- # Pre-import heavy modules so forked children get them for free. @@ -398,21 +410,37 @@ def main(argv=None): [(chunk, other_args) for chunk in chunks], ) - failed = False summaries = [] - for worker_failed, stdout, stderr, summary in results: + for _worker_failed, stdout, stderr, summary in results: if stdout: print(stdout, end="") if stderr: print(stderr, end="", file=sys.stderr) - failed = failed or worker_failed summaries.append(summary) total = _merge_summaries(summaries) wall = time.monotonic() - start _print_overall_summary(total, wall_seconds=wall) - if failed or _has_failures(total): + # Use the summary-based check rather than the raw runner ``failed`` + # boolean. The ``@tdd_expected_fail`` handler in environment.py + # inverts scenario statuses for TDD bug-capture tests, but behave's + # ``runner.run()`` tracks step failures in a local variable that + # cannot be updated by after_scenario hooks. Relying solely on the + # summary (which reflects the corrected scenario statuses) ensures + # that TDD-inverted scenarios do not cause a spurious exit-code 1. + if _has_failures(total): + sys.exit(1) + + # Safety net: if features were requested but zero scenarios ran, the + # runner crashed before executing any scenario (e.g. ``before_all`` + # failure). Treat this as a failure so CI does not silently pass. + if feature_paths and _no_scenarios_ran(total): + print( + "ERROR: features were requested but no scenarios ran — " + "possible runner-level crash.", + file=sys.stderr, + ) sys.exit(1) diff --git a/robot/helper_tdd_session_list_di.py b/robot/helper_tdd_session_list_di.py new file mode 100644 index 000000000..d2bf214f8 --- /dev/null +++ b/robot/helper_tdd_session_list_di.py @@ -0,0 +1,147 @@ +"""Helper script for tdd_session_list_di.robot smoke tests. + +Each subcommand exercises the real DI path (no mocks) to reproduce bug #554. +Because Robot Framework does not yet have ``@tdd_expected_fail`` inversion +logic (issue #628), this helper **inverts the result itself**: it prints the +sentinel and exits 0 when the bug is detected (expected), and exits 1 if +the bug appears to be fixed (meaning the ``@tdd_expected_fail`` tag should +be removed). +""" + +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) + +from typer.testing import CliRunner # 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 +# --------------------------------------------------------------------------- + + +def list_di_error() -> None: + """Invoke ``session list`` through the real DI path. + + Expected: the command fails because ``container.db()`` does not exist. + """ + db_path = _setup_real_di() + try: + result = runner.invoke(session_app, ["list"]) + if result.exit_code != 0: + # Bug present — expected failure. + print("tdd-session-list-di-error-ok") + else: + print( + "ERROR: session list succeeded — bug #554 appears fixed. " + "Remove @tdd_expected_fail tag.", + file=sys.stderr, + ) + sys.exit(1) + finally: + _teardown(db_path) + + +def service_resolution() -> None: + """Call ``_get_session_service()`` to verify DI resolution. + + Expected: raises ``AttributeError`` because ``container.db()`` is missing. + """ + db_path = _setup_real_di() + try: + try: + session_mod._get_session_service() + except AttributeError: + # Bug present — expected failure. + print("tdd-session-list-service-resolution-ok") + return + + print( + "ERROR: _get_session_service() succeeded — bug #554 appears fixed. " + "Remove @tdd_expected_fail tag.", + file=sys.stderr, + ) + sys.exit(1) + finally: + _teardown(db_path) + + +def list_json() -> None: + """Invoke ``session list --format json`` through the real DI path. + + Expected: the command fails because ``container.db()`` does not exist. + """ + db_path = _setup_real_di() + try: + result = runner.invoke(session_app, ["list", "--format", "json"]) + if result.exit_code != 0: + # Bug present — expected failure. + print("tdd-session-list-json-ok") + else: + print( + "ERROR: session list --format json succeeded — bug #554 appears " + "fixed. Remove @tdd_expected_fail tag.", + file=sys.stderr, + ) + sys.exit(1) + finally: + _teardown(db_path) + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "list-di-error": list_di_error, + "service-resolution": service_resolution, + "list-json": list_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/tdd_session_list_di.robot b/robot/tdd_session_list_di.robot new file mode 100644 index 000000000..26f3ed726 --- /dev/null +++ b/robot/tdd_session_list_di.robot @@ -0,0 +1,40 @@ +*** Settings *** +Documentation TDD Bug #554 — session list DI container missing db provider +... Integration smoke tests verifying that the session list command +... fails due to the DI container lacking a ``db`` provider. +... These tests are tagged ``tdd_expected_fail`` and are expected +... to fail until bug #554 is fixed. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tdd_session_list_di.py + +*** Test Cases *** +TDD Session List DI Error Via CLI + [Documentation] Verify that ``session list`` triggers the DI db error + [Tags] tdd_bug tdd_bug_554 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} list-di-error cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-session-list-di-error-ok + +TDD Session List DI Service Resolution + [Documentation] Verify that ``_get_session_service()`` raises AttributeError due to missing db provider + [Tags] tdd_bug tdd_bug_554 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} service-resolution cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-session-list-service-resolution-ok + +TDD Session List DI JSON Output + [Documentation] Verify that ``session list --format json`` fails due to DI db error + [Tags] tdd_bug tdd_bug_554 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} list-json cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-session-list-json-ok -- 2.52.0 From 3c7915099f7fe65f51d0d67dd269b369332fc04d Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Wed, 11 Mar 2026 00:07:07 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix(tdd):=20remove=20tdd=5Fexpected=5Ffail?= =?UTF-8?q?=20tags=20=E2=80=94=20bug=20#554=20is=20fixed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge from master brought in the DI container fix for bug #554. All 16 session-list TDD tests now pass, so the tdd_expected_fail tags are removed per the TDD listener protocol. --- features/session_list_error.feature | 25 ++++++++++++------------- features/tdd_session_list_di.feature | 3 --- robot/tdd_session_list_di.robot | 10 ++++------ 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/features/session_list_error.feature b/features/session_list_error.feature index 3be95a257..f09fda174 100644 --- a/features/session_list_error.feature +++ b/features/session_list_error.feature @@ -1,6 +1,5 @@ -# TDD tests for bug #554 — expected to fail until the DI container fix lands. -# Once the fix is applied, remove the @tdd_expected_fail tags and verify all -# scenarios pass. +# TDD tests for bug #554 — the DI container fix has landed. +# The @tdd_expected_fail tags have been removed; all scenarios now pass. Feature: Session list command handles missing database gracefully As a developer using the agents CLI I want "agents session list" to work after a fresh init @@ -9,28 +8,28 @@ Feature: Session list command handles missing database gracefully Background: Given a session-list-error CLI runner using the real DI path - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Session list returns empty list when no sessions exist When I invoke session-list-error list with default format Then the session-list-error command should exit successfully And the session-list-error output should contain "No sessions found" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Session list after init does not raise DI error When I invoke session-list-error list with default format Then the session-list-error command should exit successfully And the session-list-error output should not contain "AttributeError" And the session-list-error output should not contain "INTERNAL" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Session list returns sessions after creation via service Given a session-list-error service with a pre-populated session When I invoke session-list-error list with default format Then the session-list-error command should exit successfully And the session-list-error output should contain "Sessions (" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Session list works with rich output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "rich" @@ -38,21 +37,21 @@ Feature: Session list command handles missing database gracefully And the session-list-error output should contain "Sessions (" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Session list works with JSON output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "json" Then the session-list-error command should exit successfully And the session-list-error output should be valid JSON containing "sessions" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Session list works with plain output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "plain" Then the session-list-error command should exit successfully And the session-list-error output should contain "Sessions (" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Session list works with YAML output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "yaml" @@ -63,21 +62,21 @@ Feature: Session list command handles missing database gracefully # with explicit output formats. The production code currently bypasses # --format for empty lists, so these document the expected behaviour. - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Empty session list with JSON format produces valid JSON When I invoke session-list-error list with format "json" Then the session-list-error command should exit successfully And the session-list-error output should be valid JSON containing "sessions" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Empty session list with YAML format produces valid YAML When I invoke session-list-error list with format "yaml" Then the session-list-error command should exit successfully And the session-list-error output should be valid YAML containing "sessions" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 @tdd_expected_fail + @tdd_bug @tdd_bug_554 Scenario: Empty session list with plain format does not error When I invoke session-list-error list with format "plain" Then the session-list-error command should exit successfully diff --git a/features/tdd_session_list_di.feature b/features/tdd_session_list_di.feature index a9aa42131..6384dcd71 100644 --- a/features/tdd_session_list_di.feature +++ b/features/tdd_session_list_di.feature @@ -9,19 +9,16 @@ Feature: TDD Bug #554 — session list DI container missing db provider `container.db()`, but the Container class has no `db` provider, causing an AttributeError at runtime. - @tdd_expected_fail Scenario: Session list command succeeds via DI container Given a CLI runner using the real session DI path When I invoke the session list command Then the session list command should exit successfully - @tdd_expected_fail Scenario: Session list DI path resolves a SessionService Given a CLI runner using the real session DI path When I request the session service from the DI container Then the session service should be a valid SessionService instance - @tdd_expected_fail Scenario: Session list command produces structured output via DI Given a CLI runner using the real session DI path When I invoke the session list command with format json diff --git a/robot/tdd_session_list_di.robot b/robot/tdd_session_list_di.robot index 26f3ed726..dae35abd3 100644 --- a/robot/tdd_session_list_di.robot +++ b/robot/tdd_session_list_di.robot @@ -1,9 +1,7 @@ *** Settings *** Documentation TDD Bug #554 — session list DI container missing db provider ... Integration smoke tests verifying that the session list command -... fails due to the DI container lacking a ``db`` provider. -... These tests are tagged ``tdd_expected_fail`` and are expected -... to fail until bug #554 is fixed. +... works correctly now that bug #554 is fixed. Resource ${CURDIR}/common.resource Suite Setup Setup Test Environment Suite Teardown Cleanup Test Environment @@ -14,7 +12,7 @@ ${HELPER} ${CURDIR}/helper_tdd_session_list_di.py *** Test Cases *** TDD Session List DI Error Via CLI [Documentation] Verify that ``session list`` triggers the DI db error - [Tags] tdd_bug tdd_bug_554 tdd_expected_fail + [Tags] tdd_bug tdd_bug_554 ${result}= Run Process ${PYTHON} ${HELPER} list-di-error cwd=${WORKSPACE} Log ${result.stdout} Log ${result.stderr} @@ -23,7 +21,7 @@ TDD Session List DI Error Via CLI TDD Session List DI Service Resolution [Documentation] Verify that ``_get_session_service()`` raises AttributeError due to missing db provider - [Tags] tdd_bug tdd_bug_554 tdd_expected_fail + [Tags] tdd_bug tdd_bug_554 ${result}= Run Process ${PYTHON} ${HELPER} service-resolution cwd=${WORKSPACE} Log ${result.stdout} Log ${result.stderr} @@ -32,7 +30,7 @@ TDD Session List DI Service Resolution TDD Session List DI JSON Output [Documentation] Verify that ``session list --format json`` fails due to DI db error - [Tags] tdd_bug tdd_bug_554 tdd_expected_fail + [Tags] tdd_bug tdd_bug_554 ${result}= Run Process ${PYTHON} ${HELPER} list-json cwd=${WORKSPACE} Log ${result.stdout} Log ${result.stderr} -- 2.52.0 From bed7072dde33bd6cc731bbff6e2e5a38d5cf4df2 Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Wed, 11 Mar 2026 00:40:35 +0000 Subject: [PATCH 3/3] fix(tdd): restore tdd_expected_fail tags and fix Robot helper inversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug #554 is NOT yet fixed — the DI container still lacks a db provider. The previous commit incorrectly removed tdd_expected_fail tags, exposing the real failures in Behave unit tests (13 scenarios). Root cause: the Robot helper had its own pass/fail inversion (a workaround from before tdd_expected_fail_listener existed). The master merge brought in the listener, causing double inversion that made Robot tests appear to pass. This commit: - Rewrites the helper to report real outcomes (exit 0 = bug fixed) - Restores tdd_expected_fail on all 16 Robot + Behave scenarios - Lets the listener (Robot) and environment.py (Behave) handle inversion --- features/session_list_error.feature | 25 ++++++++------- features/tdd_session_list_di.feature | 3 ++ robot/helper_tdd_session_list_di.py | 47 +++++++++++++--------------- robot/tdd_session_list_di.robot | 15 ++++----- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/features/session_list_error.feature b/features/session_list_error.feature index f09fda174..3be95a257 100644 --- a/features/session_list_error.feature +++ b/features/session_list_error.feature @@ -1,5 +1,6 @@ -# TDD tests for bug #554 — the DI container fix has landed. -# The @tdd_expected_fail tags have been removed; all scenarios now pass. +# TDD tests for bug #554 — expected to fail until the DI container fix lands. +# Once the fix is applied, remove the @tdd_expected_fail tags and verify all +# scenarios pass. Feature: Session list command handles missing database gracefully As a developer using the agents CLI I want "agents session list" to work after a fresh init @@ -8,28 +9,28 @@ Feature: Session list command handles missing database gracefully Background: Given a session-list-error CLI runner using the real DI path - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Session list returns empty list when no sessions exist When I invoke session-list-error list with default format Then the session-list-error command should exit successfully And the session-list-error output should contain "No sessions found" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Session list after init does not raise DI error When I invoke session-list-error list with default format Then the session-list-error command should exit successfully And the session-list-error output should not contain "AttributeError" And the session-list-error output should not contain "INTERNAL" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Session list returns sessions after creation via service Given a session-list-error service with a pre-populated session When I invoke session-list-error list with default format Then the session-list-error command should exit successfully And the session-list-error output should contain "Sessions (" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Session list works with rich output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "rich" @@ -37,21 +38,21 @@ Feature: Session list command handles missing database gracefully And the session-list-error output should contain "Sessions (" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Session list works with JSON output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "json" Then the session-list-error command should exit successfully And the session-list-error output should be valid JSON containing "sessions" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Session list works with plain output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "plain" Then the session-list-error command should exit successfully And the session-list-error output should contain "Sessions (" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Session list works with YAML output format Given a session-list-error service with a pre-populated session When I invoke session-list-error list with format "yaml" @@ -62,21 +63,21 @@ Feature: Session list command handles missing database gracefully # with explicit output formats. The production code currently bypasses # --format for empty lists, so these document the expected behaviour. - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Empty session list with JSON format produces valid JSON When I invoke session-list-error list with format "json" Then the session-list-error command should exit successfully And the session-list-error output should be valid JSON containing "sessions" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Empty session list with YAML format produces valid YAML When I invoke session-list-error list with format "yaml" Then the session-list-error command should exit successfully And the session-list-error output should be valid YAML containing "sessions" And the session-list-error output should not contain "AttributeError" - @tdd_bug @tdd_bug_554 + @tdd_bug @tdd_bug_554 @tdd_expected_fail Scenario: Empty session list with plain format does not error When I invoke session-list-error list with format "plain" Then the session-list-error command should exit successfully diff --git a/features/tdd_session_list_di.feature b/features/tdd_session_list_di.feature index 6384dcd71..a9aa42131 100644 --- a/features/tdd_session_list_di.feature +++ b/features/tdd_session_list_di.feature @@ -9,16 +9,19 @@ Feature: TDD Bug #554 — session list DI container missing db provider `container.db()`, but the Container class has no `db` provider, causing an AttributeError at runtime. + @tdd_expected_fail Scenario: Session list command succeeds via DI container Given a CLI runner using the real session DI path When I invoke the session list command Then the session list command should exit successfully + @tdd_expected_fail Scenario: Session list DI path resolves a SessionService Given a CLI runner using the real session DI path When I request the session service from the DI container Then the session service should be a valid SessionService instance + @tdd_expected_fail Scenario: Session list command produces structured output via DI Given a CLI runner using the real session DI path When I invoke the session list command with format json diff --git a/robot/helper_tdd_session_list_di.py b/robot/helper_tdd_session_list_di.py index d2bf214f8..68e89f6d1 100644 --- a/robot/helper_tdd_session_list_di.py +++ b/robot/helper_tdd_session_list_di.py @@ -1,11 +1,10 @@ """Helper script for tdd_session_list_di.robot smoke tests. Each subcommand exercises the real DI path (no mocks) to reproduce bug #554. -Because Robot Framework does not yet have ``@tdd_expected_fail`` inversion -logic (issue #628), this helper **inverts the result itself**: it prints the -sentinel and exits 0 when the bug is detected (expected), and exits 1 if -the bug appears to be fixed (meaning the ``@tdd_expected_fail`` tag should -be removed). +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 @@ -64,18 +63,17 @@ def _teardown(db_path: str) -> None: def list_di_error() -> None: """Invoke ``session list`` through the real DI path. - Expected: the command fails because ``container.db()`` does not exist. + 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, ["list"]) - if result.exit_code != 0: - # Bug present — expected failure. + if result.exit_code == 0: print("tdd-session-list-di-error-ok") else: print( - "ERROR: session list succeeded — bug #554 appears fixed. " - "Remove @tdd_expected_fail tag.", + f"session list failed with exit code {result.exit_code}", file=sys.stderr, ) sys.exit(1) @@ -86,23 +84,21 @@ def list_di_error() -> None: def service_resolution() -> None: """Call ``_get_session_service()`` to verify DI resolution. - Expected: raises ``AttributeError`` because ``container.db()`` is missing. + Exits 0 with sentinel when the service resolves (bug fixed). + Exits 1 when resolution raises AttributeError (bug still present). """ db_path = _setup_real_di() try: try: session_mod._get_session_service() - except AttributeError: - # Bug present — expected failure. - print("tdd-session-list-service-resolution-ok") - return + except AttributeError as exc: + print( + f"_get_session_service() raised {exc!r}", + file=sys.stderr, + ) + sys.exit(1) - print( - "ERROR: _get_session_service() succeeded — bug #554 appears fixed. " - "Remove @tdd_expected_fail tag.", - file=sys.stderr, - ) - sys.exit(1) + print("tdd-session-list-service-resolution-ok") finally: _teardown(db_path) @@ -110,18 +106,17 @@ def service_resolution() -> None: def list_json() -> None: """Invoke ``session list --format json`` through the real DI path. - Expected: the command fails because ``container.db()`` does not exist. + 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, ["list", "--format", "json"]) - if result.exit_code != 0: - # Bug present — expected failure. + if result.exit_code == 0: print("tdd-session-list-json-ok") else: print( - "ERROR: session list --format json succeeded — bug #554 appears " - "fixed. Remove @tdd_expected_fail tag.", + f"session list --format json failed with exit code {result.exit_code}", file=sys.stderr, ) sys.exit(1) diff --git a/robot/tdd_session_list_di.robot b/robot/tdd_session_list_di.robot index dae35abd3..9675aeae9 100644 --- a/robot/tdd_session_list_di.robot +++ b/robot/tdd_session_list_di.robot @@ -1,7 +1,8 @@ *** Settings *** Documentation TDD Bug #554 — session list DI container missing db provider ... Integration smoke tests verifying that the session list command -... works correctly now that bug #554 is fixed. +... succeeds once the DI container has a proper ``db`` provider. +... Tagged ``tdd_expected_fail`` until bug #554 is resolved. Resource ${CURDIR}/common.resource Suite Setup Setup Test Environment Suite Teardown Cleanup Test Environment @@ -11,8 +12,8 @@ ${HELPER} ${CURDIR}/helper_tdd_session_list_di.py *** Test Cases *** TDD Session List DI Error Via CLI - [Documentation] Verify that ``session list`` triggers the DI db error - [Tags] tdd_bug tdd_bug_554 + [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} Log ${result.stdout} Log ${result.stderr} @@ -20,8 +21,8 @@ TDD Session List DI Error Via CLI Should Contain ${result.stdout} tdd-session-list-di-error-ok TDD Session List DI Service Resolution - [Documentation] Verify that ``_get_session_service()`` raises AttributeError due to missing db provider - [Tags] tdd_bug tdd_bug_554 + [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} Log ${result.stdout} Log ${result.stderr} @@ -29,8 +30,8 @@ TDD Session List DI Service Resolution Should Contain ${result.stdout} tdd-session-list-service-resolution-ok TDD Session List DI JSON Output - [Documentation] Verify that ``session list --format json`` fails due to DI db error - [Tags] tdd_bug tdd_bug_554 + [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} Log ${result.stdout} Log ${result.stderr} -- 2.52.0