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

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

Closed
alexthornton1 opened this issue Jan 28, 2024 · 1 comment
Closed
Assignees

Comments

@alexthornton1
Copy link

alexthornton1 commented Jan 28, 2024

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 dotnet/roslyn#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:

  • Both calls to Where contain a branch in their argument list, initializing <>O.<0>__IsLongString if it hasn't been already.
  • The first call to Where in the first test will initialize <>O.<0>__IsLongString.
  • All subsequent calls to Where will take the branch where <>O.<0>__IsLongString has already been initialized.

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 coverlet-coverage/coverlet#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
Copy link
Contributor

mariam-abdulla commented Mar 1, 2024

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.

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

No branches or pull requests

2 participants