Repository navigation
test: report PHP line coverage of the integration tests - #276
Merged
Merged
Conversation
The wrapper copied the embedded phar to the phar path on init, including a
path given by {ENV_PREFIX}PHAR_PATH, so the custom phar was replaced before it
ran. It now copies the embedded phar only to its default path in the cache
directory.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The integration tests' coverage report counts Go statements, but most of the CLI's behavior is in the legacy PHP code. make test-integration-php-coverage builds a CLI that runs the legacy source on the system PHP with PCOV, through a wrapper set at build time (legacy.PHPPath), and reports line coverage by directory, with an HTML report. A new CI job runs it, and writes the summary to the job summary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![upsun-dispatch[bot]](https://avatars.githubusercontent.com/in/4153575?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 1 warning · 🔵 1 minor point
🔍 Full review · 8 files reviewed
Verification
initnow skips the embedded-phar copy only when{PREFIX}PHAR_PATHis set, andpharPathstill returns the cache path otherwise, so release behavior is unchanged.PHPPathchanges only the binary inmakeCmd. The-nflag is still the first argument, which is why the shell wrapper's$1 = -ncheck works (Unixsettings()returns nil).- The integration tests get
TEST_CLI_PHAR_PATHfromos.Environ()intestEnv, and the CLI reads it as its{PREFIX}PHAR_PATH, because the test config'senv_prefixisTEST_CLI_. TestInitKeepsCustomPharwould have failed before the fix:initused to write the embedded phar (an empty placeholder in CI) over the custom path.report.phpcalls only php-code-coverage 11.0.12 APIs that exist in that version (abstractDriverstart/stop/nameAndVersion,Filter::includeFile,includeUncoveredFiles).
The new Go unit test TestInitKeepsCustomPhar covers the PHAR_PATH init fix, and the CI test job runs it via make test-go. Nothing tests the coverage scripts (php, prepend.php, report.php) except the new integration-test-php-coverage CI job itself.
Review details
- Commit: 139b570
- Model: claude-opus-5-5
Review 1 of 10 for this pull request · View the full run
… coverage dir The summary had no rows for files directly in src/ or src/Command/, such as CommandBase.php. They are now reported as "*.php" and "Command/*.php". The prepend file wrote with tempnam(), which falls back to the system temp directory when PHP_COVERAGE_DIR does not exist, so data was lost without an error. It now fails if the directory is missing, or if the write fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
The integration tests' coverage report counts Go statements, but most of the CLI's behavior is in the legacy PHP code. This adds PHP line coverage of the integration tests, to show which legacy behavior is untested before it is ported to Go.
make test-integration-php-coveragebuilds a CLI withlegacy.PHPPathset at build time tolegacy/scripts/coverage/php. That wrapper runs the legacy source (TEST_CLI_PHAR_PATH=legacy/bin/platform) on the system PHP with PCOV, and each process writes its coverage toPHP_COVERAGE_DIR. Release builds are unchanged.legacy/scripts/coverage/report.phpmerges the data, prints a Markdown summary by directory, and writes an HTML report todist/php-coverage/html.integration-test-php-coverage, writes the summary to the job summary and uploads the HTML report. The main integration job still runs the embedded PHP.Locally the tests cover 64.2% of the lines in
legacy/src.It also fixes a bug that this setup exposed: the wrapper copied the embedded phar to the
{ENV_PREFIX}PHAR_PATHpath on init, which overwrote the custom phar before it ran.🤖 Generated with Claude Code