diff --git a/.github/scripts/update_policyengine_package.py b/.github/scripts/update_policyengine_package.py index 089ec6f08..47af06036 100644 --- a/.github/scripts/update_policyengine_package.py +++ b/.github/scripts/update_policyengine_package.py @@ -83,8 +83,10 @@ def update_requirements( ), } - for location, dependencies in requirements.items(): - dependencies = dependencies if isinstance(dependencies, list) else [] + for location, declared_dependencies in requirements.items(): + dependencies = ( + declared_dependencies if isinstance(declared_dependencies, list) else [] + ) matches = [ dependency for dependency in dependencies @@ -140,7 +142,7 @@ def update_requirements( ) as temporary: temporary.write(text.replace(old_pin, new_pin)) temporary_path = temporary.name - os.replace(temporary_path, pyproject) + Path(temporary_path).replace(pyproject) def find_issue(title: str) -> None: diff --git a/AGENTS.md b/AGENTS.md index 9c4e05a19..909056a2d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -143,10 +143,11 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `sim.quality.every-project-tested`: Every project and library with unit tests runs them in the PR test matrix. (doc-only, not checked: follow it yourself) - `sim.quality.ruff-format-src`: Source code under projects/*/src and libs/*/src is formatted with ruff format. (blocking: ruff format) - `sim.quality.ruff-format-tests`: Test code is formatted with ruff format too; CI currently checks only src directories. (doc-only, not checked: follow it yourself) -- `sim.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (A, ANN, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, SLF, T20, TC, TD, TRY, UP, W)) +- `sim.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in ruff.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)) - `sim.quality.runtime-evaluated-models-resolve`: Every Pydantic model is runtime-evaluated for ruff: one of its bases, read through the imports of the model's own module, is listed in ruff.toml's runtime-evaluated-base-classes or is a runtime-evaluated class in that same module; and every Pydantic model resolves its annotations once its package is imported. (blocking: pytest `projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py`, `projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py`) - `sim.quality.tests-pass`: Each project's unit tests pass in its own locked environment. (blocking: CI job `test`) - `sim.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/**`) +- `sim.quality.unittest-ruff-exemptions`: The shared rules tool's tests use unittest, because the tool needs only the standard library, so ruff's pytest-style rules (PT) don't apply to them. (blocking: ruff per-file ignore of PT for `scripts/rules/tests/**`) ### CI workflows and scripts diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 63348c329..ecce4b1bf 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -4,6 +4,7 @@ - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md changed: + - Enabled ruff's argument, idiom and pytest-style rules (ARG, ASYNC, B, C4, FBT, PD, PERF, PGH, PLW, PT, PTH, RUF, SIM); unused arguments are removed, underscored or explained, FastAPI auth dependencies use Annotated, re-raised errors keep their cause, and boolean options of in-repo helpers are keyword-only; OpenAPI documents are unchanged, and the shared rules tool's unittest tests are exempt from pytest-style rules - Enabled ruff's structure and import rules (INP, PLC, SLF, N, A, FAST, E); long lines are wrapped, deferred imports that Modal images, test seams, import cycles or heavy dependencies need carry a reason, script directories are declared to ruff as namespace packages, four internal exceptions gained an Error suffix, and published field names, OpenAPI documents and logged error types are unchanged; tests are also exempt from SLF001, INP001 and PLC0415 - Enabled ruff's security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE); asserts outside tests became explicit checks, command-line scripts keep their printed output, test subprocess calls go through helpers, and every deliberate exception carries a reasoned suppression; the gateway's auth misconfiguration log no longer repeats its message, and the deployed-service tests share the unit-test ruff exemptions - Enabled ruff's exception rules (TRY, EM, BLE); exception messages are built before the raise, raises inside try blocks moved into helpers, and every deliberate catch-all or traceback-free error log carries a reasoned suppression; message text is unchanged, and error types change only in an operator script's two type checks diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 28a8a1639..ff04dfa53 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 35 | 1 | 0 | 14 | 50 | +| mechanical | 36 | 1 | 0 | 14 | 51 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 36 of 50 rules. +**Blocking:** 37 of 51 rules. ## Gaps @@ -246,10 +246,11 @@ flowchart LR | `sim.quality.every-project-tested` | Every project and library with unit tests runs them in the PR test matrix. | mechanical | doc-only | no | nothing | — | — | | `sim.quality.ruff-format-src` | Source code under projects/*/src and libs/*/src is formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | | `sim.quality.ruff-format-tests` | Test code is formatted with ruff format too; CI currently checks only src directories. | mechanical | doc-only | no | nothing | — | — | -| `sim.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in ruff.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, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, SLF, T20, TC, TD, TRY, UP, W) | `pr.yml:lint` | — | +| `sim.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in ruff.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` | — | | `sim.quality.runtime-evaluated-models-resolve` | Every Pydantic model is runtime-evaluated for ruff: one of its bases, read through the imports of the model's own module, is listed in ruff.toml's runtime-evaluated-base-classes or is a runtime-evaluated class in that same module; and every Pydantic model resolves its annotations once its package is imported. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py`, `projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py` | `pr.yml:test` | yes | | `sim.quality.tests-pass` | Each project's unit tests pass in its own locked environment. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | | `sim.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` | — | +| `sim.quality.unittest-ruff-exemptions` | The shared rules tool's tests use unittest, because the tool needs only the standard library, so ruff's pytest-style rules (PT) don't apply to them. | mechanical | enforced | yes | ruff per-file ignore of PT for `scripts/rules/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR @@ -262,7 +263,7 @@ flowchart LR e_ruff_format["ruff format"] r_sim_quality_ruff_format_tests("sim.quality.ruff-format-tests (doc-only)") r_sim_quality_ruff_lint("sim.quality.ruff-lint (enforced)") - e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TRY__UP__W_["ruff check (A, ANN, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, 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_["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)"] r_sim_quality_runtime_evaluated_models_resolve("sim.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py["pytest projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py, projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] @@ -270,20 +271,24 @@ flowchart LR e_CI_job_test["CI job test"] r_sim_quality_tests_ruff_exemptions("sim.quality.tests-ruff-exemptions (enforced)") e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____tests___["ruff per-file ignore of ANN, D1, INP001, PLC0415, PLR2004, S101, SLF001 for **/tests/**"] + r_sim_quality_unittest_ruff_exemptions("sim.quality.unittest-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___["ruff per-file ignore of PT for scripts/rules/tests/**"] r_sim_quality_deployed_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____authenticated_tests___ e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____authenticated_tests___ --> j_CI_pr_yml_lint r_sim_quality_every_project_tested -.-> gap_unenforced r_sim_quality_ruff_format_src --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint r_sim_quality_ruff_format_tests -.-> gap_unenforced - r_sim_quality_ruff_lint --> e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TRY__UP__W_ - e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TRY__UP__W_ --> j_CI_pr_yml_lint + r_sim_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_sim_quality_runtime_evaluated_models_resolve --> e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_sim_quality_tests_pass --> e_CI_job_test e_CI_job_test --> j_CI_pr_yml_test r_sim_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____tests___ e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____tests___ --> j_CI_pr_yml_lint + r_sim_quality_unittest_ruff_exemptions --> e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___ + e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___ --> j_CI_pr_yml_lint ``` ## CI workflows and scripts diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py b/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py index a1356412c..5cda13d03 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py @@ -1,6 +1,7 @@ """FastAPI dependency that verifies RS256 bearer JWTs against an issuer's JWKS.""" import logging +from typing import Annotated import jwt from fastapi import Depends, HTTPException, status @@ -20,7 +21,7 @@ class JWTDecoder: or yields None when ``auto_error`` is False. """ - def __init__(self, issuer: str, audience: str, auto_error: bool = True) -> None: + def __init__(self, issuer: str, audience: str, *, auto_error: bool = True) -> None: """Store the expected claims and create a JWKS client for the issuer. Keys are fetched from ``{issuer}.well-known/jwks.json``, so ``issuer`` @@ -34,9 +35,9 @@ def __init__(self, issuer: str, audience: str, auto_error: bool = True) -> None: def __call__( self, - token: HTTPAuthorizationCredentials | None = Depends( - HTTPBearer(auto_error=False) - ), + token: Annotated[ + HTTPAuthorizationCredentials | None, Depends(HTTPBearer(auto_error=False)) + ], ) -> dict[str, str] | None: """Return the verified token's claims. @@ -64,10 +65,10 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str ).key except jwt.exceptions.PyJWKClientError as error: LOG.error("Unable to get public key for JWT validation: %s", error) # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs - raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) from error except jwt.exceptions.DecodeError as error: LOG.error("Unable to get public key for JWT validation: %s", error) # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs - raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) from error try: payload = jwt.decode( @@ -77,7 +78,8 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str audience=self.audience, issuer=self.issuer, ) - except Exception as error: # noqa: BLE001 # any decode or claim-validation failure must become a 403, never a 500 + # Any decode or claim-validation failure must become a 403, never a 500. + except Exception as error: LOG.info("Invalid bearer token: %s", error) - raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) from error return payload diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/health/__init__.py b/libs/policyengine-fastapi/src/policyengine_fastapi/health/__init__.py index ecf94141d..b5304d599 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/health/__init__.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/health/__init__.py @@ -41,7 +41,7 @@ def __init__(self, name: str, probes: dict[str, HealthProbe]) -> None: def report(self) -> SystemStatus: """Run every probe; the system is healthy only if all of its probes are.""" - detail = [self.probes[key]() for key in self.probes.keys()] + detail = [self.probes[key]() for key in self.probes] first_unhealthy = next((d for d in detail if d.healthy is not True), None) return SystemStatus( name=self.name, healthy=first_unhealthy is None, detail=detail @@ -67,7 +67,7 @@ def register(self, reporter: HealthSystemReporter) -> None: def report(self) -> HealthStatus: """Report every system; the service is healthy only if all systems are.""" - system_status = [self.systems[key].report() for key in self.systems.keys()] + system_status = [self.systems[key].report() for key in self.systems] first_unhealthy = next( (s for s in system_status if s.healthy is not True), None ) diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/observability/emitters.py b/libs/policyengine-fastapi/src/policyengine_fastapi/observability/emitters.py index 5a65a8e94..ac8a1f7ec 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/observability/emitters.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/observability/emitters.py @@ -105,7 +105,7 @@ def emit_histogram( def record_artifact_manifest(self, manifest: TracerArtifactManifest) -> None: """Discard the manifest.""" - def span(self, name: str, attributes: Mapping[str, Any] | None = None) -> NoOpSpan: + def span(self, name: str, attributes: Mapping[str, Any] | None = None) -> NoOpSpan: # noqa: ARG002 # implements the Observability protocol's span() """Return a span that records nothing.""" return NoOpSpan() diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py index 109ce9583..6c529476a 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py @@ -36,7 +36,7 @@ def filter(self, record: logging.LogRecord) -> bool | logging.LogRecord: """Qualify the record's ``otelTraceID``, if set, then apply the base filter.""" if not hasattr(record, "otelTraceID"): return super().filter(record) - record.otelTraceID = f"projects/{self.project}/traces/{record.otelTraceID}" # type: ignore + record.otelTraceID = f"projects/{self.project}/traces/{record.otelTraceID}" # type: ignore[attr-defined] return super().filter(record) diff --git a/libs/policyengine-fastapi/tests/test_observability.py b/libs/policyengine-fastapi/tests/test_observability.py index c6de6efd3..6cfa96998 100644 --- a/libs/policyengine-fastapi/tests/test_observability.py +++ b/libs/policyengine-fastapi/tests/test_observability.py @@ -1,5 +1,6 @@ from datetime import UTC, datetime +import pytest from policyengine_fastapi.observability import ( NoOpObservability, ObservabilityConfig, @@ -30,13 +31,8 @@ def test_parse_header_value_pairs__supports_commas_and_newlines(): def test_parse_header_value_pairs__raises_for_invalid_pair(): - try: + with pytest.raises(ValueError, match="key=value"): parse_header_value_pairs("Authorization") - except ValueError as error: - assert "key=value" in str(error) - else: - msg = "Expected invalid OTLP header parsing to fail" - raise AssertionError(msg) def test_observability_config_disabled__returns_disabled_defaults(): @@ -108,7 +104,7 @@ def test_contract_models__serialize_expected_shapes(): def test_contract_models__reject_extra_fields(): - try: + with pytest.raises(ValidationError, match="unexpected"): SimulationLifecycleEvent( event_name="simulation.stage.completed", stage=SimulationStage.WORKER_COMPLETED, @@ -118,11 +114,6 @@ def test_contract_models__reject_extra_fields(): observability_id="run-789", unexpected=True, ) - except ValidationError as error: - assert "unexpected" in str(error) - else: - msg = "Expected extra field validation to fail" - raise AssertionError(msg) def test_noop_observability__accepts_calls_without_side_effects(): diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py index 9879d50d3..257eb7221 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py @@ -95,9 +95,7 @@ def _policyengine_data_runtime_uri( package_version is not None and artifact_revision is not None and reference.revision == artifact_revision - ): - selected_revision = package_version - elif selected_revision is None: + ) or selected_revision is None: selected_revision = package_version if selected_revision is not None: diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py index e7f04efa4..09b1462bc 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py @@ -79,10 +79,9 @@ def serialize_selection( # noqa: ANN201 # a return annotation would become the @classmethod def validate_as_of(cls, value: str | None) -> str | None: """Require ``as_of`` to be a canonical ``YYYY-MM-DD`` date when given.""" - if value is not None: - if date.fromisoformat(value).isoformat() != value: - msg = "as_of must be an ISO calendar date (YYYY-MM-DD)" - raise ValueError(msg) + if value is not None and date.fromisoformat(value).isoformat() != value: + msg = "as_of must be an ISO calendar date (YYYY-MM-DD)" + raise ValueError(msg) return value @model_validator(mode="after") diff --git a/libs/policyengine-simulation-contract/tests/conftest.py b/libs/policyengine-simulation-contract/tests/conftest.py index 714cb2153..22613935a 100644 --- a/libs/policyengine-simulation-contract/tests/conftest.py +++ b/libs/policyengine-simulation-contract/tests/conftest.py @@ -33,7 +33,7 @@ def mock_modal(monkeypatch): class MockModalDict: @staticmethod - def from_name(name: str, create_if_missing: bool = False): + def from_name(name: str, *, create_if_missing: bool = False): if create_if_missing and name not in mock_dicts: mock_dicts[name] = {} if name not in mock_dicts: diff --git a/libs/policyengine-simulation-contract/tests/test_budget_window_state.py b/libs/policyengine-simulation-contract/tests/test_budget_window_state.py index 1b89f77b2..2dbdf0a44 100644 --- a/libs/policyengine-simulation-contract/tests/test_budget_window_state.py +++ b/libs/policyengine-simulation-contract/tests/test_budget_window_state.py @@ -1,5 +1,6 @@ """Tests for budget-window batch state helpers.""" +import pytest from policyengine_simulation_contract.budget_window_state import ( build_batch_status_response, create_initial_batch_state, @@ -84,7 +85,8 @@ def test_build_batch_status_response_computes_progress_from_completed_years(): assert response.queued_years == ["2029"] -def test_batch_state_round_trips_through_modal_dict(mock_modal): +@pytest.mark.usefixtures("mock_modal") +def test_batch_state_round_trips_through_modal_dict(): request = BudgetWindowBatchRequest( country="us", region="state/ca", diff --git a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py index 336d229db..1b6a6efa4 100644 --- a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py +++ b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py @@ -72,7 +72,7 @@ def test_validate_hf_dataset_uri_uses_runtime_credential(monkeypatch): dataset_uri = "hf://policyengine/private-data/dataset.h5@release" seen = {} - def fake_fetch(repo_id, revision, token): + def fake_fetch(_repo_id, _revision, token): seen["token"] = token return {"siblings": [{"rfilename": "dataset.h5"}]} @@ -89,7 +89,7 @@ def test_validate_hf_dataset_uri_ignores_non_runtime_credential_names(monkeypatc dataset_uri = "hf://policyengine/private-data/dataset.h5@release" seen = {} - def fake_fetch(repo_id, revision, token): + def fake_fetch(_repo_id, _revision, token): seen["token"] = token return {"siblings": [{"rfilename": "dataset.h5"}]} @@ -106,7 +106,9 @@ def test_validate_hf_dataset_uri_rejects_revision_missing_artifact(monkeypatch): monkeypatch.setattr( hf_dataset, "_fetch_hf_dataset_revision", - lambda repo_id, revision, token: {"siblings": [{"rfilename": "other_file.h5"}]}, + lambda _repo_id, _revision, _token: { + "siblings": [{"rfilename": "other_file.h5"}] + }, ) with pytest.raises( diff --git a/libs/policyengine-simulation-contract/tests/test_spm_selection.py b/libs/policyengine-simulation-contract/tests/test_spm_selection.py index b73c38d0a..17b701aa0 100644 --- a/libs/policyengine-simulation-contract/tests/test_spm_selection.py +++ b/libs/policyengine-simulation-contract/tests/test_spm_selection.py @@ -56,7 +56,7 @@ def test_explicit_national_discards_default_metro_area(): @pytest.mark.parametrize( - "provenance,wrapper,model,accepted", + ("provenance", "wrapper", "model", "accepted"), [ ("legacy-country-dict", None, "1.500.0", True), ("legacy-country-route", None, "1.715.2", True), @@ -72,12 +72,12 @@ def test_explicit_national_discards_default_metro_area(): def test_legacy_allowance_requires_route_provenance_and_historical_model( provenance, wrapper, model, accepted ): - kwargs = dict( - capability=None, - policyengine_version=wrapper, - model_version=model, - route_provenance=provenance, - ) + kwargs = { + "capability": None, + "policyengine_version": wrapper, + "model_version": model, + "route_provenance": provenance, + } if accepted: assert resolve_spm_selection("us", None, **kwargs) is None else: @@ -114,7 +114,7 @@ def test_pinned_pre_canonical_wrapper_is_historical_without_a_stated_model( @pytest.mark.parametrize( - "wrapper,model", + ("wrapper", "model"), [ ("5.2.0", "1.824.7"), ("5.3.0", "1.824.7"), diff --git a/libs/policyengine-simulation-contract/tests/test_stage12_execution.py b/libs/policyengine-simulation-contract/tests/test_stage12_execution.py index dda655d46..6ac885892 100644 --- a/libs/policyengine-simulation-contract/tests/test_stage12_execution.py +++ b/libs/policyengine-simulation-contract/tests/test_stage12_execution.py @@ -28,6 +28,7 @@ UKLocalAuthorityBoundaryVersion, UKLocalAuthorityMetadata, ) +from pydantic import ValidationError if TYPE_CHECKING: from pydantic import JsonValue @@ -227,7 +228,7 @@ def test_aggregate_artifact_accepts_absent_spm_provenance(result) -> None: @pytest.mark.parametrize("provenance", [{}, "invalid", [], 0]) def test_aggregate_artifact_rejects_invalid_present_spm_provenance(provenance) -> None: - with pytest.raises(ValueError): + with pytest.raises(ValidationError): AggregateReportArtifactPayload( evaluation_id=UUID("00000000-0000-0000-0000-000000000001"), requested_aggregates=(ReportAggregate.BUDGET,), diff --git a/libs/policyengine-simulation-contract/tests/test_stage12_manifest.py b/libs/policyengine-simulation-contract/tests/test_stage12_manifest.py index ddf024aff..019bac88b 100644 --- a/libs/policyengine-simulation-contract/tests/test_stage12_manifest.py +++ b/libs/policyengine-simulation-contract/tests/test_stage12_manifest.py @@ -174,12 +174,16 @@ def test_v1_and_v2_publication_and_removal_are_bidirectionally_isolated() -> Non def test_manifest_rejects_incomplete_or_unvalidated_worker() -> None: value = _worker().model_dump(mode="json") value.pop("report_coordinator_callable") - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match=r"report_coordinator_callable\n Field required" + ): V2WorkerVersion.model_validate(value) value = _worker().model_dump(mode="json") value["validation"]["validated"] = False - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match=r"validation\.validated\n Input should be True" + ): V2WorkerVersion.model_validate(value) diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py index ab7a9db8b..9bdf3da14 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py @@ -154,7 +154,7 @@ def __call__( def open_connection_once( connect: Callable[[], Connection], *, - operation_name: str, + operation_name: str, # noqa: ARG001 # the ConnectionOpener protocol passes it by keyword ) -> Connection: """Open a connection once, so a refused connection fails the operation.""" return connect() diff --git a/projects/policyengine-apis-integ/tests/simulation/conftest.py b/projects/policyengine-apis-integ/tests/simulation/conftest.py index 220713638..f5b6d5183 100644 --- a/projects/policyengine-apis-integ/tests/simulation/conftest.py +++ b/projects/policyengine-apis-integ/tests/simulation/conftest.py @@ -43,7 +43,7 @@ class Settings(BaseSettings): settings = Settings() -@pytest.fixture() +@pytest.fixture def client() -> Client | AuthenticatedClient: """Create HTTP client for simulation API.""" timeout = httpx.Timeout(timeout=settings.timeout_in_millis / 1000) @@ -54,25 +54,25 @@ def client() -> Client | AuthenticatedClient: return Client(base_url=settings.base_url, timeout=timeout) -@pytest.fixture() +@pytest.fixture def us_model_version() -> str: """Return the US model version for testing specific version scenarios.""" return settings.us_model_version -@pytest.fixture() +@pytest.fixture def uk_model_version() -> str: """Return the UK model version for testing specific version scenarios.""" return settings.uk_model_version -@pytest.fixture() +@pytest.fixture def poll_interval() -> float: """Return poll interval in seconds.""" return settings.poll_interval_seconds -@pytest.fixture() +@pytest.fixture def max_wait_seconds() -> float: """Return max wait time in seconds.""" return settings.timeout_in_millis / 1000 @@ -146,13 +146,13 @@ def _poll_budget_window_batch( raise TimeoutError(msg) -@pytest.fixture() +@pytest.fixture def budget_window_years() -> list[str]: """Return the annual rows expected from the staging budget-window smoke run.""" return list(BUDGET_WINDOW_YEARS) -@pytest.fixture() +@pytest.fixture def budget_window_request(us_model_version: str) -> BudgetWindowBatchRequest: """Build the standard staging budget-window smoke request.""" return BudgetWindowBatchRequest.from_dict( @@ -169,13 +169,13 @@ def budget_window_request(us_model_version: str) -> BudgetWindowBatchRequest: ) -@pytest.fixture() +@pytest.fixture def decode_response_content(): """Return a compact formatter for non-OK HTTP response payloads.""" return _decode_response_content -@pytest.fixture() +@pytest.fixture def submit_budget_window_batch(client: Client | AuthenticatedClient): """Submit a budget-window batch through the generated client.""" submit_endpoint = submit_budget_window_batch_simulate_economy_budget_window_post @@ -189,7 +189,7 @@ def submit(request: BudgetWindowBatchRequest): return submit -@pytest.fixture() +@pytest.fixture def poll_budget_window_batch( client: Client | AuthenticatedClient, max_wait_seconds: float, diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py index 9ab1f9723..3e998db5f 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py @@ -204,7 +204,7 @@ def bind_observability_id(request: Request, value: object) -> str | None: observability_id = normalize_observability_id(value) request.state.observability_id = observability_id if observability_id is not None: - try: + try: # noqa: SIM105 # try/except keeps the swallow's reason on its except line runtime.set_context(observability_id=observability_id) except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass @@ -234,7 +234,7 @@ def adopt_backend_observability_id( current = normalize_observability_id(request.state.observability_id) downstream = backend_observability_id(headers) if current is not None and downstream is not None and current != downstream: - try: + try: # noqa: SIM105 # try/except keeps the swallow's reason on its except line runtime.event( "simulation_entry_observability_id_mismatch", attributes={ @@ -312,7 +312,7 @@ async def forward( backend_started = time.monotonic() route = _route_template(request) event_identifiers: RequestIdentifiers = identifiers or {} - try: + try: # noqa: SIM105 # try/except keeps the swallow's reason on its except line runtime.set_context(backend="old_gateway", **event_identifiers) except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py index 6344ad00d..541d47227 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py @@ -18,6 +18,11 @@ from policyengine_simulation_entry.config import Settings _bearer = HTTPBearer(auto_error=False) +# CallerAuthenticator's dependency marker, shared as a module-level default. +# The Annotated form can't be used: this module postpones annotations, and +# FastAPI resolves them against the callable's __globals__, which a callable +# instance doesn't have. +_bearer_credentials = Depends(_bearer) logger = logging.getLogger(__name__) @@ -88,7 +93,7 @@ def __init__(self, settings: Settings, runtime: ObservabilityRuntime) -> None: def __call__( self, - token: HTTPAuthorizationCredentials | None = Depends(_bearer), + token: HTTPAuthorizationCredentials | None = _bearer_credentials, ) -> CallerIdentity | None: """Return the verified caller, or None when auth is not required. diff --git a/projects/policyengine-simulation-entry/tests/test_app.py b/projects/policyengine-simulation-entry/tests/test_app.py index 061d07a7b..d1ee870c2 100644 --- a/projects/policyengine-simulation-entry/tests/test_app.py +++ b/projects/policyengine-simulation-entry/tests/test_app.py @@ -3,7 +3,7 @@ import asyncio import json import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, override from unittest.mock import Mock from uuid import UUID @@ -876,6 +876,7 @@ def test_request_validation_matches_shared_contract(client): ) def test_backend_failures_are_sanitized(error, expected_status, expected_detail): class FailingBackend(FakeBackend): + @override async def request(self, *args, **kwargs): raise error @@ -903,6 +904,7 @@ async def request(self, *args, **kwargs): def test_unexpected_failure_preserves_correlation_headers_and_request_log(caplog): class FailingBackend(FakeBackend): + @override async def request(self, *args, **kwargs): msg = "unexpected backend failure" raise RuntimeError(msg) @@ -957,7 +959,7 @@ def test_upstream_error_status_and_body_are_preserved(client, backend, status_co @pytest.mark.parametrize( - "path,extra", + ("path", "extra"), [ ("/simulate/economy/comparison", {}), ( diff --git a/projects/policyengine-simulation-entry/tests/test_auth.py b/projects/policyengine-simulation-entry/tests/test_auth.py index 8facf5b76..d1c92cf2e 100644 --- a/projects/policyengine-simulation-entry/tests/test_auth.py +++ b/projects/policyengine-simulation-entry/tests/test_auth.py @@ -42,17 +42,17 @@ def _use_static_signing_key(monkeypatch, public_key) -> None: decoder.jwks_client = SimpleNamespace( get_signing_key_from_jwt=lambda _: SimpleNamespace(key=public_key) ) - monkeypatch.setattr(auth_module, "_decoder", lambda issuer, audience: decoder) + monkeypatch.setattr(auth_module, "_decoder", lambda _issuer, _audience: decoder) def test_missing_token_preserves_403_contract(monkeypatch): class Decoder: - def __call__(self, token, runtime): + def __call__(self, token, runtime): # noqa: ARG002 # stands in for JWTDecoder.__call__ from fastapi import HTTPException raise HTTPException(status_code=403) - monkeypatch.setattr(auth_module, "_decoder", lambda issuer, audience: Decoder()) + monkeypatch.setattr(auth_module, "_decoder", lambda _issuer, _audience: Decoder()) app = create_app(settings=make_settings(), backend=FakeBackend()) with TestClient(app) as client: @@ -63,11 +63,11 @@ def __call__(self, token, runtime): def test_public_routes_do_not_require_token(monkeypatch): class Decoder: - def __call__(self, token, runtime): + def __call__(self, token, runtime): # noqa: ARG002 # stands in for JWTDecoder.__call__ msg = "public route attempted authentication" raise AssertionError(msg) - monkeypatch.setattr(auth_module, "_decoder", lambda issuer, audience: Decoder()) + monkeypatch.setattr(auth_module, "_decoder", lambda _issuer, _audience: Decoder()) app = create_app(settings=make_settings(), backend=FakeBackend()) with TestClient(app) as client: diff --git a/projects/policyengine-simulation-entry/tests/test_routing_workflow_invariants.py b/projects/policyengine-simulation-entry/tests/test_routing_workflow_invariants.py index 17c299eda..2ab1ecbf2 100644 --- a/projects/policyengine-simulation-entry/tests/test_routing_workflow_invariants.py +++ b/projects/policyengine-simulation-entry/tests/test_routing_workflow_invariants.py @@ -146,9 +146,8 @@ def _routing_commands(script: str): assert REGISTRY_INVOCATION.search(line), ( f"Routing library calls must use the guarded foreground CLI: {line}" ) - assert "$(" not in line and "`" not in line, ( - f"Unsupported routing command substitution: {line}" - ) + assert "$(" not in line, f"Unsupported routing command substitution: {line}" + assert "`" not in line, f"Unsupported routing command substitution: {line}" tokens = _shell_tokens(line) assert not any( token in {"{", "}"} @@ -250,7 +249,8 @@ def _called_workflow(job: dict, workflows: dict[str, dict]) -> str | None: # Same-repository qualified calls must obey the same writer invariant # as relative calls, including when pinned to a ref or commit. path, separator, reference = uses[len(canonical_prefix) :].partition("@") - assert separator and reference, f"Missing reusable workflow ref: {uses}" + assert separator, f"Missing reusable workflow ref: {uses}" + assert reference, f"Missing reusable workflow ref: {uses}" assert path.startswith(".github/workflows/"), uses else: return None @@ -299,7 +299,8 @@ def _guard_allowed_workflows() -> set[str]: ) value = assignment.value if isinstance(value, ast.Call): - assert isinstance(value.func, ast.Name) and value.func.id == "frozenset" + assert isinstance(value.func, ast.Name) + assert value.func.id == "frozenset" value = value.args[0] return set(ast.literal_eval(value)) @@ -406,7 +407,8 @@ def _depends_on(workflow: dict, job_id: str, predecessor: str) -> bool: def _ordered(left, right, workflows): - for (path, left_job), (other_path, right_job) in zip(left, right): + # Compare the chains' common prefix: one may call further reusable workflows. + for (path, left_job), (other_path, right_job) in zip(left, right, strict=False): assert path == other_path if left_job != right_job: return _depends_on(workflows[path], left_job, right_job) or _depends_on( @@ -673,7 +675,7 @@ def test_routing_pipeline_cannot_make_writers_parallel(): ], ) def test_routing_discovery_rejects_unsupported_shell_forms(script): - with pytest.raises(AssertionError, match="Unsupported|foreground"): + with pytest.raises(AssertionError, match=r"Unsupported|foreground"): list(_routing_commands(script)) diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py index 5f119a0b7..7ff871bfb 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py @@ -345,9 +345,7 @@ def _run_restore( def test_rollback_passes_every_attempts_plan_oldest_first(tmp_path): """#744 review: a later attempt must not hide an earlier publication.""" attempts = [1, 10, 2, 9] - result = _run_restore( - tmp_path, 10, {attempt: 1 for attempt in attempts}, present=attempts - ) + result = _run_restore(tmp_path, 10, dict.fromkeys(attempts, 1), present=attempts) assert result.returncode == 0, result.stderr passed = result.stdout.split("--restore-plan\n", 1)[1].split() assert [Path(path).parent.name for path in passed] == [ @@ -414,7 +412,8 @@ def test_rollbacks_share_the_deployment_concurrency_group(): ) rollback = group.search(ROLLBACK_WORKFLOW.read_text(encoding="utf-8")) deploy = group.search(DEPLOY_WORKFLOW.read_text(encoding="utf-8")) - assert rollback and deploy + assert rollback + assert deploy assert rollback.group("group") == deploy.group("group") == "simulation-deploy-main" assert "cancel-in-progress: false" in ROLLBACK_WORKFLOW.read_text(encoding="utf-8") diff --git a/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py b/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py index 75887df7a..aaa0a816b 100644 --- a/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py +++ b/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py @@ -253,7 +253,7 @@ def test_commit_and_push_fails_when_the_push_fails(fakes, tmp_path): assert github_output(output) == {} -def test_create_pr_opens_the_update_pr(fakes, tmp_path): +def test_create_pr_opens_the_update_pr(fakes): fakes.add("gh") result = run_script( @@ -281,7 +281,7 @@ def test_create_pr_opens_the_update_pr(fakes, tmp_path): ] -def test_create_pr_fails_when_gh_fails(fakes, tmp_path): +def test_create_pr_fails_when_gh_fails(fakes): fakes.add("gh", "exit 1\n") result = run_script( @@ -291,7 +291,7 @@ def test_create_pr_fails_when_gh_fails(fakes, tmp_path): assert result.returncode == 1 -def test_create_pr_requires_the_repository(fakes, tmp_path): +def test_create_pr_requires_the_repository(fakes): fakes.add("gh") env = fakes.env() env.pop("GITHUB_REPOSITORY", None) diff --git a/projects/policyengine-simulation-executor/fixtures/fake_modal.py b/projects/policyengine-simulation-executor/fixtures/fake_modal.py index 5c48fd6bd..977a5bcaf 100644 --- a/projects/policyengine-simulation-executor/fixtures/fake_modal.py +++ b/projects/policyengine-simulation-executor/fixtures/fake_modal.py @@ -115,7 +115,7 @@ def decorator(function: F) -> F: return decorator def local_entrypoint[F: Callable[..., object]]( - self, **kwargs: object + self, **_kwargs: object ) -> Callable[[F], F]: """Return a pass-through decorator; entrypoints are not recorded.""" diff --git a/projects/policyengine-simulation-executor/fixtures/fake_object_store.py b/projects/policyengine-simulation-executor/fixtures/fake_object_store.py index de476cb50..aed96ab02 100644 --- a/projects/policyengine-simulation-executor/fixtures/fake_object_store.py +++ b/projects/policyengine-simulation-executor/fixtures/fake_object_store.py @@ -13,7 +13,7 @@ def __init__(self) -> None: """Start with no stored objects.""" self.objects: dict[str, bytes] = {} - def upload_bytes(self, path: str, payload: bytes, *, content_type: str) -> bool: + def upload_bytes(self, path: str, payload: bytes, *, content_type: str) -> bool: # noqa: ARG002 # mirrors ArtifactStore.upload_bytes, which callers pass content_type= by keyword """Store ``payload`` at ``path`` and report whether this call created it.""" # The real store uploads with ``if_generation_match=0``: an existing # object is left in place and the call reports that it created nothing. diff --git a/projects/policyengine-simulation-executor/fixtures/identity_stubs.py b/projects/policyengine-simulation-executor/fixtures/identity_stubs.py index dc4255760..c95deee9a 100644 --- a/projects/policyengine-simulation-executor/fixtures/identity_stubs.py +++ b/projects/policyengine-simulation-executor/fixtures/identity_stubs.py @@ -65,25 +65,25 @@ def install_identity_stubs(monkeypatch: pytest.MonkeyPatch) -> SimpleNamespace: ) monkeypatch.setattr( - release_bundle, "get_country_release_bundle", lambda country: state.bundle + release_bundle, "get_country_release_bundle", lambda _country: state.bundle ) monkeypatch.setattr( - release_bundle, "_receipt_dataset", lambda country: state.receipt_entry + release_bundle, "_receipt_dataset", lambda _country: state.receipt_entry ) monkeypatch.setattr( manifest_module, "resolve_dataset_reference", - lambda country, dataset: f"hf://org/repo/{dataset}.h5@rev-abc", + lambda _country, dataset: f"hf://org/repo/{dataset}.h5@rev-abc", ) monkeypatch.setattr( manifest_module, "dataset_logical_name", - lambda reference: state.bundle.default_dataset, + lambda _reference: state.bundle.default_dataset, ) monkeypatch.setattr( manifest_module, "get_release_manifest", - lambda country: SimpleNamespace( + lambda _country: SimpleNamespace( certification=SimpleNamespace(data_build_fingerprint="fp-123") ), ) diff --git a/projects/policyengine-simulation-executor/fixtures/test_modal_scripts.py b/projects/policyengine-simulation-executor/fixtures/test_modal_scripts.py index c25ae0fd2..6a6ad6b26 100644 --- a/projects/policyengine-simulation-executor/fixtures/test_modal_scripts.py +++ b/projects/policyengine-simulation-executor/fixtures/test_modal_scripts.py @@ -2,7 +2,6 @@ from __future__ import annotations -import os import tempfile from pathlib import Path from typing import TYPE_CHECKING @@ -21,7 +20,7 @@ def temp_github_output() -> Iterator[str]: """Create a temporary file to simulate ``GITHUB_OUTPUT``.""" with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as file: yield file.name - os.unlink(file.name) + Path(file.name).unlink() @pytest.fixture @@ -29,7 +28,7 @@ def temp_github_step_summary() -> Iterator[str]: """Create a temporary file to simulate ``GITHUB_STEP_SUMMARY``.""" with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".md") as file: yield file.name - os.unlink(file.name) + Path(file.name).unlink() @pytest.fixture diff --git a/projects/policyengine-simulation-executor/scripts/dump_package_version.py b/projects/policyengine-simulation-executor/scripts/dump_package_version.py index 943fe1a8c..9bd8061b5 100644 --- a/projects/policyengine-simulation-executor/scripts/dump_package_version.py +++ b/projects/policyengine-simulation-executor/scripts/dump_package_version.py @@ -4,6 +4,7 @@ """ import sys +from pathlib import Path import tomllib @@ -17,7 +18,7 @@ def get_package_version_from_lock( ``[[package]]`` name/version layout read here. """ try: - with open(lock_file_path, "rb") as f: + with Path(lock_file_path).open("rb") as f: lock_data = tomllib.load(f) except FileNotFoundError: return None diff --git a/projects/policyengine-simulation-executor/src/modal/app.py b/projects/policyengine-simulation-executor/src/modal/app.py index ad2338521..ec5028205 100644 --- a/projects/policyengine-simulation-executor/src/modal/app.py +++ b/projects/policyengine-simulation-executor/src/modal/app.py @@ -261,7 +261,7 @@ def _configure_modal_observability( def _set_modal_call_attributes(runtime: "ObservabilityRuntime") -> None: - try: + try: # noqa: SIM105 # try/except keeps the swallow's reason on its except line runtime.set_context(function_call_id=modal.current_function_call_id()) except Exception: # noqa: BLE001, S110 # telemetry is non-fatal: a failed attribute must never fail the Modal call pass diff --git a/projects/policyengine-simulation-executor/src/modal/precompute_app.py b/projects/policyengine-simulation-executor/src/modal/precompute_app.py index 4dacc38ea..bd2365642 100644 --- a/projects/policyengine-simulation-executor/src/modal/precompute_app.py +++ b/projects/policyengine-simulation-executor/src/modal/precompute_app.py @@ -148,7 +148,7 @@ def publish_manifest(bucket: str, plan: dict) -> str: @app.local_entrypoint() -def main(force: bool = False) -> None: +def main(force: bool = False) -> None: # noqa: FBT001, FBT002 # Modal local entrypoint: Modal builds its --force CLI option from this signature """Fill the store for the installed bundle; ``--force`` recomputes stored ones.""" from policyengine_simulation_executor.artifact_store import ( # noqa: PLC0415 # local entrypoint: loaded only when the precompute runs resolve_bucket_name, diff --git a/projects/policyengine-simulation-executor/src/modal/segmented_national.py b/projects/policyengine-simulation-executor/src/modal/segmented_national.py index 40cfef5cc..37a8f4634 100644 --- a/projects/policyengine-simulation-executor/src/modal/segmented_national.py +++ b/projects/policyengine-simulation-executor/src/modal/segmented_national.py @@ -512,7 +512,7 @@ def _fail_if_budget_exhausted( @staticmethod def _cancel_call(call: modal.FunctionCall) -> None: - try: + try: # noqa: SIM105 # try/except keeps the swallow's reason on its except line call.cancel() except Exception: # noqa: BLE001, S110 # best-effort cleanup: a failed cancel must not mask the failure being raised, and superseded calls are cancelled again later pass diff --git a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py index 3db76350a..d7f00bfeb 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py @@ -33,6 +33,7 @@ import urllib.request from collections.abc import Callable, Mapping from importlib import metadata +from pathlib import Path from typing import Any SCHEMA = "policyengine-sim-api.release-sentinel/v1" @@ -328,7 +329,7 @@ def check_deployment( def _report(line: str, *, error: bool = False) -> None: """Print one line of the command's report, to stderr for a failure.""" - print(line, file=sys.stderr if error else sys.stdout) # noqa: T201 # command-line script: stdout and stderr are its report + print(line, file=sys.stderr if error else sys.stdout) def main(argv: list[str] | None = None) -> int: @@ -345,13 +346,13 @@ def main(argv: list[str] | None = None) -> int: if args.command == "expected": expected = compute_expected() - with open(args.output, "w") as handle: + with Path(args.output).open("w") as handle: json.dump(expected, handle, indent=2, sort_keys=True) for case in expected["cases"]: _report(f"{case['case_id']} ({case['seconds']}s): {case['values']}") return 0 - with open(args.expected) as handle: + with Path(args.expected).open() as handle: expected = json.load(handle) problems = check_deployment( args.base_url, diff --git a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py index 4525840e9..282830def 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py @@ -13,6 +13,7 @@ import uuid from collections.abc import Iterable from copy import deepcopy +from pathlib import Path from typing import NotRequired, TypedDict import modal @@ -1301,12 +1302,12 @@ def active_state() -> object: ) _report(f"Routing publication finalization: {outcome}") if args.github_output: - with open(args.github_output, "a") as handle: + with Path(args.github_output).open("a") as handle: handle.write(f"outcome={outcome}\n") return if args.record_plan or args.complete_plan: - with open(args.record_plan or args.complete_plan) as handle: + with Path(args.record_plan or args.complete_plan).open() as handle: plan = json.load(handle) operation = record_routing_plan if args.record_plan else complete_routing_plan operation(environment=args.environment, plan=plan) @@ -1317,7 +1318,7 @@ def active_state() -> object: return if args.apply_plan: - with open(args.apply_plan) as handle: + with Path(args.apply_plan).open() as handle: plan = json.load(handle) apply_routing_plan( environment=args.environment, @@ -1332,7 +1333,7 @@ def active_state() -> object: if args.restore_plan: plans = [] for path in args.restore_plan: - with open(path) as handle: + with Path(path).open() as handle: plans.append(json.load(handle)) outcome = restore_routing_plans( environment=args.environment, @@ -1395,7 +1396,7 @@ def active_state() -> object: publication_id=args.publication_id, ) if args.plan_out: - with open(args.plan_out, "w") as handle: + with Path(args.plan_out).open("w") as handle: json.dump(plan, handle, indent=2, sort_keys=True) _report(f"before_sha256={plan['before_sha256']}") _report(f"after_sha256={plan['after_sha256']}") diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/baseline_artifacts.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/baseline_artifacts.py index e9276ed4f..1ffe75907 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/baseline_artifacts.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/baseline_artifacts.py @@ -218,7 +218,7 @@ def ensure(self) -> None: ) self._record_outcome(OUTCOME_INCOMPLETE) if selection is not None: - setattr(self, "spm", requested_spm) + self.spm = requested_spm self.run() self.save() # Replace the in-process cache entry so this request's second diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py index f7b8b069b..49cd7b83d 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py @@ -10,7 +10,7 @@ output_path.parent.mkdir(parents=True, exist_ok=True) openapi_spec = app.openapi() -with open(output_path, "w") as f: +with output_path.open("w") as f: json.dump(openapi_spec, f, indent=4) print(f"OpenAPI spec written to {output_path}") # noqa: T201 # command-line script: reports where it wrote the spec diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/main.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/main.py index 327f8ab7e..f77e15b06 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/main.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/main.py @@ -28,7 +28,7 @@ @asynccontextmanager -async def lifespan(app: FastAPI) -> AsyncIterator[None]: +async def lifespan(app: FastAPI) -> AsyncIterator[None]: # noqa: ARG001 # FastAPI lifespan hook: FastAPI passes the app """Run the app inside the exit handler and flush observability on shutdown.""" async with app_exit.lifespan(): try: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py index 95d9ec6fa..c4aeddbbf 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py @@ -453,8 +453,10 @@ def verify_determinism_impl( differences.append(f"{entity}: missing on one side") continue left_cols, right_cols = set(left.columns), set(right.columns) - for column in sorted(left_cols ^ right_cols): - differences.append(f"{entity}.{column}: only on one side") + differences.extend( + f"{entity}.{column}: only on one side" + for column in sorted(left_cols ^ right_cols) + ) for column in sorted(left_cols & right_cols): if str(left[column].dtype) != str(right[column].dtype): differences.append(f"{entity}.{column}: dtype differs") diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_builder.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_builder.py index 44b714edc..649fa0aea 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_builder.py @@ -126,11 +126,11 @@ def serialize(self) -> dict[str, Any]: """Build the output and dump it to JSON-compatible data.""" with self._span(ANNUAL_IMPACT_STAGES.name(Stage.CALCULATION)): output = self.build() - with self._span(ANNUAL_IMPACT_STAGES.name(Stage.RESPONSE_SERIALIZATION)): - with self._span( - ANNUAL_IMPACT_STAGES.name(Stage.SIMULATION_OUTPUT_MODEL_DUMP) - ): - return output.model_dump(mode="json") + with ( + self._span(ANNUAL_IMPACT_STAGES.name(Stage.RESPONSE_SERIALIZATION)), + self._span(ANNUAL_IMPACT_STAGES.name(Stage.SIMULATION_OUTPUT_MODEL_DUMP)), + ): + return output.model_dump(mode="json") def _build_detailed_budget(self) -> DetailedBudgetOutput: with self._span(ANNUAL_IMPACT_STAGES.name(Stage.OUTPUT_DETAILED_BUDGET)): diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_distribution.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_distribution.py index 83af46d83..ed027c1c6 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_distribution.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_distribution.py @@ -102,7 +102,7 @@ def build_intra_decile_output_from_collection( if isinstance(collection, IntraDecileOutput): return collection deciles: dict[str, list[float]] = {label: [] for label in INTRA_DECILE_COLUMNS} - all_values: dict[str, float] = {label: 0.0 for label in INTRA_DECILE_COLUMNS} + all_values: dict[str, float] = dict.fromkeys(INTRA_DECILE_COLUMNS, 0.0) rows = [ row for row in sorted( diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py index 746ffb804..51852c884 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py @@ -143,7 +143,7 @@ def setup_gcp_credentials( # the context exits. We restore any prior value of # ``GOOGLE_APPLICATION_CREDENTIALS`` so a retry in the same # container doesn't silently pick up a stale path. - creds_file = tempfile.NamedTemporaryFile( + creds_file = tempfile.NamedTemporaryFile( # noqa: SIM115 # stays open across the yield; the finally block closes, and so deletes, it mode="w", suffix=".json", delete=True, @@ -325,9 +325,7 @@ def _normalise_region_code(country: str, region: object) -> str: value = value.lower() elif prefix == "country": value = value.lower().replace(" ", "_") - elif prefix in {"congressional_district", "place"}: - value = value.upper() - elif prefix == "local_authority": + elif prefix in {"congressional_district", "place", "local_authority"}: value = value.upper() return f"{prefix}/{value}" @@ -678,7 +676,7 @@ def _resolve_dataset_selection( # Region metadata can contain a URI rather than a managed dataset name. for name, uri in bundle.dataset_uris.items(): - if dataset_reference == name or dataset_reference == uri: + if dataset_reference in (name, uri): return DatasetSelection(name, uri, name == bundle.default_dataset) for name, uri in bundle.dataset_uris.items(): if dataset_reference == runtime_dataset_uri( diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py index 72272360d..71877ee60 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py @@ -123,15 +123,16 @@ def _frame_payload( columns = [ PARQUET_CONTRACT.entity_column, PARQUET_CONTRACT.row_order_column, - ] + sorted( - column - for column in combined.columns - if column - not in { - PARQUET_CONTRACT.entity_column, - PARQUET_CONTRACT.row_order_column, - } - ) + *sorted( + column + for column in combined.columns + if column + not in { + PARQUET_CONTRACT.entity_column, + PARQUET_CONTRACT.row_order_column, + } + ), + ] return combined.reindex(columns=columns), dtypes, identifier_values diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py index 9ba01e9a0..d8b5f2ac8 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py @@ -467,7 +467,7 @@ def coordinate_report( descriptors[role] = SimulationArtifactDescriptor.model_validate( call.get(timeout=SIMULATION_WAIT_TIMEOUT_SECONDS) ) - except Exception as error: + except Exception as error: # noqa: PERF203 # per child: the failing child is marked failed before the error is re-raised report_original_failure( error, runtime=runtime, diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/database_retry.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/database_retry.py index 0869b4fc2..90432c2de 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/database_retry.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/database_retry.py @@ -52,7 +52,7 @@ def open_connection_with_retry( while True: try: return connect() - except OperationalError as error: + except OperationalError as error: # noqa: PERF203 # each refused attempt is caught so the loop can retry it remaining_budget = CONNECTION_RETRY_BUDGET_SECONDS - (monotonic() - started) if remaining_budget <= 0: raise diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py index bd32b8f7c..978071635 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py @@ -375,7 +375,7 @@ def detect_uk_local_authority_metadata_from_hdf( """Validate an installed UK HDF dataset against the packaged metadata.""" observed_codes: set[object] = set() with pd.HDFStore(dataset_path, mode="r") as store: - if "/household" not in store.keys(): + if "/household" not in store.keys(): # noqa: SIM118 # an HDFStore, not a dict: keys() lists only pandas tables, while `in` matches any HDF node msg = "UK dataset contains no household table" raise ValueError(msg) try: diff --git a/projects/policyengine-simulation-executor/tests/conftest.py b/projects/policyengine-simulation-executor/tests/conftest.py index d87313280..2d00ec40c 100644 --- a/projects/policyengine-simulation-executor/tests/conftest.py +++ b/projects/policyengine-simulation-executor/tests/conftest.py @@ -23,19 +23,19 @@ class NoOpObservabilityRuntime: """Minimal explicit runtime used by executor unit tests.""" - def span(self, *args, **kwargs): + def span(self, *_args, **_kwargs): return nullcontext() - def operation(self, *args, **kwargs): + def operation(self, *_args, **_kwargs): return nullcontext() - def set_context(self, **attributes): + def set_context(self, **_attributes): return None - def event(self, *args, **kwargs): + def event(self, *_args, **_kwargs): return None - def record_exception(self, *args, **kwargs): + def record_exception(self, *_args, **_kwargs): return None def capture_context(self): diff --git a/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py b/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py index 4437595ce..f1c5c8155 100644 --- a/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py +++ b/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py @@ -21,6 +21,7 @@ def _run_modal_smoke(app_path: str) -> subprocess.CompletedProcess[str]: cwd=PROJECT_ROOT, capture_output=True, text=True, + check=False, timeout=2400, ) diff --git a/projects/policyengine-simulation-executor/tests/test_artifact_keys.py b/projects/policyengine-simulation-executor/tests/test_artifact_keys.py index c7b1dcffb..d8eaad533 100644 --- a/projects/policyengine-simulation-executor/tests/test_artifact_keys.py +++ b/projects/policyengine-simulation-executor/tests/test_artifact_keys.py @@ -18,34 +18,34 @@ ) from policyengine_simulation_executor import artifact_keys as ak -_DATASET_KWARGS = dict( - country="us", - dataset="populace_cps", - year=2026, - data_version="1.2.3", - data_artifact_revision="rev-abc", - source_sha256="feedbead" * 8, - data_build_fingerprint="fp-123", - model_version="9.9.9", - policyengine_version="4.22.0", -) +_DATASET_KWARGS = { + "country": "us", + "dataset": "populace_cps", + "year": 2026, + "data_version": "1.2.3", + "data_artifact_revision": "rev-abc", + "source_sha256": "feedbead" * 8, + "data_build_fingerprint": "fp-123", + "model_version": "9.9.9", + "policyengine_version": "4.22.0", +} _DATASET_GOLDEN = "183ffe49d74669b653fb408b0b7298128ba7c6e990f4479487712a5d79553be3" _DATASET_NO_SHA_GOLDEN = ( "25dded8192092f477e4502dae07709e72b6e579f372a9f8346b5eabce07d91b6" ) -_BASELINE_KWARGS = dict( - country="us", - region="national", - scope_key="region_group:row_filter:state_code=CA|row_filter:state_code=WV", - dataset_digest=_DATASET_GOLDEN, - model_version="9.9.9", - policyengine_version="4.22.0", +_BASELINE_KWARGS = { + "country": "us", + "region": "national", + "scope_key": "region_group:row_filter:state_code=CA|row_filter:state_code=WV", + "dataset_digest": _DATASET_GOLDEN, + "model_version": "9.9.9", + "policyengine_version": "4.22.0", # Explicit (== the default) so the perturbation matrix proves the # policy field participates in the digest. - policy=ak.CURRENT_LAW_POLICY, -) + "policy": ak.CURRENT_LAW_POLICY, +} _BASELINE_GOLDEN = "f9cac05d94509895eb48ea33ea5f72d40eb9b954989df05ca6506009d8dedae2" @@ -134,7 +134,8 @@ def stub_identity_sources(monkeypatch): class TestIdentityCollection: - def test_collected_identity_matches_golden(self, stub_identity_sources): + @pytest.mark.usefixtures("stub_identity_sources") + def test_collected_identity_matches_golden(self): identity = ak.collect_dataset_identity("us", 2026) assert identity.digest == _DATASET_GOLDEN assert identity.filename == "populace_cps_year_2026.h5" @@ -166,7 +167,8 @@ def test_expected_sha_used_when_installed_absent(self, stub_identity_sources): } assert ak.collect_dataset_identity("us", 2026).digest == _DATASET_GOLDEN - def test_baseline_identity_composes(self, stub_identity_sources): + @pytest.mark.usefixtures("stub_identity_sources") + def test_baseline_identity_composes(self): identity = ak.collect_baseline_identity( "us", 2026, @@ -194,12 +196,12 @@ def entry(self, year, *, scenario, as_of): return {"year": year, "scenario": scenario, "as_of": as_of} monkeypatch.setattr( - executor_spm, "_forecast", lambda expected_sha256: _StubForecast() + executor_spm, "_forecast", lambda _expected_sha256: _StubForecast() ) monkeypatch.setattr( executor_spm, "_prevalidate_selection", - lambda selection_json, start_year, window_size: None, + lambda _selection_json, _start_year, _window_size: None, ) capability = stub_identity_sources.installed_spm_capability() @@ -249,7 +251,7 @@ class TestWrapperStorageIdAgreement: """ @pytest.fixture - def identity(self, stub_identity_sources): + def identity(self, stub_identity_sources): # noqa: ARG002 # requested for its side effect; usefixtures has no effect on fixtures """Build the identity directly from an already-resolved selection. ``collect_baseline_identity`` resolves the selection through the diff --git a/projects/policyengine-simulation-executor/tests/test_artifact_store.py b/projects/policyengine-simulation-executor/tests/test_artifact_store.py index c79e9ce8f..a9de90435 100644 --- a/projects/policyengine-simulation-executor/tests/test_artifact_store.py +++ b/projects/policyengine-simulation-executor/tests/test_artifact_store.py @@ -6,6 +6,7 @@ """ import json +from pathlib import Path from types import SimpleNamespace import pytest @@ -34,7 +35,7 @@ def _write(self, data, **kwargs): self.store.objects[self.path] = data def upload_from_filename(self, filename, **kwargs): - self._write(open(filename, "rb").read(), **kwargs) + self._write(Path(filename).read_bytes(), **kwargs) def upload_from_string(self, data, **kwargs): self._write(data.encode() if isinstance(data, str) else data, **kwargs) @@ -45,11 +46,11 @@ def download_to_filename(self, filename): if self.path in self.store.broken_downloads: # Simulate a mid-stream failure: partial bytes hit disk, then # the transport dies. - with open(filename, "wb") as handle: + with Path(filename).open("wb") as handle: handle.write(self.store.objects[self.path][:1]) msg = "stream reset" raise ConnectionError(msg) - with open(filename, "wb") as handle: + with Path(filename).open("wb") as handle: handle.write(self.store.objects[self.path]) def download_as_bytes(self): diff --git a/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py b/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py index 13890d62f..a9544cfe3 100644 --- a/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py +++ b/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py @@ -51,16 +51,16 @@ def fake_collect(country, year, *, region, scope_key): monkeypatch.setattr(ak, "collect_baseline_identity", fake_collect) return calls - def _id(self, collected, **overrides): - kwargs = dict( - params={"scope": "macro"}, - country="us", - dataset_is_default=True, - policy=None, - region_code="us", - scoping_strategy=None, - year=2026, - ) + def _id(self, _collected, **overrides): + kwargs = { + "params": {"scope": "macro"}, + "country": "us", + "dataset_is_default": True, + "policy": None, + "region_code": "us", + "scoping_strategy": None, + "year": 2026, + } kwargs.update(overrides) return ba.deterministic_baseline_id(kwargs.pop("params"), **kwargs) @@ -138,7 +138,7 @@ def test_single_state_scoping_disqualifies(self, collected): ) def test_identity_collection_failure_degrades_to_random_id(self, monkeypatch): - def boom(*args, **kwargs): + def boom(*_args, **_kwargs): msg = "manifest unavailable" raise RuntimeError(msg) @@ -176,7 +176,7 @@ def __init__(self, *, load_result="complete"): self.load_result = load_result self.calls = [] - def resolve_entity_variables(self, simulation): + def resolve_entity_variables(self, simulation): # noqa: ARG002 # duck-types MicrosimulationModelVersion.resolve_entity_variables return {"person": ["age", "income"]} def load(self, simulation): @@ -198,7 +198,7 @@ def run(self, simulation): self.calls.append("run") simulation.output_dataset = _output(_complete_frames()) - def save(self, simulation): + def save(self, simulation): # noqa: ARG002 # duck-types MicrosimulationModelVersion.save self.calls.append("save") @@ -226,28 +226,32 @@ def _make_sim(model_version, sim_id="bl1-test"): class TestArtifactBaselineSimulation: - def test_complete_load_is_a_hit(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_complete_load_is_a_hit(self): model = FakeModelVersion(load_result="complete") sim = _make_sim(model) sim.ensure() assert sim.artifact_outcome == ba.OUTCOME_HIT assert model.calls == ["load"] - def test_absent_artifact_is_a_miss_that_computes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_absent_artifact_is_a_miss_that_computes(self): model = FakeModelVersion(load_result="absent") sim = _make_sim(model) sim.ensure() assert sim.artifact_outcome == ba.OUTCOME_MISS assert model.calls == ["load", "run", "save"] - def test_broken_load_degrades_to_miss(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_broken_load_degrades_to_miss(self): model = FakeModelVersion(load_result="broken") sim = _make_sim(model) sim.ensure() assert sim.artifact_outcome == ba.OUTCOME_MISS assert model.calls == ["load", "run", "save"] - def test_incomplete_load_recomputes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_incomplete_load_recomputes(self): model = FakeModelVersion(load_result="incomplete") sim = _make_sim(model) sim.ensure() @@ -256,7 +260,8 @@ def test_incomplete_load_recomputes(self, fresh_cache): # The guard's recompute produced the full column set. assert "income" in sim.output_dataset.data.entity_data["person"].columns - def test_recompute_replaces_cache_entry(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_recompute_replaces_cache_entry(self): model = FakeModelVersion(load_result="incomplete") first = _make_sim(model) first.ensure() @@ -317,7 +322,8 @@ def test_container_converges_after_stale_recompute(self, fresh_cache): assert later.artifact_outcome == ba.OUTCOME_HIT assert later_model.calls == [] - def test_loaded_output_without_data_recomputes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_loaded_output_without_data_recomputes(self): model = FakeModelVersion() model.load = lambda simulation: setattr( # type: ignore[method-assign] simulation, "output_dataset", SimpleNamespace(data=None) @@ -326,7 +332,8 @@ def test_loaded_output_without_data_recomputes(self, fresh_cache): sim.ensure() assert sim.artifact_outcome == ba.OUTCOME_INCOMPLETE - def test_second_ensure_keeps_a_genuine_miss(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_second_ensure_keeps_a_genuine_miss(self): """Every request ensures the baseline twice (analysis + deciles). The second call self-hits the in-process cache with a complete @@ -342,7 +349,8 @@ def test_second_ensure_keeps_a_genuine_miss(self, fresh_cache): # The second ensure() was a pure cache hit: no extra model work. assert model.calls == ["load", "run", "save"] - def test_second_ensure_keeps_an_incomplete_outcome(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_second_ensure_keeps_an_incomplete_outcome(self): model = FakeModelVersion(load_result="incomplete") sim = _make_sim(model) sim.ensure() @@ -392,7 +400,7 @@ def __init__(self, **kwargs): monkeypatch.setattr( sr, "_country_module", - lambda country: SimpleNamespace(model=SimpleNamespace(id="us-model")), + lambda _country: SimpleNamespace(model=SimpleNamespace(id="us-model")), ) def fake_deterministic_id(params, **kwargs): @@ -486,7 +494,7 @@ def collect_identity(*args, **kwargs): monkeypatch.setattr(ak, "collect_baseline_identity", collect_identity) kwargs = { "dataset": "dataset", - "dataset_selection": sr.DatasetSelection("default", "default", True), + "dataset_selection": sr.DatasetSelection("default", "default", is_default=True), "region_code": "uk", } params = {"country": "uk", "scope": "macro", "time_period": 2027} @@ -515,18 +523,18 @@ class TestIdentityErrorContract: """The stated split: the writer path fails loud, the reader degrades.""" def _kwargs(self): - return dict( - params={"scope": "macro"}, - country="us", - dataset_is_default=True, - policy=None, - region_code="us", - scoping_strategy=None, - year=2026, - ) + return { + "params": {"scope": "macro"}, + "country": "us", + "dataset_is_default": True, + "policy": None, + "region_code": "us", + "scoping_strategy": None, + "year": 2026, + } def test_qualifying_identity_propagates_collection_errors(self, monkeypatch): - def boom(*args, **kwargs): + def boom(*_args, **_kwargs): msg = "receipt unreadable" raise RuntimeError(msg) @@ -535,7 +543,7 @@ def boom(*args, **kwargs): ba.qualifying_baseline_identity(**self._kwargs()) def test_deterministic_id_swallows_collection_errors(self, monkeypatch): - def boom(*args, **kwargs): + def boom(*_args, **_kwargs): msg = "receipt unreadable" raise RuntimeError(msg) @@ -589,7 +597,7 @@ def __init__(self): self._dataset_class = PolicyEngineUSDataset self.run_calls = 0 - def resolve_entity_variables(self, simulation): + def resolve_entity_variables(self, simulation): # noqa: ARG002 # duck-types MicrosimulationModelVersion.resolve_entity_variables return { "person": ["age", "employment_income"], "household": ["household_net_income"], @@ -656,7 +664,8 @@ class TestArtifactDiskRoundTrip: triggers the recompute-and-overwrite path. """ - def test_missing_artifact_is_a_clean_miss_that_saves(self, fresh_cache, tmp_path): + @pytest.mark.usefixtures("fresh_cache") + def test_missing_artifact_is_a_clean_miss_that_saves(self, tmp_path): model = DiskModelVersion() sim = _make_disk_sim(model, tmp_path, "bl1-roundtrip-miss") sim.ensure() @@ -664,7 +673,8 @@ def test_missing_artifact_is_a_clean_miss_that_saves(self, fresh_cache, tmp_path assert model.run_calls == 1 assert (tmp_path / "bl1-roundtrip-miss.h5").exists() - def test_saved_artifact_loads_as_hit_with_weights(self, fresh_cache, tmp_path): + @pytest.mark.usefixtures("fresh_cache") + def test_saved_artifact_loads_as_hit_with_weights(self, tmp_path): model = DiskModelVersion() model._dataset_class( name="artifact", @@ -682,9 +692,8 @@ def test_saved_artifact_loads_as_hit_with_weights(self, fresh_cache, tmp_path): assert list(person["age"]) == [30.0, 40.0] assert "person_weight" in person.columns - def test_incomplete_disk_artifact_recomputes_and_overwrites( - self, fresh_cache, tmp_path - ): + @pytest.mark.usefixtures("fresh_cache") + def test_incomplete_disk_artifact_recomputes_and_overwrites(self, tmp_path): model = DiskModelVersion() model._dataset_class( name="artifact", @@ -884,7 +893,8 @@ class TestEnsureValidatesSPMReceipts: wrapper receipts have the shape the decisions are made on. """ - def test_matching_receipt_loads_as_a_hit(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_matching_receipt_loads_as_a_hit(self): model = SPMModelVersion( stored_config=deepcopy(SPM_SELECTION), stored_receipt=_spm_receipt() ) @@ -893,7 +903,8 @@ def test_matching_receipt_loads_as_a_hit(self, fresh_cache): assert sim.artifact_outcome == ba.OUTCOME_HIT assert model.calls == ["load"] - def test_artifact_built_under_another_selection_recomputes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_artifact_built_under_another_selection_recomputes(self): other = {**SPM_SELECTION, "scenario": "zero_real"} model = SPMModelVersion(stored_config=other, stored_receipt=_spm_receipt(other)) sim = _make_spm_sim(model) @@ -905,7 +916,8 @@ def test_artifact_built_under_another_selection_recomputes(self, fresh_cache): assert model.spm_at_run == [SPM_SELECTION] assert sim.spm_config == SPM_SELECTION - def test_artifact_without_a_receipt_recomputes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_artifact_without_a_receipt_recomputes(self): model = SPMModelVersion( stored_config=deepcopy(SPM_SELECTION), stored_receipt=None ) @@ -914,7 +926,8 @@ def test_artifact_without_a_receipt_recomputes(self, fresh_cache): assert sim.artifact_outcome == ba.OUTCOME_INCOMPLETE assert model.calls == ["load", "run", "save"] - def test_malformed_receipt_recomputes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_malformed_receipt_recomputes(self): malformed = _spm_receipt() del malformed["runtime_versions"] model = SPMModelVersion( @@ -925,7 +938,8 @@ def test_malformed_receipt_recomputes(self, fresh_cache): assert sim.artifact_outcome == ba.OUTCOME_INCOMPLETE assert model.calls == ["load", "run", "save"] - def test_receipt_from_another_forecast_recomputes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_receipt_from_another_forecast_recomputes(self): stale = _spm_receipt() stale["forecast_sha256"] = "b" * 64 model = SPMModelVersion( @@ -936,7 +950,8 @@ def test_receipt_from_another_forecast_recomputes(self, fresh_cache): assert sim.artifact_outcome == ba.OUTCOME_INCOMPLETE assert model.calls == ["load", "run", "save"] - def test_receipt_for_another_year_recomputes(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_receipt_for_another_year_recomputes(self): model = SPMModelVersion( stored_config=deepcopy(SPM_SELECTION), stored_receipt=_spm_receipt(year="2024"), @@ -1006,7 +1021,8 @@ def test_recompute_replaces_the_cache_with_this_request_s_selection( assert later.artifact_outcome == ba.OUTCOME_HIT assert later_model.calls == [] - def test_missing_columns_are_decided_before_the_receipt(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_missing_columns_are_decided_before_the_receipt(self): """A tax-only artifact can legitimately carry no usable receipt. Columns are checked first so the outcome is attributed to the gap @@ -1022,7 +1038,8 @@ def test_missing_columns_are_decided_before_the_receipt(self, fresh_cache): assert sim.artifact_outcome == ba.OUTCOME_INCOMPLETE assert sim._provenance_reads == [] - def test_a_request_without_a_selection_never_validates_receipts(self, fresh_cache): + @pytest.mark.usefixtures("fresh_cache") + def test_a_request_without_a_selection_never_validates_receipts(self): """Without a selection, a complete artifact is a hit even with no receipt. This is the legacy path: the wrapper surface exists but holds no diff --git a/projects/policyengine-simulation-executor/tests/test_baseline_artifacts_spm_native.py b/projects/policyengine-simulation-executor/tests/test_baseline_artifacts_spm_native.py index 2e5b26e40..6e3e9447b 100644 --- a/projects/policyengine-simulation-executor/tests/test_baseline_artifacts_spm_native.py +++ b/projects/policyengine-simulation-executor/tests/test_baseline_artifacts_spm_native.py @@ -40,7 +40,9 @@ def _baseline(dataset, *, geography="national", tax_only=False): "spm": {"geography_kind": geography}, }, dataset=dataset, - dataset_selection=DatasetSelection("bounded-native-test", "native", False), + dataset_selection=DatasetSelection( + "bounded-native-test", "native", is_default=False + ), policy=None, region_code="us", ) @@ -61,7 +63,7 @@ def _baseline(dataset, *, geography="national", tax_only=False): @pytest.mark.parametrize( - "damage, expected_outcome", + ("damage", "expected_outcome"), [ ("tax_only_columns", "incomplete"), ("empty_years", "incomplete"), diff --git a/projects/policyengine-simulation-executor/tests/test_budget_window_batch.py b/projects/policyengine-simulation-executor/tests/test_budget_window_batch.py index 2731041ab..af013dd02 100644 --- a/projects/policyengine-simulation-executor/tests/test_budget_window_batch.py +++ b/projects/policyengine-simulation-executor/tests/test_budget_window_batch.py @@ -23,7 +23,7 @@ def __init__(self, object_id: str, events, tracker): self._done = False self._tracker.started(self.object_id) - def get(self, timeout: int = 0): + def get(self, timeout: int = 0): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword event = self._events[0] if isinstance(event, TimeoutError): self._events.popleft() @@ -110,7 +110,7 @@ def get(self, key: str, default=None): class MockModalDict: @staticmethod - def from_name(name: str, create_if_missing: bool = False): + def from_name(name: str, *, create_if_missing: bool = False): if create_if_missing and name not in dicts: dicts[name] = {} if name not in dicts: diff --git a/projects/policyengine-simulation-executor/tests/test_budget_window_results.py b/projects/policyengine-simulation-executor/tests/test_budget_window_results.py index 8e05b79ae..6fc138a89 100644 --- a/projects/policyengine-simulation-executor/tests/test_budget_window_results.py +++ b/projects/policyengine-simulation-executor/tests/test_budget_window_results.py @@ -68,7 +68,7 @@ def test_extract_annual_impact_rejects_malformed_child_result(): ValueError, match=( "Malformed budget-window child result: " - "missing numeric budget.tax_revenue_impact" + r"missing numeric budget\.tax_revenue_impact" ), ): extract_annual_impact( diff --git a/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py b/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py index 072e15b0f..1e25f0f9a 100644 --- a/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py +++ b/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py @@ -140,7 +140,7 @@ def __init__( self.error = error self.runtime.child_started(object_id) - def get(self, timeout: int = 0): + def get(self, timeout: int = 0): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword self.runtime.child_finished(self.object_id) if self.error is not None: raise self.error @@ -162,7 +162,7 @@ def __init__( self._polls = 0 self._result = None - def get(self, timeout: int = 0): + def get(self, timeout: int = 0): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword if self._result is not None: return self._result if self._polls == 0: @@ -227,11 +227,11 @@ class PropagatingRuntime: def __init__(self): self.context: dict = {} - def span(self, *args, **kwargs): + def span(self, *_args, **_kwargs): return nullcontext() @contextmanager - def operation(self, *args, remote_context=None, **kwargs): + def operation(self, *_args, remote_context=None, **_kwargs): previous = self.context self.context = dict(remote_context or {}) try: @@ -245,10 +245,10 @@ def capture_context(self): def set_context(self, **attributes): self.context.update(attributes) - def event(self, *args, **kwargs): + def event(self, *_args, **_kwargs): return None - def record_exception(self, *args, **kwargs): + def record_exception(self, *_args, **_kwargs): return None runtime.observability = PropagatingRuntime() @@ -259,7 +259,7 @@ def record_exception(self, *args, **kwargs): class MockModalDict: @staticmethod - def from_name(name: str, create_if_missing: bool = False): + def from_name(name: str, *, create_if_missing: bool = False): if create_if_missing and name not in runtime.dicts: runtime.dicts[name] = {} if name not in runtime.dicts: diff --git a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py index e6efefbee..662ff425d 100644 --- a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py +++ b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py @@ -75,23 +75,23 @@ def test_country_receipt_is_compacted_at_the_executor_boundary(): def result(selection=SELECTION, year="2026"): - receipt = dict( - schema_version="canonical-spm-provenance-v2", - forecast_id="test-only", - forecast_sha256=selection["forecast_content_sha256"], - scenario=selection["scenario"], - geography_kind=selection["geography_kind"], - geography_id=selection["geography_id"], - county_vintage=selection["county_vintage"], - as_of=selection["as_of"], - years=[year], - runtime_versions={ + receipt = { + "schema_version": "canonical-spm-provenance-v2", + "forecast_id": "test-only", + "forecast_sha256": selection["forecast_content_sha256"], + "scenario": selection["scenario"], + "geography_kind": selection["geography_kind"], + "geography_id": selection["geography_id"], + "county_vintage": selection["county_vintage"], + "as_of": selection["as_of"], + "years": [year], + "runtime_versions": { "policyengine": "test-only", "policyengine-core": "test-only", "policyengine-us": "test-only", "spm-calculator": "test-only", }, - ) + } return { "spm_provenance": { "schema_version": "canonical-spm-comparison-v2", @@ -107,7 +107,7 @@ def result(selection=SELECTION, year="2026"): @pytest.mark.parametrize( - "model,extra", + ("model", "extra"), [ (SimulationRequest, {}), ( @@ -119,9 +119,13 @@ def result(selection=SELECTION, year="2026"): def test_public_request_roundtrip(model, extra): parsed = model(country="us", spm={"geography_kind": "national"}, **extra) assert json.loads(parsed.model_dump_json())["spm"]["geography_kind"] == "national" - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match=r"spm\.provider_path\n Extra inputs are not permitted" + ): model(country="us", spm={"provider_path": "x"}, **extra) - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match="Only an SPM area selection accepts geography_id" + ): model( country="us", spm={"geography_kind": "national", "geography_id": "12345"}, @@ -175,7 +179,7 @@ def test_unknown_bundle_fails_closed_and_historical_uk_remain(): @pytest.mark.parametrize( - "kind,area", [("county", None), ("national", None), ("metro", "35620")] + ("kind", "area"), [("county", None), ("national", None), ("metro", "35620")] ) def test_partial_selection_survives_ordinary_nested_json(kind, area): capability = { @@ -272,7 +276,7 @@ def test_sync_compatibility_endpoint_preserves_explicit_null( captured = [] - def capture(params, *, runtime): + def capture(params, *, runtime): # noqa: ARG001 # keyword-only, as the route passes it captured.append(params) error_code = "SPM_GEOGRAPHY_REQUIRED" raise SPMInputError(error_code, "intentional test stop") @@ -294,7 +298,7 @@ def test_year_alias_is_validated_before_dataset_loading(monkeypatch): monkeypatch.setattr( release_bundle, "get_country_release_bundle", - lambda country: SimpleNamespace( + lambda _country: SimpleNamespace( policyengine_version="test", model_version="test" ), ) @@ -302,12 +306,12 @@ def test_year_alias_is_validated_before_dataset_loading(monkeypatch): year_metadata = Mock( side_effect=SPMInputError("SPM_YEAR_UNAVAILABLE", "2040 is unavailable") ) - monkeypatch.setattr(spm, "_forecast", lambda sha: object()) + monkeypatch.setattr(spm, "_forecast", lambda _expected_sha256: object()) monkeypatch.setitem( sys.modules, "spm_calculator.policyengine_adapter", SimpleNamespace( - PolicyEngineSPMProvider=lambda *a, **k: SimpleNamespace( + PolicyEngineSPMProvider=lambda *_args, **_kwargs: SimpleNamespace( year_metadata=year_metadata ) ), @@ -338,7 +342,7 @@ def _stub_bundle(monkeypatch): monkeypatch.setattr( release_bundle, "get_country_release_bundle", - lambda country: SimpleNamespace( + lambda _country: SimpleNamespace( policyengine_version="test", model_version="test" ), ) @@ -350,10 +354,12 @@ def _stub_provider(monkeypatch, capability=CAPABILITY): year_metadata = Mock(return_value={}) constructions = Mock( - side_effect=lambda *a, **k: SimpleNamespace(year_metadata=year_metadata) + side_effect=lambda *_args, **_kwargs: SimpleNamespace( + year_metadata=year_metadata + ) ) monkeypatch.setattr(spm, "runtime_spm_capability", lambda: capability) - monkeypatch.setattr(spm, "_forecast", lambda sha: object()) + monkeypatch.setattr(spm, "_forecast", lambda _expected_sha256: object()) monkeypatch.setitem( sys.modules, "spm_calculator.policyengine_adapter", @@ -533,7 +539,7 @@ def test_actual_http_formula_error_contract(monkeypatch, code, observability_run @pytest.mark.parametrize( - "key,value", + ("key", "value"), [ ("forecast_content_sha256", "b" * 64), ("scenario", "zero_real"), @@ -544,29 +550,29 @@ def test_actual_http_formula_error_contract(monkeypatch, code, observability_run ], ) def test_each_setting_rotates_artifact_identity(key, value): - common = dict( - country="us", - region="national", - scope_key=None, - dataset_digest="d" * 64, - model_version="test", - policyengine_version="test", - ) + common = { + "country": "us", + "region": "national", + "scope_key": None, + "dataset_digest": "d" * 64, + "model_version": "test", + "policyengine_version": "test", + } changed = {**SELECTION, key: value} assert artifact_keys.baseline_key( **common, spm=SELECTION ) != artifact_keys.baseline_key(**common, spm=changed) - ds = dict( - country="us", - dataset="test", - year=2026, - data_version="test", - data_artifact_revision="test", - source_sha256=None, - data_build_fingerprint=None, - model_version="test", - policyengine_version="test", - ) + ds = { + "country": "us", + "dataset": "test", + "year": 2026, + "data_version": "test", + "data_artifact_revision": "test", + "source_sha256": None, + "data_build_fingerprint": None, + "model_version": "test", + "policyengine_version": "test", + } assert artifact_keys.dataset_key(**ds, spm=SELECTION) != artifact_keys.dataset_key( **ds, spm=changed ) @@ -606,14 +612,14 @@ def test_worker_build_passes_selection_to_baseline_and_reform(monkeypatch): from policyengine_simulation_executor import baseline_artifacts calls = [] - monkeypatch.setattr(spm, "normalize_runtime_spm", lambda params: SELECTION) + monkeypatch.setattr(spm, "normalize_runtime_spm", lambda _params: SELECTION) monkeypatch.setattr( simulation_runtime, "_country_module", lambda _: SimpleNamespace(model="test") ) monkeypatch.setattr( baseline_artifacts, "deterministic_baseline_id", - lambda *a, **k: "baseline-id" if k["policy"] is None else None, + lambda *_args, **kwargs: "baseline-id" if kwargs["policy"] is None else None, ) monkeypatch.setattr( baseline_artifacts, "ArtifactBaselineSimulation", lambda **k: calls.append(k) @@ -623,7 +629,9 @@ def test_worker_build_passes_selection_to_baseline_and_reform(monkeypatch): simulation_runtime._build_simulation( {"country": "us", "spm": SELECTION}, dataset="tiny", - dataset_selection=simulation_runtime.DatasetSelection("tiny", "tiny", True), + dataset_selection=simulation_runtime.DatasetSelection( + "tiny", "tiny", is_default=True + ), policy=policy, region_code="us", ) @@ -701,18 +709,18 @@ def test_gateway_submission_uses_registry_capability_before_spawn(monkeypatch): country_model_version="test-only", route_provenance=None, ) - monkeypatch.setattr(endpoints, "resolve_route", lambda *args: route) + monkeypatch.setattr(endpoints, "resolve_route", lambda *_args: route) bundle = PolicyEngineBundle( model_version="test-only", policyengine_version="test-only", spm=CAPABILITY ) - monkeypatch.setattr(endpoints, "_build_policyengine_bundle", lambda *args: bundle) + monkeypatch.setattr(endpoints, "_build_policyengine_bundle", lambda *_args: bundle) spawn = Mock(return_value=SimpleNamespace(object_id="job")) monkeypatch.setattr( endpoints, "modal", SimpleNamespace( Function=SimpleNamespace( - from_name=lambda *args: SimpleNamespace(spawn=spawn) + from_name=lambda *_args: SimpleNamespace(spawn=spawn) ) ), ) @@ -738,7 +746,7 @@ def test_gateway_submission_uses_registry_capability_before_spawn(monkeypatch): spawn.assert_not_called() -def test_segmented_child_input_error_is_not_redacted_or_retried(monkeypatch): +def test_segmented_child_input_error_is_not_redacted_or_retried(): from src.modal.segmented_national import ( CHILD_DEADLINE_SECONDS, ChildExecution, @@ -861,7 +869,7 @@ def test_optional_analysis_does_not_swallow_spm_input_errors(code): @pytest.mark.parametrize( - "date_fields,expected_as_of", + ("date_fields", "expected_as_of"), [({"as_of": None}, None), ({}, "2026-01-01")], ) def test_as_of_presence_survives_budget_parent(date_fields, expected_as_of): @@ -916,7 +924,7 @@ def test_versions_http_retains_capability_contract_version(monkeypatch): "spm": CAPABILITY, } monkeypatch.setattr( - registry, "build_bundle_manifest_metadata", lambda **kwargs: metadata + registry, "build_bundle_manifest_metadata", lambda **_kwargs: metadata ) state = registry.build_next_routing_state( current_state=None, diff --git a/projects/policyengine-simulation-executor/tests/test_canonical_spm_native.py b/projects/policyengine-simulation-executor/tests/test_canonical_spm_native.py index 39e95805c..cc016fc2a 100644 --- a/projects/policyengine-simulation-executor/tests/test_canonical_spm_native.py +++ b/projects/policyengine-simulation-executor/tests/test_canonical_spm_native.py @@ -42,7 +42,9 @@ def test_worker_baseline_reform_national_local_cache_and_receipts( "data": str(tmp_path / "native.h5"), "spm": {"geography_kind": "national"}, } - dataset_selection = DatasetSelection("native", str(tmp_path / "native.h5"), False) + dataset_selection = DatasetSelection( + "native", str(tmp_path / "native.h5"), is_default=False + ) baseline = _build_simulation( params, dataset=native_dataset, @@ -133,7 +135,9 @@ def test_native_state_only_worker_requires_geography_and_explicit_national_works "time_period": "2024", "data": "test-only-custom-data", } - dataset_selection = DatasetSelection("native", "test-only-custom-data", False) + dataset_selection = DatasetSelection( + "native", "test-only-custom-data", is_default=False + ) simulation = _build_simulation( params, dataset=native_dataset, @@ -141,7 +145,7 @@ def test_native_state_only_worker_requires_geography_and_explicit_national_works policy=None, region_code="us", ) - with pytest.raises(ValueError) as error: + with pytest.raises(ValueError) as error: # noqa: PT011 # the typed code is asserted below; the message depends on which installed layer rejects the missing county simulation.run() assert spm_error_detail(error.value).code == "SPM_GEOGRAPHY_REQUIRED" national = _build_simulation( diff --git a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py index 6e6661d62..4b44a8b05 100644 --- a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py +++ b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py @@ -40,8 +40,10 @@ def gate_violations(workflow: dict) -> list[str]: gated_jobs = gate.get("needs") or [] if isinstance(gated_jobs, str): gated_jobs = [gated_jobs] - for job_id in sorted(set(jobs) - {GATE_JOB} - set(gated_jobs)): - violations.append(f"{GATE_JOB} does not need {job_id}") + violations.extend( + f"{GATE_JOB} does not need {job_id}" + for job_id in sorted(set(jobs) - {GATE_JOB} - set(gated_jobs)) + ) for job_id, job in jobs.items(): if job_id == GATE_JOB: diff --git a/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py b/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py index 4994d3d3b..51ec20a36 100644 --- a/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py +++ b/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py @@ -4,6 +4,7 @@ import json import os +from pathlib import Path import pytest from policyengine_simulation_executor.simulation_runtime import ( @@ -22,13 +23,13 @@ def test_setup_gcp_credentials_deletes_temp_file_on_exit( with setup_gcp_credentials(observability_runtime): path = os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") assert path is not None - assert os.path.exists(path) - with open(path) as file: + assert Path(path).exists() + with Path(path).open() as file: assert json.loads(file.read())["project_id"] == "p" # Temp file must be gone after the context exits, and the env var # must be cleared so a retry doesn't chase a missing path. - assert not os.path.exists(path) + assert not Path(path).exists() assert os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") is None @@ -40,14 +41,18 @@ def test_setup_gcp_credentials_cleans_up_on_exception( monkeypatch.delenv("GOOGLE_APPLICATION_CREDENTIALS", raising=False) captured_path: list[str] = [] - with pytest.raises(RuntimeError): + + def fail_inside_credentials_context() -> None: with setup_gcp_credentials(observability_runtime): captured_path.append(os.environ["GOOGLE_APPLICATION_CREDENTIALS"]) msg = "boom" raise RuntimeError(msg) + with pytest.raises(RuntimeError, match="boom"): + fail_inside_credentials_context() + assert captured_path - assert not os.path.exists(captured_path[0]) + assert not Path(captured_path[0]).exists() def test_setup_gcp_credentials_preserves_existing_env( diff --git a/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py b/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py index a24940b1f..335c3b885 100644 --- a/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py +++ b/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py @@ -132,7 +132,11 @@ def test_configured_validation_uses_the_bundle_access_plan(monkeypatch) -> None: monkeypatch.setattr( "policyengine_simulation_executor.hf_access_validation." "validate_uk_private_hf_access", - lambda targets: expected if targets == (target,) else pytest.fail(), + lambda targets: ( + expected + if targets == (target,) + else pytest.fail(f"unexpected access targets: {targets!r}") + ), ) assert validate_configured_uk_private_hf_access() == expected diff --git a/projects/policyengine-simulation-executor/tests/test_image_setup_fetch.py b/projects/policyengine-simulation-executor/tests/test_image_setup_fetch.py index dcd88b211..96b546f59 100644 --- a/projects/policyengine-simulation-executor/tests/test_image_setup_fetch.py +++ b/projects/policyengine-simulation-executor/tests/test_image_setup_fetch.py @@ -108,18 +108,16 @@ def test_downloads_every_artifact_under_its_runtime_filename( target = data_folder / artifact["filename"] assert target.read_bytes() == f"bytes-{artifact['digest']}".encode() - def test_missing_store_object_fails_before_any_download( - self, fake_client, data_folder - ): + @pytest.mark.usefixtures("data_folder") + def test_missing_store_object_fails_before_any_download(self, fake_client): missing_path = _manifest()["artifacts"][1]["path"] del fake_client.objects[missing_path] with pytest.raises(RuntimeError, match=missing_path): fetch_artifacts("test-bucket", _manifest(), client=fake_client) assert fake_client.downloads == [] - def test_unresolved_manifest_fails_with_the_digest_env_var_named( - self, fake_client, data_folder - ): + @pytest.mark.usefixtures("data_folder") + def test_unresolved_manifest_fails_with_the_digest_env_var_named(self, fake_client): with pytest.raises(RuntimeError, match="POLICYENGINE_MANIFEST_DIGEST"): fetch_artifacts("test-bucket", None, client=fake_client) assert fake_client.downloads == [] @@ -134,23 +132,22 @@ def test_overwrites_a_preexisting_same_named_file(self, fake_client, data_folder class TestFreshnessGate: - def test_rejects_a_policyengine_version_mismatch( - self, monkeypatch, fake_client, data_folder - ): + @pytest.mark.usefixtures("data_folder") + def test_rejects_a_policyengine_version_mismatch(self, monkeypatch, fake_client): monkeypatch.setenv("POLICYENGINE_VERSION", "4.99.0") with pytest.raises(RuntimeError, match=r"4\.99\.0.*4\.22\.0"): fetch_artifacts("test-bucket", _manifest(), client=fake_client) assert fake_client.downloads == [] - def test_rejects_a_model_version_mismatch( - self, monkeypatch, fake_client, data_folder - ): + @pytest.mark.usefixtures("data_folder") + def test_rejects_a_model_version_mismatch(self, monkeypatch, fake_client): monkeypatch.setenv("POLICYENGINE_US_VERSION", "9.9.9") with pytest.raises(RuntimeError, match="POLICYENGINE_US_VERSION"): fetch_artifacts("test-bucket", _manifest(), client=fake_client) + @pytest.mark.usefixtures("data_folder") def test_rejects_an_installed_data_version_mismatch( - self, monkeypatch, tmp_path, fake_client, data_folder + self, monkeypatch, tmp_path, fake_client ): receipt_file = tmp_path / "bundle-receipt.json" receipt_file.write_text('{"datasets": [{"country": "us", "version": "9.9.9"}]}') @@ -158,8 +155,9 @@ def test_rejects_an_installed_data_version_mismatch( with pytest.raises(RuntimeError, match=r"9\.9\.9.*1\.2\.3"): fetch_artifacts("test-bucket", _manifest(), client=fake_client) + @pytest.mark.usefixtures("data_folder") def test_passes_when_the_installed_data_version_matches( - self, monkeypatch, tmp_path, fake_client, data_folder + self, monkeypatch, tmp_path, fake_client ): receipt_file = tmp_path / "bundle-receipt.json" receipt_file.write_text('{"datasets": [{"country": "us", "version": "1.2.3"}]}') @@ -167,8 +165,9 @@ def test_passes_when_the_installed_data_version_matches( fetch_artifacts("test-bucket", _manifest(), client=fake_client) assert len(fake_client.downloads) == 2 + @pytest.mark.usefixtures("data_folder") def test_proceeds_without_an_installed_receipt_file( - self, monkeypatch, tmp_path, fake_client, data_folder + self, monkeypatch, tmp_path, fake_client ): """Lenient by design: local fault-drill runs have no bundle receipt file.""" monkeypatch.setenv( @@ -207,8 +206,9 @@ def fake_client_factory(**kwargs): monkeypatch.setattr(storage, "Client", fake_client_factory) return recorder + @pytest.mark.usefixtures("data_folder") def test_builds_in_memory_credentials_from_the_secret_blob( - self, monkeypatch, fake_client, data_folder, patched_google + self, monkeypatch, fake_client, patched_google ): monkeypatch.setenv( "GOOGLE_APPLICATION_CREDENTIALS_JSON", @@ -225,9 +225,8 @@ def test_builds_in_memory_credentials_from_the_secret_blob( ] assert len(fake_client.downloads) == 2 - def test_unwraps_the_double_encoded_blob( - self, monkeypatch, fake_client, data_folder, patched_google - ): + @pytest.mark.usefixtures("fake_client", "data_folder") + def test_unwraps_the_double_encoded_blob(self, monkeypatch, patched_google): clean = json.dumps({"type": "service_account", "project_id": "pe-test"}) monkeypatch.setenv( "GOOGLE_APPLICATION_CREDENTIALS_JSON", clean.replace('"', '\\"') @@ -239,8 +238,9 @@ def test_unwraps_the_double_encoded_blob( {"type": "service_account", "project_id": "pe-test"} ] + @pytest.mark.usefixtures("fake_client", "data_folder") def test_falls_back_to_ambient_adc_without_a_blob( - self, monkeypatch, fake_client, data_folder, patched_google + self, monkeypatch, patched_google ): monkeypatch.delenv("GOOGLE_APPLICATION_CREDENTIALS_JSON", raising=False) diff --git a/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py b/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py index af5682dd1..9220a8b07 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py @@ -166,7 +166,7 @@ def test_modal_image_fetches_artifacts_between_env_and_local_source(monkeypatch) manifest = _fake_manifest() class FakeStore: - def __init__(self, bucket_name=None, **kwargs): + def __init__(self, bucket_name=None, **_kwargs): self.bucket_name = bucket_name def read_manifest(self, digest): diff --git a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py index 83488c406..f518c6ea6 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py @@ -60,7 +60,7 @@ def test_extracts_versions_from_policyengine_bundle(self, temp_github_output): assert result.returncode == 0, f"Script failed: {result.stderr}" - with open(temp_github_output) as f: + with Path(temp_github_output).open() as f: output = f.read() assert "policyengine_version=" in output @@ -275,7 +275,7 @@ def test_generates_success_summary(self, temp_github_step_summary): assert result.returncode == 0, f"Script failed: {result.stderr}" - with open(temp_github_step_summary) as f: + with Path(temp_github_step_summary).open() as f: summary = f.read() assert "Simulation API Deployment Summary" in summary @@ -302,7 +302,7 @@ def test_generates_skipped_summary(self, temp_github_step_summary): assert result.returncode == 0 - with open(temp_github_step_summary) as f: + with Path(temp_github_step_summary).open() as f: summary = f.read() assert "Beta deployment" in summary @@ -925,7 +925,7 @@ class TestAllScriptsHaveShebang: def test_all_scripts_have_shebang(self, all_modal_scripts): """All scripts should start with #!/bin/bash.""" for script in all_modal_scripts: - with open(script) as f: + with script.open() as f: first_line = f.readline().strip() assert first_line == "#!/bin/bash", f"{script.name} missing shebang" diff --git a/projects/policyengine-simulation-executor/tests/test_national_partition.py b/projects/policyengine-simulation-executor/tests/test_national_partition.py index 0045acf4a..ef471b7a7 100644 --- a/projects/policyengine-simulation-executor/tests/test_national_partition.py +++ b/projects/policyengine-simulation-executor/tests/test_national_partition.py @@ -10,11 +10,59 @@ # 50 states + DC as they appear in region codes. ALL_STATE_CODES = { f"state/{s}" - for s in ( - "al ak az ar ca co ct de dc fl ga hi id il in ia ks ky la me md ma " - "mi mn ms mo mt ne nv nh nj nm ny nc nd oh ok or pa ri sc sd tn tx " - "ut vt va wa wv wi wy" - ).split() + for s in [ + "al", + "ak", + "az", + "ar", + "ca", + "co", + "ct", + "de", + "dc", + "fl", + "ga", + "hi", + "id", + "il", + "in", + "ia", + "ks", + "ky", + "la", + "me", + "md", + "ma", + "mi", + "mn", + "ms", + "mo", + "mt", + "ne", + "nv", + "nh", + "nj", + "nm", + "ny", + "nc", + "nd", + "oh", + "ok", + "or", + "pa", + "ri", + "sc", + "sd", + "tn", + "tx", + "ut", + "vt", + "va", + "wa", + "wv", + "wi", + "wy", + ] } diff --git a/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py b/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py index 2d58a02e5..93a125769 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py @@ -106,6 +106,7 @@ def _run_python_snippet( env=env, capture_output=True, text=True, + check=False, ) diff --git a/projects/policyengine-simulation-executor/tests/test_precompute.py b/projects/policyengine-simulation-executor/tests/test_precompute.py index 9b2d97a88..8c183fa80 100644 --- a/projects/policyengine-simulation-executor/tests/test_precompute.py +++ b/projects/policyengine-simulation-executor/tests/test_precompute.py @@ -10,6 +10,7 @@ import sys from pathlib import Path from types import ModuleType, SimpleNamespace +from typing import ClassVar import pandas as pd import pytest @@ -287,7 +288,7 @@ def _run(self, functions, *, force=False): def test_spawns_only_misses_and_gates_on_first_computed(self): functions = self._functions() - digest, lines = self._run(functions) + digest, _lines = self._run(functions) (dataset_spawn,) = functions["build_dataset"].spawn_calls assert dataset_spawn[0] == "bucket-x" @@ -354,7 +355,7 @@ def identity_stubs(self, monkeypatch): state = install_identity_stubs(monkeypatch) monkeypatch.setattr( - sr, "get_country_release_bundle", lambda country: state.bundle + sr, "get_country_release_bundle", lambda _country: state.bundle ) @pytest.fixture @@ -374,12 +375,13 @@ def get_region(code): monkeypatch.setattr( sr, "_country_module", - lambda country: SimpleNamespace( + lambda _country: SimpleNamespace( model=SimpleNamespace(get_region=get_region) ), ) - def test_precompute_identity_equals_runtime_id(self, identity_stubs, fake_regions): + @pytest.mark.usefixtures("identity_stubs", "fake_regions") + def test_precompute_identity_equals_runtime_id(self): from policyengine_simulation_executor import simulation_runtime as sr from policyengine_simulation_executor.baseline_artifacts import ( deterministic_baseline_id, @@ -466,12 +468,12 @@ def exists(self, path): monkeypatch.setattr( national_partition, "national_region_groups", - lambda country: [["state/ca"], ["state/ny", "state/wv"]], + lambda _country: [["state/ca"], ["state/ny", "state/wv"]], ) monkeypatch.setattr( artifact_keys, "collect_dataset_identity", - lambda country, year: SimpleNamespace( + lambda _country, year: SimpleNamespace( digest=f"ds-{year}", store_path=f"datasets/us/ds-{year}/populace_year_{year}.h5", filename=f"populace_year_{year}.h5", @@ -491,7 +493,7 @@ def fake_cohort_identity(year, group): monkeypatch.setattr( release_bundle, "get_country_release_bundle", - lambda country: SimpleNamespace( + lambda _country: SimpleNamespace( policyengine_version="4.22.0", model_version="1.0.0", data_version="1.2.3", @@ -501,7 +503,8 @@ def fake_cohort_identity(year, group): ) return SimpleNamespace(existing=existing) - def test_plan_enumerates_years_and_cohorts_with_presence(self, planning_stubs): + @pytest.mark.usefixtures("planning_stubs") + def test_plan_enumerates_years_and_cohorts_with_presence(self): plan = precompute.plan_artifacts_impl("bucket-x") assert [entry.year for entry in plan.datasets] == [2026, 2027, 2025] @@ -544,11 +547,12 @@ def test_plan_enumerates_years_and_cohorts_with_presence(self, planning_stubs): assert [entry.year for entry in work.datasets] == [2027, 2025] assert len(work.baselines) == 5 - def test_empty_partition_fails_loudly(self, planning_stubs, monkeypatch): + @pytest.mark.usefixtures("planning_stubs") + def test_empty_partition_fails_loudly(self, monkeypatch): from policyengine_simulation_executor import national_partition monkeypatch.setattr( - national_partition, "national_region_groups", lambda country: [] + national_partition, "national_region_groups", lambda _country: [] ) with pytest.raises(RuntimeError, match="No national partition"): precompute.plan_artifacts_impl("bucket-x") @@ -559,16 +563,16 @@ def test_diverged_predicates_fail_loudly(self, monkeypatch): from policyengine_simulation_executor import baseline_artifacts as ba from policyengine_simulation_executor import simulation_runtime as sr - monkeypatch.setattr(sr, "_country_module", lambda country: SimpleNamespace()) + monkeypatch.setattr(sr, "_country_module", lambda _country: SimpleNamespace()) monkeypatch.setattr( sr, "_resolve_region", - lambda **kwargs: SimpleNamespace( + lambda **_kwargs: SimpleNamespace( code="x", dataset_reference=None, scoping_strategy=None ), ) monkeypatch.setattr( - ba, "qualifying_baseline_identity", lambda *args, **kwargs: None + ba, "qualifying_baseline_identity", lambda *_args, **_kwargs: None ) with pytest.raises(RuntimeError, match="diverged"): precompute.cohort_identity(2026, ["state/ca"]) @@ -610,19 +614,21 @@ def fake_ensure_datasets(*, datasets, years, data_folder): (tmp_path / state.identity.filename).write_bytes(b"h5-bytes") monkeypatch.setattr( - artifact_keys, "collect_dataset_identity", lambda c, y: state.identity + artifact_keys, + "collect_dataset_identity", + lambda _country, _year: state.identity, ) monkeypatch.setattr(artifact_store, "ArtifactStore", FakeStore) monkeypatch.setattr(sr, "resolve_data_folder", lambda: str(tmp_path)) monkeypatch.setattr( sr, "_country_module", - lambda country: SimpleNamespace(ensure_datasets=fake_ensure_datasets), + lambda _country: SimpleNamespace(ensure_datasets=fake_ensure_datasets), ) monkeypatch.setattr( release_bundle, "get_country_release_bundle", - lambda country: SimpleNamespace(default_dataset="populace_cps"), + lambda _country: SimpleNamespace(default_dataset="populace_cps"), ) return state @@ -739,18 +745,18 @@ def upload_file(self, path, local_path): monkeypatch.setattr( artifact_keys, "collect_dataset_identity", - lambda country, year: SimpleNamespace( + lambda _country, year: SimpleNamespace( store_path=f"datasets/us/ds-{year}/populace_year_{year}.h5", filename=f"populace_year_{year}.h5", ), ) monkeypatch.setattr(artifact_store, "ArtifactStore", FakeStore) monkeypatch.setattr(sr, "resolve_data_folder", lambda: str(tmp_path)) - monkeypatch.setattr(sr, "_country_module", lambda country: SimpleNamespace()) + monkeypatch.setattr(sr, "_country_module", lambda _country: SimpleNamespace()) monkeypatch.setattr( sr, "_resolve_region", - lambda **kwargs: SimpleNamespace( + lambda **_kwargs: SimpleNamespace( code="state/ca+state/wv", scoping_strategy="scoping", dataset_reference=None, @@ -759,10 +765,10 @@ def upload_file(self, path, local_path): monkeypatch.setattr( sr, "_load_dataset", - lambda params, country_module, selection: "dataset", + lambda _params, country_module, selection: "dataset", # noqa: ARG005 # the runtime passes country_module= and selection= by keyword ) monkeypatch.setattr( - sr, "_build_simulation", lambda params, **kwargs: state.baseline + sr, "_build_simulation", lambda _params, **_kwargs: state.baseline ) return state @@ -966,7 +972,7 @@ class FakeBaseline: dataset = "dataset" tax_benefit_model_version = "model-version" scoping_strategy = "scoping" - extra_variables = {"extra": 1} + extra_variables: ClassVar[dict[str, int]] = {"extra": 1} artifact_outcome = None def ensure(self): @@ -987,7 +993,7 @@ def run(self): monkeypatch.setattr( precompute, "_prepare_cohort_baseline", - lambda bucket, expected: (FakeStore(), tmp_path, FakeBaseline()), + lambda _bucket, _expected: (FakeStore(), tmp_path, FakeBaseline()), ) monkeypatch.setattr("policyengine.core.Simulation", FakeFresh) if on_disk: diff --git a/projects/policyengine-simulation-executor/tests/test_precompute_app.py b/projects/policyengine-simulation-executor/tests/test_precompute_app.py index 89a08973b..78409b902 100644 --- a/projects/policyengine-simulation-executor/tests/test_precompute_app.py +++ b/projects/policyengine-simulation-executor/tests/test_precompute_app.py @@ -149,7 +149,7 @@ def test_wrappers_validate_dicts_in_and_dump_models_out(precompute_module, monke plan = _sample_plan() seen = {} - monkeypatch.setattr(lib, "plan_artifacts_impl", lambda bucket: plan) + monkeypatch.setattr(lib, "plan_artifacts_impl", lambda _bucket: plan) assert precompute_module.plan_artifacts("bucket-x") == plan.model_dump() dataset_result = DatasetBuildResult( @@ -190,7 +190,7 @@ def fake_compute_baseline(bucket, expected): verdict = DeterminismVerdict(equal=True, differences=[]) monkeypatch.setattr( - lib, "verify_determinism_impl", lambda bucket, expected: verdict + lib, "verify_determinism_impl", lambda _bucket, _expected: verdict ) assert ( precompute_module.verify_determinism("bucket-x", plan.baselines[0].model_dump()) @@ -232,7 +232,7 @@ def fake_run_precompute(bucket, *, force, **handles): return "digest" monkeypatch.setattr( - artifact_store, "resolve_bucket_name", lambda explicit=None: "bucket-env" + artifact_store, "resolve_bucket_name", lambda _explicit=None: "bucket-env" ) monkeypatch.setattr(lib, "run_precompute", fake_run_precompute) diff --git a/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py b/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py index d1f9f3122..9e8c022ce 100644 --- a/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py +++ b/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py @@ -22,7 +22,7 @@ def _region_with(strategy): def _country_module(regions: dict): module = MagicMock() - module.model.get_region.side_effect = lambda code: regions.get(code) + module.model.get_region.side_effect = regions.get return module diff --git a/projects/policyengine-simulation-executor/tests/test_release_bundle.py b/projects/policyengine-simulation-executor/tests/test_release_bundle.py index 12f7c5a06..430bea485 100644 --- a/projects/policyengine-simulation-executor/tests/test_release_bundle.py +++ b/projects/policyengine-simulation-executor/tests/test_release_bundle.py @@ -108,9 +108,8 @@ def test_country_release_bundle_exposes_model_and_data_versions(): assert bundle.default_dataset_uri == release["default_dataset_uri"] -def test_policyengine_data_release_keeps_build_and_package_versions_distinct( - policyengine_uk_data_release, -): +@pytest.mark.usefixtures("policyengine_uk_data_release") +def test_policyengine_data_release_keeps_build_and_package_versions_distinct(): bundle = get_country_release_bundle("uk") assert bundle.data_version == "policyengine-uk-data-1.56.16" @@ -232,17 +231,15 @@ def test_resolve_runtime_bundle_dataset_uri_maps_dataset_name_to_its_bundle_uri( ) -def test_policyengine_data_release_default_resolves_to_gcs_package_version( - policyengine_uk_data_release, -): +@pytest.mark.usefixtures("policyengine_uk_data_release") +def test_policyengine_data_release_default_resolves_to_gcs_package_version(): assert resolve_runtime_bundle_dataset_uri("uk", None, prefer_local=False) == ( "gs://policyengine-uk-data-private/enhanced_frs_2024_25.h5@1.56.16" ) -def test_policyengine_data_release_preserves_explicit_legacy_populace_dataset( - policyengine_uk_data_release, -): +@pytest.mark.usefixtures("policyengine_uk_data_release") +def test_policyengine_data_release_preserves_explicit_legacy_populace_dataset(): assert resolve_runtime_bundle_dataset_uri( "uk", "populace_uk_2023", prefer_local=False ) == ( diff --git a/projects/policyengine-simulation-executor/tests/test_release_sentinel.py b/projects/policyengine-simulation-executor/tests/test_release_sentinel.py index bf52aa4da..5e4b4228d 100644 --- a/projects/policyengine-simulation-executor/tests/test_release_sentinel.py +++ b/projects/policyengine-simulation-executor/tests/test_release_sentinel.py @@ -130,7 +130,7 @@ def country_bundle(country): SimpleNamespace(get_country_release_bundle=country_bundle), ) - expected = sentinel.compute_expected(runner=lambda payload: RESULT) + expected = sentinel.compute_expected(runner=lambda _payload: RESULT) assert distributions == ["policyengine"] assert countries == ["uk"] @@ -145,7 +145,7 @@ def country_bundle(country): def test_expected_needs_the_bundle_dataset_digest(): identity = {**IDENTITY, "uk": {**IDENTITY["uk"], "dataset_sha256": None}} with pytest.raises(sentinel.SentinelError, match="dataset SHA-256"): - sentinel.compute_expected(runner=lambda payload: RESULT, identity=identity) + sentinel.compute_expected(runner=lambda _payload: RESULT, identity=identity) class FakeAPI: @@ -167,7 +167,7 @@ def __init__( self.job_status = job_status self.calls = [] - def __call__(self, method, url, *, token=None, body=None, timeout=60): + def __call__(self, method, url, *, token=None, body=None): self.calls.append((method, url, token, body)) if method == "POST": return 200, {}, {"job_id": "fc-1", "policyengine_bundle": self.bundle} @@ -332,11 +332,12 @@ def test_a_job_that_never_finishes_fails(): def test_a_rejected_submit_fails(): - def api(method, url, **kwargs): + def api(_method, _url, **_kwargs): return 400, {}, {"detail": "unknown policyengine_version"} problems, _ = _check(api) - assert problems and "submit returned HTTP 400" in problems[0] + assert problems + assert "submit returned HTTP 400" in problems[0] def test_an_expected_file_of_another_schema_is_refused(): @@ -352,9 +353,11 @@ def test_an_expected_file_of_another_schema_is_refused(): def test_agreement_admits_only_reordering_error(value, factor): tolerance = max(sentinel.ABS_TOL, sentinel.REL_TOL * abs(value)) near = value + factor * tolerance * 0.5 - assert sentinel.agree(value, near) and sentinel.agree(near, value) + assert sentinel.agree(value, near) + assert sentinel.agree(near, value) far = value + 3 * tolerance - assert not sentinel.agree(value, far) and not sentinel.agree(far, value) + assert not sentinel.agree(value, far) + assert not sentinel.agree(far, value) def test_access_token_follows_the_auth_configuration(monkeypatch): @@ -365,12 +368,12 @@ def test_access_token_follows_the_auth_configuration(monkeypatch): sentinel.access_token({"SIMULATION_TEST_AUTH_ISSUER": "https://issuer"}) seen = {} - def http(method, url, *, body=None, **kwargs): + def http(method, url, *, body=None, **_kwargs): seen.update(method=method, url=url, body=body) return 200, {}, {"access_token": "minted"} monkeypatch.setattr(sentinel, "_http", http) - environ = {name: "x" for name in sentinel._AUTH_VARS} + environ = dict.fromkeys(sentinel._AUTH_VARS, "x") environ["SIMULATION_TEST_AUTH_ISSUER"] = "https://issuer/" assert sentinel.access_token(environ) == "minted" assert seen["url"] == "https://issuer/oauth/token" diff --git a/projects/policyengine-simulation-executor/tests/test_segmented_national.py b/projects/policyengine-simulation-executor/tests/test_segmented_national.py index 2df10950e..958adb623 100644 --- a/projects/policyengine-simulation-executor/tests/test_segmented_national.py +++ b/projects/policyengine-simulation-executor/tests/test_segmented_national.py @@ -21,7 +21,9 @@ def _bare_country_module(): """Return a country module whose model has no loadable region registry.""" return SimpleNamespace( - model=SimpleNamespace(get_region=lambda code: None, region_registry=None) + model=SimpleNamespace( + get_region=lambda _region_code: None, region_registry=None + ) ) @@ -161,7 +163,7 @@ def __init__( self.cancel_attempts = 0 self.poll_count = 0 - def get(self, timeout): + def get(self, timeout): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword self.poll_count += 1 if self.before_each_poll is not None: self.before_each_poll() @@ -232,17 +234,18 @@ def _twenty_calls(**kwargs): @pytest.fixture def bare_country(monkeypatch): - monkeypatch.setattr(sr, "_country_module", lambda c: _bare_country_module()) + monkeypatch.setattr(sr, "_country_module", lambda _country: _bare_country_module()) class TestSegmentedNationalRunner: - def test__spawns_one_child_per_group_into_the_segment_pool( - self, monkeypatch, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__spawns_one_child_per_group_into_the_segment_pool(self, monkeypatch): fake = FakeModal(_twenty_calls()) runner = _runner(fake) monkeypatch.setattr( - runner, "_reduce", lambda results, country_module: {"ok": results} + runner, + "_reduce", + lambda results, country_module: {"ok": results}, # noqa: ARG005 # run() passes country_module= by keyword ) result = runner.run() @@ -266,70 +269,67 @@ def test__spawns_one_child_per_group_into_the_segment_pool( ) assert result == {"ok": [{"child": i} for i in range(20)]} - def test__collects_out_of_order_completions_in_group_order( - self, monkeypatch, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__collects_out_of_order_completions_in_group_order(self, monkeypatch): calls = [ FakeCall({"child": i}, polls_until_ready=(19 - i) % 3) for i in range(20) ] runner = _runner(FakeModal(calls)) - monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) + monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) # noqa: ARG005 # run() passes country_module= by keyword assert runner.run() == [{"child": i} for i in range(20)] - def test__child_failure_fails_fast_and_cancels_the_rest( - self, monkeypatch, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__child_failure_fails_fast_and_cancels_the_rest(self, monkeypatch): calls = _twenty_calls(polls_until_ready=5) calls[7] = FakeCall(None, error=ValueError("boom")) runner = _runner(FakeModal(calls)) - monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) + monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) # noqa: ARG005 # run() passes country_module= by keyword with pytest.raises(RuntimeError, match="Segmented national child failed"): runner.run() assert all(c.cancelled for c in calls if c.error is None) - def test__one_transient_poll_error_is_tolerated(self, monkeypatch, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__one_transient_poll_error_is_tolerated(self, monkeypatch): # A single control-plane blip on one handle must not kill the job. calls = _twenty_calls() calls[3] = FakeCall({"child": 3}, errors_once=1) runner = _runner(FakeModal(calls)) - monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) + monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) # noqa: ARG005 # run() passes country_module= by keyword assert runner.run() == [{"child": i} for i in range(20)] - def test__spawn_failure_cancels_already_spawned_children( - self, monkeypatch, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__spawn_failure_cancels_already_spawned_children(self, monkeypatch): calls = _twenty_calls(polls_until_ready=100) fake = FakeModal(calls, fail_spawn_at=15) runner = _runner(fake) - monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) + monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) # noqa: ARG005 # run() passes country_module= by keyword with pytest.raises(ConnectionError): runner.run() assert all(c.cancelled for c in calls[:15]) - def test__uncovered_registry_states_ride_in_the_last_group( - self, monkeypatch, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__uncovered_registry_states_ride_in_the_last_group(self, monkeypatch): partition_codes = [ code for group in US_NATIONAL_REGION_GROUPS for code in group ] registry = SimpleNamespace( regions=[ SimpleNamespace(code=code, region_type="state") - for code in partition_codes + ["state/pr"] + for code in [*partition_codes, "state/pr"] ] ) country_module = SimpleNamespace( model=SimpleNamespace( - get_region=lambda code: None, region_registry=registry + get_region=lambda _region_code: None, region_registry=registry ) ) - monkeypatch.setattr(sr, "_country_module", lambda c: country_module) + monkeypatch.setattr(sr, "_country_module", lambda _country: country_module) fake = FakeModal([FakeCall({"child": i}) for i in range(20)]) runner = _runner(fake) - monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) + monkeypatch.setattr(runner, "_reduce", lambda results, country_module: results) # noqa: ARG005 # run() passes country_module= by keyword runner.run() assert runner.groups[-1][-1] == "state/pr" @@ -348,7 +348,9 @@ def fake_build_national_output(child_results, **kwargs): return {"budget": {}} monkeypatch.setattr(sn, "build_national_output", fake_build_national_output) - monkeypatch.setattr(sr, "_country_module", lambda c: _bare_country_module()) + monkeypatch.setattr( + sr, "_country_module", lambda _country: _bare_country_module() + ) params = { **NATIONAL, @@ -412,7 +414,7 @@ def _stuck_call(clock): ) -def _crashing_call(clock): +def _crashing_call(_clock): """Return a child that fails outright (takes a clock to match ``_stuck_call``).""" return FakeCall(None, error=ValueError("child crashed")) @@ -424,13 +426,14 @@ def _calls_with_stuck_child(clock, respawned_call, **sibling_options): class TestSegmentedNationalChildDeadline: - def test__overdue_child_is_cancelled_and_respawned_once(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__overdue_child_is_cancelled_and_respawned_once(self): clock = ManualClock() respawned_call = FakeCall({"child": "respawned"}) calls = _calls_with_stuck_child(clock, respawned_call) fake = FakeModal(calls) runner = _runner(fake, clock=clock) - runner._reduce = lambda results, country_module: results + runner._reduce = lambda results, country_module: results # noqa: ARG005 # run() passes country_module= by keyword results = runner.run() @@ -451,7 +454,8 @@ def test__overdue_child_is_cancelled_and_respawned_once(self, bare_country): # deadline, not later; the respawn finished without moving the clock. assert clock() == sn.CHILD_DEADLINE_SECONDS - def test__respawned_child_that_is_also_overdue_fails_the_run(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__respawned_child_that_is_also_overdue_fails_the_run(self): clock = ManualClock() calls = _calls_with_stuck_child(clock, _stuck_call(clock)) runner = _runner(FakeModal(calls), clock=clock) @@ -466,7 +470,8 @@ def test__respawned_child_that_is_also_overdue_fails_the_run(self, bare_country) assert calls[STUCK_GROUP_INDEX].cancelled assert calls[-1].cancelled - def test__respawned_child_failure_names_the_respawn(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__respawned_child_failure_names_the_respawn(self): clock = ManualClock() failing_respawn = FakeCall(None, error=ValueError("child crashed")) calls = _calls_with_stuck_child(clock, failing_respawn) @@ -479,7 +484,8 @@ def test__respawned_child_failure_names_the_respawn(self, bare_country): runner.run() assert calls[STUCK_GROUP_INDEX].cancelled - def test__failed_respawn_spawn_cancels_every_child(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__failed_respawn_spawn_cancels_every_child(self): clock = ManualClock() runtime = RecordingRuntime() calls = _calls_with_stuck_child( @@ -500,7 +506,8 @@ def test__failed_respawn_spawn_cancels_every_child(self, bare_country): for update in runtime.context_updates ) - def test__late_result_from_the_superseded_child_is_ignored(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__late_result_from_the_superseded_child_is_ignored(self): clock = ManualClock() # The original's next poll after its deadline would return a result. late_original = FakeCall( @@ -512,7 +519,7 @@ def test__late_result_from_the_superseded_child_is_ignored(self, bare_country): calls[STUCK_GROUP_INDEX] = late_original calls.append(FakeCall({"child": "respawned"}, polls_until_ready=2)) runner = _runner(FakeModal(calls), clock=clock) - runner._reduce = lambda results, country_module: results + runner._reduce = lambda results, country_module: results # noqa: ARG005 # run() passes country_module= by keyword results = runner.run() @@ -526,8 +533,9 @@ def test__late_result_from_the_superseded_child_is_ignored(self, bare_country): [_crashing_call, _stuck_call], ids=["respawn_fails", "respawn_also_overdue"], ) + @pytest.mark.usefixtures("bare_country") def test__superseded_child_whose_cancel_failed_is_cancelled_on_a_later_failure( - self, bare_country, respawned_call_factory + self, respawned_call_factory ): clock = ManualClock() calls = _calls_with_stuck_child(clock, respawned_call_factory(clock)) @@ -544,15 +552,14 @@ def test__superseded_child_whose_cancel_failed_is_cancelled_on_a_later_failure( assert superseded_original.cancelled assert superseded_original.poll_count == POLLS_TO_REACH_DEADLINE - def test__superseded_child_whose_cancel_failed_is_cancelled_after_collection( - self, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__superseded_child_whose_cancel_failed_is_cancelled_after_collection(self): clock = ManualClock() calls = _calls_with_stuck_child(clock, FakeCall({"child": "respawned"})) superseded_original = calls[STUCK_GROUP_INDEX] superseded_original.cancel_failures = 1 runner = _runner(FakeModal(calls), clock=clock) - runner._reduce = lambda results, country_module: results + runner._reduce = lambda results, country_module: results # noqa: ARG005 # run() passes country_module= by keyword results = runner.run() @@ -560,7 +567,8 @@ def test__superseded_child_whose_cancel_failed_is_cancelled_after_collection( assert superseded_original.cancel_attempts == 2 assert superseded_original.cancelled - def test__no_respawn_when_parent_budget_cannot_fit_a_retry(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__no_respawn_when_parent_budget_cannot_fit_a_retry(self): clock = ManualClock() calls = _calls_with_stuck_child(clock, FakeCall({"child": "respawned"})) fake = FakeModal(calls) @@ -579,9 +587,8 @@ def test__no_respawn_when_parent_budget_cannot_fit_a_retry(self, bare_country): assert len(fake.spawned_payloads) == 20 assert calls[STUCK_GROUP_INDEX].cancelled - def test__respawn_deadline_is_clamped_to_the_remaining_parent_budget( - self, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__respawn_deadline_is_clamped_to_the_remaining_parent_budget(self): clock = ManualClock() runtime = RecordingRuntime() calls = _calls_with_stuck_child(clock, _stuck_call(clock)) @@ -603,9 +610,8 @@ def test__respawn_deadline_is_clamped_to_the_remaining_parent_budget( assert timeout_event["attributes"]["deadline_seconds"] == 600 assert not runtime.events_named("segmented_national_budget_exhausted") - def test__exhausted_parent_budget_cancels_children_before_modal_timeout( - self, bare_country - ): + @pytest.mark.usefixtures("bare_country") + def test__exhausted_parent_budget_cancels_children_before_modal_timeout(self): clock = ManualClock() calls = _calls_with_stuck_child(clock, FakeCall({"child": "respawned"})) fake = FakeModal(calls) @@ -627,12 +633,13 @@ def test__exhausted_parent_budget_cancels_children_before_modal_timeout( assert calls[STUCK_GROUP_INDEX].cancelled assert len(fake.spawned_payloads) == 20 - def test__respawn_is_recorded_in_telemetry(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__respawn_is_recorded_in_telemetry(self): clock = ManualClock() runtime = RecordingRuntime() calls = _calls_with_stuck_child(clock, FakeCall({"child": "respawned"})) runner = _runner(FakeModal(calls), runtime=runtime, clock=clock) - runner._reduce = lambda results, country_module: results + runner._reduce = lambda results, country_module: results # noqa: ARG005 # run() passes country_module= by keyword runner.run() @@ -646,7 +653,11 @@ def test__respawn_is_recorded_in_telemetry(self, bare_country): assert {"segmented_child_retry_count": 1} in runtime.context_updates @pytest.mark.parametrize( - "parent_budget_seconds,expected_attempt,expected_remaining_budget_seconds", + ( + "parent_budget_seconds", + "expected_attempt", + "expected_remaining_budget_seconds", + ), # The first budget fits both attempts with 600 s to spare; the second # leaves too little after the first attempt for a respawn. [ @@ -655,9 +666,9 @@ def test__respawn_is_recorded_in_telemetry(self, bare_country): ], ids=["respawn_also_overdue", "no_budget_for_a_respawn"], ) + @pytest.mark.usefixtures("bare_country") def test__child_timeout_is_recorded_in_telemetry( self, - bare_country, parent_budget_seconds, expected_attempt, expected_remaining_budget_seconds, @@ -688,7 +699,8 @@ def test__child_timeout_is_recorded_in_telemetry( } ] - def test__exhausted_parent_budget_is_recorded_in_telemetry(self, bare_country): + @pytest.mark.usefixtures("bare_country") + def test__exhausted_parent_budget_is_recorded_in_telemetry(self): clock = ManualClock() runtime = RecordingRuntime() calls = _calls_with_stuck_child(clock, FakeCall({"child": "respawned"})) @@ -736,16 +748,16 @@ class TestDispatchRunSimulation: def test__eligible_national_takes_the_segmented_path(self, monkeypatch): calls = {} - def fake_segmented(params, *, app_name, runtime): + def fake_segmented(params, *, app_name, runtime): # noqa: ARG001 # keyword-only, as dispatch_run_simulation passes it calls["segmented"] = (params, app_name) return {"segmented": True} monkeypatch.setattr(sn, "run_segmented_national_impl", fake_segmented) - monkeypatch.setattr(executor_spm, "normalize_runtime_spm", lambda params: None) + monkeypatch.setattr(executor_spm, "normalize_runtime_spm", lambda _params: None) monkeypatch.setattr( sr, "run_simulation_impl", - lambda params, **kwargs: pytest.fail("monolithic path must not run"), + lambda _params, **_kwargs: pytest.fail("monolithic path must not run"), ) result = sn.dispatch_run_simulation( dict(NATIONAL), app_name="app-x", runtime=TEST_RUNTIME @@ -765,11 +777,11 @@ def test__everything_else_takes_the_monolithic_path(self, monkeypatch, params): monkeypatch.setattr( sn, "run_segmented_national_impl", - lambda *a, **k: pytest.fail("segmented path must not run"), + lambda *_args, **_kwargs: pytest.fail("segmented path must not run"), ) - monkeypatch.setattr(executor_spm, "normalize_runtime_spm", lambda params: None) + monkeypatch.setattr(executor_spm, "normalize_runtime_spm", lambda _params: None) monkeypatch.setattr( - sr, "run_simulation_impl", lambda params, **kwargs: {"monolithic": True} + sr, "run_simulation_impl", lambda _params, **_kwargs: {"monolithic": True} ) assert sn.dispatch_run_simulation( params, app_name="app-x", runtime=TEST_RUNTIME @@ -838,14 +850,13 @@ def stub_reduce(monkeypatch): The reduce itself is covered by test_segmented_national_reduce. """ monkeypatch.setattr( - sn, "build_national_output", lambda children, **kwargs: {"budget": {}} + sn, "build_national_output", lambda _children, **_kwargs: {"budget": {}} ) class TestSegmentedNationalSPM: - def test__child_receipts_combine_into_one_national_receipt( - self, bare_country, stub_reduce - ): + @pytest.mark.usefixtures("bare_country", "stub_reduce") + def test__child_receipts_combine_into_one_national_receipt(self): fake = FakeModal([FakeCall(_spm_child(i)) for i in range(20)]) runner = _runner(fake, params={**NATIONAL, "spm": SPM_SELECTION}) @@ -865,7 +876,7 @@ def test__child_receipts_combine_into_one_national_receipt( assert output["budget"] == {} @pytest.mark.parametrize( - "rogue,reason", + ("rogue", "reason"), [ ( _spm_child(7, selection={**SPM_SELECTION, "scenario": "zero_real"}), @@ -875,9 +886,8 @@ def test__child_receipts_combine_into_one_national_receipt( (_spm_child_missing(7, "county_vintage"), "complete resolved"), ], ) - def test__child_receipt_disagreeing_with_the_request_is_typed( - self, bare_country, stub_reduce, rogue, reason - ): + @pytest.mark.usefixtures("bare_country", "stub_reduce") + def test__child_receipt_disagreeing_with_the_request_is_typed(self, rogue, reason): # One segment computed something other than what was requested: # fail with the public typed code, never publish a mixed national # result whose receipt does not describe every segment. @@ -889,7 +899,8 @@ def test__child_receipt_disagreeing_with_the_request_is_typed( runner.run() assert error.value.code == "SPM_CONFIGURATION_UNAVAILABLE" - def test__unrequested_child_receipt_is_typed(self, bare_country, stub_reduce): + @pytest.mark.usefixtures("bare_country", "stub_reduce") + def test__unrequested_child_receipt_is_typed(self): # A historical request must not inherit a child's stray receipt. calls = [FakeCall({"child": i}) for i in range(20)] calls[3] = FakeCall(_spm_child(3)) @@ -898,7 +909,8 @@ def test__unrequested_child_receipt_is_typed(self, bare_country, stub_reduce): with pytest.raises(SPMInputError, match="Unexpected SPM receipt"): runner.run() - def test__a_run_without_spm_adds_no_receipt_keys(self, bare_country, stub_reduce): + @pytest.mark.usefixtures("bare_country", "stub_reduce") + def test__a_run_without_spm_adds_no_receipt_keys(self): runner = _runner(FakeModal([FakeCall({"child": i}) for i in range(20)])) assert runner.run() == {"budget": {}} @@ -908,11 +920,11 @@ def test__selection_is_resolved_before_the_fan_out(self, monkeypatch): # Children and the parent's receipt check must both see the # worker-resolved selection, not the caller's partial one. monkeypatch.setattr( - executor_spm, "normalize_runtime_spm", lambda params: SPM_SELECTION + executor_spm, "normalize_runtime_spm", lambda _params: SPM_SELECTION ) seen = {} - def fake_segmented(params, *, app_name, runtime): + def fake_segmented(params, *, app_name, runtime): # noqa: ARG001 # keyword-only, as dispatch_run_simulation passes them seen["params"] = params return {"segmented": True} @@ -930,7 +942,7 @@ def fake_segmented(params, *, app_name, runtime): def test__typed_spm_error_spawns_nothing(self, monkeypatch): # Resolution failures are cheap only if they happen before 20 # children are spawned. - def unavailable(params): + def unavailable(_params): error_code = "SPM_GEOGRAPHY_REQUIRED" raise SPMInputError(error_code, "County required") @@ -938,12 +950,12 @@ def unavailable(params): monkeypatch.setattr( sn, "run_segmented_national_impl", - lambda *a, **k: pytest.fail("segmented path must not run"), + lambda *_args, **_kwargs: pytest.fail("segmented path must not run"), ) monkeypatch.setattr( sr, "run_simulation_impl", - lambda params, **kwargs: pytest.fail("monolithic path must not run"), + lambda _params, **_kwargs: pytest.fail("monolithic path must not run"), ) with pytest.raises(SPMInputError) as error: diff --git a/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py b/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py index e335019c2..1a9b8db4d 100644 --- a/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py +++ b/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py @@ -69,7 +69,7 @@ def test__rejects_empty_input(self): class TestPrecomputedSimulation: def test__ensure_never_computes(self, monkeypatch): - def boom(self): + def boom(_simulation): msg = "RECOMPUTE ATTEMPTED" raise AssertionError(msg) diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py index 87fe8d2af..d828a1f42 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py @@ -43,7 +43,7 @@ def test_households_counts_each_weight_once_on_weighted_frames(): # Households weighted 1 and 2 are 3 households, not 1² + 2² = 5. assert budget.households == 3.0 - # The other totals stay weighted sums, Σ weight × value. + # The other totals stay weighted sums, Σ weight * value. assert budget.baseline_net_income == 500.0 assert budget.tax_revenue_impact == 25.0 assert budget.state_tax_revenue_impact == 8.0 diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py index 77e1ccf79..fe09f7051 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py @@ -203,6 +203,7 @@ def _simulation_output_builder( baseline, reform, analysis=None, + *, include_cliffs: bool | None = None, resolved_region_code: str | None = "us", uk_local_authority_metadata: UKLocalAuthorityMetadata | None = None, @@ -210,10 +211,10 @@ def _simulation_output_builder( analysis = analysis or fake_analysis() def economic_impact_analysis( - baseline_simulation, - reform_simulation, + _baseline_simulation, + _reform_simulation, *, - include_cliff_impacts=False, + include_cliff_impacts=False, # noqa: ARG001 # keyword-only, as SimulationOutputBuilder passes it ): return analysis @@ -280,7 +281,7 @@ def _local_authority_impact_record() -> LocalAuthorityImpactRecord: ) -def _stub_policyengine_output_calls(monkeypatch, baseline, reform) -> None: +def _stub_policyengine_output_calls(monkeypatch, baseline, _reform) -> None: def fake_poverty_module_function(name): def compute(simulation): if "by_age" in name: @@ -521,7 +522,7 @@ def fake_build_simulation( params, *, dataset, - dataset_selection, + dataset_selection, # noqa: ARG001 # keyword-only, as the runtime passes it policy, scoping_strategy=None, region_code=None, @@ -543,11 +544,11 @@ def serialize(self): monkeypatch.delenv("SERVICE_ACCOUNT_JSON", raising=False) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._country_module", - lambda country: country_module, + lambda _country: country_module, ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._resolve_region", - lambda **kwargs: RegionResolution( + lambda **_kwargs: RegionResolution( code="us", dataset_reference=None, scoping_strategy="mock-scoping", @@ -555,7 +556,7 @@ def serialize(self): ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._load_dataset", - lambda params, country_module, selection: dataset, + lambda _params, country_module, selection: dataset, # noqa: ARG005 # the runtime passes country_module= and selection= by keyword ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._build_simulation", @@ -652,13 +653,13 @@ def test_run_simulation_impl_exports_baseline_artifact_outcome(monkeypatch): build_count = [0] def fake_build_simulation( - params, + _params, *, - dataset, - dataset_selection, - policy, - scoping_strategy=None, - region_code=None, + dataset, # noqa: ARG001 # keyword-only, as the runtime passes it + dataset_selection, # noqa: ARG001 # keyword-only, as the runtime passes it + policy, # noqa: ARG001 # keyword-only, as the runtime passes it + scoping_strategy=None, # noqa: ARG001 # keyword-only, as the runtime passes it + region_code=None, # noqa: ARG001 # keyword-only, as the runtime passes it ): build_count[0] += 1 return simulations["baseline"] if build_count[0] == 1 else simulations["reform"] @@ -680,11 +681,11 @@ def serialize(self): monkeypatch.delenv(env, raising=False) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._country_module", - lambda country: country_module, + lambda _country: country_module, ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._resolve_region", - lambda **kwargs: RegionResolution( + lambda **_kwargs: RegionResolution( code="us", dataset_reference=None, scoping_strategy="mock-scoping", @@ -692,7 +693,7 @@ def serialize(self): ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._load_dataset", - lambda params, country_module, selection: dataset, + lambda _params, country_module, selection: dataset, # noqa: ARG005 # the runtime passes country_module= and selection= by keyword ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._build_simulation", @@ -761,7 +762,7 @@ def economic_impact_analysis( output_calls = [] def record(name, value): - def wrapper(*args, **kwargs): + def wrapper(*_args, **_kwargs): output_calls.append(name) return value @@ -985,8 +986,8 @@ def test_builder_passes_include_cliffs_to_policyengine_economic_impact_analysis( calls = [] def economic_impact_analysis( - baseline_simulation, - reform_simulation, + _baseline_simulation, + _reform_simulation, *, include_cliff_impacts=False, ): @@ -1063,10 +1064,10 @@ def fake_build_simulation( params, *, dataset, - dataset_selection, + dataset_selection, # noqa: ARG001 # keyword-only, as the runtime passes it policy, scoping_strategy=None, - region_code=None, + region_code=None, # noqa: ARG001 # keyword-only, as the runtime passes it ): build_calls.append((params, dataset, policy, scoping_strategy)) return baseline_simulation if len(build_calls) == 1 else reform_simulation @@ -1084,11 +1085,11 @@ def serialize(self): ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._resolve_region", - lambda **kwargs: RegionResolution(code="us", dataset_reference=None), + lambda **_kwargs: RegionResolution(code="us", dataset_reference=None), ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._load_dataset", - lambda params, country_module, selection: dataset, + lambda _params, country_module, selection: dataset, # noqa: ARG005 # the runtime passes country_module= and selection= by keyword ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._build_simulation", @@ -1159,14 +1160,14 @@ def fake_build_simulation( dataset_selection, policy, scoping_strategy=None, - region_code=None, + region_code=None, # noqa: ARG001 # keyword-only, as the runtime passes it ): build_calls.append( (params, dataset, policy, scoping_strategy, dataset_selection) ) return baseline_simulation if len(build_calls) == 1 else reform_simulation - def fake_load_dataset(params, *, country_module, selection): + def fake_load_dataset(_params, *, country_module, selection): # noqa: ARG001 # keyword-only, as the runtime passes it load_selections.append(selection) return dataset @@ -1179,11 +1180,11 @@ def serialize(self): monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._country_module", - lambda country: country_module, + lambda _country: country_module, ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._resolve_region", - lambda **kwargs: region_resolution, + lambda **_kwargs: region_resolution, ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._load_dataset", @@ -1236,19 +1237,19 @@ def serialize(self): monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._country_module", - lambda country: country_module, + lambda _country: country_module, ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._resolve_region", - lambda **kwargs: RegionResolution(code="uk", dataset_reference=None), + lambda **_kwargs: RegionResolution(code="uk", dataset_reference=None), ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._load_dataset", - lambda params, country_module, selection: dataset, + lambda _params, country_module, selection: dataset, # noqa: ARG005 # the runtime passes country_module= and selection= by keyword ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime._build_simulation", - lambda *args, **kwargs: object(), + lambda *_args, **_kwargs: object(), ) monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime.SimulationOutputBuilder", @@ -1545,9 +1546,7 @@ def test_resolve_region_scopes_us_state_from_national_dataset(): parent_code=None, ) regions = {"state/ut": state, "us": national} - country_module = SimpleNamespace( - model=SimpleNamespace(get_region=lambda code: regions.get(code)) - ) + country_module = SimpleNamespace(model=SimpleNamespace(get_region=regions.get)) resolution = _resolve_region( country_module=country_module, @@ -1585,9 +1584,7 @@ def test_resolve_region_scopes_us_congressional_district_from_national_dataset() "state/ut": state, "us": national, } - country_module = SimpleNamespace( - model=SimpleNamespace(get_region=lambda code: regions.get(code)) - ) + country_module = SimpleNamespace(model=SimpleNamespace(get_region=regions.get)) resolution = _resolve_region( country_module=country_module, @@ -1614,9 +1611,7 @@ def test_resolve_region_rejects_unscoped_us_place_region(): parent_code="us", ) regions = {"place/UT-67000": place, "state/ut": state} - country_module = SimpleNamespace( - model=SimpleNamespace(get_region=lambda code: regions.get(code)) - ) + country_module = SimpleNamespace(model=SimpleNamespace(get_region=regions.get)) with pytest.raises(ValueError, match="US place regions are not yet supported"): _resolve_region( @@ -1640,9 +1635,7 @@ def test_resolve_region_scopes_uk_country_from_national_dataset(): parent_code=None, ) regions = {"country/england": england, "uk": uk} - country_module = SimpleNamespace( - model=SimpleNamespace(get_region=lambda code: regions.get(code)) - ) + country_module = SimpleNamespace(model=SimpleNamespace(get_region=regions.get)) resolution = _resolve_region( country_module=country_module, @@ -1661,10 +1654,10 @@ def test_builder_data_version_prefers_resolved_revision_then_dataset_metadata(): baseline, reform = _macro_baseline_reform() def economic_impact_analysis( - baseline_simulation, - reform_simulation, + _baseline_simulation, + _reform_simulation, *, - include_cliff_impacts=False, + include_cliff_impacts=False, # noqa: ARG001 # keyword-only, as SimulationOutputBuilder passes it ): return fake_analysis() @@ -1741,7 +1734,7 @@ def test_builder_budgetary_impact_uses_materialized_columns_and_uk_state_tax_zer def test_builder_budgetary_impact_propagates_required_calculation_errors(monkeypatch): baseline, reform = _macro_baseline_reform() - def fail_change_output_variable(*args, **kwargs): + def fail_change_output_variable(*_args, **_kwargs): msg = "household_tax missing" raise RuntimeError(msg) @@ -1787,7 +1780,7 @@ def compute(baseline_simulation, reform_simulation, **kwargs): monkeypatch.setattr( "policyengine_simulation_executor.simulation_output_geographic." "_required_uk_geography_lookup_csv_path", - lambda spec: CONSTITUENCY_CSV_PATH, + lambda _spec: CONSTITUENCY_CSV_PATH, ) result = _simulation_output_builder( diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py index 18d1442f7..020aeede8 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py @@ -82,7 +82,7 @@ def test_required_uk_lookup_rejects_missing_asset(monkeypatch) -> None: ) monkeypatch.setattr( "policyengine.outputs.uk_geography_impact.resolve_uk_geography_lookup_csv_path", - lambda *args, **kwargs: None, + lambda *_args, **_kwargs: None, ) with pytest.raises(FileNotFoundError, match="constituency lookup CSV"): diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py b/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py index 6845e540d..de1858686 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py @@ -1,5 +1,6 @@ """Policy compilation against the installed wrapper, without running a model.""" +import re from datetime import datetime import pytest @@ -109,9 +110,9 @@ def test_unknown_parameter_retains_wrapper_error_and_close_match_hint(uk): unknown = "gov.hmrc.income_tax.rates.uk[0].rat" raw = {unknown: {"2027-04-06.2028-04-05": 0.21}} - with pytest.raises(ValueError) as expected: + with pytest.raises(ValueError, match=re.escape(unknown)) as expected: wrapper_reform.compile_reform(raw, year=2027, model_version=uk.model) - with pytest.raises(ValueError) as actual: + with pytest.raises(ValueError, match=re.escape(unknown)) as actual: _compile_interval_policy(raw, country_module=uk, year=2027) assert str(actual.value) == str(expected.value) @@ -133,9 +134,9 @@ def test_unknown_parameter_retains_wrapper_error_and_close_match_hint(uk): def test_invalid_policy_period_names_key_in_error(uk, key): raw = {UK_RATE: {"2026-04-06.2027-04-05": 0.20, key: 0.21}} - with pytest.raises(ValueError) as normalise_error: + with pytest.raises(ValueError, match=re.escape(key)) as normalise_error: _normalise_policy(raw) - with pytest.raises(ValueError) as compile_error: + with pytest.raises(ValueError, match=re.escape(key)) as compile_error: _compile_interval_policy(raw, country_module=uk, year=2027) assert key in str(normalise_error.value) @@ -143,14 +144,15 @@ def test_invalid_policy_period_names_key_in_error(uk, key): @pytest.mark.parametrize( - "period,year", + ("period", "year"), [ ({"time_period": "2031", "year": "2030"}, 2031), ({"year": "2032"}, 2032), ({}, DEFAULT_YEAR), ], ) -def test_scalar_in_interval_policy_uses_request_year(uk, period, year): +@pytest.mark.usefixtures("uk") +def test_scalar_in_interval_policy_uses_request_year(period, year): params = {"country": "uk", "scope": "macro", **period} raw = { UK_RATE: {"2027-04-06.2028-04-05": 0.21}, @@ -163,7 +165,7 @@ def test_scalar_in_interval_policy_uses_request_year(uk, period, year): simulation = _build_simulation( params, dataset=dataset, - dataset_selection=DatasetSelection("in-memory", "in-memory", False), + dataset_selection=DatasetSelection("in-memory", "in-memory", is_default=False), policy=raw, region_code="uk", ) @@ -225,7 +227,7 @@ def recording_compiler(reform, *, year, model_version): simulation = _build_simulation( {"country": "uk", "scope": "macro", "time_period": "2027"}, dataset=Dataset(name="in-memory", description="Compilation only", year=2027), - dataset_selection=DatasetSelection("in-memory", "in-memory", False), + dataset_selection=DatasetSelection("in-memory", "in-memory", is_default=False), policy=raw, region_code="uk", ) @@ -244,7 +246,7 @@ def recording_compiler(reform, *, year, model_version): @pytest.mark.parametrize( - "key, start, stop", + ("key", "start", "stop"), [ ("2027-4-6", _naive_policy_date(2027, 4, 6), None), ( diff --git a/projects/policyengine-simulation-executor/tests/test_smoke_app.py b/projects/policyengine-simulation-executor/tests/test_smoke_app.py index ac7493958..3e778e46e 100644 --- a/projects/policyengine-simulation-executor/tests/test_smoke_app.py +++ b/projects/policyengine-simulation-executor/tests/test_smoke_app.py @@ -100,7 +100,7 @@ def test_uk_dataset_smoke_validates_installed_codes_against_packaged_resources( monkeypatch.setattr( smoke_module, "resolve_local_bundle_dataset_path", - lambda country, requested: "/installed/enhanced_frs_2024_25.h5", + lambda _country, _requested_data: "/installed/enhanced_frs_2024_25.h5", ) monkeypatch.setattr( smoke_module, diff --git a/projects/policyengine-simulation-executor/tests/test_spm_runtime_native.py b/projects/policyengine-simulation-executor/tests/test_spm_runtime_native.py index e7c78e8fa..acfc6fd05 100644 --- a/projects/policyengine-simulation-executor/tests/test_spm_runtime_native.py +++ b/projects/policyengine-simulation-executor/tests/test_spm_runtime_native.py @@ -5,6 +5,7 @@ """ import os +import re from unittest.mock import Mock import pytest @@ -38,7 +39,7 @@ def _worker_entrypoint(params, monkeypatch): @pytest.mark.parametrize( - "period,unavailable_year", + ("period", "unavailable_year"), [ ({"time_period": "2021"}, 2021), ({"time_period": "2036"}, 2036), @@ -55,7 +56,10 @@ def test_worker_year_boundary_matches_real_provider( from spm_calculator.policyengine_adapter import PolicyEngineSPMProvider provider = PolicyEngineSPMProvider(forecast, geography_kind="national") - with pytest.raises(ValueError) as provider_error: + with pytest.raises( + ValueError, + match=re.escape(f"Forecast has no entry for {unavailable_year!r}"), + ) as provider_error: provider.year_metadata(unavailable_year) expected = spm_error_detail(provider_error.value) assert expected.code == "SPM_YEAR_UNAVAILABLE" @@ -82,7 +86,9 @@ def test_worker_scenario_boundary_matches_real_calculator_contract( from spm_calculator.axiom_adapter import export_build_spec scenario = "scenario-not-in-the-pinned-forecast" - with pytest.raises(ValueError) as adapter_error: + with pytest.raises( + ValueError, match=r"^Unknown forecast scenario" + ) as adapter_error: export_build_spec(forecast, years=[2024], scenario=scenario) assert adapter_error.value.code == "SPM_SCENARIO_UNAVAILABLE" expected = spm_error_detail(adapter_error.value) diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py b/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py index 4c2cbbf74..84fd33ec6 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py @@ -53,7 +53,7 @@ class OpenedConnection: """Stand in for the connection a successful connect step returns.""" -def _upper_bound_delay(lower_bound: float, upper_bound: float) -> float: +def _upper_bound_delay(_lower_bound: float, upper_bound: float) -> float: return upper_bound diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py b/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py index aa4252c22..304cb8db9 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py @@ -50,7 +50,7 @@ class RecordingRuntime: def __init__(self) -> None: self.recorded_errors: list[tuple[Exception, bool]] = [] - def record_exception(self, error, *, handled, status_code=None) -> None: + def record_exception(self, error, *, handled, status_code=None) -> None: # noqa: ARG002 # mirrors ObservabilityRuntime.record_exception, which callers pass status_code= by keyword self.recorded_errors.append((error, handled)) @@ -64,7 +64,10 @@ def test_summary_names_type_stage_role_and_message() -> None: progress = StageProgress(role="reform") msg = "Variable 'country' not found in household data" - with pytest.raises(ValueError) as caught, progress.stage("stage12_calculation"): + with ( + pytest.raises(ValueError, match=msg) as caught, + progress.stage("stage12_calculation"), + ): raise ValueError(msg) assert failure_summary(caught.value, progress=progress) == ( diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py b/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py index 5f1f528a2..7428adbc7 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py @@ -163,7 +163,7 @@ def test_v2_static_runtime_file_smoke_uses_exact_uk_worker_image(monkeypatch) -> module = _load(monkeypatch) sys.modules.pop("src.modal.v2_static_runtime_files_smoke", None) smoke = importlib.import_module("src.modal.v2_static_runtime_files_smoke") - functions = {name: options for name, options in smoke.app.function_calls} + functions = dict(smoke.app.function_calls) assert functions["smoke_stage12_static_runtime_files"]["image"] is ( module.uk_worker_image @@ -185,7 +185,7 @@ def test_v2_app_declares_validation_workers_and_non_http_coordinator( monkeypatch, ) -> None: module = _load(monkeypatch) - functions = {name: options for name, options in module.app.function_calls} + functions = dict(module.app.function_calls) assert set(functions) == { "validate_worker_us", diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py index ea3ca3443..db2a0f3e2 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py @@ -10,6 +10,7 @@ from datetime import UTC, datetime, timedelta from hashlib import sha256 from threading import Lock +from typing import ClassVar, override from uuid import UUID, uuid4 import pandas as pd @@ -244,7 +245,7 @@ def span(self, *_args, **_kwargs): def capture_context(self): return {} - def record_exception(self, error, *, handled, status_code=None): + def record_exception(self, error, *, handled, status_code=None): # noqa: ARG002 # mirrors ObservabilityRuntime.record_exception, which callers pass status_code= by keyword assert handled self.recorded_exceptions.append(error) @@ -437,7 +438,7 @@ def add_simulation(self, simulation, frames=None): def write_simulation( self, *, - prefix, + prefix, # noqa: ARG002 # mirrors the Stage 12 artifact writer, which callers pass prefix= by keyword simulation, frames, calculation_provenance=None, @@ -478,7 +479,7 @@ def write_aggregate(self, *, prefix, payload): self.aggregate_writes.append(payload) return reference - def write_comparison(self, *, prefix, payload): + def write_comparison(self, *, prefix, payload): # noqa: ARG002 # mirrors the Stage 12 artifact writer, which callers pass prefix= by keyword encoded = canonical_json_bytes(payload) digest = sha256(encoded).hexdigest() uri = "gs://private/comparison.json" @@ -517,7 +518,7 @@ def resolve_entity_variables(self, _simulation): class Model: spm_config = None - extra_variables = {} + extra_variables: ClassVar[dict[str, list[str]]] = {} tax_benefit_model_version = ModelVersion() output_dataset = type( "OutputDataset", @@ -533,17 +534,17 @@ def ensure(self) -> None: monkeypatch.setattr( simulation_runtime, "setup_gcp_credentials", - lambda: nullcontext(), + nullcontext, ) monkeypatch.setattr( simulation_runtime, "_country_module", - lambda country: country_module, + lambda _country: country_module, ) monkeypatch.setattr( simulation_runtime, "_resolve_region", - lambda **kwargs: region, + lambda **_kwargs: region, ) def resolve_dataset(params, *, region_resolution): @@ -551,12 +552,12 @@ def resolve_dataset(params, *, region_resolution): received["region"] = region_resolution return selection - def load_dataset(params, *, selection, country_module): + def load_dataset(_params, *, selection, country_module): received["load_selection"] = selection received["load_country_module"] = country_module return dataset - def build_simulation(params, *, dataset, dataset_selection, **kwargs): + def build_simulation(_params, *, dataset, dataset_selection, **_kwargs): received["build_dataset"] = dataset received["build_selection"] = dataset_selection return Model() @@ -639,7 +640,7 @@ def test_country_metadata_alignment_rejects_missing_or_mismatched_uk_values() -> ], ) def test_uk_artifacts_without_metadata_need_a_routed_national_request( - monkeypatch, region_code: str | None, routed: bool, accepted: bool + monkeypatch, region_code: str | None, *, routed: bool, accepted: bool ) -> None: monkeypatch.setattr( "policyengine_simulation_executor.uk_local_authority_metadata." @@ -813,7 +814,7 @@ def __init__(self, ordered_events: list[tuple[str, object]]) -> None: super().__init__() self.ordered_events = ordered_events - def log(self, message, *, severity="INFO", attributes=None, error=None): + def log(self, message, *, severity="INFO", attributes=None, error=None): # noqa: ARG002 # mirrors ObservabilityRuntime.log, which callers pass severity= by keyword self.ordered_events.append( ("telemetry", (message, dict(attributes or {}), error)) ) @@ -1556,7 +1557,7 @@ def test_comparison_failure_does_not_change_successful_stage12_report() -> None: class UnavailableProductionCall: object_id = "production-job-1" - def get(self, *, timeout=None): + def get(self, *, timeout=None): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword msg = ( "production result unavailable at " "https://storage.example.com/result.json?X-Goog-Signature=signed-value" @@ -1716,6 +1717,7 @@ def test_coordinator_rejects_artifacts_that_do_not_satisfy_the_shared_plan( def test_coordinator_records_failure_when_child_persistence_fails() -> None: class FailingChildStore(FakeStore): + @override def create_or_resolve_simulation(self, record): msg = "database rejected the child record" raise RuntimeError(msg) @@ -1803,7 +1805,7 @@ def __init__(self, object_id, store, simulation_execution_id, worker_summary): self.simulation_execution_id = simulation_execution_id self.worker_summary = worker_summary - def get(self, *, timeout=None): + def get(self, *, timeout=None): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword child = self.store.children[self.simulation_execution_id] self.store.children[self.simulation_execution_id] = child.model_copy( update={ @@ -1821,7 +1823,7 @@ class UnreachedCall: def __init__(self, object_id): self.object_id = object_id - def get(self, *, timeout=None): + def get(self, *, timeout=None): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword msg = "the coordinator waits for baseline first" raise AssertionError(msg) @@ -1880,7 +1882,7 @@ def test_coordinator_keeps_the_cause_recorded_by_a_failed_worker() -> None: def test_coordinator_names_the_stage_that_rejected_the_aggregate() -> None: class RejectingAggregateArtifacts(FakeArtifacts): - def write_aggregate(self, *, prefix, payload): + def write_aggregate(self, *, prefix, payload): # noqa: ARG002 # mirrors the Stage 12 artifact writer, which callers pass prefix= by keyword SPMComparisonProvenance.model_validate(payload["result"]["spm_provenance"]) store = FakeStore() @@ -2322,7 +2324,7 @@ class TimeoutCall: def __init__(self, object_id): self.object_id = object_id - def get(self, *, timeout=None): + def get(self, *, timeout=None): # noqa: ARG002 # mirrors modal.FunctionCall.get, which callers pass timeout= by keyword msg = "child result was not ready" raise TimeoutError(msg) diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_worker_validation.py b/projects/policyengine-simulation-executor/tests/test_stage12_worker_validation.py index ee5471a76..5dbbd88ea 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_worker_validation.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_worker_validation.py @@ -86,7 +86,7 @@ def test_uk_validation_loads_packaged_local_authority_resources() -> None: dataset_path_resolver=lambda _: "/installed/enhanced_frs_2024_25.h5", dataset_check=lambda *_: None, calculation_check=lambda _: None, - local_authority_resource_check=lambda path: checks.append(path), + local_authority_resource_check=checks.append, ) assert result["validated"] is True @@ -185,7 +185,7 @@ def _uk_household_hdf(tmp_path, columns: dict[str, list]) -> str: @pytest.mark.parametrize("routed", [False, True]) def test_uk_dataset_check_validates_codes_whenever_present( - monkeypatch, tmp_path, routed: bool + monkeypatch, tmp_path, *, routed: bool ) -> None: from policyengine_simulation_executor.stage12_worker_validation import ( _check_uk_local_authority_dataset, diff --git a/projects/policyengine-simulation-executor/tests/test_standalone_simulation_contract.py b/projects/policyengine-simulation-executor/tests/test_standalone_simulation_contract.py index bbc8c7e10..9a3a6b226 100644 --- a/projects/policyengine-simulation-executor/tests/test_standalone_simulation_contract.py +++ b/projects/policyengine-simulation-executor/tests/test_standalone_simulation_contract.py @@ -59,7 +59,7 @@ def test_standalone_simulation_openapi_keeps_legacy_schema_names(): def test_standalone_simulation_route_returns_legacy_macro_contract(monkeypatch): - def fake_run_simulation_impl(params, *, runtime): + def fake_run_simulation_impl(params, *, runtime): # noqa: ARG001 # keyword-only, as the route passes it assert params == {"country": "us", "reform": {}} return CURRENT_SINGLE_YEAR_MACRO_RESULT @@ -78,7 +78,7 @@ def fake_run_simulation_impl(params, *, runtime): def test_standalone_simulation_route_forwards_include_cliffs(monkeypatch): - def fake_run_simulation_impl(params, *, runtime): + def fake_run_simulation_impl(params, *, runtime): # noqa: ARG001 # keyword-only, as the route passes it assert params == { "country": "us", "reform": {}, diff --git a/projects/policyengine-simulation-executor/tests/test_uk_local_authority_metadata.py b/projects/policyengine-simulation-executor/tests/test_uk_local_authority_metadata.py index 7fef5e41f..5bbf0cad8 100644 --- a/projects/policyengine-simulation-executor/tests/test_uk_local_authority_metadata.py +++ b/projects/policyengine-simulation-executor/tests/test_uk_local_authority_metadata.py @@ -141,7 +141,7 @@ def _national_dataset() -> SimpleNamespace: ) -def _routes_area_regions(monkeypatch, routed: bool) -> None: +def _routes_area_regions(monkeypatch, *, routed: bool) -> None: monkeypatch.setattr( "policyengine_simulation_executor.uk_local_authority_metadata." "uk_area_regions_routed", @@ -153,7 +153,7 @@ def _routes_area_regions(monkeypatch, routed: bool) -> None: def test_dataset_detector_requires_codes_unless_area_regions_are_routed( monkeypatch, region_code: str | None ) -> None: - _routes_area_regions(monkeypatch, False) + _routes_area_regions(monkeypatch, routed=False) with pytest.raises(ValueError, match="contains no la_code_oa column"): detect_uk_local_authority_metadata( @@ -164,7 +164,7 @@ def test_dataset_detector_requires_codes_unless_area_regions_are_routed( def test_dataset_detector_accepts_a_national_run_when_area_regions_are_routed( monkeypatch, ) -> None: - _routes_area_regions(monkeypatch, True) + _routes_area_regions(monkeypatch, routed=True) assert ( detect_uk_local_authority_metadata("uk", _national_dataset(), region_code="uk") @@ -178,7 +178,7 @@ def test_dataset_detector_accepts_a_national_run_when_area_regions_are_routed( def test_dataset_detector_requires_codes_for_area_runs_even_when_routed( monkeypatch, region_code: str ) -> None: - _routes_area_regions(monkeypatch, True) + _routes_area_regions(monkeypatch, routed=True) with pytest.raises(ValueError, match="contains no la_code_oa column"): detect_uk_local_authority_metadata( @@ -195,11 +195,11 @@ def test_dataset_detector_requires_codes_for_area_runs_even_when_routed( ], ) def test_area_regions_count_as_routed_only_when_both_have_a_dataset( - monkeypatch, region_types: tuple[str, ...], routed: bool + monkeypatch, region_types: tuple[str, ...], *, routed: bool ) -> None: monkeypatch.setattr( "policyengine.provenance.manifest.get_release_manifest", - lambda country: SimpleNamespace( + lambda _country: SimpleNamespace( region_datasets={region_type: object() for region_type in region_types} ), ) diff --git a/projects/policyengine-simulation-executor/tests/test_uk_weight_matrix_guard.py b/projects/policyengine-simulation-executor/tests/test_uk_weight_matrix_guard.py index 1c52bed2b..7242afaa2 100644 --- a/projects/policyengine-simulation-executor/tests/test_uk_weight_matrix_guard.py +++ b/projects/policyengine-simulation-executor/tests/test_uk_weight_matrix_guard.py @@ -45,7 +45,7 @@ def _bundle(monkeypatch, *, matrix_revision: str = "1.56.16") -> None: ) monkeypatch.setattr( "policyengine.provenance.manifest.get_release_manifest", - lambda country: manifest, + lambda _country: manifest, ) @@ -60,7 +60,7 @@ def _assets(monkeypatch, tmp_path, *, households: int = 3, certified_dir=None): monkeypatch.setenv("POLICYENGINE_UK_GEOGRAPHY_DATA_DIR", str(tmp_path)) calls: list = [] - def materialize(country, dataset, *, data_dir, **kwargs): + def materialize(country, dataset, *, data_dir, **_kwargs): target = Path(certified_dir or data_dir) / _MATRIX _write_matrix(target, households) calls.append((country, dataset, Path(data_dir))) @@ -103,7 +103,7 @@ def test_fallback_regions_never_download_the_bucket_matrix() -> None: def test_guard_ignores_regions_without_weight_matrices(monkeypatch, strategy) -> None: monkeypatch.setattr( "policyengine.provenance.manifest.get_release_manifest", - lambda country: pytest.fail("no weight matrix should be resolved"), + lambda _country: pytest.fail("no weight matrix should be resolved"), ) sr._require_certified_uk_weight_matrix(strategy, _dataset(3), _SELECTION) @@ -151,7 +151,7 @@ def test_guard_rejects_a_copy_that_shadows_the_certified_matrix( _assets(monkeypatch, tmp_path, certified_dir=tmp_path / "certified") _write_matrix(tmp_path / _MATRIX, households=3) - with pytest.raises(ValueError, match="would read .*, not the parliamentary"): + with pytest.raises(ValueError, match=r"would read .*, not the parliamentary"): sr._require_certified_uk_weight_matrix(_STRATEGY, _dataset(3), _SELECTION) diff --git a/projects/policyengine-simulation-executor/tests/test_update_version_registry.py b/projects/policyengine-simulation-executor/tests/test_update_version_registry.py index 384299dd3..42b093598 100644 --- a/projects/policyengine-simulation-executor/tests/test_update_version_registry.py +++ b/projects/policyengine-simulation-executor/tests/test_update_version_registry.py @@ -53,13 +53,14 @@ def serialized_workflow(monkeypatch): @pytest.fixture -def patched_modal(monkeypatch, serialized_workflow): +def patched_modal(monkeypatch, serialized_workflow): # noqa: ARG001 # requested for its side effect; usefixtures has no effect on fixtures stores: dict[str, FakeDict] = {} class _Dict: @staticmethod def from_name( name: str, + *, environment_name: str, create_if_missing: bool, ): @@ -125,7 +126,8 @@ def test__is_newer_version__does_not_advance_on_equal(): assert registry._is_newer_version("1.500.0", "1.500.0") is False -def test_validate_routing_state_accepts_complete_state(fake_bundle_metadata): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_validate_routing_state_accepts_complete_state(): state = registry.build_next_routing_state( current_state=None, app_name="policyengine-simulation-py4-19-1", @@ -137,8 +139,8 @@ def test_validate_routing_state_accepts_complete_state(fake_bundle_metadata): registry.validate_routing_state(state) +@pytest.mark.usefixtures("fake_bundle_metadata") def test_bundle_manifest_records_policyengine_6_spm_capability( - fake_bundle_metadata, monkeypatch, ): from policyengine.bundle import get_current_bundle @@ -148,7 +150,7 @@ def test_bundle_manifest_records_policyengine_6_spm_capability( monkeypatch.setattr( executor_spm, "_forecast", - lambda expected_sha256: _StubForecast(), + lambda _expected_sha256: _StubForecast(), ) bundle = get_current_bundle() @@ -163,7 +165,8 @@ def test_bundle_manifest_records_policyengine_6_spm_capability( } -def test_validate_routing_state_rejects_missing_latest_route(fake_bundle_metadata): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_validate_routing_state_rejects_missing_latest_route(): state = registry.build_next_routing_state( current_state=None, app_name="policyengine-simulation-py4-19-1", @@ -173,13 +176,12 @@ def test_validate_routing_state_rejects_missing_latest_route(fake_bundle_metadat ) state["routes"]["us"].pop("1.687.0") - with pytest.raises(ValueError, match="latest.us"): + with pytest.raises(ValueError, match=r"latest\.us"): registry.validate_routing_state(state) -def test_validate_routing_state_allows_migrated_policyengine_route_without_bundle( - fake_bundle_metadata, -): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_validate_routing_state_allows_migrated_policyengine_route_without_bundle(): state = registry.build_next_routing_state( current_state=None, app_name="policyengine-simulation-py4-19-1", @@ -192,9 +194,8 @@ def test_validate_routing_state_allows_migrated_policyengine_route_without_bundl registry.validate_routing_state(state) -def test_validate_routing_state_rejects_required_policyengine_route_without_bundle( - fake_bundle_metadata, -): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_validate_routing_state_rejects_required_policyengine_route_without_bundle(): state = registry.build_next_routing_state( current_state=None, app_name="policyengine-simulation-py4-19-1", @@ -211,7 +212,8 @@ def test_validate_routing_state_rejects_required_policyengine_route_without_bund ) -def test_build_next_routing_state_preserves_existing_routes(fake_bundle_metadata): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_build_next_routing_state_preserves_existing_routes(): current_state = registry.build_next_routing_state( current_state=None, app_name="policyengine-simulation-py4-18-0", @@ -240,9 +242,8 @@ def test_build_next_routing_state_preserves_existing_routes(fake_bundle_metadata assert next_state["latest"]["us"] == "1.687.0" -def test_build_next_routing_state_keeps_existing_latest_when_incoming_older( - fake_bundle_metadata, -): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_build_next_routing_state_keeps_existing_latest_when_incoming_older(): current_state = registry.build_next_routing_state( current_state=None, app_name="policyengine-simulation-py4-19-1", @@ -266,9 +267,8 @@ def test_build_next_routing_state_keeps_existing_latest_when_incoming_older( ) -def test_build_next_routing_state_force_latest_allows_downgrade( - fake_bundle_metadata, -): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_build_next_routing_state_force_latest_allows_downgrade(): current_state = registry.build_next_routing_state( current_state=None, app_name="policyengine-simulation-py4-19-1", @@ -289,9 +289,8 @@ def test_build_next_routing_state_force_latest_allows_downgrade( assert next_state["latest"]["policyengine"] == "4.18.0" -def test_build_next_routing_state_rejects_country_version_mismatch( - fake_bundle_metadata, -): +@pytest.mark.usefixtures("fake_bundle_metadata") +def test_build_next_routing_state_rejects_country_version_mismatch(): with pytest.raises(ValueError, match="US version"): registry.build_next_routing_state( current_state=None, @@ -302,9 +301,9 @@ def test_build_next_routing_state_rejects_country_version_mismatch( ) +@pytest.mark.usefixtures("fake_bundle_metadata") def test_publish_routing_state_writes_only_active_snapshot( patched_modal, - fake_bundle_metadata, ): registry.publish_routing_state( environment="main", @@ -404,9 +403,9 @@ def test_build_legacy_seed_routing_state_allows_country_only_legacy_routes(): ) +@pytest.mark.usefixtures("fake_bundle_metadata") def test_seed_active_routing_state_from_legacy_merges_existing_active( patched_modal, - fake_bundle_metadata, ): patched_modal["main/simulation-api-policyengine-versions"] = FakeDict( { @@ -450,11 +449,10 @@ def test_seed_active_routing_state_from_legacy_merges_existing_active( assert state["latest"]["policyengine"] == "4.19.1" +@pytest.mark.usefixtures("fake_bundle_metadata", "serialized_workflow") def test_main_publishes_routing_state( patched_modal, - fake_bundle_metadata, monkeypatch, - serialized_workflow, ): monkeypatch.setattr( sys, @@ -501,7 +499,7 @@ def test_country_bundle_metadata_records_the_certified_dataset_sha256(monkeypatc ) monkeypatch.setattr( "policyengine_simulation_executor.release_bundle.get_country_release_bundle", - lambda country: bundle, + lambda _country: bundle, ) metadata = registry._country_bundle_metadata("uk") @@ -573,8 +571,8 @@ def _plan( version, us_version, uk_version, - force=False, *, + force=False, publication_id=None, ): _manifests(monkeypatch, {version: (us_version, uk_version)}) @@ -641,9 +639,9 @@ def from_name(name, environment_name, create_if_missing): state["routes"]["policyengine"]["5.9.0"] = _app("5.9.0") state["routes"]["uk"]["2.1.0"] = _app("5.9.0") for version, us, uk, force in releases[:-1]: - state = _plan(monkeypatch, state, version, us, uk, force)["after"] + state = _plan(monkeypatch, state, version, us, uk, force=force)["after"] version, us, uk, force = releases[-1] - plan = _plan(monkeypatch, state, version, us, uk, force) + plan = _plan(monkeypatch, state, version, us, uk, force=force) if state is not None: stores["staging/simulation-api-routing-state"] = FakeDict( {"active": json.loads(json.dumps(state))} @@ -746,7 +744,7 @@ def test_plans_say_whether_latest_routes_to_the_release( monkeypatch, version, force, advances ): state = _publish(monkeypatch, None, "6.2.6", "1.1.0", "2.1.0") - plan = _plan(monkeypatch, state, version, "1.1.0", "2.1.0", force) + plan = _plan(monkeypatch, state, version, "1.1.0", "2.1.0", force=force) assert plan["advances_latest"] is advances @@ -796,9 +794,8 @@ def _main(monkeypatch, *args): registry.main() -def test_main_plans_applies_and_restores( - patched_modal, monkeypatch, capsys, tmp_path, serialized_workflow -): +@pytest.mark.usefixtures("serialized_workflow") +def test_main_plans_applies_and_restores(patched_modal, monkeypatch, capsys, tmp_path): state = _publish(monkeypatch, None, "6.2.6", "1.1.0", "2.1.0") store = _store(patched_modal, state) _manifests(monkeypatch, {"6.3.0": ("1.1.0", "2.2.0")}) @@ -839,9 +836,8 @@ def test_main_plans_applies_and_restores( assert store.get("active") == json.loads(json.dumps(state)) -def test_main_prints_and_restores_latest( - patched_modal, monkeypatch, capsys, serialized_workflow -): +@pytest.mark.usefixtures("serialized_workflow") +def test_main_prints_and_restores_latest(patched_modal, monkeypatch, capsys): state = _publish(monkeypatch, None, "6.2.6", "1.1.0", "2.1.0") state = _publish(monkeypatch, state, "6.3.0", "1.1.0", "2.2.0") store = _store(patched_modal, state) @@ -918,9 +914,8 @@ def test_a_chain_of_attempts_is_undone_in_order(monkeypatch, patched_modal): ) -def test_main_restores_several_plans( - patched_modal, monkeypatch, capsys, tmp_path, serialized_workflow -): +@pytest.mark.usefixtures("serialized_workflow") +def test_main_restores_several_plans(patched_modal, monkeypatch, capsys, tmp_path): a = _publish(monkeypatch, None, "6.0.0", "1.1.0", "2.1.0") store = _store(patched_modal, a) first = _plan(monkeypatch, a, "6.1.0", "1.1.0", "2.2.0") @@ -957,8 +952,9 @@ def test_plans_carry_their_publication_id(monkeypatch): ["simulation-deploy.yml", "stage12-deploy.yml", "simulation-rollback.yml"], ) @pytest.mark.parametrize("ref", ["refs/heads/main", "refs/tags/v6.3.0", "a" * 40]) +@pytest.mark.usefixtures("serialized_workflow") def test_routing_guard_accepts_the_serialized_caller_workflows( - monkeypatch, serialized_workflow, workflow, ref + monkeypatch, workflow, ref ): monkeypatch.setenv( "GITHUB_WORKFLOW_REF", @@ -1026,9 +1022,8 @@ def test_routing_guard_accepts_the_serialized_caller_workflows( ), ], ) -def test_routing_guard_requires_every_condition( - monkeypatch, serialized_workflow, variable, value -): +@pytest.mark.usefixtures("serialized_workflow") +def test_routing_guard_requires_every_condition(monkeypatch, variable, value): if value is None: monkeypatch.delenv(variable, raising=False) else: @@ -1086,8 +1081,9 @@ def test_routing_guard_requires_every_condition( ], ids=["local", "not-actions", "fork", "unserialized-workflow"], ) +@pytest.mark.usefixtures("serialized_workflow") def test_library_writers_refuse_unserialized_calls_before_validation_or_modal_access( - monkeypatch, serialized_workflow, patched_modal, writer, kwargs, variable, value + monkeypatch, patched_modal, writer, kwargs, variable, value ): if variable is None: for name in ("GITHUB_ACTIONS", "GITHUB_REPOSITORY", "GITHUB_WORKFLOW_REF"): @@ -1095,7 +1091,7 @@ def test_library_writers_refuse_unserialized_calls_before_validation_or_modal_ac else: monkeypatch.setenv(variable, value) - def forbidden_access(*args, **kwargs): + def forbidden_access(*_args, **_kwargs): pytest.fail("a refused library write accessed Modal or delegated its write") monkeypatch.setattr(registry.modal.Dict, "from_name", forbidden_access) @@ -1162,8 +1158,9 @@ def forbidden_access(*args, **kwargs): ], ids=["local", "not-actions", "fork", "unserialized-workflow"], ) +@pytest.mark.usefixtures("serialized_workflow") def test_main_refuses_unserialized_writes_before_accessing_modal_or_plan_files( - monkeypatch, patched_modal, capsys, serialized_workflow, args, variable, value + monkeypatch, patched_modal, capsys, args, variable, value ): if variable is None: for name in ("GITHUB_ACTIONS", "GITHUB_REPOSITORY", "GITHUB_WORKFLOW_REF"): @@ -1171,7 +1168,7 @@ def test_main_refuses_unserialized_writes_before_accessing_modal_or_plan_files( else: monkeypatch.setenv(variable, value) - def forbidden_modal_access(*args, **kwargs): + def forbidden_modal_access(*_args, **_kwargs): pytest.fail("a refused CLI write accessed Modal") monkeypatch.setattr(registry.modal.Dict, "from_name", forbidden_modal_access) @@ -1403,9 +1400,8 @@ def test_every_other_routing_write_refuses_an_unfinished_release( registry.apply_routing_plan(environment="staging", plan=pending) saved = json.loads(json.dumps(store.snapshot())) other = _plan(monkeypatch, pending["after"], "6.2.0", "1.2.0", "2.3.0") - with pytest.raises( - registry.RoutingConflictError, match="Unfinished routing publication" - ): + + def write_routing(): if writer == "publish": registry.publish_routing_state( environment="staging", @@ -1422,6 +1418,11 @@ def test_every_other_routing_write_refuses_an_unfinished_release( registry.seed_active_routing_state_from_legacy(environment="staging") else: registry.restore_routing_plans(environment="staging", plans=[other]) + + with pytest.raises( + registry.RoutingConflictError, match="Unfinished routing publication" + ): + write_routing() assert store.snapshot() == saved @@ -1707,7 +1708,7 @@ def test_retry_after_finalization_returns_the_prior_outcome_without_writing( assert registry.finalize_routing_publication(**args) == expected saved = json.loads(json.dumps(store.snapshot())) - def forbidden_write(*args, **kwargs): + def forbidden_write(*_args, **_kwargs): pytest.fail("an already finalized publication was written again") monkeypatch.setattr(registry, "_write_publication_record", forbidden_write) @@ -1716,7 +1717,7 @@ def forbidden_write(*args, **kwargs): assert store.snapshot() == saved -def test_operator_cannot_complete_pending_publications(monkeypatch, patched_modal): +def test_operator_cannot_complete_pending_publications(patched_modal): store = _store(patched_modal, None) with pytest.raises( ValueError, match="operator recovery requires gates_passed=false" @@ -1782,7 +1783,7 @@ def test_finalizer_owns_prod_publications_in_the_main_modal_environment( assert store.get("publication") == {"status": "completed", "plan": pending} -def test_finalizer_rejects_invalid_release_environments(monkeypatch, patched_modal): +def test_finalizer_rejects_invalid_release_environments(patched_modal): with pytest.raises(ValueError, match="release_environment must be beta or prod"): registry.finalize_routing_publication( environment="staging", release_environment="staging", gates_passed=False diff --git a/projects/policyengine-simulation-executor/tests/test_validate_v2_worker_release.py b/projects/policyengine-simulation-executor/tests/test_validate_v2_worker_release.py index d6f37a66a..2fa95589a 100644 --- a/projects/policyengine-simulation-executor/tests/test_validate_v2_worker_release.py +++ b/projects/policyengine-simulation-executor/tests/test_validate_v2_worker_release.py @@ -9,7 +9,7 @@ def test_all_country_results_must_match_the_deployment_bundle(monkeypatch) -> None: invocation_ids: list[str] = [] - def fake_spawn_validation(*, application_name, function_name, environment): + def fake_spawn_validation(*, application_name, function_name, environment): # noqa: ARG001 # keyword-only, as the validator passes them country = function_name.rsplit("_", maxsplit=1)[1] resolved = validation.load_stage12_bundle() bundle = next( diff --git a/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py b/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py index 3b2e4c9e6..e6348e452 100644 --- a/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py +++ b/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py @@ -2,7 +2,7 @@ from collections.abc import ItemsView, KeysView from copy import deepcopy -from typing import Self +from typing import ClassVar, Self import pytest from policyengine_simulation_contract.macro_output import ( @@ -98,6 +98,7 @@ def _runtime_dataset_uri( country_bundle: dict, dataset_uri: str, revision: str | None = None, + *, use_bundle_default: bool = True, ) -> str: dataset_without_revision, existing_revision = _split_revision(dataset_uri) @@ -203,8 +204,8 @@ def from_name(cls, name: str) -> Self: class MockFunctionCall: """Mock for Modal FunctionCall returned by spawn.""" - registry = {} - from_id_errors = {} + registry: ClassVar[dict[str, "MockFunctionCall"]] = {} + from_id_errors: ClassVar[dict[str, BaseException]] = {} def __init__(self, object_id: str = "mock-job-id-123") -> None: """Register a complete call whose result is a valid zeroed macro output. @@ -260,7 +261,7 @@ def __init__(self, object_id: str = "mock-job-id-123") -> None: self.running = False self.__class__.registry[object_id] = self - def get(self, timeout: int = 0) -> object: + def get(self, timeout: int = 0) -> object: # noqa: ARG002 # mirrors modal.FunctionCall.get """Return the result; raise TimeoutError while running, or the set error.""" if self.running: raise TimeoutError @@ -373,7 +374,7 @@ def mock_modal(monkeypatch: pytest.MonkeyPatch) -> dict[str, object]: class MockModalDict: @staticmethod - def from_name(name: str, create_if_missing: bool = False) -> MockDict: + def from_name(name: str, *, create_if_missing: bool = False) -> MockDict: if create_if_missing and name not in mock_dicts: mock_dicts[name] = {} if name not in mock_dicts: diff --git a/projects/policyengine-simulation-gateway/fixtures/package_imports.py b/projects/policyengine-simulation-gateway/fixtures/package_imports.py index a1e7232e2..09f59c02b 100644 --- a/projects/policyengine-simulation-gateway/fixtures/package_imports.py +++ b/projects/policyengine-simulation-gateway/fixtures/package_imports.py @@ -34,13 +34,13 @@ class GatewayImportModuleNames: fastapi: str = FASTAPI_MODULE -@pytest.fixture() +@pytest.fixture def gateway_import_module_names() -> GatewayImportModuleNames: """Return the module names the import-boundary tests assert on.""" return GatewayImportModuleNames() -@pytest.fixture() +@pytest.fixture def isolated_gateway_model_import_modules() -> Iterator[None]: """Temporarily clear modules that would mask import side effects.""" previous_modules = { @@ -62,9 +62,9 @@ def isolated_gateway_model_import_modules() -> Iterator[None]: ) -@pytest.fixture() +@pytest.fixture def import_gateway_models( - isolated_gateway_model_import_modules: None, + isolated_gateway_model_import_modules: None, # noqa: ARG001 # requested for its side effect; usefixtures has no effect on fixtures ) -> Callable[[], ModuleType]: """Import gateway models from a clean module state.""" diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py index e4d042b1f..b454e22fe 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py @@ -30,6 +30,7 @@ import functools import logging import os +from typing import Annotated from fastapi import Depends, HTTPException, Request, status from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer @@ -56,6 +57,9 @@ _bearer_scheme = HTTPBearer(auto_error=False) +BearerCredentials = Annotated[ + HTTPAuthorizationCredentials | None, Depends(_bearer_scheme) +] def _auth_disabled() -> bool: @@ -177,7 +181,7 @@ def enforce_production_auth_guard(runtime: ObservabilityRuntime) -> None: "roll back immediately." ) logger.critical(banner) - try: + try: # noqa: SIM105 # try/except keeps the swallow's reason on its except line runtime.event( "gateway_auth_disabled_bypass_active", severity="CRITICAL", @@ -226,7 +230,7 @@ def enforce_auth_configured_guard() -> None: def require_auth( request: Request, - token: HTTPAuthorizationCredentials | None = Depends(_bearer_scheme), + token: BearerCredentials, ) -> dict | None: """FastAPI dependency gating an endpoint behind a bearer JWT. @@ -264,12 +268,12 @@ def require_auth( try: decoder = _get_decoder() - except RuntimeError: + except RuntimeError as error: logger.exception("Gateway auth misconfigured") raise HTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Gateway authentication is not configured.", - ) + ) from error try: return decoder(token) diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py index 532dcef26..261a8d05f 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py @@ -471,9 +471,9 @@ def _resolve_from_legacy_dicts( raise ValueError(msg) try: app_name = policyengine_versions[policyengine_version] - except KeyError: + except KeyError as error: msg = f"Unknown policyengine.py version {policyengine_version}" - raise ValueError(msg) + raise ValueError(msg) from error return RouteResolution( app_name=app_name, response_version=policyengine_version, @@ -482,14 +482,11 @@ def _resolve_from_legacy_dicts( ) country_versions = modal.Dict.from_name(f"simulation-api-{country}-versions") - if version is None: - resolved_version = country_versions["latest"] - else: - resolved_version = version + resolved_version = country_versions["latest"] if version is None else version try: app_name = country_versions[resolved_version] - except KeyError: + except KeyError as error: policyengine_versions = _optional_modal_dict(POLICYENGINE_VERSION_DICT_NAME) if policyengine_versions is not None and version is not None: try: @@ -504,7 +501,7 @@ def _resolve_from_legacy_dicts( bundle_manifest={}, ) msg = f"Unknown version {resolved_version} for country {country}" - raise ValueError(msg) + raise ValueError(msg) from error return RouteResolution( app_name=app_name, @@ -703,7 +700,7 @@ def _bind_observability_id( observability_id = normalize_observability_id(value) http_request.state.observability_id = observability_id if observability_id is not None: - try: + try: # noqa: SIM105 # try/except keeps the swallow's reason on its except line runtime.set_context(observability_id=observability_id) except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass @@ -828,7 +825,7 @@ async def submit_simulation( ) except ValueError as e: runtime.record_exception(e, handled=True, status_code=400) - raise HTTPException(status_code=400, detail=str(e)) + raise HTTPException(status_code=400, detail=str(e)) from e runtime.set_context( resolved_app_name=route.app_name, @@ -931,7 +928,7 @@ async def submit_budget_window_batch( ) except ValueError as e: runtime.record_exception(e, handled=True, status_code=400) - raise HTTPException(status_code=400, detail=str(e)) + raise HTTPException(status_code=400, detail=str(e)) from e runtime.set_context( resolved_app_name=route.app_name, @@ -1039,7 +1036,9 @@ async def get_job_status( except Exception as exc: if _is_modal_job_not_found(exc): runtime.record_exception(exc, handled=True, status_code=404) - raise HTTPException(status_code=404, detail=f"Job not found: {job_id}") + raise HTTPException( + status_code=404, detail=f"Job not found: {job_id}" + ) from exc runtime.record_exception(exc, handled=False, status_code=500) raise @@ -1053,10 +1052,14 @@ async def get_job_status( except TimeoutError: with runtime.span(ANNUAL_IMPACT_STAGES.name(Stage.RESPONSE_SERIALIZATION)): return running_job_response(public_metadata, headers=response_headers) - except Exception as exc: # noqa: BLE001 # call.get re-raises whatever the worker raised; every failure becomes a failed-job response, logged with its traceback by log_and_redact_exception + # call.get re-raises whatever the worker raised; every failure becomes a + # failed-job response, logged with its traceback by log_and_redact_exception. + except Exception as exc: if _is_modal_job_not_found(exc): runtime.record_exception(exc, handled=True, status_code=404) - raise HTTPException(status_code=404, detail=f"Job not found: {job_id}") + raise HTTPException( + status_code=404, detail=f"Job not found: {job_id}" + ) from exc detail = spm_error_detail(exc) if detail: return failed_job_response( diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py index d1c078be0..e737fa576 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py @@ -143,7 +143,7 @@ def main() -> None: output_path = Path(__file__).resolve().parents[2] / "artifacts" / "openapi.json" output_path.parent.mkdir(parents=True, exist_ok=True) - with open(output_path, "w") as f: + with output_path.open("w") as f: json.dump(openapi_spec, f, indent=2) print(f"OpenAPI spec written to {output_path}") # noqa: T201 # command-line script: reports where it wrote the spec diff --git a/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py b/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py index ae88bb2bc..595db54c4 100644 --- a/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py +++ b/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py @@ -28,6 +28,7 @@ def test_gateway_image_smoke(): cwd=PROJECT_ROOT, capture_output=True, text=True, + check=False, timeout=1200, ) assert result.returncode == 0, result.stdout + result.stderr diff --git a/projects/policyengine-simulation-gateway/tests/test_auth.py b/projects/policyengine-simulation-gateway/tests/test_auth.py index 610667ba3..0b0881356 100644 --- a/projects/policyengine-simulation-gateway/tests/test_auth.py +++ b/projects/policyengine-simulation-gateway/tests/test_auth.py @@ -55,7 +55,7 @@ def __call__(self, token): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) - monkeypatch.setattr(auth_module, "_get_decoder", lambda: FailingDecoder()) + monkeypatch.setattr(auth_module, "_get_decoder", FailingDecoder) monkeypatch.delenv(auth_module.GATEWAY_AUTH_DISABLED_ENV, raising=False) monkeypatch.setenv(auth_module.GATEWAY_AUTH_REQUIRED_ENV, "1") monkeypatch.setenv(auth_module.GATEWAY_AUTH_ISSUER_ENV, "https://issuer.example/") @@ -64,7 +64,7 @@ def __call__(self, token): return TestClient(create_gateway_app(authenticate=False)) -@pytest.mark.parametrize("method,path,body", GATED_REQUESTS) +@pytest.mark.parametrize(("method", "path", "body"), GATED_REQUESTS) def test__given_no_bearer_token__then_gated_endpoint_returns_403( unauthenticated_client, method, path, body ): @@ -134,7 +134,7 @@ def test__given_partial_auth_config__then_dependency_raises_503(monkeypatch): assert exc_info.value.status_code == 503 -def test__given_health_endpoint__then_auth_not_required(monkeypatch): +def test__given_health_endpoint__then_auth_not_required(): """Health/ping/versions endpoints remain public by design.""" from policyengine_simulation_gateway.testing import create_gateway_app diff --git a/projects/policyengine-simulation-gateway/tests/test_endpoints.py b/projects/policyengine-simulation-gateway/tests/test_endpoints.py index 17672641c..1037eedfa 100644 --- a/projects/policyengine-simulation-gateway/tests/test_endpoints.py +++ b/projects/policyengine-simulation-gateway/tests/test_endpoints.py @@ -49,9 +49,8 @@ def expected_bundle( class TestGetAppName: """Tests for the get_app_name helper function.""" - def test__given_no_version__then_prefers_latest_policyengine_bundle( - self, mock_modal - ): + @pytest.mark.usefixtures("mock_modal") + def test__given_no_version__then_prefers_latest_policyengine_bundle(self): from policyengine_simulation_gateway.endpoints import get_app_name app_name, resolved_version = get_app_name("us", None) @@ -59,8 +58,9 @@ def test__given_no_version__then_prefers_latest_policyengine_bundle( assert resolved_version == "4.10.0" assert app_name == "policyengine-simulation-py4-10-0" + @pytest.mark.usefixtures("mock_modal") def test__given_legacy_version_matching_policyengine_bundle__then_routes_by_bundle( - self, mock_modal + self, ): from policyengine_simulation_gateway.endpoints import get_app_name @@ -69,7 +69,8 @@ def test__given_legacy_version_matching_policyengine_bundle__then_routes_by_bund assert resolved_version == "4.10.0" assert app_name == "policyengine-simulation-py4-10-0" - def test__given_us_country_no_version__then_returns_latest_app(self, mock_modal): + @pytest.mark.usefixtures("mock_modal") + def test__given_us_country_no_version__then_returns_latest_app(self): """Given 'us' and no version, when resolving, then use the 'latest' app.""" from policyengine_simulation_gateway.endpoints import get_app_name @@ -80,9 +81,8 @@ def test__given_us_country_no_version__then_returns_latest_app(self, mock_modal) assert resolved_version == "4.10.0" assert app_name == "policyengine-simulation-py4-10-0" - def test__given_us_country_with_version__then_returns_specified_app( - self, mock_modal - ): + @pytest.mark.usefixtures("mock_modal") + def test__given_us_country_with_version__then_returns_specified_app(self): """Given 'us' and a version, when resolving, then use that version's app.""" from policyengine_simulation_gateway.endpoints import get_app_name @@ -93,7 +93,8 @@ def test__given_us_country_with_version__then_returns_specified_app( assert resolved_version == "1.459.0" assert app_name == "policyengine-simulation-py3-9-0" - def test__given_uk_country__then_uses_uk_version_dict(self, mock_modal): + @pytest.mark.usefixtures("mock_modal") + def test__given_uk_country__then_uses_uk_version_dict(self): """Given country='uk', when resolving, then use the UK version dictionary.""" from policyengine_simulation_gateway.endpoints import get_app_name @@ -112,7 +113,8 @@ def test__given_invalid_country__then_raises_value_error(self): with pytest.raises(ValueError, match="Unknown country"): get_app_name("invalid", None) - def test__given_unknown_version__then_raises_value_error(self, mock_modal): + @pytest.mark.usefixtures("mock_modal") + def test__given_unknown_version__then_raises_value_error(self): """Given an unregistered version, when resolving, then raise ValueError.""" from policyengine_simulation_gateway.endpoints import get_app_name @@ -163,7 +165,8 @@ def test__given_only_legacy_country_routes__then_no_version_uses_country_latest( assert resolved_version == "1.715.2" assert app_name == "policyengine-simulation-us1-715-2-uk2-88-20" - def test__given_policyengine_version_field__then_routes_by_bundle(self, mock_modal): + @pytest.mark.usefixtures("mock_modal") + def test__given_policyengine_version_field__then_routes_by_bundle(self): from policyengine_simulation_gateway.endpoints import resolve_route route = resolve_route("us", None, "4.10.0") @@ -897,8 +900,9 @@ def test__given_submitted_job_with_header__then_polling_echoes_header( assert "observability_id" not in response.json() assert response.headers[OBSERVABILITY_ID_HEADER] == OBSERVABILITY_ID + @pytest.mark.usefixtures("mock_modal") def test__given_unknown_job_id__then_polling_returns_404( - self, mock_modal, client: TestClient, monkeypatch + self, client: TestClient, monkeypatch ): """Return 404 for a job ID the gateway never issued, without asking Modal. @@ -1137,8 +1141,9 @@ def test__given_bundle_with_dataset_sha__then_submission_names_it( assert bundle["dataset_sha256"] == "b" * 64 assert bundle["dataset"].startswith("hf://") + @pytest.mark.usefixtures("mock_modal") def test__given_active_state__then_lists_policyengine_and_country_routes( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.get("/versions") @@ -1149,16 +1154,18 @@ def test__given_active_state__then_lists_policyengine_and_country_routes( assert data["us"]["latest"] == "1.500.0" assert data["uk"]["latest"] == "2.66.0" + @pytest.mark.usefixtures("mock_modal") def test__given_active_state__then_policyengine_versions_endpoint_works( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.get("/versions/policyengine") assert response.status_code == 200 assert response.json()["latest"] == "4.10.0" + @pytest.mark.usefixtures("mock_modal") def test__given_unknown_version_kind__then_records_handled_404( - self, mock_modal, client: TestClient, monkeypatch + self, client: TestClient, monkeypatch ): recorded_errors = [] monkeypatch.setattr( @@ -1341,8 +1348,9 @@ def test__given_budget_window_submission__then_returns_parent_batch_job_id( "policyengine_bundle": expected_bundle("us", "1.500.0"), } + @pytest.mark.usefixtures("mock_modal") def test__given_unknown_budget_window_job__then_records_handled_404( - self, mock_modal, client: TestClient, monkeypatch + self, client: TestClient, monkeypatch ): recorded_errors = [] monkeypatch.setattr( @@ -1500,8 +1508,9 @@ def test__given_budget_window_submission__then_initial_poll_returns_seed_state( } assert response.headers[OBSERVABILITY_ID_HEADER] == OBSERVABILITY_ID + @pytest.mark.usefixtures("mock_modal") def test__given_batch_state__then_poll_returns_completed_response( - self, mock_modal, client: TestClient + self, client: TestClient ): from policyengine_simulation_contract.budget_window_state import ( put_batch_job_state, @@ -1580,8 +1589,9 @@ def test__given_batch_state__then_poll_returns_completed_response( assert "observability_id" not in response.json() assert response.headers[OBSERVABILITY_ID_HEADER] == OBSERVABILITY_ID + @pytest.mark.usefixtures("mock_modal") def test__given_non_integer_start_year__then_budget_window_submit_returns_422( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.post( "/simulate/economy/budget-window", @@ -1601,8 +1611,9 @@ def test__given_non_integer_start_year__then_budget_window_submit_returns_422( == "Value error, start_year must be an integer year" ) + @pytest.mark.usefixtures("mock_modal") def test__given_end_year_past_2099__then_budget_window_submit_returns_422( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.post( "/simulate/economy/budget-window", @@ -1622,8 +1633,9 @@ def test__given_end_year_past_2099__then_budget_window_submit_returns_422( == "Value error, budget-window end_year must be 2099 or earlier" ) + @pytest.mark.usefixtures("mock_modal") def test__given_window_size_above_max__then_budget_window_submit_returns_422( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.post( "/simulate/economy/budget-window", @@ -1640,8 +1652,9 @@ def test__given_window_size_above_max__then_budget_window_submit_returns_422( assert response.status_code == 422 assert response.json()["detail"][0]["loc"] == ["body", "window_size"] + @pytest.mark.usefixtures("mock_modal") def test__given_missing_region__then_budget_window_submit_returns_422( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.post( "/simulate/economy/budget-window", @@ -1657,8 +1670,9 @@ def test__given_missing_region__then_budget_window_submit_returns_422( assert response.status_code == 422 assert response.json()["detail"][0]["loc"] == ["body", "region"] + @pytest.mark.usefixtures("mock_modal") def test__given_non_general_target__then_budget_window_submit_returns_422( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.post( "/simulate/economy/budget-window", @@ -1676,8 +1690,9 @@ def test__given_non_general_target__then_budget_window_submit_returns_422( assert response.status_code == 422 assert response.json()["detail"][0]["loc"] == ["body", "target"] + @pytest.mark.usefixtures("mock_modal") def test__given_max_parallel_above_active_limit__then_submit_returns_422( - self, mock_modal, client: TestClient + self, client: TestClient ): response = client.post( "/simulate/economy/budget-window", diff --git a/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py b/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py index 5d8ac0a84..7c20a111c 100644 --- a/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py +++ b/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py @@ -121,7 +121,7 @@ def test_gateway_image_mounts_local_packages(monkeypatch): def test_web_app_secrets(monkeypatch): app = import_gateway_app(monkeypatch) - function_kwargs = {name: kwargs for name, kwargs in app.app.function_calls} + function_kwargs = dict(app.app.function_calls) assert function_kwargs["web_app"]["secrets"] == [ app.gateway_auth_secret, ] diff --git a/projects/policyengine-simulation-gateway/tests/test_package_imports.py b/projects/policyengine-simulation-gateway/tests/test_package_imports.py index 6c1cfd575..2063d71c7 100644 --- a/projects/policyengine-simulation-gateway/tests/test_package_imports.py +++ b/projects/policyengine-simulation-gateway/tests/test_package_imports.py @@ -1,6 +1,8 @@ import importlib import sys +import pytest + def test_gateway_models_import_does_not_import_fastapi_endpoints( import_gateway_models, @@ -12,8 +14,8 @@ def test_gateway_models_import_does_not_import_fastapi_endpoints( assert gateway_import_module_names.fastapi not in sys.modules +@pytest.mark.usefixtures("isolated_gateway_model_import_modules") def test_gateway_endpoints_import_does_not_import_policyengine_bundle( - isolated_gateway_model_import_modules, gateway_import_module_names, ): release_bundle_module = sys.modules.pop( diff --git a/projects/policyengine-simulation-gateway/tests/test_spm_routes.py b/projects/policyengine-simulation-gateway/tests/test_spm_routes.py index df9500900..f16ec7bbf 100644 --- a/projects/policyengine-simulation-gateway/tests/test_spm_routes.py +++ b/projects/policyengine-simulation-gateway/tests/test_spm_routes.py @@ -52,7 +52,7 @@ def legacy_route(mock_modal, source, model_version): } -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) @pytest.mark.parametrize("source", LEGACY_SOURCES) @pytest.mark.parametrize("version", [None, "1.715.2"]) def test_historical_no_wrapper_route_submits_without_spm( @@ -68,7 +68,7 @@ def test_historical_no_wrapper_route_submits_without_spm( assert "policyengine_version" not in response.json()["policyengine_bundle"] -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) @pytest.mark.parametrize("source", LEGACY_SOURCES) @pytest.mark.parametrize("selection", [{}, {"geography_kind": "national"}]) def test_historical_route_rejects_any_explicit_spm( @@ -81,9 +81,9 @@ def test_historical_route_rejects_any_explicit_spm( assert mock_modal["func"].calls == [] -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) @pytest.mark.parametrize( - "source,model_version", + ("source", "model_version"), [ ("legacy-country-dict", "1.824.7"), ("legacy-seed", "1.824.7"), @@ -102,7 +102,7 @@ def test_missing_wrapper_does_not_certify_unknown_routes( assert mock_modal["func"].calls == [] -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_legacy_seed_cannot_erase_future_wrapper_from_app_name( mock_modal, client, endpoint, extra ): @@ -144,7 +144,7 @@ def shared_app_state(mock_modal, *, sibling_model): return state -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_shared_app_resolves_unique_country_model_bundle( mock_modal, client, endpoint, extra ): @@ -167,7 +167,7 @@ def test_latest_route_alias_does_not_create_ambiguous_bundle(mock_modal, client) assert response.json()["policyengine_bundle"]["policyengine_version"] == "5.3.0" -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_shared_app_with_ambiguous_country_model_requires_explicit_bundle( mock_modal, client, endpoint, extra ): @@ -186,7 +186,7 @@ def test_shared_app_with_ambiguous_country_model_requires_explicit_bundle( assert mock_modal["func"].last_payload["spm"]["scenario"] == "ce_trend" -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) @pytest.mark.parametrize("sibling_model", [None, "", " ", 42]) def test_shared_app_with_missing_model_metadata_is_ambiguous( mock_modal, client, endpoint, extra, sibling_model @@ -436,7 +436,7 @@ def test_canonical_poll_bodies_still_carry_the_capability(mock_modal, client): assert bundle["spm"]["defaults"]["scenario"] == "ce_trend" -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_publishing_does_not_revoke_a_seeded_country_route( mock_modal, client, endpoint, extra ): @@ -460,7 +460,7 @@ def wrapper_route_without_manifest(mock_modal, wrapper): mock_modal["dicts"]["simulation-api-routing-state"] = {"active": state} -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) @pytest.mark.parametrize("wrapper", ["5.2.0", "5.3.0"]) def test_pinned_wrapper_route_without_manifest_submits_without_spm( mock_modal, client, endpoint, extra, wrapper @@ -480,7 +480,7 @@ def test_pinned_wrapper_route_without_manifest_submits_without_spm( assert response.json()["policyengine_bundle"]["model_version"] == wrapper -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) @pytest.mark.parametrize("wrapper", ["5.2.0", "5.3.0"]) def test_pinned_wrapper_through_legacy_dicts_submits_without_spm( mock_modal, client, endpoint, extra, wrapper @@ -496,7 +496,7 @@ def test_pinned_wrapper_through_legacy_dicts_submits_without_spm( assert "spm" not in mock_modal["func"].last_payload -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_unpinned_wrapper_route_without_manifest_still_fails_closed( mock_modal, client, endpoint, extra ): @@ -514,7 +514,7 @@ def test_unpinned_wrapper_route_without_manifest_still_fails_closed( assert mock_modal["func"].calls == [] -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_stated_model_version_still_contradicts_the_wrapper_pin( mock_modal, client, endpoint, extra ): @@ -557,7 +557,7 @@ def republished_wrapper_state(mock_modal, *, model_version): return state -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_stale_country_route_is_refused_rather_than_served( mock_modal, client, endpoint, extra ): @@ -573,13 +573,14 @@ def test_stale_country_route_is_refused_rather_than_served( ) assert response.status_code == 400 detail = response.json()["detail"] - assert "1.459.0" in detail and "1.470.0" in detail + assert "1.459.0" in detail + assert "1.470.0" in detail # A routing refusal, not an SPM one. assert "errors" not in response.json() assert mock_modal["func"].calls == [] -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) def test_a_country_route_its_bundle_still_states_is_served( mock_modal, client, endpoint, extra ): @@ -598,7 +599,7 @@ def test_a_country_route_its_bundle_still_states_is_served( assert bundle["model_version"] == "1.459.0" -@pytest.mark.parametrize("endpoint,extra", ENDPOINTS) +@pytest.mark.parametrize(("endpoint", "extra"), ENDPOINTS) @pytest.mark.parametrize("model_version", [None, "", " ", 42]) def test_a_bundle_that_states_no_model_version_contradicts_nothing( mock_modal, client, endpoint, extra, model_version diff --git a/ruff.toml b/ruff.toml index a90adfdeb..e296caa03 100644 --- a/ruff.toml +++ b/ruff.toml @@ -37,6 +37,8 @@ select = [ "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", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). @@ -51,6 +53,9 @@ ignore = ["D203", "D213"] # The deployed-service test suite (pytest, run against a live candidate) gets # the same test exemptions. "**/authenticated_tests/**" = ["ANN", "D1", "INP001", "PLC0415", "PLR2004", "S101", "SLF001"] +# The shared rules tool needs only the standard library, so its tests use +# unittest on purpose; pytest-style rules don't apply to them. +"scripts/rules/tests/**" = ["PT"] [lint.flake8-type-checking] # Classes whose annotations Pydantic evaluates at runtime, so their imports must diff --git a/rules/registry.toml b/rules/registry.toml index 664ffaa7e..9fe0bb8f5 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -711,7 +711,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["A", "ANN", "BLE", "D", "DTZ", "E", "E4", "E7", "E9", "EM", "ERA", "EXE", "F", "FAST", "FIX", "FLY", "FURB", "G", "I", "INP", "LOG", "N", "PIE", "PLC", "PYI", "RET", "RSE", "S", "SLF", "T20", "TC", "TD", "TRY", "UP", "W"] +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"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -736,6 +736,25 @@ workflow = ".github/workflows/pr.yml" job = "lint" invocation = "ruff check ." + +[[rule]] +id = "sim.quality.unittest-ruff-exemptions" +category = "quality" +statement = "The shared rules tool's tests use unittest, because the tool needs only the standard library, so ruff's pytest-style rules (PT) don't apply to them." +rationale = "docs/engineering/skills/rules-registry.md#baselines" +scope = ["scripts/rules/tests/**/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "scripts/rules/tests/**" +codes = ["PT"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." [[rule]] id = "sim.quality.deployed-tests-ruff-exemptions" category = "quality" diff --git a/scripts/rules/adapters.py b/scripts/rules/adapters.py index a16c5b40c..81890218d 100644 --- a/scripts/rules/adapters.py +++ b/scripts/rules/adapters.py @@ -30,7 +30,7 @@ def check_adapters(agents: Agents, root: Path) -> list[Violation]: continue text = path.read_text() - def v(message: str, fix: str) -> None: + def v(message: str, fix: str, adapter: str = adapter) -> None: violations.append(Violation(None, "adapters", f"{adapter} {message}", fix)) if instructions not in text: diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index b68a49f64..b2f87d3a6 100644 --- a/scripts/rules/baseline.py +++ b/scripts/rules/baseline.py @@ -33,6 +33,7 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: cwd=root, capture_output=True, text=True, + check=False, ) if verify.returncode != 0: raise BaseUnavailableError(ref) @@ -41,6 +42,7 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: cwd=root, capture_output=True, text=True, + check=False, ) if shown.returncode != 0: return None @@ -85,6 +87,8 @@ def check_baselines( "explicit and reviewable", ) ) - for rule_id in sorted(current_ids - {rule.id for rule in base.rules}): - notes.append(f"rule {rule_id} is new") + notes.extend( + f"rule {rule_id} is new" + for rule_id in sorted(current_ids - {rule.id for rule in base.rules}) + ) return violations, notes diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 7f04c46b6..52158a678 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -101,6 +101,7 @@ def shellcheck_failures(config: CiScripts, root: Path) -> set[str] | None: cwd=root, capture_output=True, text=True, + check=False, ) return {line.split(":", 1)[0] for line in result.stdout.splitlines() if ":" in line} @@ -177,24 +178,25 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio "fix what `shellcheck