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

CompilerGeneratedAttribute excludes new class #1437

Closed
cheesi opened this issue Jan 18, 2023 · 7 comments
Closed

CompilerGeneratedAttribute excludes new class #1437

cheesi opened this issue Jan 18, 2023 · 7 comments
Labels
as-designed Expected behaviour with repro Issue with repro

Comments

@cheesi
Copy link

cheesi commented Jan 18, 2023

Hello!

We have a strange issue in our project.

We added a new class that basically looks like this:

using MediatR;
using System.Transactions;
using ORG.PROJECT.OTHERPACKAGE;

namespace ORG.PROJECT.PACKAGE.Behaviours;

public class TransactionBehaviour<TRequest, TResponse> : IPipelineBehavior<TRequest, TResponse>
    where TRequest : IRequest<TResponse>, ITransactionCommand
{
    public async Task<TResponse> Handle(
        TRequest request,
        RequestHandlerDelegate<TResponse> next,
        CancellationToken cancellationToken)
    {
        using (var scope = new TransactionScope(
            TransactionScopeOption.Required,
            new TransactionOptions { IsolationLevel = request.TransactionIsolationLevel },
            TransactionScopeAsyncFlowOption.Enabled
            ))
        {
            var result = await next();
            scope.Complete();
            return result;
        }
    }
}

This new class (&method) does not show up in our code coverage report:
image
I also already ruled out any display or converting issues (report generator), it is definitely already missing in the opencover file, we get from our build.

However, all other classes from that package are found. The strange thing is, that our other Behaviours classes are really similar. Like same generics, same method definition and so on. The only difference would be our ITransactionCommand interface, but also already tried removing that.

After some digging I found out, that this class gets excluded with the CompilerGeneratedAttribute

Our test execution looks like this:

- task: DotNetCoreCLI@2
  displayName: Test application
  inputs:
    command: test
    projects: '**/*Tests/*.csproj'
    arguments: >-
      --configuration $(buildConfiguration)
      --collect:"XPlat Code Coverage"
      -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.Format=opencover
      -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.ExcludeByFile=**/Migrations/*.cs
      -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.ExcludeByAttribute=Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute

As soon as I remove the CompilerGeneratedAttribute from the ExcludedByAttribute list, the new class gets detected.

Our environment:

  • .NET 7.0.2
  • Microsoft.NET.Test.Sdk 17.4.1
  • xunit 2.4.2
  • xunit.runner.visualstudio 2.4.5
  • coverlet.collector 3.2.0
  • Ubuntu 22.04

This looks like a bug to me and to be honest, I have zero clue, why it is behaving in that way.
Some differences to our other behaviours:

  • Dependency to System.Transactions
  • using statement for IDisposable
  • Only this one Handle method, instead of multiple methods (that are called by the Handle method)

Apparently this was already a problem in the past: #794

If needed, I can try to create a minimal reproducible repo.

@cheesi cheesi changed the title CompilerGeneratedAttribute excludes new class CompilerGeneratedAttribute excludes new class Jan 18, 2023
@cheesi
Copy link
Author

cheesi commented Jan 23, 2023

I create a small reproducible repo here: https://github.com/cheesi/coverlet-issue-1437

I also checked in the two generated coverage results, one with the CompilerGeneratedAttribute and one without it.

Command:
dotnet test --configuration Release --collect:"XPlat Code Coverage" -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.Format=opencover -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.ExcludeByFile=**/Migrations/*.cs -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.ExcludeByAttribute=Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute

@daveMueller
Copy link
Collaborator

@cheesi Thanks a lot for the repro, as this makes it much easier for us 🙏 . We will take a look asap.

@daveMueller daveMueller added untriaged To be investigated with repro Issue with repro labels Jan 24, 2023
@daveMueller
Copy link
Collaborator

OK I analyzed this today and this is the correct behaviour if excluded by CompilerGeneratedAttribute. Every async state machine (generated class by roslyn compiler for async methods) has this attribute. In your case the whole class TransactionBehaviour is being skipped because it only has one method (for which roslyn generates the async state machine).

For instance if you leave the CompilerGeneratedAttribute and add another non-async method to the class, the class isn't skipped anymore. Although the async method Handle is still skipped. You can also see it in this issue #794 (comment).

image

@daveMueller daveMueller added as-designed Expected behaviour and removed untriaged To be investigated labels Feb 4, 2023
@cheesi
Copy link
Author

cheesi commented Feb 4, 2023

Hey @daveMueller !

Thank you very much for your help!

So the suggested way is to remove the CompilerGeneratedAttribute from the ExcludeByAttribute? Won't this have any other unwanted implications?

@daveMueller
Copy link
Collaborator

Yes I would advise to remove it. Only thing that I have in mind are auto-implemented properties that after removing could be reported as uncovered (instead of beeing skipped). But therefore there is the SkipAutoProps option (https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md#skip-auto-implemented-properties).

@petli @MarcoRossignoli @tonerdo Any other thoughts on this?

@MarcoRossignoli
Copy link
Collaborator

We cannot do a lot for it as @daveMueller explained, I also suggest to remove the attribute and handle the "noise" with other filtering.

@cheesi
Copy link
Author

cheesi commented Feb 6, 2023

Alright, thanks for your help guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as-designed Expected behaviour with repro Issue with repro
Projects
None yet
Development

No branches or pull requests

3 participants