microsoft / codecoverage

MIT License
84 stars 11 forks source link

Compiler-generated branch for method group caching is not excluded #94

Closed alexthornton1 closed 8 months ago

alexthornton1 commented 9 months ago

Starting with a .NET 6 project with 100% line and branch coverage, when I upgraded it to .NET 8, that coverage metric dropped slightly. Strangely, the missing coverage was being reported on a line containing a sequence of System.Linq calls, but containing no branches, leading me to believe that the branch was compiler-generated.

After digging into the issue a bit, it turned out to be the same issue captured in this simplified example:

public static class Repro
{
    public static IEnumerable<string> FilterAndConcat(
        IEnumerable<string> firstTexts, IEnumerable<string> secondTexts)
    {
        var filteredFirstTexts = firstTexts.Where(IsLongString);
        var filteredSecondTexts = secondTexts.Where(IsLongString);

        return filteredFirstTexts.Concat(filteredSecondTexts);
    }

    private static bool IsLongString(string text)
    {
        return text.Length > 3;
    }
}

Suppose that I run these two Xunit-based tests:

public class ReproTest
{
    [Theory]
    [InlineData(
        new string[] { "Boo Bear", "and", "Chancy" }, new string[] { "both", "are", "happy" },
        new string[] { "Boo Bear", "Chancy", "both", "happy" })]
    [InlineData(
        new string[] { "Chancy", "and", "Boo Bear" }, new string[] { "different", "but", "perfect" },
        new string[] { "Chancy", "Boo Bear", "different", "perfect" })]
    public void CanFilterAndConcat(string[] firstTexts, string[] secondTexts, string[] expected)
    {
        var result = Repro.FilterAndConcat(firstTexts, secondTexts);

        Assert.Equal(expected, result);
    }
}

The following coverage result is reported by Visual Studio Enterprise 2022 17.8.5:

image

The root cause of the problem is a compiler-generated branch that caches a delegate built for the IsLongString method group, which appears to be a compiler feature added in .NET 7, as described in https://github.com/dotnet/roslyn/issues/5835.

If I plug the repro code above into https://sharplab.io/, it shows the following C# equivalent of the generated IL:

[NullableContext(1)]
[Nullable(0)]
public static class Repro
{
    [CompilerGenerated]
    private static class <>O
    {
        [Nullable(0)]
        public static Func<string, bool> <0>__IsLongString;
    }

    public static IEnumerable<string> FilterAndConcat(IEnumerable<string> firstTexts, IEnumerable<string> secondTexts)
    {
        IEnumerable<string> first = Enumerable.Where(firstTexts, <>O.<0>__IsLongString ?? (<>O.<0>__IsLongString = new Func<string, bool>(IsLongString)));
        IEnumerable<string> second = Enumerable.Where(secondTexts, <>O.<0>__IsLongString ?? (<>O.<0>__IsLongString = new Func<string, bool>(IsLongString)));
        return Enumerable.Concat(first, second);
    }

    private static bool IsLongString(string text)
    {
        return text.Length > 3;
    }
}

The problem is this:

Consequently, it becomes impossible to achieve full coverage on the Repro.FilterAndConcat method -- or, more generally, to achieve full coverage on any class in which the same method group is cached in more than one place, since all will share the same cached static field, but only one will ever be able to initialize it.

I've attached a complete Visual Studio solution containing the repro described above: Experiments.TestCoverage.zip

For what it's worth, I found that the same problem affected Coverlet -- which I'm using in a CI build process -- and it appears that it's been fixed recently (see https://github.com/coverlet-coverage/coverlet/issues/1447) and will be included in their 6.0.1 release.

If these compiler-generated branches could also be ignored by Visual Studio's code coverage, that would be wonderful indeed.

Thanks very much!

mariam-abdulla commented 8 months ago

Hi @alexthornton1, Thank you for reporting this issue.

We now support the exclusion of those compiler-generated branches for method groups. This change will be available in VS 17.10 release.