Question

I have a bunch of assemblies with near 100% test coverage but I often run into a situation like in the example below. I cannot test the default switch case, which is there to guard against future bugs where I add more items to the enum but forget to update the switch statement to support new items.

I would like to be able to either find a pattern where I can eliminate that "untestable" code, test it or mark that line of code (but not the entire method) to be excluded by the coverage analysis.

It may sound silly but I do not want to assume the default case will never happen, and I do not want to bundle the default case with one that already exist. I want the exception to be thrown when I create such bugs. Which will happen sooner or later.

I use DotCover to calculate coverage at the moment.

Note: This is just example code but I think it illustrates a fairly common pattern.

public class Tester
{
    private enum StuffToDo
    {
        Swim = 0, 
        Bike,
        Run
    }

    public void DoSomeRandomStuff()
    {
        var random = new Random();
        DoStuff((StuffToDo)random.Next(3));
    }

    private void DoStuff(StuffToDo stuff)
    {
        switch (stuff)
        {
            case StuffToDo.Swim:
                break;
            case StuffToDo.Bike:
                break;
            case StuffToDo.Run:
                break;
            default:
                // How do I test or exclude this line from coverage?
                throw new ArgumentOutOfRangeException("stuff");
        }
    }
}
Was it helpful?

Solution

Private methods are always candidates to be extracted into its own classes. Especially, if they hold complex logic, like yours do. I suggest you make a StuffDoer class with the DoStuff() as a public method and inject it into your Tester class. Then you have:

  • A DoStuff() method, reachable by tests
  • A DoSomeRandomStuff() method that can be tested with a mock instead of relying on the outcome of DoStuff(), thus making it a true unit test.
  • NO SRP-violation.
  • All in all, nice, crisp and SOLID code.

OTHER TIPS

You can use pre-compiler directive using DEBUG like this (or create your own):

#if DEBUG
    public void DoStuff(StuffToDo stuff)
#else
    private void DoStuff(StuffToDo stuff)
#endif
    {
        switch (stuff)
        {
            case StuffToDo.Swim:
                break;
            case StuffToDo.Bike:
                break;
            case StuffToDo.Run:
                break;
            default:
                // How do I test or exclude this line from coverage?
                throw new ArgumentOutOfRangeException("stuff");
        }
    }

Just do this:

private static int GetRandomStuffInt()
{
    var random = new Random();
    DoStuff((StuffToDo)random.Next(Enum.GetNames(typeof(StuffToDo)).Length));
}

This will ensure that the number it returns is part of the enum, and therefore will never even reach the default: switch. (Link)

Update: given the answers in here, it seems some important clarifications are needed.

  1. an enum doesn't prevent you from reaching the default clause, this is a valid way to reach it: DoStuff((StuffToDo)9999)
  2. the default clause isn't required in a switch statement. In that case, the code would just silently continue after the switch
  3. If the method was internal, you could use InternalsToVisible to unit test it directly
  4. You could refactor the code in such a way that the strategy handled in the switch can be reached by the unit tests, like @Morten mentions on his answer
  5. Random, dates and similar are a typical problem for unit tests, and what you do is to change the code in such a way that those can be replaced during unit tests. My answer is just one way of the many ways to do so. I show how you can inject it via a field member, but it could be via the constructor, and it can also be an interface if desired.
  6. If the enum wasn't private, my code sample below could instead expose a Func<StuffToDo>, which would make the tests a bit more readable.

You could still do this and get to 100%:

public class Tester
{
    private enum StuffToDo
    {
        Swim = 0, 
        Bike,
        Run
    }
    public Func<int> randomStuffProvider = GetRandomStuffInt;

    public void DoSomeRandomStuff()
    {
        DoStuff((StuffToDo)randomStuffProvider());
    }
    private static int GetRandomStuffInt()
    {
        //note: don't do this, you're supposed to reuse the random instance
        var random = new Random();
        return random.Next(3);
    }

    private void DoStuff(StuffToDo stuff)
    {
        switch (stuff)
        {
            case StuffToDo.Swim:
                break;
            case StuffToDo.Bike:
                break;
            case StuffToDo.Run:
                break;
            default:
                // How do I test or exclude this line from coverage?
                throw new ArgumentOutOfRangeException("stuff");
        }
    }
}

ps. yes it exposes the RandomStuffProvider so that you can use in an unit test. I didn't make it Func<StuffToDo> because that's private.

