ExcludeFromCodeCoverage not working for autogen getters and setters

[pape77]

Noticed that when trying to apply the excludeFromCoverage attribute to getters and setters from a
class property, they are not taken into account

   public class Key
    {
       [ExcludeFromCodeCoverage]
        public int Id { get; set; }

        public Guid Uuid { get; set; }

        public Guid Kid { get; set; }
    }

"/build/src/Keys.Data/Entities/Key.cs": {
      "Keys.Data.Entities.Key": {
        "System.Int32 Keys.Data.Entities.Key::get_Id()": {
          "Lines": {
            "10": 0
          },

This can easily be seen in action in existing tests within coverlet. In InstrumenterTests.cs, in the TestInstrument_ClassesWithExcludeAttributeAreExcluded
Just remove the [ExcludeFromCodeCoverage] attribute from one of the classes that this method uses in Samples.cs and add a property like this Id one.

For example in Samples.cs change this class as follows:

    public class ClassExcludedByCoverletCodeCoverageAttr
    {

        public string Method(string input)
        {
            if(string.IsNullOrEmpty(input))
                throw new ArgumentException("Cannot be empty", nameof(input));

            return input;
        }

        [ExcludeFromCodeCoverage]
        public int Id { get; set; }
    }

Then you will that in method InstrumentType from Instrumenter.cs the attributes don't show up, because the method name is changed to get_Id() (autogenerated) and so it gets instrumented anyway.

Can we fix this somehow?

[pape77]

Similar thing happens with async methods which are read like this (with MoveNext stuff):

"Keys.Data.Repositories.KeyRepository/<FindOrCreateAsync>d__6": {
        "System.Void Keys.Data.Repositories.KeyRepository/<FindOrCreateAsync>d__6::MoveNext()":
"Lines": {
            "55": 2,
            "56": 2,
            "58": 2,
  ...
}
}

and then all attributes are lost when Instrumenter.cs 's method is evauluating

[SlowLogicBoy]

try:

public int MyProperty { [ExcludeFromCodeCoverage] get; [ExcludeFromCodeCoverage] set; }

[consultwithmike]

@SlowLogicBoy that workaround does work, but I'd say we need to get something in place that iterates the Properties collection in the Iterator as well so that we don't have to decorate the get and set, but rather the property itself.

Honestly, I don't even think we'd have to support properties, we could just add the get and set accessors to the methods listing if we see the property is excluded.

[pape77]

Thank's for the hack @SlowLogicBoy
Indeed @consultwithmike that's what I expected to happen.
I'm guessing for the async things, a similar thing happens, cuz they are autogenerated with that name or something.
I looked into the Instrumenter.cs a bit, but got no clear solution for it

[MarcoRossignoli]

@pape77 is this issue still here?

[MarcoRossignoli]

Now we can use p:ExcludeByAttribute=\"CompilerGeneratedAttribute\" to skip autogen prop

[StingyJack]

@SlowLogicBoy @MarcoRossignoli - this is still a problem for properties

Should this issue really be closed?

[MarcoRossignoli]

This issue is old can you open new one?
Which version are you using?

[StingyJack]

I was going to but found that a similar issue was opened after this and was closed as a duplicate of this.

[StingyJack]

@MarcoRossignoli As far as I can tell this was never actually fixed so closing this was not legitimate action. There is no PR or commit linked to this, just a comment for what looks like an msbuild property.

Besides like I said, other issues were opened for this same problem and were closed as duplicates of this.

[MarcoRossignoli]

@StingyJack which version are you using?can you provide a repro?

Can you try also the new feature to skip autoprop?https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md#skip-auto-implemented-properties

[StingyJack]

Both versions 1.3.0 and 3.0.3 had the same problem.

I'm not invoking this by command line, another tool is doing the invocation that I don't have immediate control of.

The repro is to put a property in a class, run coverage, see that it's uncovered, then apply the excluded attribute on the property and rerun coverage. It still shows up as uncovered. The workaround of applying the attribute to each the get and set does work, but leaves messy code

EDIT: Perhaps an example can explain better.

//when using this, it reports the property as "not covered"
public class User 
{
    [ExcludeFromCodeCoverage] 
    public object Id { get;  set; }
...
}

//when using this, it reports the property as "covered", but seems like it should not be necessary.
public class User 
{
    public object Id { [ExcludeFromCodeCoverage] get; [ExcludeFromCodeCoverage] set; }
...
}

[StingyJack]

Also I dont want to skip auto properties in the first place. Any property that is not listed as covered by a unit test is something that may be unused by the code and can be removed (YAGNI), or something I should have an assertion for in a unit test (and thus make covered). There are just some fields that are handled by an ORM or DB driver that I wouldn't unit test for, or temporary cases where the properties are needed by someone else's code that will be covering them later, but my PR needs to meet or exceed a coverage threshold to be merged.

Usages of [ExcludedFromCodeCoverage] for us need to be followed by a justification comment or it could block the changes from being merged into main. I think we may have to disable that if we cant follow the attribute with a justification comment, but I have to see if it will accept a /**/ style comment

[MarcoRossignoli]

Ok reopening for further investigation