sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.69k stars 2.2k forks source link

Migrate configuration from 10.5 to 11.4 #6023

Open llaville opened 2 days ago

llaville commented 2 days ago

Hello,

Working on my project BOX Manifest and wanted to migrate from PHPUnit 10 to 11, I've noticed that include uncovered files option was not included during migration process.

After installing PHPUnit 11.4.3, I've checks for regressions with following command :

vendor/bin/phpunit --display-phpunit-deprecations --no-coverage

And got this output :

[debug] Checking BOX_ALLOW_XDEBUG
[debug] The Xdebug extension is loaded (3.3.2) xdebug.mode=debug,develop
[debug] Process restarting (BOX_ALLOW_XDEBUG=internal|3.3.2|1|*|*)
[debug] Running: [/usr/local/bin/php, -n, -c, /tmp/nWkIvs, vendor/bin/phpunit, --display-phpunit-deprecations, --no-coverage]
[debug] Checking BOX_ALLOW_XDEBUG
[warning] Restarted (264 ms). The Xdebug extension is loaded (3.3.2) xdebug.mode=coverage
PHPUnit 11.4.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.25
Configuration: /shared/backups/bartlett/box-manifest/phpunit.xml.dist

................................................................. 65 / 91 ( 71%)
..........................                                        91 / 91 (100%)

Time: 00:03.492, Memory: 102.00 MB

There was 1 PHPUnit test runner deprecation:

1) Your XML configuration validates against a deprecated schema. Migrate your XML configuration using "--migrate-configuration"!

OK, but there were issues!
Tests: 91, Assertions: 120, PHPUnit Deprecations: 1.
[debug] Restarted process exited 0

So I've run migration process with following command :

vendor/bin/phpunit --migrate-configuration

Anf got result :

PHPUnit 11.4.3 by Sebastian Bergmann and contributors.

Created backup:         /shared/backups/bartlett/box-manifest/phpunit.xml.dist.bak
Migrated configuration: /shared/backups/bartlett/box-manifest/phpunit.xml.dist

But when I compare versions, I was surprised on coverage section that missed previous attribute includeUncoveredFiles

Checks with command :

diff phpunit.xml.dist.bak phpunit.xml.dist --ignore-space-change

That prints :

2,18c2,3
< <phpunit
<     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
<     xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd"
<     backupGlobals="false"
<     backupStaticProperties="false"
<     bootstrap="bootstrap.php"
<     colors="true"
<     stopOnError="false"
<     stopOnFailure="false"
<     stopOnIncomplete="false"
<     stopOnRisky="false"
<     stopOnSkipped="false"
<     beStrictAboutTestsThatDoNotTestAnything="false"
<     processIsolation="false"
<     cacheDirectory=".phpunit.cache"
< >
<     <coverage includeUncoveredFiles="true">
---
> <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.4/phpunit.xsd" backupGlobals="false" backupStaticProperties="false" bootstrap="bootstrap.php" colors="true" stopOnError="false" stopOnFailure="false" stopOnIncomplete="false" stopOnRisky="false" stopOnSkipped="false" beStrictAboutTestsThatDoNotTestAnything="false" processIsolation="false" cacheDirectory=".phpunit.cache">
>   <coverage>

BTW, it will be cool if we can same identation after migration. Even if my PHP editor is able to easily re-indent code !

llaville commented 2 days ago

FYI: related to my previous report https://github.com/sebastianbergmann/phpunit/issues/6019, when there is a change in configuration (even if it's not expected, like includeUncoveredFiles attribute missing), the schema change is well detected.

sebastianbergmann commented 2 days ago

it will be cool if we can same identation after migration

While I agree that losing indentation is an inconvenience, it is annoying enough to invest time/effort in finding a solution to keep it.

llaville commented 2 days ago

I've noticed that documentation still identify the includeUncoveredFiles attribute, but not the schema 11.4 while schema 10.4 did it.

So, after re-introduced the attribute missing in my phpunit.xml.dist config file, I got another (wrong/not expected) warning

There was 1 PHPUnit test runner deprecation:

1) Your XML configuration validates against a deprecated schema. Migrate your XML configuration using "--migrate-configuration"! 

I think it's an issue, because includeUncoveredFiles is not identify by v11.4

llaville commented 2 days ago

Oops, I've in mind schema 10.5 and I present schema 10.4, but both identify correctly the missing attribute.

@see https://github.com/sebastianbergmann/phpunit/blob/main/schema/10.5.xsd#L65

llaville commented 2 days ago

While I agree that losing indentation is an inconvenience, it is annoying enough to invest time/effort in finding a solution to keep it.

Agree with you that it's time consuming. But I'll give you the solution, because I wanted to learn more about PHPUnit migration command.

With DOMDocument, when you set preserveWhiteSpace and formatOutput properties before loading file contents, it won't preserve indentation, and if you did it after it works.

Preserve indentation

<?php
$filename = __DIR__ . DIRECTORY_SEPARATOR . 'phpunit.xml.dist';

$configurationDocument = new \DOMDocument();
// either
//$configurationDocument->load($filename);
// or
$configurationDocument->loadXML(file_get_contents($filename));

$configurationDocument->preserveWhiteSpace = false;
$configurationDocument->formatOutput = true;

var_export($configurationDocument->saveXML());

DO NOT preserve indentation

<?php
$filename = __DIR__ . DIRECTORY_SEPARATOR . 'phpunit.xml.dist';

$configurationDocument = new \DOMDocument();

$configurationDocument->preserveWhiteSpace = false;
$configurationDocument->formatOutput = true;

// either
//$configurationDocument->load($filename);
// or
$configurationDocument->loadXML(file_get_contents($filename));

var_export($configurationDocument->saveXML());

In summary to fix PHPUnit, you have just to remove the line https://github.com/sebastianbergmann/phpunit/blob/11.4/src/Util/Xml/Loader.php#L67 because the good lines in right place are already defined at https://github.com/sebastianbergmann/phpunit/blob/11.4/src/TextUI/Configuration/Xml/Migration/Migrator.php#L47-L48