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

Skip branches in generated MoveNext() for singleton iterators #813

Merged
merged 17 commits into from
May 10, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/coverlet.core/Symbols/CecilSymbolHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ public static List<BranchPoint> 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);
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved

foreach (Instruction instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch))
{
Expand All @@ -424,6 +425,12 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
continue;
}

if (skipSecondBranch)
{
skipSecondBranch = false;
continue;
}

if (isMoveNextInsideAsyncStateMachineProlog)
{
if (SkipMoveNextPrologueBranches(instruction) || SkipIsCompleteAwaiters(instruction))
Expand Down
88 changes: 88 additions & 0 deletions test/coverlet.core.tests/Coverage/CoverageTest.Yield.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
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)]
bert2 marked this conversation as resolved.
Show resolved Hide resolved
public void Yield_Single()
{
// We need to pass file name to remote process where it save instrumentation result
bert2 marked this conversation as resolved.
Show resolved Hide resolved
// 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<Yield>(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)]
bert2 marked this conversation as resolved.
Show resolved Hide resolved
public void Yield_Multiple()
{
string path = Path.GetTempFileName();
try
{
FunctionExecutor.Run(async (string[] pathSerialize) =>
{
CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<Yield>(instance =>
{
foreach (var _ in instance.Two());
return Task.CompletedTask;
}, persistPrepareResultToFile: pathSerialize[0]);
return 0;
}, new string[] { path });

CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);

result.Document("Instrumentation.Yield.cs")
.AssertLinesCovered((14, 1), (15, 1))
.ExpectedTotalNumberOfBranches(0);
}
finally
{
File.Delete(path);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions test/coverlet.core.tests/Samples/Instrumentation.Yield.cs
Original file line number Diff line number Diff line change
@@ -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<int> One()
{
yield return 1;
}

public System.Collections.Generic.IEnumerable<int> Two()
bert2 marked this conversation as resolved.
Show resolved Hide resolved
{
yield return 1;
yield return 2;
}
}
}
8 changes: 8 additions & 0 deletions test/coverlet.core.tests/Samples/Samples.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,14 @@ public IEnumerable<string> Fetch()
}
}

public class SingletonIterator
{
public IEnumerable<string> Fetch()
{
yield return "one";
}
}

public class AsyncAwaitStateMachine
{
async public Task AsyncAwait()
Expand Down
15 changes: 15 additions & 0 deletions test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down