Confusing error message when migration of a configuration is requested that does not need to be migrated

[oliverklee]

Q	A
PHPUnit version	10.5.7
PHP version	8.2.10
Installation Method	Composer

Summary

Migrating a PHPUnit configuration file fails with an error message if failOnNotice or failOnDeprecation are set (or both).

According to https://docs.phpunit.de/en/10.5/configuration.html, these two options are available and valid.

With PHPUnit 9, this problem does not occur.

Current behavior

PHPUnit 10.5.7 by Sebastian Bergmann and contributors.

Created backup:         /home/klee/src/php/phpunit-10-config-test/FailOnDeprecation.xml.bak
Migration failed: "/home/klee/src/php/phpunit-10-config-test/FailOnDeprecation.xml" is not a valid PHPUnit XML configuration file that can be migrated

(ditto for the other configuration file)

How to reproduce

git clone [email protected]:oliverklee/phpunit-10-config-test.git
cd phpunit-10-config-test
composer install
./vendor/bin/phpunit --migrate-configuration -c FailOnDeprecation.xml
./vendor/bin/phpunit --migrate-configuration -c FailOnNotice.xml

Expected behavior

PHPUnit 10.5.7 by Sebastian Bergmann and contributors.

Created backup:         /home/klee/src/php/phpunit-10-config-test/FailOnDeprecation.xml.bak
/home/klee/src/php/phpunit-10-config-test/FailOnDeprecation.xml does not need to be migrated.

(ditto for the other configuration file)

[sebastianbergmann]

This is an "interesting" issue, Oliver, and I am thankful that you reported it. "Interesting" because it has been quite a while since I looked at a higher level than PHPUnit\TextUI\XmlConfiguration\Migration implementations at the XML configuration migration functionality and within minutes thought: "How could this have ever worked?".

PHPUnit 10.5.7 ships with XML schema definitions for PHPUnit 10.5.7 as well as PHPUnit 8.5, PHPUnit 9.2, PHPUnit 9.5, and PHPUnit 10.0.

PHPUnit\TextUI\XmlConfiguration\SchemaDetector loops over ['10.0', '9.5', '9.2', '8.5'] to find a previous version of the schema that the configuration file is compatible with. This information is used together with the knowledge encoded in PHPUnit\TextUI\XmlConfiguration\MigrationBuilder which PHPUnit\TextUI\XmlConfiguration\Migrations need to be applied to actually perform the migration.

The issue tracked here is caused by the fact that the XML configuration file to be migrated is not valid according to either of the schema definitions that are tried as the failOnDeprecation and failOnNotice attributes were introduced in PHPUnit 10.1.

I doubt that the issue tracked here is the only one in the configuration file migration functionality ...

[sebastianbergmann]

@theseer Do you have an idea what should be done here (since you did the original design of the configuration file migration functionality)?

[theseer]

Well, for hopefully obvious reasons we cannot (reliably) migrate a configuration file that contains attributes (or nodes, for that matter) that are invalid in the respective version the configuration file claims to be targeted at.

In your assessment above you list we're looping over 10.0 and lower versions. Apparently, that's not a complete list, given you said we have added these attributes at 10.1. So, for starters, there should be (or have been) a different schema for that version.

As far as I understand the situation, we have a configuration file that is not compliant to any schema version as it mixes elements from various versions.

For the migration logic, I would consider this a won't fix. All we can do is fail "nicer" by telling the user the configuration file is not valid before attempting migrations.

Does that make sense or am I thinking in the wrong direction?

[sebastianbergmann]

As far as I understand the situation, we have a configuration file that is not compliant to any schema version as it mixes elements from various versions.

For the migration logic, I would consider this a won't fix. All we can do is fail "nicer" by telling the user the configuration file is not valid before attempting migrations.

AFAICS, the configuration file that @oliverklee is trying to migrate is valid with PHPUnit 10.5 and does not require migration.

[sebastianbergmann]

So, for starters, there should be (or have been) a different schema for that version.

I agree and that is the first thing that I will look into: ship phpunit.xsd files for all minor versions since we introduced the configuration file migration functionality and consider them in SchemaDetector.

[theseer]

As far as I understand the situation, we have a configuration file that is not compliant to any schema version as it mixes elements from various versions.
For the migration logic, I would consider this a won't fix. All we can do is fail "nicer" by telling the user the configuration file is not valid before attempting migrations.

AFAICS, the configuration file that @oliverklee is trying to migrate is valid with PHPUnit 10.5 and does not require migration.

That's not necessarily a contradiction to what I wrote ;)

I didn't look into the code (yet) but there must be something "off" with either the file or its matching schema version so we believe to have to migrate.

I'll have a look at it later if you want me to.

[sebastianbergmann]

I'll have a look at it later if you want me to.

Thank you for offering your time, but at the moment that is not necessary.

[sebastianbergmann]

The 9.6, 10.5, and main branches now ship with the schema definitions for all PHPUnit minor version since PHPUnit 8.5 and SchemaDetector tries to validate the to-be-migrated XML configuration file in reverse order, from current to 8.5.

We error out when the to-be-migrated XML configuration file does not validate against any known schema. And we "error out" when it validates against the current schema (and therefore does not require migration).

Of course, more work needs to be done here.

[sebastianbergmann]

The next releases of PHPUnit 9.6 and PHPUnit 10.5 will handle this situation better.