test(cli): TDD failing tests for actor list database update (bug #797)
CI / build (pull_request) Successful in 17s
CI / lint (pull_request) Successful in 3m20s
CI / quality (pull_request) Successful in 3m41s
CI / typecheck (pull_request) Successful in 3m57s
CI / security (pull_request) Successful in 4m3s
CI / integration_tests (pull_request) Successful in 7m30s
CI / unit_tests (pull_request) Successful in 8m26s
CI / docker (pull_request) Successful in 1m7s
CI / e2e_tests (pull_request) Successful in 9m52s
CI / coverage (pull_request) Successful in 11m26s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 1h2m29s
CI / build (pull_request) Successful in 17s
CI / lint (pull_request) Successful in 3m20s
CI / quality (pull_request) Successful in 3m41s
CI / typecheck (pull_request) Successful in 3m57s
CI / security (pull_request) Successful in 4m3s
CI / integration_tests (pull_request) Successful in 7m30s
CI / unit_tests (pull_request) Successful in 8m26s
CI / docker (pull_request) Successful in 1m7s
CI / e2e_tests (pull_request) Successful in 9m52s
CI / coverage (pull_request) Successful in 11m26s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 1h2m29s
Add TDD tests that prove bug #797 exists: the read-only `agents actor list` command triggers database writes via `ActorRegistry.list_actors()` calling `ensure_built_in_actors()`, which invokes `upsert_actor()` and potentially `set_default_actor()` on every invocation. Behave scenarios (features/tdd_actor_list_no_db_update.feature): - Assert upsert_actor is not called during actor list (fails: bug present) - Assert set_default_actor is not called during actor list (fails: bug present) Robot Framework tests (robot/tdd_actor_list_no_db_update.robot): - Integration smoke tests via helper script with same assertions - Both test cases fail as expected, inverted by tdd_expected_fail_listener All tests tagged @tdd_expected_fail @tdd_bug @tdd_bug_797 per the mandatory TDD bug-fix workflow. The inversion mechanism converts the expected failures to passes in CI. When bug #797 is fixed, the @tdd_expected_fail tag will be removed and the tests will run normally. Quality gates: lint pass, typecheck pass, coverage 98.22% (>97%). ISSUES CLOSED: #841
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
"""Step definitions for TDD Bug #797 — actor list should not cause a database update.
|
||||
|
||||
These steps verify that ``agents actor list`` does **not** trigger
|
||||
``upsert_actor()`` or ``set_default_actor()`` — both are database write
|
||||
operations. While the bug is present the assertions fail; the
|
||||
``@tdd_expected_fail`` tag on the feature inverts the result so CI passes.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from behave import given, then, when # type: ignore[import-untyped]
|
||||
from behave.runner import Context # type: ignore[import-untyped]
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from cleveragents.cli.commands.actor import app as actor_app
|
||||
from features.mocks.fake_provider import FakeProviderInfo, make_registry
|
||||
|
||||
_PATCH_TARGET: str = "cleveragents.cli.commands.actor._get_services"
|
||||
|
||||
_DEFAULT_PROVIDERS: list[FakeProviderInfo] = [
|
||||
FakeProviderInfo(
|
||||
name="FakeProvider",
|
||||
default_model="fake-org/fake-model-v1",
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Given
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given(
|
||||
"a mock actor service with a fake provider registry for tdd-actor-list-no-db-update"
|
||||
)
|
||||
def step_given_mock_service(context: Context) -> None:
|
||||
"""Set up a ``MagicMock`` actor service wired to a fake provider registry."""
|
||||
mock_service, registry = make_registry(_DEFAULT_PROVIDERS)
|
||||
context.mock_service = mock_service
|
||||
context.registry = registry
|
||||
context.runner = CliRunner()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# When
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("the actor list command is invoked for tdd-actor-list-no-db-update")
|
||||
def step_when_invoke_actor_list(context: Context) -> None:
|
||||
"""Invoke ``actor list`` through the Typer test runner."""
|
||||
with patch(
|
||||
_PATCH_TARGET,
|
||||
return_value=(context.mock_service, context.registry),
|
||||
):
|
||||
context.result = context.runner.invoke(actor_app, ["list"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("upsert_actor should not have been called for tdd-actor-list-no-db-update")
|
||||
def step_then_no_upsert(context: Context) -> None:
|
||||
"""Assert that ``upsert_actor`` was never called.
|
||||
|
||||
Bug #797: ``ActorRegistry.list_actors()`` calls
|
||||
``ensure_built_in_actors()`` which invokes ``upsert_actor()`` for
|
||||
every configured provider — a database write on a read-only command.
|
||||
"""
|
||||
call_count: int = context.mock_service.upsert_actor.call_count
|
||||
assert call_count == 0, (
|
||||
f"upsert_actor was called {call_count} time(s) during 'actor list'. "
|
||||
f"A read-only list command must not trigger database writes "
|
||||
f"(bug #797)."
|
||||
)
|
||||
|
||||
|
||||
@then("set_default_actor should not have been called for tdd-actor-list-no-db-update")
|
||||
def step_then_no_set_default(context: Context) -> None:
|
||||
"""Assert that ``set_default_actor`` was never called.
|
||||
|
||||
Bug #797: ``ActorRegistry.list_actors()`` calls
|
||||
``ensure_built_in_actors()`` which may invoke
|
||||
``set_default_actor()`` — another database write triggered by a
|
||||
read-only list command.
|
||||
"""
|
||||
call_count: int = context.mock_service.set_default_actor.call_count
|
||||
assert call_count == 0, (
|
||||
f"set_default_actor was called {call_count} time(s) during "
|
||||
f"'actor list'. A read-only list command must not trigger "
|
||||
f"database writes (bug #797)."
|
||||
)
|
||||
@@ -0,0 +1,22 @@
|
||||
@tdd_expected_fail @tdd_bug @tdd_bug_797
|
||||
Feature: TDD Bug #797 — actor list should not cause a database update
|
||||
|
||||
Bug #797 reports that running ``agents actor list`` on a fresh checkout
|
||||
triggers pending-migration prompts and database writes. The ``list``
|
||||
command is read-only from the user's perspective, so it must not call
|
||||
``upsert_actor()`` or any other write operation.
|
||||
|
||||
These scenarios prove the bug exists by asserting that no database
|
||||
writes occur during ``actor list``. While the bug is unfixed the
|
||||
assertions **fail** (proving the defect) and the ``@tdd_expected_fail``
|
||||
tag inverts the result so CI stays green.
|
||||
|
||||
Scenario: Actor list does not call upsert_actor for tdd-actor-list-no-db-update
|
||||
Given a mock actor service with a fake provider registry for tdd-actor-list-no-db-update
|
||||
When the actor list command is invoked for tdd-actor-list-no-db-update
|
||||
Then upsert_actor should not have been called for tdd-actor-list-no-db-update
|
||||
|
||||
Scenario: Actor list does not call set_default_actor for tdd-actor-list-no-db-update
|
||||
Given a mock actor service with a fake provider registry for tdd-actor-list-no-db-update
|
||||
When the actor list command is invoked for tdd-actor-list-no-db-update
|
||||
Then set_default_actor should not have been called for tdd-actor-list-no-db-update
|
||||
@@ -0,0 +1,124 @@
|
||||
"""Helper script for tdd_actor_list_no_db_update.robot smoke tests.
|
||||
|
||||
Each subcommand exercises the real ``ActorRegistry.list_actors()`` code
|
||||
path to reproduce bug #797. The ``actor list`` command is invoked via
|
||||
:class:`typer.testing.CliRunner` with a mock actor service and fake
|
||||
provider registry, and the helper checks whether ``upsert_actor()`` or
|
||||
``set_default_actor()`` was called (database writes).
|
||||
|
||||
The helper reports the **real** outcome: it exits 0 and prints the
|
||||
sentinel when the bug is fixed (no writes), and exits 1 when the bug
|
||||
is still present (writes detected). 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
|
||||
from typing import NoReturn
|
||||
from unittest.mock import patch
|
||||
|
||||
# Ensure local source tree is importable.
|
||||
_ROOT: Path = Path(__file__).resolve().parents[1]
|
||||
_SRC: str = str(_ROOT / "src")
|
||||
_FEATURES: str = str(_ROOT / "features")
|
||||
for _p in (_SRC, _FEATURES):
|
||||
if _p not in sys.path:
|
||||
sys.path.insert(0, _p)
|
||||
|
||||
from mocks.fake_provider import FakeProviderInfo, make_registry # noqa: E402
|
||||
from typer.testing import CliRunner # noqa: E402
|
||||
|
||||
from cleveragents.cli.commands.actor import app as actor_app # noqa: E402
|
||||
|
||||
runner: CliRunner = CliRunner()
|
||||
|
||||
_PATCH_TARGET: str = "cleveragents.cli.commands.actor._get_services"
|
||||
|
||||
_DEFAULT_PROVIDERS: list[FakeProviderInfo] = [
|
||||
FakeProviderInfo(
|
||||
name="FakeProvider",
|
||||
default_model="fake-org/fake-model-v1",
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
def _fail(message: str) -> NoReturn:
|
||||
"""Print an error message to stderr and exit with code 1."""
|
||||
print(message, file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Subcommands
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def check_no_upsert() -> None:
|
||||
"""Verify that ``actor list`` does not call ``upsert_actor``.
|
||||
|
||||
Exits 0 with sentinel when ``upsert_actor`` was NOT called (bug
|
||||
fixed). Exits 1 when ``upsert_actor`` WAS called (bug still
|
||||
present).
|
||||
"""
|
||||
mock_service, registry = make_registry(_DEFAULT_PROVIDERS)
|
||||
with patch(
|
||||
_PATCH_TARGET,
|
||||
return_value=(mock_service, registry),
|
||||
):
|
||||
runner.invoke(actor_app, ["list"])
|
||||
|
||||
call_count: int = mock_service.upsert_actor.call_count
|
||||
if call_count > 0:
|
||||
_fail(
|
||||
f"upsert_actor was called {call_count} time(s) during "
|
||||
f"'actor list'. A read-only list command must not trigger "
|
||||
f"database writes (bug #797)."
|
||||
)
|
||||
print("tdd-actor-list-no-db-update-no-upsert-ok")
|
||||
|
||||
|
||||
def check_no_set_default() -> None:
|
||||
"""Verify that ``actor list`` does not call ``set_default_actor``.
|
||||
|
||||
Exits 0 with sentinel when ``set_default_actor`` was NOT called
|
||||
(bug fixed). Exits 1 when ``set_default_actor`` WAS called (bug
|
||||
still present).
|
||||
"""
|
||||
mock_service, registry = make_registry(_DEFAULT_PROVIDERS)
|
||||
with patch(
|
||||
_PATCH_TARGET,
|
||||
return_value=(mock_service, registry),
|
||||
):
|
||||
runner.invoke(actor_app, ["list"])
|
||||
|
||||
call_count: int = mock_service.set_default_actor.call_count
|
||||
if call_count > 0:
|
||||
_fail(
|
||||
f"set_default_actor was called {call_count} time(s) during "
|
||||
f"'actor list'. A read-only list command must not trigger "
|
||||
f"database writes (bug #797)."
|
||||
)
|
||||
print("tdd-actor-list-no-db-update-no-set-default-ok")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dispatcher
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_COMMANDS: dict[str, Callable[[], None]] = {
|
||||
"check-no-upsert": check_no_upsert,
|
||||
"check-no-set-default": check_no_set_default,
|
||||
}
|
||||
|
||||
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: Callable[[], None] = _COMMANDS[sys.argv[1]]
|
||||
cmd()
|
||||
@@ -0,0 +1,40 @@
|
||||
*** Settings ***
|
||||
Documentation TDD Bug #797 -- actor list should not cause a database update
|
||||
... Integration smoke tests verifying that the ``actor list``
|
||||
... command does not trigger ``upsert_actor()`` or
|
||||
... ``set_default_actor()`` calls. While the bug is present
|
||||
... the helper exits 1 and the ``tdd_expected_fail_listener``
|
||||
... inverts the failure to a pass.
|
||||
Resource ${CURDIR}/common.resource
|
||||
Suite Setup Setup Test Environment
|
||||
Suite Teardown Cleanup Test Environment
|
||||
|
||||
*** Variables ***
|
||||
${HELPER} ${CURDIR}/helper_tdd_actor_list_no_db_update.py
|
||||
|
||||
*** Test Cases ***
|
||||
TDD Actor List Does Not Call Upsert Actor
|
||||
[Documentation] Verify that ``actor list`` does not call
|
||||
... ``upsert_actor()`` on the actor service.
|
||||
... Bug #797: ``list_actors()`` calls
|
||||
... ``ensure_built_in_actors()`` which writes to the DB.
|
||||
[Tags] tdd_expected_fail tdd_bug tdd_bug_797
|
||||
${result}= Run Process ${PYTHON} ${HELPER} check-no-upsert
|
||||
... cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
||||
Log ${result.stdout}
|
||||
Log ${result.stderr}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} tdd-actor-list-no-db-update-no-upsert-ok
|
||||
|
||||
TDD Actor List Does Not Call Set Default Actor
|
||||
[Documentation] Verify that ``actor list`` does not call
|
||||
... ``set_default_actor()`` on the actor service.
|
||||
... Bug #797: ``ensure_built_in_actors()`` may also
|
||||
... call ``set_default_actor()`` — another DB write.
|
||||
[Tags] tdd_expected_fail tdd_bug tdd_bug_797
|
||||
${result}= Run Process ${PYTHON} ${HELPER} check-no-set-default
|
||||
... cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
||||
Log ${result.stdout}
|
||||
Log ${result.stderr}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} tdd-actor-list-no-db-update-no-set-default-ok
|
||||
Reference in New Issue
Block a user