diff --git a/features/steps/tdd_migration_runner_get_current_revision_threading_10507_steps.py b/features/steps/tdd_migration_runner_get_current_revision_threading_10507_steps.py index 663fe3eea..ca6cb4f95 100644 --- a/features/steps/tdd_migration_runner_get_current_revision_threading_10507_steps.py +++ b/features/steps/tdd_migration_runner_get_current_revision_threading_10507_steps.py @@ -8,79 +8,15 @@ from unittest.mock import MagicMock, patch from behave import then, when +def _run_get_current_revision_and_capture_kwargs( + context: Any, +) -> None: + """Shared helper: call get_current_revision() and capture create_engine kwargs. -@when("I request the current revision and capture the engine creation args") -def step_when_capture_engine_args(context) -> None: - """Call get_current_revision and capture the create_engine call arguments.""" - fake_engine = MagicMock() - fake_connection = MagicMock() - fake_connection.__enter__ = MagicMock(return_value=fake_connection) - fake_connection.__exit__ = MagicMock(return_value=False) - fake_engine.connect.return_value = fake_connection - - migration_ctx = MagicMock() - migration_ctx.get_current_revision.return_value = None - - captured_calls: list[tuple[Any, ...]] = [] - - def fake_create_engine(url: str, **kwargs: Any) -> MagicMock: - captured_calls.append((url, kwargs)) - return fake_engine - - with ( - patch( - "cleveragents.infrastructure.database.migration_runner.create_engine", - side_effect=fake_create_engine, - ), - patch( - "cleveragents.infrastructure.database.migration_runner.MigrationContext.configure", - return_value=migration_ctx, - ), - ): - context.revision_result = context.runner.get_current_revision() - - context.engine_creation_calls = captured_calls - - -@then("the SQLite engine should be created with check_same_thread set to False") -def step_then_sqlite_engine_has_check_same_thread(context) -> None: - """Verify the SQLite engine was created with check_same_thread=False.""" - assert len(context.engine_creation_calls) == 1, ( - f"Expected exactly 1 create_engine call, got {len(context.engine_creation_calls)}" - ) - _url, kwargs = context.engine_creation_calls[0] - assert "connect_args" in kwargs, ( - "Expected connect_args in create_engine kwargs for SQLite, " - f"but got kwargs: {kwargs}" - ) - assert kwargs["connect_args"].get("check_same_thread") is False, ( - "Expected check_same_thread=False in connect_args, " - f"but got: {kwargs['connect_args']}" - ) - - -@then("the non-SQLite engine should be created without check_same_thread") -def step_then_non_sqlite_engine_no_check_same_thread(context) -> None: - """Verify non-SQLite engines are not given check_same_thread.""" - assert len(context.engine_creation_calls) == 1, ( - f"Expected exactly 1 create_engine call, got {len(context.engine_creation_calls)}" - ) - _url, kwargs = context.engine_creation_calls[0] - connect_args = kwargs.get("connect_args", {}) - assert "check_same_thread" not in connect_args, ( - "Expected check_same_thread to be absent for non-SQLite engine, " - f"but got connect_args: {connect_args}" - ) - - -@when("I verify get_current_revision uses thread-safe engine args for SQLite") -def step_when_verify_thread_safe_args(context) -> None: - """Verify get_current_revision passes check_same_thread=False for SQLite. - - This test verifies the thread-safety fix by inspecting the engine - creation arguments. The check_same_thread=False argument is what - allows SQLite connections to be used across threads, so verifying - it is present is equivalent to verifying thread-safety. + Mocks ``create_engine`` and ``MigrationContext.configure`` so the call + completes without a real database. The keyword arguments passed to + ``create_engine`` are stored on ``context.engine_creation_kwargs`` for + subsequent assertion steps. """ fake_engine = MagicMock() fake_connection = MagicMock() @@ -107,25 +43,43 @@ def step_when_verify_thread_safe_args(context) -> None: return_value=migration_ctx, ), ): - context.runner.get_current_revision() + context.revision_result = context.runner.get_current_revision() - context.captured_engine_kwargs = captured_kwargs + context.engine_creation_kwargs = captured_kwargs -@then("the SQLite engine creation args should include check_same_thread False") -def step_then_thread_safe_args_present(context) -> None: - """Assert that check_same_thread=False was passed to create_engine.""" - assert len(context.captured_engine_kwargs) == 1, ( - f"Expected exactly 1 create_engine call, " - f"got {len(context.captured_engine_kwargs)}" +@when("I request the current revision and capture the engine creation args") +def step_when_capture_engine_args(context: Any) -> None: + """Call get_current_revision and capture the create_engine call arguments.""" + _run_get_current_revision_and_capture_kwargs(context) + + +@then("the SQLite engine should be created with check_same_thread set to False") +def step_then_sqlite_engine_has_check_same_thread(context: Any) -> None: + """Verify the SQLite engine was created with check_same_thread=False.""" + assert len(context.engine_creation_kwargs) == 1, ( + f"Expected exactly 1 create_engine call, got {len(context.engine_creation_kwargs)}" ) - kwargs = context.captured_engine_kwargs[0] + kwargs = context.engine_creation_kwargs[0] assert "connect_args" in kwargs, ( "Expected connect_args in create_engine kwargs for SQLite, " f"but got kwargs: {kwargs}" ) assert kwargs["connect_args"].get("check_same_thread") is False, ( - "Expected check_same_thread=False in connect_args — this is the " - "fix for the SQLite threading bug (issue #10507). " - f"Got: {kwargs['connect_args']}" + "Expected check_same_thread=False in connect_args, " + f"but got: {kwargs['connect_args']}" + ) + + +@then("the non-SQLite engine should be created without check_same_thread") +def step_then_non_sqlite_engine_no_check_same_thread(context: Any) -> None: + """Verify non-SQLite engines are not given check_same_thread.""" + assert len(context.engine_creation_kwargs) == 1, ( + f"Expected exactly 1 create_engine call, got {len(context.engine_creation_kwargs)}" + ) + kwargs = context.engine_creation_kwargs[0] + connect_args = kwargs.get("connect_args", {}) + assert "check_same_thread" not in connect_args, ( + "Expected check_same_thread to be absent for non-SQLite engine, " + f"but got connect_args: {connect_args}" ) diff --git a/features/tdd_migration_runner_get_current_revision_threading_10507.feature b/features/tdd_migration_runner_get_current_revision_threading_10507.feature index 2cc60b022..81b500e9d 100644 --- a/features/tdd_migration_runner_get_current_revision_threading_10507.feature +++ b/features/tdd_migration_runner_get_current_revision_threading_10507.feature @@ -29,8 +29,3 @@ Feature: TDD Issue #10507 — get_current_revision() must pass check_same_thread Given a migration runner configured for "postgresql://user:pass@localhost/testdb" When I request the current revision and capture the engine creation args Then the non-SQLite engine should be created without check_same_thread - - Scenario: get_current_revision engine args are thread-safe for SQLite - Given a migration runner configured for "sqlite:///:memory:" - When I verify get_current_revision uses thread-safe engine args for SQLite - Then the SQLite engine creation args should include check_same_thread False