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

Feature request: Code Coverage exclusion on line level #23

Open
dozer75 opened this issue Feb 9, 2022 · 5 comments
Open

Feature request: Code Coverage exclusion on line level #23

dozer75 opened this issue Feb 9, 2022 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@dozer75
Copy link

dozer75 commented Feb 9, 2022

Description

This one is a tricky one to find the correct place to report, I have tried to look around in different repos that has issues that mentions ExcludeFromCodeCoverage (e.g. microsoft/vstest#2658 and dotnet/runtime#45374 ), but it seems that the responsibility for this one is a bit vague since the implementation (dotnet/runtime) and usage of this attribute (microsoft/vstest + 3rd party vendors) is assigned to different projects. But anyway, here is a description and you can choose how this should be taken further.

Today we have the ExcludeFromCodeCoverage attribute which is by Visual Studio Code Coverage (and other 3rd party vendors) used to exclude code on different levels of code like class, methods, properties and so on.

However, there is one situation where an attribute cannot be used, but I think that it should be some way to do this at that level as well. And that is on code line level.

The problem arises if you for example has a switch where the default statement is never hit (and it impossible to come to that location otherwise).

For example, you have a method that looks like this:

private static bool SomeFunc(AnEnum anEnum)
{
    switch(anEnum)
    {
        case AnEnum.Val1:
            // something
            return true;
        case AnEnum.Val2
            // something else
            return false;
        default:
            // This line is never hit since it is stopped by other code, it's only here because a default is needed
            // due to return statement in function
            throw new ArgumentOutOfRangeException(nameof(anEnum), anEnum, $"The value {anEnum} is not supported.");
    }
}

Since this is private method and the calling method already prohibits anEnum values (due to other logic), no tests can hit the default clause. So, it would be nice to exclude that from code coverage. It's simply there for the events when someone screws up something and causes havoc (and the return statement requires some kind of return that won't be hit otherwise a compile error is issued).

I am not sure how this could be solved. My first thought was preprocessor declarations like eg #skipcodecoverage enable #skipcodecoverage disable, but I'm not sure if code coverage can get these preprocessing directives. Other could be that we on ExcludeFromCodeCoverage could set attributes that excludes certain things e.g. [ExcludeFromCodeCoverage(IgnoreSwitchDefault = true)] or [ExcludefromCodeCoverage(IgnoreExceptions=typeof(ArgumentOutOfRangeException)]

Without this possibility it's difficult to get a clean code coverage and you always checks "why did not the code coverage returned % which I'm expecting"..

@jakubch1 jakubch1 self-assigned this Feb 11, 2022
@StingyJack
Copy link

The problem arises if you for example has a switch where the default statement is never hit (and it impossible to come to that location otherwise).

@dozer75- can you omit the default statement and put the throw after the switch or does AnEnum have more than two values? Perhaps an if/elseif instead of a switch? Getting 100% coverage usually means a lot of diminishing returns to go through once the percentage gets over ~95%. What you have already may not be perfect, but its probably good enough.

@jakubch1 - In TS/JS there is an /* istanbul ignore next */ which is really handy for excluding the next whatever (statement, if condition and its block, function call, class, etc). It would be equally handy to have something like this for dotnet.

@jakubch1
Copy link
Member

jakubch1 commented Feb 21, 2022

For several reasons we are not using source code for code coverage so we don't have access to any comments. We are working on IL level where we also try to not detect any patterns as they can change if roslyn will make some changes in future.

Few options to fix issue with switch default.

  1. Make your method internal and call it from your unit tests with (MyEnum)100. Expect in test to get exception.
  2. Use switch expression (https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression) without specifying default. Example is below. In this case called with (MyEnum)100 method will throw System.Runtime.CompilerServices.SwitchExpressionException. This code normally produces warning which can be removed with #pragma warning disable CS8524
    public bool MyMethod2(MyEnum myEnum)
    {
#pragma warning disable CS8524 // The switch expression does not handle some values of its input type (it is not exhaustive) involving an unnamed enum value.
        return myEnum switch
        {
            MyEnum.First => true,
            MyEnum.Second => false
        };
#pragma warning restore CS8524 // The switch expression does not handle some values of its input type (it is not exhaustive) involving an unnamed enum value.
    }

@StingyJack
Copy link

For several reasons we are not using source code for code coverage so we don't have access to any comments.

@jakubch1 - I was giving an example from another language. It need not be comments. Curious though how is possible to display source code highlighted in green (covered) or red (not covered) without access to the source code.

@jakubch1
Copy link
Member

To collect code coverage we need dll/exe files and pdb. We are just storing information for example that in file a.cs lines 10-15 are covered and we don't need to read a.cs file. To show highlighting sure we need sources.

@dozer75
Copy link
Author

dozer75 commented Feb 23, 2022

@StingyJack @jakubch1

Well, as said, this was an example of code that this request would cover this since this both is the most common "problem" for this and the easiest to reproduce. And yes, there are ways around the problem as well. But most of them are more or less ugly workarounds (like using internal which break good code quality since the methods then are available outside the class).

And, as I said, I have no idea how to solve it since I know that CC is not working at code level as said in this thread previously, this was more to throw the issue out and hoping that it could be taken into consideration at some point.

(PS: 100% CC is not (and should never be) a requirement, but unnecessary leakages would be nice to have an easy way around no matter what situation like other languages has :) )

@jakubch1 jakubch1 transferred this issue from microsoft/vstest Mar 21, 2023
@jakubch1 jakubch1 added the enhancement New feature or request label Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants