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
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
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