Over the last week, I've implemented support for
await in the Fixie test framework. Thanks to a suggestion from Pedro Reys, I found that this project was susceptible to a serious bug, one that NUnit and xUnit both encountered and addressed back when the
await keywords were introduced in C# 5.
While developing the fix, I relearned an important lesson: cutting scope is not a sign of defeat. Sometimes less really is more.
With the bug in place, a test framework can report that a test has passed even when it should fail. Consider the following test fixture:
The developer of these tests should expect TestAwaitThenPass to be the only passing test. The other four tests should all fail with one exception or another. Unfortunately, Fixie would claim that all 5 of these tests pass. To make matters even more confusing, despite "passing", TestAsyncVoid's DivideByZeroException would still be output to the user.
When you call most async methods, the method call will not actually do the work. Rather, the method will quickly return a
Task that knows how to do that work. To provoke the
Task to execute, you must call its Wait() method. I was failing to call Wait(), so I would happily report success for a test that was never actually executed in full!
In the case of an
async void method, calling the method does cause the work to take place, but the exception does not surface in the normal fashion. The test framework's own try/catch blocks won't catch it, and it will bubble all the way up before appearing in the output as an unhandled exception.
The Initial Requirements
Once I could reproduce the problem, I came up with the first version of my new requirements. Since
async methods must be declared to return
Task<T>, and since all of these pose the same risk of the test passing when it shouldn't,
async Tasktest method must be waited upon before deciding whether it passes or fails.
asyc Task<T>test method must be waited upon before deciding whether it passes or fails.
async voidtest method must be waited upon before deciding whether it passes or fails.
The Easy Part
We want to do the extra work for methods declared with the
async keyword, and fortunately we can detect that keyword using reflection. When you use this keyword, the compiled method gains an attribute available to us at runtime:
Before the fix, a test method would be executed via reflection like so:
We can fix the execution of
async Task and
asyc Task<T> by waiting for the returned
Task to complete:
When a regular test fails,
method.Invoke(...) throws. When an
async test fails,
The third requirement is problematic. If a test method is declared
method.Invoke(...) returns null, so we'll never see the
Task object and will never be able to call
task.Wait(). It turns out there is an extremely complex workaround, implemented in NUnit, which takes advantage of implementation details surrounding
await execution. After researching the technique, I lacked confidence that I would use it correctly.
The Actual Requirement
I started to question the train of thought which led to the original 3 requirements. All async methods have to be declared as returning
Task<T>, otherwise they won't compile, and I was naively assuming that all three of these variations were good test declarations.
It turns out that declaring methods
async void is frowned upon for exactly the same reason they were giving me trouble: it is crazy weird and difficult to correctly wait on a
Task when the
Task itself is inaccessible to you!
async void declarations say, "I want to fire and forget", but a test author does not want the test framework to forget what's going on! The only reason
async void even exists is for a specific edge case: async event handlers have no choice but to be declared void.
The actual requirement I needed to meet was to provide accurate pass/fail reporting: a test passes if and only if the test framework executes it in full without throwing exceptions.
In the case of
async void, I satisfy this requirement by slapping the test author's hand. I fail such a test method immediately, without bothering to execute it. The failure message explains that "void" should be replaced with "Task". Requiring that the test author replace 4 characters with 4 characters, rather than encourage a bad habit of writing
async void methods, is actually better than supporting all variations of
Less is More
Requirements are human decisions based on incomplete information. With enough information, you may better-serve the needs of your system and its users by not doing something.
In this case, supporting all 3 kinds of asynchronous methods would have introduced a great deal of complexity and risk, and I have absolutely no interest in introducing complexity or risk into something as fundamental as a test framework. By treating
async void methods as "real" test cases that always fail, I satisfy the requirement of providing accurate pass/fail reporting. By cutting scope, I'm providing a better solution.