-
Notifications
You must be signed in to change notification settings - Fork 18
Description
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:
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
Wherecontain a branch in their argument list, initializing<>O.<0>__IsLongStringif it hasn't been already. - The first call to
Wherein the first test will initialize<>O.<0>__IsLongString. - All subsequent calls to
Wherewill take the branch where<>O.<0>__IsLongStringhas 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!
