HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Unit tests for a simple composite

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simpletestsforcompositeunit

Problem

I do a lot of unit testing(tdd) and would like some comments about my style. Here is a simple example from a composite.

Is there anything I could improve?

```
public class ChangedClaimsServiceComposite : IChangedClaimsService
{
private readonly IChangedClaimsService[] changedClaimsServices;

public ChangedClaimsServiceComposite(params IChangedClaimsService[] changedClaimsServices)
{
if (changedClaimsServices == null) throw new ArgumentNullException("changedClaimsServices");
this.changedClaimsServices = changedClaimsServices;
}

public List GetChangedClaims()
{
return changedClaimsServices.SelectMany(x => x.GetChangedClaims()).ToList();
}

public void SetClaimUpdated(IChangedClaim changedClaim)
{
foreach (var changedClaimsService in changedClaimsServices)
{
changedClaimsService.SetClaimUpdated(changedClaim);
}
}
}

[TestFixture]
public class ChangedClaimsServiceCompositeTests
{
[Test]
public void GetChangedClaimsSouldCombineResultsFromDependedServices()
{
//Arrange
var changedClaimService1 = new Mock();
var changedClaimService2 = new Mock();

var changedClaim1 = CreateAnyChangedClaim();
var changedClaim2 = CreateAnyChangedClaim();
var changedClaim3 = CreateAnyChangedClaim();
var changedClaim4 = CreateAnyChangedClaim();

changedClaimService1.Setup(x => x.GetChangedClaims())
.Returns(new List() {changedClaim1, changedClaim2});
changedClaimService2.Setup(x => x.GetChangedClaims())
.Returns(new List() { changedClaim3, changedClaim4 });

var sut = new ChangedClaimsServiceComposite(
changedClaimService1.Object,
changedClaimService2.Object);

//Act
var result = sut.GetChangedClaims();

//Assert
var expectedResult = new List()
{
changedClaim1,
changedClaim2,
cha

Solution

If you rename these:

var changedClaim1 = CreateAnyChangedClaim();
    var changedClaim2 = CreateAnyChangedClaim();
    var changedClaim3 = CreateAnyChangedClaim();
    var changedClaim4 = CreateAnyChangedClaim();


Like this:

var service1changedClaim1 = CreateAnyChangedClaim();
    var service1changedClaim2 = CreateAnyChangedClaim();
    var service2changedClaim1 = CreateAnyChangedClaim();
    var service2changedClaim1 = CreateAnyChangedClaim();


Then it will be more obvious that these will be used in two different services. These names will carry more meaning than just an arbitrary list of IChangedClaim objects.

What is "sut" ? A more meaningful name would be better.

In this assert:

Assert.That(result, Is.EqualTo(expectedResult), "result");


The comment message is not very useful.
Either make it something more meaningful, or delete it.

Code Snippets

var changedClaim1 = CreateAnyChangedClaim();
    var changedClaim2 = CreateAnyChangedClaim();
    var changedClaim3 = CreateAnyChangedClaim();
    var changedClaim4 = CreateAnyChangedClaim();
var service1changedClaim1 = CreateAnyChangedClaim();
    var service1changedClaim2 = CreateAnyChangedClaim();
    var service2changedClaim1 = CreateAnyChangedClaim();
    var service2changedClaim1 = CreateAnyChangedClaim();
Assert.That(result, Is.EqualTo(expectedResult), "result");

Context

StackExchange Code Review Q#98615, answer score: 6

Revisions (0)

No revisions yet.