6

Assume we have a method that validates an object before saving it to a DB. It returns back KVP something like:

public KeyValuePair<bool, List<FooErrorReason>> ValidateABar(Bar b)
{
    var reasons = new List<FooErrorReason>();
    if (string.IsNullOrEmpty(bar.Name)) 
    {
        reasons.Add(new FooErrorReason { PropName = "Name", Message = "The name was missing" };
    }
    if (string.IsNullOrEmpty(bar.OtherName)) 
    {
        reasons.Add(new FooErrorReason { PropName = "OtherName", Message = "The other name was missing" };
    }
    return new KeyValuePair<bool, List<FooErrorReason>(reasons.Count == 0, reasons);
}

So now I want to test that, for a missing bar.Name, the error is returned. My question is on the make up of the test, concerning what to Assert.

public void ValidateABar_Fails_For_Missing_Name()
{
    //arrange
    var b = new Bar { OtherName = "Other" };

    //act
    var result = ValidateABar(b);

    //assert
    Assert.IsFalse(result.Key);
    Assert.AreEqual(1, result.Value.Count);
    Assert.IsTrue(result.Value.Any(x => x.PropName.Equals("Name")));
}

I have 3 Assert calls and it has a whiff of code smell about it, but I can't quite convince myself either way.

I'm asking myself 'What do you want to test?' and I do kind of feel that I should test that this failed, but failed specifically for the property of the test.

Is it enough to just use

Assert.IsTrue(result.Value.Any(x => x.PropName.Equals("Name")));

Or should I also check that there are no other errors I have picked up along the way?

Say for example someone added another validation rule without my knowing about it, I would think I would want my tests to tell me about this the next time I ran them. As an example, let's say someone changed the method as below:

public KeyValuePair<bool, List<FooErrorReason>> ValidateABar(Bar b)
{
    var reasons = new List<FooErrorReason>();
    if (string.IsNullOrEmpty(bar.Name)) 
    {
        reasons.Add(new FooErrorReason { PropName = "Name", Message = "The name was missing" };
    }
    if (string.IsNullOrEmpty(bar.OtherName)) 
    {
        reasons.Add(new FooErrorReason { PropName = "OtherName", Message = "The other name was missing" };
    }

    //added this!
    if (string.IsNullOrEmpty(bar.ThirdName)) 
    {
        reasons.Add(new FooErrorReason { PropName = "ThirdName", Message = "The third name was missing" };
    }
    return new KeyValuePair<bool, List<FooErrorReason>(reasons.Count == 0, reasons);
}

The test would now fail and I would see that this rule had been added, and amend my test accordingly (and presumably write another test to cover the new rule).

Am I taking the 'one Assert per test rule' too literally?

2
  • 1
    Be careful of wrapping a bool and a list of messages like that. it's not a key-value pair, so don't call it one. If you MUST take short-cuts, at least use a Tuple<T1, T2>. Much better though, to create a FooValidationReponse object with Success and Messages properties, to make it very clear what's going on.
    – pdr
    Commented Jan 3, 2013 at 13:26
  • I agree and we are using a Response object. I thought KVP was clearer for a quick question, but can see how it is not now it is written out. Will bear in mind for the future!
    – glosrob
    Commented Jan 3, 2013 at 13:28

2 Answers 2

6

You are taking one assertion per test too literally, but not too seriously. It's a good rule to follow.

However, there are cases, like this one, where you cannot fully assert your one assertion with literally one Assert method. What you are trying to assert is that the response is showing an error message. Assert.AreEqual doesn't really allow you to do that, unless you write an equality comparer for your response object.

Another option is to use the Assert.That syntax and create your own IResolveConstraint object as a comparer. With a little effort, you could have something as simple as

Assert.That(response, Fails.With.Message("Blah"));

But really, is it worth the effort? What you have is just fine and it is asserting your response is correct. That is one assertion.

In other cases, you won't actually have any calls to Assert methods, because the thing you're asserting is that a mocked method was called correctly. That's still an assertion, even if it isn't an Assert, per se.

For example, say you have a Save method, which sends data to a repository (not caring if it's SQL, file system, mock object, etc.) and then returns nothing.

public void Save()
{
     // How do I test this method with an Assert call???
     FooRepository.Save(this.Name, this.OtherName.Trim());
}

What you don't want to do is set an expectation on a mock method and then assert the response from the method under test. That would be two assertions in one test (although only one Assert call) and, if one thing goes wrong, it will be more difficult to figure out exactly what.

In a case like that, write two tests:

  • one to test that the method is called correctly
  • one to assert the contents of any response

In the latter, simply stub the method, so that it passes regardless of whether the method is called, as long as the response is correct.

TL;DR: Do not confuse asserting one thing with having one call to an Assert method. They're not the same thing.

3
  • Did your last sentence mean to say Test the expectation in one test and stub the method to assert the response object **in another test** ? I'm having a hard time following your answer though I think it might agree with me, I think you could use a TL;DR just to clarify because I'm finding it quite confusing.. Commented Jan 3, 2013 at 15:21
  • @JimmyHoffa: Well noted. Clearer now?
    – pdr
    Commented Jan 3, 2013 at 15:31
  • Thanks, I think I understand the difference now. Useful distinction between Assert and asserting.
    – glosrob
    Commented Jan 3, 2013 at 17:40
5

honestly the "one assert per test" rule is overrated, a better rule would be "on act per test" and have a smell when you have more than a few asserts which means that the act does too much

you have a method with a compound output so it makes sense to test all parts of the output. Doing this in a single test means that when a new field is required you only need to change 1 test instead of 3

I believe that you can add a string to the assert to be displayed when it fails, or you should at least see the line where it fails in the stacktrace. this way you can narrow it down what went wrong

besides the first relfex you should have should be to add a breakpoint before the first assert and run the test again and see what output you got

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.