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

ExcludeFromCodeCoverage attribute is ignored in an static class in a ASP.NET Core 3.1 project #670

Closed
jmoralesv opened this issue Dec 26, 2019 · 9 comments · Fixed by #671
Labels
bug Something isn't working duplicate This issue or pull request already exists tenet-coverage Issue related to possible incorrect coverage

Comments

@jmoralesv
Copy link

Hi all,
I'm facing an issue when using Coverlet in a ASP.NET Core 3.1 project. I can see that a static class, which has a static void method, is not being excluded from code coverage, although both the class and the method have the ExcludeFromCodeCoverage attribute applied, from the namespace System.Diagnostics.CodeAnalysis.

This static method is an extension method referenced by the Startup class in my ASP.NET Core 3.1 project, Startup and Program classes are also excluded from code coverage and they don't appear in the code coverage results, however my static class and its static void method appear in the results.

Is this a known issue with Coverlet, .NET Core 3.1 and static classes and methods? This is the command I'm running to get the results:

dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=cobertura

My unit test project references coverlet.msbuild with version 2.7.0. It also references Xunit, version 2.4.1, and my ASP.NET Core 3.1 project. The unit test project also targets .NET Core 3.1. This is extracted from my csproj file for the reference to coverlet.msbuild:

<PackageReference Include="coverlet.msbuild" Version="2.7.0"> <PrivateAssets>all</PrivateAssets> <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets> </PackageReference>

I found this issue in my build pipeline in Azure DevOps too.

Finally, I'd like to mention that the code coverage tools included in Visual Studio 2019 Enterprise, version 16.4.2, do exclude the class and the method from code coverage, so I think this is an issue with Coverlet itself, or the commands I'm using with dotnet test.

Please advise.

Regards,
Jorge

@MarcoRossignoli
Copy link
Collaborator

Thank's @jmoralesv for reporting this, we'll take a look ASAP.

@MarcoRossignoli MarcoRossignoli added the untriaged To be investigated label Dec 27, 2019
@MarcoRossignoli MarcoRossignoli added needs more info More details are needed and removed untriaged To be investigated labels Jan 3, 2020
@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jan 3, 2020

@jmoralesv the code inside the static method with attribute applied has got some async/await code?
We have an open issue that could be similar to your one #129

@jmoralesv
Copy link
Author

Hi @MarcoRossignoli ,
Yes, the static method is actually an extension method for IApplicationBuilder, and it resembles the following code snippet:

using System.Diagnostics.CodeAnalysis;
using System.Net;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Diagnostics;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;

[ExcludeFromCodeCoverage]
public static class ExceptionMiddlewareExtensions
{
	[ExcludeFromCodeCoverage]
	public static void ConfigureExceptionHandler(this IApplicationBuilder app, ILogger logger)
	{
		app.UseExceptionHandler(appBuilder =>
		{
			appBuilder.Run(async context =>
			{
				context.Response.StatusCode = (int)HttpStatusCode.InternalServerError;
				context.Response.ContentType = "application/json";

				var contextFeature = context.Features.Get<IExceptionHandlerPathFeature>();
				if (contextFeature != null)
				{
					logger.LogError($"Something went wrong at {contextFeature.Path}. Error: {contextFeature.Error}");

					await context.Response.WriteAsync($"{ 'StatusCode': {context.Response.StatusCode}, 'Message': 'Internal Server Error' }");
				}
			});
		});
	}
}

The extra attribute [ExcludeFromCodeCoverage] in the static method shouldn't be required, I added just to confirm that Coverlet is also ignoring it even though it is defined at the class level.

I checked issue #129 and I think it could be the same, although the context is different, as my static class is being referenced by my Startup class which is not static and has [ExcludeFromCodeCoverage] as well. My Startup class doesn't appear in the Coverlet results, however the static class ExceptionMiddlewareExtensions does.

A snippet of my Startup class is as follows:

[ExcludeFromCodeCoverage]
public class Startup
{
	// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
	public void Configure(IApplicationBuilder app, IWebHostEnvironment env, ILogger<Startup> logger)
	{
		app.ConfigureExceptionHandler(logger);
		
		// The rest of the code was removed for brevity
	}
}

Please let me know if you need any other information.

@MarcoRossignoli
Copy link
Collaborator

My Startup class doesn't appear in the Coverlet results, however the static class ExceptionMiddlewareExtensions does.

I have to repro but I think that it's the same issue, static class is present because the async/await state machine is generated inside that class by compiler.

@jmoralesv
Copy link
Author

Hi @MarcoRossignoli ,
I can try creating a new ASP.NET Core 3.1 Web API project and add the extension method and the static class there, but that would take some time.

I agree with you, it's the same issue, having async/await code with static methods or classes makes Coverlet ignore the [ExcludeFromCodeCoverage] attribute.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jan 4, 2020

but that would take some time.

Don't worry your code fragment is enough to me
We have an open PR almost ready #671

@jmoralesv
Copy link
Author

Hi @MarcoRossignoli,
Thank you, I'm looking forward to seeing the issue fixed in the next version of Coverlet 👍

@MarcoRossignoli
Copy link
Collaborator

Sure thing!

@MarcoRossignoli MarcoRossignoli added duplicate This issue or pull request already exists tenet-coverage Issue related to possible incorrect coverage bug Something isn't working and removed needs more info More details are needed labels Jan 5, 2020
@MarcoRossignoli
Copy link
Collaborator

@jmoralesv I did a repro and the issue is "related" to the other, btw for now I'll leave this open because I don't know if the fix will work with "nested" async machine state.

image

For now you can remove that file from coverage using filters,

dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=cobertura /p:CoverletOutput=./Results/ /p:ExcludeByAttribute="CompilerGeneratedAttribute"

ExcludeByAttribute exclude by attribute name and CompilerGeneratedAttribute is the attribute added by the compiler to the state machines

image

Or you can use filter by filename

dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=cobertura /p:CoverletOutput=./Results/ /p:ExcludeByFile=\"**/Ext.cs\"

In my repro I put the extension inside a file called Ext.cs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists tenet-coverage Issue related to possible incorrect coverage
Projects
None yet
2 participants