Repository navigation
feat: gate pull requests on changed-line coverage and report it - #66
Open
aryanjasala wants to merge 3 commits into
Open
aryanjasala wants to merge 3 commits into
aryanjasala wants to merge 3 commits into
Conversation
ci-test-php.yml and ci-test-js.yml take an opt-in coverage input (and wp-ci.yml forwards coverage and coverage-threshold). With it on, the tests run with coverage, @rtcamp/wp-tooling's coverage-gate checks the changed lines against the threshold and writes a job summary, and the report is uploaded as a coverage-php / coverage-js artifact. ci-coverage-comment.yml is a separate workflow that turns those artifacts into one pull request comment. It is its own opt-in because it needs pull-requests: write, and adding that to a job callers already use would fail validation for every caller that doesn't grant it. Uses @rtcamp/wp-tooling 2.0.0's coverage-gate, already on npm.
The coverage comment was the first one starting with a fixed marker, whoever wrote it, so it could overwrite a contributor's comment, and two caller workflows in one repository overwrote each other's. The marker now carries a hash of the caller's workflow name, and only a comment from github-actions[bot] is ever edited. Standalone runs set pcov.directory to the unit: PCOV otherwise records only src/, lib/ or app/ when one exists, and the starters keep PHP in inc/. base-ref and wp-tooling-ref now say they also drive the coverage gate, and that a branch is written origin/<branch>.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
The workflow half of #36. With
coverage: true, PHPUnit and Jest measure coverage,wp-tooling coverage-gatefails the job when too few of the lines the PR changed are covered, the HTML report is uploaded as an artifact, and an opt-in workflow posts the summary as a comment on the PR. With coverage off, every job runs the same steps as before.Closes
Closes #36
Changes
ci-test-php.ymlandci-test-js.ymlgaincoverage(defaultfalse),coverage-threshold(80),coverage-artifact-name,base-refandwp-tooling-ref. With coverage on:coverage/clover.xmlandcoverage/html(Xdebug inside wp-env, PCOV when standalone); Jest writescoverage/lcov.infoand its HTML report to an absolute--coverageDirectory, because Jest resolves it against arootDirthe starters move;coverage-gateruns with--output github: step outputs, annotations, and a per-file table of missed lines in the job summary, labelled with the job (PHPUnit, PHP 8.3, WP 6.8, plugins/acme);coverage/directory, including that table assummary.md, is uploaded even when the gate or the tests fail, which is when it is wanted most (14-day retention).wp-env and Xdebug. wp-env bakes Xdebug into the image it builds, and any later
wp-env startwithout--xdebugrebuilds it without. The starters run PHPUnit in a separate.wp-env.tests.jsonenvironment that theirpretest:phphook starts. So with coverage on, the workflow starts that environment itself with--xdebug=coverageand the default command becomesnpm run --ignore-scripts test:php --, which stops the hook from restarting it. Stop wp-env stops the same config.wp-ci.ymlgainscoverageandcoverage-threshold. It measurestest-jsand only the firsttest-phpmatrix cell (several cells would multiply the cost and could disagree), names the artifacts per unit (coverage-php,coverage-js, suffixed with the directory in a monorepo), forwardsbase-refandwp-tooling-ref, and fails fast invalidate-inputswhentest-php-excludedrops that first cell.New
ci-coverage-comment.ymlposts everysummary.mdof the run as one PR comment and updates it on later runs (or says nothing was measured). Fork PRs get a notice instead, since their token is read-only, and a refused comment (a Dependabot PR, say) is a warning, not a failure.Examples for all three workflows and a README Coverage section.
Review fixes (543bbc6):
github-actions[bot]comment is ever edited, so a contributor's comment that quotes the marker is left alone and two caller workflows in one repository keep separate comments (checked against a stubbedgh: it edits the bot's own comment, and posts a new one when only a human's or another workflow's exists);pcov.directoryto the unit, because PCOV otherwise records onlysrc/,lib/orapp/when one exists and the starters keep PHP ininc/(not yet run on a real runner);base-refandwp-tooling-refinwp-ci.ymlsay they also drive the coverage gate, and that a branch is writtenorigin/<branch>.How I verified
validate-inputswith coverage on: a plain matrix passes and emitscoverage-php-artifact=coverage-php;test-php-excludedropping the first cell ({"php":"8.2","wp":"6.7"}or just{"php":"8.2"}) fails with the error;working-dir: themes/acmegivescoverage-php-themes-acme.coverage-gatefrom@rtcamp/wp-tooling2.0.0 on a scratch repo, to confirm the summary format the label rewrite expects (### Changed-line coverageas the first line).ghon all four paths: new comment, update of an existing comment to "No coverage was measured", no summaries and no earlier comment (no call), fork PR (notice, no call).Not done yet, and needed before merge: the end-to-end check from #36 on a real consumer (a covered change passes, a later commit that adds an untested branch fails and names the lines, a new untested file shows at 0%, then the fix passes). Docker was unavailable when I built this, so the wp-env and PCOV paths have only been linted, not run.
Acceptance criteria
Runtime behavior
test-jsand the firsttest-phpcell onlyCode quality
adhoc-packagesnotes for the wp-tooling install, the same patternci-detect-changes.ymlalready uses)Housekeeping
CHANGELOG.mdedit: release-please writes it from the PR titleexamples/<workflow>.ymladded or updated to match any input changeyamllint,actionlintandbin/check-workflows.shpass locallyReviewer notes
Merge Dependabot feat(deps): bump actions/setup-node from 4.1.0 to 7.1.0 #54 (setup-node 7) before this PR. The new setup-node step in
ci-test-php.ymlsetspackage-manager-cache, which setup-node 4.1.0 doesn't know, so until feat(deps): bump actions/setup-node from 4.1.0 to 7.1.0 #54 every Test PHP run logs an "Unexpected input" warning. feat(deps): bump actions/setup-node from 4.1.0 to 7.1.0 #54 also conflicts with this file and needs a rebase either way.Why the comment is a separate workflow. A job that asks for
pull-requests: writefails validation for every caller that does not grant it, even when the job would be skipped. Putting it insidewp-ci.ymlwould break every@v1caller.--ignore-scriptsskips the consumer'spretest:php. That is deliberate, but any project that does more than start wp-env in that hook loses it under coverage. A customtest-commandmust forward its arguments to PHPUnit.Consumer side: theme-elementary's
test:phpusescomposer exec 'phpunit ...', which cannot take the appended flags. The starter PRs that turn coverage on fix that (-- vendor/bin/phpunit). New files only count oncephpunit.xml.disthasprocessUncoveredFiles(or a<source>include) and Jest hascollectCoverageFrom.Action pins: the new upload and download steps use
main's pins (upload-artifactv4.6.2,download-artifactv4.3.0). The Dependabot majors (feat(deps): bump the github-actions group across 1 directory with 2 updates #53 to feat(deps): bump actions/cache from 4.2.0 to 6.1.0 #59) move them with everything else; nothing here needs v7 or v8.Out of scope, as [ci-test] Enforce changed-line test coverage on every pull request commit #36 says: Codecov or any upload, whole-project thresholds, coverage on push events.