-
Notifications
You must be signed in to change notification settings - Fork 13
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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: * https://phpunit.de/announcements/phpunit-10.html * https://github.com/sebastianbergmann/phpunit/blob/10.0.19/ChangeLog-10.0.md * https://github.com/sebastianbergmann/phpunit/blob/10.1.3/ChangeLog-10.1.md * https://github.com/sebastianbergmann/phpunit/blob/10.2/ChangeLog-10.2.md
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
The "high" versions are automatically sorted via the matrix
auto
setting in combination with the Composer PHPUnit version requirements being widened.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 additionalfailOnNotice
andfailOnDeprecation
options.Unfortunately, these options are problematic for cross-version PHPUnit usage for the following reasons:
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 totrue
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 totrue
.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.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.include
/exclude
elements within thecoverage
element are deprecated as of PHPUnit 10.1, in favour of usingsource
withinclude
/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 whenfailOnDeprecation
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....
failOnWarning
option is added to thephpunit10.xml.dist
file as it is already available in PHPUnit 10.0.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.
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: