thephpleague / oauth2-client

Easy integration with OAuth 2.0 service providers.
http://oauth2-client.thephpleague.com
MIT License
3.65k stars 751 forks source link

GH Actions: turn on error reporting #915

Closed jrfnl closed 2 years ago

jrfnl commented 3 years ago

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:

codecov[bot] commented 2 years ago

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 commented 2 years ago

Yay! Build fails on 8.1 because of deprecation warnings. šŸ™„

jrfnl commented 2 years ago

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 ? šŸ˜‡