Tests: make testBrokenRulesetMultiError() test compatible with LibXML 2.12+

[remicollet]

Running test suite on a git clone (with PHP 8.1, 8.2, 8.3 or 8.4)

There was 1 failure:

1) PHP_CodeSniffer\Tests\Core\Ruleset\ProcessRulesetBrokenRulesetTest::testBrokenRulesetMultiError
Failed asserting that exception message 'Ruleset /dev/shm/BUILD/php-pear-PHP-CodeSniffer-3.11.2-build/PHP_CodeSniffer-1368f4a58c3c52114b86b1abe8f4098869cb0079/tests/Core/Ruleset/ProcessRulesetBrokenRulesetMultiErrorTest.xml is not valid
- On line 8, column 12: Opening and ending tag mismatch: property line 7 and rule
- On line 10, column 11: Opening and ending tag mismatch: properties line 5 and ruleset
' matches '`^Ruleset \S+ProcessRulesetBrokenRulesetMultiErrorTest\.xml is not valid\R- On line 8, column 12: Opening and ending tag mismatch: property line 7 and rule\R- On line 10, column 11: Opening and ending tag mismatch: properties line 5 and ruleset\R- On line 11, column 1: Premature end of data in tag rule line 4\R$`'.

FAILURES!

[jrfnl]

@remicollet That's a new test and in all the runs I've done with it, it is passing fine, both locally as well as in CI.
I presume you are getting the error for the images you maintain ? I suppose it could be something to do with the libxml version PHP is compiled with ? Do you use a custom libxml ?

[remicollet]

Indeed seems related to new lilbxml 2.12 in recent Fedora/RHEL

Test suite passes on

• RHEL 8.10 with libxml 2.9.7
• RHEL 9.5 with libxml 2.9.13
• Fedora 39 with libxml 2.10.4

Test fails on

• RHEL 10.0-beta with libxml 2.12.5
• Fedora 40/41 with libxml 2.12.8

[jrfnl]

So then the question really becomes: is this something the test should accommodate ? Or is this a bug in the error reporting from the new libxml version ? either in libxml itself or in how the errors are passed through in PHP ?

[remicollet]

Question is rather, are you testing libxml, or that phpcs properly find an error ? ;)

Output changes in libxml messages are quite common (example)

diff -up tests/Core/Ruleset/ProcessRulesetBrokenRulesetTest.php.old tests/Core/Ruleset/ProcessRulesetBrokenRulesetTest.php
--- tests/Core/Ruleset/ProcessRulesetBrokenRulesetTest.php.old	2024-12-12 08:42:39.216754603 +0100
+++ tests/Core/Ruleset/ProcessRulesetBrokenRulesetTest.php	2024-12-12 08:43:45.048312685 +0100
@@ -79,8 +79,7 @@ final class ProcessRulesetBrokenRulesetT
 
         $regex  = '`^Ruleset \S+ProcessRulesetBrokenRulesetMultiErrorTest\.xml is not valid\R';
         $regex .= '- On line 8, column 12: Opening and ending tag mismatch: property line 7 and rule\R';
-        $regex .= '- On line 10, column 11: Opening and ending tag mismatch: properties line 5 and ruleset\R';
-        $regex .= '- On line 11, column 1: Premature end of data in tag rule line 4\R$`';
+        $regex .= '- On line 10, column 11: Opening and ending tag mismatch: properties line 5 and ruleset\R`';
 
         $this->expectRuntimeExceptionRegex($regex);
 

[jrfnl]

What I'm testing in that test is the custom error handling within PHPCS for errors coming reported by libxml. In particular, that if there are multiple errors reported by libxml, these are displayed correctly and in a readable fashion by PHPCS.

Basically, that test is testing the multi-error handling in the foreach for this code:

PHP_CodeSniffer/src/Ruleset.php

Lines 486 to 499 in 855f261

	libxml_use_internal_errors(true);
	$ruleset = simplexml_load_string(file_get_contents($rulesetPath));
	if ($ruleset === false) {
	$errorMsg = "Ruleset $rulesetPath is not valid".PHP_EOL;
	$errors = libxml_get_errors();
	foreach ($errors as $error) {
	$errorMsg .= '- On line '.$error->line.', column '.$error->column.': '.$error->message;
	}
	libxml_clear_errors();
	throw new RuntimeException($errorMsg);
	}
	libxml_use_internal_errors(false);

Which means the test fixture deliberately has three XML errors, but it looks like libxml 2.12 is only reporting two, so even if I would ignore the specific message text from the libxml errors, I'd still want the regex to verify that exactly 3 errors are shown, while it looks like libxml is now reporting 2, which is not useful for this test.

[remicollet]

Are 2 not "multiple errors" ?

[jrfnl]

True, but I kind of wanted to test with > 2 errors.

[jrfnl]

Okay, so what I think needs to happen here:

• Adjust the test fixture to have three errors which are all reported both by libxml < 2.12, as well as by libxml >= 2.12+.
• Adjust the regex in the actual test code to accommodate the change in the errors.

I don't have access to a PHP version build with libxml 2.12+ myself, so here's me hoping someone will submit a PR to handle this.

@remicollet As for the test runs you manage, I suppose you could add a @requires extension libxml < 2.12.0 tag for the time being.

Note: the @requires tag is not an acceptable solution for the test in PHPCS itself, but could tide you over until the test is improved to be compatible with libxml < 2.12 and libxml >= 2.12+.

[remicollet]

I don't have access to a PHP version build with libxml 2.12+ myself

$ podman run --rm -ti fedora:41
[root@2c695ec63a03 PHP_CodeSniffer]# dnf install composer phpunit9 git
[root@2c695ec63a03 PHP_CodeSniffer]# git clone https://github.com/PHPCSStandards/PHP_CodeSniffer.git
[root@2c695ec63a03 PHP_CodeSniffer]# cd PHP_CodeSniffer/
[root@2c695ec63a03 PHP_CodeSniffer]# phpunit9 -d memory_limit=-1
Note: Tests are running in "CS" mode

PHPUnit 9.6.21 by Sebastian Bergmann and contributors.
...

;)

[jrfnl]

@remicollet Nice try, but I'm on Windows ;-)