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

GH Actions: turn on error reporting #915

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Oct 28, 2021

GH Actions: set error reporting to E_ALL

The default setting for error_reporting used by the SetupPHP action is error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT and display_errors is set to Off.

For the purposes of CI, I'd recommend running with E_ALL and display_errors=On to ensure all PHP notices are shown.

PHPUnit: update configuration

PHPUnit recently released version 9.5.10 and 8.5.21.

This contains a particular (IMO breaking) change:

  • PHPUnit no longer converts PHP deprecations to exceptions by default (configure convertDeprecationsToExceptions="true" to enable this)

Let's unpack this:

Previously (PHPUnit < 9.5.10/8.5.21), if PHPUnit would encounter a PHP native deprecation notice, it would:

  1. Show a test which causes a deprecation notice to be thrown as "errored",
  2. Show the first deprecation notice it encountered and
  3. PHPUnit would exit with a non-0 exit code (2), which will fail a CI build.

As of PHPUnit 9.5.10/8.5.21, if PHPUnit encounters a PHP native deprecation notice, it will no longer do so. Instead PHPUnit will:

  1. Show a test which causes a PHP deprecation notice to be thrown as "risky",
  2. Show the all deprecation notices it encountered and
  3. PHPUnit will exit with a 0 exit code, which will show a CI build as passing.

This commit reverts PHPUnit to the previous behaviour by adding convertDeprecationsToExceptions="true" to the PHPUnit configuration.
It also adds the other related directives for consistency.

Refs:

jrfnl added 2 commits October 28, 2021 15:15
The default setting for `error_reporting` used by the SetupPHP action is `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT` and `display_errors` is set to `Off`.

For the purposes of CI, I'd recommend running with `E_ALL` and `display_errors=On` to ensure **all** PHP notices are shown.
PHPUnit recently released version 9.5.10 and 8.5.21.

This contains a particular (IMO breaking) change:

> * PHPUnit no longer converts PHP deprecations to exceptions by default (configure `convertDeprecationsToExceptions="true"` to enable this)

Let's unpack this:

Previously (PHPUnit < 9.5.10/8.5.21), if PHPUnit would encounter a PHP native deprecation notice, it would:
1. Show a test which causes a deprecation notice to be thrown as **"errored"**,
2. Show the **first** deprecation notice it encountered and
3. PHPUnit would exit with a **non-0 exit code** (2), which will fail a CI build.

As of PHPUnit 9.5.10/8.5.21, if PHPUnit encounters a PHP native deprecation notice, it will no longer do so. Instead PHPUnit will:
1. Show a test which causes a PHP deprecation notice to be thrown as **"risky"**,
2. Show the **all** deprecation notices it encountered and
3. PHPUnit will exit with a **0 exit code**, which will show a CI build as passing.

This commit reverts PHPUnit to the previous behaviour by adding `convertDeprecationsToExceptions="true"` to the PHPUnit configuration.
It also adds the other related directives for consistency.

Refs:
* https://github.com/sebastianbergmann/phpunit/blob/9.5/ChangeLog-8.5.md
* https://github.com/sebastianbergmann/phpunit/blob/9.5/ChangeLog-9.5.md
@jrfnl jrfnl mentioned this pull request Oct 28, 2021
@codecov
Copy link

codecov bot commented Nov 29, 2021

Codecov Report

Merging #915 (dc490fe) into master (535de5b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##              master      #915   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       180       180           
===========================================
  Files             20        20           
  Lines            464       464           
===========================================
  Hits             464       464           

@ramsey
Copy link
Contributor

ramsey commented Nov 30, 2021

Yay! Build fails on 8.1 because of deprecation warnings. 🙄

@ramsey ramsey merged commit 3a66916 into thephpleague:master Nov 30, 2021
@jrfnl jrfnl deleted the feature/ghactions-turn-on-error-reporting branch November 30, 2021 07:59
@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 30, 2021

Yay! Build fails on 8.1 because of deprecation warnings. 🙄

And aren't you glad those are now fixed and nothing to worry about anymore ? 😇

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.

2 participants