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: run tests against PHP 8.2 #1175

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Apr 12, 2022

TL;DR

PHP 8.2 is likely to cause significant amounts of deprecation notices again in most projects.

As Mockery is a testing tool and projects will use their tests as a starting point to start fixing their code, it would be wonderful for Mockery to be PHP 8.2 ready early (before PHP 8.2-beta1).

PR #1168, #1170 and #1174 fix the bulk of the issues in Mockery itself as found via Mockery's own tests. As of now and with the WIP current state of PHP 8.2, there are only 11 failing tests left.

To keep on top of what additional work is and will be still needed to make Mockery compatible with PHP 8.2, I propose to start running the tests against PHP 8.2 in GitHub Actions.

Commit details

GH Actions: run tests against PHP 8.2

... for early warning about problems which need addressing.

For now, a failing test run against PHP 8.2 won't stop the build.

GH Actions: allow for manually triggering a workflow

Triggering a workflow for a branch manually is not supported by default in GH Actions, but has to be explicitly allowed.

This is useful if, for instance, an external action script or composer dependency has broken.
Once a fix is available, failing builds for open PRs can be retriggered manually instead of having to be re-pushed to retrigger the workflow.

Ref: https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/

GH Actions: turn on error reporting

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 -1 and display_errors=On to ensure all PHP notices are shown.

PHPUnit: update configuration

PHPUnit 9.5.10 and 8.5.21 were released a while ago and contain 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 all deprecation notices it encounters inline in the progress report 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:

... for early warning about problems which need addressing.

For now, a failing test run against PHP 8.2 won't stop the build.
Triggering a workflow for a branch manually is not supported by default in GH Actions, but has to be explicitly allowed.

This is useful if, for instance, an external action script or composer dependency has broken.
Once a fix is available, failing builds for open PRs can be retriggered manually instead of having to be re-pushed to retrigger the workflow.

Ref: https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/
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 `-1` and `display_errors=On` to ensure **all** PHP notices are shown.
PHPUnit 9.5.10 and 8.5.21 were released a while ago and contain 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 **all** deprecation notices it encounters inline in the progress report 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
@davedevelopment davedevelopment merged commit 8d128b0 into mockery:master Apr 14, 2022
@davedevelopment
Copy link
Collaborator

Amazing, thank you 👏

@jrfnl jrfnl deleted the feature/ghactions-enable-testing-against-php82 branch April 14, 2022 12:07
@jrfnl
Copy link
Contributor Author

jrfnl commented Apr 14, 2022

You're very welcome! Based on prelim test runs for various projects, I know a lot of projects will have an uphill battle to get ready for PHP 8.2, so it's in my own interest in the end if Mockery is ready early.

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.

None yet

2 participants