Moq - Checking method call on concrete class

Question

Here is a very simplistic example of what I'm trying to do:

public class Bar
{
    public void SomeMethod(string param)
    {
        //whatever
    }
}

public interface IBarRepository
{
    List<Bar> GetBarsFromStore();
}

public class FooService
{
    private readonly IBarRepository _barRepository;

    public FooService(IBarRepository barRepository)
    {
        _barRepository = barRepository;
    }

    public List<Bar> GetBars()
    {
        var bars = _barRepository.GetBarsFromStore();
        foreach (var bar in bars)
        {
            bar.SomeMethod("someValue");
        }
        return bars;
    }
}

In my test, I'm mocking IBarRepository to return a concrete List defined in the unit test and passing that mocked repository instance to the FooService constructor.

I want to verify in the FooService method GetBars that SomeMethod was called for each of the Bars returned from the repository. I'm using Moq. Is there any way to do this without mocking the list of Bars returned (if even possible) and not having to put some hacky flag in Bar (yuck) ?.

I'm following an example from a DDD book, but I'm starting to think it smells because I'm challenged in testing the implementation....

Solution 1

Revised... this passes:

public class Bar
{
    public virtual void SomeMethod(string param)
    {
        //whatever
    }
}

public interface IBarRepository
{
    List<Bar> GetBarsFromStore();
}

public class FooService
{
    private readonly IBarRepository _barRepository;

    public FooService(IBarRepository barRepository)
    {
        _barRepository = barRepository;
    }

    public List<Bar> GetBars()
    {
        var bars = _barRepository.GetBarsFromStore();
        foreach (var bar in bars)
        {
            bar.SomeMethod("someValue");
        }
        return bars;
    }
}

[TestMethod]
public void Verify_All_Bars_Called()
{
    var myBarStub = new Mock<Bar>();
    var mySecondBarStub = new Mock<Bar>();

    var myBarList = new List<Bar>() { myBarStub.Object, mySecondBarStub.Object };
    var myStub = new Mock<IBarRepository>();
    myStub.Setup(repos => repos.GetBarsFromStore()).Returns(myBarList);
    var myService = new FooService(myStub.Object);
    myService.GetBars();

    myBarStub.Verify(bar => bar.SomeMethod(It.IsAny<string>()), Times.Once());
    mySecondBarStub.Verify(bar => bar.SomeMethod(It.IsAny<string>()), Times.Once());
}

Note the slight change to class Bar (SomeMethod() is virtual). A change, but not one involving a flag... :)

Now, in terms of broader design, there is some mutation going on with your bar (whatever "SomeMethod()" actually does). The best thing to do would probably be to verify that this mutation happened on each Bar returned from FooService.GetBars(). That is, setup your repository stub to return some bars, and then verify that whatever mutation is performed by SomeMethod() has taken place. After all, you control the Bars that will be returned, so you can setup their pre-SomeMethod() state, and then inspect their post-SomeMethod() state.

Solution 2

If I was writing these classes with unit testing in mind, I would likely either have the class Bar implement an interface IBar and use that interface in my service, or make SomeMethod virtual in Bar.

Ideally like this:

public interface IBar
{
    void SomeMethod(string param);
}

public class Bar : IBar
{
    public void SomeMethod(string param) {}
}

public interface IBarRepository
{
    List<IBar> GetBarsFromStore();
}

public class FooService
{
    private readonly IBarRepository _barRepository;

    public FooService(IBarRepository barRepository)
    {
        _barRepository = barRepository;
    }

    public List<IBar> GetBars()
    {
        var bars = _barRepository.GetBarsFromStore();
        foreach (var bar in bars)
        {
            bar.SomeMethod("someValue");
        }
        return bars;
    }
}

Then my unit test would look as follows:

[Test]
public void TestSomeMethodCalledForEachBar()
{
    // Setup
    var barMocks = new Mock<IBar>[] { new Mock<IBar>(), new Mock<IBar>() };
    var barObjects = barMocks.Select(m => m.Object);
    var repoList = new List<IBar>(barsObjects);
    var repositoryMock = new Mock<IBarRepository>();
    repositoryMock.Setup(r => r.GetBarsFromStore()).Returns(repoList);

    // Execute
    var service = new FooService(repositoryMock.Object);
    service.GetBars();

    // Assert
    foreach(var barMock in barMocks)
        barMock.Verify(b => b.SomeMethod("someValue"));
}

Comments

[drogon]

Thanks! I was just in the process of mocking the Bars. I'm glad you can do that with Moq.

[Erik Dietrich]

Glad if it helped. And yes, you can mock any virtual methods/properties of a concrete class, as long as the concrete class has a parameterless constructor (if it doesn't, Moq will throw exceptions)

[Piotr Perak]

Adding interface only for tests is not good in my opinion. This leads to nonsense IFoo, IFooImpl. Just make method virtual.

[Lukazoid]

I wouldn't say there was any problem with having interfaces named like your examples. I would say they are there for a reason (to provide a layer of abstraction between the contract and the implementation). By making the method virtual, if I wanted to change the implementation of DoSomething(), I would have to derive from Bar (which is now possible), probably gaining a lot of members I wouldn't want. By coding to an interface, I would only have to implement what was required, reducing the complexity and subsequently making it easier to work with.

[Piotr Perak]

IFoo, IFooImpl is a code smell for me. If it was IAmSomeAbstraction and class ConcreteExampleOfThatAbstraction then it's ok. Having Impl postfix in a name makes no sense. Ofcourse it is implementation.

[Lukazoid]

Check here on the msdn, observe the following: Do ensure that when defining a class/interface pair where the class is a standard implementation of the interface, the names differ only by the letter I prefix on the interface name. I can think of plenty of microsoft practices code which use what you consider a 'Code Smell'. But we'll have to agree to disagre :)

[Piotr Perak]

And where do you see Impl postfix suggested? So let's say I have some kind of notifications in my software. Standard notification is by email. You (and msdn) suggest creating IEmailNotifier and a class EmailNotifier? I can't think of worse. These are 'non abstract abstractions'. That should be INotification and default class would be EmailNotifier. For more discussion look here and rest posts of this series.

[Lukazoid]

I would create an interface INotifier and maybe a default implementation of Notifier, but there would be an implementation of INotifier called EmailNotifier. I am afraid I am lost as to what you mean by Impl postfix. For the askers question, I merely said I would provide some interface, IBar, IFoo, whatever it is, just appropriately named and Bar would implement it. It is merely an example, perhaps it would have been clearer if I had used IFoo, but it seems you are making an issue over dummy names.

[Lukazoid]

I didn't realise, but you meant actually creating interfaces with the postfix impl? Yeh, I wouldn't do that and am not sure why you would end up doing that. Having an interface named IFoo would be perfectly acceptable to me, if it provided Foo functionality.

[Piotr Perak]

What would that default Notifier do? EmailNotifier is ok. I was talking about IFoo, FooImpl pairs. They are code smell for me.

[Lukazoid]

let us continue this discussion in chat