forked from HAL9000/cleveragents-core
b21e0fedea
Add 8 BDD scenarios in features/fast_init_upgrade.feature that directly exercise the _fast_init_or_upgrade closure installed by _install_template_db_patch in features/environment.py. Scenarios cover all code paths: - Non-empty DB with matching prefix → early return (original NOT invoked) - Non-existent DB with matching prefix → template copied (original NOT invoked) - Existing empty DB with matching prefix → template copied (original NOT invoked) - Non-matching prefix → delegates to original init_or_upgrade - In-memory SQLite → delegates to original init_or_upgrade - Non-SQLite URL → delegates to original init_or_upgrade - Bare sqlite:// URL → delegates to original init_or_upgrade - Delegation forwards runner instance and keyword arguments correctly Test infrastructure: - features/mocks/fast_init_test_helpers.py provides a context manager (patch_original_init_or_upgrade) that replaces the _original_init_or_upgrade reference inside the closure via cell_contents manipulation, enabling precise call-tracking without recreating the function under test. - All scenarios tagged @mock_only to skip unnecessary DB setup. - Cleanup registered via context._cleanup_handlers for temp file removal. - Works in both sequential and parallel execution modes. Review fix round: - Added bare sqlite:// URL scenario covering the second branch of the in-memory disjunction (L1). - Added scenario verifying runner instance and keyword argument forwarding through the delegation path (L2, L3). - Replaced hardcoded test credentials with clearly synthetic pattern (I4). - Fixed CHANGELOG wording that incorrectly claimed mktemp replacement (L5). ISSUES CLOSED: #733
285 lines
11 KiB
Python
285 lines
11 KiB
Python
"""Step definitions for fast_init_upgrade.feature.
|
|
|
|
Exercises the ``_fast_init_or_upgrade`` closure installed by
|
|
``features.environment._install_template_db_patch`` — specifically the
|
|
early-return path for non-empty databases, the template-copy path for
|
|
new databases, and the fallback delegation to the original
|
|
``MigrationRunner.init_or_upgrade``.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import contextlib
|
|
import filecmp
|
|
import os
|
|
import shutil
|
|
import tempfile
|
|
import types
|
|
from pathlib import Path
|
|
from typing import Any
|
|
|
|
from behave import given, then, when
|
|
from behave.runner import Context
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Internal helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _register_path_cleanup(context: Context, path: str) -> None:
|
|
"""Schedule *path* (and its SQLite sidecar files) for removal."""
|
|
if not hasattr(context, "_cleanup_handlers"):
|
|
context._cleanup_handlers = []
|
|
|
|
def _remove() -> None:
|
|
for suffix in ("", "-journal", "-wal", "-shm"):
|
|
with contextlib.suppress(OSError):
|
|
os.unlink(path + suffix)
|
|
|
|
context._cleanup_handlers.append(_remove)
|
|
|
|
|
|
def _register_directory_cleanup(context: Context, path: str) -> None:
|
|
"""Schedule directory *path* for recursive removal."""
|
|
if not hasattr(context, "_cleanup_handlers"):
|
|
context._cleanup_handlers = []
|
|
|
|
def _remove() -> None:
|
|
with contextlib.suppress(OSError):
|
|
shutil.rmtree(path)
|
|
|
|
context._cleanup_handlers.append(_remove)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Background
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("the fast-init template-DB patch is installed")
|
|
def step_verify_patch_installed(context: Context) -> None:
|
|
"""Verify that ``_install_template_db_patch`` has been applied."""
|
|
from cleveragents.infrastructure.database.migration_runner import (
|
|
MigrationRunner,
|
|
)
|
|
|
|
fast_fn: Any = MigrationRunner.init_or_upgrade
|
|
if not isinstance(fast_fn, types.FunctionType):
|
|
raise AssertionError(
|
|
"MigrationRunner.init_or_upgrade is not a plain function — "
|
|
"the _fast_init_or_upgrade patch does not appear to be installed. "
|
|
"Ensure CLEVERAGENTS_TEMPLATE_DB is set and the template exists."
|
|
)
|
|
|
|
freevars: tuple[str, ...] = fast_fn.__code__.co_freevars
|
|
if "_original_init_or_upgrade" not in freevars:
|
|
raise AssertionError(
|
|
"MigrationRunner.init_or_upgrade does not capture "
|
|
"'_original_init_or_upgrade' — the template-DB patch is not "
|
|
"installed as expected."
|
|
)
|
|
|
|
# Store template path for later steps.
|
|
template_db: str | None = os.environ.get("CLEVERAGENTS_TEMPLATE_DB")
|
|
if not template_db or not Path(template_db).is_file():
|
|
raise AssertionError(
|
|
"CLEVERAGENTS_TEMPLATE_DB is not set or the file does not exist."
|
|
)
|
|
context.fast_init_template_path = template_db
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Given — non-empty DB with matching prefix
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a non-empty SQLite database file with a matching scenario prefix")
|
|
def step_create_nonempty_db(context: Context) -> None:
|
|
"""Create a temporary non-empty file whose name starts with a matching prefix."""
|
|
fd, path = tempfile.mkstemp(suffix=".db", prefix="test_fastinit_")
|
|
os.write(fd, b"non-empty-content-to-simulate-migrated-db")
|
|
os.close(fd)
|
|
context.fast_init_db_path = path
|
|
context.fast_init_db_url = f"sqlite:///{path}"
|
|
context.fast_init_original_content = Path(path).read_bytes()
|
|
_register_path_cleanup(context, path)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Given — template-copy path (new DB)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a non-existent SQLite database path with a matching scenario prefix")
|
|
def step_create_nonexistent_db_path(context: Context) -> None:
|
|
"""Generate a path for a non-existent file with a matching prefix."""
|
|
temp_dir = tempfile.mkdtemp(prefix="test_fastinit_new_")
|
|
path = str(Path(temp_dir) / "test_fastinit.db")
|
|
context.fast_init_db_path = path
|
|
context.fast_init_db_url = f"sqlite:///{path}"
|
|
_register_path_cleanup(context, path)
|
|
_register_directory_cleanup(context, temp_dir)
|
|
|
|
|
|
@given("an empty SQLite database file with a matching scenario prefix")
|
|
def step_create_empty_db(context: Context) -> None:
|
|
"""Create an existing zero-byte DB file with a matching prefix."""
|
|
fd, path = tempfile.mkstemp(suffix=".db", prefix="test_fastinit_empty_")
|
|
os.close(fd)
|
|
context.fast_init_db_path = path
|
|
context.fast_init_db_url = f"sqlite:///{path}"
|
|
_register_path_cleanup(context, path)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Given — fallback paths (non-matching / in-memory / non-SQLite)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a SQLite database URL whose filename has a non-matching prefix")
|
|
def step_create_nonmatching_prefix_url(context: Context) -> None:
|
|
"""Create a URL whose filename prefix is not in _SCENARIO_DB_PREFIXES."""
|
|
fd, path = tempfile.mkstemp(suffix=".db", prefix="nomatch_fastinit_")
|
|
os.close(fd)
|
|
context.fast_init_db_path = path
|
|
context.fast_init_db_url = f"sqlite:///{path}"
|
|
_register_path_cleanup(context, path)
|
|
|
|
|
|
@given("an in-memory SQLite database URL for fast-init testing")
|
|
def step_create_inmemory_url(context: Context) -> None:
|
|
"""Set up an in-memory SQLite URL."""
|
|
context.fast_init_db_path = None
|
|
context.fast_init_db_url = "sqlite:///:memory:"
|
|
|
|
|
|
@given("a non-SQLite database URL for fast-init testing")
|
|
def step_create_nonsqlite_url(context: Context) -> None:
|
|
"""Set up a non-SQLite URL (PostgreSQL placeholder)."""
|
|
context.fast_init_db_path = None
|
|
context.fast_init_db_url = "postgresql://fake_user:fake_pass@localhost/testdb"
|
|
|
|
|
|
@given("a bare sqlite:// database URL for fast-init testing")
|
|
def step_create_bare_sqlite_url(context: Context) -> None:
|
|
"""Set up a bare ``sqlite://`` URL (no path, no ``:memory:``)."""
|
|
context.fast_init_db_path = None
|
|
context.fast_init_db_url = "sqlite://"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# When
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("I call init_or_upgrade on the fast-init database")
|
|
def step_call_init_or_upgrade(context: Context) -> None:
|
|
"""Invoke ``MigrationRunner.init_or_upgrade`` via the fast-init patch."""
|
|
from cleveragents.infrastructure.database.migration_runner import (
|
|
MigrationRunner,
|
|
)
|
|
from features.mocks.fast_init_test_helpers import (
|
|
patch_original_init_or_upgrade,
|
|
)
|
|
|
|
runner = MigrationRunner(database_url=context.fast_init_db_url)
|
|
|
|
with patch_original_init_or_upgrade() as mock_original:
|
|
runner.init_or_upgrade()
|
|
context.fast_init_mock_called = mock_original.called
|
|
context.fast_init_mock_call_count = mock_original.call_count
|
|
context.fast_init_mock_call_args = mock_original.call_args
|
|
context.fast_init_runner = runner
|
|
|
|
|
|
@when("I call init_or_upgrade with keyword arguments on the fast-init database")
|
|
def step_call_init_or_upgrade_with_kwargs(context: Context) -> None:
|
|
"""Invoke ``init_or_upgrade`` with keyword arguments to verify forwarding."""
|
|
from cleveragents.infrastructure.database.migration_runner import (
|
|
MigrationRunner,
|
|
)
|
|
from features.mocks.fast_init_test_helpers import (
|
|
patch_original_init_or_upgrade,
|
|
)
|
|
|
|
runner = MigrationRunner(database_url=context.fast_init_db_url)
|
|
|
|
with patch_original_init_or_upgrade() as mock_original:
|
|
runner.init_or_upgrade(require_confirmation=True)
|
|
context.fast_init_mock_called = mock_original.called
|
|
context.fast_init_mock_call_count = mock_original.call_count
|
|
context.fast_init_mock_call_args = mock_original.call_args
|
|
context.fast_init_runner = runner
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Then — original invocation assertions
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@then("the original init_or_upgrade should not have been invoked")
|
|
def step_assert_original_not_called(context: Context) -> None:
|
|
"""Assert the mocked original was never called."""
|
|
assert not context.fast_init_mock_called, (
|
|
"Expected _original_init_or_upgrade NOT to be called, "
|
|
f"but it was called {context.fast_init_mock_call_count} time(s)."
|
|
)
|
|
|
|
|
|
@then("the original init_or_upgrade should have been invoked exactly once")
|
|
def step_assert_original_called_once(context: Context) -> None:
|
|
"""Assert the mocked original was called exactly once."""
|
|
assert context.fast_init_mock_call_count == 1, (
|
|
"Expected _original_init_or_upgrade to be called exactly once, "
|
|
f"but it was called {context.fast_init_mock_call_count} time(s)."
|
|
)
|
|
|
|
|
|
@then("the original should have received the runner instance and keyword arguments")
|
|
def step_assert_original_received_runner_and_kwargs(context: Context) -> None:
|
|
"""Assert the mock was called with the correct runner and kwargs."""
|
|
call_args = context.fast_init_mock_call_args
|
|
assert call_args is not None, (
|
|
"Expected _original_init_or_upgrade to have been called, but call_args is None."
|
|
)
|
|
positional = call_args.args
|
|
assert len(positional) >= 1, (
|
|
"Expected _original_init_or_upgrade to receive the runner as "
|
|
f"first positional argument, but got {len(positional)} positional args."
|
|
)
|
|
assert positional[0] is context.fast_init_runner, (
|
|
"Expected the first positional argument to be the MigrationRunner "
|
|
"instance, but a different object was passed."
|
|
)
|
|
assert call_args.kwargs.get("require_confirmation") is True, (
|
|
"Expected keyword argument 'require_confirmation=True' to be "
|
|
f"forwarded, but got kwargs={call_args.kwargs}."
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Then — file-content assertions
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@then("the database file content should be unchanged")
|
|
def step_assert_file_unchanged(context: Context) -> None:
|
|
"""Assert the DB file still has its original content."""
|
|
current_content = Path(context.fast_init_db_path).read_bytes()
|
|
assert current_content == context.fast_init_original_content, (
|
|
"Database file content changed unexpectedly."
|
|
)
|
|
|
|
|
|
@then("the database file should be a copy of the template")
|
|
def step_assert_file_is_template_copy(context: Context) -> None:
|
|
"""Assert the DB file was created as a copy of the template."""
|
|
db_path = Path(context.fast_init_db_path)
|
|
assert db_path.exists(), f"Database file was not created at {db_path}"
|
|
assert db_path.stat().st_size > 0, "Database file is empty"
|
|
assert filecmp.cmp(
|
|
str(db_path),
|
|
context.fast_init_template_path,
|
|
shallow=False,
|
|
), "Database file does not match the template"
|