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

Composer: allow PHPUnit 10.x + update CI to match #130

Merged
merged 4 commits into from
Jun 6, 2023

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Jun 6, 2023

Composer: allow for PHPUnit 10.0.0

PHPUnit 10.0 has been released on Feb 3, 2023 and in the mean time PHPUnit 10.1 and 10.2 have also been released.

Refs:

TestListenerTest: ignore on PHPUnit 10

The initial PHPUnit Polyfills 2.0.0 release will not contain a PHPUnit 10 compatible TestListener implementation.

This allows the tests to pass on PHPUnit 10 for now until that has been resolved.

Refs:

CI: update for PHPUnit 10.0.0 support

As the PHPUnit Polyfills, as of now, will officially support PHPUnit 10.x, with the exception of the TestListeners, the GH Actions workflow should be updated to reflect this.

This commit:

  • Add builds for PHP 8.1 and 8.2 against low PHPUnit 10 versions.
    The "high" versions are automatically sorted via the matrix auto setting in combination with the Composer PHPUnit version requirements being widened.
  • Add an extra experimental build against PHP 8.3 to ensure both PHPUnit 9.x as well as PHPUnit 10.x is tested with PHP 8.3.
  • Update the experimental build against "future" PHPUnit to point to the main branch of PHPUnit.

Additionally, it updates the step which ran a "trial" build against PHPUnit 10 by splitting it into two steps: one with and one without coverage. Both are still allowed to fail when running the experimental build against PHPUnit dev-main.

Tests: fail test runs on deprecations/notices/warnings

As this package is used in the CI pipeline for other packages, it is important for this package to be ready for new PHP releases early, so failing the test suite on deprecations/notices and warnings is appropriate.

Apparently PHPUnit 10.0 already included a failOnWarning option, while PHPUnit 10.1 brings additional failOnNotice and failOnDeprecation options.

Unfortunately, these options are problematic for cross-version PHPUnit usage for the following reasons:

  1. They do not translate to the same behaviour between PHPUnit < 10 and PHPUnit 10.x.
    In PHPUnit 9.x, the convertDeprecationsToExceptions and sister-options would only act on PHP native deprecations/notices/warnings and would cause a failed build with a non-zero exit code when set to true and any PHP deprecations/notices/warnings would be found.
    In PHPUnit 10.x, the failOnWarning and sister-options act on all deprecations/notices/warnings, not just the PHP native ones, and cause a failed build with a non-zero exit code when set to true.
    In practice, this means that when the failOn* options are used, PHPUnit native deprecations/notices/warnings will now also fail a build, which was not the case in PHPUnit < 10.
  2. As the XML file is validated against the XSD file, this means that you cannot add the failOnNotice/failOnDeprecation options in a configuration file which will be used on PHPUnit 10.0 as that would fail the XSD validation, leading to a PHPUnit warning which would fail the build.
  3. Along the same lines, as the include/exclude elements within the coverage element are deprecated as of PHPUnit 10.1, in favour of using source with include/exclude, a configuration which validates on PHPUnit 10.0 will not validate correctly on PHPUnit 10.1 leading to a deprecation notice, which would fail the build when failOnDeprecation is on.
    ... etc...

This basically means that if you want PHPUnit to fail a build on PHP native deprecations/notices/warnings (which you should want for open source projects, most definitely open source projects in the PHP tool chain), you will now no longer be allowed to have any PHPUnit deprecations/notices/warnings as those will now also fail the build.

It also means that a new release of PHPUnit can randomly start failing builds if a change was made in PHPUnit which causes a new deprecation/notice/warning to be thrown.

All in all, not pretty and it makes things a heck of a lot less stable, but it is what it is for now, so we better deal with it.

So, to get round this and to have the most optimal configuration for PHPUnit 10.x possible at this time....

  1. The failOnWarning option is added to the phpunit10.xml.dist file as it is already available in PHPUnit 10.0.
  2. A new "Migrate configuration" step needs to be added to the GH Actions test workflow.
    This step will migrate the phpunit10.xml.dist file, which is compatible with PHPUnit 10.0, to a version which is compatible with whichever PHPUnit 10.x version the tests are being run on.
    This step will fail when there is nothing to migrate, so it needs a continue-on-error to ignore that failure.
    Note: the migration step will also fail when the original XML file doesn't validate against the original XSD, which is why the base configuration needs to be for PHPUnit 10.0.
  3. As for the failOnNotice/failOnDeprecation options: those cannot be added to the config for the above mentioned reasons (failure to validate against the PHPUnit 10.0 XSD), but they also cannot be added to the "default" command used for running the tests on PHPUnit 10 as a run against PHPUnit 10.0 would then fail with an "Unknown option ...." error.
    In other words, the only realistic way to add these, for now, is to add them to the command on the fly for those test runs where PHPUnit 10.1 or higher will be used.

Refs:

jrfnl added 4 commits June 6, 2023 15:07
The initial PHPUnit Polyfills 2.0.0 release will not contain a PHPUnit 10 compatible TestListener implementation.

