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

Visual Studio .coveragexml: ReportGenerator ignores compiler-generated classes #625

Closed
RoadTrain opened this issue Sep 20, 2023 · 11 comments

Comments

@RoadTrain
Copy link

Describe the bug
Hi!
When class methods contain local functions, the .net compiler (roslyn-era) generates separate classes for them. These classes (and their methods) are ignored by ReportGenerator, while there can be a lot of logic in them. The main problem is that line and block coverages of these local functions is lost in the generated report. We have classes which mainly consist of local functions, and the coverage metrics are very off for them, compared to what Visual Studio shows.

I have found a very similar issue which was closed without resolution #120.

To Reproduce
I have attached a sample hand-crafted coveragexml where class MyScenario contains both a class method and two local functions and a resulting report. Line numbers are off but it doesn't matter because you can still see the main problem -- compiler-generated classes are ignored.
vs_issue_sample.zip

@danielpalme
Copy link
Owner

Thanks for the detailed description and the sample.
I will have a look as soon as possible.

@RoadTrain
Copy link
Author

RoadTrain commented Sep 20, 2023

After taking a quick glance at the code, I can see two things:

  1. Currently there's a special case for compiler-generated iterator methods:
    if (methodKeyName.Contains("MoveNext()"))
  2. Lambdas (and consequently local functions) are excluded (which dates back to the initial import in 2015):
    || LambdaMethodNameRegex.IsMatch(fullName))

@danielpalme
Copy link
Owner

danielpalme commented Sep 24, 2023

I just found time to have a closer look. Sorry for the delay.

The main problem is that line and block coverages of these local functions is lost in the generated report. We have classes which mainly consist of local functions, and the coverage metrics are very off for them, compared to what Visual Studio shows.

Compiler generated classes are not listed on the summary page, that's correct. But the relevant lines are processed within the corresponding parent class.
In your example the summary report only shows the classes MyLogic and MyScenario.
But if you look into the MyScenario report, you will see that all coverable lines are considered. Even the block with line 729-738, which belongs to a compiler generated class.

  1. Currently there's a special case for compiler-generated iterator methods:

Yes, but that's just a naming thing. The name MoveNext is not very helpful, instead the "correct" method name is retrieved.

  1. Lambdas (and consequently local functions) are excluded

Yes, but only in the list of methods and their metrics. This does not affect the overall code coverage.

Does this help? I'm I missing something?

@RoadTrain
Copy link
Author

RoadTrain commented Sep 25, 2023

The main issue we had is the 'Blocks covered' method metric which was incorrect (i.e. apparently not including compiler-generated local functions). It seems to me that the Lines metric is calculated correctly indeed.

Can you take a look at how it's calculated? Maybe the same 'aggregation' logic is missing for block coverage?

@RoadTrain
Copy link
Author

And what about properties? They are excluded from the report too, do they count towards line metrics like compiler-generated methods do? In theory, property getters/setters can (and often do) have some logic in them, so they should count towards line/block coverage metrics.

@RoadTrain
Copy link
Author

We have been digging further, and now I'm not sure there's a problem with the library. Please put this on hold, I'll be back with a verdict shortly.

@RoadTrain
Copy link
Author

So our current understanding is that including lambda/local functions in total metrics (as suggested by me) is incorrect because they are already included by Visual Studio coverage. So the premise of this bug report is false, sorry for that.

They only thing that we would like to have is the ability to "see" these functions, i.e. be able to extract their metadata with ReportGenerator.Core library to show them on our dashboard, without affecting metrics.

@danielpalme
Copy link
Owner

They only thing that we would like to have is the ability to "see" these functions, i.e. be able to extract their metadata with ReportGenerator.Core library to show them on our dashboard, without affecting metrics.

I will see what I can do. Will come back to you as soon as possible.

@danielpalme
Copy link
Owner

danielpalme commented Oct 4, 2023

I think I have found a solution. Will publish a new release in the next days.

@danielpalme
Copy link
Owner

Release 5.1.26 is now available. Could you please test if this works for you?

@RoadTrain
Copy link
Author

Thanks! I will test it in the following days and come back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants