From 6e1b51a576d5835949a71c246d5e63bee922397a Mon Sep 17 00:00:00 2001 From: bert2 Date: Mon, 13 Apr 2020 13:31:19 +0200 Subject: [PATCH 01/17] skip branches in generated `MoveNext()` for singleton iterators --- src/coverlet.core/Symbols/CecilSymbolHelper.cs | 7 +++++++ test/coverlet.core.tests/Samples/Samples.cs | 8 ++++++++ .../Symbols/CecilSymbolHelperTests.cs | 15 +++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 3cf93afd6..3d3480579 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -413,6 +413,7 @@ public static List GetBranchPoints(MethodDefinition methodDefinitio bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition); bool isMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsMoveNextInsideAsyncStateMachineProlog(methodDefinition); bool skipFirstBranch = IsMoveNextInsideEnumerator(methodDefinition); + bool skipSecondBranch = skipFirstBranch && !instructions.Any(i => i.OpCode.Code == Code.Switch); foreach (Instruction instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch)) { @@ -424,6 +425,12 @@ public static List GetBranchPoints(MethodDefinition methodDefinitio continue; } + if (skipSecondBranch) + { + skipSecondBranch = false; + continue; + } + if (isMoveNextInsideAsyncStateMachineProlog) { if (SkipMoveNextPrologueBranches(instruction) || SkipIsCompleteAwaiters(instruction)) diff --git a/test/coverlet.core.tests/Samples/Samples.cs b/test/coverlet.core.tests/Samples/Samples.cs index d2c713a45..12e27d7ba 100644 --- a/test/coverlet.core.tests/Samples/Samples.cs +++ b/test/coverlet.core.tests/Samples/Samples.cs @@ -172,6 +172,14 @@ public IEnumerable Fetch() } } + public class SingletonIterator + { + public IEnumerable Fetch() + { + yield return "one"; + } + } + public class AsyncAwaitStateMachine { async public Task AsyncAwait() diff --git a/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs b/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs index e62f012fb..e91f19c77 100644 --- a/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs +++ b/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs @@ -259,7 +259,22 @@ public void GetBranchPoints_IgnoresSwitchIn_GeneratedMoveNext() // assert Assert.Empty(points); + } + + [Fact] + public void GetBranchPoints_IgnoresBranchesIn_GeneratedMoveNextForSingletonIterator() + { + // arrange + var nestedName = typeof(SingletonIterator).GetNestedTypes(BindingFlags.NonPublic).First().Name; + var type = _module.Types.FirstOrDefault(x => x.FullName == typeof(SingletonIterator).FullName); + var nestedType = type.NestedTypes.FirstOrDefault(x => x.FullName.EndsWith(nestedName)); + var method = nestedType.Methods.First(x => x.FullName.EndsWith("::MoveNext()")); + + // act + var points = CecilSymbolHelper.GetBranchPoints(method); + // assert + Assert.Empty(points); } [Fact] From e0757c84890c1ee9981456db1416fe5c8d955267 Mon Sep 17 00:00:00 2001 From: bert2 Date: Mon, 13 Apr 2020 13:49:00 +0200 Subject: [PATCH 02/17] use `All()` instead of `Any()` --- src/coverlet.core/Symbols/CecilSymbolHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 3d3480579..12b79c802 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -413,7 +413,7 @@ public static List GetBranchPoints(MethodDefinition methodDefinitio bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition); bool isMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsMoveNextInsideAsyncStateMachineProlog(methodDefinition); bool skipFirstBranch = IsMoveNextInsideEnumerator(methodDefinition); - bool skipSecondBranch = skipFirstBranch && !instructions.Any(i => i.OpCode.Code == Code.Switch); + bool skipSecondBranch = skipFirstBranch && instructions.All(i => i.OpCode.Code != Code.Switch); foreach (Instruction instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch)) { From ed109774584fda4754adb26c1e45d3fd216c4470 Mon Sep 17 00:00:00 2001 From: bert2 Date: Wed, 15 Apr 2020 12:18:48 +0200 Subject: [PATCH 03/17] add integration tests --- .../Coverage/CoverageTest.Yield.cs | 86 +++++++++++++++++++ .../Samples/Instrumentation.Yield.cs | 18 ++++ 2 files changed, 104 insertions(+) create mode 100644 test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs create mode 100644 test/coverlet.core.tests/Samples/Instrumentation.Yield.cs diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs new file mode 100644 index 000000000..44e2ecf44 --- /dev/null +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -0,0 +1,86 @@ +using System.IO; +using System.Threading.Tasks; + +using Coverlet.Core.Samples.Tests; +using Coverlet.Tests.Xunit.Extensions; +using Tmds.Utils; +using Xunit; + +namespace Coverlet.Core.Tests +{ + public partial class CoverageTests : ExternalProcessExecutionTest + { + [ConditionalFact] + [SkipOnOS(OS.MacOS)] + public void Yield_Singleton() + { + // We need to pass file name to remote process where it save instrumentation result + // Similar to msbuild input/output + string path = Path.GetTempFileName(); + try + { + // Lambda will run in a custom process to avoid issue with statics and file locking + FunctionExecutor.Run(async (string[] pathSerialize) => + { + // Run load and call a delegate passing class as dynamic to simplify method call + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + // We call method to trigger coverage hits + _ = instance.One().Single(); + + // For now we have only async Run helper + return Task.CompletedTask; + }, persistPrepareResultToFile: pathSerialize[0]); + + // we return 0 if we return something different assert fail + return 0; + }, new string[] { path }); + + // We retrive and load CoveragePrepareResult and run coverage calculation + // Similar to msbuild coverage result task + CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + + // Generate html report to check + // TestInstrumentationHelper.GenerateHtmlReport(result); + + // Asserts on doc/lines/branches + result.Document("Instrumentation.Yield.cs") + // (line, hits) + .AssertLinesCovered((9, 1)); + } + finally + { + // Cleanup tmp file + File.Delete(path); + } + } + + [ConditionalFact] + [SkipOnOS(OS.MacOS)] + public void Yield_Multiple() + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.Run(async (string[] pathSerialize) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + _ = instance.Two().ToArray(); + return Task.CompletedTask; + }, persistPrepareResultToFile: pathSerialize[0]); + return 0; + }, new string[] { path }); + + CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + + result.Document("Instrumentation.SelectionStatements.cs") + .AssertLinesCovered((14, 1), (15, 0)); + } + finally + { + File.Delete(path); + } + } + } +} diff --git a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs new file mode 100644 index 000000000..bada78793 --- /dev/null +++ b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs @@ -0,0 +1,18 @@ +// Remember to use full name because adding new using directives change line numbers + +namespace Coverlet.Core.Samples.Tests +{ + public class Yield + { + public System.Collections.Generic.IEnumerable One() + { + yield return 1; + } + + public System.Collections.Generic.IEnumerable Two() + { + yield return 1; + yield return 2; + } + } +} From 6d96cc048c08f9b01e493397d6a16b68d0326846 Mon Sep 17 00:00:00 2001 From: bert2 Date: Wed, 15 Apr 2020 12:30:32 +0200 Subject: [PATCH 04/17] fix tests and assert number of branches --- .../Coverage/CoverageTest.Yield.cs | 14 ++++++++------ .../Coverage/InstrumenterHelper.Assertions.cs | 5 +++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 44e2ecf44..e0de12513 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -12,7 +12,7 @@ public partial class CoverageTests : ExternalProcessExecutionTest { [ConditionalFact] [SkipOnOS(OS.MacOS)] - public void Yield_Singleton() + public void Yield_Single() { // We need to pass file name to remote process where it save instrumentation result // Similar to msbuild input/output @@ -26,7 +26,7 @@ public void Yield_Singleton() CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { // We call method to trigger coverage hits - _ = instance.One().Single(); + foreach(var _ in instance.One()); // For now we have only async Run helper return Task.CompletedTask; @@ -46,7 +46,8 @@ public void Yield_Singleton() // Asserts on doc/lines/branches result.Document("Instrumentation.Yield.cs") // (line, hits) - .AssertLinesCovered((9, 1)); + .AssertLinesCovered((9, 1)) + .ExpectedTotalNumberOfBranches(0); } finally { @@ -66,7 +67,7 @@ public void Yield_Multiple() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - _ = instance.Two().ToArray(); + foreach (var _ in instance.Two()); return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); return 0; @@ -74,8 +75,9 @@ public void Yield_Multiple() CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); - result.Document("Instrumentation.SelectionStatements.cs") - .AssertLinesCovered((14, 1), (15, 0)); + result.Document("Instrumentation.Yield.cs") + .AssertLinesCovered((14, 1), (15, 1)) + .ExpectedTotalNumberOfBranches(0); } finally { diff --git a/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs b/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs index 24d9b6f4a..c7e34c9ea 100644 --- a/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs +++ b/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs @@ -84,6 +84,11 @@ public static Document AssertBranchesCovered(this Document document, params (int return AssertBranchesCovered(document, BuildConfiguration.Debug | BuildConfiguration.Release, lines); } + public static Document ExpectedTotalNumberOfBranches(this Document document, int totalExpectedBranch) + { + return ExpectedTotalNumberOfBranches(document, BuildConfiguration.Debug | BuildConfiguration.Release, totalExpectedBranch); + } + public static Document ExpectedTotalNumberOfBranches(this Document document, BuildConfiguration configuration, int totalExpectedBranch) { if (document is null) From 9b327b697f903a62ee30dd58614a235faec2d442 Mon Sep 17 00:00:00 2001 From: bert2 Date: Wed, 15 Apr 2020 13:05:02 +0200 Subject: [PATCH 05/17] remove guideline comments and dont skip tests on MacOS --- .../Coverage/CoverageTest.Yield.cs | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index e0de12513..2470bfbc9 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -2,7 +2,6 @@ using System.Threading.Tasks; using Coverlet.Core.Samples.Tests; -using Coverlet.Tests.Xunit.Extensions; using Tmds.Utils; using Xunit; @@ -10,54 +9,37 @@ namespace Coverlet.Core.Tests { public partial class CoverageTests : ExternalProcessExecutionTest { - [ConditionalFact] - [SkipOnOS(OS.MacOS)] + [Fact] public void Yield_Single() { - // We need to pass file name to remote process where it save instrumentation result - // Similar to msbuild input/output string path = Path.GetTempFileName(); try { - // Lambda will run in a custom process to avoid issue with statics and file locking FunctionExecutor.Run(async (string[] pathSerialize) => { - // Run load and call a delegate passing class as dynamic to simplify method call CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - // We call method to trigger coverage hits foreach(var _ in instance.One()); - // For now we have only async Run helper return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); - // we return 0 if we return something different assert fail return 0; }, new string[] { path }); - // We retrive and load CoveragePrepareResult and run coverage calculation - // Similar to msbuild coverage result task CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); - // Generate html report to check - // TestInstrumentationHelper.GenerateHtmlReport(result); - - // Asserts on doc/lines/branches result.Document("Instrumentation.Yield.cs") - // (line, hits) .AssertLinesCovered((9, 1)) .ExpectedTotalNumberOfBranches(0); } finally { - // Cleanup tmp file File.Delete(path); } } - [ConditionalFact] - [SkipOnOS(OS.MacOS)] + [Fact] public void Yield_Multiple() { string path = Path.GetTempFileName(); @@ -68,6 +50,7 @@ public void Yield_Multiple() CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { foreach (var _ in instance.Two()); + return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); return 0; From 669089d2b3f8483b6289f81515dc7fcbe42f4309 Mon Sep 17 00:00:00 2001 From: bert2 Date: Sat, 18 Apr 2020 11:46:22 +0200 Subject: [PATCH 06/17] add failing test case for singleton generator with switch --- .../Coverage/CoverageTest.Yield.cs | 33 +++++++++++++++++++ .../Coverage/InstrumenterHelper.Assertions.cs | 30 +++++++++++++++++ .../Samples/Instrumentation.Yield.cs | 22 +++++++++++++ 3 files changed, 85 insertions(+) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 2470bfbc9..8cf5cd6bc 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -30,6 +30,7 @@ public void Yield_Single() CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); result.Document("Instrumentation.Yield.cs") + .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__0::MoveNext()") .AssertLinesCovered((9, 1)) .ExpectedTotalNumberOfBranches(0); } @@ -59,6 +60,7 @@ public void Yield_Multiple() CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); result.Document("Instrumentation.Yield.cs") + .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__1::MoveNext()") .AssertLinesCovered((14, 1), (15, 1)) .ExpectedTotalNumberOfBranches(0); } @@ -67,5 +69,36 @@ public void Yield_Multiple() File.Delete(path); } } + + [Fact] + public void Yield_SingleWithSwitch() + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.Run(async (string[] pathSerialize) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + foreach (var _ in instance.OneWithSwitch(2)) ; + + return Task.CompletedTask; + }, persistPrepareResultToFile: pathSerialize[0]); + + return 0; + }, new string[] { path }); + + CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + + result.Document("Instrumentation.Yield.cs") + .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__2::MoveNext()") + .AssertLinesCovered((30, 1), (31, 1), (37, 1)) + .ExpectedTotalNumberOfBranches(1); + } + finally + { + File.Delete(path); + } + } } } diff --git a/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs b/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs index c7e34c9ea..4330ba23e 100644 --- a/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs +++ b/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs @@ -79,6 +79,36 @@ public static Document Document(this CoverageResult coverageResult, string docNa throw new XunitException($"Document not found '{docName}'"); } + public static Document Method(this Document document, string methodName) + { + var methodDoc = new Document { Path = document.Path, Index = document.Index }; + + if (!document.Lines.Any() && !document.Branches.Any()) + { + return methodDoc; + } + + if (document.Lines.Values.All(l => l.Method != methodName) && document.Branches.Values.All(l => l.Method != methodName)) + { + var methods = document.Lines.Values.Select(l => $"'{l.Method}'") + .Concat(document.Branches.Values.Select(b => $"'{b.Method}'")) + .Distinct(); + throw new XunitException($"Method '{methodName}' not found. Methods in document: {string.Join(", ", methods)}"); + } + + foreach (var line in document.Lines.Where(l => l.Value.Method == methodName)) + { + methodDoc.Lines[line.Key] = line.Value; + } + + foreach (var branch in document.Branches.Where(b => b.Value.Method == methodName)) + { + methodDoc.Branches[branch.Key] = branch.Value; + } + + return methodDoc; + } + public static Document AssertBranchesCovered(this Document document, params (int line, int ordinal, int hits)[] lines) { return AssertBranchesCovered(document, BuildConfiguration.Debug | BuildConfiguration.Release, lines); diff --git a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs index bada78793..9eb27153d 100644 --- a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs +++ b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs @@ -14,5 +14,27 @@ public System.Collections.Generic.IEnumerable Two() yield return 1; yield return 2; } + + public System.Collections.Generic.IEnumerable OneWithSwitch(int n) + { + int result; + switch (n) + { + case 0: + result = 10; + break; + case 1: + result = 11; + break; + case 2: + result = 12; + break; + default: + result = -1; + break; + } + + yield return result; + } } } From 23607b78d569b72ca234ea2bcf5ee52a59d0aedc Mon Sep 17 00:00:00 2001 From: bert2 Date: Sat, 18 Apr 2020 12:45:54 +0200 Subject: [PATCH 07/17] use `Random` in switch to avoid it from being optimized away (just guessing here) --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 7 +++---- test/coverlet.core.tests/Samples/Instrumentation.Yield.cs | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 8cf5cd6bc..16fe2b0fb 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -19,7 +19,7 @@ public void Yield_Single() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach(var _ in instance.One()); + foreach(var _ in instance.One()) ; return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -50,7 +50,7 @@ public void Yield_Multiple() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach (var _ in instance.Two()); + foreach (var _ in instance.Two()) ; return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -80,7 +80,7 @@ public void Yield_SingleWithSwitch() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach (var _ in instance.OneWithSwitch(2)) ; + foreach (var _ in instance.OneWithSwitch()) ; return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -92,7 +92,6 @@ public void Yield_SingleWithSwitch() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__2::MoveNext()") - .AssertLinesCovered((30, 1), (31, 1), (37, 1)) .ExpectedTotalNumberOfBranches(1); } finally diff --git a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs index 9eb27153d..127c3f32c 100644 --- a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs +++ b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs @@ -15,10 +15,10 @@ public System.Collections.Generic.IEnumerable Two() yield return 2; } - public System.Collections.Generic.IEnumerable OneWithSwitch(int n) + public System.Collections.Generic.IEnumerable OneWithSwitch() { int result; - switch (n) + switch (new System.Random().Next()) { case 0: result = 10; From 03620d90a62e10ddb269ef24e84616978af6e9c3 Mon Sep 17 00:00:00 2001 From: bert2 Date: Sat, 18 Apr 2020 12:58:12 +0200 Subject: [PATCH 08/17] Revert "use `Random` in switch to avoid it from being optimized away (just guessing here)" This reverts commit 23607b78d569b72ca234ea2bcf5ee52a59d0aedc. --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 7 ++++--- test/coverlet.core.tests/Samples/Instrumentation.Yield.cs | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 16fe2b0fb..8cf5cd6bc 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -19,7 +19,7 @@ public void Yield_Single() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach(var _ in instance.One()) ; + foreach(var _ in instance.One()); return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -50,7 +50,7 @@ public void Yield_Multiple() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach (var _ in instance.Two()) ; + foreach (var _ in instance.Two()); return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -80,7 +80,7 @@ public void Yield_SingleWithSwitch() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach (var _ in instance.OneWithSwitch()) ; + foreach (var _ in instance.OneWithSwitch(2)) ; return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -92,6 +92,7 @@ public void Yield_SingleWithSwitch() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__2::MoveNext()") + .AssertLinesCovered((30, 1), (31, 1), (37, 1)) .ExpectedTotalNumberOfBranches(1); } finally diff --git a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs index 127c3f32c..9eb27153d 100644 --- a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs +++ b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs @@ -15,10 +15,10 @@ public System.Collections.Generic.IEnumerable Two() yield return 2; } - public System.Collections.Generic.IEnumerable OneWithSwitch() + public System.Collections.Generic.IEnumerable OneWithSwitch(int n) { int result; - switch (new System.Random().Next()) + switch (n) { case 0: result = 10; From 98605ca999fd16d55c0995924ac9b24b8ec2caab Mon Sep 17 00:00:00 2001 From: bert2 Date: Sat, 18 Apr 2020 13:20:42 +0200 Subject: [PATCH 09/17] also skip second branch if iterator contains a switch --- src/coverlet.core/Symbols/CecilSymbolHelper.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 12b79c802..266e8e79f 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -413,7 +413,7 @@ public static List GetBranchPoints(MethodDefinition methodDefinitio bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition); bool isMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsMoveNextInsideAsyncStateMachineProlog(methodDefinition); bool skipFirstBranch = IsMoveNextInsideEnumerator(methodDefinition); - bool skipSecondBranch = skipFirstBranch && instructions.All(i => i.OpCode.Code != Code.Switch); + bool skipSecondBranch = false; foreach (Instruction instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch)) { @@ -422,6 +422,7 @@ public static List GetBranchPoints(MethodDefinition methodDefinitio if (skipFirstBranch) { skipFirstBranch = false; + skipSecondBranch = instruction.OpCode.Code != Code.Switch; continue; } From c6e8f211e49ca217d5090dfd7d7c99e71e723c49 Mon Sep 17 00:00:00 2001 From: bert2 Date: Wed, 22 Apr 2020 17:35:09 +0200 Subject: [PATCH 10/17] add comment explaining `skipFirstBranch` and `skipSecondBranch` --- src/coverlet.core/Symbols/CecilSymbolHelper.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 266e8e79f..074d8153b 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -412,6 +412,11 @@ public static List GetBranchPoints(MethodDefinition methodDefinitio bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition); bool isMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsMoveNextInsideAsyncStateMachineProlog(methodDefinition); + + // State machine for enumerator uses `brfalse.s`/`beq` or `switch` opcode depending on how many `yield` we have in the method body. + // For more than one `yield` a `switch` is emitted so we should only skip the first branch. In case of a single `yield` we need to + // skip the first two branches to avoid reporting a phantom branch. The first branch (`brfalse.s`) jumps to the `yield`ed value, + // the second one (`beq`) exits the enumeration. bool skipFirstBranch = IsMoveNextInsideEnumerator(methodDefinition); bool skipSecondBranch = false; From 4104e8021fccdfb5f0c3d48562e8719826dfc636 Mon Sep 17 00:00:00 2001 From: bert2 Date: Wed, 22 Apr 2020 17:42:36 +0200 Subject: [PATCH 11/17] add more integration test cases --- .../Coverage/CoverageTest.Yield.cs | 62 ++++++++++++++++++- .../Samples/Instrumentation.Yield.cs | 15 +++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 8cf5cd6bc..f7a404364 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -41,7 +41,7 @@ public void Yield_Single() } [Fact] - public void Yield_Multiple() + public void Yield_Two() { string path = Path.GetTempFileName(); try @@ -100,5 +100,65 @@ public void Yield_SingleWithSwitch() File.Delete(path); } } + + [Fact] + public void Yield_Three() + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.Run(async (string[] pathSerialize) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + foreach (var _ in instance.Three()) ; + + return Task.CompletedTask; + }, persistPrepareResultToFile: pathSerialize[0]); + return 0; + }, new string[] { path }); + + CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + + result.Document("Instrumentation.Yield.cs") + .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__3::MoveNext()") + .AssertLinesCovered((42, 1), (43, 1), (44, 1)) + .ExpectedTotalNumberOfBranches(0); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public void Yield_Enumerable() + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.Run(async (string[] pathSerialize) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + foreach (var _ in instance.Enumerable(new[] { "one", "two", "three", "four" })) ; + + return Task.CompletedTask; + }, persistPrepareResultToFile: pathSerialize[0]); + return 0; + }, new string[] { path }); + + CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + + result.Document("Instrumentation.Yield.cs") + .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__4::MoveNext()") + .AssertLinesCovered((51, 4)) + .ExpectedTotalNumberOfBranches(1); + } + finally + { + File.Delete(path); + } + } } } diff --git a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs index 9eb27153d..0f7f0b851 100644 --- a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs +++ b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs @@ -36,5 +36,20 @@ public System.Collections.Generic.IEnumerable OneWithSwitch(int n) yield return result; } + + public System.Collections.Generic.IEnumerable Three() + { + yield return 1; + yield return 2; + yield return 3; + } + + public System.Collections.Generic.IEnumerable Enumerable(System.Collections.Generic.IList ls) + { + foreach (var item in ls) + { + yield return item; + } + } } } From d3c069f60d6172f6ccef2be706a761593aef4dc1 Mon Sep 17 00:00:00 2001 From: bert2 Date: Sun, 3 May 2020 12:13:16 +0200 Subject: [PATCH 12/17] check coverage of `switch(n)` --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index f7a404364..8d1bb9be8 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -92,7 +92,7 @@ public void Yield_SingleWithSwitch() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__2::MoveNext()") - .AssertLinesCovered((30, 1), (31, 1), (37, 1)) + .AssertLinesCovered((21, 1), (30, 1), (31, 1), (37, 1)) .ExpectedTotalNumberOfBranches(1); } finally From 65ea9f38aaf836b959ab3510c534ae4b0eeecef9 Mon Sep 17 00:00:00 2001 From: bert2 Date: Sun, 3 May 2020 12:25:12 +0200 Subject: [PATCH 13/17] check coverage of other lines in `Yield.Enumerable()` --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 8d1bb9be8..494a8d23e 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -152,7 +152,7 @@ public void Yield_Enumerable() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__4::MoveNext()") - .AssertLinesCovered((51, 4)) + .AssertLinesCovered((48, 1), (49, 11), (50, 4), (51, 4), (52, 4), (53, 1)) .ExpectedTotalNumberOfBranches(1); } finally From 93d5202f0f246d18de680b09e49f80956b3498bb Mon Sep 17 00:00:00 2001 From: bert2 Date: Sun, 3 May 2020 12:38:30 +0200 Subject: [PATCH 14/17] fix assertions for release builds --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 494a8d23e..4e0710913 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -92,7 +92,8 @@ public void Yield_SingleWithSwitch() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__2::MoveNext()") - .AssertLinesCovered((21, 1), (30, 1), (31, 1), (37, 1)) + .AssertLinesCovered(BuildConfiguration.Debug, (21, 1), (30, 1), (31, 1), (37, 1)) + .AssertLinesCovered(BuildConfiguration.Release, (30, 1), (31, 1), (37, 1)) .ExpectedTotalNumberOfBranches(1); } finally @@ -152,7 +153,8 @@ public void Yield_Enumerable() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__4::MoveNext()") - .AssertLinesCovered((48, 1), (49, 11), (50, 4), (51, 4), (52, 4), (53, 1)) + .AssertLinesCovered(BuildConfiguration.Debug, (48, 1), (49, 11), (50, 4), (51, 4), (52, 4), (53, 1)) + .AssertLinesCovered(BuildConfiguration.Release, (49, 10), (51, 4), (53, 1)) .ExpectedTotalNumberOfBranches(1); } finally From 5b1471f730484651fe01af494f94348f8e170b99 Mon Sep 17 00:00:00 2001 From: bert2 Date: Mon, 4 May 2020 12:01:12 +0200 Subject: [PATCH 15/17] remove coverage assertions for release builds --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 4e0710913..39ce0c32d 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -93,7 +93,6 @@ public void Yield_SingleWithSwitch() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__2::MoveNext()") .AssertLinesCovered(BuildConfiguration.Debug, (21, 1), (30, 1), (31, 1), (37, 1)) - .AssertLinesCovered(BuildConfiguration.Release, (30, 1), (31, 1), (37, 1)) .ExpectedTotalNumberOfBranches(1); } finally @@ -154,7 +153,6 @@ public void Yield_Enumerable() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__4::MoveNext()") .AssertLinesCovered(BuildConfiguration.Debug, (48, 1), (49, 11), (50, 4), (51, 4), (52, 4), (53, 1)) - .AssertLinesCovered(BuildConfiguration.Release, (49, 10), (51, 4), (53, 1)) .ExpectedTotalNumberOfBranches(1); } finally From e576020dc8e238d4a79e5e094245f3e25a83c7f7 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sun, 10 May 2020 17:46:30 +0200 Subject: [PATCH 16/17] refactor test a bit --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 7 +++---- test/coverlet.core.tests/Samples/Instrumentation.Yield.cs | 6 +++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index 39ce0c32d..eaf1de383 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -19,7 +19,7 @@ public void Yield_Single() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach(var _ in instance.One()); + foreach (var _ in instance.One()) ; return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -50,7 +50,7 @@ public void Yield_Two() { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { - foreach (var _ in instance.Two()); + foreach (var _ in instance.Two()) ; return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); @@ -142,7 +142,6 @@ public void Yield_Enumerable() CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { foreach (var _ in instance.Enumerable(new[] { "one", "two", "three", "four" })) ; - return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); return 0; @@ -152,7 +151,7 @@ public void Yield_Enumerable() result.Document("Instrumentation.Yield.cs") .Method("System.Boolean Coverlet.Core.Samples.Tests.Yield/d__4::MoveNext()") - .AssertLinesCovered(BuildConfiguration.Debug, (48, 1), (49, 11), (50, 4), (51, 4), (52, 4), (53, 1)) + .AssertLinesCovered(BuildConfiguration.Debug, (48, 1), (49, 1), (50, 4), (51, 5), (52, 1), (54, 4), (55, 4), (56, 4), (57, 1)) .ExpectedTotalNumberOfBranches(1); } finally diff --git a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs index 0f7f0b851..12c007c11 100644 --- a/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs +++ b/test/coverlet.core.tests/Samples/Instrumentation.Yield.cs @@ -46,7 +46,11 @@ public System.Collections.Generic.IEnumerable Three() public System.Collections.Generic.IEnumerable Enumerable(System.Collections.Generic.IList ls) { - foreach (var item in ls) + foreach ( + var item + in + ls + ) { yield return item; } From cf5d269f7392d66b5d1cd3ffc0c2b25e17080d4c Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sun, 10 May 2020 17:47:32 +0200 Subject: [PATCH 17/17] nit: style --- test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs index eaf1de383..f2aca5360 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs @@ -142,6 +142,7 @@ public void Yield_Enumerable() CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { foreach (var _ in instance.Enumerable(new[] { "one", "two", "three", "four" })) ; + return Task.CompletedTask; }, persistPrepareResultToFile: pathSerialize[0]); return 0;