Skip to content

CI: enforce code coverage threshold (PCOV) on the rule package #9

@Goosterhof

Description

@Goosterhof

The gap

v0.1.0 shipped with a self-flagged debt in CHANGELOG: "Test coverage is smoke-level for v0.1.0; full matrix for EnforceActionTransactionsRule (non-DB property exclusions, nested closure transaction detection, full 18-method write list) lands in a follow-up." That follow-up has not landed; we now have six rule artifacts (5 rules + 1 type extension) with no coverage measurement.

phpunit.xml.dist already declares <source><include><directory>src</directory></include></source> — coverage source is wired, just no driver loaded and no threshold gate. CI runs with coverage: none (faster, but no signal).

Why a coverage gate is high-leverage for this package specifically

Rule code is the most coverage-friendly code in any codebase: deterministic, branch-heavy, no side effects, fixtures are tiny, no I/O. The PHPStan RuleTestCase harness drives every branch the rule exposes via fixture analysis. A 95%+ threshold is a realistic target, not an aspirational one — it would mean the next rule we ship can't merge if a fixture-asserted branch is silently dead.

Without it, the failure mode is silent: a rule's processNode early-returns on a node-shape we never wrote a fixture for, the test suite passes, the consumer hits the unfired-branch case in production, and the doctrine quietly leaks.

What to add

  1. CI driver — switch coverage: nonecoverage: pcov in .github/workflows/ci.yml. PCOV is faster than Xdebug and built for line/branch coverage measurement only (we don't need Xdebug's debugger features). shivammathur/setup-php@v2 supports PCOV natively. Apply across both 8.4 and 8.5 matrix legs.
  2. PHPUnit threshold — add <coverage> block with <report> and a clover/cobertura output for CI tooling, plus --fail-on-incomplete semantics. Use PHPUnit 11's --coverage-clover + a --coverage-min threshold (or fall back to a coverage-check gate parsing the clover output if PHPUnit's native min flag is too coarse for our needs).
  3. Threshold value — start at 90%; ratchet to 95% once we've audited what the missing 10% actually is. Don't ship 100% — false confidence; some defensive branches (Laravel $node->class instanceof Expr fallback in the new LogRule static-call branch) are genuinely hard to fixture and the cost of a fake fixture exceeds the value of the green check.
  4. Composer scriptcomposer test:coverage for local invocation; CI runs it directly.
  5. Report artifact — upload clover.xml as a CI artifact so we can review uncovered lines on PRs without spelunking through workflow logs.

Deliberation

Open question 1: line coverage vs branch coverage. PHPUnit 11 + PCOV supports both. Branch coverage is stricter — would surface things like "every instanceof MethodCall branch returned errors but the instanceof StaticCall branch never returned errors in any fixture". Cost: PCOV branch coverage was experimental last time I checked (~2024); may have stabilised. Lean: line coverage v1, evaluate branch coverage as a follow-up after the line gate is bedded in.

Open question 2: should fixture files count toward coverage source? Currently <include>src</include> excludes them — correct. Fixtures aren't production code, they exist to be analysed by the rules under test. Keep the exclusion explicit.

Open question 3: pair this with Infection mutation testing or land standalone? Coverage proves the line ran; mutation proves the line did something. They reinforce each other. Filing Infection as a separate issue (#X) — recommend landing coverage first, then Infection on top, so we can compare "95% line coverage" against "X% mutation score" cleanly.

Versioning implication

None — pure CI / test-infra change. No consumer-visible behaviour shift. Lands in the next [Unreleased] entry as Changed → CI without a SemVer bump on its own.

References

  • Origin self-flag: CHANGELOG.md v0.1.0 §Notes — "Test coverage is smoke-level... lands in a follow-up."
  • Sister gate (frontend): fs-packages 8-gate CI ships at 100% line coverage + 90% mutation per the war-room CLAUDE.md territory entry. This package's stack is different (no build, no bundle), so 8-gate parity is not the goal — pull the gates that actually serve a single-purpose static-analysis library.
  • ADR-0021: https://adrs.script.nl/decisions/phpstan-rules-package

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions