Unit tests for testing error codes

Both unit tests pass and they both test the same thing. The production code uses the right error codes.

System Under Test (SUT)

public interface IRepository {
    string GetParameter(int id);
}

public class Repository {
    public string GetParameter(int id) {
        return "foo";
    }
}

public class ErrorInfo{
    public string ErrorMessage { get; set; }
}

public interface IErrorProvider {
    ErrorInfo BuildErrorMessage(string errorCodes);
}

public class ErrorProvider {
    public ErrorInfo BuildErrorMessage(string errorCodes)
    {
        return new ErrorInfo { ErrorMessage = errorCodes };
    }
}

public class DomainClass {

    private readonly IRepository _repository;
    private readonly IErrorProvider _errorProvider;

    public DomainClass(IRepository repository, IErrorProvider errorProvider) {
        _repository = repository;
        _errorProvider = errorProvider;
    }

    public List GetErrorList(int id) {

        var errorList = new List();
        string paramName = _repository.GetParameter(id);

        if (string.IsNullOrEmpty(paramName))
        {
            string errorCodes = string.Format("{0}, {1}", 200, 201);
            var error = _errorProvider.BuildErrorMessage(errorCodes);
            errorList.Add(error);
        }

        return errorList;
    }
}

Unit test 1

```
[TestMethod]
public void GetErrorList_WhenParameterIsEmpty_ReturnsExpectedErrorCodes()
{
//Arrange
var stubRepo = new Mock();
stubRepo.Setup(x => x.GetParameter(It.IsAny())).Returns(string.Empty);
const string expectedErrorCodes = "200, 201";
var stubErrorRepo = new Mock();
stubErrorRepo.Setup(e => e.BuildErrorMessage(expectedErrorCodes)).Returns(new ErrorInfo() { ErrorMessage = expectedErrorCodes + " some error message" });
const int parameterId = 5;
var sut = new DomainClass(stubRepo.Object, stubErrorRepo.Object);

//Act
var result = sut.GetErrorList(parameterId);

//Assert
Assert.IsTrue(result.Any(info

This question demonstrates why Test-Driven Development (TDD) is a valuable discipline. If TDD had been strictly followed, the answer would have almost presented itself.

To demonstrate that, I copied the tests only (not the 'production code') to a code base, and attempted to figure out what the implementation should be in order to pass each test. Keep in mind that an important principle in TDD is that you write only enough code to make the tests pass.

Initial state

In order to compile, the initial implementation of DomainClass looks like this:

public class DomainClass
{
    public DomainClass(IRepository repository, IErrorProvider errorProvider)
    {
    }

    public IEnumerable GetErrorList(int parameterId)
    {
        yield break;
    }
}

Obviously this doesn't pass the tests, but at least the code compiles so that you can run the tests.

Since the goal of this answer is to explore which test is most correct, I'll now isolate each test. I'll first delete test 2 so that only test 1 is left and see where that goes, and then the other way around. After that, I'll compare the two implementations to figure out which one is best.

Test 1

If test 1 is the only test of DomainClass, then this implementation passes:

public class DomainClass
{
    private readonly IErrorProvider errorProvider;

    public DomainClass(IRepository repository, IErrorProvider errorProvider)
    {
        this.errorProvider = errorProvider;
    }

    public IEnumerable GetErrorList(int id)
    {
        yield return this.errorProvider.BuildErrorMessage("200, 201");
    }
}

Compared to the 'production code' in the OP, this is obviously not the full implementation of GetErrorList, but it does pass the test(s).

Test 2

If test 2 is the only test of DomainClass, then this implementation passes:

public class DomainClass
{
    private IErrorProvider errorProvider;

    public DomainClass(IRepository repository, IErrorProvider errorProvider)
    {
        this.errorProvider = errorProvider;
    }

    public IEnumerable GetErrorList(int parameterId)
    {
        this.errorProvider.BuildErrorMessage("200, 201");
        return new List();
    }
}

This implementation passes the test(s), but doesn't look correct to me. The BuildErrorMessage method is called, but the return value is ignored. That's probably not what was intended.

Stubs and Mocks

From GOOS comes this rule of thumb (paraphrased):


Use Stubs for Queries and Mocks for Commands.

Since the BuildErrorMessage method is a Query this rules states that a Stub should be used for testing. That indicates that test 1 is more correct, which the two alternative implementations also seem to indicate.

Driving the Happy Path

Obviously, neither alternative implementation is correct as is, but if test 1 is used as a foundation, a second unit test can exercise the 'Happy Path':

[Fact]
public void HappyPath()
{
    // Fixture setup
    var repositoryStub = new Mock();
    repositoryStub.Setup(r => r.GetParameter(1)).Returns("foo");

    var sut = new DomainClass(
        repositoryStub.Object,
        new Mock().Object);
    // Exercise system
    var result = sut.GetErrorList(1);
    // Verify outcome
    Assert.False(result.Any());
    // Teardown
}

Which, together with test 1, forces you to write code like this:

public class DomainClass
{
    private readonly IRepository repository;
    private readonly IErrorProvider errorProvider;

    public DomainClass(IRepository repository, IErrorProvider errorProvider)
    {
        this.repository = repository;
        this.errorProvider = errorProvider;
    }

    public IEnumerable GetErrorList(int id)
    {
        if (string.IsNullOrEmpty(this.repository.GetParameter(1)))
            yield return this.errorProvider.BuildErrorMessage("200, 201");
        yield break;
    }
}

You may, however, notice from the hard-coded 1 parameter that this can't be the final implementation yet. However, all tests are green at this point, so you must add more test cases.

Here's the Happy Path test expanded as a Parameterized Test:

[Theory]
[InlineData(1)]
[InlineData(2)]
[InlineData(3)]
public void HappyPath(int id)
{
    // Fixture setup
    var repositoryStub = new Mock();
    repositoryStub.Setup(r => r.GetParameter(id)).Returns("foo");

    var sut = new DomainClass(
        repositoryStub.Object,
        new Mock().Object);
    // Exercise system
    var result = sut.GetErrorList(id);
    // Verify outcome
    Assert.False(result.Any());
    // Teardown
}

This drives the implementation to this point in order to pass all tests:

```
public class DomainClass
{
private readonly IRepository repository;
private readonly IErrorProvider errorProvider;

public DomainClass(IRepository repository, IErrorProvider errorProvider)
{
this.repository = repository;
this.errorProvider = errorProvider;
}

public IEnumerable GetErrorList(int id)
{
i

StackExchange Code Review Q#18591, answer score: 28