This allows the tests to pass on PHPUnit 10 for now until that has been resolved.

Refs:
* #95 (comment)
* #128
As the PHPUnit Polyfills, as of now, will officially support PHPUnit 10.x, with the exception of the TestListeners, the GH Actions workflow should be updated to reflect this.

This commit:
* Add builds for PHP 8.1 and 8.2 against low PHPUnit 10 versions.
    The "high" versions are automatically sorted via the matrix `auto` setting in combination with the Composer PHPUnit version requirements being widened.
* Add an extra experimental build against PHP 8.3 to ensure both PHPUnit 9.x as well as PHPUnit 10.x is tested with PHP 8.3.
* Update the experimental build against "future" PHPUnit to point to the `main` branch of PHPUnit.

Additionally, it updates the step which ran a "trial" build against PHPUnit 10 by splitting it into two steps: one with and one without coverage. Both are still allowed to fail when running the experimental build against PHPUnit `dev-main`.
As this package is used in the CI pipeline for other packages, it is important for this package to be ready for new PHP releases _early_, so failing the test suite on deprecations/notices and warnings is appropriate.

Apparently PHPUnit 10.0 already included a `failOnWarning` option, while PHPUnit 10.1 brings additional `failOnNotice` and `failOnDeprecation` options.

Unfortunately, these options are problematic for cross-version PHPUnit usage for the following reasons:
1. They do not translate to the same behaviour between PHPUnit < 10 and PHPUnit 10.x.
    In PHPUnit 9.x, the `convertDeprecationsToExceptions` and sister-options would only act on PHP native deprecations/notices/warnings and would cause a failed build with a non-zero exit code when set to `true` and any PHP deprecations/notices/warnings would be found.
    In PHPUnit 10.x, the `failOnWarning` and sister-options act on **_all_** deprecations/notices/warnings, not just the PHP native ones, and cause a failed build with a non-zero exit code when set to `true`.
    In practice, this means that when the `failOn*` options are used, PHPUnit native deprecations/notices/warnings will now also fail a build, which was not the case in PHPUnit < 10.
2. As the XML file is validated against the XSD file, this means that you cannot add the `failOnNotice`/`failOnDeprecation` options in a configuration file which will be used on PHPUnit 10.0 as that would fail the XSD validation, leading to a PHPUnit warning which would fail the build.
3. Along the same lines, as the `include`/`exclude` elements within the `coverage` element are deprecated as of PHPUnit 10.1, in favour of using `source` with `include`/`exclude`, a configuration which validates on PHPUnit 10.0 will not validate correctly on PHPUnit 10.1 leading to a deprecation notice, which would fail the build when `failOnDeprecation` is on.
... etc...

This basically means that if you want PHPUnit to fail a build on PHP native deprecations/notices/warnings (which you should want for open source projects, most definitely open source projects in the PHP tool chain), you will now no longer be allowed to have **_any_** PHPUnit deprecations/notices/warnings as those will now also fail the build.

It also means that a new release of PHPUnit can randomly start failing builds if a change was made in PHPUnit which causes a new deprecation/notice/warning to be thrown.

All in all, not pretty and it makes things a heck of a lot less stable, but it is what it is for now, so we better deal with it.

So, to get round this and to have the most optimal configuration for PHPUnit 10.x possible at this time....
1. The `failOnWarning` option is added to the `phpunit10.xml.dist` file as it is already available in PHPUnit 10.0.
2. A new "Migrate configuration" step needs to be added to the GH Actions test workflow.
    This step will migrate the `phpunit10.xml.dist` file, which is compatible with PHPUnit 10.0, to a version which is compatible with whichever PHPUnit 10.x version the tests are being run on.
    This step will fail when there is nothing to migrate, so it needs a `continue-on-error` to ignore that failure.
    Note: the migration step will also fail when the original XML file doesn't validate against the original XSD, which is why the base configuration needs to be for PHPUnit 10.0.
3. As for the `failOnNotice`/`failOnDeprecation` options: those cannot be added to the config for the above mentioned reasons (failure to validate against the PHPUnit 10.0 XSD), but they also cannot be added to the "default" command used for running the tests on PHPUnit 10 as a run against PHPUnit 10.0 would then fail with an "Unknown option ...." error.
    In other words, the only realistic way to add these, for now, is to add them to the command on the fly for those test runs where PHPUnit 10.1 or higher will be used.

Refs:
* sebastianbergmann/phpunit 5196
* sebastianbergmann/phpunit@fb6673f
@jrfnl jrfnl added this to the 2.0.0 milestone Jun 6, 2023
@coveralls
Copy link

Coverage Status

coverage: 96.247%. remained the same when pulling 2462abe on 2.0/phpunit-10/update-environment-and-ci into c6e39f3 on 2.x.

@jrfnl jrfnl merged commit 113223f into 2.x Jun 6, 2023
76 checks passed
@jrfnl jrfnl deleted the 2.0/phpunit-10/update-environment-and-ci branch June 6, 2023 13:27
@jrfnl jrfnl mentioned this pull request Jun 6, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants