Yoast / PHPUnit-Polyfills

Set of polyfills for changed PHPUnit functionality to allow for creating PHPUnit cross-version compatible tests
BSD 3-Clause "New" or "Revised" License
173 stars 13 forks source link

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

Closed jrfnl closed 1 year ago

jrfnl commented 1 year ago

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:

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:

coveralls commented 1 year ago

Coverage Status

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