Cobertura output async "class" merging can create duplicate method entries #630

Malivil · 2023-10-10T15:25:02Z

Describe the bug
When using reportgenerator to output a Cobertura report, classes with async method overrides with the same name will have duplicate entries in the generated XML.

For example, this MVC controller code:

public class TestController : Controller
{
    public ActionResult Index()
    {
        return View();
    }

    [HttpPost]
    [ValidateAntiForgeryToken]
    public async Task<ActionResult> Index([FromForm] IFormFile file)
    {
        return View();
    }
}

Generates this XML using Coverlet:

<?xml version="1.0" encoding="utf-8"?>
<coverage line-rate="0.2727" branch-rate="0" version="1.9" timestamp="1696950298" lines-covered="6" lines-valid="22" branches-covered="0" branches-valid="2">
  <sources>
    <source></source>
  </sources>
  <packages>
    <package name="CoverletRepro" line-rate="0.2727" branch-rate="0" complexity="4">
      <classes>
        <class name="CoverletRepro.TestController" filename="TestController.cs" line-rate="1" branch-rate="1" complexity="1">
          <methods>
            <method name="Index" signature="()" line-rate="1" branch-rate="1" complexity="1">
              <lines>
                <line number="8" hits="1" branch="False" />
                <line number="9" hits="1" branch="False" />
                <line number="10" hits="1" branch="False" />
              </lines>
            </method>
          </methods>
          <lines>
            <line number="8" hits="1" branch="False" />
            <line number="9" hits="1" branch="False" />
            <line number="10" hits="1" branch="False" />
          </lines>
        </class>
        <class name="CoverletRepro.TestController/&lt;Index&gt;d__1" filename="TestController.cs" line-rate="1" branch-rate="1" complexity="1">
          <methods>
            <method name="MoveNext" signature="()" line-rate="1" branch-rate="1" complexity="1">
              <lines>
                <line number="15" hits="1" branch="False" />
                <line number="16" hits="1" branch="False" />
                <line number="17" hits="1" branch="False" />
              </lines>
            </method>
          </methods>
          <lines>
            <line number="15" hits="1" branch="False" />
            <line number="16" hits="1" branch="False" />
            <line number="17" hits="1" branch="False" />
          </lines>
        </class>
      </classes>
    </package>
  </packages>
</coverage>

If that XML file is then fed into reportgenerator, it creates this XML file:

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE coverage SYSTEM "http://cobertura.sourceforge.net/xml/coverage-04.dtd">
<coverage line-rate="1" branch-rate="1" lines-covered="6" lines-valid="6" branches-covered="0" branches-valid="0" complexity="2" version="0" timestamp="1696936548">
  <sources>
    <source></source>
  </sources>
  <packages>
    <package name="CoverletRepro" line-rate="1" branch-rate="1" complexity="2">
      <classes>
        <class name="CoverletRepro.TestController" filename="TestController.cs" line-rate="1" branch-rate="1" complexity="2">
          <methods>
            <method name="Index" signature="()" line-rate="1" branch-rate="1" complexity="1">
              <lines>
                <line number="8" hits="1" branch="false" />
                <line number="9" hits="1" branch="false" />
                <line number="10" hits="1" branch="false" />
              </lines>
            </method>
            <method name="Index" signature="()" line-rate="1" branch-rate="1" complexity="1">
              <lines>
                <line number="15" hits="1" branch="false" />
                <line number="16" hits="1" branch="false" />
                <line number="17" hits="1" branch="false" />
              </lines>
            </method>
          </methods>
          <lines>
            <line number="8" hits="1" branch="false" />
            <line number="9" hits="1" branch="false" />
            <line number="10" hits="1" branch="false" />
            <line number="15" hits="1" branch="false" />
            <line number="16" hits="1" branch="false" />
            <line number="17" hits="1" branch="false" />
          </lines>
        </class>
      </classes>
    </package>
  </packages>
</coverage>

Which has two "Index" entries with the same empty signature.
I'm not sure if this is a limitation of the format, a problem with Coverlet, or a problem with reportgenerator but as it only occurs after running through reportgenerator I thought I would start here.

