XUnit, AutoFixture and Moq best practice

Question

I'm reading a lot of documentation and examples about how to properly unit test things combining the three components in the title. I came up with a test method for a method on my business logic, but it feels very clunky and dirty.

I'd like to get some feedback from people more experienced on this topic to see how I can improve it.

Here's the code, explanation follows:

[Fact]
public void ShouldGetItemWithSameId()
{
    var fixture = new Fixture().Customize(new AutoMoqCustomization());
    var facade = fixture.Freeze<Mock<IDataFacade>>();
    facade.Setup(c => c.Get(It.IsAny<int>())).Returns((int i) => new Item { Key = i });

    var sut = fixture.Create<BusinessLogic>();
    var expected = fixture.Create<int>();

    Assert.Equal(expected, sut.Get(expected).Key);
}

My BusinessLogic class takes an IDataFacade as constructor parameter, which is responsible in its Get(int) method to retrieve the item with the same Id, pretty basic stuff.

I freeze the IDataFacade mock and I set it up to construct an object matching the Id in It.IsAny<int>. I then create my SUT and test it. Works fine.

I'd like to understand if I can improve things considering the following:

• I have to test more complex methods, like a Query method that takes a class containing a lot of properties that will be used as filters on matching properties on the type being queried. In this case I wouldn't know how to properly do the "Setup" part of the mock, since I have to initialize all, or close to all, the properties of the returned type, and in this scenario it's not a single Item but a whole collection
• The setup part feels out of place, I'd like to be able to reuse it in more methods

I have some other tests using Theory with AutoMoqData but I was unable to achieve this test (and I think the more complex ones) using that approach, so I switched back to plain Fact with manually instantiated fixture.

Any help will be extremely appreciated.

Caracaraballo

Have you considered (Auto)NSubstitute - I held onto my 'what's wrong with Moq' defult attitude for too long. weareadaptive.com/blog/2014/09/30/why-nsubstitute – Donovan

Answer 1

Your test looks fine to me, although I would recommend one change. The following line could be tightened up to only return the expected value if the expected value is passed:

facade.Setup(c => c.Get(It.IsAny<int>())).Returns((int i) => new Item { Key = i });

All you'd need to do it move the expected variable and change the Is.IsAny like so:

var expected = fixture.Create<int>();
facade.Setup(c => c.Get(expected)).Returns((int i) => new Item { Key = i });

I have to test more complex methods, like a Query method that takes a class containing a lot of properties that will be used as filters on matching properties on the type being queried. In this case I wouldn't know how to properly do the "Setup" part of the mock, since I have to initialize all, or close to all, the properties of the returned type, and in this scenario it's not a single Item but a whole collection

I don't think you would need to initialise all of the values on the returned type. I'm guessing your DataFacade returns an object (or list of in this case)? All you'd need to do is make sure the objects returned match the references of those returned from the DataFacade, you don't need to worry about properties etc as you're not testing the construction of those objects, just that they're returned. If I've misunderstood and you're constructing the objects in the BusinessLogic then that's a different matter. Personally, I wouldn't have the business logic dependent on the data layer but that's a different discussion. :-)

The setup part feels out of place, I'd like to be able to reuse it in more methods

You can. Either extract it out to a separate method or, if it is applicable to every test in the class, put it in a setup method. I'm not familiar with XUnit but every other test framework I've used provides the ability to do common setup so I doubt XUnit will be any different.

And my final comment, treat your test code as you would treat your production code, if it looks a mess do things to make it better. Tests are great for describing the behavior of a system but if they're difficult to read (and maintain) you lose a lot of value.

Edit, turns out that isn't my final comment! If you're new to TDD, which I'm not sure you are, don't fall in to the trap of testing each and every class in your application, it's a common pattern that has become prevalent and it devalues TDD in my opinion. I've wrote a blog post on my feelings and Ian Cooper has given a superb presentation on the matter.

Ashur

A nice little wrapper to AutoFixture's create() is tdd-toolkit so var expected = fixture.Create<int>(); becomes var expected = Any.Integer(); – Thumbscrew

@Thumbscrew wrapper is a bit of a stretch; while I'm sure it has it's place, it doesn't address a pile of stuff the OP wants so why introduce the confusion of two sets of syntax/convention. – Donovan

@RubenBartelink generally agree, but in some cases it helped me achieving a little more clear code, maybe a better example is, e.g., Any.IntegerOtherThan(42) which wasn't needed her of course - just suggesting some tiny syntactic sugar which sometimes can be helpful, thanks – Thumbscrew

@Thumbscrew Generator<int>().First(x=>x!=42) from AF suits me just fine. A new lib starts off with -100 points in general. – Donovan

Answer 2

Overall, the original test looks good. It's not possible nor easy to extract the setup of Stubs and Mocks out of the test, in a generic fashion.

What you can do though, is minimize the Arrange phase of the test. Here's the original test re-written using AutoFixture.Xunit's own unit-testing DSL:

[Theory, TestConventions]
public void ShouldGetItemWithSameId(
    [Frozen]Mock<IDataFacade> facadeStub,
    BusinessLogic sut,
    int expected)
{
    facadeStub
        .Setup(c => c.Get(It.IsAny<int>()))
        .Returns((int i) => new Item { Key = i });

    var result = sut.Get(expected);
    var actual = result.Key;

    Assert.Equal(expected, actual);
}

The TestConventions attribute is defined as:

public class TestConventionsAttribute : AutoDataAttribute
{
    public TestConventionsAttribute()
        : base(new Fixture().Customize(new AutoMoqCustomization()))
    {
    }
}

HTH

---

Sample types used in the example:

public class Item
{
    public int Key { get; set; }
}

public interface IDataFacade
{
    Item Get(int p);
}

public class BusinessLogic
{
    private readonly IDataFacade facade;

    public BusinessLogic(IDataFacade facade)
    {
        this.facade = facade;
    }

    public Item Get(int p)
    {
        return this.facade.Get(p);
    }
}

Caliph

This I like. I'll try it out and comment later. Thanks :) – Caracaraballo

Answer 3

Some basics:

Your Test Class is instantiated (and its constructor called) before each single Test is run. e.g. if your Test class has three methods with [Fact] attribute, it gets instantiated three times

A TestFixture class is another class which is meant to be instanciated a single time for all the Tests in your test class.

To make this work, your test class must implement the IUseFixture interface, e.g. implement a member SetFixture()

You may use the same MyTestFixture class for several Test classes.

Inside the TestFixture you do all the Mock-Setups.

Here the general layout:

public class MyTestFixture
{    
    public Mock<MyManager> ManagerMock;

    public TestFixture() // runs once
    {
        ManagerMock.Setup(...);
    }
}

public MyTestClass : IUseFixture<MyTestFixture>
{
    private MyTestFixture fixture;

    public MyTestClass()
    {
         // ctor runs for each [Fact]
    }

    public void SetFixture(MyTestFixture fixture)
    {
        this.fixture = fixture;
    }

    [Fact]
    public void MyTest
    {
         // use Mock
         fixture.ManagerMock.DoSomething()
    }
}

Defensible

I already know all this stuff, but I think using the IUseFixture interface kind of defeats the purpose of having AutoFixture. I might be wrong though. – Caracaraballo

"Inside the TestFixture you do all the Mock-Setups" - I disagree, you should only do any setup which is applicable to all tests in that fixture. – Ashur

@Ashur yes, of course, in the TestFixture you do (only) all the things you need for all tests – Defensible

@Defensible Not also that xUnit v2 has a much cleaner way of managing fixtures (i.e. no IUseFixture, just ctor injection) – Donovan