Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Was ManuallyPumpedSynchronizationContext required in Jon Skeet's "TimeMachine" async unit test framework?

I've just seen a video-class by Jon Skeet, where he talks about unit testing asynchronous methods. It was on a paid website, but I've found something similar to what he says, in his book (just Ctrl+F "15.6.3. Unit testing asynchronous code").

The complete code can be found on his github, but I have simplified it for the sake of my question (my code is basically StockBrokerTest.CalculateNetWorthAsync_AuthenticationFailure_ThrowsDelayed() but with TimeMachine and Advancer operations inlined).

Let's suppose we have a class to test a failed login (no unit test framework to simplify the question):

public static class LoginTest
{
    private static TaskCompletionSource<Guid?> loginPromise = new TaskCompletionSource<Guid?>();

    public static void Main()
    {
        Console.WriteLine("== START ==");

        // Set up
        var context = new ManuallyPumpedSynchronizationContext(); // Comment this
        SynchronizationContext.SetSynchronizationContext(context); // Comment this

        // Run method under test
        var result = MethodToBeTested();
        Debug.Assert(!result.IsCompleted, "Result should not have been completed yet.");

        // Advancing time
        Console.WriteLine("Before advance");
        loginPromise.SetResult(null);
        context.PumpAll(); // Comment this
        Console.WriteLine("After advance");

        // Check result
        Debug.Assert(result.IsFaulted, "Result should have been faulted.");
        Debug.Assert(result.Exception.InnerException.GetType() == typeof(ArgumentException), $"The exception should have been of type {nameof(ArgumentException)}.");

        Console.WriteLine("== END ==");
        Console.ReadLine();
    }

    private static async Task<int> MethodToBeTested()
    {
        Console.WriteLine("Before login");
        var userId = await Login();
        Console.WriteLine("After login");
        if (userId == null)
        {
            throw new ArgumentException("Bad username or password");
        }

        return userId.GetHashCode();
    }

    private static Task<Guid?> Login()
    {
        return loginPromise.Task;
    }
}

Where the implementation of ManuallyPumpedSynchronizationContext is:

public sealed class ManuallyPumpedSynchronizationContext : SynchronizationContext
{
    private readonly BlockingCollection<Tuple<SendOrPostCallback, object>> callbacks;

    public ManuallyPumpedSynchronizationContext()
    {
        callbacks = new BlockingCollection<Tuple<SendOrPostCallback, object>>();
    }

    public override void Post(SendOrPostCallback callback, object state)
    {
        Console.WriteLine("Post()");
        callbacks.Add(Tuple.Create(callback, state));
    }

    public override void Send(SendOrPostCallback d, object state)
    {
        throw new NotSupportedException("Synchronous operations not supported on ManuallyPumpedSynchronizationContext");
    }

    public void PumpAll()
    {
        Tuple<SendOrPostCallback, object> callback;
        while(callbacks.TryTake(out callback))
        {
            Console.WriteLine("PumpAll()");
            callback.Item1(callback.Item2);
        }
    }
}

The output is:

== START ==
Before login
Before advance
After login
After advance
== END ==

My question is: Why do we need the ManuallyPumpedSynchronizationContext?

Why isn't the default SynchronizationContext enough? The Post() method isn't even called (based on the output). I've tried commenting the lines marked with // Comment this, and the output is the same and the asserts pass.

If I understood correctly what Jon Skeet says in the video, the SynchronizationContext.Post() method should be called when we meet an await with a not-yet-completed task. But this is not the case. What am I missing?

Aditional info

Through my researches, I stumbled across this answer. To try it, I changed the implementation of the Login() method to:

private static Task<Guid?> Login()
{
    // return loginPromise.Task;
    return Task<Guid?>.Factory.StartNew(
        () =>
        {
            Console.WriteLine("Login()");
            return null;
        },
        CancellationToken.None,
        TaskCreationOptions.None,
        TaskScheduler.FromCurrentSynchronizationContext());
}

With that modification, the Post() method was indeed called. Output:

== START ==
Before login
Post()
Before advance
PumpAll()
Login()
After login
After advance
== END ==

