3

I've got a setup like this with a concrete class that is instantiated inside the method I want to test. I want to mock this concrete class an not have it execute the code inside. Hence, no exception should be thrown:

public class Executor
{
    public bool ExecuteAction(ActionRequest request)
    {
        switch (request.ActionType)
        {
            case ActionType.Foo:
                var a = new Foo();
                return a.Execute(request);
            case ActionType.Bar:
                var b = new Bar();
                return b.Execute(request);
        }

        return true;
    }
}

public class Foo
{
    public virtual bool Execute(ActionRequest request)
    {
        throw new NotImplementedException();
    }
}

public class Bar
{
    public virtual bool Execute(ActionRequest request)
    {
        throw new NotImplementedException();
    }
}

My NUnit test looks like this:

[Test]
public void GivenARequestToFooShouldExecuteFoo()
{
    var action = new Mock<Foo>();
    action.Setup(x => x.Execute(It.IsAny<ActionRequest>())).Returns(true);

    var sut = new Mock<Executor>();
    sut.Object.ExecuteAction(new ActionRequest
    {
        ActionType = ActionType.Foo
    });
}

[Test]
public void GivenARequestToBarShouldExecuteBar()
{
    var action = new Mock<Bar>();
    action.Setup(x => x.Execute(It.IsAny<ActionRequest>())).Returns(true);

    var sut = new Mock<Executor>();
    sut.Object.ExecuteAction(new ActionRequest
    {
        ActionType = ActionType.Bar
    });
}

I fiddled around with CallBase, but it didn't get me anywhere. Is there anyway I can solve this easily without dependency injection of these classes and adding interfaces? Is this possible just using Moq?

The only thing I can think to do currently is move the Execute methods into the Executor class and rename them to ExecuteFoo() and ExecuteBar(), but I have a lot of code to move so they'd have to be partial classes (sub classes?).

Nkosi
  • 235,767
  • 35
  • 427
  • 472
Rebecca
  • 13,914
  • 10
  • 95
  • 136
  • 1
    You could do it if you made `Executor.ExecuteAction` method `virtual`. But given the fact that testing is one of the ways to detect design flaws, I'd recommend reworking what you have. Not to mention that the tests you have shown are extremely fragile due to tight coupling to the implementation (are you actually testing `switch` statement here?) – Zdeněk Jelínek Mar 23 '17 at 17:56

1 Answers1

3

The problem is not with the mocking of the method but with the creation of the concrete class. The creation of Foo and Bar need to be inverted out of the Executor. It is responsible for executing the action, not creating it. with that this interface was created to handle the creation.

public interface IActionCollection : IDictionary<ActionType, Func<IExecute>> {

}

think of this as a collection of factories or a collection of creation strategies.

A common interface was created for the actions.

public interface IExecute {
    bool Execute(ActionRequest request);
}

public class Foo : IExecute {
    public virtual bool Execute(ActionRequest request) {
        throw new NotImplementedException();
    }
}

public class Bar : IExecute {
    public virtual bool Execute(ActionRequest request) {
        throw new NotImplementedException();
    }
}

And the Executor was refactored to use dependency inversion.

public class Executor {
    readonly IActionCollection factories;

    public Executor(IActionCollection factories) {
        this.factories = factories;
    }

    public bool ExecuteAction(ActionRequest request) {
        if (factories.ContainsKey(request.ActionType)) {
            var action = factories[request.ActionType]();
            return action.Execute(request);
        }
        return false;
    }
}

With that refactor done the Executor can be tested with fake actions.

public void GivenARequestToFooShouldExecuteFoo() {
    //Arrange
    var expected = true;
    var key = ActionType.Foo;

    var action = new Mock<Foo>();
    action.Setup(x => x.Execute(It.IsAny<ActionRequest>())).Returns(expected);

    var actions = new Mock<IActionCollection>();
    actions.Setup(_ => _[key]).Returns(() => { return () => action.Object; });
    actions.Setup(_ => _.ContainsKey(key)).Returns(true);

    var sut = new Executor(actions.Object);
    var request = new ActionRequest {
        ActionType = ActionType.Foo
    };

    //Act
    var actual = sut.ExecuteAction(request);

    //Assert
    Assert.AreEqual(expected, actual);
}

A production implementation of the factory collection can look like this

public class ActionCollection : Dictionary<ActionType, Func<IExecute>>, IActionCollection {
    public ActionCollection()
        : base() {
    }
}

and configured accordingly with your concrete types.

var factories = ActionCollection();
factories[ActionType.Foo] = () => new Foo();
factories[ActionType.Bar] = () => new Bar();
Nkosi
  • 235,767
  • 35
  • 427
  • 472
  • 1
    A classic example of having to turn a class inside out in order to support unit testing, and flushing encapsulation down the toilet in the process. Now you can unit test Executor to your heart's content, but if at run time it gets initialised with the wrong set of IActionCollection factories (for whatever reason) it still won't work. The refactoring to support unit testing is actually creating the potential for new bugs that could not posiibly occur in the original properly encapsulated implementation. – Neutrino May 24 '18 at 13:19