I agree with Nate here above. That is probably the easiest (StuffToDo.None). As an alternative you could also overload the method like this:

    private void DoStuff(StuffToDo stuff)
    {
        DoStuff(stuff.ToString());
    }

    private void DoStuff(string stuff)
        switch (stuff)
        {
            case "Swim":
                break;
            case "Bike":
                break;
            case "Run":
                break;
            default:
                throw new ArgumentOutOfRangeException("stuff");
        }
    }

Then you can actually test the situation that you cover in your default statement.

One way to test private methods in unit test case is using reflection but I feel that might be a over kill in most pf the situation where its possible to test with other ways. In your code, to test the switch case, what you can do is, if your class method which is public is taking StuffToDo as a parameter then you need to write multiple test cases with each passing a different set of value for StuffToDo. This way when your switch statement is executed you can verify the behaviour using your public method itself, but again in this case I am assuming you are getting a output from your public method.

Looking at your code another feeling I get is, your public method is not taking any input and not giving any out but doing modification that does not look right. It looks like it silently changing things which can confuse.

Try having more specific method which clearly states what it is taking as input and how modifying that.

Write tests that test the out of range values and expect an exception.

The "problem" you refer is intrinsic to switch...case and thus the best thing you can do to avoid it is relying on a different alternative. I personally don't like too much switch...case because of its inflexibility. Thus, this answer intends to provide an alternative to switch statements capable to deliver the kind of zero-uncertainty you are looking for.

In this kind of situations, I usually rely on a set of conditions taken from an array populated at the start of the function which will automatically filter any unaccounted input. Example:

private void DoStuffMyWay(StuffToDo stuff)
{
    StuffToDo[] accountedStuff = new StuffToDo[] { StuffToDo.Bike, StuffToDo.Run, StuffToDo.Swim };

    if (accountedStuff.Contains(stuff))
    {
        if (stuff == accountedStuff[0])
        {
            //do bike stuff
        }
        else if (stuff == accountedStuff[1])
        {
            //do run stuff
        }
        else if (stuff == accountedStuff[2])
        {
            //do swim stuff
        }
    }
}

This is certainly not too elegant, but I guess that I am not an elegant programmer.

As far as perhaps you prefer a different type of approach to the problem, here you have another alternative; it is less efficient but looks nicer:

private void DoStuffWithStyle(StuffToDo stuff)
{
    Dictionary<StuffToDo, Action> accountedStuff = new Dictionary<StuffToDo, Action>();
    accountedStuff.Add(StuffToDo.Bike, actionBike);
    accountedStuff.Add(StuffToDo.Run, actionRun);
    accountedStuff.Add(StuffToDo.Swim, actionSwim);

    if (accountedStuff.ContainsKey(stuff))
    {
        accountedStuff[stuff].Invoke();
    }
}

private void actionBike()
{
    //do bike stuff
}

private void actionRun()
{
    //do run stuff
}

private void actionSwim()
{
   //do swim stuff
}

With this second option, you might even create the dictionary as a global variable and populate it together with the enum.

It is unreachable code, because the switch case type is Enum, And you have coded all possible cases. There will never be the case of 'Default'.

You can add 'Unknown' or some extra enum value which should not have a corresponding case statement, then this warning will go away.

private enum StuffToDo
{
    Swim = 0, 
    Bike,
    Run,
    // just for code coverage
    // will never reach here
    Unknown
}

Code Coverage does not go into depth of evaluating every possibility, so it is not question of testing it. It matches code pattern based on their precoded expected behaviour

Code Coverage thinks it this way, you have added all possibility of existing enum, there is no way default will be called. This is how their rule is setup, you cannot change their rule.

My ReSharper also "complains" about unreachable code, which I don't like while programming. Call me anal, but I like to get rid of dead code from the moment I see it.

For your case specifically, I'd pick one of these cases as the default case, and put it to something like this:

private void DoStuff(StuffToDo stuff)
{
    switch (stuff)
    {
        case StuffToDo.Swim:
            break;
        case StuffToDo.Bike:
            break;
        case StuffToDo.Run:
        default:
            break;
    }
}

Since you strive to 100% test coverage, ideally, this should break when you add an enum entry that should be treated differently.

And if you really want to be sure, you can still uglify it:

private void DoStuff(StuffToDo stuff)
{
    switch (stuff)
    {
        case StuffToDo.Swim:
            break;
        case StuffToDo.Bike:
            break;
        case StuffToDo.Run:
        default:
            if (stuff != StuffToDo.Run) throw new ArgumentOutOfRangeException("stuff");
            break;
    }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top