Files
placeholder/features/steps/skill_cli_coverage_boost_steps.py
freemo a0df5a4cd0 fix(cli): wrap format_output() in spec-required JSON/YAML envelope across all CLI commands
Implements the spec-required JSON/YAML output envelope for all CLI commands
that use format_output(). The envelope structure is:

  {
    "command": "<command that was run>",
    "status": "ok" | "warn" | "error",
    "exit_code": 0,
    "data": { ... command-specific payload ... },
    "timing": { "duration_ms": 123 },
    "messages": [{ "level": "ok", "text": "..." }]
  }

Changes:
- Add _build_envelope() helper to construct the spec-required envelope
- Add optional command, status, exit_code, messages parameters to format_output()
- Wrap json/yaml output in the envelope; plain/table/rich/color unchanged
- Add timing measurement (duration_ms) to all json/yaml outputs
- Add new BDD feature file (cli_json_envelope.feature) with 14 scenarios
  testing envelope field presence, values, and data payload
- Update 14 existing step files to unwrap the envelope when checking
  specific data keys (backward-compatible via _unwrap_envelope() helper)

Closes #3431
2026-04-05 19:48:40 +00:00

325 lines
12 KiB
Python

# pyright: reportRedeclaration=false
"""Step definitions for skill_cli_coverage_boost.feature.
Targets the remaining uncovered lines in
``cleveragents.cli.commands.skill``:
- Lines 82-103: ``_get_skill_service()`` DB initialisation and fallback
- Line 889: ``tools`` non-rich ``source_type = "agent_skills"``
- Lines 984-985: ``refresh`` defensive ``name is None`` guard
All step text uses the ``boost-`` prefix to avoid collisions.
"""
from __future__ import annotations
import json
from typing import Any
from unittest.mock import MagicMock, patch
from behave import given, then, when
from behave.runner import Context
from typer.testing import CliRunner
import cleveragents.cli.commands.skill as skill_mod
from cleveragents.application.services.skill_service import SkillService
from cleveragents.cli.commands.skill import (
_get_skill_service,
_reset_skill_service,
)
from cleveragents.cli.commands.skill import app as skill_app
from cleveragents.domain.models.core.skill import (
ResolvedToolEntry,
Skill,
SkillAgentSource,
)
_ENVELOPE_KEYS = {"command", "status", "exit_code", "data", "timing", "messages"}
def _unwrap_envelope(parsed: Any) -> Any:
"""Return the ``data`` field if *parsed* is a spec envelope, else *parsed* as-is."""
if isinstance(parsed, dict) and _ENVELOPE_KEYS.issubset(parsed.keys()):
return parsed["data"]
return parsed
# ── helpers ─────────────────────────────────────────────────
def _make_skill(
name: str,
description: str = "test skill",
tool_refs: list[str] | None = None,
agent_skills: list[SkillAgentSource] | None = None,
) -> Skill:
"""Create a Skill domain object."""
return Skill(
name=name,
description=description,
tool_refs=tool_refs or [],
includes=[],
mcp_servers=[],
agent_skills=agent_skills or [],
anonymous_tools=[],
)
def _register_skill(context: Context, skill: Skill) -> None:
"""Register a skill in the service with timestamps."""
from datetime import datetime
now = datetime.now()
context.boost_service._skills[skill.name] = skill
context.boost_service._created_at[skill.name] = now
context.boost_service._updated_at[skill.name] = now
# ── Background ──────────────────────────────────────────────
@given("boost- a fresh skill CLI service")
def step_boost_background(context: Context) -> None:
"""Reset module-level singleton and prepare runner/service."""
_reset_skill_service()
context.boost_runner = CliRunner()
context.boost_service = _get_skill_service()
context.boost_result = None
context.boost_patches: list[Any] = []
context.boost_returned_service = None
context.boost_guard_printed = False
context.boost_guard_aborted = False
# ── Given: _get_skill_service delegates to container ────────
@given("boost- the module-level _service is set to None")
def step_boost_set_service_none(context: Context) -> None:
"""No-op — _get_skill_service always delegates to the container now."""
@given("boost- the DI container returns a DB-backed SkillService")
def step_boost_mock_container_db_service(context: Context) -> None:
"""Mock get_container().skill_service() to return a SkillService with a repo."""
mock_skill_repo = MagicMock()
mock_service = SkillService(skill_repo=mock_skill_repo)
mock_container = MagicMock()
mock_container.skill_service.return_value = mock_service
p1 = patch(
"cleveragents.cli.commands.skill.get_container",
return_value=mock_container,
)
p1.start()
context.boost_patches.append(p1)
context.boost_mock_container = mock_container
# ── Given: tools with agent_skill entries ───────────────────
@given('boost- a registered skill "{name}" with agent_skills')
def step_boost_register_skill_with_agent_skills(context: Context, name: str) -> None:
"""Register a skill that has agent_skills entries."""
skill = _make_skill(
name=name,
agent_skills=[SkillAgentSource(path="/tmp/my-agent-skill")],
)
_register_skill(context, skill)
# Patch resolve_tools to return entries with agent_skill: prefix
def patched_resolve(skill_name: str) -> tuple[Skill, list[ResolvedToolEntry]]:
sk = context.boost_service.get_skill(skill_name)
entries = [
ResolvedToolEntry(
name="agent_skill:/tmp/my-agent-skill",
source_skill=skill_name,
is_inline=False,
),
]
return sk, entries
p = patch.object(
context.boost_service, "resolve_tools", side_effect=patched_resolve
)
p.start()
context.boost_patches.append(p)
# ── When: _get_skill_service ────────────────────────────────
@when("boost- I call _get_skill_service")
def step_boost_call_get_skill_service(context: Context) -> None:
"""Call _get_skill_service and store the result."""
try:
context.boost_returned_service = _get_skill_service()
finally:
_stop_patches(context)
@when("boost- I call _get_skill_service twice")
def step_boost_call_get_skill_service_twice(context: Context) -> None:
"""Call _get_skill_service twice to test caching."""
try:
context.boost_returned_service = _get_skill_service()
_get_skill_service() # second call should use cached
finally:
_stop_patches(context)
# ── When: tools command ─────────────────────────────────────
@when('boost- I invoke tools "{name}" in format "{fmt}"')
def step_boost_invoke_tools_fmt(context: Context, name: str, fmt: str) -> None:
"""Invoke skill tools with a specified format."""
context.boost_result = context.boost_runner.invoke(
skill_app, ["tools", name, "--format", fmt]
)
_stop_patches(context)
# ── When: refresh defensive guard ───────────────────────────
@when(
"boost- I call refresh directly with name None and all_skills False bypassing first guard"
)
def step_boost_refresh_bypass_first_guard(context: Context) -> None:
"""Exercise the refresh function with name=None, all_skills=False.
The first guard (lines 971-973) catches this and aborts.
Lines 984-985 are a defensive duplicate guard that can only fire if
the first guard is somehow bypassed. We verify the first guard fires
correctly (the observable behaviour), which is the intended test.
"""
import typer
# Track what console.print receives
p_print = patch.object(skill_mod, "console")
mock_console = p_print.start()
context.boost_patches.append(p_print)
try:
from cleveragents.cli.commands.skill import refresh as refresh_fn
refresh_fn(name=None, all_skills=False, fmt="rich")
except (typer.Abort, SystemExit):
context.boost_guard_aborted = True
# Check if the error message was printed
for call_args in mock_console.print.call_args_list:
if call_args and call_args[0]:
msg = str(call_args[0][0])
if "Must specify either" in msg:
context.boost_guard_printed = True
_stop_patches(context)
# ── Then: _get_skill_service assertions ─────────────────────
@then("boost- the returned service should have a skill_repo")
def step_boost_service_has_repo(context: Context) -> None:
"""Assert the returned service was created with a skill_repo."""
svc = context.boost_returned_service
assert svc is not None, "No service was returned"
assert svc._skill_repo is not None, (
"Expected service to have a skill_repo (DB-backed), but it was None"
)
@then("boost- the container skill_service should be called once")
def step_boost_container_called_once(context: Context) -> None:
"""Assert get_container().skill_service() was called only once (caching)."""
context.boost_mock_container.skill_service.assert_called_once()
@then("boost- the container skill_service should be called twice")
def step_boost_container_called_twice(context: Context) -> None:
"""Assert get_container().skill_service() is called on every access (no cache)."""
assert context.boost_mock_container.skill_service.call_count == 2
# ── Then: CLI exit code ─────────────────────────────────────
@then("boost- the CLI exit code should be 0")
def step_boost_exit_code_0(context: Context) -> None:
"""Assert CLI exited successfully."""
assert context.boost_result is not None
assert context.boost_result.exit_code == 0, (
f"Expected exit_code=0, got {context.boost_result.exit_code}\n"
f"Output: {context.boost_result.output}"
)
# ── Then: JSON output assertions ────────────────────────────
@then("boost- the JSON output should be valid")
def step_boost_json_valid(context: Context) -> None:
"""Assert the CLI output parses as valid JSON.
Stores the unwrapped ``data`` field in ``context.boost_parsed_json`` so
that downstream key-check steps work against the actual payload rather
than the spec envelope wrapper.
"""
assert context.boost_result is not None
try:
parsed = json.loads(context.boost_result.output)
except json.JSONDecodeError as e:
raise AssertionError(
f"Output is not valid JSON: {e}\nOutput: {context.boost_result.output}"
) from e
context.boost_parsed_json = _unwrap_envelope(parsed)
@then('boost- the JSON tools list should contain an entry with source "{source}"')
def step_boost_json_tools_has_source(context: Context, source: str) -> None:
"""Assert at least one tool entry has the given source type."""
data = context.boost_parsed_json
assert isinstance(data, dict), f"Expected dict, got {type(data).__name__}"
tools_list = data.get("tools", [])
assert len(tools_list) > 0, "No tools in JSON output"
sources_found = [t.get("source") for t in tools_list]
assert source in sources_found, (
f"Expected source '{source}' in tools list, found sources: {sources_found}"
)
# ── Then: refresh defensive guard assertions ────────────────
@then("boost- the second guard should have printed the error message")
def step_boost_guard_printed(context: Context) -> None:
"""Assert the guard printed 'Must specify either' error message."""
assert context.boost_guard_printed, (
"Expected the guard to have printed 'Must specify either' error message"
)
@then("boost- the function should have raised Abort")
def step_boost_guard_aborted(context: Context) -> None:
"""Assert that Abort was raised from the guard."""
assert context.boost_guard_aborted, "Expected typer.Abort to have been raised"
# ── Cleanup helper ──────────────────────────────────────────
def _stop_patches(context: Context) -> None:
"""Stop all active patches after a When step."""
import contextlib
for p in getattr(context, "boost_patches", []):
with contextlib.suppress(RuntimeError):
p.stop()
context.boost_patches = []
# Reset the module-level service to avoid leaking state
_reset_skill_service()