Using shims to turn my integration tests into unit tests

Question

I want to understand how to properly unit test a function such as this. The function doesn't return anything, but it does call other functions that themselves have already been unit tested. My understanding is that in order to properly unit test this method I will have to shim the dependant methods - otherwise this is just an integration test.

public void ResetColours(Colours colour)
{
    var colourInfo = colour.GetInfo();
    for (int i = 0; i < 9; ++i)
        this.SetColourInfo(i, colourInfo);
}

I'm having a hard time deciding on the right way to shim the methods. At the moment I am thinking of doing this - shim the GetInfo method so that it returns a hard coded value, then have the SetColourInfo shim remember the information that was passed in to make sure it matches.

[TestMethod]
public void ResetColours()
{
    Info[] colourInformation = new Info[9];

    using (ShimsContext.Create())
    {
        ShimColourExtensions.GetInfoColours = (Colours colour) => new Info("test");
        ShimFace.AllInstances.SetColourInfoIntInfo =
            (index, colourInfo) => colourInformation[index] = true;

        this.TestObject.ResetColours(Colours.Brown); //TestObject was initialized in my init method
        //ASSERT that hard coded GetInfo value was set into each array index
    }
}

Is this a valid unit test? Could I be doing anything better in situations like this? Should I even be writing a unit test for a method like this?

Answer 1

There was no need to delete the post on Stack Overflow, it belonged there just fine; perhaps even better than here.

You don't have to shim anything. Integration tests are tests that use an external dependency (system time, external API, filesystem, etc). In your case all your "dependencies" are simply method calls in your own project.

Here in your case this can simply be seen as a unit test: you're testing a single public method (~ unit) which encapsulates smaller unit tests. This is a perfectly valid unit.

Integration testing refers to a bigger set of components that and testing how these major interfaces work together.

Now, on to the bigger question: how do you test this [void method]? As is tradition: a link to an answer of mine.

void methods change the state of your program (adjusting values of variables) or they interact with an external dependency (printing to the console, logging to a file). So in order to assert against any expectations you'll have to either interpret the results or intercept them along the way.

Interpreting results

In the first case (adjusting of a variable) the answer is obvious: look at the variables that got changed and see if they now have the values you want.

In the latter case you get a "problem": you're now reading the result of an external dependency so you're entering the realm of integration testing. This is why we have the alternative solution..

Intercepting results

This is done with shims, mocks, stubs, seams, you name it. You're already aware how this works: a DI framework intercepts the call and you can provide your own values around the actual call, or you inject the unit under test with a specific implementation for testing purposes.

Answer 2

Reviewing the tested method:

public void ResetColours(Colours colour)
{
    var colourInfo = colour.GetInfo();
    for (int i = 0; i < 9; ++i)
        this.SetColourInfo(i, colourInfo);
}

The colour parameter is singular, but its type is plural. Either colour stands for many colors and it's misleadingly named, or Colours is a bad name for a type that represents a single color.

---

for (int i = 0; i < 9; ++i)

The hard-coded 9 is a problem. It's a magic number! It's not clear from just this snippet why you need 9 iterations, but the 9 should be evaluated, not hard-coded. Making it be the Length of an array, or the Count of an ICollection, or whatever, will instantly remove a future bug. Avoid magic numbers!

---

this.SetColourInfo(i, colourInfo);

This line is the body of your loop - i only exists within that scope, and because you've omitted the curly braces {}, that scope is implicit. If something ever needs to be added to the body of the loop, you'll have to make it explicit. Why not make it explicit from the start? It's a good habit to always put in those braces:

Also, as far as the compiler is concerned, the this qualifier is redundant. I've seen a redundant this qualifier to qualify fields, but qualifying methods seems like overkill to me. I'd drop it:

var iterations = someArray.Length; // assumption
for (var i = 0; i < iterations; ++i)
{
    SetColourInfo(i, colourInfo);
}

---

Your test method is named the same as the method it's testing. That's not very good naming for a test method: you want to be able to look at your list of tests, and tell at a glance what's working. It's not rare that one method requires multiple unit tests (because every test tests only one thing, right?); naming the tests after what's being tested will avoid weird naming.

I like using the word Should in my test methods. Hence, I'd call your test something like ShouldSetAllArrayIndicesAsSpecified. Don't be afraid to be descriptive.