forked from HAL9000/cleveragents-core
747d8d3c9a
## Summary Adds TDD bug-capture tests proving that `agents init --yes` fails to bypass the migration approval prompt in a TTY environment (bug #783). These tests follow the mandatory TDD bug-fix workflow defined in CONTRIBUTING.md §Bug Fix Workflow. ### Changes - **Behave feature** (`features/tdd_init_yes_no_input.feature`): Two scenarios tagged `@tdd_expected_fail @tdd_bug @tdd_bug_783` that invoke `agents init --yes` in a fresh environment with the migration prompt simulated as declining (TTY with no input). The tests verify the command exits successfully and does not display the "Apply migrations now?" prompt. - **Behave steps** (`features/steps/tdd_init_yes_no_input_steps.py`): Step definitions that create an isolated temp directory, clear auto-apply and database-URL environment variables, patch `sys.stdin` on the real `sys` module (correct mock target per project convention), and replace `MigrationRunner._default_prompt_for_migration` with a function that returns `False` (simulating a TTY prompt where the user declines). Temp directory and env var cleanup is registered via `context.add_cleanup()`. - **Robot Framework test** (`robot/tdd_init_yes_no_input.robot`): Two integration test cases tagged `tdd_bug`, `tdd_bug_783`, `tdd_expected_fail` matching the Behave scenarios. - **Robot helper** (`robot/helper_tdd_init_yes_no_input.py`): Helper script that exercises the same code path using the same mock strategy as the Behave steps. ### Bug Reproduction Mechanism The core challenge is that `CliRunner` replaces `sys.stdin` during `invoke()`, making direct `isatty()` patching ineffective for controlling the prompt path. The mock strategy addresses this with a two-pronged approach: 1. **`patch.object(sys, "stdin", mock_stdin)`** — Patches `sys.stdin` directly on the real `sys` module (correct mock target per the project's established pattern in `features/steps/migration_runner_steps.py`). Documents the intent of simulating a TTY. 2. **`patch.object(MigrationRunner, "_default_prompt_for_migration", ...)`** — Replaces the prompt function with one that returns `False` (simulating a TTY user declining migration). This is the mechanism that actually exercises the bug path, since CliRunner's stdin replacement bypasses the `isatty()` check. 3. **`CLEVERAGENTS_DATABASE_URL` with non-template filename** — Sets the database URL to a path whose filename does not match the `before_scenario` template-DB prefixes, forcing the real Alembic migration path instead of the test fast-path. With bug #783 present, `require_confirmation=True` is hardcoded in `UnitOfWork._ensure_database_initialized()`, so the prompt fires, returns `False`, causing `MigrationNotApprovedError` and a non-zero exit code. After the fix, `--yes` should bypass the prompt entirely. ### Root Cause (for the bug fix developer) `init_command()` in `cleveragents.cli.commands.project` receives the `--yes` flag but only uses it to control output format. It does not forward `yes` to the migration runner. The migration runner's `init_or_upgrade()` is called via `unit_of_work._ensure_database_initialized()` with `require_confirmation=True` hardcoded. ### Scenario 2 Limitation While the bug is present, Scenario 2's first assertion (exit code 0) fails and Behave skips subsequent steps. The "contains Initialized" assertion is only evaluated after the bug is fixed, providing distinct post-fix regression value. ### Quality Gate Results - `nox -s lint` — ✅ passed - `nox -s typecheck` — ✅ passed (0 errors) - `nox -s unit_tests` — ✅ passed (387 features, 11121 scenarios, 0 failed) - `nox -s integration_tests` — ✅ passed (1561 tests, 0 failed) - `nox -s e2e_tests` — ✅ passed (16 tests, 0 failed) - `nox -s coverage_report` — ✅ passed (97% coverage) Closes #842 Reviewed-on: cleveragents/cleveragents-core#1049 Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
243 lines
8.3 KiB
Python
243 lines
8.3 KiB
Python
"""Helper script for tdd_init_yes_no_input.robot smoke tests.
|
|
|
|
Each subcommand exercises the real ``agents init --yes`` code path
|
|
simulating a TTY environment with no real stdin input to reproduce
|
|
bug #783. The helper reports the **real** outcome: it exits 0 and
|
|
prints the sentinel when the operation succeeds (bug is fixed), and
|
|
exits 1 when the bug is still present. The
|
|
``tdd_expected_fail_listener`` on the Robot side handles pass/fail
|
|
inversion while the bug remains open.
|
|
|
|
Mock strategy
|
|
~~~~~~~~~~~~~
|
|
CliRunner replaces ``sys.stdin`` during ``invoke()``, so patching
|
|
``sys.stdin.isatty()`` alone is ineffective. Instead we:
|
|
|
|
1. Patch ``sys.stdin`` on the real ``sys`` module (documents intent
|
|
and is the correct mock target per the project convention).
|
|
2. Patch ``MigrationRunner._default_prompt_for_migration`` with a
|
|
replacement that returns ``False`` — simulating a TTY prompt where
|
|
the user declines migration.
|
|
3. Set ``CLEVERAGENTS_DATABASE_URL`` to a path whose filename does
|
|
**not** match the ``before_scenario`` template-DB prefixes so the
|
|
real Alembic migration path runs.
|
|
|
|
With bug #783, ``require_confirmation=True`` is hardcoded, the prompt
|
|
fires and returns ``False``, causing ``MigrationNotApprovedError``.
|
|
After the fix, ``--yes`` should bypass the prompt entirely.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import shutil
|
|
import sys
|
|
import tempfile
|
|
from collections.abc import Callable
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
# Ensure local source tree is importable
|
|
_SRC = str(Path(__file__).resolve().parents[1] / "src")
|
|
if _SRC not in sys.path:
|
|
sys.path.insert(0, _SRC)
|
|
|
|
from typer.testing import CliRunner # noqa: E402
|
|
|
|
from cleveragents.cli.commands.project import app as project_app # noqa: E402
|
|
from cleveragents.infrastructure.database.migration_runner import ( # noqa: E402
|
|
MigrationRunner,
|
|
)
|
|
|
|
runner = CliRunner()
|
|
|
|
|
|
def _tty_prompt_declines(_message: str) -> bool:
|
|
"""Simulate a TTY migration prompt that declines (no user input).
|
|
|
|
Replaces ``_default_prompt_for_migration`` to reproduce the TTY
|
|
code path without depending on ``sys.stdin.isatty()``, which
|
|
CliRunner overrides during ``invoke()``.
|
|
"""
|
|
return False
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Subcommands
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def init_no_stdin() -> None:
|
|
"""Invoke ``agents init --yes`` simulating a TTY with no real input.
|
|
|
|
Patches ``sys.stdin`` on the real ``sys`` module for intent
|
|
documentation and replaces ``_default_prompt_for_migration`` with
|
|
a function that returns ``False`` (simulating a TTY prompt that
|
|
declines). See module docstring for the full mock strategy.
|
|
|
|
Exits 0 with sentinel when the command completes successfully
|
|
(bug is fixed). Exits 1 when the command fails (bug still
|
|
present).
|
|
"""
|
|
tmpdir = tempfile.mkdtemp(prefix="tdd_init_yes_robot_")
|
|
try:
|
|
# Remove env vars that would bypass the prompt
|
|
env_save: dict[str, str | None] = {}
|
|
for key in (
|
|
"CLEVERAGENTS_AUTO_APPLY_MIGRATIONS",
|
|
"CI",
|
|
"BEHAVE_TESTING",
|
|
"CLEVERAGENTS_HOME",
|
|
"CLEVERAGENTS_DATABASE_URL",
|
|
"CLEVERAGENTS_TEST_DATABASE_URL",
|
|
):
|
|
env_save[key] = os.environ.pop(key, None)
|
|
|
|
os.environ["CLEVERAGENTS_HOME"] = tmpdir
|
|
|
|
# Use a DB path whose filename does NOT start with the
|
|
# template-fast-path prefixes (``cleveragents_*``, ``test_*``,
|
|
# ``db.*``) so the real Alembic migration path runs.
|
|
tdd_db_path = os.path.join(tmpdir, ".cleveragents", "init_yes_test.sqlite")
|
|
os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{tdd_db_path}"
|
|
|
|
# Create a mock stdin that reports as a TTY
|
|
mock_stdin = MagicMock(spec=sys.stdin)
|
|
mock_stdin.isatty.return_value = True
|
|
|
|
# Patch sys.stdin on the real sys module and replace the
|
|
# prompt function to simulate a TTY decline.
|
|
with (
|
|
patch.object(sys, "stdin", mock_stdin),
|
|
patch.object(
|
|
MigrationRunner,
|
|
"_default_prompt_for_migration",
|
|
staticmethod(_tty_prompt_declines),
|
|
),
|
|
):
|
|
result = runner.invoke(
|
|
project_app,
|
|
["init", "--yes", "--path", tmpdir],
|
|
input="",
|
|
)
|
|
|
|
# Restore env vars
|
|
for key, val in env_save.items():
|
|
if val is not None:
|
|
os.environ[key] = val
|
|
else:
|
|
os.environ.pop(key, None)
|
|
|
|
if result.exit_code != 0:
|
|
print(
|
|
f"init --yes failed with exit code {result.exit_code}",
|
|
file=sys.stderr,
|
|
)
|
|
print(f"Output: {result.output}", file=sys.stderr)
|
|
sys.exit(1)
|
|
|
|
# Bug fixed — command succeeded without input.
|
|
print("tdd-init-yes-no-input-ok")
|
|
finally:
|
|
# Clean up tmpdir
|
|
shutil.rmtree(tmpdir, ignore_errors=True)
|
|
|
|
|
|
def init_no_prompt() -> None:
|
|
"""Invoke ``agents init --yes`` and verify no migration prompt appears.
|
|
|
|
Uses the same mock strategy as ``init_no_stdin``. See module
|
|
docstring for details.
|
|
|
|
Exits 0 with sentinel when the output does not contain the
|
|
migration prompt text (bug is fixed). Exits 1 when the prompt
|
|
text appears or the command fails (bug still present).
|
|
"""
|
|
tmpdir = tempfile.mkdtemp(prefix="tdd_init_yes_robot_")
|
|
try:
|
|
# Remove env vars that would bypass the prompt
|
|
env_save: dict[str, str | None] = {}
|
|
for key in (
|
|
"CLEVERAGENTS_AUTO_APPLY_MIGRATIONS",
|
|
"CI",
|
|
"BEHAVE_TESTING",
|
|
"CLEVERAGENTS_HOME",
|
|
"CLEVERAGENTS_DATABASE_URL",
|
|
"CLEVERAGENTS_TEST_DATABASE_URL",
|
|
):
|
|
env_save[key] = os.environ.pop(key, None)
|
|
|
|
os.environ["CLEVERAGENTS_HOME"] = tmpdir
|
|
|
|
# Use a DB path whose filename does NOT start with the
|
|
# template-fast-path prefixes.
|
|
tdd_db_path = os.path.join(tmpdir, ".cleveragents", "init_yes_test.sqlite")
|
|
os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{tdd_db_path}"
|
|
|
|
# Create a mock stdin that reports as a TTY
|
|
mock_stdin = MagicMock(spec=sys.stdin)
|
|
mock_stdin.isatty.return_value = True
|
|
|
|
# Patch sys.stdin on the real sys module and replace the
|
|
# prompt function to simulate a TTY decline.
|
|
with (
|
|
patch.object(sys, "stdin", mock_stdin),
|
|
patch.object(
|
|
MigrationRunner,
|
|
"_default_prompt_for_migration",
|
|
staticmethod(_tty_prompt_declines),
|
|
),
|
|
):
|
|
result = runner.invoke(
|
|
project_app,
|
|
["init", "--yes", "--path", tmpdir],
|
|
input="",
|
|
)
|
|
|
|
# Restore env vars
|
|
for key, val in env_save.items():
|
|
if val is not None:
|
|
os.environ[key] = val
|
|
else:
|
|
os.environ.pop(key, None)
|
|
|
|
if result.exit_code != 0:
|
|
print(
|
|
f"init --yes failed with exit code {result.exit_code}",
|
|
file=sys.stderr,
|
|
)
|
|
print(f"Output: {result.output}", file=sys.stderr)
|
|
sys.exit(1)
|
|
|
|
# Check that no migration prompt text appears
|
|
output = result.output.lower()
|
|
if "apply migrations now" in output:
|
|
print(
|
|
"Bug present: migration prompt appeared in output",
|
|
file=sys.stderr,
|
|
)
|
|
sys.exit(1)
|
|
|
|
# Bug fixed — no prompt appeared.
|
|
print("tdd-init-yes-no-prompt-ok")
|
|
finally:
|
|
# Clean up tmpdir
|
|
shutil.rmtree(tmpdir, ignore_errors=True)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Dispatcher
|
|
# ---------------------------------------------------------------------------
|
|
|
|
_COMMANDS: dict[str, Callable[[], None]] = {
|
|
"init-no-stdin": init_no_stdin,
|
|
"init-no-prompt": init_no_prompt,
|
|
}
|
|
|
|
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 = _COMMANDS[sys.argv[1]]
|
|
cmd()
|