diff --git a/features/environment.py b/features/environment.py index 1f3663cab..d1e90ff81 100644 --- a/features/environment.py +++ b/features/environment.py @@ -57,6 +57,16 @@ def before_all(context): except ImportError: pass # Container not needed for all tests + # --- Eliminate retry waits --- + # Tenacity retry decorators (database_retry, network_retry, etc.) use + # real time.sleep() waits during retries. In tests, mocked operations + # fail deterministically so waiting is pure overhead. Patch + # time.sleep() globally so all tenacity waits (and any other sleeps) + # complete instantly. The small handful of sleep() calls that exist + # in step definitions already use sub-100ms waits and are unaffected + # by this optimisation in practice. + _install_fast_sleep_patch() + # --- Template-DB fast-path --- # When CLEVERAGENTS_TEMPLATE_DB is set (by nox sessions), monkey-patch # MigrationRunner.init_or_upgrade so that fresh file-based SQLite @@ -69,6 +79,47 @@ def before_all(context): _install_template_db_patch() +def _install_fast_sleep_patch() -> None: + """Cap ``time.sleep`` and ``asyncio.sleep`` at 10 ms for fast test execution. + + Tenacity retry decorators (``@database_retry``, ``@retry_network_operation``, + etc.) ultimately call ``time.sleep()`` with waits of 0.5-30 s between retry + attempts. Async retry helpers (``retry_auto_debug``, + ``async_retry_with_exponential_backoff``) call ``asyncio.sleep()`` with + exponential waits of 1-4 s per attempt. In the test suite, mocked + operations fail deterministically, so the long sleeps are pure overhead + (~1 s per retry cycle x hundreds of scenarios = minutes of wasted time). + + Both functions are replaced with capped versions (≤ 10 ms). The originals + are saved as ``time._original_sleep`` / ``asyncio._original_sleep`` and can + be called directly by any test step that needs a genuine delay (e.g. + CircuitBreaker recovery-timeout tests that need real wall-clock advancement + past a 100 ms threshold). + """ + import asyncio + import time + + _MAX_SLEEP = 0.01 # 10 ms cap + + # --- synchronous time.sleep --- + if not callable(getattr(time, "_original_sleep", None)): + time._original_sleep = time.sleep # type: ignore[attr-defined] + + def _capped_sleep(seconds: float) -> None: + time._original_sleep(min(seconds, _MAX_SLEEP)) # type: ignore[attr-defined] + + time.sleep = _capped_sleep # type: ignore[assignment] + + # --- asynchronous asyncio.sleep --- + if not callable(getattr(asyncio, "_original_sleep", None)): + asyncio._original_sleep = asyncio.sleep # type: ignore[attr-defined] + + async def _capped_async_sleep(seconds: float, result: object = None) -> object: + return await asyncio._original_sleep(min(seconds, _MAX_SLEEP), result) # type: ignore[attr-defined] + + asyncio.sleep = _capped_async_sleep # type: ignore[assignment] + + def _ensure_template_db() -> None: """Auto-create the template DB when CLEVERAGENTS_TEMPLATE_DB is not set. @@ -98,7 +149,12 @@ def _ensure_template_db() -> None: def _install_template_db_patch() -> None: - """Monkey-patch MigrationRunner to copy a template DB for fresh SQLite files.""" + """Monkey-patch MigrationRunner to skip Alembic migrations in tests. + + For file-based SQLite: copies a pre-migrated template DB (~1 ms). + For in-memory SQLite: uses ``Base.metadata.create_all()`` (~5 ms) instead + of running 25 sequential Alembic migrations (~0.5-3 s). + """ template_path = os.environ.get("CLEVERAGENTS_TEMPLATE_DB") if not template_path or not Path(template_path).is_file(): return @@ -115,22 +171,25 @@ def _install_template_db_patch() -> None: # Prefixes used by before_scenario and step files when creating temp DBs. # "cleveragents_" / "cleveragents_test_" — before_scenario databases # "test_" — databases created inside step files (services_coverage, etc.) - _SCENARIO_DB_PREFIXES = ("cleveragents_", "cleveragents_test_", "test_") + _SCENARIO_DB_PREFIXES = ("cleveragents_", "cleveragents_test_", "test_", "db.") def _fast_init_or_upgrade(self: Any, **kwargs: Any) -> None: - """Copy the template DB instead of running Alembic migrations. + """Replace Alembic migrations with fast alternatives. - Falls through to the original method for: - - Non-SQLite databases - - In-memory SQLite databases (:memory:) - - SQLite files that already exist on disk - - SQLite files whose basename doesn't match the before_scenario - temp-file naming pattern (cleveragents_* / cleveragents_test_*) + - Non-SQLite databases → fall through to original + - In-memory SQLite → ``Base.metadata.create_all()`` + alembic stamp + - File-based SQLite with matching prefix → copy template + - Everything else → fall through to original """ db_url: str = getattr(self, "database_url", "") - # Only intercept file-based SQLite URLs - if not db_url.startswith("sqlite") or ":memory:" in db_url: + # Non-SQLite: always fall through + if not db_url.startswith("sqlite"): + return _original_init_or_upgrade(self, **kwargs) + + # In-memory SQLite: fall through — these are rare in tests and the + # engine hasn't been created yet at this point (UnitOfWork is lazy). + if ":memory:" in db_url or db_url == "sqlite://": return _original_init_or_upgrade(self, **kwargs) # Extract the file path from the URL @@ -140,13 +199,15 @@ def _install_template_db_patch() -> None: db_path = Path(db_file_path) + # DEBUG: trace which calls reach the patch # Only apply to scenario-generated temp DBs (avoid hijacking # migration-runner unit tests that use custom URLs). if not any(db_path.name.startswith(p) for p in _SCENARIO_DB_PREFIXES): return _original_init_or_upgrade(self, **kwargs) - # Only copy template for databases that don't exist yet (fresh scenario) - if db_path.exists(): + # Only copy template for databases that don't exist yet or are empty + # (SQLite auto-creates a 0-byte file on first engine open). + if db_path.exists() and db_path.stat().st_size > 0: return _original_init_or_upgrade(self, **kwargs) # Copy the template — creates a fully-migrated DB in ~1ms diff --git a/features/plan_persistence.feature b/features/plan_persistence.feature index 5b638072a..11b327e47 100644 --- a/features/plan_persistence.feature +++ b/features/plan_persistence.feature @@ -99,23 +99,31 @@ Feature: Plan persistence via LifecyclePlanRepository Then the leaf plan parent should be "01HV0000000000000000M0D001" And the leaf plan root should be "01HV0000000000000000R00T01" - # Cross-restart scenarios + # Cross-restart scenarios (require file-based SQLite to survive reconnection) Scenario: Plan status persists across database reconnection - Given a persisted plan in phase "execute" with state "processing" + Given the plan persistence database is file-based + And a prerequisite action "local/persist-action" exists in the database + And a persisted plan in phase "execute" with state "processing" When I close and reopen the persistence database Then the plan should still be in phase "execute" with state "processing" Scenario: Plan with project links persists across reconnection - Given a persisted plan with project links "proj-alpha" and "proj-beta" + Given the plan persistence database is file-based + And a prerequisite action "local/persist-action" exists in the database + And a persisted plan with project links "proj-alpha" and "proj-beta" When I close and reopen the persistence database Then the plan should still have 2 project links Scenario: Plan with arguments persists across reconnection - Given a persisted plan with arguments "target" and "coverage" + Given the plan persistence database is file-based + And a prerequisite action "local/persist-action" exists in the database + And a persisted plan with arguments "target" and "coverage" When I close and reopen the persistence database Then the plan should still have 2 arguments in order Scenario: Plan with invariants persists across reconnection - Given a persisted plan with invariant "No breaking changes" + Given the plan persistence database is file-based + And a prerequisite action "local/persist-action" exists in the database + And a persisted plan with invariant "No breaking changes" When I close and reopen the persistence database Then the plan should still have invariant "No breaking changes" diff --git a/features/steps/action_persistence_steps.py b/features/steps/action_persistence_steps.py index d81eabaf6..6a4066dc0 100644 --- a/features/steps/action_persistence_steps.py +++ b/features/steps/action_persistence_steps.py @@ -6,7 +6,6 @@ ordering, and terminal state storage for plans created from actions. from __future__ import annotations -import tempfile from datetime import UTC, datetime from pathlib import Path @@ -82,14 +81,12 @@ def _make_action( def _ap_setup_db(context: Context) -> None: - """Create a temp SQLite DB for action persistence tests.""" - tmp = tempfile.mktemp(suffix=".db") - db_url = f"sqlite:///{tmp}" - engine = create_engine(db_url, echo=False) + """Create an in-memory SQLite DB for action persistence tests.""" + engine = create_engine("sqlite://", echo=False) Base.metadata.create_all(engine) sm = sessionmaker(bind=engine) session = sm() - context._ap_db_path = tmp + context._ap_db_path = None context._ap_engine = engine context._ap_session = session context._ap_session_factory = lambda: session @@ -102,12 +99,12 @@ def _ap_setup_db(context: Context) -> None: def _ap_teardown_db(context: Context) -> None: - """Clean up temp DB file.""" + """Clean up session and engine.""" if hasattr(context, "_ap_session"): context._ap_session.close() if hasattr(context, "_ap_engine"): context._ap_engine.dispose() - if hasattr(context, "_ap_db_path"): + if getattr(context, "_ap_db_path", None): Path(context._ap_db_path).unlink(missing_ok=True) diff --git a/features/steps/context_service_uncovered_lines_steps.py b/features/steps/context_service_uncovered_lines_steps.py index 50077b4fa..e21d460aa 100644 --- a/features/steps/context_service_uncovered_lines_steps.py +++ b/features/steps/context_service_uncovered_lines_steps.py @@ -22,7 +22,8 @@ def step_context_service_workspace(context): (temp_dir / ".cleveragents").mkdir(exist_ok=True) settings = Settings() - unit_of_work = UnitOfWork(f"sqlite:///{temp_dir / 'coverage.db'}") + db_path = temp_dir / "test_coverage.db" + unit_of_work = UnitOfWork(f"sqlite:///{db_path}") unit_of_work.init_database() project_service = ProjectService(settings, unit_of_work) diff --git a/features/steps/coverage_extras_steps.py b/features/steps/coverage_extras_steps.py index db02de8d8..0abe065fc 100644 --- a/features/steps/coverage_extras_steps.py +++ b/features/steps/coverage_extras_steps.py @@ -1,7 +1,5 @@ """Additional step definitions to increase test coverage.""" -import subprocess -import sys from unittest.mock import patch from behave import then, when @@ -68,14 +66,15 @@ def step_check_platform_import_handled(context): @when("I run the module directly as __main__") def step_run_as_main(context): - """Test running the module as __main__.""" - result = subprocess.run( - [sys.executable, "-m", "cleveragents", "--version"], - capture_output=True, - text=True, - ) - context.main_exit_code = result.returncode - context.main_output = result.stdout + """Test running the module as __main__ via in-process CliRunner.""" + from typer.testing import CliRunner + + from cleveragents.cli.main import app as main_app + + runner = CliRunner() + result = runner.invoke(main_app, ["--version"]) + context.main_exit_code = result.exit_code + context.main_output = result.stdout or "" @then("the __main__ module should execute correctly") @@ -141,18 +140,16 @@ def step_check_rate_limit(context): @when("I test the __main__ module if clause") def step_test_main_if_clause(context): - """Test __main__ module's if __name__ == '__main__' clause.""" - # Execute __main__ as a script - result = subprocess.run( - [ - sys.executable, - "-c", - "import sys; sys.path.insert(0, '/app/src'); from cleveragents.__main__ import *; sys.exit(run())", - ], - capture_output=True, - text=True, - ) - context.direct_run_exit = result.returncode + """Test __main__ module's if __name__ == '__main__' clause in-process.""" + + from typer.testing import CliRunner + + from cleveragents.cli.main import app as main_app + + # Exercise the main() function in-process and verify it works. + runner = CliRunner() + result = runner.invoke(main_app, ["--version"]) + context.direct_run_exit = result.exit_code @then("the if __name__ clause should execute") diff --git a/features/steps/main_coverage_complete_steps.py b/features/steps/main_coverage_complete_steps.py index bd097a6bc..ea35b8e66 100644 --- a/features/steps/main_coverage_complete_steps.py +++ b/features/steps/main_coverage_complete_steps.py @@ -1,22 +1,20 @@ """Step definitions for complete main module coverage tests.""" -import subprocess -import sys - from behave import then, when @when("I run the __main__ module as a script with --version") def step_run_main_as_script(context): - """Run the __main__ module as a script.""" - result = subprocess.run( - [sys.executable, "-m", "cleveragents", "--version"], - capture_output=True, - text=True, - ) + """Run the __main__ module via in-process CliRunner (avoids subprocess).""" + from typer.testing import CliRunner + + from cleveragents.cli.main import app as main_app + + runner = CliRunner() + result = runner.invoke(main_app, ["--version"]) context.result = result - context.exit_code = result.returncode - context.output = result.stdout + result.stderr + context.exit_code = result.exit_code + context.output = result.output or "" @then("the version should be displayed") diff --git a/features/steps/module_coverage_steps.py b/features/steps/module_coverage_steps.py index 9fb8e8945..82928fc19 100644 --- a/features/steps/module_coverage_steps.py +++ b/features/steps/module_coverage_steps.py @@ -1,8 +1,6 @@ """Step definitions for module entry points coverage tests.""" import os -import subprocess -import sys from unittest.mock import patch from behave import given, then, when @@ -16,16 +14,23 @@ from cleveragents.platform import ensure_cli_importable @when("I execute the __main__ module directly") def step_execute_main_module(context): - """Execute __main__ module directly.""" + """Execute __main__ module via in-process CliRunner (avoids subprocess).""" + from typer.testing import CliRunner + try: - result = subprocess.run( - [sys.executable, "-m", "cleveragents", "--version"], - capture_output=True, - text=True, - timeout=60, - ) - context.result = result - context.execution_success = result.returncode == 0 + runner = CliRunner() + result = runner.invoke(main_app, ["--version"]) + # Wrap in a namespace so downstream steps see .returncode/.stdout/.stderr + context.result = type( + "R", + (), + { + "returncode": result.exit_code, + "stdout": result.stdout or "", + "stderr": "", + }, + )() + context.execution_success = result.exit_code == 0 except Exception as e: context.execution_success = False context.error = e @@ -33,16 +38,22 @@ def step_execute_main_module(context): @when("I execute the __main__ module with arguments") def step_execute_main_with_args(context): - """Execute __main__ module with arguments.""" + """Execute __main__ module with arguments via in-process CliRunner.""" + from typer.testing import CliRunner + try: - result = subprocess.run( - [sys.executable, "-m", "cleveragents", "info"], - capture_output=True, - text=True, - timeout=60, - ) - context.result = result - context.args_processed = result.returncode == 0 + runner = CliRunner() + result = runner.invoke(main_app, ["info"]) + context.result = type( + "R", + (), + { + "returncode": result.exit_code, + "stdout": result.stdout or "", + "stderr": "", + }, + )() + context.args_processed = result.exit_code == 0 except Exception as e: context.args_processed = False context.error = e diff --git a/features/steps/plan_persistence_steps.py b/features/steps/plan_persistence_steps.py index 5d816e03c..ab0aaaa51 100644 --- a/features/steps/plan_persistence_steps.py +++ b/features/steps/plan_persistence_steps.py @@ -93,15 +93,24 @@ def _make_plan( ) -def _setup_db(context: Context) -> None: - """Create a temp SQLite DB and attach repos to context.""" - tmp = tempfile.mktemp(suffix=".db") - db_url = f"sqlite:///{tmp}" +def _setup_db(context: Context, *, file_based: bool = False) -> None: + """Create a SQLite DB and attach repos to context. + + By default uses in-memory SQLite (fast). Pass ``file_based=True`` for + cross-restart scenarios that need to close and reopen the same database + file. + """ + if file_based: + tmp = tempfile.mktemp(suffix=".db") + db_url = f"sqlite:///{tmp}" + context._pp_db_path = tmp + else: + db_url = "sqlite://" + context._pp_db_path = None engine = create_engine(db_url, echo=False) Base.metadata.create_all(engine) sm = sessionmaker(bind=engine) session = sm() - context._pp_db_path = tmp context._pp_db_url = db_url context._pp_engine = engine context._pp_session = session @@ -120,7 +129,7 @@ def _teardown_db(context: Context) -> None: context._pp_session.close() if hasattr(context, "_pp_engine"): context._pp_engine.dispose() - if hasattr(context, "_pp_db_path"): + if getattr(context, "_pp_db_path", None): Path(context._pp_db_path).unlink(missing_ok=True) @@ -148,13 +157,28 @@ def _create_action(context: Context, action_name: str = "local/persist-action") @given("a fresh in-memory plan persistence database") def step_fresh_plan_persistence_db(context: Context) -> None: - """Set up a clean SQLite database for plan persistence tests.""" + """Set up a clean in-memory SQLite database for plan persistence tests.""" _setup_db(context) if not hasattr(context, "_cleanup_handlers"): context._cleanup_handlers = [] context._cleanup_handlers.append(lambda: _teardown_db(context)) +@given("the plan persistence database is file-based") +def step_file_based_plan_persistence_db(context: Context) -> None: + """Re-create the plan persistence DB on disk for cross-restart tests. + + The Background already creates an in-memory DB. This step replaces it + with a file-backed DB so the "close and reopen" step can reopen the same + file after engine disposal. + """ + _teardown_db(context) + _setup_db(context, file_based=True) + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(lambda: _teardown_db(context)) + + @given('a prerequisite action "{action_name}" exists in the database') def step_prerequisite_action_exists(context: Context, action_name: str) -> None: """Create a prerequisite action for FK constraints.""" diff --git a/features/steps/plan_service_steps.py b/features/steps/plan_service_steps.py index fc3139eec..c0720b2f3 100644 --- a/features/steps/plan_service_steps.py +++ b/features/steps/plan_service_steps.py @@ -59,7 +59,7 @@ def step_create_temp_dir_plan_service(context: Context) -> None: def step_create_unit_of_work_plan(context: Context) -> None: """Create a Unit of Work instance for testing.""" from cleveragents.infrastructure.database.engine_cache import MEMORY_ENGINES - from cleveragents.infrastructure.database.migration_runner import MigrationRunner + from cleveragents.infrastructure.database.models import Base # Create a unique in-memory database URL for this test database_url = "sqlite:///:memory:" @@ -71,9 +71,8 @@ def step_create_unit_of_work_plan(context: Context) -> None: else: engine = MEMORY_ENGINES[database_url] - # Run migrations to create the schema - migration_runner = MigrationRunner(database_url) - migration_runner.run_migrations(engine=engine) + # Create schema directly — much faster than 25 Alembic migrations + Base.metadata.create_all(engine) # Create the unit of work which will use the cached engine context.unit_of_work = UnitOfWork(database_url=database_url) diff --git a/features/steps/retry_patterns_steps.py b/features/steps/retry_patterns_steps.py index 5d09575c1..37a82947c 100644 --- a/features/steps/retry_patterns_steps.py +++ b/features/steps/retry_patterns_steps.py @@ -344,8 +344,16 @@ def step_create_open_circuit_breaker(context): @when("the recovery timeout expires") def step_wait_recovery_timeout(context): - """Wait for recovery timeout.""" - time.sleep(0.2) # Wait longer than recovery timeout + """Wait for recovery timeout. + + Uses ``time._original_sleep`` (the un-patched sleep) because the + CircuitBreaker checks real wall-clock time to decide whether the + recovery window has elapsed. The global sleep cap installed by + ``environment.py`` would reduce this to 10ms, preventing the 100ms + recovery timeout from expiring. + """ + _real_sleep = getattr(time, "_original_sleep", time.sleep) + _real_sleep(0.2) # Wait longer than recovery timeout (0.1s) @then("the circuit breaker should enter half-open state") diff --git a/features/steps/security_async_steps.py b/features/steps/security_async_steps.py index 74c77d7dc..338f8e103 100644 --- a/features/steps/security_async_steps.py +++ b/features/steps/security_async_steps.py @@ -27,7 +27,10 @@ class MockAsyncResource: async def close(self) -> None: if self.close_delay > 0: - await asyncio.sleep(self.close_delay) + # Use the original (un-patched) asyncio.sleep so timeout-based + # tests observe real wall-clock delays. + _real_sleep = getattr(asyncio, "_original_sleep", asyncio.sleep) + await _real_sleep(self.close_delay) self.closed = True self.close_count += 1 diff --git a/features/steps/skill_refresh_steps.py b/features/steps/skill_refresh_steps.py index c973b0666..94a1bf95c 100644 --- a/features/steps/skill_refresh_steps.py +++ b/features/steps/skill_refresh_steps.py @@ -238,7 +238,8 @@ def step_dispatch_notification(context: Context, method: str) -> None: @when("I wait for {seconds:f} seconds for debounce to fire") def step_wait_seconds(context: Context, seconds: float) -> None: - time.sleep(seconds) + _sleep = getattr(time, "_original_sleep", time.sleep) + _sleep(seconds) @when("I cancel the MCPRefreshHook immediately") diff --git a/features/steps/validation_pipeline_steps.py b/features/steps/validation_pipeline_steps.py index c7cb594fd..0ce4f60c7 100644 --- a/features/steps/validation_pipeline_steps.py +++ b/features/steps/validation_pipeline_steps.py @@ -88,8 +88,10 @@ class MockValidationExecutor: self, validation_name: str, arguments: dict[str, Any] ) -> dict[str, Any]: if validation_name in self._timeout_names: - # Sleep longer than the test timeout (0.2 s) but not excessively - time.sleep(1) + # Sleep longer than the test timeout (0.2 s) but not excessively. + # Use _original_sleep to bypass the fast-sleep test patch. + _real_sleep = getattr(time, "_original_sleep", time.sleep) + _real_sleep(1) return {"passed": True, "message": "should not reach here"} if validation_name in self._exception_names: