Switch statement weird coverage

[bjornbouetsmith]

Hi,

I have the following simple class:

internal static class StateParser
{
    public static State Parse(string stateText)
    {
        if (string.IsNullOrWhiteSpace(stateText))
        {
            return State.Unknown;
        }

        switch (stateText)
        {
            case "ONLINE":
                return State.Online;
            case "DEGRADED":
                return State.Degraded;
            case "OFFLINE":
                return State.Offline;
            case "FAULTED":
                return State.Faulted;
            case "REMOVED":
                return State.Removed;
            case "UNAVAIL":
                return State.Unavailable;
            case "AVAIL":
                return State.Available;
            default:
                return State.Unknown;
        }
    }
}

No matter how I test it - I get coverage of 77.8% - with the message stating that the line of the switch(stateText) is only partially covered by tests (12 of 20 conditions).

At first I thought it was because I did not have the if (string.IsNullOrWhiteSpace(stateText)) { return State.Unknown; }

Even though the default case works just fine for null and empty strings.

But even though I have tests like this:

[TestMethod]
[DataRow("ONLINE",State.Online)]
[DataRow("OFFLINE", State.Offline)]
[DataRow("DEGRADED", State.Degraded)]
[DataRow("FAULTED", State.Faulted)]
[DataRow("REMOVED", State.Removed)]
[DataRow("AVAIL", State.Available)]
[DataRow("UNAVAIL", State.Unavailable)]
[DataRow("HOTSTUFF", State.Unknown)]
[DataRow(null, State.Unknown)]
[DataRow("", State.Unknown)]
public void ParseStateTest(string stateText, State expected)
{
    var state = StateParser.Parse(stateText);
    Assert.AreEqual(expected, state);
}

Which hits every single line of the StateParser - I still only get 77.8% coverage.

Do you have any explanation of why this is happening - and is it a bug in altcover - or something weird going on?

To have the full picture, in case you want to reproduce it - this is the State enum:

public enum State
{
    Unknown = 0,
    Online = 1,
    Degraded = 2,
    Faulted = 3,
    Offline = 4,
    Removed = 5,
    Unavailable = 6,
    Available = 7
}

Thanks in advance for any hints you can give.

[SteveGilham]

A switch on strings is syntactic sugar for a complicated mass of if/else branching in the compiled code -- it roughly goes

Compute hash of input string
if hash = hash of first string case 
then if string = first string case
       then do first case clause
       else go to default case
else if hash = hash of 2nd string case 
then if string = 2nd string case
       then do 2nd case clause
       else go to default case
else ...

with the missing coverage corresponding to the cases where the hash matches but the string is not the wanted one.

See issue #72 for a previous case of the same effect.

The code as given could of course be replaced by

public static State Parse(string stateText)
{
   if (Enum.TryParse(typeof(State), stateText, out var result))
    return result;
   else
    return State.Unknown;
}

which has simpler coverage characteristics, and defers the hard work to library code.

[bjornbouetsmith]

A switch on strings is syntactic sugar for a complicated mass of if/else branching in the compiled code -- it roughly goes The code as given could of course be replaced by

public static State Parse(string stateText)
{
   if (Enum.TryParse(typeof(State), stateText, out var result))
    return result;
   else
    return State.Unknown;
}

which has simpler coverage characteristics, and defers the hard work to library code.

But the enum which I control would have to match exactly the names of the strings that I do not control - if it were that simple I would already have just used enum.TryParse - but I do not want to name my enum as the underlying string values - which I get from calling a binary on linux.

Of course - if I get tired of looking at that 77.8 which is nonsense in my opinion, since code coverage is "code" coverage - not translated il coverage - then I could always cave and rename my enum so it matches the string values, and I would have a much simpler "parse" method as you so correctly state.

In the other issue you wrote that /p:AltCoverVisibleBranches=true would fix the issue - it also seems to fix the issue for me.

Why is it not just the default - since I would assume most people would expect the above code to generate 100% codecoverage.

[bjornbouetsmith]

But my issue was resolved - I just think its worth considering making that option default - or at least make it very prominent on the first page of this repository :-)

Also - thank you very much for taking the time to respond 👍

[SteveGilham]

It's no problem; it people are finding the project useful, then I'm happy to help them.

Branch coverage from AltCover started out heavily based on OpenCover, and that took a warts-and-all approach -- roughly speaking "there's logic here, I don't know who wrote it, but it's not being tested". That has the virtue of simplicity of implementation and is of course compiler agnostic across languages and versions. Because a coverage tool has to process code from a broad range of sources, what I've implemented for the --visibleBranches option cannot be a true decompilation; it's a heuristic that is potentially vulnerable to compiler changes which may create both false positive and negative cases.

Flipping the sense of that switch is something to add to a list of changes that will need a major version bump; for the time being I have added an entry to the FAQ, in the hope that it might be seen.