Today I was reading a book which showed an example of an unit test. And even if it was just there to show unit testing as a concept when doing ASP.Net MVC development, I really think you never, ever, ever, ever, ever.....(n)....ever should use that ugly attribute. Why, well follow along.
BTW, the example tests below is not as they were in the book and they are just created for showing you the fragility of ExpectedExceptionAttribute
Test 1: Place a bid When no bids exists Then the bid gets added
You should be able to place a bid when no other bid exists, then there should be one bid registered and that is the one you placed.
The Test
[Test]
public void WhenNoBidsExists_BidIsAdded()
{
const int artefactId = 1;
const decimal bid = 100m;
var auction = new Auction();
auction.PlaceBid(artefactId, bid);
var bids = auction.GetBids(artefactId);
Assert.AreEqual(bid, bids.Single());
}
The Implementation
public class Auction
{
private readonly Dictionary<int, List<decimal>> _bids
= new Dictionary<int, List<decimal>>();
public void PlaceBid(int artefactId, decimal bid)
{
_bids.Add(artefactId, new List<decimal> { bid });
}
public IEnumerable<decimal> GetBids(int artefactId)
{
if (!_bids.ContainsKey(artefactId))
return Enumerable.Empty<decimal>();
return _bids[artefactId];
}
}
The Result
------ Test started: Assembly: ClassLibrary1.dll ------
1 passed, 0 failed, 0 skipped, took 0,14 seconds (NUnit 2.5.5).
OK, nothing complicated here, lets move on to the next test.
Test 2: Place a bid When a higher bid exists Then it throws an InvalidOperationException
[Test]
[ExpectedException(typeof(InvalidOperationException))]
public void WhenHigherBidExists_ThrowsInvalidOperationException()
{
const int artefactId = 1;
var auction = new Auction();
auction.PlaceBid(artefactId, 10m);
auction.PlaceBid(artefactId, 5m);
}
The Implementation
public void PlaceBid(int artefactId, decimal bid)
{
var hasExistingExceedingBid = GetBids(artefactId)
.Any(existingBid => existingBid >= bid);
if (hasExistingExceedingBid)
throw new InvalidOperationException(
"The bid is to low. Existing bids with same or higher value exists.");
_bids.Add(artefactId, new List<decimal> { bid });
}
The Result
------ Test started: Assembly: ClassLibrary1.dll ------
2 passed, 0 failed, 0 skipped, took 0,14 seconds (NUnit 2.5.5).
OK, everything works and the ExpectedException is thrown and the second test passes. Lets continue and add yet another test to meet a new requirement.
Test 3: Place a bid When the bid is lower than five Then it throws an InvalidOperationException
Just by reading the test spec there should be an alarm bell going of here. I will give you a small hint: ... Then it throws an InvalidOperationException. Two tests for different requirements throws the same exception. What could go wrong? Lets see. Lets continue our journey.
The Test
[Test]
[ExpectedException(typeof(InvalidOperationException))]
public void WhenBidIsLowerThan5_ThrowsInvalidOperationException()
{
const int artefactId = 1;
var auction = new Auction();
auction.PlaceBid(artefactId, 4m);
}
The Implementation
public void PlaceBid(int artefactId, decimal bid)
{
if (bid < 5)
throw new InvalidOperationException(
"The bid is to low. A bid must be greater than 5.");
var hasExistingExceedingBid = GetBids(artefactId)
.Any(existingBid => existingBid >= bid);
if (hasExistingExceedingBid)
throw new InvalidOperationException(
"The bid is to low. Existing bids with same or higher value exists.");
_bids.Add(artefactId, new List<decimal> { bid });
}
The Result
------ Test started: Assembly: ClassLibrary1.dll ------
3 passed, 0 failed, 0 skipped, took 0,14 seconds (NUnit 2.5.5).
OK, still everything works out fine, or does it?
A change of the requirements - instead of check against bid lower than five, ten should be used
Piece of cake. Just update the test WhenBidIsLowerThan5_ThrowsInvalidOperationException and make it fail and correct the implementation so that we get a green test for the new requirement.
The Test
[Test]
[ExpectedException(typeof(InvalidOperationException))]
public void WhenBidIsLowerThan10_ThrowsInvalidOperationException()
{
const int artefactId = 1;
var auction = new Auction();
auction.PlaceBid(artefactId, 9m); //UPDATED
}
The Implementation
public void PlaceBid(int artefactId, decimal bid)
{
if (bid < 10) //UPDATED
throw new InvalidOperationException(
"The bid is to low. A bid must be greater than 10."); //UPDATED
var hasExistingExceedingBid = GetBids(artefactId)
.Any(existingBid => existingBid >= bid);
if (hasExistingExceedingBid)
throw new InvalidOperationException(
"The bid is to low. Existing bids with same or higher value exists.");
_bids.Add(artefactId, new List<decimal> { bid });
}
The Result
------ Test started: Assembly: ClassLibrary1.dll ------
3 passed, 0 failed, 0 skipped, took 0,14 seconds (NUnit 2.5.5).
Three tests passes. How about that! Nothing wrong there is it? Of course. Lets tweak the test that checks if there's any existing bids that are higher.
Updated test #1
[Test]
[ExpectedException(
typeof(InvalidOperationException),
ExpectedMessage = "The bid is to low. Existing bids with same or higher value exists.")]
public void WhenHigherBidExists_ThrowsInvalidOperationException_Better1()
{
const int artefactId = 1;
var auction = new Auction();
auction.PlaceBid(artefactId, 10m);
auction.PlaceBid(artefactId, 5m);
}
I have now put an expectation on the message, now lets see what happens.
------ Test started: Assembly: ClassLibrary1.dll ------
Test 'ClassLibrary1.AuctionTests.WhenHigherBidExists_ThrowsInvalidOperationException_Better1' failed: The exception message text was incorrect
Expected: The bid is to low. Existing bids with same or higher value exists.
but was: The bid is to low. A bid must be greater than 10.
BidTests.cs(80,0): at ClassLibrary1.Auction.PlaceBid(Int32 artefactId, Decimal bid)
BidTests.cs(56,0): at ClassLibrary1.AuctionTests.WhenHigherBidExists_ThrowsInvalidOperationException_Better1()
3 passed, 1 failed, 0 skipped, took 0,20 seconds (NUnit 2.5.5).
The exception message text was incorrect
Expected: The bid is to low. Existing bids with same or higher value exists.
but was: The bid is to low. A bid must be greater than 10.
How about that! The exception was thrown at the wrong place. Of course this situation could have been avoided with some specific business exception or even an ArgumentOutOfRangeException; but again. This is a simplified, fictive scenario which still isn't making me happy, since the ugly ExpectedExceptionAttribute still is present. Lets fix that.
Assert.Throws
Moste testing frameworks (NOT MSTest) has some sort of ability to pass a delegate to a method and the lets you specify that you expect that delegate to throw an exception of a certain type. If the exception isn't thrown, the test fails.
For you MSTest users, you can of course write this test util on your own.
Updated test #2
[Test]
public void WhenHigherBidExists_ThrowsInvalidOperationException_Better2()
{
const int artefactId = 1;
var auction = new Auction();
auction.PlaceBid(artefactId, 10m);
var ex = Assert.Throws<InvalidOperationException>(
() => auction.PlaceBid(artefactId, 5m));
Assert.AreEqual(
"The bid is to low. Existing bids with same or higher value exists.",
ex.Message);
}
I hope you get the point, and that you stop using the not so nifty ExpectedExceptionAttribute
//Daniel