How to refactor to avoid using a Shim?

Question

I'm pretty new to Unit Testing and am exploring the Microsoft Fakes framework - primarily because it's free and it allows me to mock SharePoint objects easily with the Emulators package. I've seen various mentions on SO and elsewhere that Shims are evil and I more or less understand why. What I don't get is how to avoid them in one specific case - in other words, "how should I refactor my code to avoid having to use shims?"

For the code in question, I have a JobProcessor object that has properties and methods, some of which are private as they should only be called from the public Execute method. I want to test that when Execute is called and there is a Job available that its Process method is called as I need to do some extra logging.

Here's the relevant code:

//in system under test - JobProcessor.cs

private IJob CurrentJob { get; set; }

public void Execute()
{
   GetJobToProcess();  //stores Job in CurrentJob property if found
   if (ShouldProcessJob){
       CurrentJob.ProcessJob();
   }
}

I need to do some extra things if ProcessJob is called from a test, so I set up a Stub in my Test Method to do those extra things:

StubIJob fakeJob = new StubIJob(){
    ProcessJob = () =>{
        //do my extra things here
    }

};

I'm testing the ProcessJob method itself elsewhere so I don't care that it doesn't do anything but my extra stuff here. As I understand things, I now need to set up a Shim to have the private method GetJobsToProcess from JobProcessor (my system under test) return my fake job so that my stubbed method is called:

processor = new JobProcessor();
ShimJobProcessor.AllInstances.GetJobToProcess = (@this) =>{
   var privateProcessor = new PrivateObject(processor);
   privateProcessor.SetProperty("CurrentJob", fakeJob);  //force my test Job to be processed so the Stub is used
};

In this case, how should I avoid using the Shim? Does it matter?

Thanks.

Answer 1

This is a case where rather than using a shim or stub, I'd just make the method return a boolean to notify whether or not the inner call has happened.

The problem with using fakes there is that you're assuming that some method of some object is called, which the test should not know. Tests should be dumb, and only see the outside of the code. Tests, like any other code, should not care how a value was reached, just that it is correct.

However, your code has another issue as well. You're getting some unknown object and using it within the same scope. You should remove the call to GetJobToProccess from Execute.

It's the principle of Dependency Injection: a method should not spin up and hide it's dependencies; if it depends on an object, that object should be possible to change freely or be passed in. The exact implementation of the job should not matter to the execute method, and that, along with the naming, implies that you should not be getting that object and executing it in the same call.