2012-02-06

Async Unit Tests, Part 1: The Wrong Way

"Code without tests does not exist."

(Overheard at CodeMash)

The core meaning of this quote is that code without unit tests is not as useful as code with unit tests. The speaker even goes so far as to say he won't use code without tests.

I don't take a position quite this extreme, but I definitely agree with the underlying sentiment: that code with unit tests is far more useful. Unit tests prove correct functionality (at least for a limited set of cases). Unit tests also provide a sort of documentation.

If you don't write unit tests - or if you or your manager think writing tests just delays software development - then I refer you to the best computer book ever written, Code Complete. In that book, Steve McConnell presents some very interesting hard facts about testing.

I hope we can all agree that unit testing is a fundamental skill in Modern Programming. And this brings me to a sad chapter in async/await support: the "obvious" way to do unit tests is wrong.

Let's start with a simple asynchronous method. So simple, in fact, that it will just pretend to work for a while and then do a single integer division:

public static class MyClass
{
  public static async Task<int> Divide(int numerator, int denominator)
  {
    // Work for a while...
    await Task.Delay(10); // (Use TaskEx.Delay on VS2010)

    // Return the result
    return numerator / denominator;
  }
}

Boy, it doesn't seem that there can be much wrong with that code! But as we'll see, there's a lot that can be wrong with the unit tests...

When developers write unit tests for async code, they usually take one of two mistaken approaches (with the second one being what I call "obvious"). We'll look at each of these approaches in this post and examine why they're wrong, and look at solutions next time.

Wrong Way #1: Using Task.Wait and Task.Result

This mistake is most common for people new to async: they decide to wait for the task to complete and then check its result. Well, that seems logical enough, and some unit tests written this way actually work:

[TestMethod]
public void FourDividedByTwoIsTwo()
{
  Task<int> task = MyClass.Divide(4, 2);
  task.Wait();
  Assert.AreEqual(2, task.Result);
}

But one of the problems with this approach is unit tests that check error handling:

[TestMethod]
[ExpectedException(typeof(DivideByZeroException))]
public void DenominatorIsZeroThrowsDivideByZero()
{
  Task<int> task = MyClass.Divide(4, 0);
  task.Wait();
}

This unit test is failing, even though the async method under test is throwing a DivideByZeroException. The Test Results Details explains why:

The Task class is wrapping our exception into an AggregateException. This is why the Task.Wait and Task.Result members should not be used with new async code (see the end of last week's async intro post).

Well, we could await the task, which would unwrap the exception for us. This would require our test method to be async. Congratulations, you can move on to the next section.

Wrong Way #2: Using Async Test Methods

This mistake is more common for people who have used async in some real-world code. They've observed how async "grows" through the code base, and so it's natural to extend async to the test methods. This is what I consider the "obvious" solution:

[TestMethod]
public async Task FourDividedByTwoIsTwoAsync()
{
  int result = await MyClass.Divide(4, 2);
  Assert.AreEqual(2, result);
}

Yay! It works!

...

Wait...

[TestMethod]
public async Task FourDividedByTwoIsThirteenAsync()
{
  int result = await MyClass.Divide(4, 2);
  Assert.AreEqual(13, result);
}

Um, that test should certainly not be passing! What is going on here???

We've encountered a situation very similar to async in Console programs: there is no async context provided for unit tests, so they're just using the thread pool context. This means that when we await our method under test, then our async test method returns to its caller (the unit test framework), and the remainder of the async test method - including the Assert - is scheduled to run on the thread pool. When the unit test framework sees the test method return (without an exception), then it marks the method as "Passed". Eventually, the Assert will fail on the thread pool.

There is now a race condition. There's no race condition in the test itself; it will always pass (incorrectly). The race condition is when the assertion fires. If the assertion fires after the unit test framework finishes the test run, then you'll see a successful test run (like the last screenshot). But if the assertion fires before the unit test framework finishes the test run, then you'll see something like this:

Clicking on the link shows that the Assertion is indeed failing on the thread pool, some time after the test is considered completed and "Passed":

Update: Visual Studio 2012 will correctly support "async Task" unit tests, but doesn't appear to support "async void" unit tests.

Next Time: The Right Way

During CodeMash, I gave a lightning talk about async unit testing. You could almost hear the teeth grinding at this point, when the TDD/BDD fans discovered that async unit tests were essentially broken. But do not give up hope!

Tomorrow we'll look at the right way to do async unit testing.

5 comments:

  1. Thanks! I was wondering why my "failing" tests weren't actually failing.

    ReplyDelete
  2. Errr: It seems like there are two problems here, *none* of which are related to async.

    1. Using 'async void' is just plain wrong here.
    Refer: http://msdn.microsoft.com/en-us/library/vstudio/hh156513.aspx
    - "The caller of a void-returning async method can't await it and can't catch exceptions that the method throws."

    2. ExpectedExceptionAttribute isn't a good solution here, or anywhere.
    The issue with unwrapping the AggregateException again isn't anything to do with async, it's just that ExpectedException doesn't allow you to verify the InnerException. Just write a helper function to catch the exception for you, and then verify it properly using normal asserts!.

    ReplyDelete
    Replies
    1. At the time this blog post was written (when only VS2010 had been released), "async Task" was not an option for unit tests. BTW, it is possible to provide a context for async void methods and both wait for them and retrieve their exceptions; this is how nUnit supports async void unit tests.

      I agree with your comment re ExpectedExceptionAttribute. I use and recommend a ThrowsAsync method for checking exceptions on async methods. I used ExpectedExceptionAttribute examples in this blog post because that's Microsoft's recommendation and the approach most MSTest users are familiar with.

      Delete
  3. Not coding without tests is NOT extreme. It's how you get shit done and how our trade will and is developing into and how we should be developing software. Test Driven is effective, it's not theory and if you adhere to principals and see what it can do, you never question doing tests, you ALWAYS do tests. http://blog.8thlight.com/eric-smith/2013/04/08/we-are-principled-6.html

    ReplyDelete
    Replies
    1. Agree to disagree. I read that blog post about a year ago.

      There are certain types of code such as domain objects and business rules that are very amenable to tests and are ideal for test-driven development. I would go so far as to say that there are entire applications (probably a majority of applications) that can be developed using TDD.

      However, there are also other types of code such as low-level synchronization libraries that cannot be fully developed using test-driven development; at some point a deep understanding of race conditions and other multithreading concerns *must* guide the code development. TDD alone and unguided would result in subtle bugs.

      Delete