Problem
Could you give any suggestion on the following unit tests? How to make it more readable, maintainable and trustworthy. If it is not a problem – post some code with the suggestions.
[TestClass]
public class PropertyBagCompositeTests
{
private readonly IFixture fixture;
private readonly Mock<IPropertyBag> secondPropertyBagMock;
private readonly Mock<IPropertyBag> firstPropertyBagMock;
private readonly string propertyName;
private readonly int propertyValue;
private readonly PropertyBagComposite cut;
public PropertyBagCompositeTests()
{
fixture = new Fixture();
firstPropertyBagMock = new Mock<IPropertyBag>();
secondPropertyBagMock = new Mock<IPropertyBag>();
propertyName = Any<string>();
propertyValue = Any<int>();
cut = new PropertyBagComposite(new [] { firstPropertyBagMock.Object, secondPropertyBagMock.Object});
}
[TestMethod]
public void GetProperty_DelegatedToBags()
{
cut.PropertyGet(propertyName);
firstPropertyBagMock.Verify(x => x.PropertyGet(propertyName), Times.Once());
secondPropertyBagMock.Verify(x => x.PropertyGet(propertyName), Times.Once());
}
[TestMethod]
public void GetProperty_NoBagHasProperty_ReturnsNull()
{
object result = cut.PropertyGet(propertyName);
result.Should().BeNull();
}
[TestMethod]
public void GetProperty_OneBagHasPropety_ResturnsItsResult()
{
firstPropertyBagMock.Setup(x => x.PropertyGet(propertyName)).Returns(propertyValue);
object result = cut.PropertyGet(propertyName);
result.Should().Be(propertyValue);
}
[TestMethod]
public void GetProperty_BothBagHasPropetyWithSameValue_ResturnsItsResult()
{
firstPropertyBagMock.Setup(x => x.PropertyGet(propertyName)).Returns(propertyValue);
secondPropertyBagMock.Setup(x => x.PropertyGet(propertyName)).Returns(propertyValue);
object result = cut.PropertyGet(propertyName);
result.Should().Be(propertyValue);
}
[TestMethod]
public void GetProperty_BothBagHasPropetyWithDifferentValue_ThrowsAmbiguousResultException()
{
firstPropertyBagMock.Setup(x => x.PropertyGet(propertyName)).Returns(propertyValue);
var otherPropertyValue = Any<double>();
secondPropertyBagMock.Setup(x => x.PropertyGet(propertyName)).Returns(otherPropertyValue);
Action action = () => cut.PropertyGet(propertyName);
action.ShouldThrow<AmbiguousResultException>();
}
[TestMethod]
public void SetProperty_DelegatedToBags()
{
cut.PropertySet(propertyName, propertyValue);
firstPropertyBagMock.Verify(x => x.PropertySet(propertyName, propertyValue), Times.Once());
secondPropertyBagMock.Verify(x => x.PropertySet(propertyName, propertyValue), Times.Once());
}
[TestMethod]
public void ListProperties_ReturnsAllProperties()
{
var duplicatedPropertyName = Any<string>();
var somePropertyName = Any<string>();
var anotherPropertyName = Any<string>();
firstPropertyBagMock.Setup(x => x.ListProperties()).Returns(new[] { duplicatedPropertyName, somePropertyName });
secondPropertyBagMock.Setup(x => x.ListProperties()).Returns(new[] { anotherPropertyName, duplicatedPropertyName });
string[] result = cut.ListProperties();
result.Should().Contain(new[] { duplicatedPropertyName, somePropertyName, anotherPropertyName });
}
[TestMethod]
public void ListProperties_ReturnsDistinct()
{
var duplicated = Any<string>();
firstPropertyBagMock.Setup(x => x.ListProperties()).Returns(new[] { duplicated, Any<string>() });
secondPropertyBagMock.Setup(x => x.ListProperties()).Returns(new[] { Any<string>(), duplicated });
string[] result = cut.ListProperties();
result.Count(x => x == duplicated).Should().Be(1);
}
private T Any<T>()
{
return fixture.Create<T>();
}
}
Code Under Test:
internal class PropertyBagComposite : IPropertyBag
{
private readonly IEnumerable<IPropertyBag> propertyBags;
public PropertyBagComposite(IEnumerable<IPropertyBag> propertyBags)
{
Require.NotNull(propertyBags, "propertyBags");
this.propertyBags = propertyBags;
}
public object PropertyGet(string propertyName)
{
object result = null;
foreach (object newItem in propertyBags.Select(x => x.PropertyGet(propertyName)))
{
if (result == null)
{
result = newItem;
}
else if (newItem != null && !result.Equals(newItem))
{
throw new AmbiguousResultException();
}
}
return result;
}
public void PropertySet(string propertyName, object propertyValue)
{
propertyBags
.ForEach(x => x.PropertySet(propertyName, propertyValue));
}
public string[] ListProperties()
{
string[] result = propertyBags
.SelectMany(x => x.ListProperties())
.Distinct()
.ToArray();
return result;
}
}
Solution
It’s pretty clean. In my oppinion, test parameters should be setup in each test, and not during setup. Setup is for setting up infrastructure bits. You should be able to take advantage of AutoFixture AutoMocking customization to make your tests more resiliant to change (let autofixture create your sut, so adding future dependencies won’t break your valid tests).
firstPropertyBagMock.Verify(x => x.PropertySet(propertyName, propertyValue), Times.Once());
The times.once() is kind of redundant, as this is the default.
Assertion like this:
result.Count(x => x == duplicated).Should().Be(1);
Can be rewritten a bit cleaner (Will have a better message on fail:
result.Where(x => x == duplicated).Should().HaveCount(c => c == 1)
Otherwise, you can achieve a bit more readability by creating a common base class to help with your tests, one I use (Uses rhino mocks, but gives you the idea):
public abstract class AutoFixtureBaseTest : AutoFixtureBaseTest where TSut : class {}
[TestFixture]
public abstract class AutoFixtureBaseTest<TSut, TContract>
where TSut : class, TContract
where TContract : class
{
protected IFixture Fixture { get; private set; }
[SetUp]
protected virtual void Initialize()
{
Fixture = new Fixture().Customize(new AutoRhinoMockCustomization());
Fixture.Behaviors.Add(new OmitOnRecursionBehavior());
Fixture.Customize<TSut>(x => x.FromFactory(new MethodInvoker(new GreedyConstructorQuery())));
}
public T Dep<T>()
{
return Fixture.Freeze<T>();
}
public T Fake<T>(Func<ICustomizationComposer<T>, IPostprocessComposer<T>> builder)
{
return builder(Fixture.Build<T>()).Create();
}
public T Fake<T>()
{
return Fixture.Create<T>();
}
public void Chain<TParent, TChild>(Function<TParent, TChild> expression) where TParent : class where TChild : class
{
Dep<TParent>().Stub(expression).Return(Dep<TChild>());
}
public IEnumerable<T> FakeMany<T>(Func<ICustomizationComposer<T>, IPostprocessComposer<T>> builder)
{
return builder(Fixture.Build<T>()).CreateMany();
}
public IEnumerable<T> FakeMany<T>()
{
return Fixture.CreateMany<T>();
}
public TContract Sut
{
get
{
return Fixture.Freeze<TSut>();
}
}
}
With this setup and configuration, most times you only need to setup the specific cases for each test.
EDIT: Quick example of how to convert your code to use automocking:
Take this section of code from your example:
public PropertyBagCompositeTests()
{
fixture = new Fixture();
firstPropertyBagMock = new Mock<IPropertyBag>();
secondPropertyBagMock = new Mock<IPropertyBag>();
propertyName = Any<string>();
propertyValue = Any<int>();
cut = new PropertyBagComposite(new [] { firstPropertyBagMock.Object, secondPropertyBagMock.Object});
}
You’re particular example, is somewhat complicated by the fact you are injecting an IEnumerable, for that you have to explicitly register:
public PropertyBagCompositeTests()
{
fixture = new Fixture().Customize(new AutoMoqCustomization());
propertyBags = fixture.CreateMany<Mock<IPropertyBag>>();
fixture.Register<IEnumerable<IPropertyBag>>(() => propertyBags.Select(x => x.Object));
propertyName = Any<string>();
propertyValue = Any<int>();
cut = fixture.Freeze<PropertyBagComposite>();
}
A more common example, and a good reference point:
http://blog.ploeh.dk/2010/08/19/AutoFixtureasanauto-mockingcontainer/
Using the base class similar to what I mentioned, you can do things like:
public class MyObjectSpecification : AutoFixtureBaseTest<MyClass, IMyClass> {
[TestMethod]
public void Should_do_something_with_dependency(){
var expected = Fake<Something>();
Dep<IDependency>(x => x.Setup(dep => dep.DoSomething()).Returns(expected).Verifiable());
var result = Sut.DoSomethingWithDependency();
result.Should().Be(expected);
Verify<IDependency>();
}
}