From 3c7b721b6f71f3d44817793e7807f74fa5de4806 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:17:29 +0400 Subject: [PATCH 1/8] Test that pull requests' changed lines are 80% covered (failing) Tests and proof fixtures for api.quality.diff-coverage: diff-cover --fail-under=80 rejects 7 of 10 changed lines covered and accepts 8 of 10; pr.yml's diff-coverage job downloads the test shards' and integration tests' reports, runs diff-cover against the base commit, and is needed by CI complete with no condition of its own; the rule, its proof and Codecov's patch status use the same minimum; and diff-cover is a locked dev dependency. All 4 tests fail: diff-cover isn't installed and there is no such job, rule or setting yet. Co-Authored-By: Claude Opus 5.5 --- .../at_threshold.toml | 6 + .../below_threshold.toml | 8 + tests/unit/test_diff_coverage.py | 155 ++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 rules/fixtures/api.quality.diff-coverage/at_threshold.toml create mode 100644 rules/fixtures/api.quality.diff-coverage/below_threshold.toml create mode 100644 tests/unit/test_diff_coverage.py diff --git a/rules/fixtures/api.quality.diff-coverage/at_threshold.toml b/rules/fixtures/api.quality.diff-coverage/at_threshold.toml new file mode 100644 index 000000000..faa4e3506 --- /dev/null +++ b/rules/fixtures/api.quality.diff-coverage/at_threshold.toml @@ -0,0 +1,6 @@ +# Proof fixture for api.quality.diff-coverage (at the minimum). +# +# As below_threshold.toml, with 8 of the 10 changed lines covered: 80%, which +# diff-cover --fail-under=80 must accept. +changed_lines = 10 +covered_lines = 8 diff --git a/rules/fixtures/api.quality.diff-coverage/below_threshold.toml b/rules/fixtures/api.quality.diff-coverage/below_threshold.toml new file mode 100644 index 000000000..f3561ff56 --- /dev/null +++ b/rules/fixtures/api.quality.diff-coverage/below_threshold.toml @@ -0,0 +1,8 @@ +# Proof fixture for api.quality.diff-coverage (just below the minimum). +# +# The proof test commits a module, then adds `changed_lines` lines to it in a +# second commit, and writes a coverage report in which the first +# `covered_lines` of them ran. diff-cover --fail-under=80 must fail: 7 of 10 +# changed lines is 70%, the closest a 10-line change comes below 80%. +changed_lines = 10 +covered_lines = 7 diff --git a/tests/unit/test_diff_coverage.py b/tests/unit/test_diff_coverage.py new file mode 100644 index 000000000..191a94568 --- /dev/null +++ b/tests/unit/test_diff_coverage.py @@ -0,0 +1,155 @@ +"""The lines a pull request changes are covered by tests (api.quality.diff-coverage). + +pr.yml's diff-coverage job downloads the six test shards' coverage.xml files +and the cross-database integration tests' report, and runs diff-cover against +the pull request's base commit. diff-cover fails when less than the minimum of +the changed lines in measured files (policyengine_api, .coveragerc's source) +ran, counting a line as covered if any report covers it. CI complete needs the +job, so a failure blocks the merge. +""" + +from __future__ import annotations + +import subprocess +import sys +import tomllib +from pathlib import Path + +import yaml + +REPO = Path(__file__).resolve().parents[2] +RULE_ID = "api.quality.diff-coverage" +FIXTURES = REPO / "rules" / "fixtures" / RULE_ID +PR_WORKFLOW = REPO / ".github" / "workflows" / "pr.yml" +JOB = "diff-coverage" +MINIMUM_PERCENT = 80 +DIFF_COVER = Path(sys.executable).parent / "diff-cover" +GIT = ["git", "-c", "user.name=test", "-c", "user.email=test@example.com"] + + +def _git(repo: Path, *arguments: str) -> None: + subprocess.run( + [*GIT, "-c", "commit.gpgsign=false", *arguments], + cwd=repo, + check=True, + capture_output=True, + ) + + +def _coverage_report(source: Path, hits: list[int]) -> str: + lines = "".join( + f'' + for number, count in enumerate(hits, start=1) + ) + return ( + '' + f"{source}" + '' + f"{lines}" + ) + + +def _run_diff_cover(fixture: Path, root: Path) -> subprocess.CompletedProcess[str]: + """Commit a module, change it as ``fixture`` says, and run diff-cover on it.""" + spec = tomllib.loads(fixture.read_text(encoding="utf-8")) + changed, covered = spec["changed_lines"], spec["covered_lines"] + package = root / "pkg" + package.mkdir(parents=True) + module = package / "module.py" + module.write_text("VALUE = 0\n") + _git(root, "init", "--quiet") + _git(root, "add", ".") + _git(root, "commit", "--quiet", "-m", "base") + module.write_text( + "VALUE = 0\n" + + "".join(f"value_{index} = {index}\n" for index in range(changed)) + ) + _git(root, "commit", "--quiet", "-am", "change") + hits = [1] + [1] * covered + [0] * (changed - covered) + (root / "coverage.xml").write_text(_coverage_report(package, hits)) + return subprocess.run( + [ + str(DIFF_COVER), + "coverage.xml", + "--compare-branch=HEAD~1", + f"--fail-under={MINIMUM_PERCENT}", + ], + cwd=root, + capture_output=True, + text=True, + check=False, + ) + + +def _job() -> dict: + return yaml.safe_load(PR_WORKFLOW.read_text(encoding="utf-8"))["jobs"][JOB] + + +def _step(job: dict, name: str) -> dict: + (step,) = [step for step in job["steps"] if step.get("name") == name] + return step + + +def test_rule_proof_diff_coverage(tmp_path: Path): + below = _run_diff_cover(FIXTURES / "below_threshold.toml", tmp_path / "below") + at_minimum = _run_diff_cover(FIXTURES / "at_threshold.toml", tmp_path / "at") + + assert below.returncode != 0, below.stdout + assert "Coverage: 70%" in below.stdout + assert at_minimum.returncode == 0, at_minimum.stdout + assert "Coverage: 80%" in at_minimum.stdout + + +def test_pull_requests_check_the_coverage_of_their_changed_lines(): + workflow = yaml.safe_load(PR_WORKFLOW.read_text(encoding="utf-8")) + job = workflow["jobs"][JOB] + checkout = _step(job, "Checkout repo") + shards = _step(job, "Download every test shard's coverage report") + integration = _step(job, "Download the integration coverage report") + check = _step(job, "Check the coverage of the changed lines") + + # It runs after both kinds of tests, with no condition of its own: on a + # stacked pull request above the lowest one they are skipped, so it is + # skipped too, and CI complete fails there as it already does. + assert sorted(job["needs"]) == ["test", "v2-integration-check"] + assert "if" not in job + assert JOB in workflow["jobs"]["ci-complete"]["needs"] + assert 0 < job["timeout-minutes"] <= 15 + assert job["env"] == {"UV_PYTHON": "3.12"} + # diff-cover compares with the base commit, which a shallow clone lacks. + assert checkout["with"]["fetch-depth"] == 0 + assert shards["with"]["pattern"] == "test-coverage-shard-*" + assert integration["with"]["name"] == "integration-coverage" + reports = "${{ runner.temp }}/coverage" + assert shards["with"]["path"] == reports + assert integration["with"]["path"] == f"{reports}/integration-coverage" + assert check["env"] == {"BASE_SHA": "${{ github.event.pull_request.base.sha }}"} + assert check["run"] == ( + "uv run --frozen --extra dev diff-cover " + '"$RUNNER_TEMP"/coverage/*/coverage*.xml ' + f'--compare-branch="$BASE_SHA" --fail-under={MINIMUM_PERCENT}' + ) + + +def test_the_minimum_is_the_same_in_the_rule_and_codecov(): + registry = tomllib.loads((REPO / "rules" / "registry.toml").read_text()) + (rule,) = [rule for rule in registry["rule"] if rule["id"] == RULE_ID] + codecov = yaml.safe_load((REPO / "codecov.yml").read_text(encoding="utf-8")) + patch = codecov["coverage"]["status"]["patch"]["default"] + + assert f"at least {MINIMUM_PERCENT}%" in rule["statement"] + assert rule["proof"]["threshold"] == MINIMUM_PERCENT + # Codecov's patch status shows the same minimum, without blocking. + assert patch["target"] == f"{MINIMUM_PERCENT}%" + assert patch["informational"] is True + + +def test_diff_cover_is_a_locked_development_dependency(): + project = tomllib.loads((REPO / "pyproject.toml").read_text()) + lock = tomllib.loads((REPO / "uv.lock").read_text()) + + assert any( + requirement.startswith("diff-cover") + for requirement in project["project"]["optional-dependencies"]["dev"] + ) + assert "diff-cover" in {package["name"] for package in lock["package"]} From 2f75f034a50d99f55dca02b4db5edefb8399fedf Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:19:15 +0400 Subject: [PATCH 2/8] Require 80% coverage on the lines a pull request changes - A new diff-coverage job in pr.yml runs after the test shards and the integration tests, downloads every shard's coverage.xml and the integration tests' coverage-v2.xml, and runs diff-cover --fail-under=80 against the pull request's base commit. CI complete needs it, so a pull request below the minimum can't merge; with no condition of its own, it is skipped where the tests are. - diff-cover 10.6.0 is in the dev extra and locked (11.0 is less than two weeks old). - New enforced metric rule api.quality.diff-coverage, with a proof at 70% and 80%; Codecov's patch status shows the same 80% target, still informational; testing.md has a "Changed lines" section. The 4 new tests pass. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr.yml | 42 ++++++++++++++++++++ AGENTS.md | 2 +- changelog.d/4199.added.md | 1 + codecov.yml | 6 +-- docs/engineering/skills/testing.md | 24 +++++++++++ docs/generated/rules.md | 10 ++++- pyproject.toml | 3 ++ rules/registry.toml | 24 +++++++++++ uv.lock | 64 ++++++++++++++++++++++++++++++ 9 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 changelog.d/4199.added.md diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 60bde515a..49e038f3f 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -34,6 +34,7 @@ jobs: - check-test-shards - upload-test-coverage - upload-integration-coverage + - diff-coverage runs-on: ubuntu-latest permissions: contents: read @@ -385,6 +386,47 @@ 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) 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 pull request's base commit. + 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 + - name: Check the coverage of the changed lines + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: uv run --frozen --extra dev diff-cover "$RUNNER_TEMP"/coverage/*/coverage*.xml --compare-branch="$BASE_SHA" --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 diff --git a/AGENTS.md b/AGENTS.md index 4cd0c92aa..d74797fdf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,7 +117,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.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.tests-pass`, `api.quality.tests-ruff-exemptions`, `api.quality.unittest-ruff-exemptions` ### Observability diff --git a/changelog.d/4199.added.md b/changelog.d/4199.added.md new file mode 100644 index 000000000..51143d732 --- /dev/null +++ b/changelog.d/4199.added.md @@ -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. diff --git a/codecov.yml b/codecov.yml index f28faea8b..bb17f5d4f 100644 --- a/codecov.yml +++ b/codecov.yml @@ -34,9 +34,9 @@ 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 it. + target: 80% informational: true flag_management: diff --git a/docs/engineering/skills/testing.md b/docs/engineering/skills/testing.md index 59fcb3310..88e68a551 100644 --- a/docs/engineering/skills/testing.md +++ b/docs/engineering/skills/testing.md @@ -89,6 +89,30 @@ 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/` 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 the pull request's base commit. 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. Codecov's patch status shows the same 80% target +but doesn't block a merge. + +To check before pushing, after `make test` has written `coverage.xml`: + +```bash +uv run --frozen --extra dev 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 diff --git a/docs/generated/rules.md b/docs/generated/rules.md index d1c90a4b0..bac69d9c6 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -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 | 64 | 3 | 1 | 11 | 79 | -| metric | 1 | 0 | 0 | 0 | 1 | +| metric | 2 | 0 | 0 | 0 | 2 | | judgment | 0 | 0 | 0 | 1 | 1 | -**Blocking:** 68 of 81 rules. +**Blocking:** 69 of 82 rules. ## Gaps @@ -357,6 +357,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 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 pull request's base 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` | — | @@ -381,6 +382,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"] @@ -412,6 +416,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 diff --git a/pyproject.toml b/pyproject.toml index b8e9ac635..681cdd10c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,6 +67,9 @@ dependencies = [ dev = [ "build", "coverage", + # 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", diff --git a/rules/registry.toml b/rules/registry.toml index 80f7ce6d7..7a9534bc0 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1331,6 +1331,30 @@ bad_fixture = "rules/fixtures/api.quality.coverage-measured/rejected" good_fixture = "rules/fixtures/api.quality.coverage-measured/accepted" test = "tests/unit/test_codecov_coverage.py::test_rule_proof_coverage_measured" +[[rule]] +id = "api.quality.diff-coverage" +category = "quality" +statement = "The lines a pull request changes in policyengine_api 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 pull request's base commit, and CI complete needs it." +rationale = "docs/engineering/skills/testing.md#changed-lines" +scope = ["policyengine_api/**/*.py"] +tier = "metric" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "workflow-job" +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "diff-coverage" +invocation = "diff-cover" +[rule.proof] +below_fixture = "rules/fixtures/api.quality.diff-coverage/below_threshold.toml" +above_fixture = "rules/fixtures/api.quality.diff-coverage/at_threshold.toml" +threshold = 80 +test = "tests/unit/test_diff_coverage.py::test_rule_proof_diff_coverage" +workflow = ".github/workflows/pr.yml" +job = "test" +invocation = "make test" + [[rule]] id = "api.quality.environment-variables-load" category = "quality" diff --git a/uv.lock b/uv.lock index 28401b21f..93e6ca5c8 100644 --- a/uv.lock +++ b/uv.lock @@ -424,6 +424,53 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/ae/3a/dbeec9d1ee0844c679f6bb5d6ad4e9f198b1224f4e7a32825f47f6192b0c/cffi-2.0.0-cp314-cp314t-win_arm64.whl", hash = "sha256:0a1527a803f0a659de1af2e1fd700213caba79377e27e4693648c2923da066f9", size = 184195, upload-time = "2025-09-08T23:23:43.004Z" }, ] +[[package]] +name = "chardet" +version = "7.7.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/47/92/3d893ac5d338b9a9af34c5b517d77492dcea8a10254254eead4215a96f3f/chardet-7.7.0.tar.gz", hash = "sha256:0ba38a05201b0abf7570c12d8af45c30b2af835bd5cc52bfc7105082eda82c1e", size = 1008480, upload-time = "2026-10-18T00:06:25.352Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b0/23/c607e55b2c00fd383803a20dfdaa964e966df0472645902b9cde64d50519/chardet-7.7.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:0d7dfd38f9061226fc6b79732d8fcc6f3af070dc757a421cab7025d2a780a74c", size = 1206347, upload-time = "2026-10-18T00:05:26.954Z" }, + { url = "https://files.pythonhosted.org/packages/bd/f7/a561c39b0b697757f27c7bd14e71cd30887a70bde4b76f0f18eb5a97d347/chardet-7.7.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:074a5b6efd46bd35059f53a3b385349e9b9e38111063c8d2c5b36923e2e07de6", size = 1183911, upload-time = "2026-10-18T00:05:28.554Z" }, + { url = "https://files.pythonhosted.org/packages/a3/81/7044323a1952245ecf0b5535fd3b21de92b805f34f093005e9ff5d4469c0/chardet-7.7.0-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:1292aedb9e9da26435f7972f7efa145e773140068238888c6a533c1b5f17b88b", size = 1625048, upload-time = "2026-10-18T00:05:30.07Z" }, + { url = "https://files.pythonhosted.org/packages/e2/75/df325fbfe77140e1c8dfc4915b23425bc47a55e3132bd004c4064cf132e9/chardet-7.7.0-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:7f13de78af5da07ddd813ff5a564c3aa8a113f05cba202be649e061c431a0e9a", size = 1648612, upload-time = "2026-10-18T00:05:31.53Z" }, + { url = "https://files.pythonhosted.org/packages/44/c3/4c5e2993a4bda9a432cc1094f89f1ef22fca8f0a9526673bdfaa39e5d317/chardet-7.7.0-cp311-cp311-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:78608fdf8eced38643b5e0813c2bfc969cd13b393f025a838e976fee20f74dbc", size = 1599230, upload-time = "2026-10-18T00:05:32.954Z" }, + { url = "https://files.pythonhosted.org/packages/0c/f0/ce33a549471c4b408d40b2ed061be5d4f12265934fc4ef100e3cbaa1bf68/chardet-7.7.0-cp311-cp311-win_amd64.whl", hash = "sha256:6f70dbc2dd14145d9b4011bce5a207f7ecc26e6bea49b5e87f6cac3b84261089", size = 1303330, upload-time = "2026-10-18T00:05:34.457Z" }, + { url = "https://files.pythonhosted.org/packages/d3/78/d6b54ac2b0623565e30ef920acb9f5b5db150d11963d9cab24931aa122ce/chardet-7.7.0-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:14a7a67fc65b44346c8cd9a84b3a209f9b84a2f3984c0d56a7809815c657664e", size = 1214345, upload-time = "2026-10-18T00:05:36.144Z" }, + { url = "https://files.pythonhosted.org/packages/49/d2/c920557bd0a621dcf6fb7a96e8746dd70c6b83e7bd6c03036b2c824a59de/chardet-7.7.0-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:7df40e5267a4e9952ecb39cc23109c389fca61fc27a6e807ec9ea894a41e83d7", size = 1185195, upload-time = "2026-10-18T00:05:37.826Z" }, + { url = "https://files.pythonhosted.org/packages/88/7d/f2aede138fe124daad8a6dd8a0c593cdad117429af5d734f7bc8ec290084/chardet-7.7.0-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:68db376d4bb403dc282a6d2ed5b4c4b45a7a21dc7537e79699f8be669a9e88ef", size = 1623002, upload-time = "2026-10-18T00:05:39.295Z" }, + { url = "https://files.pythonhosted.org/packages/28/af/8cf3a99fe518abbc314f873cc5aeaa412b4817c927b643cbe3fcf8b30122/chardet-7.7.0-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:feb66c96264e3e6a9ff0c7318ce0028800b6c18ad07c8000d2404a108d1ef767", size = 1658969, upload-time = "2026-10-18T00:05:41.13Z" }, + { url = "https://files.pythonhosted.org/packages/ca/08/c8d96d4c1d6a9c8ba252937bf13db1ae6e6f1f89504ea75c84c74cbb681a/chardet-7.7.0-cp312-cp312-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:169d8e5f621bec1979e351f5ade8cfca3b53da85756888bbf39094fa764bde84", size = 1589745, upload-time = "2026-10-18T00:05:42.648Z" }, + { url = "https://files.pythonhosted.org/packages/06/cb/f236829ccedb665fa0b187339d12ee8ac6c3bf5fae37a4433aa08e1e69ce/chardet-7.7.0-cp312-cp312-win_amd64.whl", hash = "sha256:e3fa9ea40e766b26b10360edbb4742cfef9e52728b644b39aae7fa39e89df767", size = 1308542, upload-time = "2026-10-18T00:05:44.308Z" }, + { url = "https://files.pythonhosted.org/packages/4e/73/0c434bc65adc411e49b1bbebee24ebb36a3511c80a058f1fde17f43fb537/chardet-7.7.0-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:11263887c082c3d8c32026aeddbdef37ac8feccd1faece5a22b2ff00a29b115a", size = 1202232, upload-time = "2026-10-18T00:05:45.657Z" }, + { url = "https://files.pythonhosted.org/packages/9d/be/222e5628af10d6b80750ccdda7f6d2610646dddb082c37947623d5e55bf7/chardet-7.7.0-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:e8c718e6961981fe1685e1243742f326ac553f0fa0dfd0c6d3d17dd4f92cd3ff", size = 1173273, upload-time = "2026-10-18T00:05:46.984Z" }, + { url = "https://files.pythonhosted.org/packages/70/ff/889cf390db3748918789818d93ba9a7d712b326030199698432fc50bf8cd/chardet-7.7.0-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:a76f4d1f769b4ae8888cefd46e604cf1f5769f1176b55e78b90eaeefa888ce92", size = 1616968, upload-time = "2026-10-18T00:05:48.457Z" }, + { url = "https://files.pythonhosted.org/packages/ca/04/0b5fd38375ae182cec6446dd3121aaff5ae18fd12b98b10161a58b720db9/chardet-7.7.0-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:c519c10c315a675bcfd4b55c0e0de1859ca5a39881d8f69750500102c2fc5d2f", size = 1658830, upload-time = "2026-10-18T00:05:49.938Z" }, + { url = "https://files.pythonhosted.org/packages/99/c0/7e642d7bc6aca242ef41b4aa5f5be27dbb57912e8ce0f38043d8d1999cd1/chardet-7.7.0-cp313-cp313-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:46a044166fe0d99931884dc28405faac558d5eabf28316a2b3d2a1a6c7dc439a", size = 1583300, upload-time = "2026-10-18T00:05:51.567Z" }, + { url = "https://files.pythonhosted.org/packages/30/ee/53147716ced6d43837682a2acfb09dd45add8bc496107f21051c5aeae1f6/chardet-7.7.0-cp313-cp313-win_amd64.whl", hash = "sha256:113bbd3ab85835e3e99499ae66829915ed3b4cf5c69525ab1742260f77638449", size = 1308030, upload-time = "2026-10-18T00:05:52.997Z" }, + { url = "https://files.pythonhosted.org/packages/1a/61/800a9442f352912c86f2a00de84fd6914dd9353d29091b1e223e7c08850e/chardet-7.7.0-cp314-cp314-macosx_10_15_x86_64.whl", hash = "sha256:ad286a0654ae7477bc454a1dee4f396cb28adffeacc6bc7d4412f607c73de0c3", size = 1200940, upload-time = "2026-10-18T00:05:54.59Z" }, + { url = "https://files.pythonhosted.org/packages/fb/92/3c6ce24dd68c626cc6aa0b5953c2aebb1d853c7b07d3535f0300b7428288/chardet-7.7.0-cp314-cp314-macosx_11_0_arm64.whl", hash = "sha256:4f67f20c9ad3f3024bf71d4aae1d25722b12da921ca9586f3cce7a4639695aa0", size = 1173087, upload-time = "2026-10-18T00:05:56.257Z" }, + { url = "https://files.pythonhosted.org/packages/4d/32/5a952b376c00dda01465b396958a8985916d9a9293a772d6bda2c37f2cc5/chardet-7.7.0-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:a05667635c0496dab47eca8c845c044d1da6d74d34a9bf687199c9c53a38119d", size = 1624001, upload-time = "2026-10-18T00:05:57.832Z" }, + { url = "https://files.pythonhosted.org/packages/04/f7/09296ca1d4621c9c2766d074822035eee6aebb79eff80c0af3bd96283d05/chardet-7.7.0-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:79eec5af9d7715334695254234c8d7c29165841526c11083951d9b9f6458e372", size = 1653785, upload-time = "2026-10-18T00:05:59.379Z" }, + { url = "https://files.pythonhosted.org/packages/cb/08/80571baad57db2962d6931b3ee1828c82e3e14e9b3464f80532f536e23ac/chardet-7.7.0-cp314-cp314-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:4167165812219e70f311083a8e06e3b627ed5b27aa8c15c3fdd56f350fd3213e", size = 1582280, upload-time = "2026-10-18T00:06:00.825Z" }, + { url = "https://files.pythonhosted.org/packages/18/82/3c07070fdcf3c4263e16680c90e9af7c2cd2e5e59cfb1d372efa60fde8cc/chardet-7.7.0-cp314-cp314-win_amd64.whl", hash = "sha256:546180f6170da39778069d16130449a9cff49a8fe144535947d1ef26f5330b96", size = 1310108, upload-time = "2026-10-18T00:06:02.342Z" }, + { url = "https://files.pythonhosted.org/packages/1b/b0/d703d408c7f8931b1564110078fc3dcff9825a159990fa69b80da52be160/chardet-7.7.0-cp314-cp314t-macosx_10_15_x86_64.whl", hash = "sha256:a4c2e83b09a540a4d33279d2bee8ed3bfd57ac6645b359ae8b8afd5b4578c3c2", size = 1237782, upload-time = "2026-10-18T00:06:03.763Z" }, + { url = "https://files.pythonhosted.org/packages/d8/3e/0eafeebd53eb096400f75138ab18614ab5be12b8794b76dd4b35ef1a8e05/chardet-7.7.0-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:259dd55b1ba05a66868151a42663483763046a3e6579bde6b0aa65d834635b5a", size = 1207710, upload-time = "2026-10-18T00:06:05.221Z" }, + { url = "https://files.pythonhosted.org/packages/b8/8d/3c200098027def56be1b6443a10c72efe013a42eb76444d889c809890ca5/chardet-7.7.0-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:56e5a5c782548b301b44371de327115d13a08de391ddaf9db4ac420d8511c5d7", size = 1660832, upload-time = "2026-10-18T00:06:06.619Z" }, + { url = "https://files.pythonhosted.org/packages/08/81/02f98389fa72345f40d23d69c2d857511d11a0911779a7cf8ed1cecb5037/chardet-7.7.0-cp314-cp314t-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:d227d2aa329fcf848f4640ed9a4d99e1753840c1cfa19f7ca5a48f30c03982ad", size = 1678291, upload-time = "2026-10-18T00:06:08.196Z" }, + { url = "https://files.pythonhosted.org/packages/5e/24/0ae866b759a148d0f6087ffb30920a2b956653d0d455b4be93026edbee95/chardet-7.7.0-cp315-cp315-macosx_10_15_x86_64.whl", hash = "sha256:c4a591b2dbfc0a047f8d60331c2fbfb4c3589911000406666a3085fa7d5da69f", size = 1200696, upload-time = "2026-10-18T00:06:09.701Z" }, + { url = "https://files.pythonhosted.org/packages/e0/a6/5e0a1f56aca6c44b562d05d9e360f31b5a8f4b311d940a324fa337d277ba/chardet-7.7.0-cp315-cp315-macosx_11_0_arm64.whl", hash = "sha256:eb625c3ab2386ca7f3be9ac2b9b822f4c5b792595c8bb8100b3b6ea68120e92c", size = 1172821, upload-time = "2026-10-18T00:06:11.04Z" }, + { url = "https://files.pythonhosted.org/packages/89/05/868f52672cde7e188a7333f3f65deaa62f5acbfcf0f81f3582013776e8ad/chardet-7.7.0-cp315-cp315-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:ddedfb252d05c86c2d6e9a3090f8bd574876bbd39679e7c588ed57d21595c097", size = 1625481, upload-time = "2026-10-18T00:06:12.499Z" }, + { url = "https://files.pythonhosted.org/packages/f9/12/2b3a006ae31a7ab4ccac11a00abb27af23b4a5d57ddb00e937375e747a2a/chardet-7.7.0-cp315-cp315-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:42c4d8a2dc8abcc067417adc13c2f64f48482e48ce48e3af769b5a44214e5a5c", size = 1660352, upload-time = "2026-10-18T00:06:13.977Z" }, + { url = "https://files.pythonhosted.org/packages/b6/f0/f995a2873a78aa3ec8308f02e61f0717deb4b01c8f1a74c2da329f325925/chardet-7.7.0-cp315-cp315-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:78db6186c1e73d38a2106cdcbbc8f43612eaed9ef8bc4adaf0fe67b6e8a2049d", size = 1597363, upload-time = "2026-10-18T00:06:15.492Z" }, + { url = "https://files.pythonhosted.org/packages/4a/7b/22b916b52c7bc4eb076ba1736ea9b99cf1065ebb7b56d356409144887c73/chardet-7.7.0-cp315-cp315-win_amd64.whl", hash = "sha256:8f1d611f15530e1642856d91b7aa259851466d7e961f27b888921e7e29718edb", size = 1309528, upload-time = "2026-10-18T00:06:16.845Z" }, + { url = "https://files.pythonhosted.org/packages/02/2f/ccaf5ffa0c8adca280e2687d75524cc07113a0df70cbf542da3445f2b300/chardet-7.7.0-cp315-cp315t-macosx_10_15_x86_64.whl", hash = "sha256:1db39957379a801cf1798a5e50cdb3d3fc32683e23c4166ecfa157b980d1931e", size = 1236698, upload-time = "2026-10-18T00:06:18.191Z" }, + { url = "https://files.pythonhosted.org/packages/0d/7e/32f45f7ddeb27d94feb529a1a23e5a38a6d3614d12e23f0c89a9eb89098b/chardet-7.7.0-cp315-cp315t-macosx_11_0_arm64.whl", hash = "sha256:7f3b2ca3bac2bdbe42c4dfe7a2888c4b98e2e86a83ddb631960b229d1980979e", size = 1207247, upload-time = "2026-10-18T00:06:19.751Z" }, + { url = "https://files.pythonhosted.org/packages/26/40/8c1ab360c39a0bb823786e8934984cbc3e83076dfce2a7f3760086ef778c/chardet-7.7.0-cp315-cp315t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:80db1c109e6e3c102ac589897449437ea2edf26dda1042a3ff669368e21d90e5", size = 1653659, upload-time = "2026-10-18T00:06:21.189Z" }, + { url = "https://files.pythonhosted.org/packages/1b/3e/bd5c25f525ff2466e6365cbfb098f928761357331b884eebc92ac50f8878/chardet-7.7.0-cp315-cp315t-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:091f50d3567b1a066f797835ee5367dbcf3cb77c5e56993b3f61b28264b8e6f8", size = 1677031, upload-time = "2026-10-18T00:06:22.564Z" }, + { url = "https://files.pythonhosted.org/packages/cc/75/88a3eb36264375ff951bd2f9397c409727ab6aa6a18e1bd933851571d421/chardet-7.7.0-py3-none-any.whl", hash = "sha256:11cf9b719ea3675a2a7d8a3071bbe4d7ea24b26bfe39a51588d515ddd05cacad", size = 706680, upload-time = "2026-10-18T00:06:24.018Z" }, +] + [[package]] name = "charset-normalizer" version = "3.4.7" @@ -648,6 +695,21 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/4e/8c/f3147f5c4b73e7550fe5f9352eaa956ae838d5c51eb58e7a25b9f3e2643b/decorator-5.2.1-py3-none-any.whl", hash = "sha256:d316bb415a2d9e2d2b3abcc4084c6502fc09240e292cd76a76afc106a1c8e04a", size = 9190, upload-time = "2025-02-24T04:41:32.565Z" }, ] +[[package]] +name = "diff-cover" +version = "10.6.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "chardet" }, + { name = "jinja2" }, + { name = "pluggy" }, + { name = "pygments" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/63/ef/479d5b18c5cd61c4fbb769c60d4829ea9d70aef1267e302d6de60deed7f3/diff_cover-10.6.0.tar.gz", hash = "sha256:8d1194dfb537f70480ac97c9c45dd40721cf4c326c9903fbe997db67a4db6195", size = 107697, upload-time = "2026-09-22T00:43:00.885Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/47/04/7130707bb385835cca7f168c80d812c9b42af9ae8230a26b3e21c9437c98/diff_cover-10.6.0-py3-none-any.whl", hash = "sha256:a701553060f72d59b81f9cb1645b75ef66204427eb9dd99873c91092220e52ce", size = 63351, upload-time = "2026-09-22T00:42:59.593Z" }, +] + [[package]] name = "distro" version = "1.9.0" @@ -2932,6 +2994,7 @@ dependencies = [ dev = [ { name = "build" }, { name = "coverage" }, + { name = "diff-cover" }, { name = "import-linter" }, { name = "mypy" }, { name = "pytest" }, @@ -2956,6 +3019,7 @@ requires-dist = [ { name = "click", specifier = ">=8,<9" }, { name = "cloud-sql-python-connector" }, { name = "coverage", marker = "extra == 'dev'" }, + { name = "diff-cover", marker = "extra == 'dev'", specifier = ">=10.6,<11" }, { name = "faiss-cpu" }, { name = "fastapi", specifier = ">=0.115,<1" }, { name = "flask", specifier = ">=3,<4" }, From 72485ef66b5330f69bef2299d6b8422594f7d578 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:44:36 +0400 Subject: [PATCH 3/8] Test diff-cover's base, unimported modules and type-only imports (failing) Review found three ways the diff-coverage job can score the wrong lines: - It compares with the pull_request event's base.sha, which often stays where the branch left master, while CI checks out a merge into master as it is now; master's own changes since then count as the pull request's. - coverage.py 6.5 lists a module no test imports only inside a package, and policyengine_api/libs and policyengine_api/routes have no __init__.py, so a new untested module there would not be measured and would pass. - Imports under `if TYPE_CHECKING:` never run and count as missed. New tests run the job's own command on a merge commit, run coverage.py with the repository's .coveragerc, and check the step's wiring. Four fail: - test_the_job_measures_only_the_lines_the_pull_request_changes: 90%, exit 0, with master_change.py counted (should be 50%, exit 1) - test_coverage_reports_every_module_no_test_imports: 15 modules in libs/ and routes/ unreported - test_type_only_imports_and_pragmas_are_not_counted_as_missed: line 4 missed - test_pull_requests_check_the_coverage_of_their_changed_lines: the step still reads BASE_SHA test_a_line_counts_as_covered_if_any_report_covers_it passes already. Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_diff_coverage.py | 232 ++++++++++++++++++++++++++++--- 1 file changed, 213 insertions(+), 19 deletions(-) diff --git a/tests/unit/test_diff_coverage.py b/tests/unit/test_diff_coverage.py index 191a94568..36bfeafe3 100644 --- a/tests/unit/test_diff_coverage.py +++ b/tests/unit/test_diff_coverage.py @@ -2,14 +2,18 @@ pr.yml's diff-coverage job downloads the six test shards' coverage.xml files and the cross-database integration tests' report, and runs diff-cover against -the pull request's base commit. diff-cover fails when less than the minimum of -the changed lines in measured files (policyengine_api, .coveragerc's source) -ran, counting a line as covered if any report covers it. CI complete needs the -job, so a failure blocks the merge. +the first parent of the merge commit CI checks out: the base branch the pull +request merges into. diff-cover fails when less than the minimum of the changed +lines in measured files (policyengine_api, .coveragerc's source) ran, counting +a line as covered if any report covers it. CI complete needs the job, so a +failure blocks the merge. """ from __future__ import annotations +import json +import os +import shlex import subprocess import sys import tomllib @@ -21,34 +25,50 @@ RULE_ID = "api.quality.diff-coverage" FIXTURES = REPO / "rules" / "fixtures" / RULE_ID PR_WORKFLOW = REPO / ".github" / "workflows" / "pr.yml" +COVERAGE_CONFIG = REPO / ".coveragerc" JOB = "diff-coverage" +CHECK_STEP = "Check the coverage of the changed lines" MINIMUM_PERCENT = 80 DIFF_COVER = Path(sys.executable).parent / "diff-cover" +LOCKED_DIFF_COVER = "uv run --frozen --extra dev diff-cover" +# The pull_request event's base.sha, which the job's step may read. +BASE_SHA_EXPRESSION = "${{ github.event.pull_request.base.sha }}" GIT = ["git", "-c", "user.name=test", "-c", "user.email=test@example.com"] -def _git(repo: Path, *arguments: str) -> None: - subprocess.run( +def _git(repo: Path, *arguments: str) -> str: + return subprocess.run( [*GIT, "-c", "commit.gpgsign=false", *arguments], cwd=repo, check=True, capture_output=True, - ) + text=True, + ).stdout.strip() -def _coverage_report(source: Path, hits: list[int]) -> str: - lines = "".join( - f'' - for number, count in enumerate(hits, start=1) +def _coverage_report(source: Path, files: dict[str, list[int]]) -> str: + """Return a coverage.py XML report: each file's hits, from its first line.""" + classes = "".join( + f'' + + "".join( + f'' + for number, count in enumerate(hits, start=1) + ) + + "" + for name, hits in files.items() ) return ( '' f"{source}" - '' - f"{lines}" + f'{classes}' + "" ) +def _write_lines(module: Path, count: int) -> None: + module.write_text("".join(f"value_{index} = {index}\n" for index in range(count))) + + def _run_diff_cover(fixture: Path, root: Path) -> subprocess.CompletedProcess[str]: """Commit a module, change it as ``fixture`` says, and run diff-cover on it.""" spec = tomllib.loads(fixture.read_text(encoding="utf-8")) @@ -66,7 +86,7 @@ def _run_diff_cover(fixture: Path, root: Path) -> subprocess.CompletedProcess[st ) _git(root, "commit", "--quiet", "-am", "change") hits = [1] + [1] * covered + [0] * (changed - covered) - (root / "coverage.xml").write_text(_coverage_report(package, hits)) + (root / "coverage.xml").write_text(_coverage_report(package, {"module.py": hits})) return subprocess.run( [ str(DIFF_COVER), @@ -81,6 +101,75 @@ def _run_diff_cover(fixture: Path, root: Path) -> subprocess.CompletedProcess[st ) +def _merged_pull_request(root: Path) -> str: + """Build the merge commit CI checks out; return where the branch left master. + + The pull request adds 10 lines in pkg/pull_request.py. After it left + master, master gained 100 lines in pkg/master_change.py. HEAD merges the + pull request into master as it is now, as GitHub's merge ref does. + """ + package = root / "pkg" + package.mkdir(parents=True) + (package / "module.py").write_text("VALUE = 0\n") + _git(root, "init", "--quiet", "--initial-branch=master") + _git(root, "add", ".") + _git(root, "commit", "--quiet", "-m", "where the pull request starts") + fork_point = _git(root, "rev-parse", "HEAD") + _git(root, "checkout", "--quiet", "-b", "pull-request") + _write_lines(package / "pull_request.py", 10) + _git(root, "add", ".") + _git(root, "commit", "--quiet", "-m", "the pull request's change") + _git(root, "checkout", "--quiet", "master") + _write_lines(package / "master_change.py", 100) + _git(root, "add", ".") + _git(root, "commit", "--quiet", "-m", "master moves on") + _git(root, "merge", "--quiet", "--no-ff", "--no-edit", "pull-request") + return fork_point + + +def _run_job_check( + repo: Path, reports: dict[str, str], base_sha: str +) -> subprocess.CompletedProcess[str]: + """Run the job's check step in ``repo`` as CI does, with the locked diff-cover. + + ``reports`` maps each report's path under $RUNNER_TEMP/coverage, where the + job's download steps put them, to its XML. ``base_sha`` is the + pull_request event's base.sha, for a step that reads it. + """ + runner_temp = repo.parent / "runner-temp" + for relative_path, report in reports.items(): + path = runner_temp / "coverage" / relative_path + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(report) + step = _step(_job(), CHECK_STEP) + assert step["run"].startswith(f"{LOCKED_DIFF_COVER} ") + command = shlex.quote(str(DIFF_COVER)) + step["run"].removeprefix(LOCKED_DIFF_COVER) + step_environment = { + name: value.replace(BASE_SHA_EXPRESSION, base_sha) + for name, value in step.get("env", {}).items() + } + assert not any("${{" in value for value in step_environment.values()) + return subprocess.run( + ["bash", "-e", "-c", command], + cwd=repo, + env={**os.environ, "RUNNER_TEMP": str(runner_temp), **step_environment}, + capture_output=True, + text=True, + check=False, + ) + + +def _run_coverage(cwd: Path, data_file: Path, *arguments: str) -> None: + """Run coverage.py with the repository's .coveragerc.""" + subprocess.run( + [sys.executable, "-m", "coverage", *arguments], + cwd=cwd, + env={**os.environ, "COVERAGE_FILE": str(data_file)}, + check=True, + capture_output=True, + ) + + def _job() -> dict: return yaml.safe_load(PR_WORKFLOW.read_text(encoding="utf-8"))["jobs"][JOB] @@ -100,13 +189,114 @@ def test_rule_proof_diff_coverage(tmp_path: Path): assert "Coverage: 80%" in at_minimum.stdout +def test_the_job_measures_only_the_lines_the_pull_request_changes(tmp_path: Path): + # The event's base.sha often stays where the branch left master, while CI + # checks out a merge into master as it is now. Master's own changes since + # then, 95% covered here, must not lift the pull request's 50%. + repo = tmp_path / "repo" + fork_point = _merged_pull_request(repo) + report = _coverage_report( + repo / "pkg", + { + "module.py": [1], + "pull_request.py": [1] * 5 + [0] * 5, + "master_change.py": [1] * 95 + [0] * 5, + }, + ) + + result = _run_job_check( + repo, {"test-coverage-shard-1/coverage.xml": report}, base_sha=fork_point + ) + + assert result.returncode != 0, result.stdout + assert "Coverage: 50%" in result.stdout + assert "master_change.py" not in result.stdout + + +def test_a_line_counts_as_covered_if_any_report_covers_it(tmp_path: Path): + # A shard covers the first five changed lines and the integration tests + # lines four to eight: 50% each, 80% together. + repo = tmp_path / "repo" + fork_point = _merged_pull_request(repo) + shard = _coverage_report(repo / "pkg", {"pull_request.py": [1] * 5 + [0] * 5}) + integration = _coverage_report( + repo / "pkg", {"pull_request.py": [0] * 3 + [1] * 5 + [0] * 2} + ) + + result = _run_job_check( + repo, + { + "test-coverage-shard-1/coverage.xml": shard, + "integration-coverage/coverage-v2.xml": integration, + }, + base_sha=fork_point, + ) + + assert result.returncode == 0, result.stdout + assert "Coverage: 80%" in result.stdout + + +def test_coverage_reports_every_module_no_test_imports(tmp_path: Path): + # diff-cover only counts the files a report lists. coverage.py lists a + # module no test imported only if every directory between it and the + # source root has an __init__.py, so an untested module elsewhere under + # policyengine_api would be left out, and its changed lines would pass. + nothing = tmp_path / "nothing.py" + nothing.write_text("") + report = tmp_path / "coverage.json" + data_file = tmp_path / ".coverage" + rcfile = f"--rcfile={COVERAGE_CONFIG}" + + _run_coverage(REPO, data_file, "run", rcfile, str(nothing)) + _run_coverage(REPO, data_file, "json", rcfile, "-q", "-o", str(report)) + + reported = set(json.loads(report.read_text())["files"]) + modules = { + path.relative_to(REPO).as_posix() + for path in (REPO / "policyengine_api").rglob("*.py") + } + assert sorted(modules - reported) == [] + + +def test_type_only_imports_and_pragmas_are_not_counted_as_missed(tmp_path: Path): + # Imports under `if TYPE_CHECKING:` never run, and ruff's TC rules move + # imports there, so counting them would fail a fully tested change. + # `pragma: no cover` keeps working too. + package = tmp_path / "policyengine_api" + package.mkdir() + (package / "__init__.py").write_text("") + (package / "module.py").write_text( + "from typing import TYPE_CHECKING\n" + "\n" + "if TYPE_CHECKING:\n" + " from collections.abc import Iterator\n" + "\n" + "VALUE = 1\n" + "\n" + "\n" + "def unused(): # pragma: no cover\n" + " return VALUE\n" + ) + (tmp_path / "run.py").write_text("import policyengine_api.module\n") + report = tmp_path / "coverage.json" + data_file = tmp_path / ".coverage" + rcfile = f"--rcfile={COVERAGE_CONFIG}" + + _run_coverage(tmp_path, data_file, "run", rcfile, "--branch", "run.py") + _run_coverage(tmp_path, data_file, "json", rcfile, "-q", "-o", str(report)) + + module = json.loads(report.read_text())["files"]["policyengine_api/module.py"] + assert module["missing_lines"] == [] + assert {4, 10} <= set(module["excluded_lines"]) + + def test_pull_requests_check_the_coverage_of_their_changed_lines(): workflow = yaml.safe_load(PR_WORKFLOW.read_text(encoding="utf-8")) job = workflow["jobs"][JOB] checkout = _step(job, "Checkout repo") shards = _step(job, "Download every test shard's coverage report") integration = _step(job, "Download the integration coverage report") - check = _step(job, "Check the coverage of the changed lines") + check = _step(job, CHECK_STEP) # It runs after both kinds of tests, with no condition of its own: on a # stacked pull request above the lowest one they are skipped, so it is @@ -116,18 +306,22 @@ def test_pull_requests_check_the_coverage_of_their_changed_lines(): assert JOB in workflow["jobs"]["ci-complete"]["needs"] assert 0 < job["timeout-minutes"] <= 15 assert job["env"] == {"UV_PYTHON": "3.12"} - # diff-cover compares with the base commit, which a shallow clone lacks. + # diff-cover compares with the merge commit's first parent, which a + # depth-1 clone lacks. assert checkout["with"]["fetch-depth"] == 0 assert shards["with"]["pattern"] == "test-coverage-shard-*" assert integration["with"]["name"] == "integration-coverage" reports = "${{ runner.temp }}/coverage" assert shards["with"]["path"] == reports assert integration["with"]["path"] == f"{reports}/integration-coverage" - assert check["env"] == {"BASE_SHA": "${{ github.event.pull_request.base.sha }}"} + # The checkout is GitHub's merge of the pull request into its base branch, + # so HEAD^1 is the base branch the pull request merges into. The event's + # base.sha can be older, which would count the base branch's own changes. + assert "env" not in check assert check["run"] == ( - "uv run --frozen --extra dev diff-cover " + f"{LOCKED_DIFF_COVER} " '"$RUNNER_TEMP"/coverage/*/coverage*.xml ' - f'--compare-branch="$BASE_SHA" --fail-under={MINIMUM_PERCENT}' + f"--compare-branch=HEAD^1 --fail-under={MINIMUM_PERCENT}" ) From 1eda3287fd2609b1a26abde72700d035f76a0404 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:46:54 +0400 Subject: [PATCH 4/8] Measure only the pull request's lines, every module, and code that runs - pr.yml's diff-coverage job compares with HEAD^1, the base branch that GitHub's merge commit is built on, instead of the pull_request event's base.sha. base.sha often stays where the branch left master (17 of 88 recent pr.yml runs, up to 1,077 lines of master's changes under policyengine_api in between), and diff-cover counted those lines too. - policyengine_api/libs and policyengine_api/routes get an __init__.py. coverage.py 6.5 reports a module no test imports only inside a package, so their untested modules were left out of every report and their changed lines passed. pyproject.toml's ruff comment no longer says they have none. - .coveragerc excludes `if TYPE_CHECKING:` blocks, which never run, and keeps coverage.py's default `pragma: no cover`, which setting exclude_lines replaces. - v2-integration-check.yml's comment says the diff-coverage job reads the integration report too. The testing skill and the rule's statement describe the HEAD^1 comparison and the __init__.py requirement. The four tests that failed now pass, with the other four in tests/unit/test_diff_coverage.py. Co-Authored-By: Claude Opus 5.5 --- .coveragerc | 9 ++++++++- .github/workflows/pr.yml | 12 ++++++++---- .github/workflows/v2-integration-check.yml | 10 ++++++---- docs/engineering/skills/testing.md | 20 +++++++++++++++----- docs/generated/rules.md | 2 +- policyengine_api/libs/__init__.py | 5 +++++ policyengine_api/routes/__init__.py | 5 +++++ pyproject.toml | 5 +++-- rules/registry.toml | 2 +- 9 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 policyengine_api/libs/__init__.py create mode 100644 policyengine_api/routes/__init__.py diff --git a/.coveragerc b/.coveragerc index 6c35d11d1..db2bcff7a 100644 --- a/.coveragerc +++ b/.coveragerc @@ -2,8 +2,15 @@ source = policyengine_api [report] +# Setting exclude_lines replaces coverage.py's default, so it lists that +# default, `pragma: no cover`, again. Imports under `if TYPE_CHECKING:` never +# run, and ruff's TC rules move imports there, so counting them as missed +# would fail a fully tested change (api.quality.diff-coverage). +exclude_lines = + pragma: no cover + if TYPE_CHECKING: omit = tests/* */tests/* tests/conftest.py - + diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 49e038f3f..0aaf4209b 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -408,7 +408,8 @@ jobs: - name: Checkout repo uses: actions/checkout@v4 with: - # diff-cover compares with the pull request's base commit. + # 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 @@ -422,10 +423,13 @@ jobs: with: name: integration-coverage path: ${{ runner.temp }}/coverage/integration-coverage + # The checkout is GitHub's merge of the pull request into its base + # branch, so HEAD^1 is that branch as the merge saw it and 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 - env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: uv run --frozen --extra dev diff-cover "$RUNNER_TEMP"/coverage/*/coverage*.xml --compare-branch="$BASE_SHA" --fail-under=80 + 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 diff --git a/.github/workflows/v2-integration-check.yml b/.github/workflows/v2-integration-check.yml index 991526696..37b94289a 100644 --- a/.github/workflows/v2-integration-check.yml +++ b/.github/workflows/v2-integration-check.yml @@ -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 diff --git a/docs/engineering/skills/testing.md b/docs/engineering/skills/testing.md index 88e68a551..72f192785 100644 --- a/docs/engineering/skills/testing.md +++ b/docs/engineering/skills/testing.md @@ -96,13 +96,23 @@ in `policyengine_api/` 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 the pull request's base commit. 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. +against `HEAD^1`. CI checks out GitHub's merge of the pull request into its +base branch, so `HEAD^1` is that branch as the merge saw it, and 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. Codecov's patch status shows the same 80% target -but doesn't block a merge. +`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 under `if TYPE_CHECKING:`, which +never run, and lines marked `# pragma: no cover` don't count (`.coveragerc`). +Codecov's patch status shows the same 80% target but doesn't block a merge. To check before pushing, after `make test` has written `coverage.xml`: diff --git a/docs/generated/rules.md b/docs/generated/rules.md index fa317d484..e36b68366 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -370,7 +370,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 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 pull request's base commit, and CI complete needs it. | metric | enforced | yes | CI job `diff-coverage` | `pr.yml:diff-coverage` | yes | +| `api.quality.diff-coverage` | The lines a pull request changes in policyengine_api 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 base branch the merge commit CI checks out is built on (HEAD^1), 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` | — | diff --git a/policyengine_api/libs/__init__.py b/policyengine_api/libs/__init__.py new file mode 100644 index 000000000..be9992757 --- /dev/null +++ b/policyengine_api/libs/__init__.py @@ -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). +""" diff --git a/policyengine_api/routes/__init__.py b/policyengine_api/routes/__init__.py new file mode 100644 index 000000000..d90c4f213 --- /dev/null +++ b/policyengine_api/routes/__init__.py @@ -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). +""" diff --git a/pyproject.toml b/pyproject.toml index 481f85e8b..84e11f05e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -118,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. diff --git a/rules/registry.toml b/rules/registry.toml index dfac68887..47a15906d 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1445,7 +1445,7 @@ test = "tests/unit/test_codecov_coverage.py::test_rule_proof_coverage_measured" [[rule]] id = "api.quality.diff-coverage" category = "quality" -statement = "The lines a pull request changes in policyengine_api 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 pull request's base commit, and CI complete needs it." +statement = "The lines a pull request changes in policyengine_api 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 base branch the merge commit CI checks out is built on (HEAD^1), and CI complete needs it." rationale = "docs/engineering/skills/testing.md#changed-lines" scope = ["policyengine_api/**/*.py"] tier = "metric" From d91472a2ed44739b0c6cf85b06367a0c6be8a0c1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 19:43:19 +0400 Subject: [PATCH 5/8] Test reports from another checkout and every pragma spelling (failing) Review round 2 found that the coverage reports name files by an absolute source directory: a report written in another checkout directory, such as a container job's, matches no changed file, and diff-cover then passes with "No lines with coverage information". It also found that .coveragerc's `pragma: no cover` is narrower than coverage.py's default pattern, which the comment says it repeats. - New test_reports_match_the_changed_files_in_another_checkout measures a module with the repository's .coveragerc in one directory and runs the job's command in another checkout of the same change. It fails: "No lines with coverage information in this diff.", exit 0 (should be 50%, exit 1). - test_type_only_imports_and_pragmas_are_not_counted_as_missed adds the `#pragma:no cover` and `# PRAGMA NO COVER` spellings. It fails: lines 14 and 18 counted as missed. - The proof's below fixture is now 799 of 1,000 changed lines (79.9%), just below the minimum, and the at fixture 800 of 1,000. The proof passes: diff-cover truncates 79.9% to 79. - test_pull_requests_check_the_coverage_of_their_changed_lines also checks that pr.yml runs only on pull_request, which HEAD^1 relies on. It passes. Co-Authored-By: Claude Opus 5.5 --- .../at_threshold.toml | 8 +-- .../below_threshold.toml | 9 +-- tests/unit/test_diff_coverage.py | 56 ++++++++++++++++++- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/rules/fixtures/api.quality.diff-coverage/at_threshold.toml b/rules/fixtures/api.quality.diff-coverage/at_threshold.toml index faa4e3506..c84e9495d 100644 --- a/rules/fixtures/api.quality.diff-coverage/at_threshold.toml +++ b/rules/fixtures/api.quality.diff-coverage/at_threshold.toml @@ -1,6 +1,6 @@ # Proof fixture for api.quality.diff-coverage (at the minimum). # -# As below_threshold.toml, with 8 of the 10 changed lines covered: 80%, which -# diff-cover --fail-under=80 must accept. -changed_lines = 10 -covered_lines = 8 +# 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 diff --git a/rules/fixtures/api.quality.diff-coverage/below_threshold.toml b/rules/fixtures/api.quality.diff-coverage/below_threshold.toml index f3561ff56..56ecb8ce4 100644 --- a/rules/fixtures/api.quality.diff-coverage/below_threshold.toml +++ b/rules/fixtures/api.quality.diff-coverage/below_threshold.toml @@ -2,7 +2,8 @@ # # The proof test commits a module, then adds `changed_lines` lines to it in a # second commit, and writes a coverage report in which the first -# `covered_lines` of them ran. diff-cover --fail-under=80 must fail: 7 of 10 -# changed lines is 70%, the closest a 10-line change comes below 80%. -changed_lines = 10 -covered_lines = 7 +# `covered_lines` of them ran. diff-cover --fail-under=80 must fail: 799 of +# 1,000 changed lines is 79.9%, which diff-cover truncates to 79. A diff-cover +# that rounded the figure up would accept it. +changed_lines = 1000 +covered_lines = 799 diff --git a/tests/unit/test_diff_coverage.py b/tests/unit/test_diff_coverage.py index 36bfeafe3..516e77088 100644 --- a/tests/unit/test_diff_coverage.py +++ b/tests/unit/test_diff_coverage.py @@ -184,7 +184,7 @@ def test_rule_proof_diff_coverage(tmp_path: Path): at_minimum = _run_diff_cover(FIXTURES / "at_threshold.toml", tmp_path / "at") assert below.returncode != 0, below.stdout - assert "Coverage: 70%" in below.stdout + assert "Coverage: 79%" in below.stdout assert at_minimum.returncode == 0, at_minimum.stdout assert "Coverage: 80%" in at_minimum.stdout @@ -236,6 +236,44 @@ def test_a_line_counts_as_covered_if_any_report_covers_it(tmp_path: Path): assert "Coverage: 80%" in result.stdout +def test_reports_match_the_changed_files_in_another_checkout(tmp_path: Path): + # A report names each file by a source directory and the path below it. + # An absolute source written in another checkout directory, such as a + # container job's, matches no changed file, and diff-cover then passes with + # "No lines with coverage information". The pull request adds an untested + # function: one of its two changed lines runs. + base = "VALUE = 0\n" + head = base + "def untested():\n return VALUE\n" + measured = tmp_path / "measured" + (measured / "policyengine_api").mkdir(parents=True) + (measured / "policyengine_api" / "__init__.py").write_text("") + (measured / "policyengine_api" / "module.py").write_text(head) + (measured / "run.py").write_text("import policyengine_api.module\n") + report = tmp_path / "coverage.xml" + data_file = tmp_path / ".coverage" + rcfile = f"--rcfile={COVERAGE_CONFIG}" + _run_coverage(measured, data_file, "run", rcfile, "--branch", "run.py") + _run_coverage(measured, data_file, "xml", rcfile, "-q", "-o", str(report)) + checked = tmp_path / "checked" + (checked / "policyengine_api").mkdir(parents=True) + (checked / "policyengine_api" / "__init__.py").write_text("") + (checked / "policyengine_api" / "module.py").write_text(base) + _git(checked, "init", "--quiet") + _git(checked, "add", ".") + _git(checked, "commit", "--quiet", "-m", "base") + (checked / "policyengine_api" / "module.py").write_text(head) + _git(checked, "commit", "--quiet", "-am", "the pull request's change") + + result = _run_job_check( + checked, + {"test-coverage-shard-1/coverage.xml": report.read_text()}, + base_sha="unused", + ) + + assert result.returncode != 0, result.stdout + assert "Coverage: 50%" in result.stdout + + def test_coverage_reports_every_module_no_test_imports(tmp_path: Path): # diff-cover only counts the files a report lists. coverage.py lists a # module no test imported only if every directory between it and the @@ -261,7 +299,8 @@ def test_coverage_reports_every_module_no_test_imports(tmp_path: Path): def test_type_only_imports_and_pragmas_are_not_counted_as_missed(tmp_path: Path): # Imports under `if TYPE_CHECKING:` never run, and ruff's TC rules move # imports there, so counting them would fail a fully tested change. - # `pragma: no cover` keeps working too. + # coverage.py's default `pragma: no cover` pattern keeps working too, in + # each spelling it accepts. package = tmp_path / "policyengine_api" package.mkdir() (package / "__init__.py").write_text("") @@ -276,6 +315,14 @@ def test_type_only_imports_and_pragmas_are_not_counted_as_missed(tmp_path: Path) "\n" "def unused(): # pragma: no cover\n" " return VALUE\n" + "\n" + "\n" + "def also_unused(): #pragma:no cover\n" + " return VALUE\n" + "\n" + "\n" + "def unused_too(): # PRAGMA NO COVER\n" + " return VALUE\n" ) (tmp_path / "run.py").write_text("import policyengine_api.module\n") report = tmp_path / "coverage.json" @@ -287,7 +334,7 @@ def test_type_only_imports_and_pragmas_are_not_counted_as_missed(tmp_path: Path) module = json.loads(report.read_text())["files"]["policyengine_api/module.py"] assert module["missing_lines"] == [] - assert {4, 10} <= set(module["excluded_lines"]) + assert {4, 10, 14, 18} <= set(module["excluded_lines"]) def test_pull_requests_check_the_coverage_of_their_changed_lines(): @@ -301,6 +348,9 @@ def test_pull_requests_check_the_coverage_of_their_changed_lines(): # It runs after both kinds of tests, with no condition of its own: on a # stacked pull request above the lowest one they are skipped, so it is # skipped too, and CI complete fails there as it already does. + # HEAD^1 below is the base branch only in a pull_request event's merge + # checkout. (PyYAML reads the `on` key as True.) + assert workflow[True] == "pull_request" assert sorted(job["needs"]) == ["test", "v2-integration-check"] assert "if" not in job assert JOB in workflow["jobs"]["ci-complete"]["needs"] From abbfd7c4639f4b2a069ee6fedebf26e6f970b3fd Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 19:44:55 +0400 Subject: [PATCH 6/8] Record relative paths and coverage.py's whole pragma pattern - .coveragerc sets relative_files = True. The reports then name files by the relative source `policyengine_api`, so a report written in another checkout directory still matches the changed files; with absolute paths diff-cover found none and passed. - exclude_lines lists coverage.py's default pattern itself, written with `\#` because a value line that starts with `#` is a comment, instead of the narrower text `pragma: no cover`. - The rule statement, the testing skill and pr.yml's comment say that the lines counted are the ones coverage.py measures: each statement's first line. The skill also says a line with a branch that never ran still counts, which tests are measured, that moved code counts as changed, and how the local command keeps Python 3.12 and starts from no `.coverage`. - codecov.yml's patch comment says how Codecov's figure can differ from the job's. The two tests that failed now pass, with the other seven in tests/unit/test_diff_coverage.py. Co-Authored-By: Claude Opus 5.5 --- .coveragerc | 16 +++++++++++----- .github/workflows/pr.yml | 8 ++++---- codecov.yml | 6 +++++- docs/engineering/skills/testing.md | 24 ++++++++++++++++++++---- docs/generated/rules.md | 2 +- rules/registry.toml | 2 +- 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/.coveragerc b/.coveragerc index db2bcff7a..7a73b4625 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,13 +1,19 @@ [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] -# Setting exclude_lines replaces coverage.py's default, so it lists that -# default, `pragma: no cover`, again. Imports under `if TYPE_CHECKING:` never -# run, and ruff's TC rules move imports there, so counting them as missed -# would fail a fully tested change (api.quality.diff-coverage). +# 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:` never run, and ruff's TC rules +# move imports there, so counting them as missed would fail a fully tested +# change (api.quality.diff-coverage). exclude_lines = - pragma: no cover + \#\s*(pragma|PRAGMA)[:\s]?\s*(no|NO)\s*(cover|COVER) if TYPE_CHECKING: omit = tests/* diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0aaf4209b..77f14d069 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -388,10 +388,10 @@ jobs: diff-coverage: # Rule api.quality.diff-coverage: at least 80% of the lines a pull request - # changes in policyengine_api (.coveragerc's source) 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. + # 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: diff --git a/codecov.yml b/codecov.yml index bb17f5d4f..273bf9a87 100644 --- a/codecov.yml +++ b/codecov.yml @@ -35,7 +35,11 @@ coverage: patch: default: # pr.yml's diff-coverage job enforces this minimum for the changed - # lines (api.quality.diff-coverage); Codecov only shows it. + # 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 diff --git a/docs/engineering/skills/testing.md b/docs/engineering/skills/testing.md index 72f192785..e84c5d358 100644 --- a/docs/engineering/skills/testing.md +++ b/docs/engineering/skills/testing.md @@ -92,7 +92,8 @@ 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/` run in the unit or integration tests. The +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` @@ -112,12 +113,27 @@ 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 under `if TYPE_CHECKING:`, which never run, and lines marked `# pragma: no cover` don't count (`.coveragerc`). -Codecov's patch status shows the same 80% target but doesn't block a merge. -To check before pushing, after `make test` has written `coverage.xml`: +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. + +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 diff-cover coverage.xml --compare-branch=origin/master --fail-under=80 +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 diff --git a/docs/generated/rules.md b/docs/generated/rules.md index e36b68366..a5a50f1f4 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -370,7 +370,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 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 base branch the merge commit CI checks out is built on (HEAD^1), and CI complete needs it. | metric | enforced | yes | CI job `diff-coverage` | `pr.yml:diff-coverage` | 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 base branch the merge commit CI checks out is built on (HEAD^1), 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` | — | diff --git a/rules/registry.toml b/rules/registry.toml index 47a15906d..18679b415 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1445,7 +1445,7 @@ test = "tests/unit/test_codecov_coverage.py::test_rule_proof_coverage_measured" [[rule]] id = "api.quality.diff-coverage" category = "quality" -statement = "The lines a pull request changes in policyengine_api 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 base branch the merge commit CI checks out is built on (HEAD^1), and CI complete needs it." +statement = "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 base branch the merge commit CI checks out is built on (HEAD^1), and CI complete needs it." rationale = "docs/engineering/skills/testing.md#changed-lines" scope = ["policyengine_api/**/*.py"] tier = "metric" From 7293515904cbb842e2b3393d92f4cbe113adaa43 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 20:12:26 +0400 Subject: [PATCH 7/8] Test that stubs don't count and a mention of TYPE_CHECKING does (failing) Review round 3 found two problems with .coveragerc's exclude_lines: - `if TYPE_CHECKING:` matches anywhere in a line, so a comment that mentions it on a `def` line excludes the whole untested function. - The `...` bodies of Protocol methods never run and count as missed: 89 such lines are measured and never run in CI run 42046754813. test_type_only_imports_and_pragmas_are_not_counted_as_missed becomes test_only_lines_that_can_run_are_counted, with a Protocol method stub and an untested function whose comment mentions `if TYPE_CHECKING:`. It fails: missing lines are [23] (the stub) instead of [27] (the untested function). Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_diff_coverage.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_diff_coverage.py b/tests/unit/test_diff_coverage.py index 516e77088..59ff8eb19 100644 --- a/tests/unit/test_diff_coverage.py +++ b/tests/unit/test_diff_coverage.py @@ -296,16 +296,19 @@ def test_coverage_reports_every_module_no_test_imports(tmp_path: Path): assert sorted(modules - reported) == [] -def test_type_only_imports_and_pragmas_are_not_counted_as_missed(tmp_path: Path): - # Imports under `if TYPE_CHECKING:` never run, and ruff's TC rules move - # imports there, so counting them would fail a fully tested change. - # coverage.py's default `pragma: no cover` pattern keeps working too, in - # each spelling it accepts. +def test_only_lines_that_can_run_are_counted(tmp_path: Path): + # Lines that never run by design aren't counted as missed: + # - imports under `if TYPE_CHECKING:`, where ruff's TC rules move them; + # - `...` bodies of Protocol methods and other stubs; + # - lines marked with coverage.py's default `pragma: no cover` pattern, in + # each spelling it accepts. + # An untested function whose comment only mentions `if TYPE_CHECKING:` + # still counts: coverage.py excludes the whole block a matching line opens. package = tmp_path / "policyengine_api" package.mkdir() (package / "__init__.py").write_text("") (package / "module.py").write_text( - "from typing import TYPE_CHECKING\n" + "from typing import TYPE_CHECKING, Protocol\n" "\n" "if TYPE_CHECKING:\n" " from collections.abc import Iterator\n" @@ -323,6 +326,15 @@ def test_type_only_imports_and_pragmas_are_not_counted_as_missed(tmp_path: Path) "\n" "def unused_too(): # PRAGMA NO COVER\n" " return VALUE\n" + "\n" + "\n" + "class Reader(Protocol):\n" + " def read(self) -> int:\n" + " ...\n" + "\n" + "\n" + "def untested(): # not under if TYPE_CHECKING:\n" + " return VALUE\n" ) (tmp_path / "run.py").write_text("import policyengine_api.module\n") report = tmp_path / "coverage.json" @@ -333,8 +345,9 @@ def test_type_only_imports_and_pragmas_are_not_counted_as_missed(tmp_path: Path) _run_coverage(tmp_path, data_file, "json", rcfile, "-q", "-o", str(report)) module = json.loads(report.read_text())["files"]["policyengine_api/module.py"] - assert module["missing_lines"] == [] - assert {4, 10, 14, 18} <= set(module["excluded_lines"]) + assert module["missing_lines"] == [27] + assert {4, 10, 14, 18, 23} <= set(module["excluded_lines"]) + assert 26 not in module["excluded_lines"] def test_pull_requests_check_the_coverage_of_their_changed_lines(): From eedee597a214f86570c8f4ea2c2f430cb25a530d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 20:14:12 +0400 Subject: [PATCH 8/8] Anchor the TYPE_CHECKING exclusion and exclude stub bodies - .coveragerc's `if TYPE_CHECKING:` pattern now matches only the whole statement, with an optional comment. coverage.py searches each line of text, so the unanchored text also matched a comment on a `def` line and excluded the whole untested function. - A new pattern excludes `...` alone on a line: the bodies of Protocol methods and other stubs, which never run. Over the 346 modules under policyengine_api, the only change from the previous configuration is 89 such statements, in 16 files, leaving the count. - The testing skill lists the stub exclusion and says that git's rename detection pairs a new file at least 50% similar to a deleted one, so only its edited lines count. - The testing skill, pr.yml's comment and the rule statement say that for a pull request stacked on another, HEAD^1 is the lower one's merge commit: refs/pull/4200/merge (c0882ac1e) is built on refs/pull/4196/merge (5ff9f7252), not on quality/codecov-flags. test_only_lines_that_can_run_are_counted now passes, with the other eight in tests/unit/test_diff_coverage.py. Co-Authored-By: Claude Opus 5.5 --- .coveragerc | 17 +++++++++------ .github/workflows/pr.yml | 12 ++++++----- docs/engineering/skills/testing.md | 34 ++++++++++++++++++------------ docs/generated/rules.md | 2 +- rules/registry.toml | 2 +- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/.coveragerc b/.coveragerc index 7a73b4625..47b79eb3c 100644 --- a/.coveragerc +++ b/.coveragerc @@ -7,14 +7,19 @@ source = policyengine_api relative_files = True [report] -# 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:` never run, and ruff's TC rules -# move imports there, so counting them as missed would fail a fully tested -# change (api.quality.diff-coverage). +# 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) - if TYPE_CHECKING: + ^\s*if TYPE_CHECKING:\s*(\#.*)?$ + ^\s*\.\.\.\s*(\#.*)?$ omit = tests/* */tests/* diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 77f14d069..d639c865c 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -423,11 +423,13 @@ jobs: with: name: integration-coverage path: ${{ runner.temp }}/coverage/integration-coverage - # The checkout is GitHub's merge of the pull request into its base - # branch, so HEAD^1 is that branch as the merge saw it and 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. + # 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 diff --git a/docs/engineering/skills/testing.md b/docs/engineering/skills/testing.md index e84c5d358..9098d6e3a 100644 --- a/docs/engineering/skills/testing.md +++ b/docs/engineering/skills/testing.md @@ -93,17 +93,19 @@ the changed lines without blocking a merge. `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 GitHub's merge of the pull request into its -base branch, so `HEAD^1` is that branch as the merge saw it, and 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. +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 @@ -111,8 +113,10 @@ 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 under `if TYPE_CHECKING:`, which -never run, and lines marked `# pragma: no cover` don't count (`.coveragerc`). +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 @@ -122,7 +126,9 @@ of its branches never ran. Only the `Test` shards and the tests in 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. +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 diff --git a/docs/generated/rules.md b/docs/generated/rules.md index a5a50f1f4..4e97a071c 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -370,7 +370,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 base branch the merge commit CI checks out is built on (HEAD^1), and CI complete needs it. | metric | enforced | yes | CI job `diff-coverage` | `pr.yml:diff-coverage` | 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` | — | diff --git a/rules/registry.toml b/rules/registry.toml index 18679b415..a0d2b663f 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1445,7 +1445,7 @@ test = "tests/unit/test_codecov_coverage.py::test_rule_proof_coverage_measured" [[rule]] id = "api.quality.diff-coverage" category = "quality" -statement = "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 base branch the merge commit CI checks out is built on (HEAD^1), and CI complete needs it." +statement = "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." rationale = "docs/engineering/skills/testing.md#changed-lines" scope = ["policyengine_api/**/*.py"] tier = "metric"