7f29874156
CI / push-validation (pull_request) Successful in 23s
CI / helm (pull_request) Successful in 37s
CI / build (pull_request) Successful in 3m48s
CI / lint (pull_request) Successful in 3m55s
CI / quality (pull_request) Successful in 4m18s
CI / typecheck (pull_request) Successful in 4m32s
CI / security (pull_request) Successful in 4m35s
CI / integration_tests (pull_request) Successful in 6m47s
CI / e2e_tests (pull_request) Successful in 6m56s
CI / unit_tests (pull_request) Successful in 7m23s
CI / docker (pull_request) Successful in 1m30s
CI / coverage (pull_request) Successful in 14m31s
CI / benchmark-regression (push) Waiting to run
CI / benchmark-publish (push) Waiting to run
CI / status-check (pull_request) Successful in 3s
CI / helm (push) Successful in 27s
CI / push-validation (push) Successful in 26s
CI / build (push) Successful in 3m43s
CI / lint (push) Successful in 3m54s
CI / quality (push) Successful in 4m16s
CI / typecheck (push) Successful in 4m28s
CI / security (push) Successful in 4m43s
CI / integration_tests (push) Successful in 7m3s
CI / e2e_tests (push) Successful in 7m13s
CI / unit_tests (push) Successful in 7m25s
CI / docker (push) Successful in 1m36s
CI / coverage (push) Successful in 13m40s
CI / status-check (push) Successful in 3s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Failing after 14m31s
Implement comprehensive fixes for all P2:should-fix and P3:nit findings from the Strategy Actor code review: Update 5 step functions that were calling the private _execute_with_llm() method directly to instead use the new strategy_tree field from StrategizeResult: - step_execute_and_inspect_tree (line 619) - step_parse_self_dep (line 877) - step_parse_duplicate_step_numbers (line 968) - step_parse_non_sequential_steps (line 1075) - step_parse_non_sequential_steps_and_inspect (line 1755) - step_build_decisions_from_llm_tree (line 1299) - also added execute() call This eliminates the fragile double-execution pattern that produced divergent ULIDs and couples tests to private implementation details. Tests now use the public StrategizeResult.strategy_tree field. P2:should-fix Fixes: - Move json import to module level in plan_executor_coverage_steps.py - Remove redundant Exception clause in plan_executor.py exception handler - Add logging to silent config service fallback in plan.py - Improve structured content block handling in _extract_content() to properly extract text from LangChain MessageContent dicts - Remove duplicated _DEFAULT_ACTOR_NAME constant and import from canonical source (strategy_resolution.py) - Add strategy_tree field to StrategizeResult to expose tree for test inspection without coupling to private _execute_with_llm() method P3:nit Fixes: - Improved docstrings and code clarity Refs: #10267
153 lines
5.4 KiB
Python
153 lines
5.4 KiB
Python
"""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 logging
|
|
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
|
|
|
|
|
|
def _suppress_structlog_stdout() -> tuple[int, bool]:
|
|
"""Prevent structlog debug lines from contaminating CLI stdout.
|
|
|
|
When ``behave-parallel`` runs multiple features in one process,
|
|
another feature may (re)configure structlog with a ``ConsoleRenderer``
|
|
or leave it at the default ``PrintLogger`` — both write to
|
|
``sys.stdout``. The ``@database_retry`` decorator logs a *debug*
|
|
event on every attempt, and ``CliRunner.invoke()`` captures
|
|
``sys.stdout``, so those debug lines end up in the captured output,
|
|
breaking JSON/YAML assertions.
|
|
|
|
Returns the previous root-logger level and ``cache_logger_on_first_use``
|
|
flag so that the caller's cleanup function can restore them.
|
|
"""
|
|
import structlog
|
|
|
|
root = logging.getLogger()
|
|
prev_level = root.level
|
|
root.setLevel(logging.CRITICAL)
|
|
|
|
# Force structlog to route through stdlib logging (not PrintLogger)
|
|
# and disable logger caching so the level change takes effect for
|
|
# loggers that were already instantiated.
|
|
prev_config = structlog.get_config()
|
|
prev_cache: bool = prev_config.get("cache_logger_on_first_use", True) # type: ignore[assignment]
|
|
structlog.configure(
|
|
logger_factory=structlog.stdlib.LoggerFactory(),
|
|
wrapper_class=structlog.stdlib.BoundLogger,
|
|
cache_logger_on_first_use=False,
|
|
)
|
|
|
|
return prev_level, prev_cache
|
|
|
|
|
|
def _restore_structlog(prev_level: int, prev_cache: bool) -> None:
|
|
"""Undo the suppression applied by :func:`_suppress_structlog_stdout`."""
|
|
import structlog
|
|
|
|
logging.getLogger().setLevel(prev_level)
|
|
structlog.configure(cache_logger_on_first_use=prev_cache)
|
|
|
|
|
|
@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
|
|
resolves the ``session_service`` provider.
|
|
"""
|
|
context.runner = CliRunner()
|
|
|
|
# Ensure we go through the real DI path — no mock service.
|
|
session_mod._service = None
|
|
|
|
# Reset singletons before setup so stale state from a parallel
|
|
# scenario does not leak in.
|
|
reset_container()
|
|
Settings._instance = None # type: ignore[attr-defined]
|
|
|
|
# Suppress structlog debug output so CliRunner captures clean output.
|
|
prev_level, prev_cache = _suppress_structlog_stdout()
|
|
|
|
# Provide a database URL so the container can be constructed.
|
|
fd, db_path = tempfile.mkstemp(suffix=".db")
|
|
os.close(fd)
|
|
context._tdd_db_path = db_path
|
|
|
|
db_url = f"sqlite:///{db_path}"
|
|
os.environ["CLEVERAGENTS_DATABASE_URL"] = db_url
|
|
|
|
# Create the schema so the database file is ready for use.
|
|
from sqlalchemy import create_engine
|
|
|
|
from cleveragents.infrastructure.database.models import Base
|
|
|
|
engine = create_engine(db_url, echo=False)
|
|
try:
|
|
Base.metadata.create_all(engine)
|
|
finally:
|
|
engine.dispose()
|
|
|
|
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 # type: ignore[attr-defined]
|
|
Path(db_path).unlink(missing_ok=True)
|
|
_restore_structlog(prev_level, prev_cache)
|
|
|
|
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."""
|
|
output = context.result.output
|
|
# Strip ANSI color codes and MCP health check warning that may prepend to JSON
|
|
output = output.lstrip()
|
|
lines = output.split("\n")
|
|
# Skip leading lines that are not JSON (like MCP warnings)
|
|
json_start_idx = 0
|
|
for i, line in enumerate(lines):
|
|
stripped = line.strip()
|
|
if stripped.startswith("{") or stripped.startswith("["):
|
|
json_start_idx = i
|
|
break
|
|
output = "\n".join(lines[json_start_idx:])
|
|
|
|
try:
|
|
json.loads(output)
|
|
except json.JSONDecodeError as exc:
|
|
raise AssertionError(
|
|
f"Output is not valid JSON:\n{context.result.output}"
|
|
) from exc
|