Confusing switch case coverage

[cbroxton]

Hi,

I've got an issue with some confusing coverage statistics being generated for switch cases. My tests have covered all branches including the default branch however AltCover is still considering there to be a missing branch until I test with null as the value. Here's a screenshot of what the coverage looks like...

I've also created a simple reproduction of the issue in the following repository...

AltCoverSwitchCaseTest

Coverage was generated with the following command...

dotnet test /p:AltCover=true /p:AltCoverSingle=true /p:AltCoverCobertura="Cobertura.xml" /p:AltCoverAssemblyExcludeFilter="xunit|.Test"

Is this a bug? Or if not could you please help clear up why there is a missing branch? As far as I can tell in the case that the value is null it should use the default branch which is already covered?

[SteveGilham]

Welcome to the world of compiler implementation details, which makes branch instrumentation and interpretation such fun.

Except in cases where the branches are a set of adjacent integers, so the IL switch instruction can be used, C# switch statements (and F# match expressions) compile to a (possibly complex and nested) series of if/else branches.

In this case, the source line

      switch(value)

compiles to

	// sequence point: (line 13, col 13) to (line 13, col 27) 
	IL_0001: ldarg.0
	IL_0002: stloc.2

	IL_0003: ldloc.2
	IL_0004: stloc.1

	IL_0005: ldloc.1
	IL_0006: brfalse.s IL_0049  // jump to default if input is null

	IL_0008: ldloc.1
	IL_0009: ldstr "a"
	IL_000e: call bool [netstandard]System.String::op_Equality(string, string)
	IL_0013: brtrue.s IL_0031 // case "a"

	IL_0015: ldloc.1
	IL_0016: ldstr "b"
	IL_001b: call bool [netstandard]System.String::op_Equality(string, string)
	IL_0020: brtrue.s IL_0039 // case "b"

	IL_0022: ldloc.1
	IL_0023: ldstr "c"
	IL_0028: call bool [netstandard]System.String::op_Equality(string, string)
	IL_002d: brtrue.s IL_0041 // case "c"

	IL_002f: br.s IL_0049 // for the rest unconditional jump to default

which has four two-way choices that offer a chance to exit the sequence point. I suspect that the 8th branch, the one not taken, is for a null input.

This is comparatively simple IL; switch statements with non-integer labels can compile into more deeply nested expressions with selection on a hash before more expensive matches on precise values -- see here for one example.

[SteveGilham]

The branch processing logic here is ultimately derived from OpenCover's algorithms; and the implicit design philosophy of that tool (as stated in several branch-coverage related issues) is to expose all the compiler generated tangle, and leave it to the end-user to decide which branches are actually sane, and which are compiler generated (and which may not even be coverable e.g. for F# inlines or match expressions on an algebraic type).

Currently, AltCover amends that behaviour, hiding simple conditional branches that terminate within the same sequence-point (compiler generated housekeeping often does this), and combining branches from IL switch instructions that go to the same location; but otherwise treats each branching instruction individually. Having already taken a "trust the compiler" approach for these cases (so tests can be focussed more on the program language level logic rather than the compiler's implementation details), it would be worthwhile to take a look at doing some more processing of branches that considers a sequence point as a whole to handle the language level switch or match constructs.

[cbroxton]

Thanks for the detailed response, have to admit though a lot of it went over my head. I've ended up changing the code to be an if, else if instead. The issue was that in our real code the switch case is in a for loop which before the switch case will continue the loop if the value is null so the branch is impossible to reach. Not sure if this is a bug or just an unavoidable thing but I've updated my repo to show the issue.

[SteveGilham]

In release v6.2.714, the --visibleBranches option clears the compiler generated clutter from reports to produce what you thought you'd get from a switch statement.

from command line

dotnet test /p:AltCover=true /p:AltCoverSingle=true /p:AltCoverCobertura="Cobertura.xml" /p:AltCoverLocalSource=true /p:AltCoverAssemblyExcludeFilter="?AltCover" /p:AltCoverVisibleBranches=true

[cbroxton]

That's great thanks, appreciate how quickly you're resolving these issues. I've added the flag to the test step of our pipeline. Feel free to close this.