feat(php-tests)!: emit text + Clover XML coverage, per-format booleans, drop coverage-command#18
Open
kojiromike wants to merge 4 commits into
Open
feat(php-tests)!: emit text + Clover XML coverage, per-format booleans, drop coverage-command#18kojiromike wants to merge 4 commits into
kojiromike wants to merge 4 commits into
Conversation
Default coverage-command now writes coverage.txt and clover.xml into coverage-report/ alongside the HTML report, so consumers (PR comments, agents, CI tools) can read coverage without scraping HTML. Assisted-by: Claude Code
Add coverage-html, coverage-text, coverage-clover (all default true) so callers can opt out of any format. The default coverage-command becomes empty and is composed from the booleans; setting coverage-command still overrides everything as an escape hatch. Assisted-by: Claude Code
The booleans (coverage-html, coverage-text, coverage-clover) are now the entire API for choosing which coverage outputs to emit. Removing the arbitrary-shell-eval escape hatch eliminates a foot-gun: callers can no longer accidentally interpolate untrusted GitHub context into a shell string, and the workflow no longer eval's a free-form input. Callers that need to customize PHPUnit (config path, suites, listeners, bootstrap, coverage exclusions, etc.) configure those in phpunit.xml, which PHPUnit auto-discovers. BREAKING CHANGE: The coverage-command input has been removed. Callers that set it must remove the input and move any custom PHPUnit configuration into phpunit.xml. Assisted-by: Claude Code
POSIX-canonical form in [[ ]] tests; reserve == for arithmetic inside (( )). Assisted-by: Claude Code
kojiromike
changed the title
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
1 participant
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.
Summary
coverage-report/: HTML report,coverage.txt(PHPUnit text report),clover.xml(machine-readable line coverage) for cheap agent / CI consumption.coverage-html,coverage-text,coverage-clover(all defaulttrue) — let callers opt out of any format. At least one must be true.coverage-commandis removed. The booleans are now the entire API for choosing outputs. Callers who need to customize PHPUnit (config path, suites, listeners, bootstrap, coverage filters, etc.) configure those inphpunit.xml, which PHPUnit auto-discovers.Pairs with openCoreEMR/github-workflows-internal#33 (surfaces the text Summary in the PR comment and links the artifact).
Why drop coverage-command
The previous design
eval'd an arbitrary string input. Even though the input is caller-trusted in GHA's model, the escape hatch invites future foot-shooting (e.g., a caller accidentally interpolating untrusted context). The booleans cover the common cases;phpunit.xmlcovers the rest.Test plan
uses:on a real caller; confirmcoverage-report/{coverage.txt,clover.xml,index.html}all appear in the uploaded artifact.coverage-clover: falseand confirmclover.xmlis absent.falseand confirm the step fails fast with a clear error.phpunit.xml(suites, exclusions) still picks it up via auto-discovery.