Add GitHub Action (GHA) for unit-tests and code-coverage.#38
Conversation
| libpng-dev \ | ||
| ${PHPIZE_DEPS} \ | ||
| && docker-php-ext-install gd \ | ||
| && pecl install xdebug-2.5.5 \ |
There was a problem hiding this comment.
🟡 Docker image fails to build for the PHP versions the project targets
The container build hard-codes an ancient coverage tool version (pecl install xdebug-2.5.5 at Dockerfile:9) that can only be compiled on PHP 7.1 and older, so building the image for any newer PHP version aborts with a compilation error.
Impact: Anyone building the Docker image for PHP 7.2 through 8.5 (the versions this project claims to support) gets a failed build and cannot run the tests in the container.
Version incompatibility between pinned Xdebug and modern PHP
The Dockerfile accepts an ARG PHP_VERSION (Dockerfile:1-2) so the image is intended to be built for any of the PHP versions in the test matrix (.github/workflows/tests.php.unit.yml:12-25, up to 8.5). However pecl install xdebug-2.5.5 (Dockerfile:9) pins Xdebug 2.5.5, which only supports PHP 5.5–7.1. For PHP 7.2+ (and especially PHP 8.x, which needs Xdebug 3.x) the PECL build fails, breaking docker build. Additionally ENV XDEBUG_MODE coverage (Dockerfile:15) is an Xdebug 3-only setting and is a no-op with the pinned 2.5.5.
Prompt for agents
The Dockerfile pins xdebug-2.5.5 via `pecl install xdebug-2.5.5`, but this version only compiles on PHP <= 7.1. Since the image is parameterized by ARG PHP_VERSION and the project's test matrix goes up to PHP 8.5, the build will fail for PHP 7.2+ (PHP 8.x requires Xdebug 3.x). Consider installing Xdebug without pinning an old version (e.g. `pecl install xdebug`) so PECL selects a version compatible with the target PHP, or select the Xdebug version based on PHP_VERSION. Note that ENV XDEBUG_MODE coverage only takes effect with Xdebug 3.x.
Was this helpful? React with 👍 or 👎 to provide feedback.
| include: | ||
| - php-version: '7.0' | ||
| allow_failure: true | ||
| - php-version: '7.1' | ||
| allow_failure: true | ||
| - php-version: '7.2' | ||
| allow_failure: true | ||
| - php-version: '7.3' | ||
| allow_failure: true | ||
| - php-version: '7.4' | ||
| allow_failure: true | ||
| - php-version: '8.0' | ||
| allow_failure: true | ||
| - php-version: '8.1' | ||
| allow_failure: true | ||
| - php-version: '8.2' | ||
| allow_failure: true | ||
| - php-version: '8.3' | ||
| allow_failure: true | ||
| - php-version: '8.4' | ||
| allow_failure: true | ||
| - php-version: '8.5' | ||
| allow_failure: true |
There was a problem hiding this comment.
🟡 Test failures on almost every supported PHP version do not fail the CI check
Every modern PHP version from 7.0 through 8.5 is marked as allowed to fail (allow_failure: true at .github/workflows/tests.php.unit.yml:26-48), leaving only the two oldest, end-of-life versions able to fail the build, so real breakages on supported versions still show a green check.
Impact: A change that breaks the library on any currently-used PHP version (7.0–8.5) will pass CI unnoticed, defeating the purpose of the test matrix.
continue-on-error covers the whole modern matrix
The include block (.github/workflows/tests.php.unit.yml:26-48) sets allow_failure: true for PHP 7.0, 7.1, 7.2, 7.3, 7.4, 8.0, 8.1, 8.2, 8.3, 8.4 and 8.5 — i.e. every version except 5.5 and 5.6. Combined with continue-on-error: ${{ matrix.allow_failure || false }} (.github/workflows/tests.php.unit.yml:49), only the PHP 5.5 and 5.6 jobs can actually turn the workflow red. Failures in all other jobs are ignored.
Prompt for agents
The workflow marks allow_failure: true for every PHP version from 7.0 through 8.5, so only the two oldest EOL versions (5.5, 5.6) can fail the build. This inverts the usual intent and means regressions on all supported/modern PHP versions produce a green check. Reconsider which versions should be allowed to fail — typically only bleeding-edge or explicitly-unsupported versions should be non-blocking, and the versions the library officially supports should be required.
Was this helpful? React with 👍 or 👎 to provide feedback.
No description provided.