Incorrect return type for Cobertura::process

[cjones-vultr]

Q	A
php-code-coverage version	10.1.3
PHP version	8.1.32
Driver	Xdebug
Xdebug version (if used)	3.4.2
Installation Method	Composer
Usage Method	PHPUnit
PHPUnit version (if used)	10.2.7

The signature of Cobertura::process is public function process(CodeCoverage $coverage, ?string $target = null): string. It has a single return statement in it that returns the result of calling $document->saveXML(). That function has a signature of public DOMDocument::saveXML(?DOMNode $node = null, int $options = 0): string|false. When it returns false, an error occurred.

[cjones-vultr]

Since this project depends on libxml, I think the function could call libxml_get_last_error() and turn the returned LibXMLError into an exception that gets thrown instead of returning false.

[sebastianbergmann]

You are right that the DOMDocument::saveXML() function can fail in theory. In practice, however, I have not seen this happen in the places where it is used in this library.

Did you raise this issue because you encountered a runtime type error? If not, I'm inclined to close this as "won't fix".

However, if you can provide a test case that demonstrates that DOMDocument::saveXML() can fail, then this should of course be addressed. This would need to be addressed not only for the Cobertura XML reporter, but also for all other places in the code where we use DOMDocument::saveXML().

[cjones-vultr]

Did you raise this issue because you encountered a runtime type error?

Yes, I'm seeing DOMDocument::saveXML() return false in our CI suite. I'm still trying to work out a reproducable case for it.

We're running this command: php -dmemory_limit=-1 -dpcov.enabled=1 -dpcov.directory=$(PWD) -dpcov.exclude='\#./vendor/.*\#' -dxdebug.mode=coverage vendor/bin/phpunit --colors=always --coverage-html $(PWD)/build/reports/ --exclude-group=nodefault

And the relevenat output is:

Generating code coverage report in Cobertura XML format ... 
An error occurred inside PHPUnit.
Message:  SebastianBergmann\CodeCoverage\Report\Cobertura::process(): Return value must be of type string, bool returned
Location: /builds/$repo/vendor/phpunit/php-code-coverage/src/Report/Cobertura.php:307
#0 /builds/$repo/vendor/phpunit/phpunit/src/Runner/CodeCoverage.php(228): SebastianBergmann\CodeCoverage\Report\Cobertura->process()
#1 /builds/$repo/vendor/phpunit/phpunit/src/TextUI/Application.php(202): PHPUnit\Runner\CodeCoverage->generateReports()
#2 /builds/$repo/vendor/phpunit/phpunit/phpunit(99): PHPUnit\TextUI\Application->run()
#3 /builds/$repo/vendor/bin/phpunit(122): include('...')
#4 {main}

I'll keep trying to figure out what the issue is here and see if I can create a test case for it.

[sebastianbergmann]

This has been fixed in versions 11.0 and 12.3. However, it will not be fixed in version 10.1, for which this issue was reported, as this version is used by PHPUnit 10, which no longer receives bug fixes.

[cjones-vultr]

Thank you so much!

[cjones-vultr]

For anyone else who finds this, I was never able to determine the cause of the bug or how to trigger it outside this one particular branch. In the end, I resolved it by upgrading to phpunit/phpunit 10.5.54 and phpunit/php-code-coverage to 10.1.16.