microsoft / codecoverage

MIT License
84 stars 11 forks source link

ReportGenerator producing duplicate class rows in summary (cobertura format) #124

Closed standsed closed 5 months ago

standsed commented 5 months ago

In scenario below I am getting duplicate rows (for the same class) in ReportGenerator summary output

For following code

public static class Class1
{
    public static async Task<string> Method1() { return "asdf"; }

    public static async Task<string> Method2<T>(T val) { if (val is string x) { return x; } throw new NotSupportedException(); }

    public static async Task<string> Method3<T, TU>(T val) { return "asdf"; }
}

Generated report looks as follows image

Full project where this is reproduced https://github.com/standsed/CodeCoverageRepro

Issue seems arise from class name in produced cobertura xml file. 1st row is from codecoverage file 2nd from coverlet

Repro.Class1.&lt;Method3&gt;d__2&lt;T, TU&gt;
Repro.Class1/&lt;Method3&gt;d__2`2

Repro.Class1.&lt;Method2&gt;d__1&lt;T&gt;
Repro.Class1/&lt;Method2&gt;d__1`1

Repro.Class1.&lt;Method1&gt;d__0
Repro.Class1/&lt;Method1&gt;d__0

Looks like dot after Class1 breaks some aggregation in ReportGenerator. If I replace this dot with / as in coverlet then report correctly displays just one row for Class1. coverlet.txt ms_codecoverage.txt

Since dot is a namespace delimiter this could be something which might need to be adjusted in codecoverage, but let me know if you see this otherwise.

fhnaseer commented 5 months ago

Can you please use latest version of reportgenerator and try again?

This has been fixed in reportgenerator => https://github.com/danielpalme/ReportGenerator/commit/edb7491fe5055f7a95888e47426bec29899a5fae#diff-dc56cf0bce5eabc34a9b64648410d7f2edf7673cfb44bb447bf0903387d245a4

standsed commented 5 months ago

I used latest released reportgenerator version 5.3.6 So unfortunately looks like some cases has slipped by in this fix

fhnaseer commented 5 months ago

I would recommend to share these cases with them. Hopefully they will fix these cases as well. Here is the existing issue.

danielpalme commented 5 months ago

I will have a look in the next days.

danielpalme commented 5 months ago

I took a look at those class names.

With ReportGenerator version 5.3.6 the part ".d__0" is removed from the classes. This results in the following names:

Input Output
Repro.Class1.<Method1>d__0 Repro.Class1
Repro.Class1.<Method2>d__1<T> Repro.Class1<T>
Repro.Class1.<Method3>d__2<T, TU> Repro.Class1<T, TU>`

If I would also remove the generic type <...> at the end, the result would be correct in your case. Reason: In your case the generic type at the end is related to the internal method.

But there are other cases where the generic type at the end is related to the class itself.

Example: Test.GenericAsyncClass.<MyAsyncMethod>d__0<T>

Corresponding class:

public class GenericAsyncClass<T>
{
  public async Task MyAsyncMethod()
  {
  }
}

The only way to distinguish between these two variants is @standsed proposal:

Since dot is a namespace delimiter this could be something which might need to be adjusted in codecoverage, but let me know if you see this otherwise.

It would be perfect if Microsoft CodeCoverage could use / as a delimiter for nested classes instead of a dot. With that change the distinction is possible. Coverlet is already implemented that way.

With coverlet the class Test.GenericAsyncClass.<MyAsyncMethod>d__0<T> becomes Test.GenericAsyncClass1/<MyAsyncMethod>d__0`.

fhnaseer commented 5 months ago

We keep the names as it is when generated. . to represent nested classes. This is also represented in the Code Coverage Results window in the Visual Studio Enterprise. Changing . to / will be a breaking change and will effect existing users.

public static class Class1
{
    [CompilerGenerated]
    private sealed class <Method1>d__0 : IAsyncStateMachine

https://sharplab.io/#v2:CYLg1APgAgTAjAWAFBQAwAIpwKwG5nJQDMmcAbJjOgMIA2AhgM6OJIDey6XmJWFUADkxkAPFlQA+dAFkApgBcAFgHtgcABQBKdG0wB2dACImwAGaHc6AL4Ek3HqX5Coo8VLlLVMEQBUJ6n3QAN3pabV0AS1N0dRDadAjGUgwAD3D9dBTLK3QlACdlAHd0ADtZYoA5ZXkAZQBXAAcG5Tz5WWAAURSAY1kG+QjlEq1s23tiR0xnVzhJGQUVYCJfABp0HwBVf0C49KgDY0YzC2tkKyA