Skip to content

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Nov 22, 2025

requires theseer/tokenizer#38

before this PR (using PHPUnit 12.4.4)

➜  slow-coverage-xml1 git:(main) ✗ hyperfine 'php coverage-xml.php'           
Benchmark 1: php coverage-xml.php
  Time (mean ± σ):     11.772 s ±  0.550 s    [User: 10.209 s, System: 1.237 s]
  Range (min … max):   11.008 s … 12.568 s    10 runs

after this PR:

➜  slow-coverage-xml1 git:(main) ✗ hyperfine 'php coverage-xml.php'
Benchmark 1: php coverage-xml.php
  Time (mean ± σ):      5.999 s ±  0.385 s    [User: 5.008 s, System: 0.789 s]
  Range (min … max):    5.647 s …  6.732 s    10 runs

env

php -v
PHP 8.3.28 (cli) (built: Nov 18 2025 22:17:16) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.3.28, Copyright (c) Zend Technologies

@staabm
Copy link
Contributor Author

staabm commented Nov 22, 2025

double check against phpstan-src@05b80ef861 on my macos m4 pro laptop

before this PR (using PHPUnit 12.4.4)

Generating code coverage report in PHPUnit XML format ... done [00:31.317]

after this PR

Generating code coverage report in PHPUnit XML format ... done [00:17.598]

@staabm staabm force-pushed the dom3 branch 2 times, most recently from c9a346b to b5a8841 Compare November 22, 2025 10:37
@sebastianbergmann
Copy link
Owner

Does Infection need the <source> element? If not, we could speed up the XML report generation even more for that use case:

