From 196d7bc1e4473182cddda0b602b1e0f98f94b8dc Mon Sep 17 00:00:00 2001 From: Mark S Date: Mon, 14 Sep 2026 17:33:57 +1000 Subject: [PATCH] ci(coverage): enforce PR coverage floor in pytest, not via Codecov (LAB-3609) Add --cov-fail-under=82 to the final PR pytest invocation so the combined unit+critical total is gated inside the job, independent of the Codecov upload succeeding. A dropped/degraded upload (fork-PR tokenless 429, Codecov outage) can no longer answer the total-coverage question with a stale carryforward. Codecov keeps the finer-grained new-code (patch) status. - .github/workflows/ci.yml: floor on the --cov-append final run; residual-risk comment rewritten (total floor now local; Codecov owns patch). - Makefile: test-cov floor 60 -> 82, mirrors CI (full suite clears it). - CONTRIBUTING.md: document the enforced total floor. 82 = current main PR-path cumulative TOTAL 84.20% (py3.12, only PR matrix version) minus codecov.yml declared 2% tolerance, floored. --- .github/workflows/ci.yml | 28 ++++++++++++++++++++-------- CONTRIBUTING.md | 5 +++++ Makefile | 5 ++++- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01a6a079..9d508f7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -136,11 +136,18 @@ jobs: --cov-report=term \ --junitxml=junit-unit.xml \ -o junit_family=legacy + # --cov-fail-under evaluates the *combined* unit+critical total (the + # --cov-append accumulation), so it gates this final invocation only, and + # reddens the job in pytest independent of the Codecov upload (rationale: + # residual-risk note below). 82 = lowest same-repo PR cumulative TOTAL + # across the PR matrix minus codecov.yml's declared 2% tolerance, floored; + # keep it in step with the Makefile test-cov floor. uv run pytest tests/critical/ -m "not slow" \ --cov=src/cachekit \ --cov-append \ --cov-report=xml \ --cov-report=term \ + --cov-fail-under=82 \ --junitxml=junit.xml \ -o junit_family=legacy @@ -192,17 +199,22 @@ jobs: # what this ticket is hardening. The flag therefore applies exactly where OIDC # really authenticates. # - # ACCEPTED RESIDUAL RISK, stated because the scoping creates it: on a fork PR a - # dropped tokenless upload is still silent, and carryforward then answers the - # patch question with an earlier commit's numbers — a stale green on exactly the - # least-trusted contributions. The wrapper's CLI signature check is likewise + # RESIDUAL, now narrowed to the PATCH status: the TOTAL-coverage floor no + # longer depends on the upload — the final PR pytest run enforces it with + # `--cov-fail-under` (above), so a dropped or degraded upload can no longer + # answer the total-coverage question with an earlier commit's carryforward; + # the job reddens inside pytest first, on same-repo and fork PRs alike. What + # Codecov still solely owns is the new-code PATCH status: it is diff-level, + # stays `carryforward: true`, and is not enforced locally, so on a fork PR + # whose tokenless upload is silently dropped it can still answer "is this + # PR's new code 80% covered?" with a previous run's numbers. Accepted: that + # status is a reporting signal, not a merge floor, and the local total floor + # already catches a gross coverage regression on the same run. + # The wrapper's CLI signature check is likewise # unenforced there (fail_ci_if_error is its switch; see the pin note below). # Accepted because a fork PR cannot reach the self-hosted `cachekit` runner # without a maintainer approving the run, no fork PR has run here to date, and - # a tampered binary reaches same-repo runs first, where it fails closed. The real - # fix is to stop depending on an external upload for the floor (a local - # `--cov-fail-under` on the PR pytest invocation); that is a coverage-policy - # change, tracked separately rather than smuggled into this diff. + # a tampered binary reaches same-repo runs first, where it fails closed. # # Do not re-pin below v7.0.0 (fb8b3582): releases published before Codecov's # June 2026 keybase migration fetch the CLI signing key from a deleted account, diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8d6af6e9..97977b47 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -194,6 +194,11 @@ make test-cov - Aim for >85% coverage for new code - All public APIs must have tests - Edge cases and error conditions must be tested +- CI enforces a **total-coverage floor** inside pytest on every PR + (`--cov-fail-under`, value in `.github/workflows/ci.yml`), independent of the + Codecov upload — a PR whose combined coverage falls below it fails the `Tests` + job. `make test-cov` enforces the same floor locally. Codecov still reports the + finer-grained new-code (patch) coverage on top of this. **Rust Test Coverage**: - ByteStorage module: 82% coverage (measured via LLVM source-based coverage) diff --git a/Makefile b/Makefile index 5e25569f..d3c41cb9 100644 --- a/Makefile +++ b/Makefile @@ -125,6 +125,9 @@ test-quick: setup-logs ## Run fast tests (skip slow tests) fi @echo "$(GREEN)✓ Fast tests completed$(RESET)" +# --cov-fail-under mirrors the PR coverage floor in .github/workflows/ci.yml. +# This target runs the full suite (>= the PR path's unit+critical coverage), so +# it clears the floor with margin. Keep the two values in step. test-cov: setup-logs ## Run tests with coverage @echo "$(BLUE)Running tests with coverage...$(RESET)" @echo "$(YELLOW)Logging to $(LOG_TEST_DIR)/cov_$(TIMESTAMP).log$(RESET)" @@ -132,7 +135,7 @@ test-cov: setup-logs ## Run tests with coverage --cov=$(PACKAGE_NAME) \ --cov-report=term-missing \ --cov-report=html:reports/htmlcov \ - --cov-fail-under=60 \ + --cov-fail-under=82 \ -q \ --tb=short \ --basetemp=$(TEST_BASETEMP) 2>&1 | tee $(LOG_TEST_DIR)/cov_$(TIMESTAMP).log; then \