Multi-line lambda false negatives in 3.0.1

[ulrichb]

Okay, it got way better with 3.0.1 (after 3.0.0's #1037 has been fixed), but there are still false negatives (compared to coverlet.collector 1.3.0).

Example 1:

Example 2:

Originally posted by @ulrichb in #1037 (comment)

... I'm trying to create a repro sample.

[ulrichb]

Okay, it's simply a multi-line lamba:

    public class SampleTest
    {
        [Fact]
        public void T1()
        {
            Do(x => Console.WriteLine(x.GetType().Name));

            Do(x => Console.WriteLine(x
                .GetType()
                .Name));

            Do2(x => x.GetType().Name.Length);

            Do2(x => x.GetType()
                .Name
                .Length);
        }

        private static void Do(Action<object> action)
        {
            action(new object());
        }

        private static object Do2(Func<object, object> func)
        {
            return func(new object());
        }
    }

(Using latest ReportGenerator 4.8.4.)

[MarcoRossignoli]

Thanks

[MarcoRossignoli]

ok I was able to repro

[ulrichb]

Nice. And btw. thanks for this great, great tool!

[MarcoRossignoli]

We changed a bit the accounting alg because it was a mess, now we need to adjust a bit. Np we'll fix it.

[MarcoRossignoli]

@ulrichb can you try the fix?

coverlet.console.3.0.2-preview.2.g6741924090.zip

[ulrichb]

Hmm, do you also have a prerelease .collector nuget package?

[MarcoRossignoli]

It's a zip with all three packages inside.
Didn't renamed sorry.

[ulrichb]

Yipee, just tested your build and my repro and my real code is 100% covered again. Thank you very much!

[MarcoRossignoli]

@martincostello @JSkimming @Malivil @Dave-EMIS someone of you can try the package on your project and let us know if it's all ok? #1056 (comment)

[Malivil]

Looks good on my solution. The only unexpected change I saw was marking a line as covered that ended a multi-line ternary statement with multi-line lambdas inside. I think it's completely understandable to show that as covered, though, so I don't think it's a problem. For example:

return DoThing ? 
    HandleThingTrue(() => // Entire lambda covered
    {
        DoOtherThing();
    })
    : HandleThingFalse(() => // Entire lambda uncovered
    {
        DoThirdThing();
    }); // Covered

In terms of the coverage numbers:

MSBuild 2.9.0:
MSBuild 3.0.1:
MSBuild 3.0.2:

The only number that went down was branch coverage and it looks, to me, like that branch is not really covered so that's a positive too

Overall, looks good to me =)

[MarcoRossignoli]

I think it's completely understandable to show that as covered, though, so I don't think it's a problem.

It's ok...lambda accounting it's a bit complex, coverlet uses sequence points(strong choice sometimes uncomfortable) and sometimes are "nested" i.e.

sequence A hitted
line 1 { hit
line 2 ... hit
line 3 ... hit
line 4 } hit

sequence B (lambda closure object) not hitted
line 2 ... no hit
line 3 ... no hit

When we have an hit for A and not for B we account for covered line 1 and 4 from A but we use sequence B for 2 and 3.

Thx @Malivil

[JSkimming]

@MarcoRossignoli Results comparing different version of coverlet and OpenCover.

Summary

All looks good.

Results

	coverlet.msbuild 2.9.0	coverlet.msbuild 3.0.1	coverlet.msbuild 3.0.2-preview.2.g6741924090	OpenCover 4.7.922
Castle.Core.AsyncInterceptor (Debug)	100% (328 of 328)	100% (328 of 328)	100% (328 of 328)	100% (333 of 333)
Castle.Core.AsyncInterceptor (Release)	100% (220 of 220)	100% (220 of 220)	100% (220 of 220)	100% (227 of 227)
abioc (Debug)	99.8% (2215 of 2219)	99.8% (2216 of 2219)	99.8% (2216 of 2219)	99.8% (2219 of 2222)
abioc (Release)	99.8% (1743 of 1745)	99.9% (1744 of 1745)	99.9% (1744 of 1745)	no longer works 🤷
Tesla.NET (Debug)	100% (683 of 683)	100% (683 of 683)	100% (683 of 683)	100% (683 of 683)
Tesla.NET (Release)	100% (589 of 589)	100% (589 of 589)	100% (589 of 589)	100% (589 of 589)

It looks like I incorrectly compared debug and release version when I stated coverlet was now giving comparable results as OpenCover, though the good news is, it already was 😃

[JSkimming]

I'm, not a big user of lambdas, I tend to use local functions or private methods for anything more complex than x => x.Property, which may be why my libraries are unaffected.

[MarcoRossignoli]

Thx for the test @JSkimming

[MarcoRossignoli]

After midnight will be on nightly https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md
I'd like need to check another issue before release, if I won't find a solution I'll release in a week.

[MarcoRossignoli]

Just released new version https://github.com/coverlet-coverage/coverlet/releases/tag/v5.6.2