0

I am trying to write a unit test around an async pub/sub system. In my unit test, I create a TaskCompletionSource<int> and assign it a value within the subscription callback. Within the subscription callback, I unsubscribe from the publications. The next time I publish, I want to verify that the callback never got hit.

[TestMethod]
[Owner("Johnathon Sullinger")]
[TestCategory("Domain")]
[TestCategory("Domain - Events")]
public async Task DomainEvents_subscription_stops_receiving_messages_after_unsubscribing()
{
    // Arrange
    string content = "Domain Test";
    var completionTask = new TaskCompletionSource<int>();

    DomainEvents.Subscribe<FakeDomainEvent>(
        (domainEvent, subscription) =>
        {
            // Set the completion source so the awaited task can fetch the result.
            completionTask.TrySetResult(1);
            subscription.Unsubscribe();
            return completionTask.Task;
        });

    // Act
    // Publish the first message
    DomainEvents.Publish(new FakeDomainEvent(content));
    await completionTask.Task;

    // Get the first result
    int firstResult = completionTask.Task.Result;

    // Publish the second message
    completionTask = new TaskCompletionSource<int>();
    DomainEvents.Publish(new FakeDomainEvent(content));
    await completionTask.Task;

    // Get the second result
    int secondResult = completionTask.Task.Result;

    // Assert
    Assert.AreEqual(1, firstResult, "The first result did not receive the expected value from the subscription delegate.");
    Assert.AreEqual(default(int), secondResult, "The second result had a value assigned to it when it shouldn't have. The unsubscription did not work.");
}

When I do this, the test hangs at the second await. I understand that this happens due to the Task never returning. What I'm not sure is how to work around it. I know I could easily create a local field that I just assign values to like this:

[TestMethod]
[Owner("Johnathon Sullinger")]
[TestCategory("Domain")]
[TestCategory("Domain - Events")]
public void omainEvents_subscription_stops_receiving_messages_after_unsubscribing()
{
    // Arrange
    string content = "Domain Test";
    int callbackResult = 0;

    DomainEvents.Subscribe<FakeDomainEvent>(
        (domainEvent, subscription) =>
        {
            // Set the completion source so the awaited task can fetch the result.
            callbackResult++;
            subscription.Unsubscribe();
            return Task.FromResult(callbackResult);
        });

    // Act
    // Publish the first message
    DomainEvents.Publish(new FakeDomainEvent(content));

    // Publish the second message
    DomainEvents.Publish(new FakeDomainEvent(content));

    // Assert
    Assert.AreEqual(1, firstResult, "The callback was hit more than expected, or not hit at all.");
}

This feels wrong though. This assumes I never perform an await operation (which I do when their are subscribers) within the entire stack. This test isn't a safe test as the test could finish before the publish is totally finished. The intent here is that my callbacks are asynchronous and publications are non-blocking background processes.

How do I handle the CompletionSource in this scenario?

Johnathon Sullinger
  • 7,097
  • 5
  • 37
  • 102
  • It sounds like a ManualResetEvent or CountdownEvent would be more appropriate. – Mike Zboray Jan 16 '16 at 05:12
  • I would also say that I think Publish should return a Task, if it does not already. This is extremely useful because callers can then await if they only wish to process when all subscribers have processed the event. It would also then make the test trivial. Just await Publish and make sure the counter is 1. – Mike Zboray Jan 16 '16 at 05:19
  • The intent in this scenario is to provide DomainEvents an asynchronous workflow, while keeping my domain models synchronous. I didn't want to make the entire domain layer async to facilitate the data and servicing layers subscribed to the events. If I make the `Publish` method return a `Task` and don't await it in my Domain, isn't that a bad practice when using TPL? – Johnathon Sullinger Jan 16 '16 at 05:22
  • I changed my Publish to go ahead and return a `Task`. My Domain won't have to await on it so it's not a big deal. I'm just not sure if that's typically frowned on when using TPL. It feels like i'm changing my code to facilitate a test, which i shouldn't have to :/ – Johnathon Sullinger Jan 16 '16 at 05:52
  • 1
    There are other possibilities. One would be to simply wait on TCS task with a timeout after the second publish instead of awaiting, say 5 seconds. The task should never complete. Another option might be to use another handler that you know would be called after the first one, although this might be relying on an implementation detail in ordering of subscriptions. Testing that something didn't happen can be difficult. – Mike Zboray Jan 16 '16 at 06:18

1 Answers1

2

It's difficult to test that something won't ever happen. About the best you can do is test that it didn't happen within a reasonable time. I have a library of asynchronous coordination primitives, and to unit test this scenario I had to resort to the hack of observing the task only for a period of time, and then assuming success (see AssertEx.NeverCompletesAsync).

That's not the only solution, though. Perhaps the cleanest solution logically is to fake out time itself. That is, if your system has sufficient hooks for a fake time system, then you can actually write a test ensuring that a callback will never be called. This sounds really weird, but it's quite powerful. The disadvantage is that it would require significant code modifications - much more than just returning a Task. If you're interested, Rx is the place to start, with their TestScheduler type.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810