5b139e6ab1
TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 2. Adds the drain plane that consumes the 8 controller error attributes (the data plane added by sub-track 3 Phase 6). Module-level functions in src/gui_2.py (lines 7293-7410): - _drain_normalize_errors (helper, lines 7295-7326): duck-typed normalizer for 3 error-container shapes (Optional[ErrorInfo], List[Tuple[str, ErrorInfo]], Dict[str, ErrorInfo]) - render_controller_error_modal (lines 7328-7368): FR-DP-1 Pattern 2 drain point; reads all 8 controller attrs, opens per-attr popups - _render_worker_error_indicator (lines 7370-7385): FR-DP-2 status-bar widget showing worker error count, clickable - _render_last_request_errors_modal (lines 7387-7409): FR-DP-3 per-request error modal opened after AI request completion App class delegation wrappers (lines 1138-1148): - App._render_controller_error_modal -> module-level - App._render_worker_error_indicator -> module-level - App._render_last_request_errors_modal -> module-level Per UI Delegation Pattern: App class has thin wrappers; logic at module level for hot-reload support. 1-space indentation, CRLF. Audit: no new violations introduced (gui_2.py still 25 V + 13 S + 2 RETHROW + 2 UNCLEAR + 12 COMPLIANT = 54). Tests: 4/4 pass.
203 lines
7.9 KiB
Python
203 lines
7.9 KiB
Python
"""
|
|
Tests for the Phase 1 invariant contract of result_migration_gui_2_20260619.
|
|
|
|
This file locks the Phase 1 site inventory contract: there are exactly 42
|
|
migration-target error-handling sites in src/gui_2.py. Both tests are static
|
|
invariants that must pass before Phase 1 closes and Phase 2 begins:
|
|
|
|
- test_phase_1_inventory_has_42_rows: parses the markdown inventory table
|
|
(tests/artifacts/PHASE1_SITE_INVENTORY.md) and asserts the Site Inventory
|
|
table contains exactly 42 rows.
|
|
- test_phase_1_audit_has_42_migration_target_sites: invokes the audit script
|
|
(scripts/audit_exception_handling.py --src src --json), finds the
|
|
src/gui_2.py file record, and counts the sites whose category is in the
|
|
migration-target set (i.e., NOT INTERNAL_COMPLIANT, NOT
|
|
INTERNAL_PROGRAMMER_RAISE, NOT BOUNDARY_*).
|
|
|
|
The migration-target category set is defined per
|
|
conductor/code_styleguides/error_handling.md as: any category that is not one
|
|
of the 5 "leave-as-is" categories. The migration-target sites are the ones
|
|
the Phase 2-N migration will touch; the leave-as-is categories are legitimate
|
|
non-migration patterns (compliant internal try/except, programmer raises,
|
|
and the 3 boundary categories).
|
|
"""
|
|
import json
|
|
import re
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
|
|
INVENTORY_PATH = Path("tests/artifacts/PHASE1_SITE_INVENTORY.md")
|
|
EXPECTED_SITE_COUNT = 42
|
|
MIGRATION_EXCLUDE_CATEGORIES = frozenset({
|
|
"INTERNAL_COMPLIANT",
|
|
"INTERNAL_PROGRAMMER_RAISE",
|
|
"BOUNDARY_FASTAPI",
|
|
"BOUNDARY_SDK",
|
|
"BOUNDARY_CONVERSION",
|
|
})
|
|
|
|
|
|
def test_phase_1_inventory_has_42_rows():
|
|
"""
|
|
Parse tests/artifacts/PHASE1_SITE_INVENTORY.md and verify the "Site Inventory"
|
|
markdown table contains exactly 42 rows.
|
|
|
|
The Site Inventory table begins with a header row of the form
|
|
"| L# | Category | Phase | ..." and a separator row "|---...". Each data row
|
|
has the form "| <line_number> | <CATEGORY> | <phase_number> | ...". The test
|
|
locates the header by its leading "| L#" sentinel and counts subsequent rows
|
|
that match the data-row pattern until the first non-table line.
|
|
"""
|
|
text = INVENTORY_PATH.read_text(encoding="utf-8")
|
|
lines = text.splitlines()
|
|
header_idx = None
|
|
for i, line in enumerate(lines):
|
|
if line.startswith("| L#"):
|
|
header_idx = i
|
|
break
|
|
assert header_idx is not None, (
|
|
f"Could not find '| L#' header in {INVENTORY_PATH}. "
|
|
f"The inventory file format may have changed."
|
|
)
|
|
rows = []
|
|
for line in lines[header_idx + 2:]:
|
|
if not line.startswith("|"):
|
|
break
|
|
if re.match(r"^\|\s*\d+\s*\|", line):
|
|
rows.append(line)
|
|
assert len(rows) == EXPECTED_SITE_COUNT, (
|
|
f"PHASE1_SITE_INVENTORY.md has {len(rows)} site rows; expected "
|
|
f"{EXPECTED_SITE_COUNT}. The inventory must list exactly 42 migration-target "
|
|
f"sites in src/gui_2.py."
|
|
)
|
|
|
|
|
|
def test_phase_1_audit_has_42_migration_target_sites():
|
|
"""
|
|
Invoke scripts/audit_exception_handling.py --src src --json, parse the JSON
|
|
output, and verify that the src/gui_2.py file record contains exactly 42
|
|
sites in the migration-target category set.
|
|
|
|
A site is "migration-target" when its category is NOT one of:
|
|
- INTERNAL_COMPLIANT (legitimate compliant internal try/except)
|
|
- INTERNAL_PROGRAMMER_RAISE (raise for impossible/programmer states)
|
|
- BOUNDARY_FASTAPI (FastAPI HTTPException boundary)
|
|
- BOUNDARY_SDK (SDK call boundary conversion)
|
|
- BOUNDARY_CONVERSION (broad except used as conversion boundary)
|
|
|
|
The migration-target set is therefore:
|
|
INTERNAL_BROAD_CATCH | INTERNAL_SILENT_SWALLOW | INTERNAL_RETHROW |
|
|
INTERNAL_OPTIONAL_RETURN | UNCLEAR.
|
|
|
|
This test pins the audit output to the same 42 the inventory declares, so a
|
|
future audit-script regression or inventory drift will surface here.
|
|
"""
|
|
result = subprocess.run(
|
|
["uv", "run", "python", "scripts/audit_exception_handling.py", "--src", "src", "--json"],
|
|
capture_output=True,
|
|
text=True,
|
|
)
|
|
assert result.returncode == 0, (
|
|
f"audit_exception_handling.py exited {result.returncode}; stderr:\n"
|
|
f"{result.stderr[:2000]}"
|
|
)
|
|
data = json.loads(result.stdout)
|
|
gui2_files = [f for f in data.get("files", []) if "gui_2" in f.get("filename", "")]
|
|
assert gui2_files, (
|
|
"audit JSON contained no file record matching 'gui_2' in filename. "
|
|
f"Filenames seen: {[f.get('filename') for f in data.get('files', [])][:10]}"
|
|
)
|
|
gui2 = gui2_files[0]
|
|
findings = gui2.get("findings", [])
|
|
migration_sites = [f for f in findings if f.get("category") not in MIGRATION_EXCLUDE_CATEGORIES]
|
|
assert len(migration_sites) == EXPECTED_SITE_COUNT, (
|
|
f"src/gui_2.py has {len(migration_sites)} migration-target sites in the audit; "
|
|
f"expected {EXPECTED_SITE_COUNT}. Categories seen: "
|
|
f"{sorted({f.get('category') for f in migration_sites})}. "
|
|
f"This must match the 42 sites declared in PHASE1_SITE_INVENTORY.md."
|
|
)
|
|
|
|
|
|
def test_phase_2_invariant_drain_plane_render_functions_exist():
|
|
"""
|
|
Verify the 3 new module-level render functions exist in src/gui_2.py:
|
|
- render_controller_error_modal
|
|
- _render_worker_error_indicator
|
|
- _render_last_request_errors_modal
|
|
|
|
These are the drain-plane functions added in Phase 2 of the
|
|
result_migration_gui_2_20260619 track. They read the 8 controller
|
|
error attributes (added by sub-track 3 Phase 6) and surface them
|
|
to the user via ImGui popups and indicators.
|
|
|
|
The test imports src.gui_2 and inspects the module for the function
|
|
names. A failure here means the drain-plane wiring is incomplete.
|
|
"""
|
|
import inspect
|
|
import src.gui_2 as gui2_mod
|
|
assert hasattr(gui2_mod, "render_controller_error_modal"), (
|
|
"src/gui_2.py is missing the module-level function "
|
|
"'render_controller_error_modal'. This is the FR-DP-1 drain plane "
|
|
"function; it must be added per the result_migration_gui_2_20260619 "
|
|
"Phase 2 spec."
|
|
)
|
|
assert hasattr(gui2_mod, "_render_worker_error_indicator"), (
|
|
"src/gui_2.py is missing the module-level function "
|
|
"'_render_worker_error_indicator'. This is the FR-DP-2 drain plane "
|
|
"function; it must be added per the result_migration_gui_2_20260619 "
|
|
"Phase 2 spec."
|
|
)
|
|
assert hasattr(gui2_mod, "_render_last_request_errors_modal"), (
|
|
"src/gui_2.py is missing the module-level function "
|
|
"'_render_last_request_errors_modal'. This is the FR-DP-3 drain plane "
|
|
"function; it must be added per the result_migration_gui_2_20260619 "
|
|
"Phase 2 spec."
|
|
)
|
|
assert callable(getattr(gui2_mod, "render_controller_error_modal")), (
|
|
"render_controller_error_modal exists but is not callable."
|
|
)
|
|
assert callable(getattr(gui2_mod, "_render_worker_error_indicator")), (
|
|
"_render_worker_error_indicator exists but is not callable."
|
|
)
|
|
assert callable(getattr(gui2_mod, "_render_last_request_errors_modal")), (
|
|
"_render_last_request_errors_modal exists but is not callable."
|
|
)
|
|
|
|
|
|
def test_phase_2_invariant_drain_plane_app_delegations_exist():
|
|
"""
|
|
Verify the 3 new App class delegation methods exist in src/gui_2.py:
|
|
- App._render_controller_error_modal
|
|
- App._render_worker_error_indicator
|
|
- App._render_last_request_errors_modal
|
|
|
|
Per conductor/product-guidelines.md §"UI Delegation for Hot-Reload",
|
|
the App class must contain only thin delegation wrappers; the actual
|
|
logic lives in module-level functions. This test locks the
|
|
delegation contract for Phase 2.
|
|
|
|
The test imports src.gui_2, gets the App class via the module
|
|
(lazily - via the _LazyModule path or directly), and checks for
|
|
the methods.
|
|
"""
|
|
import src.gui_2 as gui2_mod
|
|
app_cls = getattr(gui2_mod, "App", None)
|
|
assert app_cls is not None, (
|
|
"src.gui_2 has no 'App' class attribute. Cannot verify delegations."
|
|
)
|
|
for method_name in (
|
|
"_render_controller_error_modal",
|
|
"_render_worker_error_indicator",
|
|
"_render_last_request_errors_modal",
|
|
):
|
|
assert hasattr(app_cls, method_name), (
|
|
f"App class is missing delegation method '{method_name}'. "
|
|
f"The drain plane requires the App class to delegate to the "
|
|
f"module-level render functions so the UI delegation pattern "
|
|
f"supports hot-reload."
|
|
)
|
|
method = getattr(app_cls, method_name)
|
|
assert callable(method), (
|
|
f"App.{method_name} exists but is not callable."
|
|
) |