Handling of configuration files

[anomiex]

PHPUnit 10 brings both several changes to the configuration file and a misfeature of causing phpunit to exit with a non-zero status if the config file does not strictly validate.

So, for example, if you had been relying on convert{X}ToExceptions="true", you might want to set displayDetailsOnTestsThatTrigger{X}="true" and failOn{X}="true" for PHPUnit 10 to maintain that behavior going forward. But unless you also remove convert{X}ToExceptions="true", changing the behavior for runs with earlier versions of PHP, PHPUnit 10 will whine about not recognizing the old setting and will exit with a failure status even if all actual tests pass.

I don't know what the best way around this might be.

Simplest might be to provide a wrapper script that, if not passed -c or --configuration or --no-configuration, looks for various config file names and picks the best one for the current version of PHPUnit to set --configuration before execing phpunit. For example, it might check for any "phpunit.{VER}.xml.dist" and pick the one with the highest {VER} that's not greater than PHPUnit's own version.

A little fancier might be a wrapper that determines the configuration file in the same way PHPUnit would, copies it to a temporary file while stripping a predefined list of settings based on the PHPUnit version, and calls phpunit with --configuration pointing to the temporary file.

Unfortunately PHPUnit doesn't provide any way to hook into its own code to handle things more directly. There might still be ways, but they'd be fragile.

[jrfnl]

@anomiex PHPUnit offers a --migrate-configuration option since PHPUnit 9.3, which can be run before running PHPUnit for the test run, to convert a configuration file to a more recent format.
While definitely not perfect, as it doesn't handle the convert*ToExceptions settings, it is the recommended way to work with PHPUnit 9.x vs 10.x configuration files.

I think improving that --migrate-configuration option in PHPUnit should be explored first, before consideration should be given to adding any such functionality to the PHPUnit Polyfills.

I do feel your pain in this though, but there are more sticky issues around this as the PHPUnit 10.x failOn* options also do not translate to the same run results (exit code) in PHPUnit 9.x with convert*ToExceptions as in PHPUnit 9.x only PHP native errors/notices etc were taken into account for the exit code, while in PHPUnit 10.x, PHPUnit native ones are also taken into account, making any setup using the failOn* options brittle.

I think this needs further discussion in the PHPUnit repo first. If you do open an issue there, please add the link to this ticket as I'd very much would like to follow the discussion.

In the mean time, here are some links for interim solutions I've used in certain projects (take note of the info in the commit messages):

• For this repo (which needs to support the config for both PHPUnit < 10, PHPUnit 10.0, as well as PHPUnit 10.1):
  • Tests: add separate configuration file for PHPUnit 10 #111
  • 2462abe
• In a repo which can rely on the latest PHPUnit being used of each PHPUnit major:
  • PHPCompatibility/PHPCompatibility@df9cca7

[anomiex]

Thanks for pointing out that failOnDeprecation will also fail on PHPUnit's own deprecation messages, I had missed that.

Someone already filed sebastianbergmann/phpunit#5199 about thee test runner exiting with a non-zero code on warnings even when failOnWarning is false, so it seems unlikely he'd do anything about not wanting it to exit with a non-zero code when failOnWarning is true.

Also I'm having a hard time coming up with a reason for wanting to ignore PHPUnit's own deprecation warnings but not PHP deprecation warnings that's more compelling than "we'd have to wait on upgrading phpunit until yoast/phpunit-polyfills gets a polyfill", except for the "deprecation" about an older-schema config file.

I filed sebastianbergmann/phpunit#5405 for one actual bug I've encountered in the migration.

I'm skeptical he'll consider failing to migrate convert*ToExceptions to failOn* a bug, but I filed sebastianbergmann/phpunit#5406 anyway.

[jrfnl]

@anomiex Thanks for that. I've subscribed to both issues. I hadn't gotten around to do a proper write-up of the issues myself yet.

I'm having a hard time coming up with a reason for wanting to ignore PHPUnit's own deprecation warnings but not PHP deprecation warnings that's more compelling than "we'd have to wait on upgrading phpunit until yoast/phpunit-polyfills gets a polyfill"

Except that is not the real issue.

There have been multiple times in the past when PHPUnit would warn about something being deprecated without replacement, only for it to be reconsidered and a replacement being added after all at a later point in time.

This happened even as recently as PHPUnit 9.6 which warns about assertObjectHasAttribute() being deprecated without replacement. That deprecation got adjusted in PHPUnit 9.6.7 after the release of PHPUnit 10.1 which introduced the assertObjectHasProperty() method.

There are also warnings ("data providers have to be static") which don't necessarily affect the running of the test suite until a later point in time.

Along the same lines as the reasoning for PHP deprecations, you may want to ignore those until those can be addressed as a managed mini-project, but that doesn't necessarily mean that PHP and PHPUnit deprecations should be addressed in the same mini-project or at the same time.

They have a different time line and a different scope.

Additionally, PHPUnit has recently had a tendency to change the configuration file a lot and to add deprecations in minor/patch versions which come out fairly regularly, which is very different from PHP, where such deprecations are only added once a year in a minor.

What this means in practice is that, while for a new PHP version, you may add a CI build once a year with a continue-on-error for the time being until all deprecations have been fixed (managed), a new PHPUnit minor/patch version may now start failing builds at random (uncontrollable unless you fix the max PHPUnit version in Composer and let Dependabot manage the updates every week or so, which is a complete drag).

In other words, the current PHPUnit 10.x XML attributes to manage warnings/deprecations etc just do not allow for updating PHPUnit related issues in a controlled way without blocking automatic use of the latest PHPUnit version.

Does that clarify things a little more ?

[anomiex]

which is very different from PHP, where such deprecations are only added once a year in a minor.

That's a good point, PHP does limit their deprecations to a new minor. And we (like probably everyone else) tend to treat minor PHP versions similarly to major versions of other stuff like PHPUnit.

Sounds like you should handle filing the PHPUnit bug about this one, you have a much better grasp on the issue. 😉

[jrfnl]

FYI: in case you missed it: as of PHPUnit 10.5.32 and 11.3.3 PHPUnit deprecation notices are now decoupled from PHP deprecation notices, which should make live a hell of a lot easier. (still doesn't sort out the config, but does sort out some of the other things discussed above)

Ref: sebastianbergmann/phpunit#5937

[anomiex]

I finally came back to this. I decided to go with the simplest implementation: a wrapper that assumes you'll have configs systematically named like phpunit.8.xml, phpunit.9.xml, phpunit.11.xml, etc for each relevant major version, with no logic trying to fall back if the named file doesn't exist. I figure symlinks can be used if someone wants the same config for multiple versions or something.

The plan is to publish it to Packagist as automattic/phpunit-select-config. The PR with the code in our monorepo is at Automattic/jetpack#41860.