patterncsharpMinor
Unit tests: to cast or not to cast?
Viewed 0 times
nottestsunitcast
Problem
Suppose the following example unit test for an ASP.NET MVC controller:
Resharper unwittingly complains on the fourth line of code that
This has some advantages:
It also has one main disadvantage:
Some other alternatives I've considered include :
This is five lines of code, but has two very clear asserts.
I've even considered this version:
All in all I see the following points
[Test]
public void Delete_Will_Redirect_To_Index_1()
{
bookRepository.Add(someBook);
var result = booksController.Delete(someBook.Id) as RedirectToRouteResult;
Assert.That(result, Is.Not.Null);
Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}Resharper unwittingly complains on the fourth line of code that
result.RouteValues may cause a possible NullReferenceException. I hate those squiggly lines, so I figured I could just as easily write it as follows:[Test]
public void Delete_Will_Redirect_To_Index_2()
{
bookRepository.Add(someBook);
var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}This has some advantages:
- One LoC less;
- More to the point;
It also has one main disadvantage:
- Code will fail with a cast exception if the result is of unexpected type. This seems(?) less clear than a failed assert.
Some other alternatives I've considered include :
[Test]
public void Delete_Will_Redirect_To_Index_3()
{
bookRepository.Add(someBook);
var result = booksController.Delete(someBook.Id);
Assert.That(result, Is.InstanceOf());
var redirectResult = (RedirectToRouteResult)result;
Assert.That(redirectResult.RouteValues["Action"], Is.EqualTo("Index"));
}This is five lines of code, but has two very clear asserts.
I've even considered this version:
[Test]
public void Delete_Will_Redirect()
{
bookRepository.Add(someBook);
var result = booksController.Delete(someBook.Id);
Assert.That(result, Is.InstanceOf());
}
[Test]
public void Delete_Will_Redirect_To_Index_4()
{
bookRepository.Add(someBook);
var redirectResult = (RedirectToRouteResult)booksController.Delete(someBook.Id);
Assert.That(redirectResult.RouteValues["Action"], Is.EqualTo("Index"));
}All in all I see the following points
Solution
You could make a strong argument that you need 2 tests.
General theory has a unit test (as opposed to an integration test) containing only one assert, see e.g. Programmers.StackExchange
So you probably want:
This has the added advantage that it is much easier to find the bug later based on the name of the failed test alone.
For example, if your type test fails, presumably your redirect test will also fail, but I know which I'd look at first during debugging!
If, on the other hand, the
If you have multiple tests, or a guard assertion, the reader (i.e. you at some point in the future) will assume that it must be possible to get, say, a
General theory has a unit test (as opposed to an integration test) containing only one assert, see e.g. Programmers.StackExchange
So you probably want:
[Test]
public void Delete_WithValidBook_ReturnsRedirectToRouteResult()
{
bookRepository.Add(someBook);
var result = booksController.Delete(someBook.Id);
Assert.That(result.Type, Is.EqualTo(typeof(RedirectToRouteResult)));
}
[Test]
public void Delete_WithValidBook_RedirectsToValidIndex()
{
bookRepository.Add(someBook);
var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}This has the added advantage that it is much easier to find the bug later based on the name of the failed test alone.
For example, if your type test fails, presumably your redirect test will also fail, but I know which I'd look at first during debugging!
If, on the other hand, the
Delete method is guaranteed (by the framework) to return a RedirectToRouteResult, then you only want one test:[Test]
public void Delete_WithValidBook_RedirectsToValidIndex()
{
bookRepository.Add(someBook);
var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}If you have multiple tests, or a guard assertion, the reader (i.e. you at some point in the future) will assume that it must be possible to get, say, a
null result, which will mean you may feel the need to add null checks, etc, to other parts of the code that may not be required.Code Snippets
[Test]
public void Delete_WithValidBook_ReturnsRedirectToRouteResult()
{
bookRepository.Add(someBook);
var result = booksController.Delete(someBook.Id);
Assert.That(result.Type, Is.EqualTo(typeof(RedirectToRouteResult)));
}
[Test]
public void Delete_WithValidBook_RedirectsToValidIndex()
{
bookRepository.Add(someBook);
var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}[Test]
public void Delete_WithValidBook_RedirectsToValidIndex()
{
bookRepository.Add(someBook);
var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}Context
StackExchange Code Review Q#46676, answer score: 8
Revisions (0)
No revisions yet.