Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion .coveragerc
Original file line number Diff line number Diff line change
@@ -1,9 +1,27 @@
[run]
source = policyengine_api
# Record file paths relative to the checkout, so a report written in one
# checkout directory, such as a container job's, matches the same files in
# another; with absolute paths diff-cover finds no changed file in it and
# passes (api.quality.diff-coverage).
relative_files = True

[report]
# Lines that never run by design, which would otherwise count as missed and
# fail a fully tested change (api.quality.diff-coverage). coverage.py searches
# each line of text for each pattern, and a matching line that opens a block
# excludes the whole block, so the last two match only the whole statement.
# - Setting exclude_lines replaces coverage.py's default, so the first
# pattern is that default again, with `\#` because a value that starts with
# `#` is read as a comment.
# - Imports under `if TYPE_CHECKING:`, where ruff's TC rules move them.
# - `...` alone on a line: the bodies of Protocol methods and other stubs.
exclude_lines =
\#\s*(pragma|PRAGMA)[:\s]?\s*(no|NO)\s*(cover|COVER)
^\s*if TYPE_CHECKING:\s*(\#.*)?$
^\s*\.\.\.\s*(\#.*)?$
omit =
tests/*
*/tests/*
tests/conftest.py

48 changes: 48 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jobs:
- check-test-shards
- upload-test-coverage
- upload-integration-coverage
- diff-coverage
runs-on: ubuntu-latest
permissions:
contents: read
Expand Down Expand Up @@ -385,6 +386,53 @@ jobs:
# codecov.yml defines the flag and carries it forward.
flags: integration

diff-coverage:
# Rule api.quality.diff-coverage: at least 80% of the lines a pull request
# changes in policyengine_api (.coveragerc's source) that coverage.py
# measures run in the unit or integration tests. diff-cover reads every
# test shard's report and the integration tests' report, and counts a line
# as covered if any of them covers it. Its log lists the uncovered lines.
name: Check the coverage of the changed lines
runs-on: ubuntu-latest
needs:
- test
- v2-integration-check
timeout-minutes: 15
permissions:
contents: read
env:
# uv installs uv.lock on the Cloud Run image's Python, not the 3.14
# that .python-version selects.
UV_PYTHON: "3.12"
steps:
- name: Checkout repo
uses: actions/checkout@v4
with:
# diff-cover compares with the merge commit's first parent,
# which a depth-1 clone lacks.
fetch-depth: 0
- name: Set up uv
uses: astral-sh/setup-uv@v6
- name: Download every test shard's coverage report
uses: actions/download-artifact@v4
with:
pattern: test-coverage-shard-*
path: ${{ runner.temp }}/coverage
- name: Download the integration coverage report
uses: actions/download-artifact@v4
with:
name: integration-coverage
path: ${{ runner.temp }}/coverage/integration-coverage
# The checkout is the merge commit GitHub builds for the pull request,
# so HEAD^1 is the commit that merge is built on: the base branch, or
# for a pull request stacked on another, the lower one's merge commit
# (this job is skipped there). Either way the diff holds only the pull
# request's changes. The event's base.sha can be older: it often stays
# where the branch left master, and comparing with it would count
# master's later changes too.
- name: Check the coverage of the changed lines
run: uv run --frozen --extra dev diff-cover "$RUNNER_TEMP"/coverage/*/coverage*.xml --compare-branch=HEAD^1 --fail-under=80

check-test-shards:
# Fails unless every shard collected the same tests and each test ran in
# exactly one shard (scripts/release_test_shards.py check-coverage). It
Expand Down
10 changes: 6 additions & 4 deletions .github/workflows/v2-integration-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,12 @@ jobs:
- name: Export v2 integration coverage
run: uv run --frozen --extra dev coverage xml -i -o coverage-v2.xml
# The calling workflow's upload-integration-coverage job sends the report
# to Codecov, flagged integration. Nothing that waits for these tests,
# such as push.yml's versioning, then waits for Codecov.
# A failure to keep it fails only the caller's upload job, which then
# finds no report, not these tests, which push.yml's release waits for.
# to Codecov, flagged integration, and pr.yml's diff-coverage job reads
# it. Nothing that waits for these tests, such as push.yml's versioning,
# then waits for Codecov. A failure to keep it fails the jobs that read
# it, which then find no report (in pr.yml, diff-coverage's failure
# fails CI complete), not these tests, which push.yml's release waits
# for.
- name: Keep the integration coverage report
continue-on-error: true
uses: actions/upload-artifact@v4
Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`.

### Code quality and types

- Checked: `api.quality.alembic-revisions-unused-imports`, `api.quality.ci-cd-scripts-ruff-exemptions`, `api.quality.command-line-modules-ruff-exemptions`, `api.quality.command-line-scripts-ruff-exemptions`, `api.quality.coverage-measured`, `api.quality.environment-variables-load`, `api.quality.file-length`, `api.quality.mypy-v2`, `api.quality.ruff-format`, `api.quality.ruff-lint`, `api.quality.runtime-evaluated-models-resolve`, `api.quality.suppressions-explained`, `api.quality.tests-assert`, `api.quality.tests-pass`, `api.quality.tests-ruff-exemptions`, `api.quality.unittest-ruff-exemptions`
- Checked: `api.quality.alembic-revisions-unused-imports`, `api.quality.ci-cd-scripts-ruff-exemptions`, `api.quality.command-line-modules-ruff-exemptions`, `api.quality.command-line-scripts-ruff-exemptions`, `api.quality.coverage-measured`, `api.quality.diff-coverage`, `api.quality.environment-variables-load`, `api.quality.file-length`, `api.quality.mypy-v2`, `api.quality.ruff-format`, `api.quality.ruff-lint`, `api.quality.runtime-evaluated-models-resolve`, `api.quality.suppressions-explained`, `api.quality.tests-assert`, `api.quality.tests-pass`, `api.quality.tests-ruff-exemptions`, `api.quality.unittest-ruff-exemptions`

### Observability

Expand Down
1 change: 1 addition & 0 deletions changelog.d/4199.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Pull requests must now cover at least 80% of the lines they change in `policyengine_api/` with the unit and integration tests together, checked by a new diff-cover job that `CI complete` needs.
10 changes: 7 additions & 3 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,13 @@ coverage:
threshold: 0.5%
patch:
default:
target: auto
# Reported but not blocking until the minimum for changed lines is
# decided and enforced.
# pr.yml's diff-coverage job enforces this minimum for the changed
# lines (api.quality.diff-coverage); Codecov only shows a figure. It
# can differ from the job's: Codecov compares with the master commit
# the branch left from or last merged in, not the merge commit's
# first parent, counts a line with a branch that never ran as not
# covered, and includes flags carried forward from earlier commits.
target: 80%
informational: true

flag_management:
Expand Down
56 changes: 56 additions & 0 deletions docs/engineering/skills/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,62 @@ otherwise.
The project status allows a 0.5% drop. The patch status reports coverage of
the changed lines without blocking a merge.

### Changed lines

`api.quality.diff-coverage`: at least 80% of the lines a pull request changes
in `policyengine_api/` that coverage.py measures run in the unit or
integration tests. The `diff-coverage` job in `pr.yml` runs after the `Test`
shards and the integration tests. It downloads every shard's `coverage.xml`
and the integration tests' `coverage-v2.xml`, and runs
`diff-cover --fail-under=80` against `HEAD^1`. CI checks out the merge commit
GitHub builds for the pull request, so `HEAD^1` is the commit that merge is
built on: the base branch, or for a pull request stacked on another, the
lower one's merge commit (the job runs only on the lowest pull request of a
stack). Either way the diff holds only the pull request's changes. The
event's `base.sha` isn't used: it can stay where the branch left `master`,
which would count `master`'s later changes as the pull request's. A line
counts as covered if any report covers it. `CI complete` needs the job, so a
pull request below the minimum can't merge. The job's log lists each changed
file's uncovered lines.

Only measured files count, so a pull request that changes no code under
`policyengine_api/` passes. coverage.py measures every module there, whether
or not a test imports it, but only inside a package: every directory under
`policyengine_api/` needs an `__init__.py`, or its untested modules are left
out and their changed lines pass.
`tests/unit/test_diff_coverage.py::test_coverage_reports_every_module_no_test_imports`
fails for a directory without one. Lines that never run by design don't
count (`.coveragerc`): imports under `if TYPE_CHECKING:`, `...` alone on a
line (the bodies of Protocol methods and other stubs), and lines marked
`# pragma: no cover`.

What counts as a line is coverage.py's: the first line of each statement.
Changing the second or a later line of a statement that spans several lines
adds nothing to the count, and a line that ran counts as covered even if one
of its branches never ran. Only the `Test` shards and the tests in
`v2-integration-check.yml` are measured: code that runs only in the Alembic
checks, the environment-variable tests, `push.yml`'s live tests or a
subprocess a test starts counts as uncovered. Moving code into a new file
counts its lines as changed, so splitting a module that tests cover poorly
needs tests for the moved code. git's rename detection is the exception: a
new file at least 50% similar to a file the pull request deletes is paired
with it, and only its edited lines count.

Codecov's patch status shows an 80% target too, but doesn't block a merge,
and its figure can differ: it compares with the `master` commit the branch
left from or last merged in, and counts a line with a branch that never ran
as not covered.

To check before pushing, delete `.coverage` (`make test` adds to it), run
`make test`, which writes `coverage.xml`, and then:

```bash
uv run --frozen --extra dev --python 3.12 diff-cover coverage.xml --compare-branch=origin/master --fail-under=80
```

That leaves out the integration tests, which need the databases
`v2-integration-check.yml` starts, so CI's figure can be higher.

## Tests That Run a Country Model

A new unit test stands in for the country model: a calculation with the
Expand Down
23 changes: 17 additions & 6 deletions docs/generated/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce
| Tier | enforced | ratcheted | advisory | doc-only | Total |
| --- | ---: | ---: | ---: | ---: | ---: |
| mechanical | 69 | 7 | 1 | 8 | 85 |
| metric | 1 | 0 | 0 | 0 | 1 |
| metric | 2 | 0 | 0 | 0 | 2 |
| judgment | 0 | 0 | 0 | 1 | 1 |

**Blocking:** 77 of 87 rules.
**Blocking:** 78 of 88 rules.

## Gaps

Expand Down Expand Up @@ -372,6 +372,7 @@ flowchart LR
| `api.quality.command-line-modules-ruff-exemptions` | Application modules that also run as command-line programs (if __name__ == "__main__") may print their output (T201); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of T201 for `policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/privilege_reconciliation,data/v2/user_association_migration_qualification,worker_spm_release}.py` | `pr.yml:lint` | — |
| `api.quality.command-line-scripts-ruff-exemptions` | Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date and pick random samples (T201, S603, S607, S310, DTZ011, S311); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of DTZ011, S310, S311, S603, S607, T201 for `scripts/**` | `pr.yml:lint` | — |
| `api.quality.coverage-measured` | Codecov measures the same tests on master as on pull requests: every CI job that measures test coverage sends it to Codecov with a flag that codecov.yml carries forward (unit for make test, in pr.yml and in push.yml's release tests; integration for the cross-database integration tests), passing the CODECOV_TOKEN secret, from a job with a timeout of at most 15 minutes; every shard of a report is uploaded, and in a master run an upload fails the run when it fails and no other job waits for it. | mechanical | enforced | yes | pytest `tests/unit/test_codecov_coverage.py` | `pr.yml:test` | yes |
| `api.quality.diff-coverage` | The lines a pull request changes in policyengine_api that coverage.py measures (the first line of each statement) are at least 80% covered by the unit and integration tests together: pr.yml's diff-coverage job runs diff-cover --fail-under=80 on every test shard's and the integration tests' coverage reports, against the commit that the merge commit CI checks out is built on (HEAD^1: the base branch, or a lower stacked pull request's merge commit), and CI complete needs it. | metric | enforced | yes | CI job `diff-coverage` | `pr.yml:diff-coverage` | yes |
| `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — |
| `api.quality.file-length` | A tracked Python file has at most 800 physical lines, or at most 1,500 if it is test code (a file under a tests or fixtures directory, or a conftest.py); a longer file is split by responsibility into modules, and keeps its public import path. | metric | enforced | yes | quality guard `file-length` | `pr.yml:quality-guards` | yes |
| `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, the gzip negotiation, the user sign-in and account-deletion libraries, the country list and its startup check pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — |
Expand All @@ -398,6 +399,9 @@ flowchart LR
r_api_quality_coverage_measured("api.quality.coverage-measured (enforced)")
e_pytest_tests_unit_test_codecov_coverage_py["pytest tests/unit/test_codecov_coverage.py"]
j_CI_pr_yml_test["CI pr.yml:test"]
r_api_quality_diff_coverage("api.quality.diff-coverage (enforced)")
e_CI_job_diff_coverage["CI job diff-coverage"]
j_CI_pr_yml_diff_coverage["CI pr.yml:diff-coverage"]
r_api_quality_environment_variables_load("api.quality.environment-variables-load (enforced)")
e_CI_job_test_env_vars["CI job test_env_vars"]
j_CI_pr_yml_test_env_vars["CI pr.yml:test_env_vars"]
Expand All @@ -421,10 +425,6 @@ flowchart LR
r_api_quality_tests_pass("api.quality.tests-pass (enforced)")
e_CI_job_test__check_test_shards["CI job test, check-test-shards"]
j_CI_pr_yml_check_test_shards["CI pr.yml:check-test-shards"]
r_api_quality_tests_ruff_exemptions("api.quality.tests-ruff-exemptions (enforced)")
e_ruff_per_file_ignore_of_ANN__ARG001__ARG002__ARG005__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___["ruff per-file ignore of ANN, ARG001, ARG002, ARG005, D1, DTZ001, DTZ011, INP001, N802, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for **/tests/**"]
r_api_quality_unittest_ruff_exemptions("api.quality.unittest-ruff-exemptions (enforced)")
e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___["ruff per-file ignore of PT for scripts/rules/tests/**"]
r_api_quality_alembic_revisions_unused_imports -.-> gap_unenforced
r_api_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_N999__S310__S603__S607__T201__TID253_for__github___
e_ruff_per_file_ignore_of_N999__S310__S603__S607__T201__TID253_for__github___ --> j_CI_pr_yml_lint
Expand All @@ -434,6 +434,8 @@ flowchart LR
e_ruff_per_file_ignore_of_DTZ011__S310__S311__S603__S607__T201_for_scripts___ --> j_CI_pr_yml_lint
r_api_quality_coverage_measured --> e_pytest_tests_unit_test_codecov_coverage_py
e_pytest_tests_unit_test_codecov_coverage_py --> j_CI_pr_yml_test
r_api_quality_diff_coverage --> e_CI_job_diff_coverage
e_CI_job_diff_coverage --> j_CI_pr_yml_diff_coverage
r_api_quality_environment_variables_load --> e_CI_job_test_env_vars
e_CI_job_test_env_vars --> j_CI_pr_yml_test_env_vars
r_api_quality_file_length --> e_quality_guard_file_length
Expand All @@ -455,6 +457,15 @@ flowchart LR
r_api_quality_tests_pass --> e_CI_job_test__check_test_shards
e_CI_job_test__check_test_shards --> j_CI_pr_yml_test
e_CI_job_test__check_test_shards --> j_CI_pr_yml_check_test_shards
```

```mermaid
flowchart LR
r_api_quality_tests_ruff_exemptions("api.quality.tests-ruff-exemptions (enforced)")
e_ruff_per_file_ignore_of_ANN__ARG001__ARG002__ARG005__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___["ruff per-file ignore of ANN, ARG001, ARG002, ARG005, D1, DTZ001, DTZ011, INP001, N802, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for **/tests/**"]
j_CI_pr_yml_lint["CI pr.yml:lint"]
r_api_quality_unittest_ruff_exemptions("api.quality.unittest-ruff-exemptions (enforced)")
e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___["ruff per-file ignore of PT for scripts/rules/tests/**"]
r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__ARG001__ARG002__ARG005__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___
e_ruff_per_file_ignore_of_ANN__ARG001__ARG002__ARG005__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___ --> j_CI_pr_yml_lint
r_api_quality_unittest_ruff_exemptions --> e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___
Expand Down
5 changes: 5 additions & 0 deletions policyengine_api/libs/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"""Simulation Entrypoint and Auth0 clients, and authentication for user routes.

This __init__.py makes the directory a regular package, so coverage.py reports
the modules here that no test imports (api.quality.diff-coverage).
"""
5 changes: 5 additions & 0 deletions policyengine_api/routes/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"""The v1 API's Flask blueprints: its routes and its app-wide error handlers.

This __init__.py makes the directory a regular package, so coverage.py reports
the modules here that no test imports (api.quality.diff-coverage).
"""
8 changes: 6 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ dev = [
# (api.dependencies.declared-and-used); its findings and report format can
# change between minor versions, so an upgrade is its own change.
"deptry>=0.25.1,<0.26",
# pr.yml's diff-coverage job runs it (api.quality.diff-coverage). 11.0 is
# newer than two weeks; moving to it is its own change.
"diff-cover>=10.6,<11",
"mypy>=1.15,<2",
"pytest",
"pytest-snapshot",
Expand Down Expand Up @@ -115,8 +118,9 @@ allow-direct-references = true
# Directories with no __init__.py on purpose (INP001). Ruff treats each one, and
# every directory below it, as a package when it resolves modules.
# - policyengine_api is a namespace package, which mypy reads with
# explicit_package_bases. This also covers its routes and libs directories,
# which have no __init__.py either.
# explicit_package_bases. Its subdirectories still need an __init__.py:
# coverage.py leaves out the modules no test imports in one that lacks
# it (tests/unit/test_diff_coverage.py checks this).
# - .github (with .github/scripts) and migrations/v1 and v2 hold standalone
# scripts and Alembic environments, run but never imported.
# - rules/fixtures holds rule proof fixtures, which tests read as source.
Expand Down
6 changes: 6 additions & 0 deletions rules/fixtures/api.quality.diff-coverage/at_threshold.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Proof fixture for api.quality.diff-coverage (at the minimum).
#
# As below_threshold.toml, with 800 of the 1,000 changed lines covered: 80%,
# which diff-cover --fail-under=80 must accept.
changed_lines = 1000
covered_lines = 800
Loading
Loading