To Reproduce
The following input helps to reproduce your issue:

Download coverage.cobertura.txt and TestController.txt from this issue. Both extensions were changed so GitHub would support them
Run latest reportgenerator like so: reportgenerator "-reports:coverage.cobertura.txt" "-targetdir:." "-reporttypes:Cobertura"
See XML like the last code block above

danielpalme · 2023-10-10T15:52:29Z

I will have a look within the days.

Malivil · 2023-10-10T16:42:26Z

Possibly related, this happens with nested classes as well.

public static class Outer
{
    public static class InnerFirst
    {
    }

    public static class InnerSecond
    {
    }
}

Will create two entries under "Outer" with the name of ".cctr" and an empty signature

danielpalme · 2023-10-10T18:56:04Z

I just had a look at the two problems:

Duplicate Index method

This is the relevant information from coverage.cobertura.txt

<class name="CoverletRepro.TestController" filename="TestController.cs" line-rate="1" branch-rate="1" complexity="1">
  <methods>
    <method name="Index" signature="()" line-rate="1" branch-rate="1" complexity="1">
    ...
    
<class name="CoverletRepro.TestController/&lt;Index&gt;d__1" filename="TestController.cs" line-rate="1" branch-rate="1" complexity="1">
  <methods>
    <method name="MoveNext" signature="()" line-rate="1" branch-rate="1" complexity="1">

Both signature attributes show an empty method signature.
This results in the described output with two methods that look the same.

ReportGenerator does not parse the corresponding source code, it relies on the coverage file.
In this case the result is not perfect, but the input is neither.

Nested classes
This behavior is "as designed". Nested classes are processed as a part of the parent file.
Otherwise the HTML reports could become bloated with nested and compiler generated classes.

Methods and/or constructors of nested classes will be listed in the context of the parent class and therefore several entries with the same name and signature can appear. They are not referring to the same code lines though, you can see different <line> elements within those method elements.

Malivil · 2023-10-10T19:01:29Z

This originally came up because the code coverage plugin I use in Jenkins updated and now errors if there are duplicated entries (See jenkinsci/code-coverage-api-plugin#785).

Your point about the nested classes seems to say this behavior of rejecting duplicate method names is incorrect.

danielpalme · 2023-10-10T20:13:50Z

Your point about the nested classes seems to say this behavior of rejecting duplicate method names is incorrect.

"Incorrect" is a bit too hard. I would say, it would be good not to rely on unique names.
There are situations where it can happen (obviously).

uhafner · 2023-10-11T08:27:42Z

This behavior is "as designed". Nested classes are processed as a part of the parent file.
Otherwise the HTML reports could become bloated with nested and compiler generated classes.

I understand that this makes sense for the user interface, but wouldn't it be more appropriate if the model would retain the original structure and add these nested classes to the parent file? E.g. in JaCoCo reports those nested classes are also shown as direct descendants of the associated file, but they are not removed (i.e. the class in class hierarchy is flattened). Can't you ignore these classes in the HTML reports only?

I added a fix to ignore and skip those method nodes on my side (see jenkinsci/coverage-model#39) but it would be more appropriate if the generated model would retain this information.

danielpalme · 2023-10-11T16:11:45Z

@uhafner
You are right, for the Cobertura report it would make sense to retain the original structure.
I will have a look, if I can change the structure. But it will take some time, since this will require some rework and I'm pretty busy at the moment :-)

uhafner · 2023-10-11T17:24:01Z

No worries, I released a new version of the Jenkins coverage plugin that is more resilient about "errors" during parsing.

Malivil · 2023-10-11T19:00:31Z

I appreciate the dialog here and the work you've both done on your respective tools =)

danielpalme · 2024-01-06T09:44:00Z

I decided to not implement the required changes (at least not at the moment).

Reasons:

It would require quite a lot of work, as I would have to change all the different parsers.
It would be a breaking change for many users. One option could be, that the new behavior is only enabled with an additional command line option, but then only very few users will notice the new option.
For many .NET based projects merging coverage report is no longer necessary. If you use Microsoft.CodeCoverage and execute the tests at the solution level, you'll get a single coverage file.
See also: https://devblogs.microsoft.com/dotnet/whats-new-in-our-code-coverage-tooling/ (section "Auto-Merge for solutions")

viceice · 2024-03-15T21:33:59Z

Ms code coverage isn't a solution because of

[cobertura] Missing branch coverage when all branches are covered microsoft/codecoverage#104

…e data of nested or compiler generated classes is included in the parent class. This is useful to merge several Cobertura files into one file.

danielpalme · 2024-03-19T21:02:50Z

I just released version 5.2.4. This version includes a new setting (settings:rawMode=true).

The setting disables that coverage data of nested or compiler generated classes is included in the parent class.
This is useful to merge several Cobertura files into a single file, since the original class structure remains untouched.

Limitations:

Raw mode is currently only supported for Cobertura files.
If you want HTML report and Cobertura output, you should execute ReportGenerator twice:
- Cobertura: settings:rawMode=true
- HTML: settings:rawMode=false

You need a PRO license to use the new feature.

Documentation of the new feature:
https://reportgenerator.io/features#rawmode
https://reportgenerator.io/pro

Malivil mentioned this issue Oct 19, 2023

Exception for Cobertura reports that contain duplicate methods jenkinsci/code-coverage-api-plugin#785

Closed

medianick mentioned this issue Nov 1, 2023

Add a new option to ignore parser errors jenkinsci/code-coverage-api-plugin#788

Merged

danielpalme added the enhancement label Nov 17, 2023

danielpalme added the wontfix label Mar 15, 2024

danielpalme closed this as completed Mar 15, 2024

danielpalme added a commit that referenced this issue Mar 18, 2024

#630 Added "raw mode" (settings:rawMode=true) to disable that coverag…

792a664

…e data of nested or compiler generated classes is included in the parent class. This is useful to merge several Cobertura files into one file.

danielpalme added a commit that referenced this issue Mar 18, 2024

#630 Added "raw mode" (settings:rawMode=true) to disable that coverag…

d08defc

…e data of nested or compiler generated classes is included in the parent class. This is useful to merge several Cobertura files into one file.

danielpalme reopened this Mar 18, 2024

danielpalme added sponsorship Requires sponsorship and removed wontfix labels Mar 18, 2024

danielpalme closed this as completed Apr 17, 2024

theKBro mentioned this issue Aug 5, 2024

wrong handling of internal record structs #685

Closed

This was referenced Aug 28, 2024

Duplicate method names in Cobertura Reports #687

Closed

Distinct Method Class in Cobertura Report #688

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cobertura output async "class" merging can create duplicate method entries #630

Cobertura output async "class" merging can create duplicate method entries #630

Malivil commented Oct 10, 2023 •

edited

Loading

danielpalme commented Oct 10, 2023

Malivil commented Oct 10, 2023

danielpalme commented Oct 10, 2023

Malivil commented Oct 10, 2023

danielpalme commented Oct 10, 2023

uhafner commented Oct 11, 2023

danielpalme commented Oct 11, 2023

uhafner commented Oct 11, 2023

Malivil commented Oct 11, 2023

danielpalme commented Jan 6, 2024

viceice commented Mar 15, 2024

danielpalme commented Mar 19, 2024

Cobertura output async "class" merging can create duplicate method entries #630

Cobertura output async "class" merging can create duplicate method entries #630

Comments

Malivil commented Oct 10, 2023 • edited Loading

danielpalme commented Oct 10, 2023

Malivil commented Oct 10, 2023

danielpalme commented Oct 10, 2023

Malivil commented Oct 10, 2023

danielpalme commented Oct 10, 2023

uhafner commented Oct 11, 2023

danielpalme commented Oct 11, 2023

uhafner commented Oct 11, 2023

Malivil commented Oct 11, 2023

danielpalme commented Jan 6, 2024

viceice commented Mar 15, 2024

danielpalme commented Mar 19, 2024

Malivil commented Oct 10, 2023 •

edited

Loading