From 0e407f7f19099cbd48b5879de0c8bdc525e82141 Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Thu, 26 Mar 2026 02:06:28 +0000 Subject: [PATCH] =?UTF-8?q?test:=20add=20TDD=20bug-capture=20test=20for=20?= =?UTF-8?q?#1022=20=E2=80=94=20InvariantService=20persistence=20(#1109)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add Behave BDD and Robot Framework integration tests that capture bug #1022 — `InvariantService` stores invariants in an in-memory dict only, losing them across CLI process invocations. ### Motivation Per the project's TDD Bug Fix Workflow (CONTRIBUTING.md), every bug must first have a test written that captures the buggy behavior before the fix is implemented. This PR fulfills the TDD counterpart issue #1032 for bug #1022. ### What was done **Behave tests** (`features/tdd_invariant_persistence.feature`): Four scenarios tagged `@tdd_expected_fail @tdd_bug @tdd_bug_1022 @mock_only` that simulate separate CLI invocations via fresh `InvariantService` instances and assert cross-instance data visibility: 1. Project invariant persistence across service instances 2. Global invariant persistence across service instances 3. CLI add/list across separate invocations 4. Cross-instance soft-delete by invariant ID Step definitions in `features/steps/tdd_invariant_persistence_steps.py`. **Robot integration tests** (`robot/tdd_invariant_persistence.robot`): Three integration test cases with a helper script (`robot/helper_tdd_invariant_persistence.py`) that exercises add-then-list and add-then-remove across fresh service instances. ### How it passes CI All tests carry `@tdd_expected_fail`, which inverts the test result via the environment hooks (Behave) and listener (Robot). The underlying assertions fail (proving the bug exists), but are reported as passed to CI. The `@tdd_expected_fail` tag will be removed when bug #1022 is fixed, at which point the tests will run normally and must pass. ### Review fixes applied - **M1**: Moved `InvariantScope` import from function body in `robot/helper_tdd_invariant_persistence.py` to module-level top imports. - **m1**: Added `@mock_only` tag to `features/tdd_invariant_persistence.feature` — these tests use purely in-memory services and never touch the database. - **m2**: Added `exit_code == 0` assertion to `step_invoke_list_cli` in `features/steps/tdd_invariant_persistence_steps.py`, matching the pattern used in `step_invoke_add_cli`. - **m3**: Changed all `context: Any` parameter types to `context: Context` (from `behave.runner`), following the project-wide convention used in 335+ step files. - **m4**: Narrowed `except Exception` to `except NotFoundError` in `robot/helper_tdd_invariant_persistence.py`, matching the specific error being tested. - **n3**: Branch rebased onto latest `master`. ### Quality Gates - **lint**: ✅ passed - **typecheck**: ✅ passed (Pyright, 0 errors) - **unit_tests**: ✅ 1 feature, 4 scenarios, 16 steps — all passed via `@tdd_expected_fail` - **coverage**: ≥ 97% (test-only changes, no source modifications) Closes #1032 Co-authored-by: Jeffrey Phillips Freeman Reviewed-on: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1109 Reviewed-by: Jeffrey Phillips Freeman Co-authored-by: Brent E. Edwards Co-committed-by: Brent E. Edwards --- CHANGELOG.md | 5 + .../steps/tdd_invariant_persistence_steps.py | 174 ++++++++++++++++++ features/tdd_invariant_persistence.feature | 48 +++++ robot/helper_tdd_invariant_persistence.py | 134 ++++++++++++++ robot/tdd_invariant_persistence.robot | 43 +++++ 5 files changed, 404 insertions(+) create mode 100644 features/steps/tdd_invariant_persistence_steps.py create mode 100644 features/tdd_invariant_persistence.feature create mode 100644 robot/helper_tdd_invariant_persistence.py create mode 100644 robot/tdd_invariant_persistence.robot diff --git a/CHANGELOG.md b/CHANGELOG.md index 113e1db03..914f62f4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- Added TDD bug-capture tests for bug #1022 — InvariantService in-memory + storage only. Four Behave BDD scenarios and three Robot Framework + integration tests verify invariant persistence across simulated CLI + process restarts. Tests use `@tdd_expected_fail` until bug #1022 is + fixed. (#1032) - Added TDD bug-capture test for bug #988 — ReactiveEventBus.emit() swallows exception details. Behave BDD scenario (`@tdd_bug @tdd_bug_988 @tdd_expected_fail`) captures the missing exception message and traceback in diff --git a/features/steps/tdd_invariant_persistence_steps.py b/features/steps/tdd_invariant_persistence_steps.py new file mode 100644 index 000000000..3a6877941 --- /dev/null +++ b/features/steps/tdd_invariant_persistence_steps.py @@ -0,0 +1,174 @@ +"""Step definitions for tdd_invariant_persistence.feature (bug #1022). + +TDD bug-capture tests verifying that ``InvariantService`` persists invariants +across simulated CLI process restarts (separate service instances). + +Bug #1022: ``InvariantService`` stores invariants in an in-memory dict +(``self._invariants``) with no database persistence layer. Each CLI +invocation spawns a fresh process with a new ``InvariantService()`` +instance, so all invariants are lost when the process exits. + +These steps exercise the current (buggy) behaviour by creating fresh +``InvariantService`` instances to simulate separate process invocations. +When the bug is fixed, the service will use a database repository and +fresh instances backed by the same database will share state. + +The tests carry ``@tdd_expected_fail`` so CI passes while the bug is +unfixed. The tag will be removed when bug #1022 is fixed. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from behave import given, then, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.application.services.invariant_service import InvariantService +from cleveragents.cli.commands.invariant import app as invariant_app +from cleveragents.core.exceptions import NotFoundError +from cleveragents.domain.models.core.invariant import InvariantScope + +runner = CliRunner() + + +# --------------------------------------------------------------------------- +# Given steps — instance A +# --------------------------------------------------------------------------- + + +@given( + 'I add a project invariant "{text}" to project "{project}" ' + "via invariant service instance A" +) +def step_add_project_invariant_instance_a( + context: Context, text: str, project: str +) -> None: + """Add a project-scoped invariant via a fresh InvariantService (instance A).""" + context.invariant_svc_a = InvariantService() + context.invariant_added_a = context.invariant_svc_a.add_invariant( + text=text, + scope=InvariantScope.PROJECT, + source_name=project, + ) + + +@given('I add a global invariant "{text}" via invariant service instance A') +def step_add_global_invariant_instance_a(context: Context, text: str) -> None: + """Add a global invariant via a fresh InvariantService (instance A).""" + context.invariant_svc_a = InvariantService() + context.invariant_added_a = context.invariant_svc_a.add_invariant( + text=text, + scope=InvariantScope.GLOBAL, + source_name="system", + ) + + +@given("I capture the invariant ID from instance A") +def step_capture_invariant_id(context: Context) -> None: + """Store the invariant ID from instance A for later use.""" + context.captured_invariant_id = context.invariant_added_a.id + + +@given( + 'I invoke invariant add via CLI with "{flags}" and text "{text}" ' + "using service invocation {n:d}" +) +def step_invoke_add_cli(context: Context, flags: str, text: str, n: int) -> None: + """Invoke ``invariant add`` via CLI with a fresh service (simulated invocation).""" + svc = InvariantService() + args = ["add", *flags.split(), text] + with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc): + result = runner.invoke(invariant_app, args) + setattr(context, f"invariant_cli_result_{n}", result) + setattr(context, f"invariant_svc_{n}", svc) + assert result.exit_code == 0, ( + f"invariant add should exit 0 but got {result.exit_code}. " + f"output:\n{result.output}" + ) + + +# --------------------------------------------------------------------------- +# When steps — instance B / fresh instance +# --------------------------------------------------------------------------- + + +@when("I create a fresh invariant service instance B") +def step_create_fresh_instance_b(context: Context) -> None: + """Create a completely new InvariantService, simulating a new CLI process.""" + context.invariant_svc_b = InvariantService() + + +@when('I list project invariants for "{project}" via instance B') +def step_list_project_invariants_instance_b(context: Context, project: str) -> None: + """List project-scoped invariants via the fresh instance B.""" + context.invariant_list_b = context.invariant_svc_b.list_invariants( + scope=InvariantScope.PROJECT, + source_name=project, + ) + + +@when("I list global invariants via instance B") +def step_list_global_invariants_instance_b(context: Context) -> None: + """List global invariants via the fresh instance B.""" + context.invariant_list_b = context.invariant_svc_b.list_invariants( + scope=InvariantScope.GLOBAL, + ) + + +@when('I invoke invariant list via CLI with "{flags}" using service invocation {n:d}') +def step_invoke_list_cli(context: Context, flags: str, n: int) -> None: + """Invoke ``invariant list`` via CLI with a fresh service (simulated invocation).""" + svc = InvariantService() + args = ["list", *flags.split()] + with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc): + result = runner.invoke(invariant_app, args) + setattr(context, f"invariant_cli_result_{n}", result) + setattr(context, f"invariant_svc_{n}", svc) + assert result.exit_code == 0, ( + f"invariant list should exit 0 but got {result.exit_code}. " + f"output:\n{result.output}" + ) + + +@when("I attempt to remove the captured invariant ID via instance B") +def step_remove_via_instance_b(context: Context) -> None: + """Attempt to soft-delete an invariant using the fresh instance B.""" + try: + context.invariant_svc_b.remove_invariant(context.captured_invariant_id) + context.invariant_remove_b_error = None + except NotFoundError as exc: + context.invariant_remove_b_error = exc + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then('the invariant list from instance B should contain "{text}"') +def step_assert_list_b_contains(context: Context, text: str) -> None: + """Assert the invariant list from instance B contains the given text.""" + inv_texts = [inv.text for inv in context.invariant_list_b] + assert text in inv_texts, ( + f"Expected invariant '{text}' in instance B list but got: {inv_texts}" + ) + + +@then('the CLI list output from invocation {n:d} should contain "{text}"') +def step_assert_cli_list_contains(context: Context, n: int, text: str) -> None: + """Assert the CLI list output from invocation N contains the given text.""" + result = getattr(context, f"invariant_cli_result_{n}") + assert text in result.output, ( + f"Expected '{text}' in CLI invocation {n} output but got:\n{result.output}" + ) + + +@then("the remove operation via instance B should succeed without NotFoundError") +def step_assert_remove_b_success(context: Context) -> None: + """Assert that the remove operation succeeded (no NotFoundError).""" + assert context.invariant_remove_b_error is None, ( + f"Expected remove to succeed but got NotFoundError: " + f"{context.invariant_remove_b_error}" + ) diff --git a/features/tdd_invariant_persistence.feature b/features/tdd_invariant_persistence.feature new file mode 100644 index 000000000..e12c9312f --- /dev/null +++ b/features/tdd_invariant_persistence.feature @@ -0,0 +1,48 @@ +# TDD bug-capture test for bug #1022 — InvariantService in-memory storage only. +# +# InvariantService stores invariants in an in-memory dict (self._invariants) +# with no database persistence layer. Each CLI invocation spawns a fresh +# process with a new InvariantService() instance, so all invariants added in +# one invocation are lost when the process exits. +# +# These scenarios prove the bug exists by simulating separate CLI invocations +# (fresh InvariantService instances) and asserting that data added in one +# invocation is visible in the next. They FAIL until the bug is fixed. +# The @tdd_expected_fail tag inverts the result so CI passes. +# +# See: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1022 + +@tdd_expected_fail @tdd_bug @tdd_bug_1022 @mock_only +Feature: TDD Bug #1022 — InvariantService invariants lost across process restarts + As a developer using the agents CLI + I want invariants added via "agents invariant add" to persist across CLI invocations + So that "agents invariant list" in a subsequent invocation returns previously added invariants + + InvariantService uses in-memory dict storage only. Each CLI invocation + creates a fresh InvariantService() instance, so invariants are lost when + the process exits. These tests simulate separate process invocations by + creating fresh service instances and verifying cross-instance data visibility. + + Scenario: Invariant added in one service instance is visible in a fresh instance + Given I add a project invariant "All APIs must validate auth tokens" to project "local/api-service" via invariant service instance A + When I create a fresh invariant service instance B + And I list project invariants for "local/api-service" via instance B + Then the invariant list from instance B should contain "All APIs must validate auth tokens" + + Scenario: Global invariant persists across simulated process restarts + Given I add a global invariant "Never delete production data" via invariant service instance A + When I create a fresh invariant service instance B + And I list global invariants via instance B + Then the invariant list from instance B should contain "Never delete production data" + + Scenario: Invariant added via CLI add is visible via CLI list in a new invocation + Given I invoke invariant add via CLI with "--project local/webapp" and text "All changes need tests" using service invocation 1 + When I invoke invariant list via CLI with "--project local/webapp" using service invocation 2 + Then the CLI list output from invocation 2 should contain "All changes need tests" + + Scenario: Invariant soft-deleted in a fresh instance after being added in another + Given I add a project invariant "Temporary constraint" to project "local/temp" via invariant service instance A + And I capture the invariant ID from instance A + When I create a fresh invariant service instance B + And I attempt to remove the captured invariant ID via instance B + Then the remove operation via instance B should succeed without NotFoundError diff --git a/robot/helper_tdd_invariant_persistence.py b/robot/helper_tdd_invariant_persistence.py new file mode 100644 index 000000000..bedd260d4 --- /dev/null +++ b/robot/helper_tdd_invariant_persistence.py @@ -0,0 +1,134 @@ +"""Helper script for tdd_invariant_persistence.robot (bug #1022). + +Exercises InvariantService cross-invocation persistence at the integration +level. Each subcommand simulates a fresh CLI process by creating a new +InvariantService instance, mirroring how the real CLI works (each +``python -m cleveragents`` call gets its own service). + +Bug #1022: InvariantService stores invariants in an in-memory dict only. +Invariants added in one CLI invocation are lost when the process exits. + +This helper is called from Robot Framework via ``Run Process``. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import patch + +_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.application.services.invariant_service import ( # noqa: E402 + InvariantService, +) +from cleveragents.cli.commands.invariant import app as invariant_app # noqa: E402 +from cleveragents.core.exceptions import NotFoundError # noqa: E402 +from cleveragents.domain.models.core.invariant import InvariantScope # noqa: E402 + +runner = CliRunner() + + +def add_then_list_project() -> None: + """Add a project invariant in invocation 1, list in invocation 2. + + Simulates two separate CLI invocations by using fresh InvariantService + instances. The list in invocation 2 should show the invariant added + in invocation 1 — but it won't because of bug #1022. + """ + # Invocation 1: add + svc1 = InvariantService() + with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc1): + add_result = runner.invoke( + invariant_app, + ["add", "--project", "local/test-proj", "Must validate inputs"], + ) + if add_result.exit_code != 0: + print(f"FAIL-ADD: exit={add_result.exit_code} out={add_result.output}") + sys.exit(1) + + # Invocation 2: list (fresh service — simulates new process) + svc2 = InvariantService() + with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc2): + list_result = runner.invoke( + invariant_app, ["list", "--project", "local/test-proj"] + ) + + # The list output should contain the invariant — if it doesn't, bug exists + if "Must validate inputs" in list_result.output: + print("invariant-persist-project-ok") + else: + print( + f"FAIL-PERSIST: invariant not found in second invocation. " + f"output:\n{list_result.output}" + ) + sys.exit(1) + + +def add_then_list_global() -> None: + """Add a global invariant in invocation 1, list in invocation 2.""" + svc1 = InvariantService() + with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc1): + add_result = runner.invoke( + invariant_app, + ["add", "--global", "Never expose credentials"], + ) + if add_result.exit_code != 0: + print(f"FAIL-ADD: exit={add_result.exit_code} out={add_result.output}") + sys.exit(1) + + svc2 = InvariantService() + with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc2): + list_result = runner.invoke(invariant_app, ["list", "--global"]) + + if "Never expose credentials" in list_result.output: + print("invariant-persist-global-ok") + else: + print( + f"FAIL-PERSIST: invariant not found in second invocation. " + f"output:\n{list_result.output}" + ) + sys.exit(1) + + +def add_then_remove_cross_instance() -> None: + """Add invariant in instance 1, remove by ID in instance 2.""" + svc1 = InvariantService() + inv = svc1.add_invariant( + text="Temporary constraint", + scope=InvariantScope.PROJECT, + source_name="local/temp", + ) + inv_id = inv.id + + # Fresh instance — simulates new CLI process + svc2 = InvariantService() + try: + svc2.remove_invariant(inv_id) + print("invariant-cross-remove-ok") + except NotFoundError as exc: + print(f"FAIL-REMOVE: {type(exc).__name__}: {exc}") + sys.exit(1) + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +COMMANDS = { + "add-then-list-project": add_then_list_project, + "add-then-list-global": add_then_list_global, + "add-then-remove-cross": add_then_remove_cross_instance, +} + +if __name__ == "__main__": + cmd = sys.argv[1] if len(sys.argv) > 1 else None + if cmd not in COMMANDS: + print(f"Unknown command: {cmd}", file=sys.stderr) + print(f"Available: {', '.join(sorted(COMMANDS))}", file=sys.stderr) + sys.exit(2) + COMMANDS[cmd]() diff --git a/robot/tdd_invariant_persistence.robot b/robot/tdd_invariant_persistence.robot new file mode 100644 index 000000000..5a99408e0 --- /dev/null +++ b/robot/tdd_invariant_persistence.robot @@ -0,0 +1,43 @@ +*** Settings *** +Documentation TDD Bug #1022 — InvariantService invariants lost across CLI invocations +... Integration tests verifying that invariants added via the CLI +... persist across simulated process restarts. InvariantService +... stores invariants in an in-memory dict only, so each CLI +... invocation starts with an empty service. These tests exercise +... add-then-list and add-then-remove across fresh service +... instances. They fail until the bug is fixed; the +... tdd_expected_fail tag inverts the result so CI passes. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tdd_invariant_persistence.py + +*** Test Cases *** +TDD Invariant Add Then List Project Across Invocations + [Documentation] Add a project invariant in invocation 1, list in invocation 2. + [Tags] tdd_expected_fail tdd_bug tdd_bug_1022 + ${result}= Run Process ${PYTHON} ${HELPER} add-then-list-project cwd=${WORKSPACE} timeout=30s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} invariant-persist-project-ok + +TDD Invariant Add Then List Global Across Invocations + [Documentation] Add a global invariant in invocation 1, list in invocation 2. + [Tags] tdd_expected_fail tdd_bug tdd_bug_1022 + ${result}= Run Process ${PYTHON} ${HELPER} add-then-list-global cwd=${WORKSPACE} timeout=30s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} invariant-persist-global-ok + +TDD Invariant Remove Cross Instance + [Documentation] Add invariant in instance 1, remove by ID in instance 2. + [Tags] tdd_expected_fail tdd_bug tdd_bug_1022 + ${result}= Run Process ${PYTHON} ${HELPER} add-then-remove-cross cwd=${WORKSPACE} timeout=30s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} invariant-cross-remove-ok