diff --git a/tests/test_app_controller_result.py b/tests/test_app_controller_result.py new file mode 100644 index 00000000..759d1a23 --- /dev/null +++ b/tests/test_app_controller_result.py @@ -0,0 +1,112 @@ +""" +Tests for the data-oriented error handling convention applied to src/app_controller.py. + +This file verifies the contract that the 32 INTERNAL_BROAD_CATCH + 8 INTERNAL_SILENT_SWALLOW ++ 4 INTERNAL_RETHROW + 1 INTERNAL_OPTIONAL_RETURN sites in src/app_controller.py are migrated +to the Result[T] pattern (per conductor/code_styleguides/error_handling.md). + +The tests are pattern templates: +- test_offload_entry_payload_returns_dict - the _offload_entry_payload helper returns a dict + (the existing test for the regression fix is in test_app_controller_offloading.py; this + one is a shape check). +- test_migrated_method_returns_result_on_success - methods migrated to Result[T] return + Result[data=...] with no errors on the success path. +- test_migrated_method_returns_result_with_error_on_failure - methods migrated to Result[T] + return Result with errors=... on the failure path. +- test_app_controller_does_not_use_broad_except - static check: src/app_controller.py + has no `except Exception:` clauses left (all 32 are migrated to specific exceptions). +- test_offload_entry_payload_preserves_unchanged_payload - verifies the no-op path for + entries without tool_call or tool_result kinds. +""" +import pytest +from unittest.mock import MagicMock, patch +from src.app_controller import AppController +from src.result_types import Result, ErrorInfo, ErrorKind + + +def test_offload_entry_payload_returns_dict(): + """ + Shape contract: _offload_entry_payload returns a dict. + The actual happy-path and error-path tests are in test_app_controller_offloading.py. + """ + with patch("src.app_controller.performance_monitor.PerformanceMonitor"): + ctrl = AppController() + out = ctrl._offload_entry_payload({"kind": "request", "payload": {"message": "hi"}, "ts": "12:00:00"}) + assert isinstance(out, dict) + assert out["kind"] == "request" + assert out["payload"]["message"] == "hi" + + +def test_migrated_method_returns_result_on_success(): + """ + Pattern template: methods migrated to Result[T] return Result[data=...] with no errors + on the success path. The 5 callback-handler sites (Batch 1) include _handle_custom_callback + and _handle_click; both return Result[None] after migration. + """ + from src.app_controller import _handle_custom_callback + controller = MagicMock() + controller._predefined_callbacks = {} + # Use a no-op callback so the body runs without error + task = {"callback": (lambda: None), "args": []} + result = _handle_custom_callback(controller, task) + assert isinstance(result, Result) + assert result.ok is True + assert result.errors == [] + + +def test_migrated_method_returns_result_with_error_on_failure(): + """ + Pattern template: methods migrated to Result[T] return Result with errors=... when + the underlying call raises. The migrated callback sites catch the specific exception + (e.g. TypeError, ValueError) and convert it to ErrorInfo. + """ + from src.app_controller import _handle_custom_callback + controller = MagicMock() + controller._predefined_callbacks = {} + # Callback that raises a specific exception + def bad_cb(): + raise ValueError("simulated failure") + task = {"callback": bad_cb, "args": []} + result = _handle_custom_callback(controller, task) + assert isinstance(result, Result) + assert result.ok is False + assert any("ValueError" in e.message or "simulated failure" in e.message for e in result.errors) + + +def test_app_controller_does_not_use_broad_except(): + """ + Static check: src/app_controller.py has no `except Exception:` clauses left + (all 32 are migrated to specific exceptions). + """ + import ast + import pathlib + src = pathlib.Path("src/app_controller.py").read_text(encoding="utf-8") + tree = ast.parse(src) + broad_catch_lines: list[int] = [] + for node in ast.walk(tree): + if isinstance(node, ast.ExceptHandler) and node.type is not None: + # `except Exception:` (bare Exception class as the type) + if isinstance(node.type, ast.Name) and node.type.id == "Exception": + broad_catch_lines.append(node.lineno) + # `except (Exception,):` or `except (Exception, Foo):` + elif isinstance(node.type, ast.Tuple): + for elt in node.type.elts: + if isinstance(elt, ast.Name) and elt.id == "Exception": + broad_catch_lines.append(node.lineno) + break + assert broad_catch_lines == [], ( + f"src/app_controller.py still has {len(broad_catch_lines)} `except Exception` clauses at lines {broad_catch_lines}. " + f"All 32 INTERNAL_BROAD_CATCH sites must be migrated to specific exceptions." + ) + + +def test_offload_entry_payload_preserves_unchanged_payload(): + """ + Verifies the no-op path: entries without tool_call or tool_result kinds are returned + with the payload unchanged (no [REF:...] rewriting). + """ + with patch("src.app_controller.performance_monitor.PerformanceMonitor"): + ctrl = AppController() + entry = {"kind": "request", "payload": {"message": "hi"}, "ts": "12:00:00"} + out = ctrl._offload_entry_payload(entry) + assert out == entry