Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/coverlet.core/Instrumentation/InstrumenterResult.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.Serialization;

namespace Coverlet.Core.Instrumentation
Expand All @@ -17,6 +18,7 @@ public class Line
public int Hits;
}

[DebuggerDisplay("Line = {Number} Offset = {Offset} EndOffset = {EndOffset} Path = {Path} Ordinal = {Ordinal} Hits = {Hits}")]
[DataContract]
public class Branch : Line
{
Expand All @@ -30,6 +32,7 @@ public class Branch : Line
public uint Ordinal;
}

[DebuggerDisplay("line = {Line} Ordinal = {Ordinal}")]
// Implements IEquatable because is used by dictionary key https://docs.microsoft.com/en-us/dotnet/api/system.iequatable-1?view=netcore-2.2#remarks
[DataContract]
public class BranchKey : IEquatable<BranchKey>
Expand Down
36 changes: 33 additions & 3 deletions test/coverlet.core.tests/CoverageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void TestCoverageWithTestAssembly()
}

[Fact]
public void Condition_If()
public void SelectionStatements_If()
{
// We need to pass file name to remote process where it save instrumentation result
// Similar to msbuild input/output
Expand All @@ -73,7 +73,7 @@ public void Condition_If()
RemoteExecutor.Invoke(async pathSerialize =>
{
// Run load and call a delegate passing class as dynamic to simplify method call
CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<Conditions>(instance =>
CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<SelectionStatements>(instance =>
{
// We call method to trigger coverage hits
instance.If(true);
Expand All @@ -91,7 +91,7 @@ public void Condition_If()
CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);

// Asserts on doc/lines/branches
result.Document("Instrumentation.cs")
result.Document("Instrumentation.SelectionStatements.cs")
// (line, hits)
.AssertLinesCovered((11, 1), (15, 0))
// (line,ordinal,hits)
Expand All @@ -106,5 +106,35 @@ public void Condition_If()
File.Delete(path);
}
}

[Fact]
public void SelectionStatements_Switch()
{
string path = Path.GetTempFileName();
try
{
RemoteExecutor.Invoke(async pathSerialize =>
{
CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<SelectionStatements>(instance =>
{
instance.Switch(1);
return Task.CompletedTask;
}, pathSerialize);
return 0;
}, path).Dispose();

CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);

result.Document("Instrumentation.SelectionStatements.cs")
.AssertLinesCovered(BuildConfiguration.Release, (24, 1), (26, 0), (28, 0))
.AssertBranchesCovered(BuildConfiguration.Release, (24, 1, 1))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it make the test very dependent on the optimisations in the compiler, so they could start breaking on a different SDK version. Since these tests are about matching specific lines it seems better to just use Debug runs and not include any expected outcome for Release runs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it make the test very dependent on the optimisations in the compiler,

Yep it's true...talked also with Andrew #396 (comment) and we had same issue in past #389
Yes we could test only for debug config...but why if some bug happen on release and not on debug config for some reason related to "instrumentation" bug?If in future we put some conditional code related to some new roslyn behaviour?Maybe I'm too concern about something that won't never happen 😅 , but could be a good "doorbell", and maybe we could add the possibility to run test only for specific SDK like a custom xunit attribute [RunOnSdks("2.2.301","..")], to be very rigorous.
Another thing that I'm not sure is if also "debug" config will be stable...I mean also that config could depend on different SDK version, less likely but I'm not 100% sure.
BTW I don't have a super strong opinion and we could test only debug config...I tried to be complete as possible...
Thank's for review Peter!!!
@tonerdo thoughts?

.AssertLinesCovered(BuildConfiguration.Debug, (20, 1), (21, 1), (24, 1), (30, 1))
.AssertBranchesCovered(BuildConfiguration.Debug, (21, 0, 0), (21, 1, 1), (21, 2, 0), (21, 3, 0));
}
finally
{
File.Delete(path);
}
}
}
}
48 changes: 48 additions & 0 deletions test/coverlet.core.tests/InstrumenterHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@

namespace Coverlet.Core.Tests
{
[Flags]
public enum BuildConfiguration
{
Debug = 1,
Release = 2
}

public static class TestInstrumentationAssert
{
public static Document Document(this CoverageResult coverageResult, string docName)
Expand All @@ -42,12 +49,24 @@ public static Document Document(this CoverageResult coverageResult, string docNa
}

public static Document AssertBranchesCovered(this Document document, params (int line, int ordinal, int hits)[] lines)
{
return AssertBranchesCovered(document, BuildConfiguration.Debug | BuildConfiguration.Release, lines);
}

public static Document AssertBranchesCovered(this Document document, BuildConfiguration configuration, params (int line, int ordinal, int hits)[] lines)
{
if (document is null)
{
throw new ArgumentNullException(nameof(document));
}

BuildConfiguration buildConfiguration = GetAssemblyBuildConfiguration();

if ((buildConfiguration & configuration) != buildConfiguration)
{
return document;
}

List<string> branchesToCover = new List<string>(lines.Select(b => $"[line {b.line} ordinal {b.ordinal}]"));
foreach (KeyValuePair<BranchKey, Branch> branch in document.Branches)
{
Expand Down Expand Up @@ -77,12 +96,24 @@ public static Document AssertBranchesCovered(this Document document, params (int
}

public static Document AssertLinesCovered(this Document document, params (int line, int hits)[] lines)
{
return AssertLinesCovered(document, BuildConfiguration.Debug | BuildConfiguration.Release, lines);
}

public static Document AssertLinesCovered(this Document document, BuildConfiguration configuration, params (int line, int hits)[] lines)
{
if (document is null)
{
throw new ArgumentNullException(nameof(document));
}

BuildConfiguration buildConfiguration = GetAssemblyBuildConfiguration();

if ((buildConfiguration & configuration) != buildConfiguration)
{
return document;
}

List<int> linesToCover = new List<int>(lines.Select(l => l.line));
foreach (KeyValuePair<int, Line> line in document.Lines)
{
Expand All @@ -106,6 +137,23 @@ public static Document AssertLinesCovered(this Document document, params (int li

return document;
}

private static BuildConfiguration GetAssemblyBuildConfiguration()
{
var configurationAttribute = Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyConfigurationAttribute>();
if (configurationAttribute.Configuration.Equals("Debug", StringComparison.InvariantCultureIgnoreCase))
{
return Tests.BuildConfiguration.Debug;
}
else if (configurationAttribute.Configuration.Equals("Release", StringComparison.InvariantCultureIgnoreCase))
{
return Tests.BuildConfiguration.Release;
}
else
{
throw new NotSupportedException($"Build configuration '{configurationAttribute.Configuration}' not supported");
}
}
}

public static class TestInstrumentationHelper
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Remember to use full name because adding new using directives change line numbers

namespace Coverlet.Core.Samples.Tests
{
public class SelectionStatements
{
public int If(bool condition)
{
if (condition)
{
return 1;
}
else
{
return 0;
}
}

public int Switch(int caseSwitch)
{
switch (caseSwitch)
{
case 1:
return 1;
case 2:
return 2;
default:
return 0;
}
}
}
}
19 changes: 0 additions & 19 deletions test/coverlet.core.tests/Samples/Instrumentation.cs

This file was deleted.