test(cli): add failing TDD test for session create silently suppressing facade dispatch exceptions
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Failing after 57s
CI / lint (pull_request) Successful in 1m16s
CI / typecheck (pull_request) Successful in 1m29s
CI / quality (pull_request) Successful in 1m9s
CI / build (pull_request) Successful in 48s
CI / push-validation (pull_request) Successful in 29s
CI / helm (pull_request) Successful in 45s
CI / security (pull_request) Successful in 1m50s
CI / integration_tests (pull_request) Successful in 4m16s
CI / e2e_tests (pull_request) Failing after 6m14s
CI / unit_tests (pull_request) Successful in 7m46s
CI / docker (pull_request) Successful in 1m52s
CI / coverage (pull_request) Successful in 14m44s
CI / status-check (pull_request) Failing after 9s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Failing after 57s
CI / lint (pull_request) Successful in 1m16s
CI / typecheck (pull_request) Successful in 1m29s
CI / quality (pull_request) Successful in 1m9s
CI / build (pull_request) Successful in 48s
CI / push-validation (pull_request) Successful in 29s
CI / helm (pull_request) Successful in 45s
CI / security (pull_request) Successful in 1m50s
CI / integration_tests (pull_request) Successful in 4m16s
CI / e2e_tests (pull_request) Failing after 6m14s
CI / unit_tests (pull_request) Successful in 7m46s
CI / docker (pull_request) Successful in 1m52s
CI / coverage (pull_request) Successful in 14m44s
CI / status-check (pull_request) Failing after 9s
Added a new BDD feature file: features/tdd_session_create_suppress_exception.feature Added step definitions: features/steps/tdd_session_create_suppress_exception_steps.py The test captures bug #10414: session create uses contextlib.suppress(Exception) to silently discard ALL exceptions from _facade_dispatch() without any logging The test is tagged @tdd_issue @tdd_issue_10414 @tdd_expected_fail per TDD workflow The test FAILS before the fix (no logging currently) and the @tdd_expected_fail tag inverts it to a CI pass The test will pass after the fix adds _log.warning(..., exc_info=True) inside the suppress block Removed # type: ignore[import-untyped] suppressions from behave imports per project zero-tolerance policy ISSUES CLOSED: #10414
This commit is contained in:
@@ -0,0 +1,156 @@
|
||||
"""Step definitions for tdd_session_create_suppress_exception.feature.
|
||||
|
||||
This test captures bug #10414: ``session create`` in
|
||||
``src/cleveragents/cli/commands/session.py`` uses
|
||||
``contextlib.suppress(Exception)`` to silently discard ALL exceptions raised
|
||||
by ``_facade_dispatch()`` without any logging. Programming errors,
|
||||
unexpected failures, and infrastructure issues in the facade dispatch are
|
||||
completely invisible.
|
||||
|
||||
The scenario is tagged ``@tdd_expected_fail`` so the underlying assertion
|
||||
failure (confirming the bug exists) is inverted to a CI pass. Once the
|
||||
fix for #10414 is merged, remove the ``@tdd_expected_fail`` tag from the
|
||||
feature file and this test will run normally as a regression guard.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
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
|
||||
from cleveragents.domain.models.core.session import Session
|
||||
|
||||
# A distinctive error message to confirm the right exception was raised.
|
||||
_DISPATCH_ERROR_MESSAGE = "facade dispatch failed for tdd_10414"
|
||||
|
||||
|
||||
def _make_mock_session() -> MagicMock:
|
||||
"""Build a minimal mock Session object sufficient for the create command."""
|
||||
mock_session = MagicMock(spec=Session)
|
||||
mock_session.session_id = "01JTEST00000000000000000000"
|
||||
mock_session.actor_name = None
|
||||
mock_session.namespace = "default"
|
||||
mock_session.message_count = 0
|
||||
mock_session.created_at = __import__("datetime").datetime(2026, 1, 1, 0, 0, 0)
|
||||
mock_session.updated_at = __import__("datetime").datetime(2026, 1, 1, 0, 0, 0)
|
||||
return mock_session
|
||||
|
||||
|
||||
@given("a session create command with a mocked session service")
|
||||
def step_given_mocked_session_service(context: Context) -> None:
|
||||
"""Set up a CLI runner with a mocked session service.
|
||||
|
||||
The mock service returns a valid session so that the create command
|
||||
proceeds past the service call and reaches the ``_facade_dispatch``
|
||||
suppress block.
|
||||
"""
|
||||
context.runner = CliRunner()
|
||||
|
||||
mock_service = MagicMock()
|
||||
mock_service.create.return_value = _make_mock_session()
|
||||
|
||||
# Patch the module-level service so the CLI uses our mock.
|
||||
context._orig_service = session_mod._service
|
||||
session_mod._service = mock_service
|
||||
|
||||
def _cleanup() -> None:
|
||||
session_mod._service = context._orig_service
|
||||
|
||||
context.add_cleanup(_cleanup)
|
||||
|
||||
|
||||
@given("the facade dispatch is patched to raise a RuntimeError")
|
||||
def step_given_facade_dispatch_raises(context: Context) -> None:
|
||||
"""Patch ``_facade_dispatch`` so it always raises a RuntimeError.
|
||||
|
||||
This simulates a real failure in the facade layer (e.g. the A2A
|
||||
bootstrap is unavailable) and exercises the ``contextlib.suppress``
|
||||
block in the ``create`` command.
|
||||
"""
|
||||
context._facade_patcher = patch(
|
||||
"cleveragents.cli.commands.session._facade_dispatch",
|
||||
side_effect=RuntimeError(_DISPATCH_ERROR_MESSAGE),
|
||||
)
|
||||
context._facade_patcher.start()
|
||||
|
||||
def _cleanup() -> None:
|
||||
context._facade_patcher.stop()
|
||||
|
||||
context.add_cleanup(_cleanup)
|
||||
|
||||
|
||||
@when("I invoke the session create command via the CLI runner")
|
||||
def step_when_invoke_create(context: Context) -> None:
|
||||
"""Invoke ``session create`` and capture log records during the call."""
|
||||
# Capture WARNING-level log records from the session module logger.
|
||||
session_logger = logging.getLogger("cleveragents.cli.commands.session")
|
||||
handler = _CapturingHandler()
|
||||
handler.setLevel(logging.WARNING)
|
||||
session_logger.addHandler(handler)
|
||||
session_logger.setLevel(logging.WARNING)
|
||||
|
||||
try:
|
||||
context.result = context.runner.invoke(session_app, ["create"])
|
||||
finally:
|
||||
session_logger.removeHandler(handler)
|
||||
|
||||
context.captured_log_records = handler.records
|
||||
|
||||
|
||||
class _CapturingHandler(logging.Handler):
|
||||
"""A logging handler that stores all emitted records in a list."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
super().__init__()
|
||||
self.records: list[logging.LogRecord] = []
|
||||
|
||||
def emit(self, record: logging.LogRecord) -> None:
|
||||
self.records.append(record)
|
||||
|
||||
|
||||
@then("a WARNING log entry should have been emitted for the facade dispatch failure")
|
||||
def step_then_warning_logged(context: Context) -> None:
|
||||
"""Assert that a WARNING log entry was emitted when the facade dispatch raised.
|
||||
|
||||
Bug #10414: The ``contextlib.suppress(Exception)`` block in
|
||||
``session create`` does NOT log the suppressed exception. This
|
||||
assertion therefore FAILS while the bug exists, and the
|
||||
``@tdd_expected_fail`` tag causes CI to report the scenario as passed.
|
||||
|
||||
Once the fix is applied (adding ``_log.warning(..., exc_info=True)``
|
||||
inside the suppress block), this assertion will pass and the
|
||||
``@tdd_expected_fail`` tag must be removed.
|
||||
"""
|
||||
# The session create command should still succeed (non-fatal).
|
||||
assert context.result.exit_code == 0, (
|
||||
f"Expected session create to exit 0 even when facade dispatch fails, "
|
||||
f"but got exit code {context.result.exit_code}.\n"
|
||||
f"Output:\n{context.result.output}"
|
||||
)
|
||||
|
||||
# Assert that a WARNING log record was emitted.
|
||||
warning_records = [
|
||||
r for r in context.captured_log_records if r.levelno >= logging.WARNING
|
||||
]
|
||||
assert warning_records, (
|
||||
f"Bug #10414: No WARNING log record was emitted when _facade_dispatch() "
|
||||
f"raised a RuntimeError inside contextlib.suppress(). "
|
||||
f"The exception was silently discarded with no logging. "
|
||||
f"Captured records: {context.captured_log_records}"
|
||||
)
|
||||
|
||||
# Assert the log record references the facade dispatch failure.
|
||||
all_messages = " ".join(r.getMessage() for r in warning_records)
|
||||
assert _DISPATCH_ERROR_MESSAGE in all_messages or any(
|
||||
r.exc_info is not None for r in warning_records
|
||||
), (
|
||||
f"Bug #10414: WARNING log record was found but did not contain the "
|
||||
f"expected error message {_DISPATCH_ERROR_MESSAGE!r} or exc_info. "
|
||||
f"Log messages: {all_messages!r}"
|
||||
)
|
||||
@@ -0,0 +1,30 @@
|
||||
@tdd_issue @tdd_issue_10414
|
||||
Feature: TDD Issue #10414 — session create silently suppresses facade dispatch exceptions without logging
|
||||
As a developer debugging a session creation failure
|
||||
I want exceptions from _facade_dispatch() to be logged at WARNING level
|
||||
So that I can diagnose why the facade layer failed without silent data loss
|
||||
|
||||
The ``session create`` command in
|
||||
``src/cleveragents/cli/commands/session.py`` (lines 199–205) uses
|
||||
``contextlib.suppress(Exception)`` to silently discard ALL exceptions
|
||||
raised by ``_facade_dispatch()``. There is no logging call before or
|
||||
after the suppress block, making it impossible to diagnose failures in
|
||||
the facade layer.
|
||||
|
||||
The fix must add a ``_log.warning(...)`` call (with ``exc_info=True``)
|
||||
inside the suppress block so that the exception is recorded but the
|
||||
session creation remains non-fatal.
|
||||
|
||||
The scenario below is tagged ``@tdd_expected_fail`` because the assertion
|
||||
FAILS while the bug exists (no logging currently). Once the fix is
|
||||
merged, remove ``@tdd_expected_fail`` so the scenario runs as a
|
||||
regression guard.
|
||||
|
||||
See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags.
|
||||
|
||||
@tdd_issue @tdd_issue_10414 @tdd_expected_fail
|
||||
Scenario: Bug #10414 — session create logs a warning when facade dispatch raises
|
||||
Given a session create command with a mocked session service
|
||||
And the facade dispatch is patched to raise a RuntimeError
|
||||
When I invoke the session create command via the CLI runner
|
||||
Then a WARNING log entry should have been emitted for the facade dispatch failure
|
||||
Reference in New Issue
Block a user