So with Jon Skeet's use of TaskCompletionSource, was his creation of ManuallyPumpedSynchronizationContext not required?

Note: I think the video I saw was made just around C# 5 release date.

like image 413
vyrp Avatar asked Mar 20 '17 08:03

vyrp


3 Answers

In this case, the SetResult is executing its continuation synchronously (directly). This is due to a couple of undocumented details:

  1. await will schedule its continuations with the TaskContinuationOption.ExecuteSynchronously flag. When I first discovered this behavior, I reported it as a bug. While I still think it's less surprising to have asynchronous continuations, there is a valid efficiency argument in favor of synchronous execution.
  2. When await captures a SynchronizationContext, it will allow synchronous continuations if the current SynchronizationContext is the same instance as the captured SynchronizationContext (reference equality). Again, this is for performance reasons; equality on SyncCtx instances is not well-defined, but this works well enough in the real world.

So, you're seeing this behavior because at the SetResult line, SynchronizationContext.Current is set to the same SyncCtx that was captured by the await in MethodToBeTested.

A more realistic example would clear the current SyncCtx after calling the system under test. Thus the unit test code doesn't exist "inside" the SyncCtx; it only provides a SyncCtx for the system under test:

...
// Set up
var context = new ManuallyPumpedSynchronizationContext(); // Comment this
SynchronizationContext.SetSynchronizationContext(context); // Comment this

// Run method under test
var result = MethodToBeTested();
Debug.Assert(!result.IsCompleted, "Result should not have been completed yet.");

// Tear down SyncCtx.
SynchronizationContext.SetSynchronizationContext(null);

// Advancing time
...

Alternatively, you can pass TaskCreationOptions.RunContinuationsAsynchronously to the TaskCompletionSource<T> constructor. However, note this bug currently present in the .NET Framework will prevent this from working on full-desktop console apps; it only works for .NET Core console apps.

Or, of course, you can just wrap the SetResult in a Task.Run:

Task.Run(() => loginPromise.SetResult(null)).Wait();

which forces the continuation on a thread pool thread (without a SyncCtx), so the continuation will have to call Post.

As a final note, you may want to use my AsyncContext type from the AsyncEx library; it is a more fleshed-out custom SynchronizationContext that ties itself to a specific thread. I originally wrote AsyncContext for use with unit tests. When the SUT has asynchronous code, it commonly needs a SyncCtx. So much so, in fact, that xUnit provides its own built right into the test framework.

like image 131
Stephen Cleary Avatar answered Sep 30 '22 23:09

Stephen Cleary


I won't discuss what is the intended goal of this code, since I did not read the book or whole code on github link that you posted. I will just work with the code you posted in current question.

I would argue that in the code you provided, ManuallyPumpedSynchronizationContext is not used (regardless of where you run it: in console app, unit test, UI application and so on). It's Post method will not be called, because there is no synchronization context switch. It's usually stated that continuation of await will be Posted to captured synchronization context, and that is in general true, but if after awaited method completes you are still on the same synchronization context - there is no reason to post anything - you are on the same context and can just continue. This is what happens here. When you call:

loginPromise.SetResult(null);

Current context is still ManuallyPumpedSynchronizationContext.

However, if you change it like this:

SynchronizationContext.SetSynchronizationContext(null);
loginPromise.SetResult(null);

Now when Login() completes you are not on captured context any more, so continuation will indeed be Posted to it, and so continuation will be delayed until you call PumpAll.

UPDATE: see @StephenCleary for a more complete explanation of this behavior (there is one more factor involved that is not mentioned in my answer).

like image 21
Evk Avatar answered Oct 01 '22 23:10

Evk


Because you executing your code in console application.
Console application doesn't have synchronization context SynchronizationContext.Current will be always null.

Purpose of ManuallyPumpedSynchronizationContext is "save" synchronization context where test method is executed and "pump" the result provided by of completed task to the saved context.

In console application saved context is null so you didn't see any difference

like image 32
Fabio Avatar answered Oct 02 '22 23:10

Fabio