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

Option to ignore field/property initializers and auto-implemented properties #328

Closed
rickardp opened this issue Jan 29, 2019 · 12 comments · Fixed by #912
Closed

Option to ignore field/property initializers and auto-implemented properties #328

rickardp opened this issue Jan 29, 2019 · 12 comments · Fixed by #912
Labels
feature-request New feature request

Comments

@rickardp
Copy link

Consider a class, e.g.

class MyClass
{
    public int MyValue { get; set; }
}

The property would be excluded because it's getter/setter has the CompilerGeneratedAttribute set. Now consider this class:

class MyClass
{
    public int MyValue { get; set; } = 42;
}

The line is included in coverage. I think this is because in the IL code, the initializers are actually part of the constructor. While this may be a sensible default even if explicitly opting out of compiler generated code, there should be an option to exclude initializers from coverage analysis.

The use case for this is when you have large model objects that contain code that you actually want coverage on, but also contains lots of pure data members.

Suggestion is to include an option, e.g. ExcludeInitializers, that ignores all initializers from coverage.

I saw e.g. issue #316, but I would argue something more specific than an arbitrary "IgnoreTrivial" is more suitable.

@rickardp
Copy link
Author

rickardp commented Jan 29, 2019

I had a quick look at the IL code and I think it could be implemented quite easily by opting to ignore all IL code until the super constructor is called. This would work for both implicit and declared constructors.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 2, 2019

For reference OpenCover has got -skipautoprops
/cc @tonerdo @petli

@rickardp
Copy link
Author

rickardp commented Feb 2, 2019

@MarcoRossignoli Auto properties without initializers are already handled. Does OpenCover handle initializers?

@MarcoRossignoli MarcoRossignoli added the enhancement General enhancement request label May 6, 2019
@MarcoRossignoli MarcoRossignoli added feature-request New feature request and removed enhancement General enhancement request labels Sep 12, 2019
@MarcoRossignoli MarcoRossignoli changed the title Option to ignore field/property initializers Option to ignore field/property initializers and auto-implemented properties Apr 10, 2020
@MarcoRossignoli MarcoRossignoli pinned this issue Apr 10, 2020
@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jul 12, 2020

@MarcoRossignoli Auto properties without initializers are already handled. Does OpenCover handle initializers?

Not at the moment
https://github.com/OpenCover/opencover/blob/67980c8132b1d574beb1feaa7d269c9dca47b812/main/OpenCover.Framework/Symbols/CecilSymbolManager.cs#L328
For now we'll implement same feature, ctor initialization is a bit complex to handle, we could have also other(non autogenerated) code inside ctor.
image

@svengeance
Copy link

svengeance commented Aug 4, 2020

Just passing by - I was able to use this today effectively, as the overall coverage % wasn't valuable to me while I was unable to skip such trivial code. I was searching for such a feature all of Friday, and just saw it in the documentation today.

So thank you, this was wonderful timing.

@ulrichb
Copy link

ulrichb commented Sep 30, 2020

As of coverlet.collector 1.3.0 this still doesn't seem to work:

image

My config:

<?xml version="1.0" encoding="utf-8"?>

<RunSettings>
    <DataCollectionRunSettings>
        <DataCollectors>
            <DataCollector friendlyName="XPlat code coverage">
                <Configuration>
                    <Format>opencover</Format>
                    <Include>[...snip...]</Include>
                    <ExcludeByAttribute>GeneratedCodeAttribute,TestSDKAutoGeneratedCode</ExcludeByAttribute>
                    <ExcludeByFile>**/*.Designer.cs</ExcludeByFile>
                    <IncludeTestAssembly>true</IncludeTestAssembly>
                    <SkipAutoProps>true</SkipAutoProps>
                </Configuration>
            </DataCollector>
        </DataCollectors>
    </DataCollectionRunSettings>
</RunSettings>

@ulrichb
Copy link

ulrichb commented Sep 30, 2020

... oh just saw that #912 has't been released yet. Right?

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Oct 1, 2020

Right?

Yep

@wdspider
Copy link

wdspider commented Apr 8, 2021

    public sealed record RouteResponse
    {
        public string Id { get; init; } = string.Empty;
    }

I'm still experiencing that autoprops like these in records are still counting against coverage even though SkipAutoProps is set to true. Will the fix mentioned in the thread above fix this issue too? If so, when will it be released for the coverlet.collector ?

@Malivil
Copy link

Malivil commented Apr 8, 2021

This change has already been implemented and doesn't cover the case where an auto-prop has an inline assignment like that.
I would file another issue about that so it can be handled separately.

@wdspider
Copy link

wdspider commented Apr 8, 2021

This change has already been implemented and doesn't cover the case where an auto-prop has an inline assignment like that.
I would file another issue about that so it can be handled separately.

Opened #1139 as requested.

@StingyJack
Copy link
Contributor

Consider a class, e.g.

class MyClass
{
    public int MyValue { get; set; }
}

The property would be excluded because it's getter/setter has the CompilerGeneratedAttribute set. Now consider this class:

@rickardp - If its reporting a class member that is unused or untouched by any of the unit tests it should be reported as not covered. Is this what you mean?

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

Successfully merging a pull request may close this issue.

7 participants