❯ git diff
src/Report/Xml/Facade.php --- 1/2 --- PHP
45        public const string XML_NAMESPACE = 'https://schema.phpunit.de/coverage/1.0';
46        private string $target;
47        private Project $project;
48        private readonly string $phpUnitVersion;
50        public function __construct(string $version)
   49     private readonly bool $includeSource;
   51     public function __construct(string $version, bool $includeSource = true)
   54         $this->includeSource  = $includeSource;
   55     }
   56
   57     /**
   58      * @throws XmlException

src/Report/Xml/Facade.php --- 2/2 --- PHP
169                $coverage = $fileReport->lineCoverage((string) $line);
170                $coverage->finalize($tests);
171            }
   175         if ($this->includeSource) {
   179         }
   180
   181         $this->saveDocument($fileReport->asDom(), $file->id());
   182     }
   183

Of course, we would need to expose this as a configuration option. This is just an idea so far.

@staabm
Copy link
Contributor Author

staabm commented Nov 27, 2025

Does Infection need the <source> element? If not, we could speed up the XML report generation even more for that use

great idea.

I did some testing and it seems the source element is not required for infection. will double check with the infection team though.

I also did some perf testing on phpstan-src and I can see a ~15% perf improvement when source is not at all generated.

I think we should tackle this in a separate PR though


for this PR here to continue we need a release of the xml-tokenizer including theseer/tokenizer#38 though

@staabm staabm changed the title Drop DOM dependency Drop DOM dependency in XML Report Nov 27, 2025
@sebastianbergmann
Copy link
Owner

I think we should tackle this in a separate PR though

Of course.

c1888bb will be released in time for PHPUnit 12.5 which uses this for sebastianbergmann/phpunit@023845a.

@staabm
Copy link
Contributor Author

staabm commented Dec 6, 2025

progress on this PR is blocked on a new theseer/tokenizer release, which is blocked on a new phpunit 10.x release with fixed prefixing

@sebastianbergmann
Copy link
Owner

blocked on a new phpunit 10.x release with fixed prefixing

PHPUnit 10.5.60 has been released.

@staabm
Copy link
Contributor Author

staabm commented Dec 6, 2025

thank you!

@staabm staabm force-pushed the dom3 branch 2 times, most recently from 49fd2ac to ae31bc1 Compare December 6, 2025 08:36
@codecov
Copy link

codecov bot commented Dec 6, 2025

Codecov Report

❌ Patch coverage is 98.87640% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.42%. Comparing base (bca180c) to head (8b04878).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/Report/Xml/Facade.php 97.67% 1 Missing ⚠️
src/Report/Xml/Report.php 94.44% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1125      +/-   ##
============================================
- Coverage     88.70%   88.42%   -0.29%     
+ Complexity     1391     1390       -1     
============================================
  Files           105      105              
  Lines          4782     4700      -82     
============================================
- Hits           4242     4156      -86     
- Misses          540      544       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

to speedup xml report generation
@staabm staabm marked this pull request as ready for review December 6, 2025 10:28
@staabm
Copy link
Contributor Author

staabm commented Dec 6, 2025

running xml-report generation on the sebastianbergmann/php-code-coverage codebase:

before this PR:

➜  php-code-coverage git:(main) ✗ XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-xml tmp/coverage 
PHPUnit 12.5.0 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.28 with Xdebug 3.5.0
Configuration: /Users/staabm/workspace/php-code-coverage/phpunit.xml

......................................SSSSS....................  63 / 226 ( 27%)
..I............................................................ 126 / 226 ( 55%)
............................................................... 189 / 226 ( 83%)
.....................................                           226 / 226 (100%)

Time: 00:00.608, Memory: 30.00 MB

OK, but there were issues!
Tests: 226, Assertions: 645, Skipped: 5, Incomplete: 1.

Generating code coverage report in PHPUnit XML format ... done [00:00.295]

after this PR:

➜  php-code-coverage git:(dom3) ✗ XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-xml tmp/coverage                                   
PHPUnit 12.5.0 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.28 with Xdebug 3.5.0
Configuration: /Users/staabm/workspace/php-code-coverage/phpunit.xml

......................................SSSSS....................  63 / 226 ( 27%)
..I............................................................ 126 / 226 ( 55%)
............................................................... 189 / 226 ( 83%)
.....................................                           226 / 226 (100%)

Time: 00:00.605, Memory: 30.00 MB

OK, but there were issues!
Tests: 226, Assertions: 645, Skipped: 5, Incomplete: 1.

Generating code coverage report in PHPUnit XML format ... done [00:00.209]

note

+ Generating code coverage report in PHPUnit XML format ... done [00:00.209]
- Generating code coverage report in PHPUnit XML format ... done [00:00.295]

staabm referenced this pull request in theseer/tokenizer Dec 6, 2025
</line>
</coverage>
<source>
<source xmlns="https://schema.phpunit.de/coverage/1.0">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staabm @theseer Are these changes intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its not a change from a semantic point of view.

before this PR we imported one document into another which lead to the involved php extensions normalizing this namespace away. technically it is not necessary because its the same namespace as in the root phpunit element, but as this normalization no longer happens this xmlns="https://schema.phpunit.de/coverage/1.0" is showing up now.

<token name="T_WHITESPACE"> </token>
<token name="T_OPEN_BRACKET">(</token>
<token name="T_AMPERSAND_FOLLOWED_BY_VAR_OR_VARARG">&amp;</token>
<token name="T_AMPERSAND">&amp;</token>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why this change happens though

Copy link
Contributor Author

@staabm staabm Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diving into it more.. T_AMPERSAND was a token type in PHP4.x and it only existed in theseer/tokenizer, because it was hard-coded in https://github.com/theseer/tokenizer/blob/d1dd771235a40694cb5cb7239e531cf9b9702682/src/Tokenizer.php#L35

since PHP5+ the builtin token names are
T_AMPERSAND_FOLLOWED_BY_VAR_OR_VARARG
and/or T_AMPERSAND_NOT_FOLLOWED_BY_VAR_OR_VARARG

https://3v4l.org/tU46U#veol

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened a upstream discussion: theseer/tokenizer#44

@sebastianbergmann sebastianbergmann merged commit 5b74f62 into sebastianbergmann:main Dec 8, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants