Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cobertura package name attribute is always empty #935

Conversation

tm1000
Copy link
Contributor

@tm1000 tm1000 commented Sep 10, 2022

In certain utilities that parse Cobertura files they look to the <Package name= attribute. In PHPUnit this attribute is always empty (null)

This causes results to look like the following (in this parser at least):
image

As you can tell this makes no sense (The parser on the other end adds "[filename] Package 1+n" whenever it encounters an empty package name)

See: https://github.com/sebastianbergmann/phpunit/blob/main/src/TextUI/TestRunner.php#L479

The third parameter is never passed.

It appears that the Cobertura report generator was copied from the Clover report generator but they are not one-in-the same. If $name was properly set in the Cobertura report then every "package" would have the same name. This would confuse Cobertura parsers because the way PHPUnit works with each package is it's essentially each $report therefore we should be setting the packageName the same as the filename (or the class but filename seems more flexible when the file contains no classes)

Summary:
In a Clover file the "name" attribute is global, see: https://github.com/sebastianbergmann/php-code-coverage/blob/main/src/Report/Clover.php#L46

In Cobertura the "name" attribute is attached to PackageName and a package is generated for each report.

This change isn't really breaking because if name would have ever been passed it would have been added to each package and completely confused any parser (worse than it is now)

@tm1000 tm1000 changed the base branch from main to 9.2 September 10, 2022 02:53
@codecov
Copy link

codecov bot commented Sep 10, 2022

Codecov Report

Merging #935 (1034210) into 9.2 (e2223eb) will increase coverage by 0.45%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                9.2     #935      +/-   ##
============================================
+ Coverage     83.64%   84.10%   +0.45%     
- Complexity     1164     1177      +13     
============================================
  Files            59       71      +12     
  Lines          3418     3453      +35     
============================================
+ Hits           2859     2904      +45     
+ Misses          559      549      -10     
Impacted Files Coverage Δ
src/Report/Cobertura.php 97.04% <100.00%> (-0.02%) ⬇️
src/StaticAnalysis/CodeUnitFindingVisitor.php 86.77% <0.00%> (-0.32%) ⬇️
src/Report/Text.php 79.33% <0.00%> (-0.14%) ⬇️
src/Report/Html/Renderer/Dashboard.php 96.42% <0.00%> (-0.04%) ⬇️
src/Report/Html/Renderer.php 99.24% <0.00%> (-0.01%) ⬇️
src/Filter.php 97.56% <0.00%> (ø)
src/Version.php 100.00% <0.00%> (ø)
src/Node/File.php 81.10% <0.00%> (ø)
src/Node/Builder.php 97.46% <0.00%> (ø)
src/Node/Iterator.php 75.00% <0.00%> (ø)
... and 50 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sebastianbergmann sebastianbergmann merged commit eb8c670 into sebastianbergmann:9.2 Sep 10, 2022
@sebastianbergmann sebastianbergmann changed the title Fix cobertura package name Cobertura package name attribute is always empty Sep 10, 2022
@tm1000 tm1000 deleted the feature/fix-cobertura-package-name-92 branch September 14, 2022 06:18
@tm1000 tm1000 restored the feature/fix-cobertura-package-name-92 branch September 14, 2022 06:18
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.

None yet

2 participants