test(coverage): add Behave BDD tests for 8 under-covered modules
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 19s
CI / security (pull_request) Successful in 36s
CI / typecheck (pull_request) Successful in 38s
CI / integration_tests (pull_request) Successful in 2m44s
CI / lint (push) Successful in 12s
CI / quality (push) Successful in 16s
CI / build (push) Successful in 23s
CI / typecheck (push) Successful in 31s
CI / security (push) Successful in 32s
CI / benchmark-regression (push) Has been skipped
CI / integration_tests (push) Successful in 3m25s
CI / unit_tests (pull_request) Successful in 13m3s
CI / docker (pull_request) Successful in 1m2s
CI / benchmark-publish (push) Successful in 16m35s
CI / benchmark-regression (pull_request) Successful in 22m21s
CI / unit_tests (push) Successful in 24m45s
CI / docker (push) Successful in 8s
CI / coverage (pull_request) Successful in 1h1m47s
CI / coverage (push) Successful in 1h11m37s

Added targeted Behave BDD feature files and step definitions to improve
unit test coverage for:

- decision_service.py: Full coverage of all 7 service methods (18 scenarios)
- plan_apply_service.py: Branch coverage for handle_merge_failure (2 scenarios)
- plan_executor.py: Edge cases for rollback, checkpoint, and parse_steps (15 scenarios)
- cli/commands/plan.py: Uncovered region lines 1950-2273 (23 scenarios)
- repositories.py: Remaining missed branches and lines (14 scenarios)
- sandbox/checkpoint.py: Full coverage of CheckpointManager (26 scenarios)
- langgraph/bridge.py: Remaining uncovered lines and branches (10 scenarios)
- cli/commands/config.py: Safety net to maintain 100% coverage (42 scenarios)

Total: 150 new scenarios, 596 steps, all passing.

Also fixed a step definition collision in plan_lifecycle_coverage by renaming
"the delete result should be false" to "the plan delete result should be false".

ISSUES CLOSED: #475
This commit is contained in:
2026-02-28 20:59:49 +00:00
committed by Forgejo
parent edd739c169
commit 2eb31a598c
18 changed files with 5691 additions and 2 deletions
@@ -0,0 +1,474 @@
"""Step definitions for decision_service_coverage.feature.
Exercises every method of ``DecisionService`` using mocked UnitOfWork
and repository objects so that decision_service.py achieves full
line-rate and branch-rate coverage.
All step names use the ``dsvc-`` prefix to avoid collisions with
existing step definitions in other feature files.
"""
from __future__ import annotations
from contextlib import contextmanager
from typing import Any
from unittest.mock import MagicMock
from behave import given, then, when
from behave.runner import Context
from ulid import ULID
from cleveragents.application.services.decision_service import DecisionService
from cleveragents.domain.models.core.decision import (
Decision,
DecisionType,
)
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
_PLAN_ID = str(ULID())
_DECISION_ID = str(ULID())
_NEW_DECISION_ID = str(ULID())
_LEAF_ID = str(ULID())
_ROOT_ID = str(ULID())
def _make_decision(
decision_id: str | None = None,
plan_id: str = _PLAN_ID,
parent_decision_id: str | None = None,
sequence_number: int = 0,
decision_type: DecisionType = DecisionType.PROMPT_DEFINITION,
superseded_by: str | None = None,
) -> Decision:
"""Create a minimal valid Decision for testing."""
kwargs: dict[str, Any] = {
"plan_id": plan_id,
"sequence_number": sequence_number,
"decision_type": decision_type,
"question": "Which approach?",
"chosen_option": "REST API",
}
if decision_id is not None:
kwargs["decision_id"] = decision_id
if parent_decision_id is not None:
kwargs["parent_decision_id"] = parent_decision_id
if superseded_by is not None:
kwargs["superseded_by"] = superseded_by
return Decision(**kwargs)
def _build_mock_uow() -> tuple[MagicMock, MagicMock, MagicMock]:
"""Build a mock UnitOfWork with a mock transaction context manager.
Returns:
(mock_uow, mock_ctx, mock_decisions_repo)
"""
mock_decisions = MagicMock(name="decisions_repo")
mock_ctx = MagicMock(name="uow_context")
mock_ctx.decisions = mock_decisions
mock_uow = MagicMock(name="unit_of_work")
@contextmanager
def _fake_transaction():
yield mock_ctx
mock_uow.transaction = _fake_transaction
return mock_uow, mock_ctx, mock_decisions
def _build_service(mock_uow: MagicMock) -> tuple[DecisionService, MagicMock]:
"""Construct a DecisionService with mock settings and the given UoW.
Returns:
(service, mock_settings)
"""
mock_settings = MagicMock(name="settings")
service = DecisionService(settings=mock_settings, unit_of_work=mock_uow)
return service, mock_settings
# ---------------------------------------------------------------------------
# Constructor
# ---------------------------------------------------------------------------
@given("dsvc- a mock settings object and a mock UnitOfWork")
def step_dsvc_mock_deps(context: Context) -> None:
context.dsvc_settings = MagicMock(name="settings")
context.dsvc_uow = MagicMock(name="unit_of_work")
@when("dsvc- I construct a DecisionService with those dependencies")
def step_dsvc_construct(context: Context) -> None:
context.dsvc_service = DecisionService(
settings=context.dsvc_settings,
unit_of_work=context.dsvc_uow,
)
@then("dsvc- the service should store the settings")
def step_dsvc_check_settings(context: Context) -> None:
assert context.dsvc_service.settings is context.dsvc_settings, (
"Service did not store settings"
)
@then("dsvc- the service should store the unit_of_work")
def step_dsvc_check_uow(context: Context) -> None:
assert context.dsvc_service.unit_of_work is context.dsvc_uow, (
"Service did not store unit_of_work"
)
@then("dsvc- the service should have a bound structlog logger")
def step_dsvc_check_logger(context: Context) -> None:
# The _logger is a BoundLogger with service="decision"
assert hasattr(context.dsvc_service, "_logger"), "Service missing _logger attribute"
# ---------------------------------------------------------------------------
# Shared Given: service + mock UoW
# ---------------------------------------------------------------------------
@given("dsvc- a DecisionService with a mocked UnitOfWork")
def step_dsvc_service_with_mock_uow(context: Context) -> None:
mock_uow, mock_ctx, mock_decisions = _build_mock_uow()
service, _ = _build_service(mock_uow)
# Attach a mock logger so non-logger scenarios can still verify calls
mock_logger = MagicMock(name="structlog_logger")
mock_logger.bind = MagicMock(return_value=mock_logger)
service._logger = mock_logger
context.dsvc_service = service
context.dsvc_uow = mock_uow
context.dsvc_ctx = mock_ctx
context.dsvc_decisions = mock_decisions
context.dsvc_logger = mock_logger
@given("dsvc- a DecisionService with a mocked UnitOfWork and captured logger")
def step_dsvc_service_with_logger(context: Context) -> None:
mock_uow, mock_ctx, mock_decisions = _build_mock_uow()
service, _ = _build_service(mock_uow)
# Replace the _logger with a MagicMock so we can assert calls
mock_logger = MagicMock(name="structlog_logger")
mock_logger.bind = MagicMock(return_value=mock_logger)
service._logger = mock_logger
context.dsvc_service = service
context.dsvc_uow = mock_uow
context.dsvc_ctx = mock_ctx
context.dsvc_decisions = mock_decisions
context.dsvc_logger = mock_logger
# ---------------------------------------------------------------------------
# record_decision
# ---------------------------------------------------------------------------
@given("dsvc- a sample root Decision object")
def step_dsvc_sample_decision(context: Context) -> None:
context.dsvc_decision = _make_decision(plan_id=_PLAN_ID)
@when("dsvc- I call record_decision with the sample decision")
def step_dsvc_call_record(context: Context) -> None:
context.dsvc_result = context.dsvc_service.record_decision(
context.dsvc_decision,
)
@then("dsvc- the UoW transaction should have been entered")
def step_dsvc_txn_entered(context: Context) -> None:
# The fact that ctx.decisions.create was called proves we entered
# the transaction context manager successfully. We verify that
# the mock context's decisions repo was accessed.
assert context.dsvc_decisions.create.called, (
"Transaction context was not entered — create was never called"
)
@then("dsvc- ctx.decisions.create should have been called with the decision")
def step_dsvc_create_called(context: Context) -> None:
context.dsvc_decisions.create.assert_called_once_with(context.dsvc_decision)
@then("dsvc- the returned decision should be the same object")
def step_dsvc_record_returns_same(context: Context) -> None:
assert context.dsvc_result is context.dsvc_decision, (
"record_decision should return the same Decision instance"
)
# ---------------------------------------------------------------------------
# record_decision logging
# ---------------------------------------------------------------------------
@then('dsvc- the logger should have recorded an info call with "{event}"')
def step_dsvc_logger_info(context: Context, event: str) -> None:
info_calls = context.dsvc_logger.info.call_args_list
events = [c.args[0] if c.args else c.kwargs.get("event", "") for c in info_calls]
assert event in events, f"Expected info event '{event}' in {events}"
@then('dsvc- the logger should have recorded a debug call with "{event}"')
def step_dsvc_logger_debug(context: Context, event: str) -> None:
debug_calls = context.dsvc_logger.debug.call_args_list
events = [c.args[0] if c.args else c.kwargs.get("event", "") for c in debug_calls]
assert event in events, f"Expected debug event '{event}' in {events}"
# ---------------------------------------------------------------------------
# get_decision
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo get method returns a Decision")
def step_dsvc_mock_get_returns(context: Context) -> None:
context.dsvc_expected_decision = _make_decision(
decision_id=_DECISION_ID,
plan_id=_PLAN_ID,
)
context.dsvc_decisions.get.return_value = context.dsvc_expected_decision
@given("dsvc- the mock repo get method returns None")
def step_dsvc_mock_get_returns_none(context: Context) -> None:
context.dsvc_decisions.get.return_value = None
@when("dsvc- I call get_decision with a known ID")
def step_dsvc_call_get(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_decision(_DECISION_ID)
@when("dsvc- I call get_decision with an unknown ID")
def step_dsvc_call_get_unknown(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_decision("NONEXISTENT_ID_12345678")
@then("dsvc- ctx.decisions.get should have been called with the ID")
def step_dsvc_get_called(context: Context) -> None:
context.dsvc_decisions.get.assert_called_once_with(_DECISION_ID)
@then("dsvc- the returned value should be the expected Decision")
def step_dsvc_get_returns_expected(context: Context) -> None:
assert context.dsvc_result is context.dsvc_expected_decision, (
"get_decision did not return the expected Decision"
)
@then("dsvc- the returned value should be None")
def step_dsvc_result_is_none(context: Context) -> None:
assert context.dsvc_result is None, f"Expected None, got {context.dsvc_result!r}"
# ---------------------------------------------------------------------------
# get_decisions_for_plan
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo get_by_plan method returns {count:d} decisions")
def step_dsvc_mock_get_by_plan(context: Context, count: int) -> None:
decisions = [
_make_decision(
plan_id=_PLAN_ID,
sequence_number=i,
decision_type=(
DecisionType.PROMPT_DEFINITION
if i == 0
else DecisionType.STRATEGY_CHOICE
),
parent_decision_id=None if i == 0 else str(ULID()),
)
for i in range(count)
]
context.dsvc_decisions.get_by_plan.return_value = decisions
context.dsvc_expected_count = count
@when("dsvc- I call get_decisions_for_plan with a plan ID")
def step_dsvc_call_get_for_plan(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_decisions_for_plan(_PLAN_ID)
@then("dsvc- ctx.decisions.get_by_plan should have been called with the plan ID")
def step_dsvc_get_by_plan_called(context: Context) -> None:
context.dsvc_decisions.get_by_plan.assert_called_once_with(_PLAN_ID)
@then("dsvc- the returned list should have {count:d} decisions")
def step_dsvc_list_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} decisions, got {len(context.dsvc_result)}"
)
# ---------------------------------------------------------------------------
# get_decision_tree
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo get_tree method returns {count:d} decisions")
def step_dsvc_mock_get_tree(context: Context, count: int) -> None:
root = _make_decision(decision_id=_ROOT_ID, plan_id=_PLAN_ID)
decisions = [root] + [
_make_decision(
plan_id=_PLAN_ID,
sequence_number=i,
decision_type=DecisionType.STRATEGY_CHOICE,
parent_decision_id=_ROOT_ID,
)
for i in range(1, count)
]
context.dsvc_decisions.get_tree.return_value = decisions
context.dsvc_expected_tree_count = count
@when("dsvc- I call get_decision_tree with a root ID")
def step_dsvc_call_get_tree(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_decision_tree(_ROOT_ID)
@then("dsvc- ctx.decisions.get_tree should have been called with the root ID")
def step_dsvc_get_tree_called(context: Context) -> None:
context.dsvc_decisions.get_tree.assert_called_once_with(_ROOT_ID)
@then("dsvc- the returned tree list should have {count:d} decisions")
def step_dsvc_tree_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} tree nodes, got {len(context.dsvc_result)}"
)
# ---------------------------------------------------------------------------
# get_path_to_root
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo get_path_to_root method returns {count:d} decisions")
def step_dsvc_mock_get_path(context: Context, count: int) -> None:
decisions = [
_make_decision(
plan_id=_PLAN_ID,
sequence_number=count - 1 - i,
decision_type=(
DecisionType.PROMPT_DEFINITION
if i == count - 1
else DecisionType.STRATEGY_CHOICE
),
parent_decision_id=None if i == count - 1 else str(ULID()),
)
for i in range(count)
]
context.dsvc_decisions.get_path_to_root.return_value = decisions
context.dsvc_expected_path_count = count
@when("dsvc- I call get_path_to_root with a leaf ID")
def step_dsvc_call_get_path(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_path_to_root(_LEAF_ID)
@then("dsvc- ctx.decisions.get_path_to_root should have been called with the leaf ID")
def step_dsvc_get_path_called(context: Context) -> None:
context.dsvc_decisions.get_path_to_root.assert_called_once_with(_LEAF_ID)
@then("dsvc- the returned path list should have {count:d} decisions")
def step_dsvc_path_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} path nodes, got {len(context.dsvc_result)}"
)
# ---------------------------------------------------------------------------
# mark_superseded
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo update_superseded_by method returns a superseded Decision")
def step_dsvc_mock_superseded(context: Context) -> None:
context.dsvc_superseded_decision = _make_decision(
decision_id=_DECISION_ID,
plan_id=_PLAN_ID,
superseded_by=_NEW_DECISION_ID,
)
context.dsvc_decisions.update_superseded_by.return_value = (
context.dsvc_superseded_decision
)
@when("dsvc- I call mark_superseded with old and new IDs")
def step_dsvc_call_mark_superseded(context: Context) -> None:
context.dsvc_result = context.dsvc_service.mark_superseded(
_DECISION_ID,
_NEW_DECISION_ID,
)
@then("dsvc- ctx.decisions.update_superseded_by should have been called with both IDs")
def step_dsvc_update_superseded_called(context: Context) -> None:
context.dsvc_decisions.update_superseded_by.assert_called_once_with(
_DECISION_ID,
_NEW_DECISION_ID,
)
@then("dsvc- the returned decision should be the superseded one")
def step_dsvc_superseded_returned(context: Context) -> None:
assert context.dsvc_result is context.dsvc_superseded_decision, (
"mark_superseded did not return the expected superseded Decision"
)
# ---------------------------------------------------------------------------
# list_by_type
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo list_by_type method returns {count:d} decisions")
def step_dsvc_mock_list_by_type(context: Context, count: int) -> None:
decisions = [
_make_decision(
plan_id=_PLAN_ID,
sequence_number=i,
decision_type=DecisionType.STRATEGY_CHOICE,
parent_decision_id=str(ULID()),
)
for i in range(count)
]
context.dsvc_decisions.list_by_type.return_value = decisions
context.dsvc_expected_type_count = count
@when('dsvc- I call list_by_type with plan ID and type "{dtype}"')
def step_dsvc_call_list_by_type(context: Context, dtype: str) -> None:
context.dsvc_list_type_arg = dtype
context.dsvc_result = context.dsvc_service.list_by_type(_PLAN_ID, dtype)
@then("dsvc- ctx.decisions.list_by_type should have been called with plan ID and type")
def step_dsvc_list_by_type_called(context: Context) -> None:
context.dsvc_decisions.list_by_type.assert_called_once_with(
_PLAN_ID,
context.dsvc_list_type_arg,
)
@then("dsvc- the returned type list should have {count:d} decisions")
def step_dsvc_type_list_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} decisions by type, got {len(context.dsvc_result)}"
)