diff --git a/.github/bump_version.py b/.github/bump_version.py index dee431c03..78dc45bb0 100644 --- a/.github/bump_version.py +++ b/.github/bump_version.py @@ -5,6 +5,9 @@ from pathlib import Path from typing import TextIO +# A fragment named "..md" has at least two dot-separated stem parts. +STEM_PARTS_WITH_CATEGORY = 2 + def _report(message: str, *, file: TextIO | None = None) -> None: """Print one line of this command's output, to stdout unless ``file`` is given.""" @@ -40,7 +43,7 @@ def infer_bump(changelog_dir: Path) -> str: categories = {f.suffix.lstrip(".") for f in fragments} for f in fragments: parts = f.stem.split(".") - if len(parts) >= 2: + if len(parts) >= STEM_PARTS_WITH_CATEGORY: categories.add(parts[-1]) if "breaking" in categories: diff --git a/.github/scripts/check_updates.py b/.github/scripts/check_updates.py index ba4fbea3b..645fa2f90 100755 --- a/.github/scripts/check_updates.py +++ b/.github/scripts/check_updates.py @@ -12,6 +12,7 @@ import argparse import re import sys +from http import HTTPStatus from typing import TypedDict import requests @@ -38,7 +39,7 @@ def fetch_changelog(package: str) -> str | None: for branch in ("main", "master"): url = f"https://raw.githubusercontent.com/{repo}/{branch}/CHANGELOG.md" resp = requests.get(url, timeout=30) - if resp.status_code == 200: + if resp.status_code == HTTPStatus.OK: return resp.text return None diff --git a/.github/scripts/verify_observability_collector.py b/.github/scripts/verify_observability_collector.py index f3d64eec4..24dcc2f25 100644 --- a/.github/scripts/verify_observability_collector.py +++ b/.github/scripts/verify_observability_collector.py @@ -8,6 +8,7 @@ import time import uuid from datetime import UTC, datetime +from http import HTTPStatus from typing import NamedTuple from urllib.error import HTTPError from urllib.parse import quote, urlencode, urlparse @@ -71,6 +72,14 @@ class VerificationProbe(NamedTuple): started_at: float +class CloudReadback(NamedTuple): + """The Google Cloud project, metric location and token for reading telemetry.""" + + project_id: str + metric_location: str + access_token: str + + def _authority(endpoint: str) -> str: parsed = urlparse(endpoint) if parsed.scheme != "https" or not parsed.hostname: @@ -207,7 +216,7 @@ def _get_json(url: str, access_token: str) -> dict[str, object] | None: with urlopen(request, timeout=15) as response: # noqa: S310 # callers pass only https Google Cloud API URLs return json.load(response) except HTTPError as error: - if error.code == 404: + if error.code == HTTPStatus.NOT_FOUND: return None detail = error.read().decode("utf-8", errors="replace") msg = f"Google Cloud verification request failed ({error.code}): {detail}" @@ -268,12 +277,8 @@ def _metric_available( def _wait_for_delivery( *, - project_id: str, - trace_id: str, - verification_id: str, - metric_location: str, - access_token: str, - started_at: float, + readback: CloudReadback, + probe: VerificationProbe, timeout_seconds: float, poll_seconds: float = 5.0, ) -> None: @@ -282,14 +287,16 @@ def _wait_for_delivery( metric_found = False while True: if not trace_found: - trace_found = _trace_available(project_id, trace_id, access_token) + trace_found = _trace_available( + readback.project_id, probe.trace_id, readback.access_token + ) if not metric_found: metric_found = _metric_available( - project_id, - verification_id, - metric_location, - access_token, - started_at, + readback.project_id, + probe.verification_id, + readback.metric_location, + readback.access_token, + probe.started_at, ) if trace_found and metric_found: return @@ -312,9 +319,7 @@ def verify( endpoint: str, token: str, *, - project_id: str, - metric_location: str, - access_token: str, + readback: CloudReadback, timeout_seconds: float = 15.0, delivery_timeout_seconds: float = 600.0, ) -> VerificationProbe: @@ -361,16 +366,11 @@ def verify( else: msg = "collector unexpectedly accepted application logs" raise RuntimeError(msg) + probe = VerificationProbe(verification_id, trace_id, started_at) _wait_for_delivery( - project_id=project_id, - trace_id=trace_id, - verification_id=verification_id, - metric_location=metric_location, - access_token=access_token, - started_at=started_at, - timeout_seconds=delivery_timeout_seconds, + readback=readback, probe=probe, timeout_seconds=delivery_timeout_seconds ) - return VerificationProbe(verification_id, trace_id, started_at) + return probe def main() -> None: @@ -387,9 +387,11 @@ def main() -> None: probe = verify( args.endpoint, os.environ["COLLECTOR_ID_TOKEN"], - project_id=args.project_id, - metric_location=args.metric_location, - access_token=os.environ["GOOGLE_OAUTH_ACCESS_TOKEN"], + readback=CloudReadback( + project_id=args.project_id, + metric_location=args.metric_location, + access_token=os.environ["GOOGLE_OAUTH_ACCESS_TOKEN"], + ), ) print( # noqa: T201 # the command reports the verified ingestion "Verified collector ingestion " diff --git a/AGENTS.md b/AGENTS.md index f15fcbf23..a0cfbf3e5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,7 +123,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`) - `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy) - `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format) -- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (A, ANN, ARG, ASYNC, B, BLE, C4, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FBT, FIX, FLY, FURB, G, I, INP, LOG, N, PD, PERF, PGH, PIE, PLC, PLW, PT, PTH, PYI, RET, RSE, RUF, S, SIM, SLF, T20, TC, TD, TRY, UP, W)) +- `api.quality.ruff-lint`: All Python code passes ruff check with every ruff rule selected (ALL), with no ratchet, except the ignores in pyproject.toml: COM812 and ISC001, which conflict with ruff format, and D203 and D213, which conflict with the kept D211 and D212. (blocking: ruff check (ALL)) - `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`) - `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`) - `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, INP001, PLC0415, PLR2004, S101, SLF001 for `**/tests/**`) diff --git a/changelog.d/ruff-all.changed.md b/changelog.d/ruff-all.changed.md new file mode 100644 index 000000000..e48fd4a93 --- /dev/null +++ b/changelog.d/ruff-all.changed.md @@ -0,0 +1 @@ +Selected every ruff rule (ALL) now that each rule family passes, ignoring only COM812 and ISC001, which conflict with ruff format, and D203 and D213, which conflict with the kept D211 and D212; the ruff-lint rule in the registry now names ALL, and published schemas and the HTTP API are unchanged. diff --git a/changelog.d/ruff-complexity-tests-scripts.changed.md b/changelog.d/ruff-complexity-tests-scripts.changed.md new file mode 100644 index 000000000..e6e0702ee --- /dev/null +++ b/changelog.d/ruff-complexity-tests-scripts.changed.md @@ -0,0 +1 @@ +Refactored the tests, the repository scripts and the rules tool under ruff's complexity rules (C901, PLR): long tests and script functions are split into named steps or helpers, related arguments travel as small value objects, and magic numbers are named, with characterization tests pinning the scripts' and rules tool's behaviour first; command output, test coverage and published schemas are unchanged. diff --git a/docs/engineering/skills/rules-registry.md b/docs/engineering/skills/rules-registry.md index d78d7824b..b0afb4e27 100644 --- a/docs/engineering/skills/rules-registry.md +++ b/docs/engineering/skills/rules-registry.md @@ -115,9 +115,12 @@ fail the guard yet. ## Baselines -- **Ruff is never ratcheted.** Adopting a ruff rule family is one standalone PR - that changes the select list and fixes every resulting violation. Don't use - `--add-noqa` or blanket per-file ignores. +- **Ruff is never ratcheted.** `select = ["ALL"]`, so every ruff rule applies + except the four listed in `ignore` (two formatter conflicts and one of each + mutually exclusive docstring pair) and the registered per-file ignores. When + a ruff upgrade adds rules, the upgrade PR fixes every new violation. Don't + use `--add-noqa` or blanket per-file ignores; a justified exception is an + inline `# noqa: CODE # reason`. - Other enforcers (mypy, Pyright, Import Linter, custom guards) may start `ratcheted` with a baseline file of tolerated violations. Baselines may only shrink: the guard compares counts against the base branch. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index b0ac13436..9833d09f4 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -272,7 +272,7 @@ flowchart LR | `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — | | `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — | | `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | -| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (A, ANN, ARG, ASYNC, B, BLE, C4, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FBT, FIX, FLY, FURB, G, I, INP, LOG, N, PD, PERF, PGH, PIE, PLC, PLW, PT, PTH, PYI, RET, RSE, RUF, S, SIM, SLF, T20, TC, TD, TRY, UP, W) | `pr.yml:lint` | — | +| `api.quality.ruff-lint` | All Python code passes ruff check with every ruff rule selected (ALL), with no ratchet, except the ignores in pyproject.toml: COM812 and ISC001, which conflict with ruff format, and D203 and D213, which conflict with the kept D211 and D212. | mechanical | enforced | yes | ruff check (ALL) | `pr.yml:lint` | — | | `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | | `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | | `api.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415); every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, INP001, PLC0415, PLR2004, S101, SLF001 for `**/tests/**` | `pr.yml:lint` | — | @@ -293,7 +293,7 @@ flowchart LR e_ruff_format["ruff format"] j_CI_pr_yml_lint["CI pr.yml:lint"] r_api_quality_ruff_lint("api.quality.ruff-lint (enforced)") - e_ruff_check__A__ANN__ARG__ASYNC__B__BLE__C4__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FBT__FIX__FLY__FURB__G__I__INP__LOG__N__PD__PERF__PGH__PIE__PLC__PLW__PT__PTH__PYI__RET__RSE__RUF__S__SIM__SLF__T20__TC__TD__TRY__UP__W_["ruff check (A, ANN, ARG, ASYNC, B, BLE, C4, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FBT, FIX, FLY, FURB, G, I, INP, LOG, N, PD, PERF, PGH, PIE, PLC, PLW, PT, PTH, PYI, RET, RSE, RUF, S, SIM, SLF, T20, TC, TD, TRY, UP, W)"] + e_ruff_check__ALL_["ruff check (ALL)"] r_api_quality_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_tests_unit_test_runtime_evaluated_annotations_py["pytest tests/unit/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] @@ -311,8 +311,8 @@ flowchart LR e_mypy --> j_CI_pr_yml_typecheck_v2 r_api_quality_ruff_format --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint - r_api_quality_ruff_lint --> e_ruff_check__A__ANN__ARG__ASYNC__B__BLE__C4__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FBT__FIX__FLY__FURB__G__I__INP__LOG__N__PD__PERF__PGH__PIE__PLC__PLW__PT__PTH__PYI__RET__RSE__RUF__S__SIM__SLF__T20__TC__TD__TRY__UP__W_ - e_ruff_check__A__ANN__ARG__ASYNC__B__BLE__C4__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FBT__FIX__FLY__FURB__G__I__INP__LOG__N__PD__PERF__PGH__PIE__PLC__PLW__PT__PTH__PYI__RET__RSE__RUF__S__SIM__SLF__T20__TC__TD__TRY__UP__W_ --> j_CI_pr_yml_lint + r_api_quality_ruff_lint --> e_ruff_check__ALL_ + e_ruff_check__ALL_ --> j_CI_pr_yml_lint r_api_quality_runtime_evaluated_models_resolve --> e_pytest_tests_unit_test_runtime_evaluated_annotations_py e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_api_quality_tests_pass --> e_CI_job_test diff --git a/pyproject.toml b/pyproject.toml index a3a9d197e..83e2b4480 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -99,31 +99,18 @@ namespace-packages = [ ] [tool.ruff.lint] -# Rule families are enabled one themed group per PR, each fixing everything it -# finds (no ratchet), until the select list is "ALL". See -# docs/engineering/skills/rules-registry.md and rules/registry.toml. -select = [ - "E4", "E7", "E9", "F", - # Group 1: safe auto-fixes. - "FLY", "FURB", "I", "PIE", "PYI", "RET", "RSE", "UP", "W", - # Group 2: typing-only imports. - "TC", - # Group 3: annotations. - "ANN", - # Group 4: docstrings. - "D", - # Group 5: exceptions and messages. - "BLE", "EM", "TRY", - # Group 6: security, logging and debugging leftovers. - "DTZ", "ERA", "EXE", "FIX", "G", "LOG", "S", "T20", "TD", - # Group 7: structure and imports. - "A", "E", "FAST", "INP", "N", "PLC", "SLF", - # Group 8a: arguments, idioms and pytest style. - "ARG", "ASYNC", "B", "C4", "FBT", "PD", "PERF", "PGH", "PLW", "PT", "PTH", "RUF", "SIM", +# Every ruff rule applies, with no ratchet, apart from the ignores below +# (api.quality.ruff-lint). See docs/engineering/skills/rules-registry.md and +# rules/registry.toml. +select = ["ALL"] +ignore = [ + # These conflict with ruff format; ruff's documentation says to disable + # them when using the formatter. + "COM812", "ISC001", + # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a + # class docstring) and D212 (summary on the first line). + "D203", "D213", ] -# Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class -# docstring) and D212 (summary on the first line). -ignore = ["D203", "D213"] [tool.ruff.lint.per-file-ignores] # Alembic revisions must stay exactly as Alembic generates them diff --git a/rules/registry.toml b/rules/registry.toml index 911027c00..5b4fbab63 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -691,7 +691,7 @@ invocation = "ruff format --check" [[rule]] id = "api.quality.ruff-lint" category = "quality" -statement = "All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL." +statement = "All Python code passes ruff check with every ruff rule selected (ALL), with no ratchet, except the ignores in pyproject.toml: COM812 and ISC001, which conflict with ruff format, and D203 and D213, which conflict with the kept D211 and D212." rationale = "docs/engineering/skills/rules-registry.md#baselines" scope = ["**/*.py"] tier = "mechanical" @@ -700,7 +700,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["A", "ANN", "ARG", "ASYNC", "B", "BLE", "C4", "D", "DTZ", "E", "E4", "E7", "E9", "EM", "ERA", "EXE", "F", "FAST", "FBT", "FIX", "FLY", "FURB", "G", "I", "INP", "LOG", "N", "PD", "PERF", "PGH", "PIE", "PLC", "PLW", "PT", "PTH", "PYI", "RET", "RSE", "RUF", "S", "SIM", "SLF", "T20", "TC", "TD", "TRY", "UP", "W"] +codes = ["ALL"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" diff --git a/scripts/capture_migration_baseline.py b/scripts/capture_migration_baseline.py index 76ef4cff0..0e23eeb70 100644 --- a/scripts/capture_migration_baseline.py +++ b/scripts/capture_migration_baseline.py @@ -75,25 +75,22 @@ def _percentile(values: list[float], percentile: float) -> float | None: def _request_probe( client: httpx.Client, + probe: RouteProbe, *, - name: str, - method: str, - path: str, - expected_statuses: tuple[int, ...], json_payload: dict | None = None, ) -> tuple[ProbeResult, httpx.Response | None]: started_at = time.perf_counter() try: - response = client.request(method, path, json=json_payload) + response = client.request(probe.method, probe.path, json=json_payload) latency_ms = (time.perf_counter() - started_at) * 1000 return ( ProbeResult( - name=name, - method=method, - path=path, + name=probe.name, + method=probe.method, + path=probe.path, status_code=response.status_code, latency_ms=round(latency_ms, 2), - ok=response.status_code in expected_statuses, + ok=response.status_code in probe.expected_statuses, ), response, ) @@ -101,9 +98,9 @@ def _request_probe( latency_ms = (time.perf_counter() - started_at) * 1000 return ( ProbeResult( - name=name, - method=method, - path=path, + name=probe.name, + method=probe.method, + path=probe.path, status_code=0, latency_ms=round(latency_ms, 2), ok=False, @@ -119,13 +116,7 @@ def run_probes(base_url: str, repetitions: int) -> list[ProbeResult]: with httpx.Client(base_url=base_url.rstrip("/"), timeout=90.0) as client: for _ in range(repetitions): for probe in DEFAULT_PROBES: - result, _ = _request_probe( - client, - name=probe.name, - method=probe.method, - path=probe.path, - expected_statuses=probe.expected_statuses, - ) + result, _ = _request_probe(client, probe) results.append(result) return results @@ -149,10 +140,12 @@ def run_simulation_gateway_probe( with httpx.Client(base_url=gateway_url.rstrip("/"), timeout=90.0) as client: submit_result, submit_response = _request_probe( client, - name="simulation_gateway_submit", - method="POST", - path="/simulate/economy/comparison", - expected_statuses=(202,), + RouteProbe( + name="simulation_gateway_submit", + method="POST", + path="/simulate/economy/comparison", + expected_statuses=(202,), + ), json_payload=payload, ) results.append(submit_result) @@ -182,10 +175,12 @@ def run_simulation_gateway_probe( while time.perf_counter() < deadline: poll_result, poll_response = _request_probe( client, - name="simulation_gateway_poll", - method="GET", - path=f"/jobs/{job_id}", - expected_statuses=(200, 202), + RouteProbe( + name="simulation_gateway_poll", + method="GET", + path=f"/jobs/{job_id}", + expected_statuses=(200, 202), + ), ) last_result = poll_result results.append(poll_result) diff --git a/scripts/guards/migration_contracts.py b/scripts/guards/migration_contracts.py index b7ad061fa..0efbe4b7c 100644 --- a/scripts/guards/migration_contracts.py +++ b/scripts/guards/migration_contracts.py @@ -2,6 +2,7 @@ from __future__ import annotations +from http import HTTPStatus from pathlib import Path from typing import Any @@ -59,6 +60,45 @@ def _check_route_groups(payload: dict[str, Any]) -> list[str]: return violations +def _request_key(request: dict[str, Any]) -> str: + """Key a request by method, path and credential headers. + + One route may be recorded once per credential that can call it. + """ + headers = request.get("headers", []) + return " ".join((request["method"], request["path"], *sorted(headers))) + + +def _request_violations( + workflow_name: str, + request: dict[str, Any], + declared_route_groups: set[str], +) -> list[str]: + """Return one contract request's problems, in the order they are checked.""" + violations = [] + headers = request.get("headers", []) + context = f"{workflow_name} {request['method']} {request['path']}" + if request["route_group"] not in declared_route_groups: + violations.append(f"{context}: unknown route_group {request['route_group']!r}") + if ( + not request["stable_response_fields"] + and request["expected_status"] != HTTPStatus.NO_CONTENT + ): + violations.append(f"{context}: stable_response_fields is required") + if not request["path"].startswith("/"): + violations.append(f"{context}: path must start with /") + if request["path"].startswith( + USER_OWNED_PATH_PREFIXES + ) and USER_CREDENTIAL_HEADERS.isdisjoint(headers): + violations.append( + f"{context}: user-owned routes must require the " + f"{AUTHORIZATION_HEADER} or {UNAUTHENTICATED_USER_HEADER} header" + ) + if request["expected_status"] not in {200, 201, 202, 204}: + violations.append(f"{context}: unexpected status {request['expected_status']}") + return violations + + def _check_workflows(payload: dict[str, Any]) -> list[str]: violations = [] declared_route_groups = {group["name"] for group in payload["route_groups"]} @@ -80,34 +120,10 @@ def _check_workflows(payload: dict[str, Any]) -> list[str]: violations.append(f"{workflow['name']}: at least one request is required") for request in workflow["requests"]: - headers = request.get("headers", []) - context = f"{workflow['name']} {request['method']} {request['path']}" - # One route may be recorded once per credential that can call it. - request_keys.append( - " ".join((request["method"], request["path"], *sorted(headers))) + request_keys.append(_request_key(request)) + violations.extend( + _request_violations(workflow["name"], request, declared_route_groups) ) - if request["route_group"] not in declared_route_groups: - violations.append( - f"{context}: unknown route_group {request['route_group']!r}" - ) - if ( - not request["stable_response_fields"] - and request["expected_status"] != 204 - ): - violations.append(f"{context}: stable_response_fields is required") - if not request["path"].startswith("/"): - violations.append(f"{context}: path must start with /") - if request["path"].startswith( - USER_OWNED_PATH_PREFIXES - ) and USER_CREDENTIAL_HEADERS.isdisjoint(headers): - violations.append( - f"{context}: user-owned routes must require the " - f"{AUTHORIZATION_HEADER} or {UNAUTHENTICATED_USER_HEADER} header" - ) - if request["expected_status"] not in {200, 201, 202, 204}: - violations.append( - f"{context}: unexpected status {request['expected_status']}" - ) violations.extend(_check_unique_values(request_keys, label="contract request")) return violations diff --git a/scripts/measure_cloud_run_runtime.py b/scripts/measure_cloud_run_runtime.py index 914f6ed59..b8967dfbe 100644 --- a/scripts/measure_cloud_run_runtime.py +++ b/scripts/measure_cloud_run_runtime.py @@ -22,7 +22,9 @@ import time import uuid from collections import Counter +from dataclasses import dataclass from datetime import UTC, datetime +from http import HTTPStatus from pathlib import Path import httpx @@ -51,11 +53,11 @@ def _utc_now_iso() -> str: def _classify(status_code: int) -> str: - if status_code == 429: + if status_code == HTTPStatus.TOO_MANY_REQUESTS: return OUTCOME_HTTP_429 - if status_code >= 500: + if status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: return OUTCOME_HTTP_5XX - if status_code >= 400: + if status_code >= HTTPStatus.BAD_REQUEST: return OUTCOME_HTTP_4XX return OUTCOME_OK @@ -72,38 +74,49 @@ async def _wait_for_readiness( raise RuntimeError(msg) try: response = await client.get(READINESS_PATH, timeout=20.0) - if response.status_code == 200: + if response.status_code == HTTPStatus.OK: return waited except httpx.HTTPError: pass await asyncio.sleep(5.0) +@dataclass(frozen=True) +class _LoadCellRun: + """One load cell's settings and clock, shared by every closed-loop client. + + Each client appends one record per request to ``records``. + """ + + args: argparse.Namespace + base_payload: dict + cache_bust: bool + run_token: str + warmup_until: float + deadline: float + records: list[dict] + + async def _client_loop( *, client_index: int, client: httpx.AsyncClient, - args: argparse.Namespace, - base_payload: dict, - cache_bust: bool, - run_token: str, - warmup_until: float, - deadline: float, - records: list[dict], + cell: _LoadCellRun, ) -> None: + args = cell.args rng = random.Random(args.seed * 1000 + client_index) # noqa: S311 # a seeded generator makes the request mix reproducible; nothing secret depends on it seq = 0 - while time.monotonic() < deadline: + while time.monotonic() < cell.deadline: seq += 1 is_calculate = rng.random() < args.calculate_share if is_calculate: - payload = copy.deepcopy(base_payload) - if cache_bust: + payload = copy.deepcopy(cell.base_payload) + if cell.cache_bust: # run_token makes the nonce unique across runs: without it, # re-running a cell within Redis's 300s TTL replays identical # body hashes and measures the cache instead of the engine. payload["_loadtest"] = ( - f"{run_token}-{args.cell_label}-{client_index}-{seq}" + f"{cell.run_token}-{args.cell_label}-{client_index}-{seq}" ) method, path = "POST", CALCULATE_PATH timeout = args.timeout_calculate @@ -112,7 +125,7 @@ async def _client_loop( method, path = "GET", METADATA_PATH timeout = args.timeout_metadata - warmup = time.monotonic() < warmup_until + warmup = time.monotonic() < cell.warmup_until started = time.perf_counter() status = 0 try: @@ -123,7 +136,7 @@ async def _client_loop( outcome = OUTCOME_TIMEOUT except httpx.HTTPError: outcome = OUTCOME_CONNECT_ERROR - records.append( + cell.records.append( { "endpoint": "calculate" if is_calculate else "metadata", "warmup": warmup, @@ -182,19 +195,18 @@ async def _run(args: argparse.Namespace) -> dict: start = time.monotonic() warmup_until = start + args.warmup_seconds deadline = warmup_until + args.duration_seconds + cell = _LoadCellRun( + args=args, + base_payload=base_payload, + cache_bust=cache_bust, + run_token=run_token, + warmup_until=warmup_until, + deadline=deadline, + records=records, + ) await asyncio.gather( *( - _client_loop( - client_index=i, - client=client, - args=args, - base_payload=base_payload, - cache_bust=cache_bust, - run_token=run_token, - warmup_until=warmup_until, - deadline=deadline, - records=records, - ) + _client_loop(client_index=i, client=client, cell=cell) for i in range(args.clients) ) ) diff --git a/scripts/process_v1_household_mirror_event.py b/scripts/process_v1_household_mirror_event.py index cf6bfde03..9cb1ab7bd 100755 --- a/scripts/process_v1_household_mirror_event.py +++ b/scripts/process_v1_household_mirror_event.py @@ -7,7 +7,7 @@ import json import os import sys -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, NamedTuple from sqlalchemy import Engine, create_engine from sqlalchemy.engine import URL @@ -31,6 +31,14 @@ class HouseholdEventCommandConfigurationError(RuntimeError): V1_MIGRATION_USER = "policyengine_schema_migrator" V1_DATABASE_NAME = "policyengine" V1_PROXY_HOST = "127.0.0.1" +MAX_TCP_PORT = 65535 + + +class _SelectedEvent(NamedTuple): + """The household create-event the operator selected on the command line.""" + + country_id: str + legacy_household_id: int def _required(environ: Mapping[str, str], name: str) -> str: @@ -114,7 +122,7 @@ def _proxy_event_service( except ValueError as error: msg = "POLICYENGINE_DB_PROXY_PORT must be an integer" raise HouseholdEventCommandConfigurationError(msg) from error - if not 1 <= port <= 65535: + if not 1 <= port <= MAX_TCP_PORT: msg = "POLICYENGINE_DB_PROXY_PORT is outside the valid range" raise HouseholdEventCommandConfigurationError(msg) @@ -148,8 +156,7 @@ def _parser() -> argparse.ArgumentParser: def _record_command_outcome( *, identity: Mapping[str, str] | None, - country_id: str, - legacy_household_id: int, + event: _SelectedEvent, outcome: str, destination_household_id: object | None = None, error_type: str | None = None, @@ -170,8 +177,8 @@ def _record_command_outcome( "operator_identity": ( identity.get("operator_identity") if identity else None ), - "country_id": country_id, - "legacy_household_id": legacy_household_id, + "country_id": event.country_id, + "legacy_household_id": event.legacy_household_id, "destination_household_id": ( str(destination_household_id) if destination_household_id is not None @@ -198,6 +205,7 @@ def main( if args.legacy_household_id < 0: _parser().error("--legacy-household-id must be non-negative") values = os.environ if environ is None else environ + event = _SelectedEvent(args.country_id, args.legacy_household_id) identity: dict[str, str] | None = None v1_engine: Engine | None = None try: @@ -220,8 +228,7 @@ def main( try: # noqa: SIM105 # kept as try/except: outcome logging must not change the command result _record_command_outcome( identity=identity, - country_id=args.country_id, - legacy_household_id=args.legacy_household_id, + event=event, outcome="error", error_type=type(error).__name__, ) @@ -252,8 +259,7 @@ def main( try: # noqa: SIM105 # kept as try/except: outcome logging must not change the command result _record_command_outcome( identity=identity, - country_id=args.country_id, - legacy_household_id=args.legacy_household_id, + event=event, outcome="ok", destination_household_id=result.household_id, ) diff --git a/scripts/rules/cli.py b/scripts/rules/cli.py index ac232ae35..7917b2f4c 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -28,6 +28,38 @@ def _print(violations: list, notes: list[str] | None = None) -> int: return 1 if violations else 0 +def _ci_scripts_command(root: Path, *, write_baselines: bool) -> int: + """Check the workflow scripts, or record today's findings as the baselines.""" + if not write_baselines: + violations, notes = ci_scripts.run_check(root) + return _print(violations, notes) + registry, violations = load_registry(root) + if registry is None or registry.ci_scripts is None: + return _print(violations or ["no [ci_scripts] table"]) + ci_scripts.write_baselines( + registry, ci_scripts.find(registry.ci_scripts, root), root + ) + _report("wrote ci_scripts baselines; update the rules' baseline counts") + return 0 + + +def _registry_command(command: str, root: Path) -> int: + """Validate the registry and, for ``render``, write its views once it is valid.""" + clear_caches() + registry, violations = load_registry(root) + if registry is None: + return _print(violations) + violations += validate(registry, root, date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date + if command != "render": + return _print(violations) + if violations: + _report("render refused: fix the registry first") + return _print(violations) + for path in write_outputs(registry, root): + _report(f"wrote {path.relative_to(root)}") + return 0 + + def main(argv: list[str] | None = None) -> int: """Run one rules command and return 1 if it found violations. @@ -56,32 +88,8 @@ def main(argv: list[str] | None = None) -> int: root = args.root.resolve() if args.command == "ci-scripts": - if args.write_baselines: - registry, violations = load_registry(root) - if registry is None or registry.ci_scripts is None: - return _print(violations or ["no [ci_scripts] table"]) - ci_scripts.write_baselines( - registry, ci_scripts.find(registry.ci_scripts, root), root - ) - _report("wrote ci_scripts baselines; update the rules' baseline counts") - return 0 - violations, notes = ci_scripts.run_check(root) - return _print(violations, notes) - + return _ci_scripts_command(root, write_baselines=args.write_baselines) if args.command == "guard": result = run_guard(root, base_ref=args.base_ref) return _print(result.violations, result.notes) - - clear_caches() - registry, violations = load_registry(root) - if registry is None: - return _print(violations) - violations += validate(registry, root, date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date - if args.command == "render": - if violations: - _report("render refused: fix the registry first") - return _print(violations) - for path in write_outputs(registry, root): - _report(f"wrote {path.relative_to(root)}") - return 0 - return _print(violations) + return _registry_command(args.command, root) diff --git a/scripts/rules/load.py b/scripts/rules/load.py index 936a3a9f4..0a235a2b7 100644 --- a/scripts/rules/load.py +++ b/scripts/rules/load.py @@ -230,23 +230,23 @@ def _ci_scripts(c: _Collector, raw: object) -> CiScripts | None: ) -def _rule(c: _Collector, raw: object, index: int) -> Rule | None: - if not isinstance(raw, dict): - c.add(None, f"rule #{index + 1} must be a table", "write [[rule]]") - return None - rule_id = raw.get("id") if isinstance(raw.get("id"), str) else None - label = rule_id or f"rule #{index + 1}" - c.unknown_keys(rule_id, raw, _RULE_KEYS, label) - +def _enforcer( + c: _Collector, rule_id: str | None, raw: dict[str, Any], label: str +) -> Enforcer: + """Read ``[rule.enforcer]``; a missing table or unknown kind enforces nothing.""" enforcer_raw = raw.get("enforcer") - enforcer = Enforcer(Kind.NONE) - if isinstance(enforcer_raw, dict): - kind = c.enum(rule_id, Kind, enforcer_raw.get("kind"), f"{label}.enforcer.kind") - fields = {k: v for k, v in enforcer_raw.items() if k != "kind"} - enforcer = Enforcer(kind or Kind.NONE, fields) - else: + if not isinstance(enforcer_raw, dict): c.add(rule_id, f"{label}.enforcer must be a table", "add [rule.enforcer]") + return Enforcer(Kind.NONE) + kind = c.enum(rule_id, Kind, enforcer_raw.get("kind"), f"{label}.enforcer.kind") + fields = {k: v for k, v in enforcer_raw.items() if k != "kind"} + return Enforcer(kind or Kind.NONE, fields) + +def _ci_refs( + c: _Collector, rule_id: str | None, raw: dict[str, Any], label: str +) -> tuple[CiRef, ...]: + """Read the ``[[rule.ci]]`` entries, skipping any that are not tables.""" ci_refs = [] ci_raw = raw.get("ci", []) for entry in ci_raw if isinstance(ci_raw, list) else [ci_raw]: @@ -261,29 +261,45 @@ def _rule(c: _Collector, raw: object, index: int) -> Rule | None: c.string(rule_id, entry, "invocation", f"{label}.ci"), ) ) + return tuple(ci_refs) + + +def _proof( + c: _Collector, rule_id: str | None, raw: dict[str, Any], label: str +) -> Proof | None: + """Read ``[rule.proof]``, which is optional but must be a table when present.""" + if "proof" not in raw: + return None + if isinstance(raw["proof"], dict): + return Proof(dict(raw["proof"])) + c.add(rule_id, f"{label}.proof must be a table", "write [rule.proof]") + return None + + +def _baseline( + c: _Collector, rule_id: str | None, raw: dict[str, Any], label: str +) -> Baseline | None: + """Read ``[rule.baseline]``, which needs a file and an integer count.""" + if "baseline" not in raw: + return None + braw = raw["baseline"] + if isinstance(braw, dict) and isinstance(braw.get("count"), int): + c.unknown_keys(rule_id, braw, {"file", "count"}, f"{label}.baseline") + return Baseline( + c.string(rule_id, braw, "file", f"{label}.baseline"), braw["count"] + ) + c.add( + rule_id, + f"{label}.baseline needs 'file' and an integer 'count'", + "write [rule.baseline] with file and count", + ) + return None - proof = None - if "proof" in raw: - if isinstance(raw["proof"], dict): - proof = Proof(dict(raw["proof"])) - else: - c.add(rule_id, f"{label}.proof must be a table", "write [rule.proof]") - - baseline = None - if "baseline" in raw: - braw = raw["baseline"] - if isinstance(braw, dict) and isinstance(braw.get("count"), int): - c.unknown_keys(rule_id, braw, {"file", "count"}, f"{label}.baseline") - baseline = Baseline( - c.string(rule_id, braw, "file", f"{label}.baseline"), braw["count"] - ) - else: - c.add( - rule_id, - f"{label}.baseline needs 'file' and an integer 'count'", - "write [rule.baseline] with file and count", - ) +def _exceptions( + c: _Collector, rule_id: str | None, raw: dict[str, Any], label: str +) -> tuple[RuleException, ...]: + """Read the ``[[rule.exception]]`` entries; one without a date never expires.""" exceptions = [] exc_raw = raw.get("exception", []) for entry in exc_raw if isinstance(exc_raw, list) else [exc_raw]: @@ -305,24 +321,53 @@ def _rule(c: _Collector, raw: object, index: int) -> Rule | None: expires or date.max, ) ) + return tuple(exceptions) - downgrade_reason = None - if "downgrade" in raw: - draw = raw["downgrade"] - if isinstance(draw, dict) and isinstance(draw.get("reason"), str): - downgrade_reason = draw["reason"] - else: - c.add( - rule_id, - f"{label}.downgrade needs a 'reason'", - 'write [rule.downgrade] reason = "..."', - ) +def _downgrade_reason( + c: _Collector, rule_id: str | None, raw: dict[str, Any], label: str +) -> str | None: + """Read the reason in ``[rule.downgrade]``, which is optional.""" + if "downgrade" not in raw: + return None + draw = raw["downgrade"] + if isinstance(draw, dict) and isinstance(draw.get("reason"), str): + reason: str = draw["reason"] + return reason + c.add( + rule_id, + f"{label}.downgrade needs a 'reason'", + 'write [rule.downgrade] reason = "..."', + ) + return None + + +def _autofix( + c: _Collector, rule_id: str | None, raw: dict[str, Any], label: str +) -> bool: + """Read ``autofix``, which must be a boolean; anything else counts as false.""" autofix = raw.get("autofix") - if not isinstance(autofix, bool): - c.add(rule_id, f"{label}.autofix must be true or false", "set autofix") - autofix = False + if isinstance(autofix, bool): + return autofix + c.add(rule_id, f"{label}.autofix must be true or false", "set autofix") + return False + + +def _rule(c: _Collector, raw: object, index: int) -> Rule | None: + if not isinstance(raw, dict): + c.add(None, f"rule #{index + 1} must be a table", "write [[rule]]") + return None + rule_id = raw.get("id") if isinstance(raw.get("id"), str) else None + label = rule_id or f"rule #{index + 1}" + c.unknown_keys(rule_id, raw, _RULE_KEYS, label) + enforcer = _enforcer(c, rule_id, raw, label) + ci_refs = _ci_refs(c, rule_id, raw, label) + proof = _proof(c, rule_id, raw, label) + baseline = _baseline(c, rule_id, raw, label) + exceptions = _exceptions(c, rule_id, raw, label) + downgrade_reason = _downgrade_reason(c, rule_id, raw, label) + autofix = _autofix(c, rule_id, raw, label) review_by = None if "review_by" in raw: review_by = c.day(rule_id, raw["review_by"], f"{label}.review_by") @@ -342,11 +387,11 @@ def _rule(c: _Collector, raw: object, index: int) -> Rule | None: status=status, autofix=autofix, enforcer=enforcer, - ci=tuple(ci_refs), + ci=ci_refs, review_by=review_by, proof=proof, baseline=baseline, - exceptions=tuple(exceptions), + exceptions=exceptions, downgrade_reason=downgrade_reason, ) diff --git a/scripts/rules/model.py b/scripts/rules/model.py index 401becaaa..95f65570f 100644 --- a/scripts/rules/model.py +++ b/scripts/rules/model.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING, Any if TYPE_CHECKING: - from collections.abc import Mapping + from collections.abc import Callable, Mapping from datetime import date @@ -43,6 +43,40 @@ class Kind(StrEnum): NONE = "none" +def _describe_pytest(fields: Mapping[str, Any]) -> str: + """Name the test files behind a pytest enforcer, under its project if any.""" + prefix = f"{fields['project'].rstrip('/')}/" if "project" in fields else "" + files = sorted({node.split("::", 1)[0] for node in fields.get("nodes", [])}) + return "pytest " + ", ".join(f"`{prefix}{path}`" for path in files) + + +def _describe_ruff(fields: Mapping[str, Any]) -> str: + """Describe a ruff enforcer by its mode: format, a per-file ignore or a check.""" + codes = ", ".join(fields.get("codes", [])) + mode = fields.get("mode", "") + if mode == "format": + return "ruff format" + if mode == "per-file-ignore": + return f"ruff per-file ignore of {codes} for `{fields.get('pattern', '')}`" + return f"ruff check ({codes})" + + +# How each kind of enforcer describes itself; a kind not listed enforces nothing. +_ENFORCER_DESCRIPTIONS: dict[Kind, Callable[[Mapping[str, Any]], str]] = { + Kind.PYTEST: _describe_pytest, + Kind.QUALITY_GUARD: lambda fields: f"quality guard `{fields.get('name', '')}`", + Kind.RUFF: _describe_ruff, + Kind.MYPY: lambda _fields: "mypy", + Kind.WORKFLOW_JOB: lambda _fields: "CI job", + Kind.PYRIGHT: lambda fields: f"{Kind.PYRIGHT.value} `{fields.get('config', '')}`", + Kind.IMPORT_LINTER: ( + lambda fields: f"{Kind.IMPORT_LINTER.value} `{fields.get('config', '')}`" + ), + Kind.DOCS_GUARD: lambda fields: f"docs guard on `{fields.get('doc', '')}`", + Kind.LLM_REVIEW: lambda fields: f"LLM review with `{fields.get('prompt', '')}`", +} + + @dataclass(frozen=True) class Violation: """One guard failure. ``fix`` always tells the reader what to do next.""" @@ -124,34 +158,8 @@ def get( @property def description(self) -> str: """Short human-readable description of what enforces the rule.""" - kind, fields = self.kind, self.fields - if kind is Kind.PYTEST: - prefix = f"{fields['project'].rstrip('/')}/" if "project" in fields else "" - files = sorted({node.split("::", 1)[0] for node in fields.get("nodes", [])}) - return "pytest " + ", ".join(f"`{prefix}{path}`" for path in files) - if kind is Kind.QUALITY_GUARD: - return f"quality guard `{fields.get('name', '')}`" - if kind is Kind.RUFF: - codes = ", ".join(fields.get("codes", [])) - mode = fields.get("mode", "") - if mode == "format": - return "ruff format" - if mode == "per-file-ignore": - return ( - f"ruff per-file ignore of {codes} for `{fields.get('pattern', '')}`" - ) - return f"ruff check ({codes})" - if kind is Kind.MYPY: - return "mypy" - if kind is Kind.WORKFLOW_JOB: - return "CI job" - if kind in (Kind.PYRIGHT, Kind.IMPORT_LINTER): - return f"{kind.value} `{fields.get('config', '')}`" - if kind is Kind.DOCS_GUARD: - return f"docs guard on `{fields.get('doc', '')}`" - if kind is Kind.LLM_REVIEW: - return f"LLM review with `{fields.get('prompt', '')}`" - return "nothing" + describe = _ENFORCER_DESCRIPTIONS.get(self.kind) + return "nothing" if describe is None else describe(self.fields) @property def expects_proof(self) -> bool: diff --git a/scripts/rules/render.py b/scripts/rules/render.py index a6d5b639d..e22985d5f 100644 --- a/scripts/rules/render.py +++ b/scripts/rules/render.py @@ -63,46 +63,64 @@ def _mermaid_id(prefix: str, text: str) -> str: return prefix + "_" + re.sub(r"[^A-Za-z0-9]", "_", text) -def _mermaid(rules: list[Rule]) -> list[str]: - lines = ["```mermaid", "flowchart LR"] - declared: set[str] = set() - edges: list[str] = [] - - def node(node_id: str, label: str, shape: str = "box") -> str: - if node_id not in declared: - declared.add(node_id) - safe = label.replace('"', "'") - if shape == "round": - lines.append(f' {node_id}("{safe}")') - elif shape == "flag": - lines.append(f' {node_id}>"{safe}"]') - else: - lines.append(f' {node_id}["{safe}"]') +# Mermaid node declarations by shape: a box, a rounded box, or a flag. +_MERMAID_SHAPES = { + "box": ' {node_id}["{label}"]', + "round": ' {node_id}("{label}")', + "flag": ' {node_id}>"{label}"]', +} + + +class _MermaidGraph: + """A flowchart under construction: each node is declared once, in first use.""" + + def __init__(self) -> None: + self.nodes: list[str] = [] + self.edges: list[str] = [] + self._declared: set[str] = set() + + def node(self, node_id: str, label: str, shape: str = "box") -> str: + """Declare the node the first time it is used, and return its id.""" + if node_id not in self._declared: + self._declared.add(node_id) + template = _MERMAID_SHAPES.get(shape, _MERMAID_SHAPES["box"]) + self.nodes.append( + template.format(node_id=node_id, label=label.replace('"', "'")) + ) return node_id + def edge_once(self, edge: str) -> None: + """Add an edge unless the graph already has it.""" + if edge not in self.edges: + self.edges.append(edge) + + +def _add_rule_to_graph(graph: _MermaidGraph, rule: Rule) -> None: + """Link a rule to its enforcer, and the enforcer to its CI jobs or a gap.""" + rule_node = graph.node( + _mermaid_id("r", rule.id), f"{rule.id} ({rule.status})", "round" + ) + if rule.enforcer.kind is Kind.NONE: + gap = graph.node("gap_unenforced", "not enforced", "flag") + graph.edges.append(f" {rule_node} -.-> {gap}") + return + label = _enforced_by(rule).replace("`", "") + enforcer_node = graph.node(_mermaid_id("e", label), label) + graph.edges.append(f" {rule_node} --> {enforcer_node}") + for ref in rule.ci: + job_label = f"CI {Path(ref.workflow).name}:{ref.job}" + job_node = graph.node(_mermaid_id("j", job_label), job_label) + graph.edge_once(f" {enforcer_node} --> {job_node}") + if not rule.ci: + local = graph.node("local_only", "not run in CI", "flag") + graph.edge_once(f" {enforcer_node} -.-> {local}") + + +def _mermaid(rules: list[Rule]) -> list[str]: + graph = _MermaidGraph() for rule in rules: - rule_node = node( - _mermaid_id("r", rule.id), f"{rule.id} ({rule.status})", "round" - ) - if rule.enforcer.kind is Kind.NONE: - gap = node("gap_unenforced", "not enforced", "flag") - edges.append(f" {rule_node} -.-> {gap}") - continue - label = _enforced_by(rule).replace("`", "") - enforcer_node = node(_mermaid_id("e", label), label) - edges.append(f" {rule_node} --> {enforcer_node}") - for ref in rule.ci: - job_label = f"CI {Path(ref.workflow).name}:{ref.job}" - job_node = node(_mermaid_id("j", job_label), job_label) - edge = f" {enforcer_node} --> {job_node}" - if edge not in edges: - edges.append(edge) - if not rule.ci: - local = node("local_only", "not run in CI", "flag") - edge = f" {enforcer_node} -.-> {local}" - if edge not in edges: - edges.append(edge) - return lines + edges + ["```"] + _add_rule_to_graph(graph, rule) + return ["```mermaid", "flowchart LR", *graph.nodes, *graph.edges, "```"] def _mermaid_chunks(rules: list[Rule]) -> list[list[str]]: diff --git a/scripts/rules/resolve.py b/scripts/rules/resolve.py index 6f548139c..f27e5f44b 100644 --- a/scripts/rules/resolve.py +++ b/scripts/rules/resolve.py @@ -10,6 +10,7 @@ import ast import re import tomllib +from dataclasses import dataclass from functools import cache from typing import TYPE_CHECKING, Any @@ -381,15 +382,13 @@ def resolve_ci(rule: Rule, root: Path) -> list[Violation]: job, problems = _job_for(rule.id, ref, root, "ci", "rule.ci") violations += problems if job is not None and rule.enforcer.kind is Kind.PYTEST: - violations += _test_coverage( - rule.id, - root_relative_nodes(rule), - rule.enforcer.get("project"), - ref.invocation, - job, - root, - "ci", + required = _RequiredTests( + rule_id=rule.id, + check="ci", + nodes=root_relative_nodes(rule), + project=rule.enforcer.get("project"), ) + violations += _test_coverage(required, ref.invocation, job, root) return violations @@ -429,22 +428,27 @@ def resolve_proof_ci(rule: Rule, root: Path) -> list[Violation]: job, problems = _job_for(rule.id, ref, root, "proof", "proof") violations += problems if job is not None: - violations += _test_coverage( - rule.id, [node], project, ref.invocation, job, root, "proof" + required = _RequiredTests( + rule_id=rule.id, check="proof", nodes=[node], project=project ) + violations += _test_coverage(required, ref.invocation, job, root) return violations +@dataclass(frozen=True) +class _RequiredTests: + """Root-relative test nodes a CI job must collect, and how to report a miss.""" + + rule_id: str + check: str + nodes: list[str] + project: str | None + + def _test_coverage( - rule_id: str, - nodes: list[str], - project: str | None, - invocation: str, - job: JobBlock, - root: Path, - check: str, + required: _RequiredTests, invocation: str, job: JobBlock, root: Path ) -> list[Violation]: - """Check that the job's test command collects these root-relative test nodes. + """Check that the job's test command collects the required test nodes. Only ``make`` targets and ``unittest discover`` commands are inspected; any other command is trusted. @@ -461,30 +465,30 @@ def _test_coverage( collected = [match.group(1)] if match else None described = f"'{invocation}'" if collected: - for node in nodes: + for node in required.nodes: path = split_node(node)[0] if not any( path == p or path.startswith(p.rstrip("/") + "/") for p in collected ): violations.append( Violation( - rule_id, - check, + required.rule_id, + required.check, f"{path} is not collected by {described} " f"(collects {collected})", "add it to what the job runs, or move the test", ) ) if ( - project + required.project and job.matrix - and not any(project in values for values in job.matrix.values()) + and not any(required.project in values for values in job.matrix.values()) ): violations.append( Violation( - rule_id, - check, - f"project {project} is not in the matrix of job '{job.id}'", + required.rule_id, + required.check, + f"project {required.project} is not in the matrix of job '{job.id}'", "add the project to the job matrix", ) ) diff --git a/scripts/rules/tests/fixtures/validate/broken_rules.toml b/scripts/rules/tests/fixtures/validate/broken_rules.toml new file mode 100644 index 000000000..24d4b4b33 --- /dev/null +++ b/scripts/rules/tests/fixtures/validate/broken_rules.toml @@ -0,0 +1,159 @@ +[[category]] +id = "layers" +title = "Layers again" + +[agents] +instructions = "AGENTS.md" +adapters = ["CLAUDE.md", "MISSING.md"] + +[ci_scripts] +workflows = ".github/workflows/*.yml" +scripts_dir = ".github/scripts" +test_globs = ["tests/**/test_*.py"] +multiline_baseline = "rules/baselines/ci-multiline-run.txt" +untested_baseline = "rules/baselines/missing-untested.txt" +shellcheck_baseline = "rules/baselines/missing-shellcheck.txt" + +[[rule]] +id = "ex.layers.no-sql-in-routes" +category = "layers" +statement = "Duplicate id." +rationale = "docs/guide.md" +scope = ["pkg/*.py"] +tier = "mechanical" +status = "doc-only" +autofix = false +[rule.enforcer] +kind = "none" + +[[rule]] +id = "Bad_ID" +category = "nowhere" +statement = "Doc-only rule that still claims everything." +rationale = "docs/missing.md#section" +scope = ["pkg/*.py", "nothing/**/*.py"] +tier = "judgment" +status = "doc-only" +autofix = false +review_by = 2026-05-01 +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +extra = 1 +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check" +[rule.baseline] +file = "rules/baselines/missing.txt" +count = 1 +[[rule.exception]] +path = "nothing/*.py" +reason = "legacy" +expires = 2026-01-01 +[[rule.exception]] +path = "pkg/*.py" +reason = "current" +expires = 2027-01-01 + +[[rule]] +id = "ex.quality.unenforced" +category = "quality" +statement = "Enforced but has no enforcer or CI." +rationale = "docs/guide.md" +scope = ["pkg/*.py"] +tier = "mechanical" +status = "ratcheted" +autofix = false +[rule.enforcer] +kind = "none" + +[[rule]] +id = "ex.quality.review-only" +category = "quality" +statement = "LLM review of a mechanical rule." +rationale = "docs/guide.md" +scope = ["pkg/*.py"] +tier = "mechanical" +status = "advisory" +autofix = false +[rule.enforcer] +kind = "llm-review" +prompt = 5 +[rule.proof] +bad_fixture = "fixtures/bad.py" +good_fixture = "pkg/routes.py" +test = "tests/unit/test_boundaries.py::test_missing" +workflow = ".github/workflows/pr.yml" +job = "test" + +[[rule]] +id = "ex.quality.pytest-empty" +category = "quality" +statement = "Pytest enforcer without nodes." +rationale = "docs/guide.md" +scope = ["pkg/*.py"] +tier = "metric" +status = "advisory" +autofix = false +[rule.enforcer] +kind = "pytest" +nodes = [] +[rule.proof] +below_fixture = "pkg/routes.py" +above_fixture = "fixtures/above.py" +threshold = "high" +test = "test_boundaries.py::test_routes_have_no_sql" +project = "tests/unit/" + +[[rule]] +id = "ex.quality.ruff-modes" +category = "quality" +statement = "Ruff lint without codes." +rationale = "docs/guide.md" +scope = ["pkg/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "lint" +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check" +[rule.baseline] +file = "rules/baselines/ruff.txt" +count = 3 + +[[rule]] +id = "ex.quality.ruff-bad-mode" +category = "quality" +statement = "Ruff with an unknown mode." +rationale = "docs/guide.md" +scope = ["pkg/*.py"] +tier = "mechanical" +status = "advisory" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "fix" + +[[rule]] +id = "ex.quality.judged" +category = "quality" +statement = "Judgment rule with a proof." +rationale = "docs/guide.md" +scope = ["pkg/*.py"] +tier = "judgment" +status = "advisory" +autofix = false +[rule.enforcer] +kind = "llm-review" +prompt = "prompts/review.md" +model = "claude" +[rule.proof] +eval_set = "evals/missing.jsonl" +min_precision = 0.9 +measured_precision = "high" +measured_on = 2026-01-01 diff --git a/scripts/rules/tests/fixtures/validate/broken_rules_violations.txt b/scripts/rules/tests/fixtures/validate/broken_rules_violations.txt new file mode 100644 index 000000000..aae2c3fcc --- /dev/null +++ b/scripts/rules/tests/fixtures/validate/broken_rules_violations.txt @@ -0,0 +1,41 @@ +[registry] schema: agents file MISSING.md does not exist. Fix: fix [agents] or create the file. +[registry] schema: ci_scripts.scripts_dir .github/scripts does not exist. Fix: fix [ci_scripts]. +[registry] schema: ci_scripts baseline rules/baselines/missing-untested.txt does not exist. Fix: create it (it may be empty). +[registry] schema: ci_scripts baseline rules/baselines/missing-shellcheck.txt does not exist. Fix: create it (it may be empty). +[registry] schema: duplicate category 'layers'. Fix: give each category a unique id. +[ex.layers.no-sql-in-routes] schema: duplicate rule id. Fix: give each rule a unique id. +[Bad_ID] schema: id must be lowercase dotted, e.g. 'api.v2.routes-no-sql'. Fix: rename the rule. +[Bad_ID] schema: unknown category 'nowhere'. Fix: use one of ['layers', 'quality', 'layers'] or add a [[category]]. +[Bad_ID] schema: rationale docs/missing.md does not exist. Fix: link an existing doc. +[Bad_ID] schema: scope 'nothing/**/*.py' matches no files. Fix: fix the glob. +[Bad_ID] schema: unknown key 'extra' in enforcer (ruff). Fix: remove it; allowed: ['codes', 'config', 'mode', 'pattern']. +[Bad_ID] schema: ruff per-file-ignore needs codes. Fix: list the ruff codes. +[Bad_ID] schema: ruff per-file-ignore needs a pattern. Fix: set pattern. +[Bad_ID] schema: doc-only rules have no enforcer. Fix: set enforcer.kind = "none", or pick a stronger status. +[Bad_ID] schema: doc-only rules have no CI wiring. Fix: remove rule.ci, or pick a stronger status. +[Bad_ID] schema: only ratcheted rules may have a baseline. Fix: remove [rule.baseline] or set status = "ratcheted". +[Bad_ID] schema: ruff is never ratcheted. Fix: remove the baseline and fix every violation in a standalone PR that also changes the ruff select list. +[Bad_ID] schema: judgment rules are enforced by review, not by a mechanical checker. Fix: use enforcer.kind = "llm-review" or "none". +[Bad_ID] baseline: baseline file rules/baselines/missing.txt does not exist. Fix: create it. +[Bad_ID] expiry: review_by 2026-05-01 has passed. Fix: review the rule, then remove or extend review_by. +[Bad_ID] expiry: exception for nothing/*.py expired on 2026-01-01. Fix: remove the exception (fix the code) or extend it with a reason. +[Bad_ID] schema: exception path 'nothing/*.py' matches no files. Fix: remove the stale exception. +[ex.quality.unenforced] schema: status ratcheted needs an enforcer. Fix: add [rule.enforcer] or use doc-only/advisory. +[ex.quality.unenforced] schema: ratcheted rules must run in CI. Fix: add a [[rule.ci]] reference. +[ex.quality.unenforced] schema: ratcheted rules need a baseline. Fix: add [rule.baseline] or use enforced. +[ex.quality.review-only] schema: enforcer (llm-review).prompt has the wrong type. Fix: make it a . +[ex.quality.review-only] schema: enforcer (llm-review) is missing 'model'. Fix: add model. +[ex.quality.review-only] schema: llm-review only enforces judgment rules. Fix: set tier = "judgment". +[ex.quality.review-only] schema: proof names its CI job only partly. Fix: set all of proof.workflow, proof.job and proof.invocation, or none. +[ex.quality.review-only] proof: proof bad_fixture fixtures/bad.py does not exist. Fix: add the fixture or fix the path. +[ex.quality.review-only] proof: tests/unit/test_boundaries.py has no test 'test_missing'. Fix: point proof.test at the test that runs the fixtures. +[ex.quality.pytest-empty] schema: pytest enforcer has no nodes. Fix: list at least one test node. +[ex.quality.pytest-empty] schema: proof (metric).threshold has the wrong type. Fix: make it a (, ). +[ex.quality.pytest-empty] proof: proof above_fixture fixtures/above.py does not exist. Fix: add the fixture or fix the path. +[ex.quality.ruff-modes] schema: ruff lint needs codes. Fix: list the ruff codes. +[ex.quality.ruff-modes] schema: only ratcheted rules may have a baseline. Fix: remove [rule.baseline] or set status = "ratcheted". +[ex.quality.ruff-modes] schema: ruff is never ratcheted. Fix: remove the baseline and fix every violation in a standalone PR that also changes the ruff select list. +[ex.quality.ruff-modes] baseline: baseline count is 3 but rules/baselines/ruff.txt lists 2. Fix: make the count match the file. +[ex.quality.ruff-bad-mode] schema: ruff mode is 'fix'. Fix: use one of ['format', 'lint', 'per-file-ignore']. +[ex.quality.judged] schema: proof (judgment).measured_precision has the wrong type. Fix: make it a . +[ex.quality.judged] proof: proof eval_set evals/missing.jsonl does not exist. Fix: add the fixture or fix the path. diff --git a/scripts/rules/tests/test_cli.py b/scripts/rules/tests/test_cli.py new file mode 100644 index 000000000..a9862f54a --- /dev/null +++ b/scripts/rules/tests/test_cli.py @@ -0,0 +1,239 @@ +"""Tests for the rules command line: what each command prints and returns.""" + +from __future__ import annotations + +import contextlib +import io +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +from scripts.rules.ci_scripts import Findings +from scripts.rules.cli import main +from scripts.rules.guard import GuardResult +from scripts.rules.model import Violation +from scripts.rules.tests.support import REGISTRY, Repo + +MISSING_REGISTRY = ( + " - [registry] schema: rules/registry.toml is missing. " + "Fix: create rules/registry.toml." +) +DUPLICATE_CATEGORY = ( + " - [registry] schema: duplicate category 'layers'. " + "Fix: give each category a unique id." +) +CI_SCRIPTS_TABLE = """ +[ci_scripts] +workflows = ".github/workflows/*.yml" +scripts_dir = ".github/scripts" +test_globs = ["tests/**/test_*.py"] +multiline_baseline = "rules/baselines/ci-multiline-run.txt" +untested_baseline = "rules/baselines/ci-untested-scripts.txt" +shellcheck_baseline = "rules/baselines/ci-shellcheck.txt" +""" + + +def run_cli(*argv: str) -> tuple[int, list[str]]: + """Run the command line and return its exit status and printed lines.""" + output = io.StringIO() + with contextlib.redirect_stdout(output): + status = main(list(argv)) + return status, output.getvalue().splitlines() + + +def with_duplicate_category(repo: Repo) -> None: + """Append a second category with an existing id, which validation rejects.""" + repo.write( + "rules/registry.toml", + repo.read("rules/registry.toml") + + '\n[[category]]\nid = "layers"\ntitle = "Layers again"\n', + ) + + +class ValidateCommandTest(unittest.TestCase): + def test_valid_registry_passes(self) -> None: + with Repo() as repo: + self.assertEqual( + run_cli("validate", "--root", str(repo.root)), (0, ["rules: passed"]) + ) + + def test_missing_registry_fails(self) -> None: + with tempfile.TemporaryDirectory() as root: + self.assertEqual( + run_cli("validate", "--root", root), + (1, [MISSING_REGISTRY, "rules: failed"]), + ) + + def test_violations_are_listed(self) -> None: + with Repo(render=False) as repo: + with_duplicate_category(repo) + self.assertEqual( + run_cli("validate", "--root", str(repo.root)), + (1, [DUPLICATE_CATEGORY, "rules: failed"]), + ) + + +class RenderCommandTest(unittest.TestCase): + def test_render_writes_both_views(self) -> None: + with Repo(render=False) as repo: + self.assertEqual( + run_cli("render", "--root", str(repo.root)), + (0, ["wrote docs/generated/rules.md", "wrote AGENTS.md"]), + ) + self.assertTrue((repo.root / "docs/generated/rules.md").is_file()) + self.assertIn("BEGIN GENERATED: rules", repo.read("AGENTS.md")) + + def test_render_refuses_an_invalid_registry(self) -> None: + with Repo(render=False) as repo: + with_duplicate_category(repo) + self.assertEqual( + run_cli("render", "--root", str(repo.root)), + ( + 1, + [ + "render refused: fix the registry first", + DUPLICATE_CATEGORY, + "rules: failed", + ], + ), + ) + self.assertFalse((repo.root / "docs/generated/rules.md").exists()) + + def test_render_reports_a_missing_registry(self) -> None: + with tempfile.TemporaryDirectory() as root: + self.assertEqual( + run_cli("render", "--root", root), + (1, [MISSING_REGISTRY, "rules: failed"]), + ) + self.assertFalse((Path(root) / "docs").exists()) + + +class GuardCommandTest(unittest.TestCase): + def test_guard_prints_notes_then_violations(self) -> None: + result = GuardResult( + violations=[Violation("ex.rule", "ci", "job is missing", "add the job")], + notes=["base ref unavailable"], + ) + with ( + Repo(render=False) as repo, + mock.patch("scripts.rules.cli.run_guard", return_value=result) as guard, + ): + self.assertEqual( + run_cli("guard", "--root", str(repo.root), "--base-ref", "origin/main"), + ( + 1, + [ + "note: base ref unavailable", + " - [ex.rule] ci: job is missing. Fix: add the job.", + "rules: failed", + ], + ), + ) + guard.assert_called_once_with(repo.root.resolve(), base_ref="origin/main") + + def test_clean_guard_passes(self) -> None: + with ( + Repo(render=False) as repo, + mock.patch( + "scripts.rules.cli.run_guard", return_value=GuardResult() + ) as guard, + ): + self.assertEqual( + run_cli("guard", "--root", str(repo.root)), (0, ["rules: passed"]) + ) + guard.assert_called_once_with(repo.root.resolve(), base_ref=None) + + +class CiScriptsCommandTest(unittest.TestCase): + def test_check_prints_notes_and_violations(self) -> None: + violation = Violation(None, "ci-scripts", "untested script x.sh", "test it") + with ( + Repo(render=False) as repo, + mock.patch( + "scripts.rules.ci_scripts.run_check", + return_value=([violation], ["shellcheck is not installed"]), + ) as run_check, + ): + self.assertEqual( + run_cli("ci-scripts", "--root", str(repo.root)), + ( + 1, + [ + "note: shellcheck is not installed", + " - [registry] ci-scripts: untested script x.sh. " + "Fix: test it.", + "rules: failed", + ], + ), + ) + run_check.assert_called_once_with(repo.root.resolve()) + + def test_check_without_findings_passes(self) -> None: + with ( + Repo(render=False) as repo, + mock.patch("scripts.rules.ci_scripts.run_check", return_value=([], [])), + ): + self.assertEqual( + run_cli("ci-scripts", "--root", str(repo.root)), (0, ["rules: passed"]) + ) + + def test_write_baselines_needs_a_registry(self) -> None: + with tempfile.TemporaryDirectory() as root: + self.assertEqual( + run_cli("ci-scripts", "--root", root, "--write-baselines"), + (1, [MISSING_REGISTRY, "rules: failed"]), + ) + + def test_write_baselines_needs_a_ci_scripts_table(self) -> None: + with Repo(render=False) as repo: + self.assertEqual( + run_cli("ci-scripts", "--root", str(repo.root), "--write-baselines"), + (1, [" - no [ci_scripts] table", "rules: failed"]), + ) + + def test_write_baselines_reports_schema_problems_first(self) -> None: + with Repo(render=False) as repo: + repo.edit("rules/registry.toml", 'repo = "example"', "repo = 7") + self.assertEqual( + run_cli("ci-scripts", "--root", str(repo.root), "--write-baselines"), + ( + 1, + [ + " - [registry] schema: registry.repo must be a non-empty " + "string. Fix: set repo.", + "rules: failed", + ], + ), + ) + + def test_write_baselines_records_the_findings(self) -> None: + findings = Findings( + multiline={"wf.yml :: build :: Build"}, + untested={".github/scripts/b.sh", ".github/scripts/a.sh"}, + shellcheck=None, + ) + with ( + Repo({"rules/registry.toml": REGISTRY + CI_SCRIPTS_TABLE}) as repo, + mock.patch("scripts.rules.ci_scripts.find", return_value=findings) as find, + ): + self.assertEqual( + run_cli("ci-scripts", "--root", str(repo.root), "--write-baselines"), + (0, ["wrote ci_scripts baselines; update the rules' baseline counts"]), + ) + find.assert_called_once() + self.assertEqual(find.call_args.args[1], repo.root.resolve()) + header = "# Tolerated existing violations. This list may only shrink.\n" + self.assertEqual( + repo.read("rules/baselines/ci-multiline-run.txt"), + header + "wf.yml :: build :: Build\n", + ) + self.assertEqual( + repo.read("rules/baselines/ci-untested-scripts.txt"), + header + ".github/scripts/a.sh\n.github/scripts/b.sh\n", + ) + self.assertFalse((repo.root / "rules/baselines/ci-shellcheck.txt").exists()) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/rules/tests/test_load.py b/scripts/rules/tests/test_load.py new file mode 100644 index 000000000..a25b017c2 --- /dev/null +++ b/scripts/rules/tests/test_load.py @@ -0,0 +1,194 @@ +"""Tests for loading rule tables: every schema problem is reported, in order.""" + +from __future__ import annotations + +import tomllib +import unittest +from datetime import date + +from scripts.rules.load import parse +from scripts.rules.model import ( + Baseline, + CiRef, + Enforcer, + Kind, + Proof, + Rule, + RuleException, + Status, + Tier, +) +from scripts.rules.tests.support import REGISTRY + +BOUNDARY_NODE = "tests/unit/test_boundaries.py::test_routes_have_no_sql" +KNOWN_RULE_KEYS = ( + "['autofix', 'baseline', 'category', 'ci', 'downgrade', 'enforcer', " + "'exception', 'id', 'proof', 'rationale', 'review_by', 'scope', 'statement', " + "'status', 'tier']" +) +KINDS = ( + "['pytest', 'quality-guard', 'ruff', 'mypy', 'pyright', 'import-linter', " + "'workflow-job', 'docs-guard', 'llm-review', 'none']" +) +MALFORMED = "[ex.layers.malformed] schema: ex.layers.malformed" + +MALFORMED_RULE = { + "id": "ex.layers.malformed", + "category": "layers", + "statement": "Malformed.", + "rationale": "docs/guide.md", + "scope": ["pkg/*.py"], + "tier": "mechanical", + "status": "enforced", + "autofix": "yes", + "review_by": "someday", + "enforcer": "pytest", + "ci": [ + 5, + { + "workflow": ".github/workflows/pr.yml", + "job": "", + "invocation": "make test", + "extra": 1, + }, + ], + "proof": "proof.py", + "baseline": {"file": "rules/baselines/x.txt"}, + "exception": [ + 7, + {"path": "pkg/*.py", "reason": "r", "expires": "soon", "extra": 1}, + ], + "downgrade": {"why": "x"}, + "bogus": 1, +} +UNKNOWN_TIER_RULE = { + "category": "layers", + "tier": "unknown", + "status": "enforced", + "autofix": False, + "enforcer": {"kind": "flake8", "codes": ["E1"]}, +} +SINGLE_TABLES_RULE = { + "id": "ex.layers.single-tables", + "category": "layers", + "statement": "Single tables.", + "rationale": "docs/guide.md", + "scope": ["pkg/*.py"], + "tier": "metric", + "status": "ratcheted", + "autofix": True, + "review_by": date(2026, 7, 1), + "enforcer": {"kind": "pytest", "nodes": [BOUNDARY_NODE]}, + "ci": { + "workflow": ".github/workflows/pr.yml", + "job": "test", + "invocation": "make test", + }, + "proof": {"test": BOUNDARY_NODE}, + "baseline": {"file": "rules/baselines/x.txt", "count": 2, "extra": True}, + "exception": {"path": "pkg/*.py", "reason": "legacy", "expires": "2026-12-31"}, + "downgrade": {"reason": "too noisy"}, +} + + +class RuleTableTest(unittest.TestCase): + def setUp(self) -> None: + raw = tomllib.loads(REGISTRY) + raw["rule"] = [ + "not a table", + MALFORMED_RULE, + UNKNOWN_TIER_RULE, + SINGLE_TABLES_RULE, + ] + self.registry, self.violations = parse(raw) + + def test_every_problem_is_reported_in_order(self) -> None: + self.assertEqual( + [str(violation) for violation in self.violations], + [ + "[registry] schema: rule #1 must be a table. Fix: write [[rule]].", + "[ex.layers.malformed] schema: unknown key 'bogus' in " + "ex.layers.malformed. Fix: remove it or use one of " + f"{KNOWN_RULE_KEYS}.", + f"{MALFORMED}.enforcer must be a table. Fix: add [rule.enforcer].", + f"{MALFORMED}.ci entries must be tables. Fix: write [[rule.ci]].", + "[ex.layers.malformed] schema: unknown key 'extra' in " + "ex.layers.malformed.ci. Fix: remove it or use one of " + "['invocation', 'job', 'workflow'].", + f"{MALFORMED}.ci.job must be a non-empty string. Fix: set job.", + f"{MALFORMED}.proof must be a table. Fix: write [rule.proof].", + f"{MALFORMED}.baseline needs 'file' and an integer 'count'. " + "Fix: write [rule.baseline] with file and count.", + f"{MALFORMED}.exception entries must be tables. " + "Fix: write [[rule.exception]].", + "[ex.layers.malformed] schema: unknown key 'extra' in " + "ex.layers.malformed.exception. Fix: remove it or use one of " + "['expires', 'path', 'reason'].", + f"{MALFORMED}.exception.expires must be a date. " + "Fix: write it as YYYY-MM-DD.", + f"{MALFORMED}.downgrade needs a 'reason'. " + 'Fix: write [rule.downgrade] reason = "...".', + f"{MALFORMED}.autofix must be true or false. Fix: set autofix.", + f"{MALFORMED}.review_by must be a date. Fix: write it as YYYY-MM-DD.", + f"[registry] schema: rule #3.enforcer.kind is 'flake8'. " + f"Fix: use one of {KINDS}.", + "[registry] schema: rule #3.tier is 'unknown'. " + "Fix: use one of ['mechanical', 'metric', 'judgment'].", + "[ex.layers.single-tables] schema: unknown key 'extra' in " + "ex.layers.single-tables.baseline. Fix: remove it or use one of " + "['count', 'file'].", + ], + ) + + def test_malformed_fields_fall_back_to_safe_values(self) -> None: + assert self.registry is not None + self.assertEqual( + self.registry.rule("ex.layers.malformed"), + Rule( + id="ex.layers.malformed", + category="layers", + statement="Malformed.", + rationale="docs/guide.md", + scope=("pkg/*.py",), + tier=Tier.MECHANICAL, + status=Status.ENFORCED, + autofix=False, + enforcer=Enforcer(Kind.NONE), + ci=(CiRef(".github/workflows/pr.yml", "", "make test"),), + exceptions=(RuleException("pkg/*.py", "r", date.max),), + ), + ) + + def test_a_rule_with_an_unknown_tier_is_dropped(self) -> None: + assert self.registry is not None + self.assertEqual( + [rule.id for rule in self.registry.rules], + ["ex.layers.malformed", "ex.layers.single-tables"], + ) + + def test_single_tables_stand_for_one_entry_lists(self) -> None: + assert self.registry is not None + self.assertEqual( + self.registry.rule("ex.layers.single-tables"), + Rule( + id="ex.layers.single-tables", + category="layers", + statement="Single tables.", + rationale="docs/guide.md", + scope=("pkg/*.py",), + tier=Tier.METRIC, + status=Status.RATCHETED, + autofix=True, + enforcer=Enforcer(Kind.PYTEST, {"nodes": [BOUNDARY_NODE]}), + ci=(CiRef(".github/workflows/pr.yml", "test", "make test"),), + review_by=date(2026, 7, 1), + proof=Proof({"test": BOUNDARY_NODE}), + baseline=Baseline("rules/baselines/x.txt", 2), + exceptions=(RuleException("pkg/*.py", "legacy", date(2026, 12, 31)),), + downgrade_reason="too noisy", + ), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/rules/tests/test_render.py b/scripts/rules/tests/test_render.py new file mode 100644 index 000000000..dbed7c2dc --- /dev/null +++ b/scripts/rules/tests/test_render.py @@ -0,0 +1,211 @@ +"""Tests for the rendered views: enforcer descriptions and the mermaid graphs.""" + +from __future__ import annotations + +import unittest + +from scripts.rules.model import ( + Category, + CiRef, + Discovery, + Enforcer, + Kind, + Registry, + Rule, + Status, + Tier, +) +from scripts.rules.render import render_rules_md + +PR_WORKFLOW = ".github/workflows/pr.yml" + + +def make_rule( + rule_id: str, + enforcer: Enforcer, + ci: tuple[CiRef, ...] = (), + status: Status = Status.ENFORCED, +) -> Rule: + return Rule( + id=rule_id, + category="checks", + statement="A statement.", + rationale="docs/guide.md", + scope=("pkg/*.py",), + tier=Tier.MECHANICAL, + status=status, + autofix=False, + enforcer=enforcer, + ci=ci, + ) + + +def mermaid_blocks(rules: tuple[Rule, ...]) -> list[list[str]]: + """Render the rules in one category and return each mermaid block's lines.""" + registry = Registry( + schema_version=1, + repo="example", + categories=(Category("checks", "Checks"),), + discovery=Discovery(), + rules=rules, + ) + blocks: list[list[str]] = [] + current: list[str] | None = None + for line in render_rules_md(registry).splitlines(): + if line == "```mermaid": + current = [line] + elif current is not None: + current.append(line) + if line == "```": + blocks.append(current) + current = None + return blocks + + +def rule_node(number: int) -> str: + """Return the mermaid declaration of the numbered rule in the chunking test.""" + return f' r_ex_many_rule_{number:02d}("ex.many.rule-{number:02d} (enforced)")' + + +class EnforcerDescriptionTest(unittest.TestCase): + def test_each_kind_describes_its_checker(self) -> None: + cases: list[tuple[Kind, dict[str, object], str]] = [ + ( + Kind.PYTEST, + {"project": "proj/", "nodes": ["b.py::t", "a.py::u", "a.py::v"]}, + "pytest `proj/a.py`, `proj/b.py`", + ), + (Kind.PYTEST, {"nodes": ["x.py"]}, "pytest `x.py`"), + (Kind.PYTEST, {}, "pytest "), + ( + Kind.QUALITY_GUARD, + {"name": "rules-registry"}, + "quality guard `rules-registry`", + ), + (Kind.QUALITY_GUARD, {}, "quality guard ``"), + (Kind.RUFF, {"codes": ["E1"], "mode": "format"}, "ruff format"), + ( + Kind.RUFF, + {"codes": ["E1", "F2"], "mode": "per-file-ignore", "pattern": "t/**"}, + "ruff per-file ignore of E1, F2 for `t/**`", + ), + (Kind.RUFF, {"mode": "per-file-ignore"}, "ruff per-file ignore of for ``"), + (Kind.RUFF, {"codes": ["E1", "F2"], "mode": "lint"}, "ruff check (E1, F2)"), + (Kind.RUFF, {}, "ruff check ()"), + (Kind.MYPY, {"paths": ["pkg"]}, "mypy"), + (Kind.WORKFLOW_JOB, {"step": "Build"}, "CI job"), + ( + Kind.PYRIGHT, + {"config": "pyrightconfig.json"}, + "pyright `pyrightconfig.json`", + ), + (Kind.PYRIGHT, {}, "pyright ``"), + ( + Kind.IMPORT_LINTER, + {"config": ".importlinter"}, + "import-linter `.importlinter`", + ), + (Kind.DOCS_GUARD, {"doc": "docs/x.md"}, "docs guard on `docs/x.md`"), + (Kind.DOCS_GUARD, {}, "docs guard on ``"), + (Kind.LLM_REVIEW, {"prompt": "p.md"}, "LLM review with `p.md`"), + (Kind.LLM_REVIEW, {}, "LLM review with ``"), + (Kind.NONE, {"name": "ignored"}, "nothing"), + ] + for kind, fields, expected in cases: + with self.subTest(kind=kind, fields=fields): + self.assertEqual(Enforcer(kind, fields).description, expected) + + +class MermaidTest(unittest.TestCase): + def test_graph_declares_each_node_once_and_skips_repeated_edges(self) -> None: + guard = Enforcer(Kind.QUALITY_GUARD, {"name": 'say "hi"'}) + mypy = Enforcer(Kind.MYPY, {"paths": ["pkg"]}) + rules = ( + make_rule("ex.a.unenforced", Enforcer(Kind.NONE), status=Status.DOC_ONLY), + make_rule( + "ex.b.also-unenforced", Enforcer(Kind.NONE), status=Status.DOC_ONLY + ), + make_rule( + "ex.c.guard", + guard, + ( + CiRef(PR_WORKFLOW, "guards", "run"), + CiRef(".github/workflows/push.yml", "guards", "run"), + ), + ), + make_rule("ex.d.same-guard", guard, (CiRef(PR_WORKFLOW, "guards", "run"),)), + make_rule("ex.e.local", mypy), + make_rule("ex.f.local-too", mypy), + make_rule( + "ex.g.job", + Enforcer(Kind.WORKFLOW_JOB), + ( + CiRef(PR_WORKFLOW, "build", "make"), + CiRef(PR_WORKFLOW, "deploy", "make"), + ), + ), + ) + self.assertEqual( + mermaid_blocks(rules), + [ + [ + "```mermaid", + "flowchart LR", + ' r_ex_a_unenforced("ex.a.unenforced (doc-only)")', + ' gap_unenforced>"not enforced"]', + ' r_ex_b_also_unenforced("ex.b.also-unenforced (doc-only)")', + ' r_ex_c_guard("ex.c.guard (enforced)")', + " e_quality_guard_say__hi_[\"quality guard say 'hi'\"]", + ' j_CI_pr_yml_guards["CI pr.yml:guards"]', + ' j_CI_push_yml_guards["CI push.yml:guards"]', + ' r_ex_d_same_guard("ex.d.same-guard (enforced)")', + ' r_ex_e_local("ex.e.local (enforced)")', + ' e_mypy["mypy"]', + ' local_only>"not run in CI"]', + ' r_ex_f_local_too("ex.f.local-too (enforced)")', + ' r_ex_g_job("ex.g.job (enforced)")', + ' e_CI_job_build__deploy["CI job build, deploy"]', + ' j_CI_pr_yml_build["CI pr.yml:build"]', + ' j_CI_pr_yml_deploy["CI pr.yml:deploy"]', + " r_ex_a_unenforced -.-> gap_unenforced", + " r_ex_b_also_unenforced -.-> gap_unenforced", + " r_ex_c_guard --> e_quality_guard_say__hi_", + " e_quality_guard_say__hi_ --> j_CI_pr_yml_guards", + " e_quality_guard_say__hi_ --> j_CI_push_yml_guards", + " r_ex_d_same_guard --> e_quality_guard_say__hi_", + " r_ex_e_local --> e_mypy", + " e_mypy -.-> local_only", + " r_ex_f_local_too --> e_mypy", + " r_ex_g_job --> e_CI_job_build__deploy", + " e_CI_job_build__deploy --> j_CI_pr_yml_build", + " e_CI_job_build__deploy --> j_CI_pr_yml_deploy", + "```", + ] + ], + ) + + def test_large_graphs_are_split_into_chunks_of_rules(self) -> None: + rules = tuple( + make_rule( + f"ex.many.rule-{number:02d}", + Enforcer(Kind.QUALITY_GUARD, {"name": f"guard-{number:02d}"}), + (CiRef(PR_WORKFLOW, f"job-{number:02d}", "run"),), + ) + for number in range(20) + ) + blocks = mermaid_blocks(rules) + self.assertEqual([len(block) for block in blocks], [78, 28]) + self.assertEqual( + [ + [line for line in block if line.startswith(" r_") and "(" in line] + for block in blocks + ], + [ + [rule_node(number) for number in range(15)], + [rule_node(number) for number in range(15, 20)], + ], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/rules/tests/test_resolve.py b/scripts/rules/tests/test_resolve.py new file mode 100644 index 000000000..750f83341 --- /dev/null +++ b/scripts/rules/tests/test_resolve.py @@ -0,0 +1,196 @@ +"""Tests that a rule's CI job really collects its test nodes and matrix project.""" + +from __future__ import annotations + +import unittest + +from scripts.rules.model import CiRef, Enforcer, Kind, Proof, Rule, Status, Tier +from scripts.rules.resolve import resolve_ci, resolve_proof_ci +from scripts.rules.tests.support import Repo + +PR_WORKFLOW = ".github/workflows/pr.yml" +WORKFLOW = """ +jobs: + test: + strategy: + matrix: + project: [projects/alpha, projects/beta] + steps: + - run: make test + unit: + steps: + - run: python -m unittest discover -s tools/tests -t . + unit-default: + steps: + - run: python -m unittest discover + other: + steps: + - run: tox -e py +""" +MAKEFILE = ( + "test:\n" + "\tpython -m pytest tests/unit tests/contract/test_a.py --disable-pytest-warnings\n" +) +MAKE_TEST = "'make test' (collects ['tests/unit', 'tests/contract/test_a.py'])" +UNITTEST = "'unittest discover -s tools/tests' (collects ['tools/tests'])" + + +def make_rule( + fields: dict[str, object], + ci: tuple[CiRef, ...] = (), + proof: dict[str, object] | None = None, + kind: Kind = Kind.PYTEST, +) -> Rule: + return Rule( + id="ex.r", + category="checks", + statement="A statement.", + rationale="docs/guide.md", + scope=("pkg/*.py",), + tier=Tier.MECHANICAL, + status=Status.ENFORCED, + autofix=False, + enforcer=Enforcer(kind, fields), + ci=ci, + proof=Proof(proof) if proof is not None else None, + ) + + +def not_collected(check: str, path: str, command: str) -> str: + return ( + f"[ex.r] {check}: {path} is not collected by {command}. " + "Fix: add it to what the job runs, or move the test." + ) + + +def not_in_matrix(check: str, project: str) -> str: + return ( + f"[ex.r] {check}: project {project} is not in the matrix of job 'test'. " + "Fix: add the project to the job matrix." + ) + + +class TestCoverageTest(unittest.TestCase): + def setUp(self) -> None: + self.repo = Repo({PR_WORKFLOW: WORKFLOW, "Makefile": MAKEFILE}, render=False) + self.addCleanup(self.repo.__exit__) + + def ci_messages(self, rule: Rule) -> list[str]: + return [str(violation) for violation in resolve_ci(rule, self.repo.root)] + + def proof_messages(self, rule: Rule) -> list[str]: + return [str(violation) for violation in resolve_proof_ci(rule, self.repo.root)] + + def test_make_target_must_collect_every_node(self) -> None: + rule = make_rule( + { + "nodes": [ + "tests/unit/test_a.py::t", + "tests/other/test_b.py::t", + "tests/contract/test_a.py", + "tests/unitx/test_c.py", + ] + }, + (CiRef(PR_WORKFLOW, "test", "make test"),), + ) + self.assertEqual( + self.ci_messages(rule), + [ + not_collected("ci", "tests/other/test_b.py", MAKE_TEST), + not_collected("ci", "tests/unitx/test_c.py", MAKE_TEST), + ], + ) + + def test_project_must_be_in_the_job_matrix(self) -> None: + missing = make_rule( + {"project": "projects/gamma", "nodes": ["tests/test_g.py::t"]}, + (CiRef(PR_WORKFLOW, "test", "make test"),), + ) + self.assertEqual( + self.ci_messages(missing), + [ + not_collected("ci", "projects/gamma/tests/test_g.py", MAKE_TEST), + not_in_matrix("ci", "projects/gamma"), + ], + ) + listed = make_rule( + {"project": "projects/alpha", "nodes": ["tests/test_g.py::t"]}, + (CiRef(PR_WORKFLOW, "test", "make test"),), + ) + self.assertEqual( + self.ci_messages(listed), + [not_collected("ci", "projects/alpha/tests/test_g.py", MAKE_TEST)], + ) + + def test_unittest_start_directory_must_hold_every_node(self) -> None: + rule = make_rule( + { + "nodes": [ + "tools/tests/test_a.py::T::t", + "tools/testsx/test_b.py", + "other/test_c.py", + ] + }, + (CiRef(PR_WORKFLOW, "unit", "unittest discover -s tools/tests"),), + ) + self.assertEqual( + self.ci_messages(rule), + [ + not_collected("ci", "tools/testsx/test_b.py", UNITTEST), + not_collected("ci", "other/test_c.py", UNITTEST), + ], + ) + + def test_other_commands_are_trusted(self) -> None: + default_discovery = make_rule( + {"nodes": ["other/test_c.py"]}, + (CiRef(PR_WORKFLOW, "unit-default", "unittest discover"),), + ) + other_command = make_rule( + {"project": "projects/gamma", "nodes": ["other/test_c.py"]}, + (CiRef(PR_WORKFLOW, "other", "tox"),), + ) + self.assertEqual(self.ci_messages(default_discovery), []) + self.assertEqual(self.ci_messages(other_command), []) + + def test_named_proof_job_must_collect_the_proof(self) -> None: + rule = make_rule( + {"paths": ["pkg"]}, + proof={ + "test": "elsewhere/test_p.py::t", + "project": "projects/gamma/", + "workflow": PR_WORKFLOW, + "job": "test", + "invocation": "make test", + }, + kind=Kind.MYPY, + ) + self.assertEqual( + self.proof_messages(rule), + [ + not_collected("proof", "projects/gamma/elsewhere/test_p.py", MAKE_TEST), + not_in_matrix("proof", "projects/gamma/"), + ], + ) + + def test_pytest_proof_runs_in_each_ci_job(self) -> None: + rule = make_rule( + {"nodes": ["tests/unit/test_a.py::t"]}, + ( + CiRef(PR_WORKFLOW, "unit", "unittest discover -s tools/tests"), + CiRef(PR_WORKFLOW, "other", "tox"), + ), + proof={"test": "tests/unit/test_p.py::t"}, + ) + self.assertEqual( + self.ci_messages(rule), + [not_collected("ci", "tests/unit/test_a.py", UNITTEST)], + ) + self.assertEqual( + self.proof_messages(rule), + [not_collected("proof", "tests/unit/test_p.py", UNITTEST)], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/rules/tests/test_validate.py b/scripts/rules/tests/test_validate.py new file mode 100644 index 000000000..b3c575dbc --- /dev/null +++ b/scripts/rules/tests/test_validate.py @@ -0,0 +1,40 @@ +"""Tests for registry validation: every problem is reported, in a stable order.""" + +from __future__ import annotations + +import unittest +from pathlib import Path + +from scripts.rules.load import load_text +from scripts.rules.tests.support import REGISTRY, TODAY, Repo +from scripts.rules.validate import validate + +FIXTURES = Path(__file__).parent / "fixtures" / "validate" + + +class BrokenRegistryTest(unittest.TestCase): + """One registry that breaks every validation check at least once.""" + + def test_every_violation_is_reported_in_order(self) -> None: + files = { + "rules/registry.toml": REGISTRY + + "\n" + + (FIXTURES / "broken_rules.toml").read_text(), + "CLAUDE.md": "Read AGENTS.md.\n", + "rules/baselines/ci-multiline-run.txt": "", + "rules/baselines/ruff.txt": ( + "# header\n\nentry one\n # indented comment\nentry two\n" + ), + } + with Repo(files, render=False) as repo: + registry, load_violations = load_text(repo.read("rules/registry.toml")) + self.assertEqual(load_violations, []) + assert registry is not None + self.assertEqual( + [str(violation) for violation in validate(registry, repo.root, TODAY)], + (FIXTURES / "broken_rules_violations.txt").read_text().splitlines(), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/rules/tests/test_workflows.py b/scripts/rules/tests/test_workflows.py index bf03f566e..e89793932 100644 --- a/scripts/rules/tests/test_workflows.py +++ b/scripts/rules/tests/test_workflows.py @@ -8,7 +8,7 @@ import unittest from pathlib import Path -from scripts.rules.workflows import parse_jobs +from scripts.rules.workflows import JobBlock, RunBlock, parse_jobs FIXTURES = Path(__file__).parent / "fixtures" @@ -83,5 +83,160 @@ def test_anchors_are_reported(self) -> None: self.assertTrue(problems) +SYNTHETIC_WORKFLOW = """ +name: Synthetic +on: push +jobs: + test: + name: "Quoted name" + strategy: + matrix: + project: [a, "b", 'c'] + python: + - "3.11" + - 3.12 + + - '3.13' + include: + - project: x + steps: + - name: First + run: > + folded one + folded two + - uses: actions/checkout@v4 + - run: echo a && echo b + - "just a string" + - name: Third + run: | + echo "a; b" + + echo ${{ x || y }} + - name: Fourth + uses: org/action@v1 + with: + - name: not a step + reusable: + uses: org/repo/.github/workflows/x.yml@main + other: + steps: + - name: Unindented list + run: make test + - run: | + multi + line + name: Named after run +permissions: {} +""" + + +class JobParsingTest(unittest.TestCase): + """Pins every field the reader extracts from a job, quirks included.""" + + def setUp(self) -> None: + self.jobs, self.problems = parse_jobs(SYNTHETIC_WORKFLOW) + + def test_jobs_end_at_the_next_top_level_key(self) -> None: + self.assertEqual(self.problems, []) + self.assertEqual(list(self.jobs), ["test", "reusable", "other"]) + + def test_job_with_matrix_and_steps(self) -> None: + self.assertEqual( + self.jobs["test"], + JobBlock( + id="test", + name="Quoted name", + run_text=( + 'folded one\nfolded two\necho a && echo b\necho "a; b"\n\n' + "echo ${{ x || y }}" + ), + uses="", + step_names=("First", "Third", "Fourth", "not a step"), + step_uses=("actions/checkout@v4", "org/action@v1"), + # A blank line ends a block-style matrix list. + matrix={ + "project": ("a", "b", "c"), + "python": ("3.11", "3.12"), + "include": (), + }, + run_blocks=( + RunBlock( + step="First", + code_lines=2, + chained=False, + text="folded one\nfolded two", + ), + RunBlock( + step="step 3", + code_lines=1, + chained=True, + text="echo a && echo b", + ), + RunBlock( + step="Third", + code_lines=2, + chained=False, + text='echo "a; b"\n\necho ${{ x || y }}', + ), + ), + ), + ) + + def test_reusable_workflow_job(self) -> None: + self.assertEqual( + self.jobs["reusable"], + JobBlock( + id="reusable", + name="reusable", + run_text="", + uses="org/repo/.github/workflows/x.yml@main", + step_names=(), + ), + ) + + def test_steps_listed_at_the_key_indent(self) -> None: + self.assertEqual( + self.jobs["other"], + JobBlock( + id="other", + name="other", + run_text="make test\nmulti\nline", + uses="", + step_names=("Unindented list", "Named after run"), + run_blocks=( + RunBlock( + step="Unindented list", + code_lines=1, + chained=False, + text="make test", + ), + RunBlock( + step="Named after run", + code_lines=2, + chained=False, + text="multi\nline", + ), + ), + ), + ) + + def test_flow_style_jobs_are_reported(self) -> None: + self.assertEqual( + parse_jobs("jobs: {a: 1}\n"), + ({}, ["flow-style 'jobs:' mapping is not supported"]), + ) + + def test_workflow_without_jobs(self) -> None: + self.assertEqual(parse_jobs("name: x\n"), ({}, [])) + + def test_stray_job_level_line_is_reported(self) -> None: + jobs, problems = parse_jobs( + "jobs:\n a:\n steps: []\n - stray\n b:\n name: B\n" + ) + self.assertEqual(list(jobs), ["a", "b"]) + self.assertEqual(jobs["b"].name, "B") + self.assertEqual(problems, ["unexpected line at job level: '- stray'"]) + + if __name__ == "__main__": unittest.main() diff --git a/scripts/rules/validate.py b/scripts/rules/validate.py index 9fffbb5a4..67f17ec4e 100644 --- a/scripts/rules/validate.py +++ b/scripts/rules/validate.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING, Any from scripts.rules.model import Kind, Registry, Rule, Status, Tier, Violation -from scripts.rules.resolve import node_exists +from scripts.rules.resolve import PROOF_CI_FIELDS, node_exists if TYPE_CHECKING: from pathlib import Path @@ -144,53 +144,100 @@ def _validate_enforcer(rule: Rule) -> list[Violation]: return violations -def _validate_cross_fields(rule: Rule) -> list[Violation]: - violations = [] +def _schema_violation(rule: Rule, message: str, fix: str) -> Violation: + """Report a schema problem with one rule.""" + return Violation(rule.id, "schema", message, fix) - def v(message: str, fix: str) -> None: - violations.append(Violation(rule.id, "schema", message, fix)) - kind, status, tier = rule.enforcer.kind, rule.status, rule.tier +def _status_conflicts(rule: Rule) -> list[Violation]: + """Check that the rule's status fits its enforcer, CI wiring and baseline.""" + kind, status = rule.enforcer.kind, rule.status + violations = [] if status is Status.DOC_ONLY and kind is not Kind.NONE: - v( - "doc-only rules have no enforcer", - 'set enforcer.kind = "none", or pick a stronger status', + violations.append( + _schema_violation( + rule, + "doc-only rules have no enforcer", + 'set enforcer.kind = "none", or pick a stronger status', + ) ) if status is Status.DOC_ONLY and rule.ci: - v( - "doc-only rules have no CI wiring", - "remove rule.ci, or pick a stronger status", + violations.append( + _schema_violation( + rule, + "doc-only rules have no CI wiring", + "remove rule.ci, or pick a stronger status", + ) ) if kind is Kind.NONE and status not in (Status.DOC_ONLY, Status.ADVISORY): - v( - f"status {status} needs an enforcer", - "add [rule.enforcer] or use doc-only/advisory", + violations.append( + _schema_violation( + rule, + f"status {status} needs an enforcer", + "add [rule.enforcer] or use doc-only/advisory", + ) ) if status in (Status.ENFORCED, Status.RATCHETED) and not rule.ci: - v(f"{status} rules must run in CI", "add a [[rule.ci]] reference") + violations.append( + _schema_violation( + rule, f"{status} rules must run in CI", "add a [[rule.ci]] reference" + ) + ) if status is Status.RATCHETED and rule.baseline is None: - v("ratcheted rules need a baseline", "add [rule.baseline] or use enforced") + violations.append( + _schema_violation( + rule, + "ratcheted rules need a baseline", + "add [rule.baseline] or use enforced", + ) + ) if rule.baseline is not None and status is not Status.RATCHETED: - v( - "only ratcheted rules may have a baseline", - 'remove [rule.baseline] or set status = "ratcheted"', + violations.append( + _schema_violation( + rule, + "only ratcheted rules may have a baseline", + 'remove [rule.baseline] or set status = "ratcheted"', + ) ) + return violations + + +def _checker_conflicts(rule: Rule) -> list[Violation]: + """Check that the rule's enforcer kind suits its tier and baseline.""" + kind, tier = rule.enforcer.kind, rule.tier + violations = [] if kind is Kind.RUFF and rule.baseline is not None: - v( - "ruff is never ratcheted", - "remove the baseline and fix every violation in a standalone PR that " - "also changes the ruff select list", + violations.append( + _schema_violation( + rule, + "ruff is never ratcheted", + "remove the baseline and fix every violation in a standalone PR " + "that also changes the ruff select list", + ) ) if tier is Tier.JUDGMENT and kind not in (Kind.LLM_REVIEW, Kind.NONE): - v( - "judgment rules are enforced by review, not by a mechanical checker", - 'use enforcer.kind = "llm-review" or "none"', + violations.append( + _schema_violation( + rule, + "judgment rules are enforced by review, not by a mechanical checker", + 'use enforcer.kind = "llm-review" or "none"', + ) ) if kind is Kind.LLM_REVIEW and tier is not Tier.JUDGMENT: - v("llm-review only enforces judgment rules", 'set tier = "judgment"') + violations.append( + _schema_violation( + rule, + "llm-review only enforces judgment rules", + 'set tier = "judgment"', + ) + ) return violations +def _validate_cross_fields(rule: Rule) -> list[Violation]: + return _status_conflicts(rule) + _checker_conflicts(rule) + + def _validate_proof(rule: Rule, root: Path) -> list[Violation]: if rule.proof is None: return [] @@ -203,8 +250,8 @@ def _validate_proof(rule: Rule, root: Path) -> list[Violation]: violations = _check_fields( rule, fields, PROOF_FIELDS[rule.tier], optional, f"proof ({rule.tier})" ) - named = [key for key in ("workflow", "job", "invocation") if key in fields] - if named and len(named) != 3: + named = [key for key in PROOF_CI_FIELDS if key in fields] + if named and len(named) != len(PROOF_CI_FIELDS): violations.append( Violation( rule.id, @@ -243,120 +290,198 @@ def _validate_proof(rule: Rule, root: Path) -> list[Violation]: return violations -def validate(registry: Registry, root: Path, today: date) -> list[Violation]: - """Check the registry's fields and file references against the tree at ``root``. +def _validate_agents(registry: Registry, root: Path) -> list[Violation]: + """Check that every agent instruction file named in ``[agents]`` exists.""" + if registry.agents is None: + return [] + return [ + Violation( + None, + "schema", + f"agents file {path} does not exist", + "fix [agents] or create the file", + ) + for path in (registry.agents.instructions, *registry.agents.adapters) + if path and not (root / path).is_file() + ] - ``today`` decides whether review dates and exceptions have expired. - """ - violations: list[Violation] = [] - - def v(rule_id: str | None, message: str, fix: str, check: str = "schema") -> None: - violations.append(Violation(rule_id, check, message, fix)) - - if registry.agents is not None: - for path in (registry.agents.instructions, *registry.agents.adapters): - if path and not (root / path).is_file(): - v( - None, - f"agents file {path} does not exist", - "fix [agents] or create the file", - ) - if registry.ci_scripts is not None: - config = registry.ci_scripts - if config.scripts_dir and not (root / config.scripts_dir).is_dir(): - v( + +def _validate_ci_scripts(registry: Registry, root: Path) -> list[Violation]: + """Check that the ``[ci_scripts]`` directory and baseline files exist.""" + config = registry.ci_scripts + if config is None: + return [] + violations = [] + if config.scripts_dir and not (root / config.scripts_dir).is_dir(): + violations.append( + Violation( None, + "schema", f"ci_scripts.scripts_dir {config.scripts_dir} does not exist", "fix [ci_scripts]", ) + ) + violations.extend( + Violation( + None, + "schema", + f"ci_scripts baseline {path} does not exist", + "create it (it may be empty)", + ) for path in ( config.multiline_baseline, config.untested_baseline, config.shellcheck_baseline, - ): - if path and not (root / path).is_file(): - v( - None, - f"ci_scripts baseline {path} does not exist", - "create it (it may be empty)", - ) + ) + if path and not (root / path).is_file() + ) + return violations - category_ids = [category.id for category in registry.categories] - for duplicate in sorted({c for c in category_ids if category_ids.count(c) > 1}): - v(None, f"duplicate category '{duplicate}'", "give each category a unique id") +def _duplicate_ids(registry: Registry, category_ids: list[str]) -> list[Violation]: + """Report each category id, then each rule id, that is used more than once.""" + violations = [ + Violation( + None, + "schema", + f"duplicate category '{duplicate}'", + "give each category a unique id", + ) + for duplicate in sorted({c for c in category_ids if category_ids.count(c) > 1}) + ] rule_ids = [rule.id for rule in registry.rules] - for duplicate in sorted({r for r in rule_ids if rule_ids.count(r) > 1}): - v(duplicate, "duplicate rule id", "give each rule a unique id") + violations.extend( + Violation( + duplicate, "schema", "duplicate rule id", "give each rule a unique id" + ) + for duplicate in sorted({r for r in rule_ids if rule_ids.count(r) > 1}) + ) + return violations - for rule in registry.rules: - if rule.id and not RULE_ID.match(rule.id): - v( - rule.id, + +def _validate_identity( + rule: Rule, root: Path, category_ids: list[str] +) -> list[Violation]: + """Check the rule's id, category, rationale link and scope globs.""" + violations = [] + if rule.id and not RULE_ID.match(rule.id): + violations.append( + _schema_violation( + rule, "id must be lowercase dotted, e.g. 'api.v2.routes-no-sql'", "rename the rule", ) - if rule.category not in category_ids: - v( - rule.id, + ) + if rule.category not in category_ids: + violations.append( + _schema_violation( + rule, f"unknown category '{rule.category}'", f"use one of {category_ids} or add a [[category]]", ) - - rationale_path = rule.rationale.split("#", 1)[0] - if rationale_path and not (root / rationale_path).exists(): - v( - rule.id, + ) + rationale_path = rule.rationale.split("#", 1)[0] + if rationale_path and not (root / rationale_path).exists(): + violations.append( + _schema_violation( + rule, f"rationale {rationale_path} does not exist", "link an existing doc", ) + ) + violations.extend( + _schema_violation(rule, f"scope '{pattern}' matches no files", "fix the glob") + for pattern in rule.scope + if not any(root.glob(pattern)) + ) + return violations - for pattern in rule.scope: - if not any(root.glob(pattern)): - v(rule.id, f"scope '{pattern}' matches no files", "fix the glob") - violations.extend(_validate_enforcer(rule)) - violations.extend(_validate_cross_fields(rule)) - violations.extend(_validate_proof(rule, root)) +def _validate_baseline(rule: Rule, root: Path) -> list[Violation]: + """Check that a ratcheted rule's baseline file exists and matches its count.""" + if rule.baseline is None: + return [] + path = root / rule.baseline.file + if not path.is_file(): + return [ + Violation( + rule.id, + "baseline", + f"baseline file {rule.baseline.file} does not exist", + "create it", + ) + ] + if baseline_entries(path) != rule.baseline.count: + return [ + Violation( + rule.id, + "baseline", + f"baseline count is {rule.baseline.count} but " + f"{rule.baseline.file} lists {baseline_entries(path)}", + "make the count match the file", + ) + ] + return [] - if rule.baseline is not None: - path = root / rule.baseline.file - if not path.is_file(): - v( - rule.id, - f"baseline file {rule.baseline.file} does not exist", - "create it", - "baseline", - ) - elif baseline_entries(path) != rule.baseline.count: - v( - rule.id, - f"baseline count is {rule.baseline.count} but " - f"{rule.baseline.file} lists {baseline_entries(path)}", - "make the count match the file", - "baseline", - ) - if rule.review_by is not None and rule.review_by < today: - v( +def _validate_expiry(rule: Rule, root: Path, today: date) -> list[Violation]: + """Check the review date and each exception's expiry and path on ``today``.""" + violations = [] + if rule.review_by is not None and rule.review_by < today: + violations.append( + Violation( rule.id, + "expiry", f"review_by {rule.review_by} has passed", "review the rule, then remove or extend review_by", - "expiry", ) - for exception in rule.exceptions: - if exception.expires < today: - v( + ) + for exception in rule.exceptions: + if exception.expires < today: + violations.append( + Violation( rule.id, + "expiry", f"exception for {exception.path} expired on {exception.expires}", "remove the exception (fix the code) or extend it with a reason", - "expiry", ) - if not any(root.glob(exception.path)): - v( - rule.id, + ) + if not any(root.glob(exception.path)): + violations.append( + _schema_violation( + rule, f"exception path '{exception.path}' matches no files", "remove the stale exception", ) + ) + return violations + + +def _validate_rule( + rule: Rule, root: Path, category_ids: list[str], today: date +) -> list[Violation]: + """Run every per-rule check, in the order the guard reports them.""" + return [ + *_validate_identity(rule, root, category_ids), + *_validate_enforcer(rule), + *_validate_cross_fields(rule), + *_validate_proof(rule, root), + *_validate_baseline(rule, root), + *_validate_expiry(rule, root, today), + ] + +def validate(registry: Registry, root: Path, today: date) -> list[Violation]: + """Check the registry's fields and file references against the tree at ``root``. + + ``today`` decides whether review dates and exceptions have expired. + """ + category_ids = [category.id for category in registry.categories] + violations = [ + *_validate_agents(registry, root), + *_validate_ci_scripts(registry, root), + *_duplicate_ids(registry, category_ids), + ] + for rule in registry.rules: + violations.extend(_validate_rule(rule, root, category_ids, today)) return violations diff --git a/scripts/rules/workflows.py b/scripts/rules/workflows.py index 71ae03b13..53a2803f1 100644 --- a/scripts/rules/workflows.py +++ b/scripts/rules/workflows.py @@ -15,10 +15,13 @@ import re from dataclasses import dataclass, field +from typing import NamedTuple _KEY = re.compile(r"^(\s*)(-\s+)?([A-Za-z0-9_-]+):\s*(.*)$") _LIST_ITEM = re.compile(r"^(\s*)-\s+(.*)$") _UNSUPPORTED = re.compile(r"(<<:|:\s*[&*][A-Za-z0-9_-]+\s*$)") +# A quoted scalar is at least its opening and closing quote. +_QUOTE_PAIR_LENGTH = 2 @dataclass(frozen=True) @@ -77,7 +80,7 @@ def _strip_comment(line: str) -> str: def _unquote(value: str) -> str: value = value.strip() - if len(value) >= 2 and value[0] == value[-1] and value[0] in "'\"": + if len(value) >= _QUOTE_PAIR_LENGTH and value[0] == value[-1] and value[0] in "'\"": return value[1:-1] return value @@ -134,149 +137,201 @@ def _inline_list(value: str) -> tuple[str, ...]: return tuple(_unquote(item) for item in inner.split(",") if item.strip()) -def _parse_job(job_id: str, lines: list[str]) -> JobBlock: - name = "" - uses = "" - step_uses: list[str] = [] - runs: list[str] = [] - steps: list[str] = [] - matrix: dict[str, tuple[str, ...]] = {} - body_indent = None - # Step tracking, so each run: block can be attributed to its step. - item_indent: int | None = None - in_steps = False - step_index = 0 - step_key_column = -1 - step_labels: dict[int, str] = {} - blocks: list[tuple[int, str, str]] = [] - index = 0 - while index < len(lines): - line = lines[index] - match = _KEY.match(line) - if not match: - item = _LIST_ITEM.match(line) - if ( - in_steps - and item - and item_indent is not None - and len(item.group(1)) == item_indent - ): - step_index += 1 - index += 1 - continue +class _KeyLine(NamedTuple): + """One ``key: value`` line of a job, with the column its key starts at.""" + + leading: str + dash: str | None + key: str + value: str + column: int + top_level: bool + + +class _JobReader: + """Reads one job's lines, attributing each ``run:`` block to its step.""" + + def __init__(self, lines: list[str]) -> None: + self.lines = lines + self.index = 0 + self.name = "" + self.uses = "" + self.step_uses: list[str] = [] + self.runs: list[str] = [] + self.steps: list[str] = [] + self.matrix: dict[str, tuple[str, ...]] = {} + self.body_indent: int | None = None + # Step tracking, so each run: block can be attributed to its step. + self.item_indent: int | None = None + self.in_steps = False + self.step_index = 0 + self.step_key_column = -1 + self.step_labels: dict[int, str] = {} + self.blocks: list[tuple[int, str, str]] = [] + + def read(self) -> None: + """Read every line of the job.""" + while self.index < len(self.lines): + match = _KEY.match(self.lines[self.index]) + if match: + self._read_key(match) + else: + self._count_bare_step(self.lines[self.index]) + self.index += 1 + + def job_block(self, job_id: str) -> JobBlock: + """Return what was read as a job named ``job_id`` unless it has a name.""" + return JobBlock( + id=job_id, + name=self.name or job_id, + run_text="\n".join(self.runs), + uses=self.uses, + step_uses=tuple(self.step_uses), + step_names=tuple(self.steps), + matrix=self.matrix, + run_blocks=tuple( + RunBlock( + step=self.step_labels.get(number) or f"step {number}", + code_lines=code_lines(text), + chained=chains_commands(text), + text=text, + ) + for number, text, _style in self.blocks + ), + ) + + def _still_inside(self, column: int) -> bool: + """Whether the current line is blank or indented past ``column``.""" + return self.index < len(self.lines) and ( + not self.lines[self.index].strip() + or _indent(self.lines[self.index]) > column + ) + + def _count_bare_step(self, line: str) -> None: + """Count a list item that is not a key (such as a quoted string) as a step.""" + item = _LIST_ITEM.match(line) + if ( + self.in_steps + and item + and self.item_indent is not None + and len(item.group(1)) == self.item_indent + ): + self.step_index += 1 + + def _read_key(self, match: re.Match[str]) -> None: leading, dash, key, value = match.groups() - key_column = len(leading) + (len(dash) if dash else 0) - if body_indent is None and not dash: - body_indent = key_column - top_level = not dash and key_column == body_indent - if top_level: - in_steps = key == "steps" and not value - item_indent = None - elif in_steps and dash: - if item_indent is None: - item_indent = len(leading) - if len(leading) == item_indent: - step_index += 1 - step_key_column = key_column - if in_steps and key == "name" and key_column == step_key_column: - step_labels[step_index] = _unquote(value) - - if key == "name" and top_level: - name = _unquote(value) - elif key == "name": - steps.append(_unquote(value)) - elif key == "uses" and top_level: - uses = _unquote(value) - elif key == "uses": - step_uses.append(_unquote(value)) - elif key == "run": - if value.strip()[:1] in ("|", ">"): - block = [] - index += 1 - while index < len(lines) and ( - not lines[index].strip() or _indent(lines[index]) > key_column - ): - block.append(lines[index].strip()) - index += 1 - runs.append("\n".join(block)) - blocks.append((step_index, "\n".join(block), value.strip()[:1])) - continue - runs.append(_unquote(value)) - blocks.append((step_index, _unquote(value), "|")) - elif key == "matrix" and not value: - index += 1 - while index < len(lines) and ( - not lines[index].strip() or _indent(lines[index]) > key_column - ): - entry = _KEY.match(lines[index]) - if entry and not entry.group(2): - entry_key, entry_value = entry.group(3), entry.group(4) - entry_column = len(entry.group(1)) - if entry_value.startswith("["): - matrix[entry_key] = _inline_list(entry_value) - index += 1 - continue - items = [] - index += 1 - while index < len(lines) and ( - not lines[index].strip() - or _indent(lines[index]) >= entry_column - ): - item = _LIST_ITEM.match(lines[index]) - if not item or _KEY.match(lines[index]): - break - items.append(_unquote(item.group(2))) - index += 1 - matrix[entry_key] = tuple(items) - continue - index += 1 - continue - index += 1 - return JobBlock( - id=job_id, - name=name or job_id, - run_text="\n".join(runs), - uses=uses, - step_uses=tuple(step_uses), - step_names=tuple(steps), - matrix=matrix, - run_blocks=tuple( - RunBlock( - step=step_labels.get(number) or f"step {number}", - code_lines=code_lines(text), - chained=chains_commands(text), - text=text, - ) - for number, text, _style in blocks - ), - ) + column = len(leading) + (len(dash) if dash else 0) + if self.body_indent is None and not dash: + self.body_indent = column + entry = _KeyLine( + leading=leading, + dash=dash, + key=key, + value=value, + column=column, + top_level=not dash and column == self.body_indent, + ) + self._track_step(entry) + if entry.key == "run" and entry.value.strip()[:1] in ("|", ">"): + self._read_block_scalar(entry) + elif entry.key == "matrix" and not entry.value: + self._read_matrix(entry.column) + else: + self._record_scalar(entry) + self.index += 1 + + def _track_step(self, entry: _KeyLine) -> None: + """Note when a new step starts, and the name it gives itself.""" + if entry.top_level: + self.in_steps = entry.key == "steps" and not entry.value + self.item_indent = None + elif self.in_steps and entry.dash: + if self.item_indent is None: + self.item_indent = len(entry.leading) + if len(entry.leading) == self.item_indent: + self.step_index += 1 + self.step_key_column = entry.column + if ( + self.in_steps + and entry.key == "name" + and entry.column == self.step_key_column + ): + self.step_labels[self.step_index] = _unquote(entry.value) + + def _record_scalar(self, entry: _KeyLine) -> None: + """Record a one-line ``name:``, ``uses:`` or ``run:`` value.""" + if entry.key == "name" and entry.top_level: + self.name = _unquote(entry.value) + elif entry.key == "name": + self.steps.append(_unquote(entry.value)) + elif entry.key == "uses" and entry.top_level: + self.uses = _unquote(entry.value) + elif entry.key == "uses": + self.step_uses.append(_unquote(entry.value)) + elif entry.key == "run": + self.runs.append(_unquote(entry.value)) + self.blocks.append((self.step_index, _unquote(entry.value), "|")) + + def _read_block_scalar(self, entry: _KeyLine) -> None: + """Read a ``|`` or ``>`` run: block, one stripped line per script line.""" + block = [] + self.index += 1 + while self._still_inside(entry.column): + block.append(self.lines[self.index].strip()) + self.index += 1 + self.runs.append("\n".join(block)) + self.blocks.append((self.step_index, "\n".join(block), entry.value.strip()[:1])) + + def _read_matrix(self, column: int) -> None: + """Read the ``strategy.matrix`` entries nested under the key at ``column``.""" + self.index += 1 + while self._still_inside(column): + entry = _KEY.match(self.lines[self.index]) + if entry and not entry.group(2): + self._read_matrix_entry(entry) + else: + self.index += 1 + + def _read_matrix_entry(self, entry: re.Match[str]) -> None: + """Read one matrix key's values, written inline or as a block list.""" + key, value = entry.group(3), entry.group(4) + self.index += 1 + if value.startswith("["): + self.matrix[key] = _inline_list(value) + return + # Block list items may sit at the key's own column. + list_column = len(entry.group(1)) - 1 + items = [] + while self._still_inside(list_column): + item = _LIST_ITEM.match(self.lines[self.index]) + if not item or _KEY.match(self.lines[self.index]): + break + items.append(_unquote(item.group(2))) + self.index += 1 + self.matrix[key] = tuple(items) -def parse_jobs(text: str) -> tuple[dict[str, JobBlock], list[str]]: - """Return the jobs in a workflow, plus descriptions of unsupported constructs.""" - lines = [_strip_comment(line) for line in text.splitlines()] - problems = [ - f"line {number}: YAML anchors/aliases/merge keys are not supported" - for number, line in enumerate(lines, 1) - if _UNSUPPORTED.search(line) - ] +def _parse_job(job_id: str, lines: list[str]) -> JobBlock: + reader = _JobReader(lines) + reader.read() + return reader.job_block(job_id) - start = next( - (i for i, line in enumerate(lines) if re.match(r"^jobs:\s*$", line)), None - ) - if start is None: - if any(re.match(r"^jobs:\s*\S", line) for line in lines): - problems.append("flow-style 'jobs:' mapping is not supported") - return {}, problems +def _jobs_body(lines: list[str], start: int) -> list[str]: + """Return the lines after ``jobs:`` up to the next top-level key.""" body: list[str] = [] for line in lines[start + 1 :]: if line.strip() and _indent(line) == 0: break body.append(line) + return body + +def _split_jobs(body: list[str]) -> tuple[dict[str, JobBlock], list[str]]: + """Parse each job under ``jobs:``, and describe any stray job-level line.""" job_indent = next((_indent(line) for line in body if line.strip()), 0) jobs: dict[str, JobBlock] = {} + problems: list[str] = [] current: str | None = None current_lines: list[str] = [] for line in body: @@ -292,3 +347,24 @@ def parse_jobs(text: str) -> tuple[dict[str, JobBlock], list[str]]: if current: jobs[current] = _parse_job(current, current_lines) return jobs, problems + + +def parse_jobs(text: str) -> tuple[dict[str, JobBlock], list[str]]: + """Return the jobs in a workflow, plus descriptions of unsupported constructs.""" + lines = [_strip_comment(line) for line in text.splitlines()] + problems = [ + f"line {number}: YAML anchors/aliases/merge keys are not supported" + for number, line in enumerate(lines, 1) + if _UNSUPPORTED.search(line) + ] + + start = next( + (i for i, line in enumerate(lines) if re.match(r"^jobs:\s*$", line)), None + ) + if start is None: + if any(re.match(r"^jobs:\s*\S", line) for line in lines): + problems.append("flow-style 'jobs:' mapping is not supported") + return {}, problems + + jobs, job_problems = _split_jobs(_jobs_body(lines, start)) + return jobs, problems + job_problems diff --git a/scripts/smoke_test_local_boot.py b/scripts/smoke_test_local_boot.py index 1a08bc2f0..0cea25d40 100755 --- a/scripts/smoke_test_local_boot.py +++ b/scripts/smoke_test_local_boot.py @@ -17,7 +17,9 @@ import time import urllib.error import urllib.request +from http import HTTPStatus from pathlib import Path +from typing import NamedTuple REPO_ROOT = Path(__file__).resolve().parents[1] HEALTH_PATHS = ("/liveness-check", "/readiness-check") @@ -58,50 +60,48 @@ def print_section(title: str, body: str) -> None: _report(body.rstrip() if body else "(empty)") -def main() -> int: - """Boot the Flask app on a free port and return 1 unless both health checks pass. +class _AppLogs(NamedTuple): + """Where the app's captured stdout and stderr are written.""" - On failure print the app's captured stdout and stderr; always stop the app. - """ - port = find_free_port() - with tempfile.NamedTemporaryFile( - mode="w+", - encoding="utf-8", - delete=False, - prefix="policyengine-api-boot-", - suffix=".stdout.log", - ) as stdout_file: - stdout_path = Path(stdout_file.name) + stdout: Path + stderr: Path + + +def _log_path(suffix: str) -> Path: + """Create an empty log file that outlives this script, and return its path.""" with tempfile.NamedTemporaryFile( mode="w+", encoding="utf-8", delete=False, prefix="policyengine-api-boot-", - suffix=".stderr.log", - ) as stderr_file: - stderr_path = Path(stderr_file.name) + suffix=suffix, + ) as log_file: + return Path(log_file.name) + +def _app_environment() -> dict[str, str]: + """Return this process's environment with Flask debug mode switched on.""" env = os.environ.copy() env["FLASK_DEBUG"] = "1" env.setdefault("PYTHONUNBUFFERED", "1") + return env + +def _policyengine_version() -> str: + """Return the installed policyengine version, or "not installed".""" try: - policyengine_version = importlib.metadata.version("policyengine") + return importlib.metadata.version("policyengine") except importlib.metadata.PackageNotFoundError: - policyengine_version = "not installed" + return "not installed" - _report(f"repo: {REPO_ROOT}") - _report(f"python: {sys.executable}") - _report(f"policyengine: {policyengine_version}") - _report(f"port: {port}") - _report(f"stdout log: {stdout_path}") - _report(f"stderr log: {stderr_path}") +def _start_app(port: int, logs: _AppLogs, env: dict[str, str]) -> subprocess.Popen[str]: + """Start the Flask app on ``port``, writing its output to ``logs``.""" with ( - stdout_path.open("w", encoding="utf-8") as stdout_handle, - stderr_path.open("w", encoding="utf-8") as stderr_handle, + logs.stdout.open("w", encoding="utf-8") as stdout_handle, + logs.stderr.open("w", encoding="utf-8") as stderr_handle, ): - process = subprocess.Popen( # noqa: S603 # starts this repository's app with the current interpreter + return subprocess.Popen( # noqa: S603 # starts this repository's app with the current interpreter [ sys.executable, "-m", @@ -121,55 +121,103 @@ def main() -> int: text=True, ) - try: - deadline = time.time() + BOOT_TIMEOUT_SECONDS - ready = False - while time.time() < deadline: - if process.poll() is not None: - break - - try: - status, body = fetch(f"http://127.0.0.1:{port}/liveness-check") - if status == 200 and body.strip() == "OK": - ready = True - break - except (urllib.error.URLError, TimeoutError, ConnectionError): - time.sleep(0.5) - continue +def _is_ok(status: int, body: str) -> bool: + """Whether a health endpoint answered 200 with the body "OK".""" + return status == HTTPStatus.OK and body.strip() == "OK" + + +def _wait_until_live(process: subprocess.Popen[str], port: int) -> bool: + """Poll the liveness check until it passes, the app exits, or time runs out.""" + deadline = time.time() + BOOT_TIMEOUT_SECONDS + while time.time() < deadline: + if process.poll() is not None: + return False + + try: + status, body = fetch(f"http://127.0.0.1:{port}/liveness-check") + if _is_ok(status, body): + return True + except (urllib.error.URLError, TimeoutError, ConnectionError): time.sleep(0.5) + continue + + time.sleep(0.5) + return False - if not ready: - process.wait( - timeout=5 - ) if process.poll() is not None else process.terminate() - if process.poll() is None: - process.wait(timeout=5) - _report("\nBoot smoke test failed: app did not become ready.") - print_section("stdout", read_text(stdout_path)) - print_section("stderr", read_text(stderr_path)) - return 1 + +def _stop_unready_app(process: subprocess.Popen[str]) -> None: + """Reap an app that exited, or stop one that is still starting.""" + if process.poll() is not None: + process.wait(timeout=5) + else: + process.terminate() + if process.poll() is None: + process.wait(timeout=5) + + +def _probe_health(port: int) -> bool: + """Print each health endpoint's answer, stopping at the first that is not OK.""" + for path in HEALTH_PATHS: + status, body = fetch(f"http://127.0.0.1:{port}{path}") + _report(f"{path}: {status} {body.strip()}") + if not _is_ok(status, body): + return False + return True + + +def _report_failure(message: str, logs: _AppLogs) -> int: + """Print the failure and the app's captured output, and return exit status 1.""" + _report(message) + print_section("stdout", read_text(logs.stdout)) + print_section("stderr", read_text(logs.stderr)) + return 1 + + +def _stop_app(process: subprocess.Popen[str]) -> None: + """Terminate the app if it is still running, killing it if it will not stop.""" + if process.poll() is None: + process.terminate() + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=5) + + +def main() -> int: + """Boot the Flask app on a free port and return 1 unless both health checks pass. + + On failure print the app's captured stdout and stderr; always stop the app. + """ + port = find_free_port() + logs = _AppLogs(stdout=_log_path(".stdout.log"), stderr=_log_path(".stderr.log")) + env = _app_environment() + policyengine_version = _policyengine_version() + + _report(f"repo: {REPO_ROOT}") + _report(f"python: {sys.executable}") + _report(f"policyengine: {policyengine_version}") + _report(f"port: {port}") + _report(f"stdout log: {logs.stdout}") + _report(f"stderr log: {logs.stderr}") + + process = _start_app(port, logs, env) + try: + if not _wait_until_live(process, port): + _stop_unready_app(process) + return _report_failure( + "\nBoot smoke test failed: app did not become ready.", logs + ) _report("\nBoot smoke test reached a live app. Probing health endpoints:") - for path in HEALTH_PATHS: - status, body = fetch(f"http://127.0.0.1:{port}{path}") - _report(f"{path}: {status} {body.strip()}") - if status != 200 or body.strip() != "OK": - _report("\nHealth endpoint probe failed.") - print_section("stdout", read_text(stdout_path)) - print_section("stderr", read_text(stderr_path)) - return 1 + if not _probe_health(port): + return _report_failure("\nHealth endpoint probe failed.", logs) _report("\nBoot smoke test passed.") return 0 finally: - if "process" in locals() and process.poll() is None: - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait(timeout=5) + _stop_app(process) if __name__ == "__main__": diff --git a/scripts/v1_database_migration.py b/scripts/v1_database_migration.py index e51da59a2..0a6bf8bda 100644 --- a/scripts/v1_database_migration.py +++ b/scripts/v1_database_migration.py @@ -25,6 +25,10 @@ MIGRATION_USER = "policyengine_schema_migrator" PROXY_HOST = "127.0.0.1" PROXY_PORT = 3307 +# Alembic's metadata differences: ("remove_table", table) and, for a column, +# (operation, schema, table, column, existing settings, old value, new value). +TABLE_DIFFERENCE_LENGTH = 2 +COLUMN_DIFFERENCE_LENGTH = 7 class DatabaseState(StrEnum): @@ -88,14 +92,14 @@ def describe_metadata_difference(difference: object) -> str: raise ValueError(msg) operation = item[0] - if operation == "remove_table" and len(item) >= 2: + if operation == "remove_table" and len(item) >= TABLE_DIFFERENCE_LENGTH: return f"extra_table:{item[1].name}" - if operation == "modify_nullable" and len(item) >= 7: + if operation == "modify_nullable" and len(item) >= COLUMN_DIFFERENCE_LENGTH: return ( f"nullable:{item[2]}.{item[3]}:" f"{str(item[5]).lower()}->{str(item[6]).lower()}" ) - if operation == "modify_default" and len(item) >= 7: + if operation == "modify_default" and len(item) >= COLUMN_DIFFERENCE_LENGTH: existing = "none" if item[5] is None else "present" target = "none" if item[6] is None else "present" return f"default:{item[2]}.{item[3]}:{existing}->{target}" diff --git a/tests/contract/test_simulation_gateway_contract.py b/tests/contract/test_simulation_gateway_contract.py index c890d4a88..1b4bc817f 100644 --- a/tests/contract/test_simulation_gateway_contract.py +++ b/tests/contract/test_simulation_gateway_contract.py @@ -3,7 +3,7 @@ import httpx import pytest -import policyengine_api.libs.simulation_entrypoint as simulation_entrypoint +from policyengine_api.libs import simulation_entrypoint from policyengine_api.libs.simulation_entrypoint import SimulationAPIModal from tests.fixtures.libs.simulation_entrypoint import ( MOCK_BATCH_JOB_ID, diff --git a/tests/contract/test_v1_route_contracts.py b/tests/contract/test_v1_route_contracts.py index 9326f965c..8bdc272fc 100644 --- a/tests/contract/test_v1_route_contracts.py +++ b/tests/contract/test_v1_route_contracts.py @@ -1,6 +1,8 @@ +import copy import importlib import sys from contextlib import ExitStack +from dataclasses import dataclass, field from types import SimpleNamespace from unittest.mock import patch @@ -69,27 +71,16 @@ def _assert_no_stage12_private_fields(value: object) -> None: _assert_no_stage12_private_fields(child) +@dataclass(kw_only=True) class _BudgetWindowEconomicImpactResult: - def __init__( - self, - *, - data: dict | None, - progress: int | None = None, - completed_years: list[str] | None = None, - computing_years: list[str] | None = None, - queued_years: list[str] | None = None, - message: str | None = None, - error: str | None = None, - cache_status: str | None = None, - ): - self.data = data - self.progress = progress - self.completed_years = completed_years or [] - self.computing_years = computing_years or [] - self.queued_years = queued_years or [] - self.message = message - self.error = error - self.cache_status = cache_status + data: dict | None + progress: int | None = None + completed_years: list[str] = field(default_factory=list) + computing_years: list[str] = field(default_factory=list) + queued_years: list[str] = field(default_factory=list) + message: str | None = None + error: str | None = None + cache_status: str | None = None @classmethod def completed(cls, data: dict): @@ -217,41 +208,58 @@ def _resolved_path(path: str) -> str: ) +# Request bodies by (path, method); a None method stands for any method. +_JSON_PAYLOADS: dict[tuple[str, str | None], dict] = { + ("/us/policy", None): { + "label": "Utah reform", + "data": {"gov.example.parameter": 1}, + }, + ("/us/user-policy", "POST"): { + "reform_id": 22, + "reform_label": "Tax reform", + "baseline_id": 2, + "baseline_label": "Current law", + "user_id": "auth0|one", + "year": "2026", + "geography": "us", + "dataset": "enhanced_cps_2024", + "number_of_provisions": 3, + "api_version": "1", + "added_date": 1, + "updated_date": 2, + }, + ("/us/user-policy", "PUT"): {"id": 10, "reform_label": "Updated tax reform"}, + ("/us/household", None): {"label": "Empty household", "data": {}}, + ("/us/household/{household_id}", None): { + "label": "Updated household", + "data": {"people": {"you": {}}}, + }, + ("/us/calculate", None): {"household": {"people": {"you": {}}}, "policy": {}}, + ("/us/simulation", None): { + "population_id": "household-1", + "population_type": "household", + "policy_id": 22, + }, + ("/us/report", None): { + "simulation_1_id": 11, + "simulation_2_id": None, + "year": "2026", + }, +} + + +def _route_entry( + entries: dict[tuple[str, str | None], dict], contract: ContractRequest +) -> dict | None: + """Return a fresh copy of the contract's entry, by path and method, or None.""" + entry = entries.get( + (contract.path, contract.method), entries.get((contract.path, None)) + ) + return copy.deepcopy(entry) + + def _json_payload(contract: ContractRequest) -> dict | None: - if contract.path == "/us/policy": - return {"label": "Utah reform", "data": {"gov.example.parameter": 1}} - if contract.path == "/us/user-policy" and contract.method == "POST": - return { - "reform_id": 22, - "reform_label": "Tax reform", - "baseline_id": 2, - "baseline_label": "Current law", - "user_id": "auth0|one", - "year": "2026", - "geography": "us", - "dataset": "enhanced_cps_2024", - "number_of_provisions": 3, - "api_version": "1", - "added_date": 1, - "updated_date": 2, - } - if contract.path == "/us/user-policy" and contract.method == "PUT": - return {"id": 10, "reform_label": "Updated tax reform"} - if contract.path == "/us/household": - return {"label": "Empty household", "data": {}} - if contract.path == "/us/household/{household_id}": - return {"label": "Updated household", "data": {"people": {"you": {}}}} - if contract.path == "/us/calculate": - return {"household": {"people": {"you": {}}}, "policy": {}} - if contract.path == "/us/simulation": - return { - "population_id": "household-1", - "population_type": "household", - "policy_id": 22, - } - if contract.path == "/us/report": - return {"simulation_1_id": 11, "simulation_2_id": None, "year": "2026"} - return None + return _route_entry(_JSON_PAYLOADS, contract) def _fake_country(): @@ -482,95 +490,113 @@ def _patched_route_dependencies(): return stack -def _expected_subset(contract: ContractRequest) -> dict: - if contract.path == "/us/policy": - return { - "status": "ok", - "message": "Policy created successfully", - "result": {"policy_id": 123}, - } - if contract.path == "/us/policy/{policy_id}": - return {"status": "ok", "message": None, "result": {"label": "Current law"}} - if contract.path == "/us/policies": - return { - "status": "ok", - "message": "Policies found", - "result": [{"id": 123, "label": "Tax reform"}], - } - if contract.path == "/us/user-policy" and contract.method == "POST": - return { - "status": "ok", - "message": "Record created successfully", - "result": { - "id": 10, - "reform_id": 22, - "reform_label": "Tax reform", - "baseline_id": 2, - "user_id": "auth0|one", - }, - } - if contract.path == "/us/user-policy/{user_id}": - return { - "status": "ok", - "message": None, - "result": [{"id": 10, "reform_label": "Tax reform"}], - } - if contract.path == "/us/user-policy" and contract.method == "PUT": - return { - "status": "ok", - "message": "Record updated successfully", - "result": {"id": 10}, - } - if contract.path == "/us/household": - return {"status": "ok", "message": None, "result": {"household_id": 456}} - if contract.path == "/us/household/{household_id}": - return {"status": "ok", "result": {"id": 456, "label": "Empty household"}} - if contract.path == "/us/calculate": - return { - "status": "ok", - "message": None, - "result": {"people": {"you": {"age": {"2026": 40}}}}, - } - if contract.path in {"/us/metadata", "/uk/metadata"}: - country_id = contract.path.strip("/").split("/")[0] - return { - "status": "ok", - "message": None, - "result": { - "current_law_id": 2 if country_id == "us" else 1, - "economy_options": { - "region": [{"name": country_id}], - "time_period": [{"name": 2026}], - }, +def _expected_metadata_subset(country_id: str) -> dict: + return { + "status": "ok", + "message": None, + "result": { + "current_law_id": 2 if country_id == "us" else 1, + "economy_options": { + "region": [{"name": country_id}], + "time_period": [{"name": 2026}], }, - } - if contract.path == "/us/simulation": - return { - "status": "ok", - "message": "Simulation created successfully", - "result": {"id": 11, "status": "pending"}, - } - if contract.path == "/us/simulation/{simulation_id}": - return {"status": "ok", "message": None, "result": {"id": 11}} - if contract.path == "/us/report": - return { - "status": "ok", - "message": "Report output created successfully", - "result": {"id": 33, "status": "pending"}, - } - if contract.path == "/us/report/{report_id}": - return {"status": "ok", "message": None, "result": {"id": 33}} + }, + } + + +# Expected response subsets by (path, method); a None method stands for any method. +_EXPECTED_SUBSETS: dict[tuple[str, str | None], dict] = { + ("/us/policy", None): { + "status": "ok", + "message": "Policy created successfully", + "result": {"policy_id": 123}, + }, + ("/us/policy/{policy_id}", None): { + "status": "ok", + "message": None, + "result": {"label": "Current law"}, + }, + ("/us/policies", None): { + "status": "ok", + "message": "Policies found", + "result": [{"id": 123, "label": "Tax reform"}], + }, + ("/us/user-policy", "POST"): { + "status": "ok", + "message": "Record created successfully", + "result": { + "id": 10, + "reform_id": 22, + "reform_label": "Tax reform", + "baseline_id": 2, + "user_id": "auth0|one", + }, + }, + ("/us/user-policy/{user_id}", None): { + "status": "ok", + "message": None, + "result": [{"id": 10, "reform_label": "Tax reform"}], + }, + ("/us/user-policy", "PUT"): { + "status": "ok", + "message": "Record updated successfully", + "result": {"id": 10}, + }, + ("/us/household", None): { + "status": "ok", + "message": None, + "result": {"household_id": 456}, + }, + ("/us/household/{household_id}", None): { + "status": "ok", + "result": {"id": 456, "label": "Empty household"}, + }, + ("/us/calculate", None): { + "status": "ok", + "message": None, + "result": {"people": {"you": {"age": {"2026": 40}}}}, + }, + ("/us/metadata", None): _expected_metadata_subset("us"), + ("/uk/metadata", None): _expected_metadata_subset("uk"), + ("/us/simulation", None): { + "status": "ok", + "message": "Simulation created successfully", + "result": {"id": 11, "status": "pending"}, + }, + ("/us/simulation/{simulation_id}", None): { + "status": "ok", + "message": None, + "result": {"id": 11}, + }, + ("/us/report", None): { + "status": "ok", + "message": "Report output created successfully", + "result": {"id": 33, "status": "pending"}, + }, + ("/us/report/{report_id}", None): { + "status": "ok", + "message": None, + "result": {"id": 33}, + }, +} +_EXPECTED_BUDGET_WINDOW_SUBSET: dict[str, object] = { + "status": "ok", + "message": None, + "result": {"kind": "budgetWindow", "windowSize": 1}, + "progress": 100, + "completed_years": [], + "computing_years": [], + "queued_years": [], + "error": None, +} + + +def _expected_subset(contract: ContractRequest) -> dict: + expected = _route_entry(_EXPECTED_SUBSETS, contract) + if expected is not None: + return expected if "budget-window" in contract.path: - return { - "status": "ok", - "message": None, - "result": {"kind": "budgetWindow", "windowSize": 1}, - "progress": 100, - "completed_years": [], - "computing_years": [], - "queued_years": [], - "error": None, - } + return copy.deepcopy(_EXPECTED_BUDGET_WINDOW_SUBSET) msg = f"Missing expected subset for {contract}" raise AssertionError(msg) diff --git a/tests/fixtures/services/economy_service.py b/tests/fixtures/services/economy_service.py index d7aa5b14e..9b83e074c 100644 --- a/tests/fixtures/services/economy_service.py +++ b/tests/fixtures/services/economy_service.py @@ -1,5 +1,6 @@ import datetime import json +from dataclasses import dataclass from unittest.mock import MagicMock, patch from uuid import UUID @@ -217,10 +218,7 @@ def mock_submission_claim_id(): def create_mock_reform_impact( status="ok", reform_impact_json=None, - execution_id=MOCK_MODAL_JOB_ID, options_hash=MOCK_OPTIONS_HASH, - start_time=None, - time_period=MOCK_TIME_PERIOD, message=None, ): """Build a ReformImpact record populated with the shared mock identifiers.""" @@ -243,15 +241,15 @@ def create_mock_reform_impact( baseline_policy_id=MOCK_BASELINE_POLICY_ID, region=MOCK_REGION, dataset=MOCK_DATASET, - time_period=time_period, + time_period=MOCK_TIME_PERIOD, options_json=MOCK_OPTIONS, options_hash=options_hash, status=status, api_version=MOCK_API_VERSION, reform_impact_json=reform_impact_json or default_reform_impact_json, - execution_id=execution_id, + execution_id=MOCK_MODAL_JOB_ID, message=message, - start_time=start_time or datetime.datetime(2025, 6, 26, 12, 0, 0), # noqa: DTZ001 # reform_impact start_time is a naive column + start_time=datetime.datetime(2025, 6, 26, 12, 0, 0), # noqa: DTZ001 # reform_impact start_time is a naive column end_time=(datetime.datetime(2025, 6, 26, 12, 5, 0) if status == "ok" else None), # noqa: DTZ001 # reform_impact end_time is a naive column ) impact.observability_id = MOCK_OBSERVABILITY_ID @@ -264,9 +262,8 @@ def create_mock_modal_execution( result=None, error=None, policyengine_bundle=None, - run_progress=None, ): - """Build a mock Modal simulation execution. + """Build a mock Modal simulation execution that reports no run progress. Parameters ---------- @@ -280,8 +277,6 @@ def create_mock_modal_execution( The error message if failed. policyengine_bundle : dict or None The runtime bundle the execution reports; defaults to MOCK_RUNTIME_BUNDLE. - run_progress : SimulationRunProgress or None - The stage fields the execution reports while it runs. Returns ------- @@ -297,31 +292,39 @@ def create_mock_modal_execution( mock_execution.error = error mock_execution.policyengine_bundle = policyengine_bundle or MOCK_RUNTIME_BUNDLE mock_execution.resolved_app_name = MOCK_RESOLVED_APP_NAME - mock_execution.run_progress = run_progress + mock_execution.run_progress = None return mock_execution +@dataclass(frozen=True) +class MockBatchProgress: + """How far a mock budget-window batch has got, overall and year by year.""" + + progress: int | None = None + completed_years: list[str] | None = None + running_years: list[str] | None = None + queued_years: list[str] | None = None + failed_years: list[str] | None = None + + def create_mock_budget_window_batch_execution( - batch_job_id=MOCK_MODAL_JOB_ID, - status=MODAL_EXECUTION_STATUS_SUBMITTED, - progress=None, - completed_years=None, - running_years=None, - queued_years=None, - failed_years=None, - result=None, - error=None, -): + batch_job_id: str = MOCK_MODAL_JOB_ID, + status: str = MODAL_EXECUTION_STATUS_SUBMITTED, + batch_progress: MockBatchProgress | None = None, + result: object = None, + error: str | None = None, +) -> MagicMock: """Build a mock budget-window batch execution; year lists default to empty.""" + batch_progress = batch_progress or MockBatchProgress() mock_execution = MagicMock() mock_execution.batch_job_id = batch_job_id mock_execution.name = batch_job_id mock_execution.status = status - mock_execution.progress = progress - mock_execution.completed_years = completed_years or [] - mock_execution.running_years = running_years or [] - mock_execution.queued_years = queued_years or [] - mock_execution.failed_years = failed_years or [] + mock_execution.progress = batch_progress.progress + mock_execution.completed_years = batch_progress.completed_years or [] + mock_execution.running_years = batch_progress.running_years or [] + mock_execution.queued_years = batch_progress.queued_years or [] + mock_execution.failed_years = batch_progress.failed_years or [] mock_execution.result = result mock_execution.error = error return mock_execution diff --git a/tests/fixtures/v2_catalog.py b/tests/fixtures/v2_catalog.py index 6e69f789b..45dc22016 100644 --- a/tests/fixtures/v2_catalog.py +++ b/tests/fixtures/v2_catalog.py @@ -2,6 +2,7 @@ from __future__ import annotations +from dataclasses import dataclass from datetime import UTC, datetime from types import SimpleNamespace from typing import TYPE_CHECKING @@ -36,28 +37,32 @@ def _strategy(field: str, value: str | int) -> SimpleNamespace: ) -def _region( - *, - code: str, - label: str, - region_type: str, - parent_code: str | None = None, - dataset_path: str | None = None, - strategy: SimpleNamespace | None = None, - state_code: str | None = None, - state_name: str | None = None, -) -> SimpleNamespace: - return SimpleNamespace( - code=code, - label=label, - region_type=region_type, - parent_code=parent_code, - dataset_path=dataset_path, - scoping_strategy=strategy, - requires_filter=strategy is not None, - state_code=state_code, - state_name=state_name, - ) +@dataclass(frozen=True, kw_only=True) +class _Region: + """One public region; ``source`` gives the mutable object the registry holds.""" + + code: str + label: str + region_type: str + parent_code: str | None = None + dataset_path: str | None = None + strategy: SimpleNamespace | None = None + state_code: str | None = None + state_name: str | None = None + + def source(self) -> SimpleNamespace: + """Return the region as a registry entry; it needs a filter if scoped.""" + return SimpleNamespace( + code=self.code, + label=self.label, + region_type=self.region_type, + parent_code=self.parent_code, + dataset_path=self.dataset_path, + scoping_strategy=self.strategy, + requires_filter=self.strategy is not None, + state_code=self.state_code, + state_name=self.state_name, + ) def bundle(*, policyengine_version: str = POLICYENGINE_VERSION) -> dict: @@ -165,13 +170,13 @@ def source_models( country_package_version=DEPENDENCY_VERSIONS["policyengine-us"], policyengine_version=policyengine_version, regions=[ - _region( + _Region( code="us", label="United States", region_type="national", dataset_path=us_default_uri, - ), - _region( + ).source(), + _Region( code="state/ca", label="California", region_type="state", @@ -180,15 +185,15 @@ def source_models( strategy=_strategy("state_fips", 6), state_code="CA", state_name="California", - ), - _region( + ).source(), + _Region( code="place/CA-44000", label="Los Angeles", region_type="place", parent_code="state/ca", state_code="CA", state_name="California", - ), + ).source(), ], ), "uk": _model_source( @@ -196,19 +201,19 @@ def source_models( country_package_version=DEPENDENCY_VERSIONS["policyengine-uk"], policyengine_version=policyengine_version, regions=[ - _region( + _Region( code="uk", label="United Kingdom", region_type="national", dataset_path=uk_default_uri, - ), - _region( + ).source(), + _Region( code="country/england", label="England", region_type="country", parent_code="uk", strategy=_strategy("country", "ENGLAND"), - ), + ).source(), ], ), } diff --git a/tests/integration/test_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index 9c1a28701..814971236 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -12,7 +12,7 @@ from alembic.autogenerate import compare_metadata from alembic.config import Config from alembic.migration import MigrationContext -from sqlalchemy import create_engine, inspect, text +from sqlalchemy import Engine, create_engine, inspect, text from sqlalchemy.dialects import postgresql from policyengine_api.constants import REPO @@ -193,6 +193,165 @@ def _assert_stage_12a_association_shape(engine) -> None: } <= user_checks +def _upgrade_and_assert_head(config: Config, engine: Engine) -> None: + command.upgrade(config, "head") + command.check(config) + _assert_head(engine) + + +def _assert_revision(engine: Engine, revision: str) -> None: + with engine.connect() as connection: + context = MigrationContext.configure(connection) + assert context.get_current_revision() == revision + + +def _assert_revision_without_enum_types( + engine: Engine, revision: str | None, enum_query: str +) -> None: + """Check the database revision, then that ``enum_query`` counts no enum types.""" + with engine.connect() as connection: + context = MigrationContext.configure(connection) + assert context.get_current_revision() == revision + enum_count = connection.execute(text(enum_query)).scalar_one() + assert enum_count == 0 + + +def _insert_legacy_stage12_report(engine: Engine) -> UUID: + """Store a report from before the comparison columns existed; return its id.""" + legacy_report_id = uuid4() + with engine.begin() as connection: + connection.execute( + text( + "INSERT INTO stage12_evaluation_reports " + "(evaluation_id, contract_version, status, aggregation_status, " + "environment, calculation_flow, originating_request_id, " + "production_identity, worker_version, modal_application, " + "report_coordinator_callable, version_manifest_sha256, " + "policyengine_version, country_package_name, " + "country_package_version, country, dataset_identity, dataset_uri, " + "data_package_name, data_package_version, data_artifact_revision, " + "retention_expires_at) VALUES " + "(:evaluation_id, 1, 'pending', 'not_started', 'test', " + "'economy', 'request', 'production', '5.2.0', 'application', " + "'coordinate_report', :digest, '5.2.0', 'policyengine-us', " + "'1.0.0', 'us', 'dataset', 'hf://dataset/file@revision', " + "'data-package', '1.0.0', 'revision', now() + interval '1 day')" + ), + {"evaluation_id": legacy_report_id, "digest": "a" * 64}, + ) + return legacy_report_id + + +def _assert_comparison_downgrade_then_store_legacy_report(engine: Engine) -> UUID: + """Check the comparison revision is gone, then store a pre-comparison report.""" + _assert_revision_without_enum_types( + engine, + STAGE_12_COMPARISON_PREVIOUS_REVISION, + "SELECT count(*) FROM pg_type " + "WHERE typname = 'v2_stage12_result_comparison_status' " + "AND typtype = 'e'", + ) + report_columns = { + column["name"] + for column in inspect(engine).get_columns( + "stage12_evaluation_reports", + schema="public", + ) + } + assert not any(name.startswith("comparison_") for name in report_columns) + return _insert_legacy_stage12_report(engine) + + +def _assert_legacy_report_is_not_compared( + engine: Engine, legacy_report_id: UUID +) -> None: + with engine.connect() as connection: + comparison_status = connection.execute( + text( + "SELECT comparison_status FROM stage12_evaluation_reports " + "WHERE evaluation_id = :evaluation_id" + ), + {"evaluation_id": legacy_report_id}, + ).scalar_one() + assert comparison_status == "not_requested" + + +def _assert_stage12_downgrade(engine: Engine) -> None: + _assert_revision_without_enum_types( + engine, + STAGE_12_PREVIOUS_REVISION, + "SELECT count(*) FROM pg_type " + "WHERE typname LIKE 'v2_stage12_%' AND typtype = 'e'", + ) + assert { + "stage12_evaluation_reports", + "stage12_evaluation_simulations", + }.isdisjoint(inspect(engine).get_table_names(schema="public")) + + +def _assert_stage11_downgrade(engine: Engine) -> None: + _assert_revision(engine, STAGE_11_PREVIOUS_REVISION) + assert "legacy_household_mappings" not in inspect(engine).get_table_names( + schema="public" + ) + household_columns = { + column["name"]: column + for column in inspect(engine).get_columns( + "households", + schema="public", + ) + } + assert {"country", "label", "year"} <= household_columns.keys() + assert { + "country_id", + "default_year", + "canonicalization_version", + "content_hash", + }.isdisjoint(household_columns) + association_columns = { + column["name"]: column + for column in inspect(engine).get_columns( + "user_household_associations", + schema="public", + ) + } + assert {"country", "label"} <= association_columns.keys() + assert {"country_id", "name", "description"}.isdisjoint(association_columns) + user_policy_id = next( + column + for column in inspect(engine).get_columns( + "user_policies", + schema="public", + ) + if column["name"] == "user_id" + ) + assert isinstance(user_policy_id["type"], postgresql.UUID) + user_columns = { + column["name"]: column + for column in inspect(engine).get_columns("users", schema="public") + } + assert all( + user_columns[field_name]["nullable"] + for field_name in ("first_name", "last_name", "email") + ) + + +def _assert_baseline_downgrade(engine: Engine) -> None: + _assert_revision(engine, BASELINE_REVISION) + assert "uq_parameter_values_canonical_parameter_start_date" not in { + index["name"] for index in inspect(engine).get_indexes("parameter_values") + } + + +def _assert_base(engine: Engine) -> None: + assert set(inspect(engine).get_table_names(schema="public")) <= {"alembic_version"} + _assert_revision_without_enum_types( + engine, + None, + "SELECT count(*) FROM pg_type WHERE typname LIKE 'v2_%' AND typtype = 'e'", + ) + + def test_empty_upgrade_check_base_downgrade_and_reupgrade() -> None: database_url = _disposable_url() config = _config() @@ -204,180 +363,34 @@ def test_empty_upgrade_check_base_downgrade_and_reupgrade() -> None: "alembic_version" } - command.upgrade(config, "head") - command.check(config) - _assert_head(engine) + _upgrade_and_assert_head(config, engine) command.downgrade(config, STAGE_12A_PREVIOUS_REVISION) - with engine.connect() as connection: - context = MigrationContext.configure(connection) - assert context.get_current_revision() == STAGE_12A_PREVIOUS_REVISION + _assert_revision(engine, STAGE_12A_PREVIOUS_REVISION) _assert_pre_stage_12a_association_shape(engine) - - command.upgrade(config, "head") - command.check(config) - _assert_head(engine) + _upgrade_and_assert_head(config, engine) _assert_stage_12a_association_shape(engine) command.downgrade(config, STAGE_12_COMPARISON_PREVIOUS_REVISION) - with engine.connect() as connection: - context = MigrationContext.configure(connection) - assert ( - context.get_current_revision() == STAGE_12_COMPARISON_PREVIOUS_REVISION - ) - comparison_enum_count = connection.execute( - text( - "SELECT count(*) FROM pg_type " - "WHERE typname = 'v2_stage12_result_comparison_status' " - "AND typtype = 'e'" - ) - ).scalar_one() - assert comparison_enum_count == 0 - report_columns = { - column["name"] - for column in inspect(engine).get_columns( - "stage12_evaluation_reports", - schema="public", - ) - } - assert not any(name.startswith("comparison_") for name in report_columns) - legacy_report_id = uuid4() - with engine.begin() as connection: - connection.execute( - text( - "INSERT INTO stage12_evaluation_reports " - "(evaluation_id, contract_version, status, aggregation_status, " - "environment, calculation_flow, originating_request_id, " - "production_identity, worker_version, modal_application, " - "report_coordinator_callable, version_manifest_sha256, " - "policyengine_version, country_package_name, " - "country_package_version, country, dataset_identity, dataset_uri, " - "data_package_name, data_package_version, data_artifact_revision, " - "retention_expires_at) VALUES " - "(:evaluation_id, 1, 'pending', 'not_started', 'test', " - "'economy', 'request', 'production', '5.2.0', 'application', " - "'coordinate_report', :digest, '5.2.0', 'policyengine-us', " - "'1.0.0', 'us', 'dataset', 'hf://dataset/file@revision', " - "'data-package', '1.0.0', 'revision', now() + interval '1 day')" - ), - {"evaluation_id": legacy_report_id, "digest": "a" * 64}, - ) - - command.upgrade(config, "head") - command.check(config) - _assert_head(engine) - with engine.connect() as connection: - comparison_status = connection.execute( - text( - "SELECT comparison_status FROM stage12_evaluation_reports " - "WHERE evaluation_id = :evaluation_id" - ), - {"evaluation_id": legacy_report_id}, - ).scalar_one() - assert comparison_status == "not_requested" + legacy_report_id = _assert_comparison_downgrade_then_store_legacy_report(engine) + _upgrade_and_assert_head(config, engine) + _assert_legacy_report_is_not_compared(engine, legacy_report_id) command.downgrade(config, STAGE_12_PREVIOUS_REVISION) - with engine.connect() as connection: - context = MigrationContext.configure(connection) - assert context.get_current_revision() == STAGE_12_PREVIOUS_REVISION - remaining_stage_12_enum_count = connection.execute( - text( - "SELECT count(*) FROM pg_type " - "WHERE typname LIKE 'v2_stage12_%' AND typtype = 'e'" - ) - ).scalar_one() - assert remaining_stage_12_enum_count == 0 - assert { - "stage12_evaluation_reports", - "stage12_evaluation_simulations", - }.isdisjoint(inspect(engine).get_table_names(schema="public")) - - command.upgrade(config, "head") - command.check(config) - _assert_head(engine) + _assert_stage12_downgrade(engine) + _upgrade_and_assert_head(config, engine) command.downgrade(config, STAGE_11_PREVIOUS_REVISION) - with engine.connect() as connection: - context = MigrationContext.configure(connection) - assert context.get_current_revision() == STAGE_11_PREVIOUS_REVISION - assert "legacy_household_mappings" not in inspect(engine).get_table_names( - schema="public" - ) - household_columns = { - column["name"]: column - for column in inspect(engine).get_columns( - "households", - schema="public", - ) - } - assert {"country", "label", "year"} <= household_columns.keys() - assert { - "country_id", - "default_year", - "canonicalization_version", - "content_hash", - }.isdisjoint(household_columns) - association_columns = { - column["name"]: column - for column in inspect(engine).get_columns( - "user_household_associations", - schema="public", - ) - } - assert {"country", "label"} <= association_columns.keys() - assert {"country_id", "name", "description"}.isdisjoint(association_columns) - user_policy_id = next( - column - for column in inspect(engine).get_columns( - "user_policies", - schema="public", - ) - if column["name"] == "user_id" - ) - assert isinstance(user_policy_id["type"], postgresql.UUID) - user_columns = { - column["name"]: column - for column in inspect(engine).get_columns("users", schema="public") - } - assert all( - user_columns[field_name]["nullable"] - for field_name in ("first_name", "last_name", "email") - ) - - command.upgrade(config, "head") - command.check(config) - _assert_head(engine) + _assert_stage11_downgrade(engine) + _upgrade_and_assert_head(config, engine) command.downgrade(config, BASELINE_REVISION) - with engine.connect() as connection: - context = MigrationContext.configure(connection) - assert context.get_current_revision() == BASELINE_REVISION - assert "uq_parameter_values_canonical_parameter_start_date" not in { - index["name"] for index in inspect(engine).get_indexes("parameter_values") - } - - command.upgrade(config, "head") - command.check(config) - _assert_head(engine) + _assert_baseline_downgrade(engine) + _upgrade_and_assert_head(config, engine) command.downgrade(config, "base") - assert set(inspect(engine).get_table_names(schema="public")) <= { - "alembic_version" - } - with engine.connect() as connection: - context = MigrationContext.configure(connection) - assert context.get_current_revision() is None - remaining_enum_count = connection.execute( - text( - "SELECT count(*) FROM pg_type " - "WHERE typname LIKE 'v2_%' AND typtype = 'e'" - ) - ).scalar_one() - assert remaining_enum_count == 0 - - command.upgrade(config, "head") - command.check(config) - _assert_head(engine) + _assert_base(engine) + _upgrade_and_assert_head(config, engine) finally: command.upgrade(config, "head") engine.dispose() @@ -415,32 +428,39 @@ def _new_required_columns(parent_table: sa.Table, head_table: sa.Table) -> set[s } +def _string_test_value(column_type: sa.String, sequence: Iterator[int]) -> str: + length = column_type.length or 255 + if length == 64: + return uuid4().hex + uuid4().hex + return f"value-{next(sequence)}"[:length] + + +# Checked in order: a column takes the value of the first type it is an +# instance of, so Enum, a String subclass, comes before String. +TYPED_TEST_VALUES = ( + (sa.Enum, lambda column_type, _sequence: column_type.enums[0]), + (sa.Uuid, lambda _column_type, _sequence: uuid4()), + (sa.Boolean, lambda _column_type, _sequence: False), + (sa.Integer, lambda _column_type, _sequence: 1), + (sa.Float, lambda _column_type, _sequence: 0.0), + # Later than every server-default created_at, as retention checks need. + ( + sa.DateTime, + lambda _column_type, _sequence: datetime.now(UTC) + timedelta(days=1), + ), + (sa.JSON, lambda _column_type, _sequence: {}), + (sa.String, _string_test_value), +) + + def _test_value(table: sa.Table, column: sa.Column, sequence: Iterator[int]): if (table.name, column.name) in CHECKED_VALUES: return CHECKED_VALUES[(table.name, column.name)] if column.name in COUNTRY_COLUMN_NAMES: return "us" - column_type = column.type - if isinstance(column_type, sa.Enum): - return column_type.enums[0] - if isinstance(column_type, sa.Uuid): - return uuid4() - if isinstance(column_type, sa.Boolean): - return False - if isinstance(column_type, sa.Integer): - return 1 - if isinstance(column_type, sa.Float): - return 0.0 - if isinstance(column_type, sa.DateTime): - # Later than every server-default created_at, as retention checks need. - return datetime.now(UTC) + timedelta(days=1) - if isinstance(column_type, sa.JSON): - return {} - if isinstance(column_type, sa.String): - length = column_type.length or 255 - if length == 64: - return uuid4().hex + uuid4().hex - return f"value-{next(sequence)}"[:length] + for column_type, test_value in TYPED_TEST_VALUES: + if isinstance(column.type, column_type): + return test_value(column.type, sequence) msg = f"no test value for {table.name}.{column.name}" raise AssertionError(msg) diff --git a/tests/integration/test_live_economy.py b/tests/integration/test_live_economy.py index 6e68d8421..f25abd92d 100644 --- a/tests/integration/test_live_economy.py +++ b/tests/integration/test_live_economy.py @@ -1,5 +1,6 @@ import json import math +from dataclasses import dataclass from datetime import date from pathlib import Path @@ -77,22 +78,31 @@ def test_live_utah_macro_reform(api_client, integration_probe_id, poll_live_endp assert 0 <= lose_less_than_5 <= 1, result +@dataclass(frozen=True) +class MacroReformScenario: + """A saved reform to run through the live economy route for one region.""" + + country_id: str + reform_filename: str + region: str + probe_suffix: str + + def _assert_live_macro_reform( api_client, integration_probe_id, poll_live_endpoint, - *, - country_id: str, - reform_filename: str, - region: str, - probe_suffix: str, + scenario: MacroReformScenario, ) -> None: + country_id = scenario.country_id metadata_response = api_client.get(f"/{country_id}/metadata") metadata_response.raise_for_status() metadata = metadata_response.json()["result"] - reform_payload = _load_reform_payload(reform_filename) - reform_payload["label"] = f"Live economy {integration_probe_id}-{probe_suffix}" + reform_payload = _load_reform_payload(scenario.reform_filename) + reform_payload["label"] = ( + f"Live economy {integration_probe_id}-{scenario.probe_suffix}" + ) policy_response = api_client.post( f"/{country_id}/policy", json=reform_payload, @@ -104,10 +114,10 @@ def _assert_live_macro_reform( api_client, f"/{country_id}/economy/{policy_id}/over/{metadata['current_law_id']}", { - "region": region, + "region": scenario.region, "time_period": _pick_time_period(metadata), }, - route_name=f"{country_id}-{probe_suffix}-economy", + route_name=f"{country_id}-{scenario.probe_suffix}-economy", ) assert payload["status"] == "ok", payload @@ -127,8 +137,10 @@ def test_live_california_eitc_macro_reform( api_client, integration_probe_id, poll_live_endpoint, - country_id="us", - reform_filename="california_eitc_reform.json", - region="state/ca", - probe_suffix="california-eitc", + MacroReformScenario( + country_id="us", + reform_filename="california_eitc_reform.json", + region="state/ca", + probe_suffix="california-eitc", + ), ) diff --git a/tests/integration/test_live_v2_policies.py b/tests/integration/test_live_v2_policies.py index 6d408d649..b68f742d3 100644 --- a/tests/integration/test_live_v2_policies.py +++ b/tests/integration/test_live_v2_policies.py @@ -28,6 +28,7 @@ from __future__ import annotations import os +from typing import NamedTuple from uuid import uuid4 import httpx @@ -266,6 +267,146 @@ def test_live_native_policy_resources_and_v2_policy_save( assert deleted.status_code == 204, deleted.text[:500] +class _SavedResource(NamedTuple): + path: str + legacy_field: str + legacy_id: int + + +def _assert_saved_resource_lifecycle( + api_client, + headers: dict[str, str], + saved: _SavedResource, + *, + name: str, + user_id: str, +) -> None: + """Save, reject an unknown v1 id, read, list, describe and delete one record.""" + params = {"country_id": COUNTRY_ID} + resource, legacy_field, legacy_id = saved + created = api_client.post( + f"/v2/{resource}", + params=params, + headers=headers, + json={"country_id": COUNTRY_ID, legacy_field: legacy_id, "name": name}, + ) + assert created.status_code == 201, (resource, created.text[:500]) + item = created.json()["result"]["item"] + assert item["user_id"] == user_id + assert item[legacy_field] == legacy_id + + missing = api_client.post( + f"/v2/{resource}", + params=params, + headers=headers, + json={"country_id": COUNTRY_ID, legacy_field: UNKNOWN_LEGACY_ID}, + ) + assert missing.status_code == 422, (resource, missing.text[:500]) + + detail_path = f"/v2/{resource}/{item['id']}" + detail = api_client.get(detail_path, params=params, headers=headers) + assert detail.status_code == 200, (resource, detail.text[:500]) + assert detail.json()["result"]["item"] == item + listed = api_client.get( + f"/v2/{resource}", + params={**params, legacy_field: legacy_id}, + headers=headers, + ) + assert listed.status_code == 200, (resource, listed.text[:500]) + assert item["id"] in {row["id"] for row in listed.json()["result"]["items"]} + patched = api_client.patch( + detail_path, + params=params, + headers=headers, + json={"description": "Renamed by the live probe"}, + ) + assert patched.status_code == 200, (resource, patched.text[:500]) + assert patched.json()["result"]["item"]["description"] == ( + "Renamed by the live probe" + ) + deleted = api_client.delete(detail_path, params=params, headers=headers) + assert deleted.status_code == 204, (resource, deleted.text[:500]) + absent = api_client.get(detail_path, params=params, headers=headers) + assert absent.status_code == 404, (resource, absent.text[:500]) + + +def _assert_export_links_the_saved_reform( + api_client, headers: dict[str, str], legacy: dict[str, int], *, user_id: str +) -> None: + """Save a reform and a report that links it, then check the account export.""" + params = {"country_id": COUNTRY_ID} + policy = api_client.post( + "/v2/user-policies", + params=params, + headers=headers, + json={ + "country_id": COUNTRY_ID, + "legacy_policy_id": legacy["policy"], + "baseline": "current-law", + "provenance": {"source": "manual"}, + }, + ) + assert policy.status_code == 201, policy.text[:500] + policy_id = policy.json()["result"]["item"]["id"] + report = api_client.post( + "/v2/user-reports", + params=params, + headers=headers, + json={ + "country_id": COUNTRY_ID, + "legacy_report_id": legacy["report"], + "user_policy_id": policy_id, + }, + ) + assert report.status_code == 201, report.text[:500] + report_id = report.json()["result"]["item"]["id"] + + exported = api_client.get("/v2/me/export", params=params, headers=headers) + assert exported.status_code == 200, exported.text[:500] + export = exported.json()["result"] + assert export["user"]["id"] == user_id + assert export["user"]["user_type"] == "standard" + exported_report = next(row for row in export["reports"] if row["id"] == report_id) + assert exported_report["user_policy_id"] == policy_id + assert [row["id"] for row in export["policies"]] == [policy_id] + + +def _assert_signed_out_saving( + api_client, browser: dict[str, str], legacy_policy_id: int +) -> dict: + """Check a signed-out browser saves a policy without editing its account. + + Return the saved policy. + """ + params = {"country_id": COUNTRY_ID} + # Reads never create an unauthenticated user. + absent = api_client.get("/v2/me", params=params, headers=browser) + assert absent.status_code == 404, absent.text[:500] + assert _policy_ids(api_client, browser) == set() + + saved = _save_policy(api_client, browser, legacy_policy_id) + assert saved["user_id"] == browser[UNAUTHENTICATED_USER_HEADER] + detail = api_client.get( + f"/v2/user-policies/{saved['id']}", + params=params, + headers=browser, + ) + assert detail.status_code == 200, detail.text[:500] + assert detail.json()["result"]["item"] == saved + me = api_client.get("/v2/me", params=params, headers=browser) + assert me.status_code == 200, me.text[:500] + assert me.json()["result"]["item"]["user_type"] == "unauthenticated" + assert me.json()["result"]["item"]["primary_country"] is None + edited = api_client.patch( + "/v2/me", + params=params, + headers=browser, + json={"first_name": "Signed out"}, + ) + assert edited.status_code == 403, edited.text[:500] + return saved + + def test_live_signed_in_user_lifecycle( api_client, integration_probe_id: str, @@ -291,100 +432,27 @@ def test_live_signed_in_user_lifecycle( assert patched_me.json()["result"]["item"]["first_name"] == "Stage 12A" legacy = _create_v1_artifacts(api_client) - resources = { - "user-policies": ("legacy_policy_id", legacy["policy"]), - "user-households": ("legacy_household_id", legacy["household"]), - "user-simulations": ("legacy_simulation_id", legacy["simulation"]), - "user-reports": ("legacy_report_id", legacy["report"]), - } - for resource, (legacy_field, legacy_id) in resources.items(): - created = api_client.post( - f"/v2/{resource}", - params=params, - headers=user_headers, - json={ - "country_id": COUNTRY_ID, - legacy_field: legacy_id, - "name": f"Stage 12A {resource} {integration_probe_id}", - }, + for saved in ( + _SavedResource("user-policies", "legacy_policy_id", legacy["policy"]), + _SavedResource( + "user-households", "legacy_household_id", legacy["household"] + ), + _SavedResource( + "user-simulations", "legacy_simulation_id", legacy["simulation"] + ), + _SavedResource("user-reports", "legacy_report_id", legacy["report"]), + ): + _assert_saved_resource_lifecycle( + api_client, + user_headers, + saved, + name=f"Stage 12A {saved.path} {integration_probe_id}", + user_id=user_id, ) - assert created.status_code == 201, (resource, created.text[:500]) - item = created.json()["result"]["item"] - assert item["user_id"] == user_id - assert item[legacy_field] == legacy_id - - missing = api_client.post( - f"/v2/{resource}", - params=params, - headers=user_headers, - json={"country_id": COUNTRY_ID, legacy_field: UNKNOWN_LEGACY_ID}, - ) - assert missing.status_code == 422, (resource, missing.text[:500]) - - detail_path = f"/v2/{resource}/{item['id']}" - detail = api_client.get(detail_path, params=params, headers=user_headers) - assert detail.status_code == 200, (resource, detail.text[:500]) - assert detail.json()["result"]["item"] == item - listed = api_client.get( - f"/v2/{resource}", - params={**params, legacy_field: legacy_id}, - headers=user_headers, - ) - assert listed.status_code == 200, (resource, listed.text[:500]) - assert item["id"] in {row["id"] for row in listed.json()["result"]["items"]} - patched = api_client.patch( - detail_path, - params=params, - headers=user_headers, - json={"description": "Renamed by the live probe"}, - ) - assert patched.status_code == 200, (resource, patched.text[:500]) - assert patched.json()["result"]["item"]["description"] == ( - "Renamed by the live probe" - ) - deleted = api_client.delete( - detail_path, params=params, headers=user_headers - ) - assert deleted.status_code == 204, (resource, deleted.text[:500]) - absent = api_client.get(detail_path, params=params, headers=user_headers) - assert absent.status_code == 404, (resource, absent.text[:500]) - policy = api_client.post( - "/v2/user-policies", - params=params, - headers=user_headers, - json={ - "country_id": COUNTRY_ID, - "legacy_policy_id": legacy["policy"], - "baseline": "current-law", - "provenance": {"source": "manual"}, - }, - ) - assert policy.status_code == 201, policy.text[:500] - policy_id = policy.json()["result"]["item"]["id"] - report = api_client.post( - "/v2/user-reports", - params=params, - headers=user_headers, - json={ - "country_id": COUNTRY_ID, - "legacy_report_id": legacy["report"], - "user_policy_id": policy_id, - }, + _assert_export_links_the_saved_reform( + api_client, user_headers, legacy, user_id=user_id ) - assert report.status_code == 201, report.text[:500] - report_id = report.json()["result"]["item"]["id"] - - exported = api_client.get("/v2/me/export", params=params, headers=user_headers) - assert exported.status_code == 200, exported.text[:500] - export = exported.json()["result"] - assert export["user"]["id"] == user_id - assert export["user"]["user_type"] == "standard" - exported_report = next( - row for row in export["reports"] if row["id"] == report_id - ) - assert exported_report["user_policy_id"] == policy_id - assert [row["id"] for row in export["policies"]] == [policy_id] _delete_account(api_client, user_headers) reprovisioned = api_client.get("/v2/me", params=params, headers=user_headers) @@ -416,31 +484,9 @@ def test_live_unauthenticated_users_round_trip_and_upgrade( # The upgrade must convert, so no account may exist for the test user yet. _delete_account(api_client, user_headers) try: - # Reads never create an unauthenticated user. - absent = api_client.get("/v2/me", params=params, headers=converted_browser) - assert absent.status_code == 404, absent.text[:500] - assert _policy_ids(api_client, converted_browser) == set() - - saved = _save_policy(api_client, converted_browser, legacy_policy_id) - assert saved["user_id"] == converted_browser[UNAUTHENTICATED_USER_HEADER] - detail = api_client.get( - f"/v2/user-policies/{saved['id']}", - params=params, - headers=converted_browser, - ) - assert detail.status_code == 200, detail.text[:500] - assert detail.json()["result"]["item"] == saved - me = api_client.get("/v2/me", params=params, headers=converted_browser) - assert me.status_code == 200, me.text[:500] - assert me.json()["result"]["item"]["user_type"] == "unauthenticated" - assert me.json()["result"]["item"]["primary_country"] is None - edited = api_client.patch( - "/v2/me", - params=params, - headers=converted_browser, - json={"first_name": "Signed out"}, + saved = _assert_signed_out_saving( + api_client, converted_browser, legacy_policy_id ) - assert edited.status_code == 403, edited.text[:500] # A signed-out browser's work is invisible to the signed-in user... other_work = _save_policy(api_client, signed_out_only_browser, legacy_policy_id) diff --git a/tests/unit/data/test_sqlalchemy_v2.py b/tests/unit/data/test_sqlalchemy_v2.py index 095ace664..8f3f41ab0 100644 --- a/tests/unit/data/test_sqlalchemy_v2.py +++ b/tests/unit/data/test_sqlalchemy_v2.py @@ -3,7 +3,7 @@ import sqlalchemy -import policyengine_api.data.orm as orm +from policyengine_api.data import orm def test_sqlalchemy_v2_or_newer_is_installed(): diff --git a/tests/unit/data/test_v1_database_migration.py b/tests/unit/data/test_v1_database_migration.py index 232bc15b6..75cc9e514 100644 --- a/tests/unit/data/test_v1_database_migration.py +++ b/tests/unit/data/test_v1_database_migration.py @@ -231,3 +231,46 @@ def execute(self, *args, **kwargs): with pytest.raises(RuntimeError, match="no valid Alembic revision"): upgrade_database(FakeConnection(), backup_id="verified-backup") + + +@pytest.mark.parametrize( + ("difference", "message"), + [ + (("remove_table",), "Unsupported metadata difference operation: remove_table"), + ( + ("modify_nullable", None, "t", "c", {}, True), + "Unsupported metadata difference operation: modify_nullable", + ), + ( + ("modify_default", None, "t", "c", {}, None), + "Unsupported metadata difference operation: modify_default", + ), + ((), "Unsupported metadata difference shape"), + ([], "Unsupported metadata difference shape"), + ( + [("remove_table",), ("remove_table",)], + "Unsupported metadata difference shape", + ), + ("remove_table", "Unsupported metadata difference shape"), + ], +) +def test_metadata_difference_rejects_short_or_unknown_shapes( + difference: object, message: str +) -> None: + with pytest.raises(ValueError, match=f"^{message}$"): + describe_metadata_difference(difference) + + +def test_metadata_difference_accepts_longer_tuples_and_unwraps_one_item_lists() -> None: + assert ( + describe_metadata_difference( + [("modify_default", None, "t", "c", {}, None, "now()", "extra")] + ) + == "default:t.c:none->present" + ) + assert ( + describe_metadata_difference( + ("modify_nullable", "schema", "t", "c", {}, False, True) + ) + == "nullable:t.c:false->true" + ) diff --git a/tests/unit/routes/test_migration_context_logging.py b/tests/unit/routes/test_migration_context_logging.py index e2c1835b0..f2b7076d4 100644 --- a/tests/unit/routes/test_migration_context_logging.py +++ b/tests/unit/routes/test_migration_context_logging.py @@ -1,4 +1,5 @@ from types import SimpleNamespace +from typing import NamedTuple from unittest.mock import Mock, patch import pytest @@ -499,43 +500,61 @@ def test_v2_household_resources_log_actual_supabase_read_and_write_sources( } +class _UserResourceRequest(NamedTuple): + method: str + path: str + route_group: str + db_read: str | None + db_write: str | None + + +_USER_RESOURCE_REQUESTS = ( + _UserResourceRequest("GET", "/v2/me", "user_profile", "supabase", None), + _UserResourceRequest("PATCH", "/v2/me", "user_profile", None, "supabase"), + _UserResourceRequest("DELETE", "/v2/me", "user_profile", None, "supabase"), + _UserResourceRequest("GET", "/v2/me/export", "user_profile", "supabase", None), + _UserResourceRequest("POST", "/v2/me/upgrade", "user_profile", None, "supabase"), + _UserResourceRequest("GET", "/v2/user-simulations", "simulation", "supabase", None), + _UserResourceRequest("POST", "/v2/user-households", "household", None, "supabase"), + _UserResourceRequest( + "POST", "/v2/user-simulations", "simulation", None, "supabase" + ), + _UserResourceRequest( + "DELETE", + "/v2/user-simulations/00000000-0000-0000-0000-000000000001", + "simulation", + None, + "supabase", + ), + _UserResourceRequest( + "GET", + "/v2/user-reports/00000000-0000-0000-0000-000000000001", + "report", + "supabase", + None, + ), + _UserResourceRequest( + "PATCH", + "/v2/user-reports/00000000-0000-0000-0000-000000000001", + "report", + None, + "supabase", + ), +) + + @pytest.mark.parametrize( - ("method", "path", "route_group", "db_read", "db_write"), + "request_case", [ - ("GET", "/v2/me", "user_profile", "supabase", None), - ("PATCH", "/v2/me", "user_profile", None, "supabase"), - ("DELETE", "/v2/me", "user_profile", None, "supabase"), - ("GET", "/v2/me/export", "user_profile", "supabase", None), - ("POST", "/v2/me/upgrade", "user_profile", None, "supabase"), - ("GET", "/v2/user-simulations", "simulation", "supabase", None), - ("POST", "/v2/user-households", "household", None, "supabase"), - ("POST", "/v2/user-simulations", "simulation", None, "supabase"), - ( - "DELETE", - "/v2/user-simulations/00000000-0000-0000-0000-000000000001", - "simulation", - None, - "supabase", - ), - ( - "GET", - "/v2/user-reports/00000000-0000-0000-0000-000000000001", - "report", - "supabase", - None, - ), - ( - "PATCH", - "/v2/user-reports/00000000-0000-0000-0000-000000000001", - "report", - None, - "supabase", - ), + # The IDs pytest generated for the five separate parameters. + pytest.param(case, id="-".join(str(value) for value in case)) + for case in _USER_RESOURCE_REQUESTS ], ) def test_signed_in_user_resources_log_supabase_sources( - monkeypatch, method, path, route_group, db_read, db_write + monkeypatch, request_case: _UserResourceRequest ): + method, path, route_group, db_read, db_write = request_case for name in ( "DB_READ_SIMULATION", "DB_WRITE_SIMULATION", diff --git a/tests/unit/routes/test_report_household_identity.py b/tests/unit/routes/test_report_household_identity.py index c1a63c5c2..8e78d1a93 100644 --- a/tests/unit/routes/test_report_household_identity.py +++ b/tests/unit/routes/test_report_household_identity.py @@ -1,10 +1,13 @@ """Report linkage preserves historical US household identity and saved spelling.""" import json +from typing import Any import pytest from flask import Flask +from flask.testing import FlaskClient from sqlalchemy import select +from sqlalchemy.orm import Session, sessionmaker from policyengine_api.data.v1_models import ( Household, @@ -17,11 +20,27 @@ from policyengine_api.services.report_spec_service import ReportSpecService from policyengine_api.services.simulation_service import SimulationService - -def test_historical_baseline_and_new_reform_persist_complete_comparison_report( - monkeypatch, orm_session_factory -): - historical_output = {"household": {"people": {"you": {"age": {"2026": 40}}}}} +EXPECTED_COMPARISON_SPEC = { + "country_id": "us", + "report_kind": "household_comparison", + "time_period": "2026", + "simulation_1": { + "population_type": "household", + "population_id": "00001", + "policy_id": 1, + }, + "simulation_2": { + "population_type": "household", + "population_id": "1", + "policy_id": 2, + }, +} + + +def _seed_historical_baseline( + orm_session_factory: sessionmaker[Session], historical_output: dict[str, Any] +) -> int: + """Store a historical household and its completed baseline; return its id.""" with orm_session_factory.begin() as session: session.add( Household( @@ -43,8 +62,14 @@ def test_historical_baseline_and_new_reform_persist_complete_comparison_report( ) session.add(baseline) session.flush() - baseline_id = baseline.id + baseline_id: int = baseline.id + return baseline_id + +def _report_client( + monkeypatch: pytest.MonkeyPatch, orm_session_factory: sessionmaker[Session] +) -> FlaskClient: + """Serve the simulation and report routes from real services.""" monkeypatch.setattr( simulation_routes, "simulation_service", SimulationService(orm_session_factory) ) @@ -57,87 +82,112 @@ def test_historical_baseline_and_new_reform_persist_complete_comparison_report( app.config["TESTING"] = True app.register_blueprint(simulation_routes.simulation_bp) app.register_blueprint(report_output_routes.report_output_bp) - client = app.test_client() + return app.test_client() + +def _replay_baseline_and_create_reform( + client: FlaskClient, baseline_id: int, historical_output: dict[str, Any] +) -> int: + """Replay the saved baseline, create a new reform, and return the reform id.""" replay = client.post( "/us/simulation", json={"population_id": "00001", "population_type": "household", "policy_id": 1}, ) - assert replay.status_code == 200, replay.json - assert replay.json["result"]["id"] == baseline_id - assert replay.json["result"]["population_id"] == "00001" - assert replay.json["result"]["status"] == "complete" - assert json.loads(replay.json["result"]["output"]) == historical_output + replay_body: Any = replay.json + assert replay.status_code == 200, replay_body + assert replay_body["result"]["id"] == baseline_id + assert replay_body["result"]["population_id"] == "00001" + assert replay_body["result"]["status"] == "complete" + assert json.loads(replay_body["result"]["output"]) == historical_output reform = client.post( "/us/simulation", json={"population_id": "00001", "population_type": "household", "policy_id": 2}, ) - assert reform.status_code == 201, reform.json - reform_id = reform.json["result"]["id"] + reform_body: Any = reform.json + assert reform.status_code == 201, reform_body + reform_id: int = reform_body["result"]["id"] assert reform_id != baseline_id - assert reform.json["result"]["population_id"] == "1" - assert reform.json["result"]["status"] == "pending" + assert reform_body["result"]["population_id"] == "1" + assert reform_body["result"]["status"] == "pending" + return reform_id + +def _create_replay_and_read_report( + client: FlaskClient, baseline_id: int, reform_id: int +) -> int: + """Create the comparison report, replay and read it back; return its id.""" payload = { "simulation_1_id": baseline_id, "simulation_2_id": reform_id, "year": "2026", } - expected_spec = { - "country_id": "us", - "report_kind": "household_comparison", - "time_period": "2026", - "simulation_1": { - "population_type": "household", - "population_id": "00001", - "policy_id": 1, - }, - "simulation_2": { - "population_type": "household", - "population_id": "1", - "policy_id": 2, - }, - } created = client.post("/us/report", json=payload) - assert created.status_code == 201, created.json - report_id = created.json["result"]["id"] + created_body: Any = created.json + assert created.status_code == 201, created_body + report_id: int = created_body["result"]["id"] replayed = client.post("/us/report", json=payload) retrieved = client.get(f"/us/report/{report_id}") assert replayed.status_code == retrieved.status_code == 200 for response in (created, replayed, retrieved): - result = response.json["result"] + response_body: Any = response.json + result = response_body["result"] assert result["id"] == report_id assert result["report_kind"] == "household_comparison" - assert result["report_spec_json"] == expected_spec + assert result["report_spec_json"] == EXPECTED_COMPARISON_SPEC assert result["report_spec_schema_version"] == 1 assert result["report_spec_status"] == "explicit" assert result["status"] == "pending" + return report_id + + +def _assert_saved_simulations( + session: Session, + baseline_id: int, + reform_id: int, + historical_output: dict[str, Any], +) -> None: + saved_baseline: Any = session.get(Simulation, baseline_id) + saved_reform: Any = session.get(Simulation, reform_id) + assert saved_baseline.population_id == "00001" + assert saved_baseline.output == historical_output + assert saved_baseline.simulation_spec_json["population_id"] == "00001" + assert saved_reform.population_id == "1" + assert saved_reform.simulation_spec_json["population_id"] == "1" + + +def _assert_saved_report(session: Session, report_id: int) -> None: + saved_report: Any = session.get(ReportOutput, report_id) + assert saved_report.report_kind == "household_comparison" + assert saved_report.report_spec_json == EXPECTED_COMPARISON_SPEC + assert saved_report.report_spec_schema_version == 1 + assert saved_report.report_spec_status == "explicit" + runs = list( + session.scalars( + select(ReportOutputRun).where(ReportOutputRun.report_output_id == report_id) + ) + ) + assert len(runs) == 1 + assert saved_report.active_run_id == runs[0].id + assert runs[0].report_spec_snapshot_json == EXPECTED_COMPARISON_SPEC + assert runs[0].status == "pending" + + +def test_historical_baseline_and_new_reform_persist_complete_comparison_report( + monkeypatch, orm_session_factory +): + historical_output = {"household": {"people": {"you": {"age": {"2026": 40}}}}} + baseline_id = _seed_historical_baseline(orm_session_factory, historical_output) + client = _report_client(monkeypatch, orm_session_factory) + + reform_id = _replay_baseline_and_create_reform( + client, baseline_id, historical_output + ) + report_id = _create_replay_and_read_report(client, baseline_id, reform_id) with orm_session_factory() as session: - saved_baseline = session.get(Simulation, baseline_id) - saved_reform = session.get(Simulation, reform_id) - assert saved_baseline.population_id == "00001" - assert saved_baseline.output == historical_output - assert saved_baseline.simulation_spec_json["population_id"] == "00001" - assert saved_reform.population_id == "1" - assert saved_reform.simulation_spec_json["population_id"] == "1" - saved_report = session.get(ReportOutput, report_id) - assert saved_report.report_kind == "household_comparison" - assert saved_report.report_spec_json == expected_spec - assert saved_report.report_spec_schema_version == 1 - assert saved_report.report_spec_status == "explicit" - runs = list( - session.scalars( - select(ReportOutputRun).where( - ReportOutputRun.report_output_id == report_id - ) - ) - ) - assert len(runs) == 1 - assert saved_report.active_run_id == runs[0].id - assert runs[0].report_spec_snapshot_json == expected_spec - assert runs[0].status == "pending" + _assert_saved_simulations(session, baseline_id, reform_id, historical_output) + _assert_saved_report(session, report_id) def comparison_models(country_id, population_type, first_id, second_id): diff --git a/tests/unit/routes/test_spm_year_worker_polling.py b/tests/unit/routes/test_spm_year_worker_polling.py index 39d037fee..17f3bd203 100644 --- a/tests/unit/routes/test_spm_year_worker_polling.py +++ b/tests/unit/routes/test_spm_year_worker_polling.py @@ -1,5 +1,8 @@ """Typed worker failures become durable terminal HTTP polling responses.""" +from dataclasses import dataclass +from typing import Any + import httpx import pytest from flask import Flask @@ -64,81 +67,133 @@ def segmented_result(year, invalid_side=None): return result -@pytest.mark.parametrize( - "failure", [400, 422, "segmented-baseline", "segmented-reform"] -) -@pytest.mark.parametrize("budget_window", [False, True], ids=["annual", "window"]) -def test_typed_worker_error_is_terminal_and_replays_after_service_recreation( - monkeypatch, failure, budget_window -): - # Certification is synthetic; the HTTP consumer, routes and both caches are - # real. No worker execution or dataset loading is needed for this boundary. - monkeypatch.setattr(worker_spm, "normalize_spm_selection", lambda *_: SELECTION) - monkeypatch.setattr(economy_module, "COUNTRY_PACKAGE_VERSIONS", {"us": "1.0.0"}) - monkeypatch.setattr(economy_module, "POLICYENGINE_VERSION", "5.3.1") - job_id = "unsupported-year-job" - job_path = f"/budget-window-jobs/{job_id}" if budget_window else f"/jobs/{job_id}" - received = [] - worker_app = "test-worker" - typed_error = YEAR_ERROR - if isinstance(failure, str): - typed_error = { - "code": "SPM_CONFIGURATION_UNAVAILABLE", - "message": "baseline and reform SPM receipts differ", - } - side = failure.removeprefix("segmented-") - worker_result = ( - { - "kind": "budgetWindow", - "windowSize": 2, - "annualImpacts": [ - segmented_result("2035"), - segmented_result("2036", side), - ], - } - if budget_window - else segmented_result("2036", side) +def _versions_response(path: str, worker_app: str) -> httpx.Response | None: + """Answer the worker's version discovery routes, or return None.""" + if path == "/versions": + return httpx.Response( + 200, + json={ + "policyengine": {"5.3.1": worker_app}, + "spm_capabilities": { + "5.3.1": { + "contract_version": "canonical-spm-v1", + "defaults": SELECTION, + } + }, + }, ) + if path == "/versions/policyengine": + return httpx.Response(200, json={"5.3.1": worker_app}) + return None + + +class _FailingJobWorker: + """A worker HTTP API whose job fails once with a typed error, then expires.""" + + def __init__( + self, + *, + job_path: str, + failure: int | str, + typed_error: dict[str, str], + worker_result: dict[str, Any] | None, + ) -> None: + self.job_path = job_path + self.failure = failure + self.typed_error = typed_error + self.worker_result = worker_result + self.app_name = "test-worker" + self.received: list[tuple[str, str]] = [] - def transport(request): - received.append((request.method, request.url.path)) - if request.method == "POST" and worker_app == "replacement-worker": + def __call__(self, request: httpx.Request) -> httpx.Response: + self.received.append((request.method, request.url.path)) + if request.method == "POST" and self.app_name == "replacement-worker": assert request.url.path == "/simulate/economy/comparison" return httpx.Response( 200, json={"job_id": "replacement-job", "status": "submitted"} ) assert request.method == "GET", "Polling must not submit a replacement job" - if request.url.path == "/versions": - return httpx.Response( - 200, - json={ - "policyengine": {"5.3.1": worker_app}, - "spm_capabilities": { - "5.3.1": { - "contract_version": "canonical-spm-v1", - "defaults": SELECTION, - } - }, - }, - ) - if request.url.path == "/versions/policyengine": - return httpx.Response(200, json={"5.3.1": worker_app}) - assert request.url.path == job_path - if received.count(("GET", job_path)) > 1: + versions = _versions_response(request.url.path, self.app_name) + if versions is not None: + return versions + assert request.url.path == self.job_path + if self.received.count(("GET", self.job_path)) > 1: return httpx.Response(404, json={"detail": "Job expired"}) - if isinstance(failure, str): + if isinstance(self.failure, str): return httpx.Response( - 200, json={"status": "complete", "result": worker_result} + 200, json={"status": "complete", "result": self.worker_result} ) return httpx.Response( - failure, - json={"status": "error", "errors": [typed_error]}, + self.failure, + json={"status": "error", "errors": [self.typed_error]}, ) + +def _versions_only_worker(request: httpx.Request) -> httpx.Response: + """Answer as a worker HTTP API that serves only version discovery.""" + assert request.method == "GET", "A stored result must not submit a job" + versions = _versions_response(request.url.path, "test-worker") + if versions is not None: + return versions + msg = f"Unexpected worker call: {request.url.path}" + raise AssertionError(msg) + + +def _typed_failure( + failure: int | str, *, budget_window: bool +) -> tuple[dict[str, str], dict[str, Any] | None]: + """Return the typed error the routes must report, and the worker's result.""" + if not isinstance(failure, str): + return YEAR_ERROR, None + side = failure.removeprefix("segmented-") + worker_result = ( + { + "kind": "budgetWindow", + "windowSize": 2, + "annualImpacts": [ + segmented_result("2035"), + segmented_result("2036", side), + ], + } + if budget_window + else segmented_result("2036", side) + ) + typed_error = { + "code": "SPM_CONFIGURATION_UNAVAILABLE", + "message": "baseline and reform SPM receipts differ", + } + return typed_error, worker_result + + +def _patch_spm_release(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(worker_spm, "normalize_spm_selection", lambda *_: SELECTION) + monkeypatch.setattr(economy_module, "COUNTRY_PACKAGE_VERSIONS", {"us": "1.0.0"}) + monkeypatch.setattr(economy_module, "POLICYENGINE_VERSION", "5.3.1") + + +def _worker_gateway() -> SimulationEntrypointClient: gateway = object.__new__(SimulationEntrypointClient) gateway.base_url = "https://worker.invalid" - backend = InMemoryCacheBackend() - namespace = CacheNamespace("test", "typed-year-worker-polling") + return gateway + + +@dataclass +class _EconomyStack: + """The caches and service one process builds over the shared storage.""" + + annual_cache: ReformImpactCache + annual_impacts: ReformImpactsService + window_cache: BudgetWindowCache + service: EconomyService + + +def _install_economy_stack( + monkeypatch: pytest.MonkeyPatch, + gateway: SimulationEntrypointClient, + backend: Any, + namespace: CacheNamespace, +) -> _EconomyStack: + """Build fresh cache facades and a service over ``backend`` and route to it.""" annual_cache = ReformImpactCache(backend, namespace) annual_impacts = ReformImpactsService(annual_cache) window_cache = BudgetWindowCache(backend, namespace) @@ -148,50 +203,137 @@ def transport(request): simulation_entrypoint_=gateway, ) monkeypatch.setattr(economy_routes, "economy_service", service) + return _EconomyStack(annual_cache, annual_impacts, window_cache, service) + + +def _economy_app() -> Flask: app = Flask(__name__) app.config["TESTING"] = True app.register_blueprint(economy_routes.economy_bp) + return app - with httpx.Client(transport=httpx.MockTransport(transport)) as gateway.client: - setup = service._build_economic_impact_setup_options( - EconomicImpactRequest( - country_id="us", - policy_id=1, - baseline_policy_id=2, - region="us", - dataset="default", - options={}, - api_version="1.0.0", - ), - time_period="budget_window:2035:2" if budget_window else "2036", - ) + +def _setup_options(service: EconomyService, *, budget_window: bool) -> Any: + return service._build_economic_impact_setup_options( + EconomicImpactRequest( + country_id="us", + policy_id=1, + baseline_policy_id=2, + region="us", + dataset="default", + options={}, + api_version="1.0.0", + ), + time_period="budget_window:2035:2" if budget_window else "2036", + ) + + +def _store_annual_impact( + stack: Any, + setup: Any, + *, + status: str, + reform_impact_json: dict[str, Any], + execution_id: str, +) -> None: + stack.service._resolve_runtime_bundle_for_setup_options(setup) + stack.annual_impacts.set_reform_impact( + stack.service._reform_impact_identity(setup), + ReformImpactExecution(execution_id=execution_id, start_time=None), + options=setup.options, + status=status, + reform_impact_json=reform_impact_json, + ) + + +def _economy_request(*, budget_window: bool) -> tuple[str, dict[str, str]]: + """Return the economy route and query for an annual or budget-window run.""" + if budget_window: + return "/us/economy/1/over/2/budget-window", { + "start_year": "2035", + "window_size": "2", + } + return "/us/economy/1/over/2", {"time_period": "2036"} + + +def _get_economy(app: Flask, url: str, query: dict[str, str]) -> Any: + return app.test_client().get( + url, + query_string={"region": "us", "version": "1.0.0", **query}, + ) + + +def _assert_window_failure_recorded( + state: Any, typed_error: dict[str, str], setup: Any +) -> None: + assert state is not None + assert state.status == "failed" + assert state.failure_type == "spm_validation" + assert state.error == typed_error + assert state.observability_id == setup.observability_id + + +def _assert_annual_failure_recorded( + stored: Any, typed_error: dict[str, str], setup: Any +) -> None: + assert stored.status == "error" + assert stored.reform_impact_json == {} + assert stored.error_code == typed_error["code"] + assert stored.message == typed_error["message"] + assert stored.execution_id is None + assert stored.end_time is not None + assert stored.options_hash == setup.options_hash + assert stored.options_json == setup.options + + +@pytest.mark.parametrize( + "failure", [400, 422, "segmented-baseline", "segmented-reform"] +) +@pytest.mark.parametrize("budget_window", [False, True], ids=["annual", "window"]) +def test_typed_worker_error_is_terminal_and_replays_after_service_recreation( + monkeypatch, failure, budget_window +): + # Certification is synthetic; the HTTP consumer, routes and both caches are + # real. No worker execution or dataset loading is needed for this boundary. + _patch_spm_release(monkeypatch) + job_id = "unsupported-year-job" + job_path = f"/budget-window-jobs/{job_id}" if budget_window else f"/jobs/{job_id}" + typed_error, worker_result = _typed_failure(failure, budget_window=budget_window) + worker = _FailingJobWorker( + job_path=job_path, + failure=failure, + typed_error=typed_error, + worker_result=worker_result, + ) + + gateway = _worker_gateway() + backend = InMemoryCacheBackend() + namespace = CacheNamespace("test", "typed-year-worker-polling") + stack = _install_economy_stack(monkeypatch, gateway, backend, namespace) + app = _economy_app() + + with httpx.Client(transport=httpx.MockTransport(worker)) as gateway.client: + setup = _setup_options(stack.service, budget_window=budget_window) if budget_window: - cache_key = service._build_budget_window_cache_key(setup) - window_cache.store_submitted( + cache_key = stack.service._build_budget_window_cache_key(setup) + stack.window_cache.store_submitted( cache_key, job_id, setup.observability_id, ) - url = "/us/economy/1/over/2/budget-window" - query = {"start_year": "2035", "window_size": "2"} else: - service._resolve_runtime_bundle_for_setup_options(setup) - annual_impacts.set_reform_impact( - service._reform_impact_identity(setup), - ReformImpactExecution(execution_id=job_id, start_time=None), - options=setup.options, + _store_annual_impact( + stack, + setup, status="computing", reform_impact_json={}, + execution_id=job_id, ) - url = "/us/economy/1/over/2" - query = {"time_period": "2036"} + url, query = _economy_request(budget_window=budget_window) - received.clear() + worker.received.clear() for _ in range(2): - response = app.test_client().get( - url, - query_string={"region": "us", "version": "1.0.0", **query}, - ) + response = _get_economy(app, url, query) assert response.status_code == 400, response.json assert response.json == { "status": "error", @@ -200,53 +342,33 @@ def transport(request): "errors": [typed_error], } if budget_window: - state = window_cache.get_state(cache_key) - assert state is not None - assert state.status == "failed" - assert state.failure_type == "spm_validation" - assert state.error == typed_error - assert state.observability_id == setup.observability_id + _assert_window_failure_recorded( + stack.window_cache.get_state(cache_key), typed_error, setup + ) else: - stored = annual_cache.get_by_execution_id(job_id) - assert stored.status == "error" - assert stored.reform_impact_json == {} - assert stored.error_code == typed_error["code"] - assert stored.message == typed_error["message"] - assert stored.execution_id is None - assert stored.end_time is not None - assert stored.options_hash == setup.options_hash - assert stored.options_json == setup.options + _assert_annual_failure_recorded( + stack.annual_cache.get_by_execution_id(job_id), typed_error, setup + ) # The worker job vanishes after its first failure. Recreate both # services and cache facades to prove replay uses shared storage. - annual_cache = ReformImpactCache(backend, namespace) - annual_impacts = ReformImpactsService(annual_cache) - window_cache = BudgetWindowCache(backend, namespace) - service = EconomyService( - reform_impacts_service_=annual_impacts, - budget_window_cache_=window_cache, - simulation_entrypoint_=gateway, - ) - monkeypatch.setattr(economy_routes, "economy_service", service) + stack = _install_economy_stack(monkeypatch, gateway, backend, namespace) if not budget_window: # Preserve the existing retry policy: only a newly resolved worker # creates a fresh canonical cache identity and permits submission. - worker_app = "replacement-worker" - monkeypatch.setattr(service, "_get_policy_jsons", lambda *_: ({}, {})) - response = app.test_client().get( - url, - query_string={"region": "us", "version": "1.0.0", **query}, - ) + worker.app_name = "replacement-worker" + monkeypatch.setattr(stack.service, "_get_policy_jsons", lambda *_: ({}, {})) + response = _get_economy(app, url, query) assert response.status_code == 200, response.json assert response.json["status"] == "computing" - replacement = annual_cache.get_by_execution_id("replacement-job") + replacement = stack.annual_cache.get_by_execution_id("replacement-job") assert replacement.options_hash != setup.options_hash assert replacement.options_json == setup.options - assert annual_cache.get_by_execution_id(job_id).status == "error" - assert received.count(("POST", "/simulate/economy/comparison")) == 1 + assert stack.annual_cache.get_by_execution_id(job_id).status == "error" + assert worker.received.count(("POST", "/simulate/economy/comparison")) == 1 - assert received.count(("GET", job_path)) == 1 + assert worker.received.count(("GET", job_path)) == 1 @pytest.mark.parametrize("budget_window", [False, True], ids=["annual", "window"]) @@ -260,68 +382,23 @@ def test_an_uncertifiable_stored_result_becomes_terminal_instead_of_repeating( re-validated on every poll: a 400 the caller can never move past and a worker handle that is never released. """ - monkeypatch.setattr(worker_spm, "normalize_spm_selection", lambda *_: SELECTION) - monkeypatch.setattr(economy_module, "COUNTRY_PACKAGE_VERSIONS", {"us": "1.0.0"}) - monkeypatch.setattr(economy_module, "POLICYENGINE_VERSION", "5.3.1") + _patch_spm_release(monkeypatch) job_id = "uncertifiable-stored-result" - received = [] - def transport(request): - received.append((request.method, request.url.path)) - assert request.method == "GET", "A stored result must not submit a job" - if request.url.path == "/versions": - return httpx.Response( - 200, - json={ - "policyengine": {"5.3.1": "test-worker"}, - "spm_capabilities": { - "5.3.1": { - "contract_version": "canonical-spm-v1", - "defaults": SELECTION, - } - }, - }, - ) - if request.url.path == "/versions/policyengine": - return httpx.Response(200, json={"5.3.1": "test-worker"}) - msg = f"Unexpected worker call: {request.url.path}" - raise AssertionError(msg) - - gateway = object.__new__(SimulationEntrypointClient) - gateway.base_url = "https://worker.invalid" + gateway = _worker_gateway() backend = InMemoryCacheBackend() namespace = CacheNamespace("test", "uncertifiable-stored-result") - annual_cache = ReformImpactCache(backend, namespace) - annual_impacts = ReformImpactsService(annual_cache) - window_cache = BudgetWindowCache(backend, namespace) - service = EconomyService( - reform_impacts_service_=annual_impacts, - budget_window_cache_=window_cache, - simulation_entrypoint_=gateway, - ) - monkeypatch.setattr(economy_routes, "economy_service", service) - app = Flask(__name__) - app.config["TESTING"] = True - app.register_blueprint(economy_routes.economy_bp) + stack = _install_economy_stack(monkeypatch, gateway, backend, namespace) + app = _economy_app() # A stored payload whose reform receipt covers the wrong year: a success # this build cannot certify, exactly as a worker defect would leave it. - with httpx.Client(transport=httpx.MockTransport(transport)) as gateway.client: - setup = service._build_economic_impact_setup_options( - EconomicImpactRequest( - country_id="us", - policy_id=1, - baseline_policy_id=2, - region="us", - dataset="default", - options={}, - api_version="1.0.0", - ), - time_period="budget_window:2035:2" if budget_window else "2036", - ) + transport = httpx.MockTransport(_versions_only_worker) + with httpx.Client(transport=transport) as gateway.client: + setup = _setup_options(stack.service, budget_window=budget_window) if budget_window: - cache_key = service._build_budget_window_cache_key(setup) - window_cache.set_completed_result( + cache_key = stack.service._build_budget_window_cache_key(setup) + stack.window_cache.set_completed_result( cache_key, { "kind": "budgetWindow", @@ -333,26 +410,19 @@ def transport(request): }, setup.observability_id, ) - url = "/us/economy/1/over/2/budget-window" - query = {"start_year": "2035", "window_size": "2"} else: - service._resolve_runtime_bundle_for_setup_options(setup) - annual_impacts.set_reform_impact( - service._reform_impact_identity(setup), - ReformImpactExecution(execution_id=job_id, start_time=None), - options=setup.options, + _store_annual_impact( + stack, + setup, status="ok", reform_impact_json=segmented_result("2036", "reform"), + execution_id=job_id, ) - url = "/us/economy/1/over/2" - query = {"time_period": "2036"} + url, query = _economy_request(budget_window=budget_window) first = None for _ in range(2): - response = app.test_client().get( - url, - query_string={"region": "us", "version": "1.0.0", **query}, - ) + response = _get_economy(app, url, query) assert response.status_code == 400, response.json assert response.json["errors"][0]["code"] == "SPM_CONFIGURATION_UNAVAILABLE" if first is None: @@ -360,26 +430,18 @@ def transport(request): assert response.json == first if budget_window: - state = window_cache.get_state(cache_key) + state = stack.window_cache.get_state(cache_key) assert state is not None assert state.status == "failed" assert state.failure_type == "spm_validation" assert state.error is not None assert state.error["code"] == "SPM_CONFIGURATION_UNAVAILABLE" else: - stored = annual_cache.get_by_execution_id(job_id) + stored = stack.annual_cache.get_by_execution_id(job_id) assert stored.status == "error" assert stored.error_code == "SPM_CONFIGURATION_UNAVAILABLE" assert stored.execution_id is None assert stored.options_hash == setup.options_hash # Replay must come from storage, not from re-reading the payload. - annual_cache = ReformImpactCache(backend, namespace) - annual_impacts = ReformImpactsService(annual_cache) - window_cache = BudgetWindowCache(backend, namespace) - service = EconomyService( - reform_impacts_service_=annual_impacts, - budget_window_cache_=window_cache, - simulation_entrypoint_=gateway, - ) - monkeypatch.setattr(economy_routes, "economy_service", service) + stack = _install_economy_stack(monkeypatch, gateway, backend, namespace) diff --git a/tests/unit/routes/test_user_policy_dual_write_routes.py b/tests/unit/routes/test_user_policy_dual_write_routes.py index d0c6e9d01..b6307f5d9 100644 --- a/tests/unit/routes/test_user_policy_dual_write_routes.py +++ b/tests/unit/routes/test_user_policy_dual_write_routes.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from typing import NamedTuple from unittest.mock import MagicMock, patch import pytest @@ -304,96 +305,115 @@ def test_saved_policy_mirror_failure_returns_503_and_retry_completes( assert create.call_count == mirror.call_count == 2 -@pytest.mark.parametrize( - ( - "method", - "service_method", - "body", - "error", - "expected_status", - "expected_category", - "expected_operation", +class _PersistenceFailure(NamedTuple): + """A saved-policy request whose service call fails, and the response expected.""" + + method: str + service_method: str + body: dict[str, object] + error: Exception + expected_status: int + expected_category: str + expected_operation: str + + +_PERSISTENCE_FAILURES = ( + _PersistenceFailure( + "post", + "create_or_get_user_policy", + _body(), + SQLAlchemyTimeoutError("credential=timeout-secret"), + 503, + "timeout", + "create", ), - [ - ( - "post", - "create_or_get_user_policy", - _body(), - SQLAlchemyTimeoutError("credential=timeout-secret"), - 503, - "timeout", - "create", - ), - ( - "put", - "update_user_policy", - {"id": 10, "reform_label": "Renamed"}, - OperationalError( - "UPDATE user_policies SET secret=:secret", - {"secret": "bound-parameter-secret"}, - RuntimeError("driver-secret"), - ), - 503, - "unavailable", - "update", - ), - ( - "post", - "create_or_get_user_policy", - _body(), - IntegrityError( - "INSERT caller-private-data", - {"user_id": "caller-private-data"}, - RuntimeError("integrity-secret"), - ), - 500, - "integrity", - "create", + _PersistenceFailure( + "put", + "update_user_policy", + {"id": 10, "reform_label": "Renamed"}, + OperationalError( + "UPDATE user_policies SET secret=:secret", + {"secret": "bound-parameter-secret"}, + RuntimeError("driver-secret"), ), - ( - "put", - "update_user_policy", - {"id": 10, "year": "2027"}, - SQLAlchemyError("database-secret"), - 500, - "database", - "update", - ), - ( - "post", - "create_or_get_user_policy", - _body(), - RuntimeError("unexpected-secret"), - 500, - "unexpected", - "create", + 503, + "unavailable", + "update", + ), + _PersistenceFailure( + "post", + "create_or_get_user_policy", + _body(), + IntegrityError( + "INSERT caller-private-data", + {"user_id": "caller-private-data"}, + RuntimeError("integrity-secret"), ), + 500, + "integrity", + "create", + ), + _PersistenceFailure( + "put", + "update_user_policy", + {"id": 10, "year": "2027"}, + SQLAlchemyError("database-secret"), + 500, + "database", + "update", + ), + _PersistenceFailure( + "post", + "create_or_get_user_policy", + _body(), + RuntimeError("unexpected-secret"), + 500, + "unexpected", + "create", + ), + _PersistenceFailure( + "put", + "update_user_policy", + {"id": "caller-id-secret", "year": "2027"}, + RuntimeError("unexpected-secret"), + 500, + "unexpected", + "update", + ), +) + + +def _persistence_failure_id(index: int, failure: _PersistenceFailure) -> str: + """Return the test ID pytest gave this case when each field was a parameter.""" + return "-".join( ( - "put", - "update_user_policy", - {"id": "caller-id-secret", "year": "2027"}, - RuntimeError("unexpected-secret"), - 500, - "unexpected", - "update", - ), + failure.method, + failure.service_method, + f"body{index}", + f"error{index}", + str(failure.expected_status), + failure.expected_category, + failure.expected_operation, + ) + ) + + +@pytest.mark.parametrize( + "failure", + [ + pytest.param(failure, id=_persistence_failure_id(index, failure)) + for index, failure in enumerate(_PERSISTENCE_FAILURES) ], ) def test_saved_policy_persistence_failures_use_allowlisted_records( monkeypatch, - method, - service_method, - body, - error, - expected_status, - expected_category, - expected_operation, + failure, ) -> None: monkeypatch.setenv("DB_WRITE_POLICY", "cloud_sql") with ( patch( - f"policyengine_api.routes.policy_routes.user_policy_service.{service_method}", - side_effect=UserPolicyPersistenceError.from_exception(error), + f"policyengine_api.routes.policy_routes.user_policy_service.{failure.service_method}", + side_effect=UserPolicyPersistenceError.from_exception(failure.error), ), patch( "policyengine_api.routes.policy_routes.current_request_id", @@ -401,14 +421,16 @@ def test_saved_policy_persistence_failures_use_allowlisted_records( ), patch("policyengine_api.routes.policy_routes.logger.log_struct") as log_struct, ): - response = getattr(_client(), method)("/us/user-policy", json=body) + response = getattr(_client(), failure.method)( + "/us/user-policy", json=failure.body + ) expected_message = ( "Policy database is temporarily unavailable; please try again later." - if expected_status == 503 + if failure.expected_status == 503 else "Internal database error; please try again later." ) - assert response.status_code == expected_status + assert response.status_code == failure.expected_status assert response.json == {"message": expected_message} log_struct.assert_called_once() @@ -430,11 +452,11 @@ def test_saved_policy_persistence_failures_use_allowlisted_records( "http_status", "duration_ms", } - assert payload["operation"] == expected_operation - assert payload["failure_category"] == expected_category - assert payload["http_status"] == expected_status + assert payload["operation"] == failure.expected_operation + assert payload["failure_category"] == failure.expected_category + assert payload["http_status"] == failure.expected_status assert payload["request_id"] == "request-123" - supplied_id = body.get("id") + supplied_id = failure.body.get("id") expected_logged_id = ( supplied_id if isinstance(supplied_id, int) diff --git a/tests/unit/services/test_budget_window_cache.py b/tests/unit/services/test_budget_window_cache.py index e97367392..bd028052a 100644 --- a/tests/unit/services/test_budget_window_cache.py +++ b/tests/unit/services/test_budget_window_cache.py @@ -32,7 +32,6 @@ def get(self, key): # noqa: ARG002 # implements the Redis client methods Budge if self.method == "get": msg = "redis unavailable" raise RuntimeError(msg) - return def set(self, key, value, *, nx=False, ex=None): # noqa: ARG002 # implements the Redis client methods BudgetWindowCache calls if self.method == "set": diff --git a/tests/unit/services/test_economy_service.py b/tests/unit/services/test_economy_service.py index 498e3fd6a..306c0e8db 100644 --- a/tests/unit/services/test_economy_service.py +++ b/tests/unit/services/test_economy_service.py @@ -1,6 +1,6 @@ import json from dataclasses import replace -from typing import Literal +from typing import Literal, NamedTuple from unittest.mock import MagicMock, call, patch import httpx @@ -55,6 +55,7 @@ MOCK_RESOLVED_DATASET, MOCK_SUBMISSION_CLAIM_ID, MOCK_TIME_PERIOD, + MockBatchProgress, create_mock_budget_window_batch_execution, create_mock_reform_impact, ) @@ -83,6 +84,15 @@ def setup_options_for(country_id: str, region: str) -> EconomicImpactSetupOption ) +class EconomyServiceHarness(NamedTuple): + """The service under test, its call parameters and the mocks it talks to.""" + + service: EconomyService + params: dict + reform_impacts_service: MagicMock + simulation_entrypoint: MagicMock + + def hash_prefix_lookup(reform_impacts_service: MagicMock) -> MagicMock: """Return the mocked lookup of reform impacts by options-hash prefix.""" lookup: MagicMock = ( @@ -192,6 +202,21 @@ def base_params(self): "time_period": MOCK_TIME_PERIOD, } + @pytest.fixture + def economy_service_harness( + self, + economy_service: EconomyService, + base_params: dict, + mock_reform_impacts_service: MagicMock, + mock_simulation_entrypoint: MagicMock, + ) -> EconomyServiceHarness: + return EconomyServiceHarness( + service=economy_service, + params=base_params, + reform_impacts_service=mock_reform_impacts_service, + simulation_entrypoint=mock_simulation_entrypoint, + ) + @pytest.mark.usefixtures( "mock_country_package_versions", "mock_policyengine_version", @@ -282,26 +307,24 @@ def test__given_cached_error_impact__returns_error_message( self, cached_message, expected_message, - economy_service, - base_params, - mock_reform_impacts_service, - mock_simulation_entrypoint, + economy_service_harness, ): + harness = economy_service_harness failed_impact = create_mock_reform_impact( status="error", message=cached_message, ) - hash_prefix_lookup(mock_reform_impacts_service).return_value = [ + hash_prefix_lookup(harness.reform_impacts_service).return_value = [ failed_impact ] - result = economy_service.get_economic_impact(**base_params) + result = harness.service.get_economic_impact(**harness.params) assert result.status == ImpactStatus.ERROR assert result.data is None assert result.message == expected_message - mock_simulation_entrypoint.run.assert_not_called() - mock_simulation_entrypoint.get_execution_by_id.assert_not_called() + harness.simulation_entrypoint.run.assert_not_called() + harness.simulation_entrypoint.get_execution_by_id.assert_not_called() @pytest.mark.usefixtures( "mock_country_package_versions", @@ -1392,10 +1415,12 @@ def test__given_cached_batch_id__returns_running_batch_progress( create_mock_budget_window_batch_execution( batch_job_id="fc-budget-123", status="running", - progress=33, - completed_years=["2026"], - running_years=["2027"], - queued_years=["2028"], + batch_progress=MockBatchProgress( + progress=33, + completed_years=["2026"], + running_years=["2027"], + queued_years=["2028"], + ), ) ) @@ -1429,8 +1454,9 @@ def test__given_completed_batch_poll__caches_result_and_returns_completed( create_mock_budget_window_batch_execution( batch_job_id="fc-budget-123", status="complete", - progress=100, - completed_years=["2026", "2027", "2028"], + batch_progress=MockBatchProgress( + progress=100, completed_years=["2026", "2027", "2028"] + ), result=completed_result, ) ) @@ -1464,10 +1490,12 @@ def test__given_completed_batch_without_result__returns_error_without_caching( create_mock_budget_window_batch_execution( batch_job_id="fc-budget-123", status="complete", - progress=100, - completed_years=["2026", "2027"], - running_years=[], - queued_years=["2028"], + batch_progress=MockBatchProgress( + progress=100, + completed_years=["2026", "2027"], + running_years=[], + queued_years=["2028"], + ), result=malformed_result, ) ) @@ -1501,8 +1529,9 @@ def test__given_completed_batch_cache_write_fails__does_not_clear_batch_id( create_mock_budget_window_batch_execution( batch_job_id="fc-budget-123", status="complete", - progress=100, - completed_years=["2026", "2027", "2028"], + batch_progress=MockBatchProgress( + progress=100, completed_years=["2026", "2027", "2028"] + ), result=completed_result, ) ) @@ -1533,10 +1562,12 @@ def test__given_failed_batch_poll__returns_failed( create_mock_budget_window_batch_execution( batch_job_id="fc-budget-123", status="failed", - progress=33, - completed_years=["2026"], - queued_years=["2028"], - failed_years=["2027"], + batch_progress=MockBatchProgress( + progress=33, + completed_years=["2026"], + queued_years=["2028"], + failed_years=["2027"], + ), error="Budget window failed for 2027", ) ) diff --git a/tests/unit/services/test_economy_service_request_plumbing.py b/tests/unit/services/test_economy_service_request_plumbing.py index 2903cc94e..173807a51 100644 --- a/tests/unit/services/test_economy_service_request_plumbing.py +++ b/tests/unit/services/test_economy_service_request_plumbing.py @@ -40,6 +40,7 @@ MOCK_POLICYENGINE_VERSION, MOCK_RESOLVED_APP_NAME, MOCK_SUBMISSION_CLAIM_ID, + MockBatchProgress, create_mock_budget_window_batch_execution, ) from tests.fixtures.spm import resolved_options @@ -357,10 +358,12 @@ def test_budget_window_poll_reports_running_years( create_mock_budget_window_batch_execution( batch_job_id="batch-7", status="running", - progress=None, - completed_years=["2026"], - running_years=["2027", "2028", "2029"], - queued_years=[], + batch_progress=MockBatchProgress( + progress=None, + completed_years=["2026"], + running_years=["2027", "2028", "2029"], + queued_years=[], + ), ) ) diff --git a/tests/unit/services/test_simulation_service.py b/tests/unit/services/test_simulation_service.py index 8940d3865..bfabce180 100644 --- a/tests/unit/services/test_simulation_service.py +++ b/tests/unit/services/test_simulation_service.py @@ -281,9 +281,9 @@ def test_creating_a_simulation_never_locks_a_household_row( statements = [] engine = orm_session_factory.kw["bind"] - @event.listens_for(engine, "before_cursor_execute") - def record(conn, cursor, statement, parameters, context, executemany): # noqa: ARG001 # SQLAlchemy calls before_cursor_execute listeners with these arguments - statements.append(" ".join(statement.split()).lower()) + @event.listens_for(engine, "before_cursor_execute", named=True) + def record(**event_arguments): + statements.append(" ".join(event_arguments["statement"].split()).lower()) try: result = service.get_or_create_simulation( diff --git a/tests/unit/test_asgi_factory.py b/tests/unit/test_asgi_factory.py index 56c037bac..3f221e35c 100644 --- a/tests/unit/test_asgi_factory.py +++ b/tests/unit/test_asgi_factory.py @@ -12,7 +12,7 @@ from flask import Flask, Response, jsonify, make_response, request from flask_cors import CORS -import policyengine_api.asgi_factory as asgi_factory +from policyengine_api import asgi_factory from policyengine_api.asgi_factory import create_asgi_app from policyengine_api.migration_flags import ( RouteImplementation, diff --git a/tests/unit/test_bump_version.py b/tests/unit/test_bump_version.py new file mode 100644 index 000000000..4929f3c6a --- /dev/null +++ b/tests/unit/test_bump_version.py @@ -0,0 +1,66 @@ +"""Characterization tests for the changelog-driven version bump inference.""" + +import importlib.util +from pathlib import Path +from types import ModuleType + +import pytest + +BUMP_VERSION_PATH = Path(__file__).parents[2] / ".github" / "bump_version.py" + + +def _load_bump_version() -> ModuleType: + spec = importlib.util.spec_from_file_location("bump_version", BUMP_VERSION_PATH) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _changelog_dir(tmp_path: Path, *fragment_names: str) -> Path: + changelog_dir = tmp_path / "changelog.d" + changelog_dir.mkdir() + for name in fragment_names: + (changelog_dir / name).write_text("A change.\n") + return changelog_dir + + +@pytest.mark.parametrize( + ("fragment_names", "expected_bump"), + [ + (("1.fixed.md",), "patch"), + (("1.fixed",), "patch"), + (("notes",), "patch"), + (("1.added.md",), "minor"), + (("1.added",), "minor"), + (("1.removed.md", "2.fixed.md"), "minor"), + (("a.b.breaking.md", "1.added.md"), "major"), + (("1.breaking",), "major"), + (("breaking.md",), "patch"), + ((".added.md",), "minor"), + (("1.changed.md", ".gitkeep"), "patch"), + ], +) +def test_infer_bump_reads_the_category_from_the_suffix_or_the_stem( + tmp_path: Path, fragment_names: tuple[str, ...], expected_bump: str +) -> None: + bump_version = _load_bump_version() + + assert ( + bump_version.infer_bump(_changelog_dir(tmp_path, *fragment_names)) + == expected_bump + ) + + +def test_infer_bump_exits_when_only_gitkeep_is_present( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + bump_version = _load_bump_version() + changelog_dir = _changelog_dir(tmp_path, ".gitkeep") + + with pytest.raises(SystemExit) as exit_info: + bump_version.infer_bump(changelog_dir) + + assert exit_info.value.code == 1 + assert capsys.readouterr().err == "No changelog fragments found\n" diff --git a/tests/unit/test_capture_migration_baseline.py b/tests/unit/test_capture_migration_baseline.py index b7e1c73b2..ca8537b36 100644 --- a/tests/unit/test_capture_migration_baseline.py +++ b/tests/unit/test_capture_migration_baseline.py @@ -1,6 +1,7 @@ import json import httpx +import pytest from scripts import capture_migration_baseline @@ -159,3 +160,143 @@ def handler(request): ] assert summary["completion_count"] == 1 assert summary["completion_failure_count"] == 0 + + +def _client_with_transport( + monkeypatch: pytest.MonkeyPatch, transport: httpx.MockTransport +) -> None: + real_client = httpx.Client + monkeypatch.setattr( + capture_migration_baseline.httpx, + "Client", + lambda **kwargs: real_client( + transport=transport, + base_url=kwargs["base_url"], + timeout=kwargs["timeout"], + ), + ) + + +def _without_latency( + result: capture_migration_baseline.ProbeResult, +) -> dict[str, object]: + return {**result.__dict__, "latency_ms": None} + + +def test_run_probes_records_transport_errors_as_failed_probes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def refuse(request: httpx.Request) -> httpx.Response: + msg = "connection refused" + raise httpx.ConnectError(msg, request=request) + + _client_with_transport(monkeypatch, httpx.MockTransport(refuse)) + + results = capture_migration_baseline.run_probes("https://api.test", 1) + + assert [_without_latency(result) for result in results] == [ + { + "name": probe.name, + "method": "GET", + "path": probe.path, + "status_code": 0, + "latency_ms": None, + "ok": False, + "error": "connection refused", + "completion_ms": None, + "completed": None, + } + for probe in capture_migration_baseline.DEFAULT_PROBES + ] + assert all(result.latency_ms >= 0 for result in results) + + +def test_gateway_probe_sends_the_payload_and_records_each_request( + monkeypatch: pytest.MonkeyPatch, +) -> None: + bodies: list[bytes] = [] + + def handler(request: httpx.Request) -> httpx.Response: + bodies.append(request.content) + if request.method == "POST": + return httpx.Response(202, json={"job_id": "job-1"}) + return httpx.Response(200, json={"status": "failed", "error": "boom"}) + + _client_with_transport(monkeypatch, httpx.MockTransport(handler)) + + results = capture_migration_baseline.run_simulation_gateway_probe( + "https://simulation.test", + {"country": "us"}, + poll_timeout_seconds=1, + poll_interval_seconds=0, + ) + + assert bodies == [b'{"country":"us"}', b""] + assert [ + {**_without_latency(result), "completion_ms": None} for result in results + ] == [ + { + "name": "simulation_gateway_submit", + "method": "POST", + "path": "/simulate/economy/comparison", + "status_code": 202, + "latency_ms": None, + "ok": True, + "error": None, + "completion_ms": None, + "completed": None, + }, + { + "name": "simulation_gateway_poll", + "method": "GET", + "path": "/jobs/job-1", + "status_code": 200, + "latency_ms": None, + "ok": True, + "error": None, + "completion_ms": None, + "completed": None, + }, + { + "name": "simulation_gateway_completion", + "method": "GET", + "path": "/jobs/job-1", + "status_code": 200, + "latency_ms": None, + "ok": False, + "error": "boom", + "completion_ms": None, + "completed": False, + }, + ] + assert results[2].latency_ms == results[1].latency_ms + + +def test_gateway_probe_stops_when_the_submission_is_rejected( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _client_with_transport( + monkeypatch, + httpx.MockTransport(lambda _request: httpx.Response(400, json={})), + ) + + results = capture_migration_baseline.run_simulation_gateway_probe( + "https://simulation.test", + {"country": "us"}, + poll_timeout_seconds=1, + poll_interval_seconds=0, + ) + + assert [_without_latency(result) for result in results] == [ + { + "name": "simulation_gateway_submit", + "method": "POST", + "path": "/simulate/economy/comparison", + "status_code": 400, + "latency_ms": None, + "ok": False, + "error": None, + "completion_ms": None, + "completed": None, + } + ] diff --git a/tests/unit/test_check_updates.py b/tests/unit/test_check_updates.py new file mode 100644 index 000000000..1e3687ab9 --- /dev/null +++ b/tests/unit/test_check_updates.py @@ -0,0 +1,71 @@ +"""Characterization tests for fetching a package's changelog from GitHub.""" + +import importlib.util +from pathlib import Path +from types import ModuleType, SimpleNamespace + +import pytest + +CHECK_UPDATES_PATH = ( + Path(__file__).parents[2] / ".github" / "scripts" / "check_updates.py" +) + + +def _load_check_updates() -> ModuleType: + spec = importlib.util.spec_from_file_location("check_updates", CHECK_UPDATES_PATH) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.mark.parametrize( + ("status_by_branch", "expected_text", "expected_branches"), + [ + ({"main": 200, "master": 200}, "changelog from main", ["main"]), + ({"main": 404, "master": 200}, "changelog from master", ["main", "master"]), + ({"main": 404, "master": 500}, None, ["main", "master"]), + ({"main": 201, "master": 204}, None, ["main", "master"]), + ], +) +def test_fetch_changelog_uses_the_first_branch_that_answers_ok( + monkeypatch: pytest.MonkeyPatch, + status_by_branch: dict[str, int], + expected_text: str | None, + expected_branches: list[str], +) -> None: + check_updates = _load_check_updates() + requested: list[tuple[str, int]] = [] + + def fake_get(url: str, timeout: int) -> SimpleNamespace: + branch = url.split("/")[-2] + requested.append((url, timeout)) + return SimpleNamespace( + status_code=status_by_branch[branch], text=f"changelog from {branch}" + ) + + monkeypatch.setattr(check_updates.requests, "get", fake_get) + + assert check_updates.fetch_changelog("policyengine-us") == expected_text + assert requested == [ + ( + "https://raw.githubusercontent.com/PolicyEngine/policyengine-us/" + f"{branch}/CHANGELOG.md", + 30, + ) + for branch in expected_branches + ] + + +def test_fetch_changelog_skips_unknown_packages( + monkeypatch: pytest.MonkeyPatch, +) -> None: + check_updates = _load_check_updates() + monkeypatch.setattr( + check_updates.requests, + "get", + lambda *_args, **_kwargs: pytest.fail("no request expected"), + ) + + assert check_updates.fetch_changelog("policyengine-ca") is None diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index e3a99e285..ea1852b97 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -1849,25 +1849,9 @@ def test_set_cloud_run_revision_dry_run_targets_service_override(): ) -def test_push_workflow_runs_release_and_cloud_run_staging_tests_in_deploy_job(): - workflow = _push_workflow() - cloud_run_deploy = _workflow_job_block(workflow, "deploy-cloud-run-staging") - parallel_live_tests = _workflow_job_block( - workflow, - "parallel-live-simulation-tests-staging-cloud-run", - ) - cloud_run_promotion = _workflow_job_block(workflow, "promote-cloud-run-staging") - production_check = _workflow_job_block( - workflow, - "ensure-production-model-version-aligns-with-sim-api", - ) - cloud_run_test_command = ( - "python -m pytest tests/integration/test_cloud_run_candidate.py " - "tests/integration/test_live_v2_metadata.py " - "tests/integration/test_live_v2_policies.py -v" - ) - live_test_script = (REPO / ".github/scripts/run_live_staging_tests.sh").read_text() - +def _assert_staging_deploy_runs_release_and_integration_tests( + cloud_run_deploy: str, cloud_run_test_command: str +) -> None: assert "bash .github/scripts/run_release_tests.sh" in cloud_run_deploy assert cloud_run_deploy.index( "bash .github/scripts/run_release_tests.sh" @@ -1882,6 +1866,11 @@ def test_push_workflow_runs_release_and_cloud_run_staging_tests_in_deploy_job(): assert cloud_run_deploy.index( "Wait for Cloud Run staging health" ) < cloud_run_deploy.index("Run staging integration tests") + + +def _assert_live_tests_run_in_parallel_after_readiness( + workflow: str, parallel_live_tests: str, live_test_script: str +) -> None: assert "integration-tests-staging-cloud-run:" not in workflow assert "strategy:" not in parallel_live_tests assert "matrix:" not in parallel_live_tests @@ -1912,6 +1901,11 @@ def test_push_workflow_runs_release_and_cloud_run_staging_tests_in_deploy_job(): ) assert "github.run_id" in parallel_live_tests assert "github.run_attempt" in parallel_live_tests + + +def _assert_promotion_verifies_the_tested_candidate( + workflow: str, production_check: str, cloud_run_promotion: str +) -> None: assert "needs: promote-cloud-run-staging" in production_check assert "- integration-tests-staging-cloud-run" not in production_check assert "- integration-tests-staging-cloud-run" not in cloud_run_promotion @@ -1949,6 +1943,36 @@ def test_push_workflow_runs_release_and_cloud_run_staging_tests_in_deploy_job(): ) +def test_push_workflow_runs_release_and_cloud_run_staging_tests_in_deploy_job(): + workflow = _push_workflow() + cloud_run_deploy = _workflow_job_block(workflow, "deploy-cloud-run-staging") + parallel_live_tests = _workflow_job_block( + workflow, + "parallel-live-simulation-tests-staging-cloud-run", + ) + cloud_run_promotion = _workflow_job_block(workflow, "promote-cloud-run-staging") + production_check = _workflow_job_block( + workflow, + "ensure-production-model-version-aligns-with-sim-api", + ) + cloud_run_test_command = ( + "python -m pytest tests/integration/test_cloud_run_candidate.py " + "tests/integration/test_live_v2_metadata.py " + "tests/integration/test_live_v2_policies.py -v" + ) + live_test_script = (REPO / ".github/scripts/run_live_staging_tests.sh").read_text() + + _assert_staging_deploy_runs_release_and_integration_tests( + cloud_run_deploy, cloud_run_test_command + ) + _assert_live_tests_run_in_parallel_after_readiness( + workflow, parallel_live_tests, live_test_script + ) + _assert_promotion_verifies_the_tested_candidate( + workflow, production_check, cloud_run_promotion + ) + + def test_push_workflow_uses_local_redis_for_predeployment_test_suite(): staging = _workflow_job_block(_push_workflow(), "deploy-cloud-run-staging") test_step_start = staging.index("- name: Run release tests") diff --git a/tests/unit/test_household_event_command.py b/tests/unit/test_household_event_command.py index 632d75f6d..d18fbda07 100644 --- a/tests/unit/test_household_event_command.py +++ b/tests/unit/test_household_event_command.py @@ -7,12 +7,15 @@ import pytest from sqlalchemy import Engine +from sqlalchemy.engine import URL +from sqlalchemy.pool import NullPool from policyengine_api.services.v2.households.types import ( LegacyHouseholdPersistenceResult, ) from scripts import process_v1_household_mirror_event as command +PROXY_PASSWORD = "proxy-password" # noqa: S105 # a fake test password STAGING_ENVIRONMENT = { "DEPLOYMENT_ENVIRONMENT": "staging", "V2_SUPABASE_ENVIRONMENT": "staging", @@ -192,3 +195,233 @@ def test_command_redacts_processing_failure_and_records_error( assert "secret" not in error_output assert "private-host" not in error_output assert audit.call_args.args[0]["outcome"] == "error" + + +def _staging_identity_fields() -> dict[str, str]: + return { + "environment": "staging", + "cloud_sql_instance": "project:region:staging-db", + "supabase_project_ref": "abcdefghijklmnopqrst", + "operator_identity": "stage11-operator@example.iam", + } + + +def _run_staging_command( + environ: dict[str, str], legacy_household_id: str = "42" +) -> int: + return command.main( + [ + "--environment", + "staging", + "--country-id", + "us", + "--legacy-household-id", + legacy_household_id, + ], + environ=environ, + ) + + +def test_success_audit_record_lists_every_field( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + destination_id = UUID("00000000-0000-0000-0000-000000000012") + monkeypatch.setattr( + command, + "process_household_event_after_commit", + Mock( + return_value=LegacyHouseholdPersistenceResult( + household_id=destination_id, + household_created=True, + mapping_created=False, + ) + ), + ) + audit = Mock() + monkeypatch.setattr(command.logger, "log_struct", audit) + + assert _run_staging_command(STAGING_ENVIRONMENT) == 0 + + audit.assert_called_once_with( + { + "message": "Explicit v1 household create-event processing completed", + "metric_name": "v1_household_event_operator_operations", + "metric_value": 1, + **_staging_identity_fields(), + "country_id": "us", + "legacy_household_id": 42, + "destination_household_id": str(destination_id), + "outcome": "ok", + "error_type": None, + }, + severity="INFO", + ) + assert capsys.readouterr().out == ( + '{"country_id": "us", "destination_household_id": ' + '"00000000-0000-0000-0000-000000000012", "environment": "staging", ' + '"household_created": true, "legacy_household_id": 42, ' + '"mapping_created": false, "outcome": "ok"}\n' + ) + + +def test_failure_audit_record_names_only_the_error_type( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.setattr( + command, + "process_household_event_after_commit", + Mock(side_effect=LookupError("secret detail")), + ) + audit = Mock() + monkeypatch.setattr(command.logger, "log_struct", audit) + + assert _run_staging_command(STAGING_ENVIRONMENT) == 1 + + audit.assert_called_once_with( + { + "message": "Explicit v1 household create-event processing completed", + "metric_name": "v1_household_event_operator_operations", + "metric_value": 1, + **_staging_identity_fields(), + "country_id": "us", + "legacy_household_id": 42, + "destination_household_id": None, + "outcome": "error", + "error_type": "LookupError", + }, + severity="ERROR", + ) + assert capsys.readouterr().err == ( + '{"country_id": "us", "error": {"message": "household create-event ' + 'processing failed", "type": "LookupError"}, "legacy_household_id": 42, ' + '"outcome": "error"}\n' + ) + + +def test_configuration_failure_audit_record_has_no_identity( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + audit = Mock() + monkeypatch.setattr(command.logger, "log_struct", audit) + environ = {**STAGING_ENVIRONMENT, "DEPLOYMENT_ENVIRONMENT": ""} + + assert _run_staging_command(environ, legacy_household_id="7") == 1 + + audit.assert_called_once_with( + { + "message": "Explicit v1 household create-event processing completed", + "metric_name": "v1_household_event_operator_operations", + "metric_value": 1, + "environment": None, + "cloud_sql_instance": None, + "supabase_project_ref": None, + "operator_identity": None, + "country_id": "us", + "legacy_household_id": 7, + "destination_household_id": None, + "outcome": "error", + "error_type": "HouseholdEventCommandConfigurationError", + }, + severity="ERROR", + ) + assert capsys.readouterr().err == ( + '{"country_id": "us", "error": {"message": "DEPLOYMENT_ENVIRONMENT is ' + 'required", "type": "HouseholdEventCommandConfigurationError"}, ' + '"legacy_household_id": 7, "outcome": "error"}\n' + ) + + +def test_audit_failure_does_not_change_the_command_result( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + command, + "process_household_event_after_commit", + Mock( + return_value=LegacyHouseholdPersistenceResult( + household_id=UUID("00000000-0000-0000-0000-000000000013"), + household_created=True, + mapping_created=True, + ) + ), + ) + monkeypatch.setattr( + command.logger, "log_struct", Mock(side_effect=RuntimeError("log down")) + ) + + assert _run_staging_command(STAGING_ENVIRONMENT) == 0 + + +@pytest.mark.parametrize("raw_port", ["", " "]) +def test_proxy_is_not_used_without_a_port(raw_port: str) -> None: + assert command._proxy_event_service({"POLICYENGINE_DB_PROXY_PORT": raw_port}) == ( + None, + None, + ) + assert command._proxy_event_service({}) == (None, None) + + +@pytest.mark.parametrize( + ("raw_port", "message"), + [ + ("proxy", "POLICYENGINE_DB_PROXY_PORT must be an integer"), + ("0", "POLICYENGINE_DB_PROXY_PORT is outside the valid range"), + ("65536", "POLICYENGINE_DB_PROXY_PORT is outside the valid range"), + ("-1", "POLICYENGINE_DB_PROXY_PORT is outside the valid range"), + ], +) +def test_proxy_port_must_be_a_valid_tcp_port(raw_port: str, message: str) -> None: + with pytest.raises( + command.HouseholdEventCommandConfigurationError, match=f"^{message}$" + ): + command._proxy_event_service( + { + "POLICYENGINE_DB_PROXY_PORT": raw_port, + "POLICYENGINE_DB_PASSWORD": PROXY_PASSWORD, + } + ) + + +@pytest.mark.parametrize(("raw_port", "port"), [("1", 1), (" 65535 ", 65535)]) +def test_proxy_connects_as_the_migration_user( + monkeypatch: pytest.MonkeyPatch, raw_port: str, port: int +) -> None: + engine = Mock(spec=Engine) + create_engine = Mock(return_value=engine) + session_factory = Mock() + build_session_factory = Mock(return_value=session_factory) + household_service = Mock() + monkeypatch.setattr(command, "create_engine", create_engine) + monkeypatch.setattr(command, "build_session_factory", build_session_factory) + monkeypatch.setattr(command, "HouseholdService", household_service) + + service, returned_engine = command._proxy_event_service( + { + "POLICYENGINE_DB_PROXY_PORT": raw_port, + "POLICYENGINE_DB_PASSWORD": PROXY_PASSWORD, + } + ) + + assert returned_engine is engine + assert service is household_service.return_value + household_service.assert_called_once_with(session_factory) + build_session_factory.assert_called_once_with(engine) + create_engine.assert_called_once_with( + URL.create( + "mysql+pymysql", + username="policyengine_schema_migrator", + password=PROXY_PASSWORD, + host="127.0.0.1", + port=port, + database="policyengine", + ), + poolclass=NullPool, + ) + + +def test_proxy_requires_the_database_password() -> None: + with pytest.raises( + command.HouseholdEventCommandConfigurationError, + match=r"^POLICYENGINE_DB_PASSWORD is required$", + ): + command._proxy_event_service({"POLICYENGINE_DB_PROXY_PORT": "3307"}) diff --git a/tests/unit/test_measure_cloud_run_runtime.py b/tests/unit/test_measure_cloud_run_runtime.py new file mode 100644 index 000000000..4bfc7b782 --- /dev/null +++ b/tests/unit/test_measure_cloud_run_runtime.py @@ -0,0 +1,288 @@ +"""Characterization tests for the Cloud Run closed-loop load generator.""" + +import argparse +import asyncio +import json +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import httpx +import pytest + +from scripts import measure_cloud_run_runtime as load_generator + +CALCULATE_PAYLOAD = Path(__file__).parents[1] / "data" / "calculate_us_1_data.json" + + +class _CountingClock: + """Monotonic and performance counters that advance one second per reading.""" + + def __init__(self) -> None: + self.readings = 0 + + def _read(self) -> float: + self.readings += 1 + return float(self.readings) + + def monotonic(self) -> float: + return self._read() + + def perf_counter(self) -> float: + return self._read() + + +@pytest.mark.parametrize( + ("status_code", "outcome"), + [ + (200, "ok"), + (204, "ok"), + (399, "ok"), + (400, "http_4xx"), + (404, "http_4xx"), + (428, "http_4xx"), + (429, "http_429"), + (430, "http_4xx"), + (499, "http_4xx"), + (500, "http_5xx"), + (503, "http_5xx"), + ], +) +def test_classify_groups_status_codes(status_code: int, outcome: str) -> None: + assert load_generator._classify(status_code) == outcome + + +def test_readiness_wait_polls_until_the_service_answers_ok( + monkeypatch: pytest.MonkeyPatch, +) -> None: + clock = _CountingClock() + monkeypatch.setattr(load_generator, "time", clock) + sleeps: list[float] = [] + + async def fake_sleep(seconds: float) -> None: + sleeps.append(seconds) + + monkeypatch.setattr(load_generator.asyncio, "sleep", fake_sleep) + statuses = iter([503, 201, 200]) + requests: list[tuple[str, Any]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append((request.url.path, request.extensions["timeout"])) + status = next(statuses) + if status == 201: + msg = "reset" + raise httpx.ConnectError(msg, request=request) + return httpx.Response(status) + + async def wait() -> float: + async with httpx.AsyncClient( + transport=httpx.MockTransport(handler), base_url="https://api.test" + ) as client: + return await load_generator._wait_for_readiness(client, 60.0) + + assert asyncio.run(wait()) == 3.0 + assert [path for path, _timeout in requests] == ["/readiness-check"] * 3 + assert requests[0][1]["read"] == 20.0 + assert sleeps == [5.0, 5.0] + + +def test_readiness_wait_gives_up_after_the_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(load_generator, "time", _CountingClock()) + + async def wait() -> float: + async with httpx.AsyncClient( + transport=httpx.MockTransport(lambda _request: httpx.Response(500)), + base_url="https://api.test", + ) as client: + return await load_generator._wait_for_readiness(client, 0.5) + + with pytest.raises(RuntimeError, match=r"^/readiness-check not ready after 0s$"): + asyncio.run(wait()) + + +def _load_args(**overrides: object) -> argparse.Namespace: + values = { + "base_url": "https://api.test/", + "cell_label": "cell-a", + "clients": 2, + "duration_seconds": 20.0, + "warmup_seconds": 6.0, + "calculate_share": 0.5, + "axes_count": 7, + "cache_bust": "always", + "seed": 42, + "calculate_payload": str(CALCULATE_PAYLOAD), + "timeout_calculate": 290.0, + "timeout_metadata": 60.0, + "readiness_timeout": 420.0, + } + return argparse.Namespace(**{**values, **overrides}) + + +def _run_load_cell( + monkeypatch: pytest.MonkeyPatch, args: argparse.Namespace +) -> tuple[dict[str, Any], list[tuple[Any, ...]]]: + monkeypatch.setattr(load_generator, "time", _CountingClock()) + monkeypatch.setattr( + load_generator, + "uuid", + SimpleNamespace(uuid4=lambda: SimpleNamespace(hex="0123456789abcdef")), + ) + timestamps = iter(["2026-01-01T00:00:00Z", "2026-01-01T00:10:00Z"]) + monkeypatch.setattr(load_generator, "_utc_now_iso", lambda: next(timestamps)) + requests: list[tuple[Any, ...]] = [] + responses_and_errors: list[httpx.Response | httpx.HTTPError] = [ + httpx.Response(200), + httpx.Response(200), + httpx.Response(429), + httpx.Response(200), + httpx.Response(503), + httpx.Response(404), + httpx.ReadTimeout("slow"), + httpx.Response(200), + httpx.ConnectError("refused"), + ] + outcomes = iter(responses_and_errors * 20) + + def handler(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) if request.content else None + requests.append( + ( + request.method, + request.url.path, + None if body is None else body.get("_loadtest"), + None if body is None else body["household"]["axes"][0][0]["count"], + request.extensions["timeout"]["read"], + ) + ) + if request.url.path == "/readiness-check": + return httpx.Response(200) + outcome = next(outcomes) + if isinstance(outcome, Exception): + raise outcome + return outcome + + real_async_client = httpx.AsyncClient + monkeypatch.setattr( + load_generator.httpx, + "AsyncClient", + lambda **kwargs: real_async_client( + transport=httpx.MockTransport(handler), **kwargs + ), + ) + summary = asyncio.run(load_generator._run(args)) + return summary, requests + + +def _endpoint_summary(**fields: object) -> dict[str, object]: + return { + "request_count": 0, + "achieved_rps": 0.0, + "p50_latency_ms": None, + "p90_latency_ms": None, + "p95_latency_ms": None, + "p99_latency_ms": None, + "max_latency_ms": None, + "outcome_counts": {}, + "status_counts": {}, + **fields, + } + + +ONE_SECOND_LATENCIES = { + "p50_latency_ms": 1000.0, + "p90_latency_ms": 1000.0, + "p95_latency_ms": 1000.0, + "p99_latency_ms": 1000.0, + "max_latency_ms": 1000.0, +} + + +def test_load_cell_runs_seeded_clients_and_summarizes_each_endpoint( + monkeypatch: pytest.MonkeyPatch, +) -> None: + summary, requests = _run_load_cell(monkeypatch, _load_args()) + + # The mock transport never yields, so the first client runs to the deadline + # before the second one starts. + assert requests == [ + ("GET", "/readiness-check", None, None, 20.0), + ("POST", "/us/calculate", "01234567-cell-a-throwaway", 7, 290.0), + ("POST", "/us/calculate", "01234567-cell-a-0-1", 7, 290.0), + ("GET", "/us/metadata", None, None, 60.0), + ("GET", "/us/metadata", None, None, 60.0), + ("GET", "/us/metadata", None, None, 60.0), + ("POST", "/us/calculate", "01234567-cell-a-0-5", 7, 290.0), + ("GET", "/us/metadata", None, None, 60.0), + ("GET", "/us/metadata", None, None, 60.0), + ] + assert summary == { + "cell": "cell-a", + "base_url": "https://api.test/", + "clients": 2, + "duration_seconds": 20.0, + "warmup_seconds": 6.0, + "calculate_share": 0.5, + "axes_count": 7, + "cache_bust": "always", + "seed": 42, + "readiness_wait_seconds": 1.0, + "started_at_utc": "2026-01-01T00:00:00Z", + "ended_at_utc": "2026-01-01T00:10:00Z", + "warmup_request_count": 1, + "measured_request_count": 6, + "measured_elapsed_seconds": 25.0, + "endpoints": { + "metadata": _endpoint_summary( + request_count=5, + achieved_rps=0.2, + **ONE_SECOND_LATENCIES, + outcome_counts={"http_429": 1, "ok": 2, "http_5xx": 1, "timeout": 1}, + status_counts={"429": 1, "200": 2, "503": 1, "0": 1}, + ), + "calculate": _endpoint_summary( + request_count=1, + achieved_rps=0.04, + outcome_counts={"http_4xx": 1}, + status_counts={"404": 1}, + ), + }, + } + assert list(summary["endpoints"]["metadata"]["outcome_counts"]) == [ + "http_429", + "ok", + "http_5xx", + "timeout", + ] + + +def test_load_cell_without_cache_busting_repeats_the_same_payload( + monkeypatch: pytest.MonkeyPatch, +) -> None: + summary, requests = _run_load_cell( + monkeypatch, _load_args(cache_bust="never", clients=1, calculate_share=1.0) + ) + + assert requests == [ + ("GET", "/readiness-check", None, None, 20.0), + ("POST", "/us/calculate", "01234567-cell-a-throwaway", 7, 290.0), + *[("POST", "/us/calculate", None, 7, 290.0)] * 7, + ] + assert summary["endpoints"] == { + "metadata": _endpoint_summary(), + "calculate": _endpoint_summary( + request_count=6, + achieved_rps=0.25, + **ONE_SECOND_LATENCIES, + outcome_counts={ + "http_429": 1, + "ok": 2, + "http_5xx": 1, + "http_4xx": 1, + "timeout": 1, + }, + status_counts={"429": 1, "200": 2, "503": 1, "404": 1, "0": 1}, + ), + } diff --git a/tests/unit/test_migration_contract_artifacts.py b/tests/unit/test_migration_contract_artifacts.py index ab6b199e7..d08090feb 100644 --- a/tests/unit/test_migration_contract_artifacts.py +++ b/tests/unit/test_migration_contract_artifacts.py @@ -144,3 +144,56 @@ def test_user_contracts_require_a_signed_in_or_signed_out_credential(): ("Authorization", "PolicyEngine-Unauthenticated-User"): 1, } assert all("user_id=" not in request["path"] for request in user_requests) + + +def _contract_request( + path: str, status: int, fields: list[str], *, route_group: str = "metadata" +) -> dict[str, object]: + return { + "method": "GET", + "path": path, + "route_group": route_group, + "expected_status": status, + "stable_response_fields": fields, + } + + +def test_workflow_guard_reports_each_request_problem_in_order() -> None: + payload = { + "route_groups": [{"name": "metadata"}], + "workflows": [ + { + "name": "read", + "current_contract": "api_v1_compatible", + "future_owner_pr": "PR 9", + "requests": [ + _contract_request("/us/metadata", 200, ["result"]), + _contract_request("/us/deleted", 204, []), + _contract_request("/us/empty", 200, []), + _contract_request( + "us/no-slash", 201, ["result"], route_group="unknown" + ), + _contract_request("/us/teapot", 418, ["result"]), + _contract_request("/us/metadata", 202, ["result"]), + ], + }, + { + "name": "read", + "current_contract": "legacy", + "future_owner_pr": "", + "requests": [], + }, + ], + } + + assert migration_contracts._check_workflows(payload) == [ + "duplicate workflow: 'read'", + "read GET /us/empty: stable_response_fields is required", + "read GET us/no-slash: unknown route_group 'unknown'", + "read GET us/no-slash: path must start with /", + "read GET /us/teapot: unexpected status 418", + "read: current_contract is not recognized", + "read: future_owner_pr is required", + "read: at least one request is required", + "duplicate contract request: 'GET /us/metadata'", + ] diff --git a/tests/unit/test_migration_flags.py b/tests/unit/test_migration_flags.py index 38cf15e6b..e110da879 100644 --- a/tests/unit/test_migration_flags.py +++ b/tests/unit/test_migration_flags.py @@ -1,6 +1,6 @@ import pytest -import policyengine_api.migration_flags as migration_flags +from policyengine_api import migration_flags from policyengine_api.migration_flags import ( get_migration_context, infer_route_group, diff --git a/tests/unit/test_observability_collector_verifier.py b/tests/unit/test_observability_collector_verifier.py index bda1d268f..437c0706b 100644 --- a/tests/unit/test_observability_collector_verifier.py +++ b/tests/unit/test_observability_collector_verifier.py @@ -1,14 +1,23 @@ from __future__ import annotations import importlib.util +import io from contextlib import nullcontext +from email.message import Message from pathlib import Path from types import SimpleNamespace +from typing import TYPE_CHECKING, Literal +from urllib.error import HTTPError from urllib.parse import parse_qs, urlparse import grpc import pytest +if TYPE_CHECKING: + from collections.abc import Mapping + from types import ModuleType + from urllib.request import Request + ROOT = Path(__file__).parents[2] VERIFIER_PATH = ROOT / ".github" / "scripts" / "verify_observability_collector.py" ACCESS_TOKEN = "access-token" # noqa: S105 # a fake test token @@ -76,12 +85,15 @@ def Export(self, request, *, metadata, timeout): # noqa: N802 # mirrors the gR lambda **kwargs: delivery.update(kwargs), ) - probe = verifier.verify( - "https://collector.example", - "identity-token", + readback = verifier.CloudReadback( project_id="observability-project", metric_location="us-central1", access_token=ACCESS_TOKEN, + ) + probe = verifier.verify( + "https://collector.example", + "identity-token", + readback=readback, timeout_seconds=3.0, delivery_timeout_seconds=9.0, ) @@ -114,12 +126,16 @@ def Export(self, request, *, metadata, timeout): # noqa: N802 # mirrors the gR == "collector log rejection verification" ) assert delivery == { - "project_id": "observability-project", - "trace_id": probe.trace_id, - "verification_id": probe.verification_id, - "metric_location": "us-central1", - "access_token": ACCESS_TOKEN, - "started_at": probe.started_at, + "readback": verifier.CloudReadback( + project_id="observability-project", + metric_location="us-central1", + access_token=ACCESS_TOKEN, + ), + "probe": verifier.VerificationProbe( + verification_id=probe.verification_id, + trace_id=probe.trace_id, + started_at=probe.started_at, + ), "timeout_seconds": 9.0, } @@ -149,9 +165,11 @@ def Export(self, *_args, **_kwargs): # noqa: N802 # mirrors the gRPC stub meth verifier.verify( "https://collector.example", "identity-token", - project_id="observability-project", - metric_location="us-central1", - access_token=ACCESS_TOKEN, + readback=verifier.CloudReadback( + project_id="observability-project", + metric_location="us-central1", + access_token=ACCESS_TOKEN, + ), ) @@ -215,3 +233,190 @@ def test_authority_rejects_non_https_or_hostless_endpoints(endpoint: str) -> Non with pytest.raises(ValueError, match="HTTPS URL"): verifier._authority(endpoint) + + +class _JsonResponse: + def __init__(self, body: bytes) -> None: + self._body = io.BytesIO(body) + + def __enter__(self) -> io.BytesIO: + return self._body + + def __exit__(self, *_exc_info: object) -> Literal[False]: + return False + + +def test_get_json_sends_the_access_token_and_decodes_the_body( + monkeypatch: pytest.MonkeyPatch, +) -> None: + verifier = _load_verifier() + requests: list[tuple[str, Mapping[str, str], float]] = [] + + def fake_urlopen(request: Request, timeout: float) -> _JsonResponse: + requests.append((request.full_url, request.headers, timeout)) + return _JsonResponse(b'{"traceId": "abc"}') + + monkeypatch.setattr(verifier, "urlopen", fake_urlopen) + + assert verifier._get_json("https://cloud.example/x", ACCESS_TOKEN) == { + "traceId": "abc" + } + assert requests == [ + ("https://cloud.example/x", {"Authorization": f"Bearer {ACCESS_TOKEN}"}, 15) + ] + + +def test_get_json_treats_not_found_as_missing(monkeypatch: pytest.MonkeyPatch) -> None: + verifier = _load_verifier() + + def fake_urlopen(request: Request, timeout: float) -> _JsonResponse: # noqa: ARG001 # mirrors urlopen's signature + raise HTTPError(request.full_url, 404, "Not Found", Message(), io.BytesIO(b"")) + + monkeypatch.setattr(verifier, "urlopen", fake_urlopen) + + assert verifier._get_json("https://cloud.example/x", ACCESS_TOKEN) is None + + +def test_get_json_reports_other_http_errors_with_their_body( + monkeypatch: pytest.MonkeyPatch, +) -> None: + verifier = _load_verifier() + + def fake_urlopen(request: Request, timeout: float) -> _JsonResponse: # noqa: ARG001 # mirrors urlopen's signature + raise HTTPError( + request.full_url, + 403, + "Forbidden", + Message(), + io.BytesIO(b"permission denied"), + ) + + monkeypatch.setattr(verifier, "urlopen", fake_urlopen) + + with pytest.raises(RuntimeError) as error: + verifier._get_json("https://cloud.example/x", ACCESS_TOKEN) + + assert str(error.value) == ( + "Google Cloud verification request failed (403): permission denied" + ) + assert isinstance(error.value.__cause__, HTTPError) + + +class _FakeClock: + """Monotonic time that only moves when the code under test sleeps.""" + + def __init__(self) -> None: + self.now = 100.0 + self.sleeps: list[float] = [] + + def monotonic(self) -> float: + return self.now + + def sleep(self, seconds: float) -> None: + self.sleeps.append(seconds) + self.now += seconds + + +def _stub_delivery_checks( + monkeypatch: pytest.MonkeyPatch, + verifier: ModuleType, + *, + trace_results: list[bool], + metric_results: list[bool], +) -> tuple[dict[str, list[tuple[object, ...]]], _FakeClock]: + calls: dict[str, list[tuple]] = {"trace": [], "metric": []} + trace_answers = iter(trace_results) + metric_answers = iter(metric_results) + + def trace_available(*args: object) -> bool: + calls["trace"].append(args) + return next(trace_answers) + + def metric_available(*args: object) -> bool: + calls["metric"].append(args) + return next(metric_answers) + + monkeypatch.setattr(verifier, "_trace_available", trace_available) + monkeypatch.setattr(verifier, "_metric_available", metric_available) + clock = _FakeClock() + monkeypatch.setattr(verifier, "time", clock) + return calls, clock + + +def _wait_for_test_delivery(verifier: ModuleType, *, timeout_seconds: float) -> None: + verifier._wait_for_delivery( + readback=verifier.CloudReadback( + project_id="observability-project", + metric_location="us-central1", + access_token=ACCESS_TOKEN, + ), + probe=verifier.VerificationProbe( + verification_id="collector-verify-123", + trace_id="trace-123", + started_at=1_700_000_000.0, + ), + timeout_seconds=timeout_seconds, + ) + + +TRACE_CHECK = ("observability-project", "trace-123", ACCESS_TOKEN) +METRIC_CHECK = ( + "observability-project", + "collector-verify-123", + "us-central1", + ACCESS_TOKEN, + 1_700_000_000.0, +) + + +def test_delivery_wait_returns_once_both_signals_are_stored( + monkeypatch: pytest.MonkeyPatch, +) -> None: + verifier = _load_verifier() + calls, clock = _stub_delivery_checks( + monkeypatch, + verifier, + trace_results=[True], + metric_results=[False, False, True], + ) + + _wait_for_test_delivery(verifier, timeout_seconds=12.0) + + assert calls == {"trace": [TRACE_CHECK], "metric": [METRIC_CHECK] * 3} + assert clock.sleeps == [5.0, 5.0] + + +def test_delivery_wait_names_the_signals_still_missing_at_the_deadline( + monkeypatch: pytest.MonkeyPatch, +) -> None: + verifier = _load_verifier() + calls, clock = _stub_delivery_checks( + monkeypatch, + verifier, + trace_results=[False, False, False, True], + metric_results=[False] * 4, + ) + + with pytest.raises( + RuntimeError, + match=r"^collector verification data was not stored: metric$", + ): + _wait_for_test_delivery(verifier, timeout_seconds=12.0) + + assert calls == {"trace": [TRACE_CHECK] * 4, "metric": [METRIC_CHECK] * 4} + assert clock.sleeps == [5.0, 5.0, 2.0] + + +def test_delivery_wait_lists_both_missing_signals( + monkeypatch: pytest.MonkeyPatch, +) -> None: + verifier = _load_verifier() + _stub_delivery_checks( + monkeypatch, verifier, trace_results=[False], metric_results=[False] + ) + + with pytest.raises( + RuntimeError, + match=r"^collector verification data was not stored: trace, metric$", + ): + _wait_for_test_delivery(verifier, timeout_seconds=0.0) diff --git a/tests/unit/test_openapi_spm.py b/tests/unit/test_openapi_spm.py index dcf9d47b1..6bd504239 100644 --- a/tests/unit/test_openapi_spm.py +++ b/tests/unit/test_openapi_spm.py @@ -1,6 +1,7 @@ """The served legacy specification describes the canonical SPM HTTP contract.""" import json +from typing import TYPE_CHECKING from flask import Flask @@ -18,6 +19,9 @@ SPMSelection, ) +if TYPE_CHECKING: + from pydantic import BaseModel + def test_economy_query_and_receipt_schemas_match_public_http_contract(): spec = load_specification() @@ -104,25 +108,15 @@ def test_household_specification_exposes_creation_without_content_update(): assert "label remains editable" not in simulation_description -def test_spm_documentation_uses_public_models_and_actual_route_envelopes(): - app = Flask(__name__) - app.register_blueprint(system_bp) - response = app.test_client().get("/specification") - assert response.status_code == 200 - spec = response.get_json() - assert spec["openapi"] == "3.0.0" - paths = spec["paths"] - schemas = spec["components"]["schemas"] - selection_ref = {"$ref": "#/components/schemas/SPMSelection"} - provenance_ref = {"$ref": "#/components/schemas/SPMProvenance"} - - for name, model in { +def _assert_spm_models_match_component_schemas(schemas: dict) -> None: + models: dict[str, type[BaseModel]] = { "SPMSelection": SPMSelection, "SPMRuntimeVersions": SPMRuntimeVersions, "SPMProvenance": SPMProvenance, "SPMExecutionProvenance": SPMExecutionProvenance, "SPMComparisonProvenance": SPMComparisonProvenance, - }.items(): + } + for name, model in models.items(): schema = schemas[name] expected_fields = { field.alias or field_name @@ -132,6 +126,8 @@ def test_spm_documentation_uses_public_models_and_actual_route_envelopes(): assert schema["additionalProperties"] is False assert '"type": "null"' not in json.dumps(schema) + +def _assert_selection_and_provenance_schemas(schemas: dict) -> None: selection = schemas["SPMSelection"]["properties"] assert selection["geography_kind"]["enum"] == ["county", "national", "metro"] assert all("default" not in field for field in selection.values()) @@ -182,6 +178,10 @@ def test_spm_documentation_uses_public_models_and_actual_route_envelopes(): } assert not removed_rich_fields.intersection(provenance["properties"]) + +def _assert_spm_route_envelopes( + paths: dict, selection_ref: dict, provenance_ref: dict +) -> None: for route in ("calculate", "calculate-full"): operation = paths[f"/{{country_id}}/{route}"]["post"] request = operation["requestBody"]["content"]["application/json"]["schema"] @@ -228,6 +228,8 @@ def test_spm_documentation_uses_public_models_and_actual_route_envelopes(): assert result["properties"]["spm"] == selection_ref assert result["properties"]["household_json"]["type"] == "object" + +def _assert_spm_validation_error_response(spec: dict) -> None: error = spec["components"]["responses"]["SPMValidationError"]["content"][ "application/json" ]["schema"] @@ -236,3 +238,21 @@ def test_spm_documentation_uses_public_models_and_actual_route_envelopes(): assert error["properties"]["errors"]["items"]["required"] == ["message"] assert "code" in error["properties"]["errors"]["items"]["properties"] assert error["properties"]["result"]["nullable"] is True + + +def test_spm_documentation_uses_public_models_and_actual_route_envelopes(): + app = Flask(__name__) + app.register_blueprint(system_bp) + response = app.test_client().get("/specification") + assert response.status_code == 200 + spec = response.get_json() + assert spec["openapi"] == "3.0.0" + paths = spec["paths"] + schemas = spec["components"]["schemas"] + selection_ref = {"$ref": "#/components/schemas/SPMSelection"} + provenance_ref = {"$ref": "#/components/schemas/SPMProvenance"} + + _assert_spm_models_match_component_schemas(schemas) + _assert_selection_and_provenance_schemas(schemas) + _assert_spm_route_envelopes(paths, selection_ref, provenance_ref) + _assert_spm_validation_error_response(spec) diff --git a/tests/unit/test_smoke_test_local_boot.py b/tests/unit/test_smoke_test_local_boot.py new file mode 100644 index 000000000..efd6f2f93 --- /dev/null +++ b/tests/unit/test_smoke_test_local_boot.py @@ -0,0 +1,302 @@ +"""Characterization tests for the local app boot smoke test.""" + +import importlib.metadata +import subprocess +import sys +import tempfile +import urllib.error +from collections.abc import Callable +from email.message import Message +from pathlib import Path +from typing import Any + +import pytest + +from scripts import smoke_test_local_boot as smoke + +PORT = 51234 + + +class _FakeClock: + """Wall-clock time that only moves when the smoke test sleeps.""" + + def __init__(self) -> None: + self.now = 1_000.0 + self.sleeps: list[float] = [] + + def time(self) -> float: + return self.now + + def sleep(self, seconds: float) -> None: + self.sleeps.append(seconds) + self.now += seconds + + +class _FakeApp: + """Stands in for the Flask child process and records how it is managed.""" + + def __init__( + self, *, exits_after_polls: int | None = None, ignores_terminate: bool = False + ) -> None: + self.exits_after_polls = exits_after_polls + self.ignores_terminate = ignores_terminate + self.returncode: int | None = None + self.polls = 0 + self.calls: list[tuple[object, ...]] = [] + self.popen_args: tuple[list[str], dict[str, Any]] | None = None + + def popen(self, command: list[str], **kwargs: Any) -> "_FakeApp": + self.popen_args = (command, kwargs) + kwargs["stdout"].write("app stdout\n") + kwargs["stderr"].write("app stderr\n") + return self + + def poll(self) -> int | None: + self.polls += 1 + if self.exits_after_polls is not None and self.polls > self.exits_after_polls: + self.returncode = 3 + return self.returncode + + def terminate(self) -> None: + self.calls.append(("terminate",)) + + def wait(self, timeout: float) -> int: + self.calls.append(("wait", timeout)) + if self.ignores_terminate and ("kill",) not in self.calls: + command_name = "flask" + raise subprocess.TimeoutExpired(command_name, timeout) + if self.returncode is None: + self.returncode = -15 + return self.returncode + + def kill(self) -> None: + self.calls.append(("kill",)) + + +FetchOutcome = tuple[int, str] | Exception +BootRunner = Callable[[_FakeApp, list[FetchOutcome]], tuple[int, list[str], _FakeClock]] + + +@pytest.fixture +def boot(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> BootRunner: + """Patch the clock, port, version and temp directory; return a runner.""" + clock = _FakeClock() + monkeypatch.setattr(smoke, "time", clock) + monkeypatch.setattr(smoke, "find_free_port", lambda: PORT) + monkeypatch.setattr(tempfile, "tempdir", str(tmp_path)) + monkeypatch.setattr(importlib.metadata, "version", lambda _name: "9.9.9") + monkeypatch.delenv("PYTHONUNBUFFERED", raising=False) + + def run( + app: _FakeApp, responses: list[FetchOutcome] + ) -> tuple[int, list[str], _FakeClock]: + fetched: list[str] = [] + + def fake_fetch(url: str) -> tuple[int, str]: + fetched.append(url) + response = responses.pop(0) + if isinstance(response, Exception): + raise response + return response + + monkeypatch.setattr(smoke.subprocess, "Popen", app.popen) + monkeypatch.setattr(smoke, "fetch", fake_fetch) + return smoke.main(), fetched, clock + + return run + + +def _log_paths(app: _FakeApp) -> tuple[str, str]: + assert app.popen_args is not None + _command, kwargs = app.popen_args + return kwargs["stdout"].name, kwargs["stderr"].name + + +def _header(app: _FakeApp) -> list[str]: + stdout_path, stderr_path = _log_paths(app) + return [ + f"repo: {smoke.REPO_ROOT}", + f"python: {sys.executable}", + "policyengine: 9.9.9", + f"port: {PORT}", + f"stdout log: {stdout_path}", + f"stderr log: {stderr_path}", + ] + + +CAPTURED_OUTPUT = [ + "", + "=== stdout ===", + "app stdout", + "", + "=== stderr ===", + "app stderr", +] +LIVENESS_URL = f"http://127.0.0.1:{PORT}/liveness-check" +READINESS_URL = f"http://127.0.0.1:{PORT}/readiness-check" + + +def test_boot_passes_when_both_health_checks_answer_ok( + boot: BootRunner, capsys: pytest.CaptureFixture[str] +) -> None: + app = _FakeApp() + + status, fetched, clock = boot(app, [(200, "OK\n"), (200, "OK\n"), (200, " OK ")]) + + assert status == 0 + assert capsys.readouterr().out.splitlines() == [ + *_header(app), + "", + "Boot smoke test reached a live app. Probing health endpoints:", + "/liveness-check: 200 OK", + "/readiness-check: 200 OK", + "", + "Boot smoke test passed.", + ] + assert fetched == [LIVENESS_URL, LIVENESS_URL, READINESS_URL] + assert clock.sleeps == [] + assert app.calls == [("terminate",), ("wait", 5)] + assert app.popen_args is not None + command, kwargs = app.popen_args + assert command == [ + sys.executable, + "-m", + "flask", + "--app", + "policyengine_api.api", + "run", + "--without-threads", + "--no-reload", + "--port", + str(PORT), + ] + assert kwargs["cwd"] == smoke.REPO_ROOT + assert kwargs["text"] is True + assert kwargs["env"]["FLASK_DEBUG"] == "1" + assert kwargs["env"]["PYTHONUNBUFFERED"] == "1" + assert kwargs["stdout"].closed + assert kwargs["stderr"].closed + + +def test_boot_retries_until_the_app_is_live( + boot: BootRunner, capsys: pytest.CaptureFixture[str] +) -> None: + app = _FakeApp() + + status, fetched, clock = boot( + app, + [ + urllib.error.URLError("refused"), + TimeoutError(), + ConnectionError(), + (503, "starting"), + (200, "not ok"), + (200, "OK"), + (200, "OK"), + (200, "OK"), + ], + ) + + assert status == 0 + assert fetched == [LIVENESS_URL] * 6 + [LIVENESS_URL, READINESS_URL] + assert clock.sleeps == [0.5] * 5 + assert "Boot smoke test passed." in capsys.readouterr().out + + +def test_boot_fails_when_the_app_exits_before_it_is_live( + boot: BootRunner, capsys: pytest.CaptureFixture[str] +) -> None: + app = _FakeApp(exits_after_polls=1) + + status, fetched, clock = boot(app, [urllib.error.URLError("refused")]) + + assert status == 1 + assert capsys.readouterr().out.splitlines() == [ + *_header(app), + "", + "Boot smoke test failed: app did not become ready.", + *CAPTURED_OUTPUT, + ] + assert fetched == [LIVENESS_URL] + assert clock.sleeps == [0.5] + assert app.calls == [("wait", 5)] + + +def test_boot_stops_an_app_that_never_becomes_live( + boot: BootRunner, capsys: pytest.CaptureFixture[str] +) -> None: + app = _FakeApp() + responses: list[FetchOutcome] = [urllib.error.URLError("refused")] * 1200 + + status, fetched, clock = boot(app, responses) + + assert status == 1 + assert capsys.readouterr().out.splitlines() == [ + *_header(app), + "", + "Boot smoke test failed: app did not become ready.", + *CAPTURED_OUTPUT, + ] + assert len(fetched) == 1200 + assert clock.sleeps == [0.5] * 1200 + assert app.calls == [("terminate",), ("wait", 5)] + + +def test_boot_fails_when_a_health_probe_is_not_ok( + boot: BootRunner, capsys: pytest.CaptureFixture[str] +) -> None: + app = _FakeApp() + + status, fetched, _clock = boot( + app, [(200, "OK"), (200, "OK"), (503, " starting \n")] + ) + + assert status == 1 + assert capsys.readouterr().out.splitlines() == [ + *_header(app), + "", + "Boot smoke test reached a live app. Probing health endpoints:", + "/liveness-check: 200 OK", + "/readiness-check: 503 starting", + "", + "Health endpoint probe failed.", + *CAPTURED_OUTPUT, + ] + assert fetched == [LIVENESS_URL, LIVENESS_URL, READINESS_URL] + assert app.calls == [("terminate",), ("wait", 5)] + + +def test_boot_kills_an_app_that_ignores_terminate(boot: BootRunner) -> None: + app = _FakeApp(ignores_terminate=True) + + status, _fetched, _clock = boot(app, [(200, "OK"), (200, "OK"), (200, "OK")]) + + assert status == 0 + assert app.calls == [("terminate",), ("wait", 5), ("kill",), ("wait", 5)] + + +def test_health_probe_errors_propagate_after_stopping_the_app( + boot: BootRunner, +) -> None: + app = _FakeApp() + error = urllib.error.HTTPError(READINESS_URL, 500, "boom", Message(), None) + + with pytest.raises(urllib.error.HTTPError): + boot(app, [(200, "OK"), (200, "OK"), error]) + + assert app.calls == [("terminate",), ("wait", 5)] + + +def test_boot_reports_a_missing_policyengine_package( + boot: BootRunner, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + def missing(_name: str) -> str: + raise importlib.metadata.PackageNotFoundError + + monkeypatch.setattr(importlib.metadata, "version", missing) + + boot(_FakeApp(), [(200, "OK"), (200, "OK"), (200, "OK")]) + + assert "policyengine: not installed" in capsys.readouterr().out.splitlines() diff --git a/tests/unit/v2/test_catalog_extraction_failures.py b/tests/unit/v2/test_catalog_extraction_failures.py index 83c10d91a..fa46c3f80 100644 --- a/tests/unit/v2/test_catalog_extraction_failures.py +++ b/tests/unit/v2/test_catalog_extraction_failures.py @@ -184,6 +184,8 @@ class _Colour(Enum): class _Pair(tuple, Enum): + __slots__ = () + FIRST = (1, 2)