Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests: fail on deprecation notices #265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jrfnl
Copy link

@jrfnl jrfnl commented Sep 12, 2024

This (small) change needs a little explaining....

First of all, the setup-php action, by default uses a "production"-type ini file. This means that error_reporting is set to E_ALL & ~E_DEPRECATED & ~E_STRICT and display_errors is set to Off.

For the purposes of CI, I'd recommend running with -1 (always include all error levels on all PHP versions) and display_errors=On to ensure all PHP notices are shown.

This is now fixed by adding the ini-values setting to setup-php.

I've also made minor other tweaks to the setup-php config:

  • tools: composer is not needed as Composer is installed by default.
  • extension: xdebug is not the correct way to enable xdebug for code coverage. coverage: xdebug is (and is Xdebug cross-version compatible for Xdebug 2 and 3).

Second of all, a change was made in PHPUnit 8.5.21 and 9.5.10, which changes the default value of the PHPUnit convertDeprecationsToExceptions attribute from true to false, meaning that tests would no longer fail on deprecation notices.

Not great for open source packages which generally need to handle deprecations in a timely matter so as not to block other packages.

So adding the convertDeprecationsToExceptions attribute and explicitly setting the value to true brings back the old behaviour and will allow for failing the tests on deprecation notices.

This (small) change needs a little explaining....

First of all, the `setup-php` action, by default uses a "production"-type ini file. This means that `error_reporting` is set to `E_ALL & ~E_DEPRECATED & ~E_STRICT` and `display_errors` is set to `Off`.

For the purposes of CI, I'd recommend running with `-1` (always include all error levels on all PHP versions) and `display_errors=On` to ensure **all** PHP notices are shown.

This is now fixed by adding the `ini-values` setting to `setup-php`.

I've also made minor other tweaks to the `setup-php` config:
* `tools: composer` is not needed as Composer is installed by default.
* `extension: xdebug` is not the correct way to enable `xdebug` for code coverage. `coverage: xdebug` is (and is Xdebug cross-version compatible for Xdebug 2 and 3).

Second of all, a change was made in PHPUnit 8.5.21 and 9.5.10, which changes the default value of the PHPUnit `convertDeprecationsToExceptions` attribute from `true` to `false`, meaning that tests would no longer fail on deprecation notices.

Not great for open source packages which generally need to handle deprecations in a timely matter so as not to block other packages.

So adding the `convertDeprecationsToExceptions` attribute and explicitly setting the value to `true` brings back the old behaviour and will allow for failing the tests on deprecation notices.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant