Am I writing my unit tests correctly? NUnit + NSubstitute

Question

I recently started learning how to write unit tests, and what part of the unit to test for functionality and what to mock out. I'm using NSubstitute as my mocking framework. My example basically calls a repo class, which then makes an WEB API web call to an external service, ie: AddCreditCard, which then returns a result. I created 2 unit tests for AddCreditCard, one for Success, and one for Fail. I'm still not 100% sure I'm doing all of this correctly. The unit tests are passing, but im not sure if my Asserts are done on the correct data... All help and suggestions welcome!

public interface IApiResponse<T>
{
    HttpStatusCode StatusCode { get; set; }
    T Result { get; set; }
    string ErrorMessage { get; }
    bool HasErrors { get; }
}

public interface ISignedRequest
{
    void ConfigureSettings(SignedRequestSettings settings);

    IApiResponse Post(string jsonData, Dictionary<string, string> parameters = null,
        IOptionalHeaders optionalHeaders = null);
}


public class PaymentRepository
{
    private readonly SignedRequestSettings _settings;
    private readonly ISignedRequest _signedRequest;

    public PaymentRepository(ISignedRequest signedRequest = null)
    {
        if (signedRequest == null)
            _signedRequest = new SignedRequest();
        else
            _signedRequest = signedRequest;
    }

    public IApiResponse AddCreditCard(CreditCard request)
    {
        var jsonData =
            JsonConvert.SerializeObject(request);

        string action = string.Format("customers/{0}/paymentmethods", request.ConnectId);
        _settings.Url = string.Format("{0}{1}", String.Empty, action);
        _signedRequest.ConfigureSettings(_settings);

        return _signedRequest.Post(jsonData);
    }
 }

    [Test]
    public void AddCreditCard_GivenValidCreditCard_ReturnsCreatedResult()
    {
        //Given
        var apiResponse = Substitute.For<IApiResponse>();
        apiResponse.StatusCode = HttpStatusCode.Created;

        var signedRequest = Substitute.For<ISignedRequest>();
        signedRequest.Post(Arg.Any<String>()).Returns(apiResponse);

        var creditCard = Substitute.For<CreditCard>();
        creditCard.ConnectId = Guid.Parse("1fc1ad83-cd4e-4b68-bce6-e03ee8f47fb6");

        var repo = new PaymentRepository(signedRequest);

        //When
        var addCreditCardResponse = repo.AddCreditCard(creditCard);

        //Then
        signedRequest.Received(1).ConfigureSettings(Arg.Any<SignedRequestSettings>());
        signedRequest.Received(1).Post(Arg.Any<String>());

        Assert.AreEqual(HttpStatusCode.Created, addCreditCardResponse.StatusCode);
    }

[Test]
    public void AddCreditCard_GivenInvalidCreditCard_ReturnsHasErrorsResult()
    {
        //Given
        var apiResponse = Substitute.For<IApiResponse>();
        apiResponse.HasErrors.Returns(true);
        apiResponse.ErrorMessage.Returns("Add credit card error message");

        var signedRequest = Substitute.For<ISignedRequest>();
        signedRequest.Post(Arg.Any<String>()).Returns(apiResponse);

        var creditCard = Substitute.For<CreditCard>();
        creditCard.ConnectId = Guid.Parse("1fc1ad83-cd4e-4b68-bce6-e03ee8f47fb6");

        var repo = new PaymentRepository(signedRequest);

        //When
        var addCreditCardResponse = repo.AddCreditCard(creditCard);

        //Then
        signedRequest.Received(1).ConfigureSettings(Arg.Any<SignedRequestSettings>());
        signedRequest.Received(1).Post(Arg.Any<String>());

        Assert.AreEqual(apiResponse.HasErrors, addCreditCardResponse.HasErrors);
        Assert.AreEqual(apiResponse.ErrorMessage, addCreditCardResponse.ErrorMessage);
    }

On first blush these look like great unit tests. One suggestion I'd have is to use specific arguments or [argument matchers](http://nsubstitute.github.io/help/argument-matchers/) in your `signedRequest.Received(1)` calls. Unless those values are not important to what you are testing. — Kris McGinnes, Apr 14 '15 at 19:38

score 2 · Accepted Answer · answered May 19 '15 at 15:07

I think your tests are mostly OK, but there are some bits I would question. Both of your tests have these lines at the bottom:

signedRequest.Received(1).ConfigureSettings(Arg.Any<SignedRequestSettings>());
signedRequest.Received(1).Post(Arg.Any<String>());

Is it really important to those tests that these methods have been called on your signedRequest substitute? I'd suggest that it probably isn't. If these calls weren't made I'd argue that your test would have failed anyway (although this is to some extent a stylistic decision).

The second thing I'd say is that you're missing one or more tests (again the number is a bit stylistic). As it stands, you're not validating the information that's being supplied to the ConfigureSettings or Post calls on your signedRequest substitute. Your repository code could simply be doing _signedRequest.Post("some random string"); and your test would still pass. I'd add another test that validates these calls to ensure that the request is actually sent correctly. This is where the Received validation would make sense. Something like:

signedRequest.Received(1).ConfigureSettings(Arg.Is<SignedRequestSettings>(x=>x.Url == someHardCodedUrl));
signedRequest.Received(1).Post(someHardCodedJsonString);

Am I writing my unit tests correctly? NUnit + NSubstitute

1 Answers1