diff --git a/.github/scripts/update_policyengine_package.py b/.github/scripts/update_policyengine_package.py index f570b34ec..af723b329 100644 --- a/.github/scripts/update_policyengine_package.py +++ b/.github/scripts/update_policyengine_package.py @@ -32,6 +32,11 @@ def _json_from_stdin() -> object: return json.load(sys.stdin) +def _write_result(result: object) -> None: + """Print one result line for update-policyengine-package.sh to capture.""" + print(result) + + def _requirement_name(requirement: str) -> str: return re.split(r"[\s\[<>=!~;@]", requirement, maxsplit=1)[0] @@ -155,7 +160,7 @@ def find_issue(title: str) -> None: and isinstance(item.get("number"), int) ) if matches: - print(matches[0]) + _write_result(matches[0]) def verify_issue(expected_number: int, expected_title: str) -> None: @@ -191,7 +196,7 @@ def latest_version(package: str) -> None: if not isinstance(version, str) or not version: msg = f"PyPI response has no latest version for {package}" raise SystemExit(msg) - print(version) + _write_result(version) def bundle_versions() -> None: @@ -264,7 +269,7 @@ def certificate_sha256(country: str) -> str: "uk_certificate_sha256": certificate_sha256("uk"), } for name, version in outputs.items(): - print(f"{name}={version}") + _write_result(f"{name}={version}") #: Release-dispatch fields compared with the installed bundle: (payload key @@ -353,9 +358,9 @@ def verify_dispatch() -> None: ): msg = f"The release dispatch says the UK pair was not promotable ({verdict})" raise SystemExit(msg) - print(f"dispatch_verified={'true' if compared else 'false'}") - print(f"uk_qualification_verdict={verdict}") - print(f"uk_qualification_receipt_sha256={receipt_sha256}") + _write_result(f"dispatch_verified={'true' if compared else 'false'}") + _write_result(f"uk_qualification_verdict={verdict}") + _write_result(f"uk_qualification_receipt_sha256={receipt_sha256}") def _parser() -> argparse.ArgumentParser: @@ -398,7 +403,7 @@ def main() -> None: """ args = _parser().parse_args() if args.command == "current-version": - print(current_version(args.pyproject, args.package)) + _write_result(current_version(args.pyproject, args.package)) elif args.command == "update-requirements": update_requirements(args.pyproject, args.package, args.current, args.latest) elif args.command == "find-issue": diff --git a/AGENTS.md b/AGENTS.md index d6c088c79..15ba224be 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,13 +140,17 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Code quality and tests +- `sim.quality.ci-cd-scripts-ruff-exemptions`: CI/CD scripts under .github may print their output (T201); every other ruff rule applies to them. (blocking: ruff per-file ignore of T201 for `.github/**`) +- `sim.quality.command-line-scripts-ruff-exemptions`: Command-line scripts under a scripts directory may print their output (T201), run other programs (S603, S607) and use today's date (DTZ011); every other ruff rule applies to them. (blocking: ruff per-file ignore of DTZ011, S603, S607, T201 for `**/scripts/**`) +- `sim.quality.deployed-tests-ruff-exemptions`: The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (sim.quality.tests-ruff-exemptions); every other ruff rule applies to it. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/authenticated_tests/**`) - `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 (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, 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 (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, 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.src-programs-ruff-exemptions`: Programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands) may print their output (T201) and open URLs from their arguments (S310); every other ruff rule applies to them. (blocking: ruff per-file ignore of S310, T201 for `projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.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 and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`) +- `sim.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also run repository scripts (S603, S607) and use fake credentials (S105, S106), temporary paths (S108), naive datetimes (DTZ001, DTZ007), pickle (S301) and exec (S102). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/tests/**`) ### CI workflows and scripts diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 4d1de430b..99adea86d 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 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; tests, CI/CD scripts and command-line scripts take their deliberate exceptions as registered file-level ruff ignores instead of inline suppressions, and the rules guard rejects file-level ignores anywhere else - 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 - Enabled ruff's docstring rules (D) and documented every module, class and function outside tests; published contract models and routes now carry these descriptions in their JSON schema and OpenAPI, with no structural schema change - Enabled ruff's annotation rules (ANN) and annotated every function outside tests; test files are exempt from ANN, missing docstrings, magic numbers and bare-assert rules diff --git a/docs/engineering/skills/rules-registry.md b/docs/engineering/skills/rules-registry.md index b4b843a05..a8ee8c544 100644 --- a/docs/engineering/skills/rules-registry.md +++ b/docs/engineering/skills/rules-registry.md @@ -69,7 +69,7 @@ gap is visible, rather than leaving it out. | --- | --- | --- | | `pytest` | `nodes` (`path::test` or `path::Class::test`), optional `project` (sub-project directory the paths are relative to) | Each test exists, and the CI job's command collects it | | `quality-guard` | `name`, optional `runner` | The name is in `GUARDS` in `scripts/run_quality_guards.py` | -| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists | +| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists and covers only [files allowed file-level exceptions](#file-level-exceptions) | | `mypy` | `paths` | Each path is in `[tool.mypy].files` | | `workflow-job` | optional `step` | The CI job exists (the job itself is the check) | | `pyright`, `import-linter` | `config` (+ `contract` for Import Linter) | The config file exists | @@ -117,13 +117,35 @@ fail the guard yet. - **Ruff is never ratcheted.** Adopting a ruff rule family is one standalone PR that changes the select list and fixes every resulting violation. Don't use - `--add-noqa` or blanket per-file ignores. + `--add-noqa`. A finding that is kept gets an inline `# noqa: CODE # reason`, + except in the files below. - Other enforcers (mypy, Pyright, Import Linter, custom guards) may start `ratcheted` with a baseline file of tolerated violations. Baselines may only shrink: the guard compares counts against the base branch. - Moving a rule from enforced/ratcheted to advisory/doc-only needs an explicit `[rule.downgrade] reason`. +## File-level exceptions + +A ruff `per-file-ignores` entry switches rules off for whole files. It is +allowed only where that's the norm rather than an exception to it: + +- **tests**: files under a `tests` or `authenticated_tests` directory; +- **CI/CD scripts**: files under `.github`; +- **command-line scripts**: files under a `scripts` directory, or modules that + run as a program through `if __name__ == "__main__":`. + +Everywhere else, keep a finding only with an inline suppression and its reason. + +Each entry needs a `per-file-ignore` rule (see below). The guard expands the +entry's pattern like ruff does and fails when the pattern: + +- covers any other file; +- is negated; +- matches nothing. + +`scripts/rules/file_exceptions.py` holds these definitions. + ## Hidden rules The guard fails when something enforces a rule that the registry doesn't diff --git a/docs/generated/rules.md b/docs/generated/rules.md index f2daf773d..3be6b78e9 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 | 39 | 1 | 0 | 14 | 54 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 36 of 50 rules. +**Blocking:** 40 of 54 rules. ## Gaps @@ -246,43 +246,63 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | +| `sim.quality.ci-cd-scripts-ruff-exemptions` | CI/CD scripts under .github may print their output (T201); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of T201 for `.github/**` | `pr.yml:lint` | — | +| `sim.quality.command-line-scripts-ruff-exemptions` | Command-line scripts under a scripts directory may print their output (T201), run other programs (S603, S607) and use today's date (DTZ011); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of DTZ011, S603, S607, T201 for `**/scripts/**` | `pr.yml:lint` | — | +| `sim.quality.deployed-tests-ruff-exemptions` | The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (sim.quality.tests-ruff-exemptions); every other ruff rule applies to it. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/authenticated_tests/**` | `pr.yml:lint` | — | | `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 (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, 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 (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, 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.src-programs-ruff-exemptions` | Programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands) may print their output (T201) and open URLs from their arguments (S310); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of S310, T201 for `projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py` | `pr.yml:lint` | — | | `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 and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**` | `pr.yml:lint` | — | +| `sim.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also run repository scripts (S603, S607) and use fake credentials (S105, S106), temporary paths (S108), naive datetimes (DTZ001, DTZ007), pickle (S301) and exec (S102). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR + r_sim_quality_ci_cd_scripts_ruff_exemptions("sim.quality.ci-cd-scripts-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_T201_for__github___["ruff per-file ignore of T201 for .github/**"] + j_CI_pr_yml_lint["CI pr.yml:lint"] + r_sim_quality_command_line_scripts_ruff_exemptions("sim.quality.command-line-scripts-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_DTZ011__S603__S607__T201_for____scripts___["ruff per-file ignore of DTZ011, S603, S607, T201 for **/scripts/**"] + r_sim_quality_deployed_tests_ruff_exemptions("sim.quality.deployed-tests-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____authenticated_tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for **/authenticated_tests/**"] r_sim_quality_every_project_tested("sim.quality.every-project-tested (doc-only)") gap_unenforced>"not enforced"] r_sim_quality_ruff_format_src("sim.quality.ruff-format-src (enforced)") e_ruff_format["ruff format"] - j_CI_pr_yml_lint["CI pr.yml:lint"] 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__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_["ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W)"] + e_ruff_check__ANN__BLE__D__DTZ__E4__E7__E9__EM__ERA__EXE__F__FIX__FLY__FURB__G__I__LOG__PIE__PYI__RET__RSE__S__T20__TC__TD__TRY__UP__W_["ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, 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"] + r_sim_quality_src_programs_ruff_exemptions("sim.quality.src-programs-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_S310__T201_for_projects___src__modal_utils___policyengine_simulation_entry_generate_openapi_policyengine_simulation_entry_stage12_infrastructure_policyengine_simulation_gateway_generate_openapi__py["ruff per-file ignore of S310, T201 for projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py"] r_sim_quality_tests_pass("sim.quality.tests-pass (enforced)") 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__PLR2004__S101_for____tests___["ruff per-file ignore of ANN, D1, PLR2004, S101 for **/tests/**"] + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for **/tests/**"] + r_sim_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_T201_for__github___ + e_ruff_per_file_ignore_of_T201_for__github___ --> j_CI_pr_yml_lint + r_sim_quality_command_line_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_DTZ011__S603__S607__T201_for____scripts___ + e_ruff_per_file_ignore_of_DTZ011__S603__S607__T201_for____scripts___ --> j_CI_pr_yml_lint + r_sim_quality_deployed_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____authenticated_tests___ + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_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__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ - e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ --> j_CI_pr_yml_lint + r_sim_quality_ruff_lint --> e_ruff_check__ANN__BLE__D__DTZ__E4__E7__E9__EM__ERA__EXE__F__FIX__FLY__FURB__G__I__LOG__PIE__PYI__RET__RSE__S__T20__TC__TD__TRY__UP__W_ + e_ruff_check__ANN__BLE__D__DTZ__E4__E7__E9__EM__ERA__EXE__F__FIX__FLY__FURB__G__I__LOG__PIE__PYI__RET__RSE__S__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_src_programs_ruff_exemptions --> e_ruff_per_file_ignore_of_S310__T201_for_projects___src__modal_utils___policyengine_simulation_entry_generate_openapi_policyengine_simulation_entry_stage12_infrastructure_policyengine_simulation_gateway_generate_openapi__py + e_ruff_per_file_ignore_of_S310__T201_for_projects___src__modal_utils___policyengine_simulation_entry_generate_openapi_policyengine_simulation_entry_stage12_infrastructure_policyengine_simulation_gateway_generate_openapi__py --> j_CI_pr_yml_lint 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__PLR2004__S101_for____tests___ - e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___ --> j_CI_pr_yml_lint + r_sim_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____tests___ + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____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 cdc6ce37c..a1356412c 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py @@ -63,10 +63,10 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str token.credentials ).key except jwt.exceptions.PyJWKClientError as error: - LOG.error(f"Unable to get public key for JWT validation: {error}") # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs + 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) except jwt.exceptions.DecodeError as error: - LOG.error(f"Unable to get public key for JWT validation: {error}") # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs + 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) try: @@ -78,6 +78,6 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str issuer=self.issuer, ) except Exception as error: # noqa: BLE001 # any decode or claim-validation failure must become a 403, never a 500 - LOG.info(f"Invalid bearer token: {error}") + LOG.info("Invalid bearer token: %s", error) raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) return payload diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py index d2312e5a3..fc2b13c21 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py @@ -51,14 +51,21 @@ def _get_project_id() -> str: def _get_project_id_from_metadata() -> str | None: from urllib import request - url = "http://metadata.google.internal/computeMetadata/v1/project/project-id" - req = request.Request(url) - req.add_header("Metadata-Flavor", "Google") try: - return request.urlopen(req).read().decode() + return ( + request.urlopen( + request.Request( + "http://metadata.google.internal/computeMetadata/v1/project/project-id", + headers={"Metadata-Flavor": "Google"}, + ) + ) + .read() + .decode() + ) except Exception as err: # noqa: BLE001 # best-effort probe: any failure falls back to GOOGLE_CLOUD_PROJECT log.info( - f"Unable to get the google project id from a local metadata service: {err}" + "Unable to get the google project id from a local metadata service: %s", + err, ) return None diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py index d76e67da7..49b053282 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py @@ -83,9 +83,9 @@ def _fetch_hf_dataset_revision( if token is not None: headers["Authorization"] = f"Bearer {token}" - request = Request(url, headers=headers) + request = Request(url, headers=headers) # noqa: S310 # Hub API URL: the HF_ENDPOINT base (https by default) and quoted path parts try: - with urlopen(request, timeout=HF_REQUEST_TIMEOUT_SECONDS) as response: + with urlopen(request, timeout=HF_REQUEST_TIMEOUT_SECONDS) as response: # noqa: S310 # the Hub API request built above return json.loads(response.read().decode("utf-8")) except HTTPError as exc: detail = exc.reason or f"HTTP {exc.code}" 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 450950d69..bf47aadc3 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py @@ -229,7 +229,8 @@ class SPMRuntimeVersions(BaseModel): spm_calculator: str = Field(min_length=1, alias="spm-calculator") -def _require_complete_selection(selection: SPMSelection) -> None: +def _require_complete_selection(selection: SPMSelection) -> tuple[str, str]: + """Return a complete selection's artifact hash and scenario, else raise ValueError.""" required = set(SPMSelection.model_fields) missing = required.difference(selection.model_fields_set) if missing: @@ -242,18 +243,17 @@ def _require_complete_selection(selection: SPMSelection) -> None: if selection.scenario is None: msg = "Resolved SPM selection has no scenario" raise ValueError(msg) + return selection.forecast_content_sha256, selection.scenario def _resolved_spm_configuration( selection: SPMSelection, ) -> SPMResolvedConfiguration: """Convert a resolved request selection to the completed-result contract.""" - _require_complete_selection(selection) - assert selection.forecast_content_sha256 is not None - assert selection.scenario is not None + forecast_content_sha256, scenario = _require_complete_selection(selection) return SPMResolvedConfiguration( - forecast_content_sha256=selection.forecast_content_sha256, - scenario=selection.scenario, + forecast_content_sha256=forecast_content_sha256, + scenario=scenario, geography_kind=selection.geography_kind, geography_id=selection.geography_id, county_vintage=selection.county_vintage, @@ -284,17 +284,15 @@ def build_spm_provenance( runtime_versions: SPMRuntimeVersions, ) -> SPMProvenance: """Build one compact receipt from resolved calculation inputs.""" - _require_complete_selection(selection) - assert selection.forecast_content_sha256 is not None - assert selection.scenario is not None - if forecast_sha256 != selection.forecast_content_sha256: + forecast_content_sha256, scenario = _require_complete_selection(selection) + if forecast_sha256 != forecast_content_sha256: msg = "SPM receipt artifact hash differs from the selection" raise ValueError(msg) return SPMProvenance( schema_version="canonical-spm-provenance-v2", forecast_id=forecast_id, forecast_sha256=forecast_sha256, - scenario=selection.scenario, + scenario=scenario, geography_kind=selection.geography_kind, geography_id=selection.geography_id, county_vintage=selection.county_vintage, diff --git a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py index ab2473134..5d24d630a 100644 --- a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py +++ b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py @@ -13,6 +13,8 @@ with_hf_revision, ) +RUNTIME_TOKEN = "runtime-token" + class _FakeResponse: def __init__(self, payload: dict): @@ -75,12 +77,12 @@ def fake_fetch(repo_id, revision, token): return {"siblings": [{"rfilename": "dataset.h5"}]} monkeypatch.setattr(hf_dataset, "_fetch_hf_dataset_revision", fake_fetch) - monkeypatch.setenv("HUGGING_FACE_TOKEN", "runtime-token") + monkeypatch.setenv("HUGGING_FACE_TOKEN", RUNTIME_TOKEN) monkeypatch.setenv("PE_UK_PRIVATE_HF_READ_TOKEN", "deployment-token") monkeypatch.setenv("HF_TOKEN", "legacy-token") assert validate_hf_dataset_uri(dataset_uri) == dataset_uri - assert seen["token"] == "runtime-token" + assert seen["token"] == RUNTIME_TOKEN def test_validate_hf_dataset_uri_ignores_non_runtime_credential_names(monkeypatch): diff --git a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py index dc3a4f9d1..119c3b3f5 100644 --- a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py +++ b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py @@ -91,7 +91,7 @@ def log_and_redact_exception( "gateway_error_redacted", attributes=payload, ) - except Exception: # noqa: BLE001 # pragma: no cover - defensive, never raise from logger + except Exception: # noqa: BLE001, S110 # pragma: no cover - defensive, never raise from logger pass return f"{GENERIC_JOB_FAILURE_MESSAGE} (correlation_id={correlation_id})" diff --git a/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py b/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py index 80cfecf78..e399b24ab 100644 --- a/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py +++ b/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py @@ -27,6 +27,22 @@ ) +def _run_bash( + *arguments: str | Path, + check: bool, + capture_output: bool = False, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository deployment script, with text output.""" + return subprocess.run( + ["bash", *arguments], + check=check, + capture_output=capture_output, + text=True, + env=env, + ) + + def _revision(name: str, created_at: str, *, ready: bool = True) -> dict: return { "metadata": { @@ -96,11 +112,10 @@ def _run_cleanup( ) revisions_json.write_text(json.dumps(revisions), encoding="utf-8") - return subprocess.run( - ["bash", CLEANUP_SCRIPT], + return _run_bash( + CLEANUP_SCRIPT, check=False, capture_output=True, - text=True, env=os.environ | { "GCLOUD_BIN": str(_write_fake_gcloud(tmp_path)), @@ -321,5 +336,5 @@ def test_revision_cleanup_runs_after_promoted_deployment_completion(): def test_revision_cleanup_script_has_valid_shell_syntax_and_is_executable(): - subprocess.run(["bash", "-n", CLEANUP_SCRIPT], check=True) + _run_bash("-n", CLEANUP_SCRIPT, check=True) assert os.access(CLEANUP_SCRIPT, os.X_OK) diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py index df84ef1fb..58a0d8f50 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py @@ -58,10 +58,28 @@ ) +def _run_bash( + *arguments: str | Path, + check: bool, + capture_output: bool = False, + env: dict[str, str] | None = None, + cwd: Path | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository deployment script, with text output.""" + return subprocess.run( + ["bash", *arguments], + check=check, + capture_output=capture_output, + text=True, + env=env, + cwd=cwd, + ) + + def test_deployment_scripts_have_valid_shell_syntax(): - subprocess.run(["bash", "-n", TRAFFIC_SCRIPT], check=True) - subprocess.run(["bash", "-n", SMOKE_SCRIPT], check=True) - subprocess.run(["bash", "-n", CLOUD_RUN_DEPLOY_SCRIPT], check=True) + _run_bash("-n", TRAFFIC_SCRIPT, check=True) + _run_bash("-n", SMOKE_SCRIPT, check=True) + _run_bash("-n", CLOUD_RUN_DEPLOY_SCRIPT, check=True) assert os.access(SMOKE_SCRIPT, os.X_OK) assert os.access(CLOUD_RUN_DEPLOY_SCRIPT, os.X_OK) @@ -167,17 +185,13 @@ def test_stable_smoke_retries_the_previous_revision_until_target_is_served( [old_revision, old_revision, target_revision], ) - subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://simulation.example.test", - target_revision, - old_revision, - ], + _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + target_revision, + old_revision, check=True, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -193,17 +207,13 @@ def test_stable_smoke_rejects_an_unexpected_revision_without_retrying(tmp_path: [unexpected_revision, target_revision], ) - result = subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://simulation.example.test", - target_revision, - old_revision, - ], + result = _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + target_revision, + old_revision, check=False, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -220,16 +230,12 @@ def test_candidate_smoke_rejects_a_revision_mismatch_without_retrying(tmp_path: [old_revision, target_revision], ) - result = subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://candidate.example.test", - target_revision, - ], + result = _run_bash( + SMOKE_SCRIPT, + "https://candidate.example.test", + target_revision, check=False, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -248,17 +254,13 @@ def test_stable_smoke_fails_when_the_previous_revision_does_not_converge( [old_revision, old_revision, old_revision], ) - result = subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://simulation.example.test", - target_revision, - old_revision, - ], + result = _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + target_revision, + old_revision, check=False, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -1220,12 +1222,16 @@ def test_a_failing_sentinel_fails_its_step(tmp_path, job): script = step["run"].replace( "${{ needs.deploy_entrypoint.outputs.candidate_url }}", "https://candidate" ) - result = subprocess.run( + result = _run_bash( # GitHub runs `run` blocks with bash -e -o pipefail. - ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", script], + "--noprofile", + "--norc", + "-eo", + "pipefail", + "-c", + script, check=False, capture_output=True, - text=True, env={ "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "RUNNER_TEMP": str(tmp_path), @@ -1294,11 +1300,15 @@ def _run_routing_step_with_a_failing_command(tmp_path, step, failed_command, **e plan = runner_temp / "routing-plans" / "routing-plan-prod-1" / "plan.json" plan.parent.mkdir(parents=True) plan.write_text("{}") - result = subprocess.run( - ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", step["run"]], + result = _run_bash( + "--noprofile", + "--norc", + "-eo", + "pipefail", + "-c", + step["run"], check=False, capture_output=True, - text=True, env={ "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "RUNNER_TEMP": str(runner_temp), @@ -1548,19 +1558,15 @@ def test_failed_release_gate_cannot_report_a_successful_release(tmp_path, failed } env[failed_gate] = "failure" # A later step cannot run after this gate under Actions' bash -e behavior. - result = subprocess.run( - [ - "bash", - "-eo", - "pipefail", - "-c", - gate["run"] + '\ntouch "$1"', - "gate", - str(marker), - ], + result = _run_bash( + "-eo", + "pipefail", + "-c", + gate["run"] + '\ntouch "$1"', + "gate", + str(marker), check=False, capture_output=True, - text=True, env=env, cwd=REPOSITORY_ROOT, ) @@ -1591,11 +1597,13 @@ def test_release_success_requires_completed_finalization_for_published_routing( ] if step.get("name") == "Check deployment result" ) - completed = subprocess.run( - ["bash", "-eo", "pipefail", "-c", gate["run"]], + completed = _run_bash( + "-eo", + "pipefail", + "-c", + gate["run"], check=False, capture_output=True, - text=True, cwd=REPOSITORY_ROOT, env={ "PATH": os.environ["PATH"], @@ -1691,11 +1699,10 @@ def test_traffic_changes_are_exact_revision_validated_and_reversible(tmp_path): "SIMULATION_ENTRYPOINT_SERVICE": "policyengine-simulation-entry", } - subprocess.run( - ["bash", TRAFFIC_SCRIPT], + _run_bash( + TRAFFIC_SCRIPT, check=True, capture_output=True, - text=True, env={ **base_env, "SIMULATION_ENTRYPOINT_TARGET_REVISION": "policyengine-simulation-entry-00002-new", @@ -1707,11 +1714,10 @@ def test_traffic_changes_are_exact_revision_validated_and_reversible(tmp_path): == "policyengine-simulation-entry-00002-new" ) - subprocess.run( - ["bash", TRAFFIC_SCRIPT], + _run_bash( + TRAFFIC_SCRIPT, check=True, capture_output=True, - text=True, env={ **base_env, "SIMULATION_ENTRYPOINT_TARGET_REVISION": "policyengine-simulation-entry-00001-old", @@ -1742,11 +1748,10 @@ def test_traffic_change_refuses_an_intervening_promotion(tmp_path): ) fake_gcloud.chmod(0o755) - result = subprocess.run( - ["bash", TRAFFIC_SCRIPT], + result = _run_bash( + TRAFFIC_SCRIPT, check=False, capture_output=True, - text=True, env={ **os.environ, "GCLOUD_BIN": str(fake_gcloud), @@ -1825,11 +1830,12 @@ def _run_identity_smoke(tmp_path: Path, versions: dict, **expected: str): "FAKE_CURL_VERSIONS": str(versions_file), **expected, } - return subprocess.run( - ["bash", SMOKE_SCRIPT, "https://simulation.example.test", revision], + return _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + revision, check=False, capture_output=True, - text=True, env=env, ) diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py index 972e43b3d..1288e79f3 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py @@ -30,12 +30,27 @@ ) -def _validate(**env: str) -> subprocess.CompletedProcess[str]: +def _run_bash( + *arguments: str | Path, + check: bool, + capture_output: bool = False, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository rollback script or step, with text output.""" return subprocess.run( - ["bash", VALIDATOR], + ["bash", *arguments], + check=check, + capture_output=capture_output, + text=True, + env=env, + ) + + +def _validate(**env: str) -> subprocess.CompletedProcess[str]: + return _run_bash( + VALIDATOR, check=False, capture_output=True, - text=True, env={"PATH": os.environ["PATH"], **env}, ) @@ -306,11 +321,13 @@ def _run_restore( bin_dir = tmp_path / "bin" bin_dir.mkdir() _fake_gh(bin_dir, attempts, kept, inventory) - return subprocess.run( - ["bash", "-eo", "pipefail", "-c", _restore_script()], + return _run_bash( + "-eo", + "pipefail", + "-c", + _restore_script(), check=False, capture_output=True, - text=True, env={ "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "RUNNER_TEMP": str(runner_temp), @@ -400,5 +417,5 @@ def test_rollbacks_share_the_deployment_concurrency_group(): def test_validator_has_valid_shell_syntax_and_is_executable(): - subprocess.run(["bash", "-n", VALIDATOR], check=True) + _run_bash("-n", VALIDATOR, check=True) assert os.access(VALIDATOR, os.X_OK) diff --git a/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py b/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py index 4eb899072..c2d58faa9 100644 --- a/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py +++ b/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py @@ -11,20 +11,23 @@ DEPLOY_WORKFLOW = REPO_ROOT / ".github/workflows/simulation-deploy.reusable.yml" -def test_infrastructure_validation_is_bounded_and_valid_shell() -> None: - result = subprocess.run( - ["bash", "-n", str(SCRIPT)], +def _run_bash( + *arguments: str, env: dict[str, str] | None = None +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository script, capturing text output; a failure is returned.""" + return subprocess.run( + ["bash", *arguments], capture_output=True, text=True, + env=env, check=False, ) + + +def test_infrastructure_validation_is_bounded_and_valid_shell() -> None: + result = _run_bash("-n", str(SCRIPT)) assert result.returncode == 0, result.stderr - sync_result = subprocess.run( - ["bash", "-n", str(SYNC_SCRIPT)], - capture_output=True, - text=True, - check=False, - ) + sync_result = _run_bash("-n", str(SYNC_SCRIPT)) assert sync_result.returncode == 0, sync_result.stderr source = SCRIPT.read_text(encoding="utf-8") assert "STAGE12_ENVIRONMENT" in source @@ -64,13 +67,7 @@ def test_infrastructure_validation_is_bounded_and_valid_shell() -> None: def test_infrastructure_validation_rejects_missing_configuration() -> None: - result = subprocess.run( - ["bash", str(SCRIPT)], - capture_output=True, - text=True, - env={}, - check=False, - ) + result = _run_bash(str(SCRIPT), env={}) assert result.returncode != 0 assert "STAGE12_ENVIRONMENT is required" in result.stderr @@ -98,13 +95,7 @@ def test_storage_validation_uses_only_the_modal_worker_credentials() -> None: def test_modal_secret_sync_rejects_missing_configuration() -> None: - result = subprocess.run( - ["bash", str(SYNC_SCRIPT), "staging"], - capture_output=True, - text=True, - env={}, - check=False, - ) + result = _run_bash(str(SYNC_SCRIPT), "staging", env={}) assert result.returncode != 0 assert "STAGE12_GCP_PROJECT_ID is required" in result.stderr diff --git a/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py b/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py index 84b849f4c..edc93e3fc 100644 --- a/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py +++ b/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py @@ -250,8 +250,8 @@ def updater_env(fake_bin: Path, **extra: str) -> dict[str, str]: def run_updater(*args: str, env: dict[str, str]) -> subprocess.CompletedProcess[str]: """Run the updater from the repo root; a non-zero exit is returned, not raised.""" - return subprocess.run( - ["bash", str(SCRIPT), *args], + return subprocess.run( # noqa: S603 # runs the repository updater script with test-built arguments + ["bash", str(SCRIPT), *args], # noqa: S607 # bash from PATH, as the workflow runs the script cwd=REPO_ROOT, env=env, capture_output=True, diff --git a/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py b/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py index 2d091ee87..14f6c642a 100644 --- a/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py @@ -24,7 +24,9 @@ def __init__(self, payload: dict[str, object]) -> None: def model_dump(self, *, mode: str) -> dict[str, object]: """Return the payload; fails unless the adapter asks for JSON mode.""" - assert mode == "json" + if mode != "json": + msg = f"The adapter must dump outputs in JSON mode, not {mode!r}" + raise AssertionError(msg) return self.payload diff --git a/projects/policyengine-simulation-executor/src/modal/app.py b/projects/policyengine-simulation-executor/src/modal/app.py index bb15a0318..97734e86b 100644 --- a/projects/policyengine-simulation-executor/src/modal/app.py +++ b/projects/policyengine-simulation-executor/src/modal/app.py @@ -259,7 +259,7 @@ def _configure_modal_observability( def _set_modal_call_attributes(runtime: "ObservabilityRuntime") -> None: try: runtime.set_context(function_call_id=modal.current_function_call_id()) - except Exception: # noqa: BLE001 # telemetry is non-fatal: a failed attribute must never fail the Modal call + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal: a failed attribute must never fail the Modal call pass @@ -314,7 +314,7 @@ def _record_stage_durations( for stage, seconds in durations.items() }, ) - except Exception: # noqa: BLE001 # telemetry is non-fatal + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass diff --git a/projects/policyengine-simulation-executor/src/modal/segmented_national.py b/projects/policyengine-simulation-executor/src/modal/segmented_national.py index f01a0b577..d66b0775a 100644 --- a/projects/policyengine-simulation-executor/src/modal/segmented_national.py +++ b/projects/policyengine-simulation-executor/src/modal/segmented_national.py @@ -524,7 +524,7 @@ def _fail_if_budget_exhausted( def _cancel_call(call: modal.FunctionCall) -> None: try: call.cancel() - except Exception: # noqa: BLE001 # best-effort cleanup: a failed cancel must not mask the failure being raised, and superseded calls are cancelled again later + 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 def _cancel_superseded_children(self) -> None: diff --git a/projects/policyengine-simulation-executor/src/modal/smoke_app.py b/projects/policyengine-simulation-executor/src/modal/smoke_app.py index 65ad946e8..40320881c 100644 --- a/projects/policyengine-simulation-executor/src/modal/smoke_app.py +++ b/projects/policyengine-simulation-executor/src/modal/smoke_app.py @@ -169,5 +169,5 @@ def smoke_import_executor() -> dict: def main() -> None: """Run the image smoke remotely and print its report.""" report = smoke_import_executor.remote() - print(report) - print("executor image smoke OK") + print(report) # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report + print("executor image smoke OK") # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report 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 ea776a9ad..c930145f9 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py @@ -323,6 +323,11 @@ def check_deployment( return problems +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) + + def main(argv: list[str] | None = None) -> int: """Run the ``expected`` or ``check`` command and return its exit status.""" parser = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) @@ -340,7 +345,7 @@ def main(argv: list[str] | None = None) -> int: with open(args.output, "w") as handle: json.dump(expected, handle, indent=2, sort_keys=True) for case in expected["cases"]: - print(f"{case['case_id']} ({case['seconds']}s): {case['values']}") + _report(f"{case['case_id']} ({case['seconds']}s): {case['values']}") return 0 with open(args.expected) as handle: @@ -352,13 +357,11 @@ def main(argv: list[str] | None = None) -> int: timeout_seconds=args.timeout_seconds, ) if problems: - print( - "The deployed API does not reproduce the bundle's numbers:", file=sys.stderr - ) + _report("The deployed API does not reproduce the bundle's numbers:", error=True) for problem in problems: - print(f" {problem}", file=sys.stderr) + _report(f" {problem}", error=True) return 1 - print( + _report( f"{args.base_url} reproduces {len(expected['cases'])} sentinel case(s) for " f"policyengine {expected['policyengine_version']}, UK dataset " f"{expected['uk']['dataset_sha256']}" 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 6fef5ef89..9ab9f69f0 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 @@ -1085,6 +1085,11 @@ def restore_routing_state( return next_state +def _report(line: str = "") -> None: + """Print one line of the command's progress report.""" + print(line) + + def main() -> None: """Publish a deployed app's routes, or seed the state from legacy Dicts.""" parser = argparse.ArgumentParser( @@ -1240,17 +1245,17 @@ def active_state() -> object: return snapshot.get(ROUTING_STATE_ACTIVE_KEY) if args.print_latest: - print(read_latest_policyengine_version(environment=args.environment) or "") + _report(read_latest_policyengine_version(environment=args.environment) or "") return if args.print_identity: for key, value in latest_release_identity(active_state()).items(): - print(f"{key}={value}") + _report(f"{key}={value}") return if args.print_pending_plan: store = _routing_store(args.environment, create_if_missing=False) - print(json.dumps(_unfinished_publication(store), indent=2, sort_keys=True)) + _report(json.dumps(_unfinished_publication(store), indent=2, sort_keys=True)) return writes_routing = ( @@ -1279,10 +1284,10 @@ def active_state() -> object: store = _routing_store(args.environment, create_if_missing=False) pending = _unfinished_publication(store) if pending is None: - print("No pending routing publication; routing is unchanged.") + _report("No pending routing publication; routing is unchanged.") else: - print("Pending routing publication:") - print( + _report("Pending routing publication:") + _report( json.dumps(store.get(ROUTING_PUBLICATION_KEY), indent=2, sort_keys=True) ) outcome = finalize_routing_publication( @@ -1292,10 +1297,10 @@ def active_state() -> object: publication_run=args.publication_run, ) if outcome == "foreign-publication": - print( + _report( "Pending routing publication belongs to another run or environment; routing is unchanged." ) - print(f"Routing publication finalization: {outcome}") + _report(f"Routing publication finalization: {outcome}") if args.github_output: with open(args.github_output, "a") as handle: handle.write(f"outcome={outcome}\n") @@ -1306,7 +1311,7 @@ def active_state() -> object: plan = json.load(handle) operation = record_routing_plan if args.record_plan else complete_routing_plan operation(environment=args.environment, plan=plan) - print( + _report( f"Routing publication {plan['publication_id']}: " f"{'pending' if args.record_plan else 'completed'}" ) @@ -1319,7 +1324,7 @@ def active_state() -> object: environment=args.environment, plan=plan, ) - print( + _report( f"Published policyengine {plan['policyengine_version']} routing " f"({plan['after_sha256']}) in Modal environment {args.environment}" ) @@ -1334,7 +1339,7 @@ def active_state() -> object: environment=args.environment, plans=plans, ) - print( + _report( f"Routing for policyengine {plans[-1]['policyengine_version']}: " f"{outcome} in Modal environment {args.environment}" ) @@ -1346,28 +1351,28 @@ def active_state() -> object: policyengine_version=args.restore_latest, expected_latest=args.expected_latest, ) - print(f"Restored routing state in Modal environment: {args.environment}") - print(f" latest.policyengine: {state['latest']['policyengine']}") - print(f" latest.us: {state['latest']['us']}") - print(f" latest.uk: {state['latest']['uk']}") + _report(f"Restored routing state in Modal environment: {args.environment}") + _report(f" latest.policyengine: {state['latest']['policyengine']}") + _report(f" latest.us: {state['latest']['us']}") + _report(f" latest.uk: {state['latest']['uk']}") return if args.expected_latest: parser.error("--expected-latest needs --restore-latest") if args.seed_active_from_legacy: - print( + _report( "Seeding active routing state from legacy Modal dicts in " f"environment: {args.environment}" ) state = seed_active_routing_state_from_legacy(environment=args.environment) - print() - print(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") - print(f" policyengine routes: {len(state['routes']['policyengine'])}") - print(f" US routes: {len(state['routes']['us'])}") - print(f" UK routes: {len(state['routes']['uk'])}") - print(f" bundle manifests: {len(state['bundles'])}") - print() - print("Legacy routing state seed completed successfully.") + _report() + _report(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") + _report(f" policyengine routes: {len(state['routes']['policyengine'])}") + _report(f" US routes: {len(state['routes']['us'])}") + _report(f" UK routes: {len(state['routes']['uk'])}") + _report(f" bundle manifests: {len(state['bundles'])}") + _report() + _report("Legacy routing state seed completed successfully.") return required_args = { @@ -1393,17 +1398,17 @@ def active_state() -> object: if args.plan_out: with open(args.plan_out, "w") as handle: json.dump(plan, handle, indent=2, sort_keys=True) - print(f"before_sha256={plan['before_sha256']}") - print(f"after_sha256={plan['after_sha256']}") - print(f"advances_latest={'true' if plan['advances_latest'] else 'false'}") + _report(f"before_sha256={plan['before_sha256']}") + _report(f"after_sha256={plan['after_sha256']}") + _report(f"advances_latest={'true' if plan['advances_latest'] else 'false'}") return - print(f"Publishing routing state in Modal environment: {args.environment}") - print(f" App name: {args.app_name}") - print(f" policyengine.py version: {args.policyengine_version}") - print(f" US version: {args.us_version}") - print(f" UK version: {args.uk_version}") - print() + _report(f"Publishing routing state in Modal environment: {args.environment}") + _report(f" App name: {args.app_name}") + _report(f" policyengine.py version: {args.policyengine_version}") + _report(f" US version: {args.us_version}") + _report(f" UK version: {args.uk_version}") + _report() state = publish_routing_state( environment=args.environment, @@ -1413,13 +1418,13 @@ def active_state() -> object: uk_version=args.uk_version, force_latest=args.force_latest, ) - print() - print(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") - print(f" latest.policyengine: {state['latest']['policyengine']}") - print(f" latest.us: {state['latest']['us']}") - print(f" latest.uk: {state['latest']['uk']}") - print() - print("Routing state published successfully.") + _report() + _report(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") + _report(f" latest.policyengine: {state['latest']['policyengine']}") + _report(f" latest.us: {state['latest']['us']}") + _report(f" latest.uk: {state['latest']['uk']}") + _report() + _report("Routing state published successfully.") if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py b/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py index 47c15c8b9..3c022295a 100644 --- a/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py +++ b/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py @@ -71,4 +71,4 @@ def main() -> None: f"UK dataset boundary version: {observed!r}" ) raise RuntimeError(msg) - print("Stage 12 static runtime file smoke OK") + print("Stage 12 static runtime file smoke OK") # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report 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 3bf906d7e..f7b8b069b 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 @@ -13,4 +13,4 @@ with open(output_path, "w") as f: json.dump(openapi_spec, f, indent=4) -print(f"OpenAPI spec written to {output_path}") +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/hf_access_validation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/hf_access_validation.py index 30358bb9b..d08f89188 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/hf_access_validation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/hf_access_validation.py @@ -16,8 +16,8 @@ ) HF_RUNTIME_ENV_NAME = "HUGGING_FACE_TOKEN" -HF_MODAL_SECRET_NAME = "pe-uk-private-hf-read-token" -EXPECTED_HF_TOKEN_DISPLAY_NAME = "pe-uk-private-hf-read-token" +HF_MODAL_SECRET_NAME = "pe-uk-private-hf-read-token" # noqa: S105 # the Modal secret's name, not its value +EXPECTED_HF_TOKEN_DISPLAY_NAME = "pe-uk-private-hf-read-token" # noqa: S105 # the token's Hugging Face display name, not its value class HFAccessTarget(BaseModel): diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py index feed3409a..2c2aadf7c 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py @@ -446,7 +446,7 @@ def _receipt_path() -> Path: explicit = os.environ.get("POLICYENGINE_BUNDLE_RECEIPT") if explicit: return Path(explicit) - data_folder = os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") + data_folder = os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") # noqa: S108 # fixed data directory inside the Modal container return Path(data_folder) / BUNDLE_RECEIPT_FILENAME diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py index 57203d8fb..d42e5da1c 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py @@ -13,7 +13,7 @@ ) from policyengine_simulation_executor.simulation_runtime import run_simulation_impl -logger = logging.getLogger(__file__) +logger = logging.getLogger(__name__) def create_router(runtime: ObservabilityRuntime) -> APIRouter: 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 001bfd0ba..5e3d9d079 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 @@ -130,7 +130,7 @@ def setup_gcp_credentials( for k, v in os.environ.items() if "GOOGLE" in k or "GCP" in k or "CREDENTIAL" in k } - logger.info(f"GCP-related env vars: {list(gcp_vars.keys())}") + logger.info("GCP-related env vars: %s", list(gcp_vars.keys())) if previous: logger.info("GOOGLE_APPLICATION_CREDENTIALS already set") @@ -158,7 +158,7 @@ def setup_gcp_credentials( creds_file.write(normalized) creds_file.flush() os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = creds_file.name - logger.info(f"GCP credentials written to {creds_file.name}") + logger.info("GCP credentials written to %s", creds_file.name) yield finally: @@ -222,7 +222,7 @@ def _parse_policy_period_dates(text: str) -> tuple[datetime, datetime | None]: raise ValueError # The wrapper has always parsed dates with strptime, which also takes # unpadded months and days ("2027-4-6"); accept what it accepted. - dates = [datetime.strptime(part, "%Y-%m-%d") for part in parts] + dates = [datetime.strptime(part, "%Y-%m-%d") for part in parts] # noqa: DTZ007 # naive like policyengine.py's own period parsing; its ParameterValue dates are naive start = dates[0] stop = dates[1] if len(dates) == 2 else None if stop is not None and stop < start: @@ -647,13 +647,13 @@ def resolve_data_folder() -> str: this folder for ``ensure_datasets``/``Simulation.ensure()`` to find, so any divergence from ``_load_dataset``'s resolution strands them. """ - return os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") + return os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") # noqa: S108 # fixed data directory inside the Modal container def _nondefault_data_folder(country: str, name: str, uri: str) -> str: """Separate managed datasets even if their source filenames have the same stem.""" identity = hashlib.sha256(f"{country}:{name}:{uri}".encode()).hexdigest() - return f"/tmp/policyengine-alternate-data/{identity}" + return f"/tmp/policyengine-alternate-data/{identity}" # noqa: S108 # fixed data directory inside the Modal container def _resolve_dataset_selection( diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py index 0522f525c..34473ae93 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py @@ -241,11 +241,11 @@ def qualify_report_parity( run_simulation_impl, ) - local_runtime = _local_qualification_runtime() + qualification_runtime = _local_qualification_runtime() + local_runtime = qualification_runtime def run_existing(request: dict[str, Any]) -> Mapping[str, Any]: - assert local_runtime is not None - return run_simulation_impl(request, runtime=local_runtime) + return run_simulation_impl(request, runtime=qualification_runtime) existing_runner = run_existing if single_simulation_runner is None: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py index 9f7baa350..1fa12f85c 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py @@ -86,7 +86,7 @@ ) _BEARER_TOKEN = re.compile(r"(?i)\bbearer\s+[A-Za-z0-9._~+/=-]+") _SECRET_KEY = ( - r"[\w-]*(?:password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key" + r"[\w-]*(?:password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key" # noqa: S105 # a pattern for secret-looking key names, not a secret r"|private[_-]?key|credentials?|signature|sig)" ) _SECRET_ASSIGNMENT = re.compile( 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 cf7919c9f..843a966f0 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 @@ -13,43 +13,27 @@ PROJECT_ROOT = Path(__file__).resolve().parents[2] -@pytest.mark.integration -def test_executor_image_smoke(): +def _run_modal_smoke(app_path: str) -> subprocess.CompletedProcess[str]: + """Run one smoke app with ``modal run`` in the ``MODAL_SMOKE_ENV`` environment.""" env_name = os.environ.get("MODAL_SMOKE_ENV", "staging") - result = subprocess.run( - [ - "uv", - "run", - "modal", - "run", - f"--env={env_name}", - "src/modal/smoke_app.py", - ], + return subprocess.run( + ["uv", "run", "modal", "run", f"--env={env_name}", app_path], cwd=PROJECT_ROOT, capture_output=True, text=True, timeout=2400, ) + + +@pytest.mark.integration +def test_executor_image_smoke(): + result = _run_modal_smoke("src/modal/smoke_app.py") assert result.returncode == 0, result.stdout + result.stderr assert "executor image smoke OK" in result.stdout @pytest.mark.integration def test_stage12_static_runtime_file_image_smoke(): - env_name = os.environ.get("MODAL_SMOKE_ENV", "staging") - result = subprocess.run( - [ - "uv", - "run", - "modal", - "run", - f"--env={env_name}", - "src/modal/v2_static_runtime_files_smoke.py", - ], - cwd=PROJECT_ROOT, - capture_output=True, - text=True, - timeout=2400, - ) + result = _run_modal_smoke("src/modal/v2_static_runtime_files_smoke.py") assert result.returncode == 0, result.stdout + result.stderr assert "Stage 12 static runtime file smoke OK" in result.stdout 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 798704db2..94fec7bb4 100644 --- a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py +++ b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py @@ -103,16 +103,25 @@ def test_rule_proof_gate_check() -> None: assert gate_violations(good) == [] +def _run_gate_script(env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["bash", GATE_SCRIPT], + cwd=REPO, + env=env, + text=True, + capture_output=True, + check=False, + ) + + def _run_gate( job_results: dict[str, str], *, pull_request_base_ref: str = "main", stack_base_ref: str = "", ) -> subprocess.CompletedProcess[str]: - return subprocess.run( - ["bash", GATE_SCRIPT], - cwd=REPO, - env={ + return _run_gate_script( + { "HOME": os.environ.get("HOME", ""), "PATH": os.environ["PATH"], "JOB_RESULTS": json.dumps( @@ -123,10 +132,7 @@ def _run_gate( ), "PULL_REQUEST_BASE_REF": pull_request_base_ref, "STACK_BASE_REF": stack_base_ref, - }, - text=True, - capture_output=True, - check=False, + } ) @@ -179,13 +185,8 @@ def test_gate_fails_when_a_job_was_skipped_on_full_ci() -> None: def test_gate_requires_the_job_results() -> None: - result = subprocess.run( - ["bash", GATE_SCRIPT], - cwd=REPO, - env={"PATH": os.environ["PATH"], "PULL_REQUEST_BASE_REF": "main"}, - text=True, - capture_output=True, - check=False, + result = _run_gate_script( + {"PATH": os.environ["PATH"], "PULL_REQUEST_BASE_REF": "main"} ) assert result.returncode != 0 diff --git a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py index 3b9b9f45f..150748110 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py @@ -6,12 +6,31 @@ import os import subprocess +from pathlib import Path import pytest from fixtures.test_modal_scripts import REPO_ROOT, SCRIPTS_DIR pytest_plugins = ("fixtures.test_modal_scripts",) +FAKE_HF_TOKEN = "hf_test" + + +def _run_bash( + *arguments: str, + env: dict[str, str] | None = None, + cwd: Path | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository script, capturing text output; a failure is returned.""" + return subprocess.run( + ["bash", *arguments], + capture_output=True, + text=True, + env=env, + cwd=cwd, + check=False, + ) + class TestModalExtractVersions: """Tests for modal-extract-versions.sh.""" @@ -24,11 +43,7 @@ def test_script_exists(self): def test_script_is_executable_or_can_be_run_with_bash(self): """Script should be runnable with bash.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error in script: {result.stderr}" def test_extracts_versions_from_policyengine_bundle(self, temp_github_output): @@ -41,12 +56,7 @@ def test_extracts_versions_from_policyengine_bundle(self, temp_github_output): env = os.environ.copy() env["GITHUB_OUTPUT"] = temp_github_output - result = subprocess.run( - ["bash", str(self.script), str(project_dir)], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), str(project_dir), env=env) assert result.returncode == 0, f"Script failed: {result.stderr}" @@ -101,28 +111,18 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_base_url_argument(self): """Should fail when no URL is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without URL argument" def test_fails_on_unreachable_url(self): """Should fail when URL is unreachable.""" - result = subprocess.run( - ["bash", str(self.script), "http://localhost:99999/nonexistent", "1", "1"], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), "http://localhost:99999/nonexistent", "1", "1" ) assert result.returncode != 0, "Should fail on unreachable URL" @@ -150,13 +150,7 @@ def test_requires_hf_token_before_running_modal(self, tmp_path): ) env.pop("PE_UK_PRIVATE_HF_READ_TOKEN", None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "PE_UK_PRIVATE_HF_READ_TOKEN is required" in result.stderr @@ -176,23 +170,17 @@ def test_syncs_expected_hf_key_before_image_smokes(self, tmp_path): { "PATH": f"{fake_bin}:{env['PATH']}", "UV_CALLS_LOG": str(uv_calls_log), - "PE_UK_PRIVATE_HF_READ_TOKEN": "hf_test", + "PE_UK_PRIVATE_HF_READ_TOKEN": FAKE_HF_TOKEN, } ) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode == 0, result.stderr calls = uv_calls_log.read_text(encoding="utf-8").splitlines() assert calls[0] == ( "run modal secret create pe-uk-private-hf-read-token " - "HUGGING_FACE_TOKEN=hf_test --env=staging --force" + f"HUGGING_FACE_TOKEN={FAKE_HF_TOKEN} --env=staging --force" ) assert len(calls) == 4 @@ -268,11 +256,7 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_generates_success_summary(self, temp_github_step_summary): @@ -280,17 +264,12 @@ def test_generates_success_summary(self, temp_github_step_summary): env = os.environ.copy() env["GITHUB_STEP_SUMMARY"] = temp_github_step_summary - result = subprocess.run( - [ - "bash", - str(self.script), - "success", - "https://beta.example.com", - "success", - "https://prod.example.com", - ], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), + "success", + "https://beta.example.com", + "success", + "https://prod.example.com", env=env, ) @@ -312,17 +291,12 @@ def test_generates_skipped_summary(self, temp_github_step_summary): env = os.environ.copy() env["GITHUB_STEP_SUMMARY"] = temp_github_step_summary - result = subprocess.run( - [ - "bash", - str(self.script), - "skipped", - "", - "success", - "https://prod.example.com", - ], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), + "skipped", + "", + "success", + "https://prod.example.com", env=env, ) @@ -345,38 +319,25 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_modal_environment_argument(self): """Should fail when no modal environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without modal environment" def test_fails_when_gateway_auth_config_is_partial(self): """Should fail before touching Modal when auth config is partial.""" env = os.environ.copy() - env["PE_UK_PRIVATE_HF_READ_TOKEN"] = "hf_test" + env["PE_UK_PRIVATE_HF_READ_TOKEN"] = FAKE_HF_TOKEN env["GCP_CREDENTIALS_JSON"] = '{"type":"service_account"}' env["GATEWAY_AUTH_ISSUER"] = "https://tenant.auth0.com" env.pop("GATEWAY_AUTH_AUDIENCE", None) env.pop("GATEWAY_AUTH_CLIENT_ID", None) env.pop("GATEWAY_AUTH_CLIENT_SECRET", None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "Gateway auth config is partial." in result.stderr @@ -384,7 +345,7 @@ def test_fails_when_gateway_auth_config_is_partial(self): def test_fails_when_auth_required_but_gateway_auth_vars_missing(self): """Required auth must refuse deploy when the GitHub secrets are absent.""" env = os.environ.copy() - env["PE_UK_PRIVATE_HF_READ_TOKEN"] = "hf_test" + env["PE_UK_PRIVATE_HF_READ_TOKEN"] = FAKE_HF_TOKEN env["GCP_CREDENTIALS_JSON"] = '{"type":"service_account"}' env["GATEWAY_AUTH_REQUIRED"] = "1" for key in ( @@ -395,12 +356,7 @@ def test_fails_when_auth_required_but_gateway_auth_vars_missing(self): ): env.pop(key, None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "GATEWAY_AUTH_REQUIRED is enabled" in result.stderr @@ -408,16 +364,10 @@ def test_fails_when_auth_required_but_gateway_auth_vars_missing(self): def test_requires_gcp_credentials(self): """Artifact credential synchronization must fail before deployment.""" env = os.environ.copy() - env["PE_UK_PRIVATE_HF_READ_TOKEN"] = "hf_test" + env["PE_UK_PRIVATE_HF_READ_TOKEN"] = FAKE_HF_TOKEN env.pop("GCP_CREDENTIALS_JSON", None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "GCP_CREDENTIALS_JSON is required" in result.stderr @@ -436,7 +386,7 @@ def test_creates_gateway_secret_with_normalized_issuer(self, tmp_path): { "PATH": f"{fake_bin}:{env['PATH']}", "UV_CALLS_LOG": str(uv_calls_log), - "PE_UK_PRIVATE_HF_READ_TOKEN": "hf_test", + "PE_UK_PRIVATE_HF_READ_TOKEN": FAKE_HF_TOKEN, "GCP_CREDENTIALS_JSON": '{"type":"service_account"}', "GATEWAY_AUTH_ISSUER": "https://tenant.auth0.com", "GATEWAY_AUTH_AUDIENCE": "https://simulation-api-beta.policyengine.org", @@ -446,12 +396,7 @@ def test_creates_gateway_secret_with_normalized_issuer(self, tmp_path): } ) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode == 0, result.stderr calls = uv_calls_log.read_text() @@ -485,7 +430,7 @@ def test_gcp_secret_sync_failure_stops_deployment(self, tmp_path): { "PATH": f"{fake_bin}:{env['PATH']}", "UV_CALLS_LOG": str(uv_calls_log), - "PE_UK_PRIVATE_HF_READ_TOKEN": "hf_test", + "PE_UK_PRIVATE_HF_READ_TOKEN": FAKE_HF_TOKEN, "GCP_CREDENTIALS_JSON": '{"type":"service_account"}', } ) @@ -498,13 +443,7 @@ def test_gcp_secret_sync_failure_stops_deployment(self, tmp_path): ): env.pop(key, None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 calls = uv_calls_log.read_text() @@ -555,12 +494,7 @@ def _run_with_fake_uv( else: env["POLICYENGINE_ARTIFACT_BUCKET"] = bucket - result = subprocess.run( - ["bash", str(self.script), *args], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), *args, env=env) calls = ( log_path.read_text(encoding="utf-8").splitlines() if log_path.exists() @@ -577,20 +511,12 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_modal_environment_argument(self): """Should fail when no modal environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without modal environment" def test_requires_artifact_bucket(self, tmp_path): @@ -784,12 +710,7 @@ def _run_with_fake_uv(self, tmp_path, *args, bucket="policyengine-sim-artifacts" else: env["POLICYENGINE_ARTIFACT_BUCKET"] = bucket - result = subprocess.run( - ["bash", str(self.script), *args], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), *args, env=env) calls = ( log_path.read_text(encoding="utf-8").splitlines() if log_path.exists() @@ -803,11 +724,7 @@ def test_script_exists(self): def test_requires_environment_argument(self): """Should fail with no arguments.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0 def test_requires_manifest_digest_argument(self, tmp_path): @@ -879,20 +796,12 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_modal_environment_argument(self): """Should fail when no modal environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without modal environment" @@ -907,11 +816,7 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" @@ -926,29 +831,17 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_environment_argument(self): """Should fail when no environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without environment" def test_requires_base_url_argument(self): """Should fail when no base URL is provided.""" - result = subprocess.run( - ["bash", str(self.script), "beta"], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script), "beta") assert result.returncode != 0, "Should fail without base URL" def test_fails_when_auth_config_is_partial(self): @@ -959,12 +852,7 @@ def test_fails_when_auth_config_is_partial(self): env.pop("SIMULATION_TEST_AUTH_CLIENT_ID", None) env.pop("SIMULATION_TEST_AUTH_CLIENT_SECRET", None) - result = subprocess.run( - ["bash", str(self.script), "beta", "https://example.com"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "beta", "https://example.com", env=env) assert result.returncode != 0 assert "Simulation integration-test auth config is partial." in result.stderr @@ -981,12 +869,7 @@ def test_fails_when_auth_required_but_auth_vars_missing(self): ): env.pop(key, None) - result = subprocess.run( - ["bash", str(self.script), "beta", "https://example.com"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "beta", "https://example.com", env=env) assert result.returncode != 0 assert "SIMULATION_TEST_AUTH_REQUIRED is enabled" in result.stderr @@ -1018,17 +901,12 @@ def test_exports_us_and_uk_model_versions_to_integration_tests(self, tmp_path): ): env.pop(key, None) - result = subprocess.run( - [ - "bash", - str(self.script), - "prod", - "https://example.com", - "1.690.7", - "2.88.20", - ], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), + "prod", + "https://example.com", + "1.690.7", + "2.88.20", env=env, cwd=REPO_ROOT, ) @@ -1060,11 +938,7 @@ def test_all_scripts_have_strict_mode(self, all_modal_scripts): def test_all_scripts_have_valid_syntax(self, all_modal_scripts): """All scripts should pass bash syntax check.""" for script in all_modal_scripts: - result = subprocess.run( - ["bash", "-n", str(script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(script)) assert result.returncode == 0, ( f"{script.name} has syntax errors: {result.stderr}" ) 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 1450fe5b7..ab3fc4828 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py @@ -96,6 +96,19 @@ def _modal_import_env() -> dict[str, str]: return env +def _run_python_snippet( + code: str, env: dict[str, str] +) -> subprocess.CompletedProcess[str]: + """Run ``code`` in a fresh interpreter from the project root, capturing text output.""" + return subprocess.run( + [sys.executable, "-c", code], + cwd=REPO_ROOT, + env=env, + capture_output=True, + text=True, + ) + + def test_modal_app_remote_import_uses_image_version_env(): code = """ import json @@ -124,13 +137,7 @@ def fail(message): """ env = _modal_import_env() env.update(VERSION_ENV) - result = subprocess.run( - [sys.executable, "-c", code], - cwd=REPO_ROOT, - env=env, - capture_output=True, - text=True, - ) + result = _run_python_snippet(code, env) assert result.returncode == 0, result.stderr output = result.stdout.strip() @@ -147,13 +154,7 @@ def test_modal_app_remote_import_fails_clearly_without_version_env(): import src.modal.app """ - result = subprocess.run( - [sys.executable, "-c", code], - cwd=REPO_ROOT, - env=_modal_import_env(), - capture_output=True, - text=True, - ) + result = _run_python_snippet(code, _modal_import_env()) assert result.returncode != 0 assert "POLICYENGINE_VERSION must be set" in result.stderr diff --git a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py index a627dd5a8..85d095d2d 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py @@ -403,15 +403,30 @@ def test_update_policyengine_package_rejects_direct_component_requirements( ) -def _verify(payload: str) -> subprocess.CompletedProcess[str]: - import os - +def _run_helper( + command: str, + *, + check: bool = False, + input_text: str | None = None, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run one command of the repository's Python update helper.""" return subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=INSTALLED, + [sys.executable, str(HELPER), command], + input=input_text, capture_output=True, text=True, - check=False, + check=check, + env=env, + ) + + +def _verify(payload: str) -> subprocess.CompletedProcess[str]: + import os + + return _run_helper( + "verify-dispatch", + input_text=INSTALLED, env={**os.environ, "RELEASE_PAYLOAD": payload}, ) @@ -476,12 +491,9 @@ def test_verify_dispatch_compares_the_certificate_when_installed_reports_one() - ) def verify(payload: str) -> subprocess.CompletedProcess[str]: - return subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=installed, - capture_output=True, - text=True, - check=False, + return _run_helper( + "verify-dispatch", + input_text=installed, env={**os.environ, "RELEASE_PAYLOAD": payload}, ) @@ -496,12 +508,9 @@ def test_verify_dispatch_fails_closed_when_the_installed_bundle_cannot_answer() import os installed = INSTALLED.replace("uk_dataset_sha256=" + "a" * 64, "uk_dataset_sha256=") - result = subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=installed, - capture_output=True, - text=True, - check=False, + result = _run_helper( + "verify-dispatch", + input_text=installed, env={**os.environ, "RELEASE_PAYLOAD": _payload()}, ) assert result.returncode != 0 @@ -589,10 +598,8 @@ def test_bundle_versions_matches_policyengine_release_identity() -> None: bundle = get_current_bundle() record = bundle["data_releases"]["uk"] - output = subprocess.run( - [sys.executable, str(HELPER), "bundle-versions"], - capture_output=True, - text=True, + output = _run_helper( + "bundle-versions", check=True, ).stdout installed = dict(line.split("=", 1) for line in output.splitlines() if "=" in line) @@ -608,12 +615,9 @@ def test_bundle_versions_matches_policyengine_release_identity() -> None: } }, } - result = subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=output, - capture_output=True, - text=True, - check=False, + result = _run_helper( + "verify-dispatch", + input_text=output, env={**os.environ, "RELEASE_PAYLOAD": json.dumps(payload)}, ) assert result.returncode == 0, result.stderr @@ -641,10 +645,8 @@ def test_bundle_versions_reports_the_receipt_certificate_digest(tmp_path) -> Non "sys.modules['policyengine.provenance.receipt'] = module\n" ) env = {**os.environ, "PYTHONPATH": str(stand_in)} - output = subprocess.run( - [sys.executable, str(HELPER), "bundle-versions"], - capture_output=True, - text=True, + output = _run_helper( + "bundle-versions", check=True, env=env, ).stdout @@ -657,12 +659,9 @@ def test_bundle_versions_reports_the_receipt_certificate_digest(tmp_path) -> Non record = get_current_bundle()["data_releases"]["uk"] expected = hashlib.sha256(json.dumps(record, sort_keys=True).encode()).hexdigest() assert installed["uk_certificate_sha256"] == expected - mismatch = subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=output, - capture_output=True, - text=True, - check=False, + mismatch = _run_helper( + "verify-dispatch", + input_text=output, env={ **os.environ, "RELEASE_PAYLOAD": json.dumps( 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 6b157f1fd..500b24477 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py @@ -80,6 +80,8 @@ ) from policyengine_simulation_observability.stages import Stage +CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" + def _params_with_spm(params, selection): """Return request parameters after the runtime resolves SPM settings.""" @@ -1773,7 +1775,7 @@ def compute(baseline_simulation, reform_simulation, **kwargs): assert baseline_simulation is baseline assert reform_simulation is reform assert kwargs == { - "constituency_csv_path": "/tmp/constituencies_2024.csv", + "constituency_csv_path": CONSTITUENCY_CSV_PATH, "download_missing_assets": False, } return SimpleNamespace(constituency_results=expected) @@ -1787,7 +1789,7 @@ def compute(baseline_simulation, reform_simulation, **kwargs): monkeypatch.setattr( "policyengine_simulation_executor.simulation_output_geographic." "_required_uk_geography_lookup_csv_path", - lambda spec: "/tmp/constituencies_2024.csv", + 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 74b05af23..0a5f65343 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py @@ -14,6 +14,8 @@ ) from policyengine_simulation_executor import simulation_output_geographic +CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" + def _complete_constituency_record() -> dict[str, object]: return { @@ -54,7 +56,7 @@ def resolve(spec, **kwargs): assert spec is CONSTITUENCY_ASSET_SPEC assert kwargs == {"download_missing_assets": True} observed.append("lookup-resolved") - return "/tmp/constituencies_2024.csv" + return CONSTITUENCY_CSV_PATH monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime.setup_gcp_credentials", @@ -69,7 +71,7 @@ def resolve(spec, **kwargs): CONSTITUENCY_ASSET_SPEC ) - assert result == "/tmp/constituencies_2024.csv" + assert result == CONSTITUENCY_CSV_PATH assert observed == ["credentials-entered", "lookup-resolved"] 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 1c2ee7fb7..a5ee58b1d 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py @@ -21,6 +21,15 @@ US_SALT_CAP = "gov.irs.deductions.itemized.salt_and_real_estate.cap.JOINT" +def _naive_policy_date(year: int, month: int, day: int) -> datetime: + """Return midnight on a date as a naive datetime. + + policyengine.py parses policy period keys with ``strptime`` and no zone, so + its ``ParameterValue`` dates are naive; an aware value never compares equal. + """ + return datetime(year, month, day) + + @pytest.fixture(scope="module") def uk(): country = _country_module("uk") @@ -59,7 +68,7 @@ def test_uk_interval_policy_preserves_inclusive_stop_date(uk, stop): assert value.parameter.name == UK_RATE assert value.parameter.tax_benefit_model_version is uk.model assert value.parameter.data_type is float - assert value.start_date == datetime(2027, 4, 6) + assert value.start_date == _naive_policy_date(2027, 4, 6) assert value.end_date == datetime.strptime(stop, "%Y-%m-%d") assert value.value == 0.21 assert reform_dict_from_parameter_values(policy.parameter_values) == raw @@ -83,9 +92,9 @@ def test_mixed_effective_dates_and_intervals_are_chronological(uk, reverse): (value.start_date, value.end_date, value.value) for value in policy.parameter_values ] == [ - (datetime(2026, 4, 6), None, 0.20), - (datetime(2027, 4, 6), datetime(2028, 4, 5), 0.21), - (datetime(2029, 4, 6), None, 0.22), + (_naive_policy_date(2026, 4, 6), None, 0.20), + (_naive_policy_date(2027, 4, 6), _naive_policy_date(2028, 4, 5), 0.21), + (_naive_policy_date(2029, 4, 6), None, 0.22), ] assert reform_dict_from_parameter_values(policy.parameter_values) == { UK_RATE: { @@ -165,7 +174,7 @@ def test_scalar_in_interval_policy_uses_request_year(uk, period, year): for value in simulation.policy.parameter_values if value.parameter.name == UK_ALLOWANCE ) - assert scalar.start_date == datetime(year, 1, 1) + assert scalar.start_date == _naive_policy_date(year, 1, 1) assert scalar.end_date is None assert scalar.value == 13000 assert scalar.parameter.data_type is int @@ -188,8 +197,8 @@ def test_us_interval_policy_uses_same_public_parameter_values(): assert value.parameter.name == US_SALT_CAP assert value.parameter.tax_benefit_model_version is us.model assert value.parameter.data_type is int - assert value.start_date == datetime(2027, 1, 1) - assert value.end_date == datetime(2027, 12, 31) + assert value.start_date == _naive_policy_date(2027, 1, 1) + assert value.end_date == _naive_policy_date(2027, 12, 31) assert value.value == 50000 assert reform_dict_from_parameter_values(policy.parameter_values) == raw @@ -237,8 +246,12 @@ def recording_compiler(reform, *, year, model_version): @pytest.mark.parametrize( "key, start, stop", [ - ("2027-4-6", datetime(2027, 4, 6), None), - ("2027-4-6.2028-4-5", datetime(2027, 4, 6), datetime(2028, 4, 5)), + ("2027-4-6", _naive_policy_date(2027, 4, 6), None), + ( + "2027-4-6.2028-4-5", + _naive_policy_date(2027, 4, 6), + _naive_policy_date(2028, 4, 5), + ), ], ) def test_unpadded_dates_are_accepted_as_the_wrapper_always_accepted_them( 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 d7f942bf1..e4d042b1f 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py @@ -186,7 +186,7 @@ def enforce_production_auth_guard(runtime: ObservabilityRuntime) -> None: "ack_value_present": True, }, ) - except Exception: # noqa: BLE001 # pragma: no cover - observability must never block startup + except Exception: # noqa: BLE001, S110 # pragma: no cover - observability must never block startup pass @@ -264,8 +264,8 @@ def require_auth( try: decoder = _get_decoder() - except RuntimeError as exc: - logger.error("Gateway auth misconfigured: %s", exc, exc_info=True) + except RuntimeError: + logger.exception("Gateway auth misconfigured") raise HTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Gateway authentication is not configured.", diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py index 6b53b8312..647afeb65 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py @@ -69,7 +69,11 @@ def smoke_import_gateway() -> dict: def main() -> None: """Run the in-image smoke remotely; fail unless modules and /health loaded.""" report = smoke_import_gateway.remote() - print(report) - assert report["modules_imported"] > 0 - assert any("/health" in route for route in report["routes"]) - print("gateway image smoke OK") + print(report) # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report + if report["modules_imported"] <= 0: + msg = f"Gateway image smoke imported no modules: {report!r}" + raise RuntimeError(msg) + if not any("/health" in route for route in report["routes"]): + msg = f"Gateway image smoke found no /health route: {report!r}" + raise RuntimeError(msg) + print("gateway image smoke OK") # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report diff --git a/ruff.toml b/ruff.toml index 1aa85b582..5afddf284 100644 --- a/ruff.toml +++ b/ruff.toml @@ -19,15 +19,41 @@ select = [ "D", # Group 5: exceptions and messages. "BLE", "EM", "TRY", + # Group 6: security, logging and debugging leftovers. + "DTZ", "ERA", "EXE", "FIX", "G", "LOG", "S", "T20", "TD", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). ignore = ["D203", "D213"] [lint.per-file-ignores] +# File-level exceptions are allowed only for tests, CI/CD scripts and +# command-line scripts; the rules guard rejects any other pattern. See +# docs/engineering/skills/rules-registry.md#file-level-exceptions. +# # Tests may use pytest's bare assert, and need no docstrings, annotations or -# named constants for magic numbers. Every other rule applies to tests. -"**/tests/**" = ["ANN", "D1", "PLR2004", "S101"] +# named constants for magic numbers. They also run repository scripts and use +# fake credentials, temporary paths, naive datetimes, pickle and exec on +# purpose. Every other rule applies to tests. +"**/tests/**" = [ + "ANN", "D1", "DTZ001", "DTZ007", "PLR2004", + "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607", +] +# The deployed-service test suite (pytest, run against a live candidate) gets +# the same test exemptions. +"**/authenticated_tests/**" = [ + "ANN", "D1", "DTZ001", "DTZ007", "PLR2004", + "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607", +] +# CI/CD scripts report on stdout. +".github/**" = ["T201"] +# Command-line scripts report on stdout, run other programs and use today's +# date. +"**/scripts/**" = ["DTZ011", "S603", "S607", "T201"] +# Programs under src: the Modal deployment utilities, and the OpenAPI and +# Stage 12 infrastructure commands. They report on stdout, and the release +# sentinel opens URLs from its arguments. +"projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py" = ["S310", "T201"] [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 17cc3f656..99fa344c5 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -733,7 +733,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["ANN", "BLE", "D", "E4", "E7", "E9", "EM", "F", "FLY", "FURB", "I", "PIE", "PYI", "RET", "RSE", "TC", "TRY", "UP", "W"] +codes = ["ANN", "BLE", "D", "DTZ", "E4", "E7", "E9", "EM", "ERA", "EXE", "F", "FIX", "FLY", "FURB", "G", "I", "LOG", "PIE", "PYI", "RET", "RSE", "S", "T20", "TC", "TD", "TRY", "UP", "W"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -742,8 +742,8 @@ invocation = "ruff check ." [[rule]] id = "sim.quality.tests-ruff-exemptions" category = "quality" -statement = "Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests." -rationale = "docs/engineering/skills/rules-registry.md#baselines" +statement = "Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also run repository scripts (S603, S607) and use fake credentials (S105, S106), temporary paths (S108), naive datetimes (DTZ001, DTZ007), pickle (S301) and exec (S102). Every other ruff rule applies to tests." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = ["projects/*/tests/**/*.py", "libs/*/tests/**/*.py", "scripts/rules/tests/**/*.py"] tier = "mechanical" status = "enforced" @@ -752,7 +752,83 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = "**/tests/**" -codes = ["ANN", "D1", "PLR2004", "S101"] +codes = ["ANN", "D1", "DTZ001", "DTZ007", "PLR2004", "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "sim.quality.deployed-tests-ruff-exemptions" +category = "quality" +statement = "The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (sim.quality.tests-ruff-exemptions); every other ruff rule applies to it." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = ["projects/*/authenticated_tests/**/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "**/authenticated_tests/**" +codes = ["ANN", "D1", "DTZ001", "DTZ007", "PLR2004", "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "sim.quality.ci-cd-scripts-ruff-exemptions" +category = "quality" +statement = "CI/CD scripts under .github may print their output (T201); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = [".github/scripts/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = ".github/**" +codes = ["T201"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "sim.quality.command-line-scripts-ruff-exemptions" +category = "quality" +statement = "Command-line scripts under a scripts directory may print their output (T201), run other programs (S603, S607) and use today's date (DTZ011); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = ["scripts/**/*.py", "projects/*/scripts/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "**/scripts/**" +codes = ["DTZ011", "S603", "S607", "T201"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "sim.quality.src-programs-ruff-exemptions" +category = "quality" +statement = "Programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands) may print their output (T201) and open URLs from their arguments (S310); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = ["projects/policyengine-simulation-executor/src/modal/utils/*.py", "projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py", "projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py", "projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py" +codes = ["S310", "T201"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 0a0edecd2..571b63ac2 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -149,7 +149,9 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio A check that could not run is skipped rather than reported. """ config = registry.ci_scripts - assert config is not None + if config is None: + msg = "the registry has no [ci_scripts] table" + raise TypeError(msg) checks = ( ( "multiline", @@ -197,7 +199,9 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio def write_baselines(registry: Registry, findings: Findings, root: Path) -> None: """Record today's findings as the baselines (used once, when adopting the rule).""" config = registry.ci_scripts - assert config is not None + if config is None: + msg = "the registry has no [ci_scripts] table" + raise TypeError(msg) for found, path in ( (findings.multiline, config.multiline_baseline), (findings.untested, config.untested_baseline), diff --git a/scripts/rules/cli.py b/scripts/rules/cli.py index a4134cacb..78ebf6149 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -14,12 +14,17 @@ from scripts.rules.validate import validate +def _report(message: str) -> None: + """Print one line of the command's output.""" + print(message) + + def _print(violations: list, notes: list[str] | None = None) -> int: for note in notes or []: - print(f"note: {note}") + _report(f"note: {note}") for violation in violations: - print(f" - {violation}") - print("rules: failed" if violations else "rules: passed") + _report(f" - {violation}") + _report("rules: failed" if violations else "rules: passed") return 1 if violations else 0 @@ -55,7 +60,7 @@ def main(argv: list[str] | None = None) -> int: ci_scripts.write_baselines( registry, ci_scripts.find(registry.ci_scripts, root), root ) - print("wrote ci_scripts baselines; update the rules' baseline counts") + _report("wrote ci_scripts baselines; update the rules' baseline counts") return 0 violations, notes = ci_scripts.run_check(root) return _print(violations, notes) @@ -71,9 +76,9 @@ def main(argv: list[str] | None = None) -> int: violations += validate(registry, root, date.today()) if args.command == "render": if violations: - print("render refused: fix the registry first") + _report("render refused: fix the registry first") return _print(violations) for path in write_outputs(registry, root): - print(f"wrote {path.relative_to(root)}") + _report(f"wrote {path.relative_to(root)}") return 0 return _print(violations) diff --git a/scripts/rules/file_exceptions.py b/scripts/rules/file_exceptions.py new file mode 100644 index 000000000..2bca805b4 --- /dev/null +++ b/scripts/rules/file_exceptions.py @@ -0,0 +1,160 @@ +"""Where file-level lint exceptions are allowed. + +A ruff ``per-file-ignores`` entry switches rules off for whole files, so it is +allowed only for files where that is the norm rather than an exception to it: + +- **tests**: files under a ``tests`` or ``authenticated_tests`` directory; +- **CI/CD scripts**: files under ``.github``; +- **command-line scripts**: files under a ``scripts`` directory, or Python + modules that run as a program through ``if __name__ == "__main__":``. + +Everywhere else a kept finding gets an inline suppression with its reason. See +docs/engineering/skills/rules-registry.md#file-level-exceptions. +""" + +from __future__ import annotations + +import ast +import os +import re +from functools import cache +from pathlib import Path + +TEST_DIRECTORIES = frozenset({"tests", "authenticated_tests"}) +CI_DIRECTORY = ".github" +SCRIPT_DIRECTORY = "scripts" +LINTED_SUFFIXES = (".py", ".pyi") +# Directories that hold no source of this repository. +SKIPPED_DIRECTORIES = frozenset( + { + ".git", + ".venv", + "venv", + "node_modules", + "__pycache__", + ".mypy_cache", + ".ruff_cache", + } +) + + +# One piece of a ruff (globset) pattern: a wildcard, a character class, an +# alternation character, or a run of literal characters. +GLOB_PIECE = re.compile(r"\*\*/|\*\*|\*|\?|\[!?[^\]]*\]|[{},\[]|[^*?\[{},]+") +# As in ruff, "*" and "?" also match "/". +WILDCARDS = {"**/": "(?:.*/)?", "**": ".*", "*": ".*", "?": "."} + + +def _character_class(piece: str) -> str: + if piece == "[": + return re.escape(piece) + body = piece[1:-1].replace("\\", "\\\\") + if body.startswith("!"): + body = "^" + body[1:] + return f"[{body}]" + + +def glob_regex(pattern: str) -> re.Pattern[str]: + """Translate a ruff (globset) file pattern into a regex over relative paths. + + Supports ``**``, ``*``, ``?``, ``[...]`` and ``{a,b}`` alternatives. As in + ruff, ``*`` and ``?`` also match ``/``, so ``pkg/*.py`` covers + ``pkg/sub/module.py`` too. + """ + parts = [] + open_alternations = 0 + for piece in GLOB_PIECE.findall(pattern): + if piece in WILDCARDS: + parts.append(WILDCARDS[piece]) + elif piece.startswith("["): + parts.append(_character_class(piece)) + elif piece == "{": + parts.append("(?:") + open_alternations += 1 + elif piece == "}" and open_alternations: + parts.append(")") + open_alternations -= 1 + elif piece == "," and open_alternations: + parts.append("|") + else: + parts.append(re.escape(piece)) + return re.compile("".join(parts) + "$") + + +@cache +def linted_files(root: Path) -> tuple[str, ...]: + """Return the repository's Python files as sorted root-relative POSIX paths.""" + found = [] + for directory, subdirectories, files in os.walk(root): + subdirectories[:] = [ + name + for name in subdirectories + if name not in SKIPPED_DIRECTORIES + and (not name.startswith(".") or name == CI_DIRECTORY) + ] + found.extend( + Path(directory, name).relative_to(root).as_posix() + for name in files + if name.endswith(LINTED_SUFFIXES) + ) + return tuple(sorted(found)) + + +def matching_files(root: Path, pattern: str) -> list[str]: + """Return the files a ruff ``per-file-ignores`` pattern applies to. + + Like ruff, a pattern matches either a file's path or its name. + """ + regex = glob_regex(pattern) + return [ + path + for path in linted_files(root) + if regex.match(path) or regex.match(path.rsplit("/", 1)[-1]) + ] + + +def runs_as_program(root: Path, path: str) -> bool: + """Return whether a module has a top-level ``if __name__ == "__main__":``.""" + try: + tree = ast.parse((root / path).read_text(encoding="utf-8")) + except (OSError, SyntaxError, UnicodeDecodeError): + return False + for node in tree.body: + if ( + isinstance(node, ast.If) + and isinstance(node.test, ast.Compare) + and isinstance(node.test.left, ast.Name) + and node.test.left.id == "__name__" + and any( + isinstance(comparator, ast.Constant) and comparator.value == "__main__" + for comparator in node.test.comparators + ) + ): + return True + return False + + +def allows_file_level_exceptions(root: Path, path: str) -> bool: + """Return whether a file is a test, a CI/CD script or a command-line script.""" + directories = path.split("/")[:-1] + return ( + any(directory in TEST_DIRECTORIES for directory in directories) + or (bool(directories) and directories[0] == CI_DIRECTORY) + or SCRIPT_DIRECTORY in directories + or runs_as_program(root, path) + ) + + +def out_of_scope_files(root: Path, pattern: str) -> list[str] | None: + """Return the files a pattern covers that may not take file-level exceptions. + + ``None`` means the pattern can't be checked: it is negated, so it covers + every file it doesn't name. + """ + if pattern.startswith("!"): + return None + return [ + path + for path in matching_files(root, pattern) + if not allows_file_level_exceptions(root, path) + ] diff --git a/scripts/rules/resolve.py b/scripts/rules/resolve.py index ed2bba559..d00d15e91 100644 --- a/scripts/rules/resolve.py +++ b/scripts/rules/resolve.py @@ -13,6 +13,11 @@ from functools import cache from typing import TYPE_CHECKING, Any +from scripts.rules.file_exceptions import ( + linted_files, + matching_files, + out_of_scope_files, +) from scripts.rules.model import CiRef, Kind, Rule, Violation from scripts.rules.workflows import JobBlock, parse_jobs @@ -23,6 +28,8 @@ DEFAULT_RUNNER = "scripts/run_quality_guards.py" RUFF_DEFAULT_SELECT = ("E4", "E7", "E9", "F") PROOF_CI_FIELDS = ("workflow", "job", "invocation") +# How many offending files a file-level exception violation names. +SHOWN_OUT_OF_SCOPE_FILES = 3 # --- shared readers ------------------------------------------------------- @@ -38,6 +45,7 @@ def clear_caches() -> None: _read.cache_clear() workflow_jobs.cache_clear() test_names.cache_clear() + linted_files.cache_clear() @cache @@ -264,6 +272,45 @@ def _resolve_ruff(rule: Rule, root: Path) -> list[Violation]: "match enforcer.pattern/codes to the ruff config", ) ] + return _file_level_exception_scope(rule, root, pattern) + return [] + + +def _file_level_exception_scope( + rule: Rule, root: Path, pattern: str +) -> list[Violation]: + """Check a per-file ignore covers only tests, CI/CD and command-line scripts.""" + out_of_scope = out_of_scope_files(root, pattern) + if out_of_scope is None: + return [ + _v( + rule, + f"ruff per-file-ignores pattern {pattern!r} is negated", + "name the files the exception covers", + ) + ] + if not matching_files(root, pattern): + return [ + _v( + rule, + f"ruff per-file-ignores pattern {pattern!r} matches no files", + "fix the pattern, or remove the exception", + ) + ] + if out_of_scope: + shown = ", ".join(out_of_scope[:SHOWN_OUT_OF_SCOPE_FILES]) + hidden = len(out_of_scope) - SHOWN_OUT_OF_SCOPE_FILES + more = f" and {hidden} more" if hidden > 0 else "" + return [ + _v( + rule, + f"ruff per-file-ignores {pattern!r} covers files that may not take " + f"file-level exceptions: {shown}{more}", + "file-level exceptions are only for tests, CI/CD scripts and " + "command-line scripts; narrow the pattern and suppress other " + "findings inline with a reason", + ) + ] return [] diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 400a68b7e..00151ed23 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -38,6 +38,34 @@ def with_rule(extra: str) -> dict[str, str]: return {"rules/registry.toml": REGISTRY + extra} +def per_file_ignore_rule(name: str, pattern: str) -> str: + """Return a registry rule describing a ruff per-file ignore of E501.""" + return f""" +[[rule]] +id = "ex.quality.{name}-ignores" +category = "quality" +statement = "Long lines are allowed here." +rationale = "docs/guide.md" +scope = ["rules/registry.toml"] +tier = "mechanical" +status = "advisory" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "{pattern}" +codes = ["E501"] +""" + + +def per_file_ignore_config(pattern: str) -> str: + """Return a pyproject.toml whose ruff config ignores E501 for ``pattern``.""" + return ( + '[tool.mypy]\nfiles = ["pkg"]\n\n' + f'[tool.ruff.lint.per-file-ignores]\n"{pattern}" = ["E501"]\n' + ) + + class ValidRepositoryTest(unittest.TestCase): def test_valid_repository_passes(self) -> None: with Repo() as repo: @@ -442,6 +470,66 @@ def test_unregistered_ruff_ignore(self) -> None: ) self.assertViolation(repo, "ruff per-file-ignores 'pkg/*.py' has no rule") + def test_file_level_exceptions_only_for_tests_ci_and_scripts(self) -> None: + program = 'def main():\n pass\n\n\nif __name__ == "__main__":\n main()\n' + allowed = { + "tests/**": "tests", + ".github/**": "ci", + "scripts/**": "scripts", + "pkg/cli.py": "cli", + } + for pattern, name in allowed.items(): + with ( + self.subTest(pattern=pattern), + Repo( + { + "pkg/cli.py": program, + ".github/scripts/release.py": "print(1)\n", + **with_rule(per_file_ignore_rule(name, pattern)), + } + ) as repo, + ): + repo.write("pyproject.toml", per_file_ignore_config(pattern)) + self.assertEqual([m for m in repo.messages() if "per-file" in m], []) + + def test_file_level_exception_outside_allowed_files_is_rejected(self) -> None: + with Repo(with_rule(per_file_ignore_rule("routes", "pkg/*.py"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("pkg/*.py")) + self.assertViolation( + repo, + "covers files that may not take file-level exceptions: pkg/routes.py", + ) + + def test_file_level_exception_needs_a_program_outside_scripts(self) -> None: + # A module with a main() that is never run as a program isn't a + # command-line script. + with Repo( + { + "pkg/cli.py": "def main():\n pass\n", + **with_rule(per_file_ignore_rule("cli", "pkg/cli.py")), + } + ) as repo: + repo.write("pyproject.toml", per_file_ignore_config("pkg/cli.py")) + self.assertViolation(repo, "may not take file-level exceptions: pkg/cli.py") + + def test_file_level_exception_star_crosses_directories(self) -> None: + # Ruff's * also matches "/", so "*/routes.py" reaches pkg/routes.py. + with Repo(with_rule(per_file_ignore_rule("star", "*/routes.py"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("*/routes.py")) + self.assertViolation( + repo, "may not take file-level exceptions: pkg/routes.py" + ) + + def test_negated_file_level_exception_is_rejected(self) -> None: + with Repo(with_rule(per_file_ignore_rule("negated", "!tests/**"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("!tests/**")) + self.assertViolation(repo, "pattern '!tests/**' is negated") + + def test_file_level_exception_must_match_a_file(self) -> None: + with Repo(with_rule(per_file_ignore_rule("stale", "nowhere/**"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("nowhere/**")) + self.assertViolation(repo, "pattern 'nowhere/**' matches no files") + def test_unregistered_ci_job(self) -> None: with Repo() as repo: text = repo.read(".github/workflows/pr.yml")