API proposal: rewrite blocking elements of TestKit to return awaitable Tasks

[Aaronontheweb]

Version: Akka.NET 1.5.0 and later

As part of the Akka.NET v1.4..0 sprint I've spent a lot of time going through our test suite and disabling parts of it that are especially timing-sensitive. I think the reasons for most of these races boils down to the following:

1. On Azure DevOps we can't be sure of the size and load of the hosted build agent pool. I haven't been able to find out for certain looking through the docs but I assume agents are single tenant when they're actually running a live job (not running jobs for multiple tenants in parallel.)
2. Many of the operations inside the testkit explicitly block threads, i.e. TestLatch.Ready, ExpectMsg, AwaitCondition, etc.
3. We do want to block the flow of execution in our tests, for sure - that's how we make it sane and easy to write them. However, the fact that there are blocked threads means that an assertion like ExpectMsg<T>(TimeSpan.FromSeconds(300)) can succeed or fail depending on how many cores are available and wait the blocking state of the work queue looks like.
4. If these were written to use async and await instead, it could work with XUnit's ability to take Task off of the test method and would allow for background threads with work to do to run while foreground threads that are waiting on results are also unblocked.
5. I think this would help make our test suite much less racy overall and make the TestKit more reliable.

However, this comes with some flies in the ointment:

1. More cruft to deal with inside the writing of each test - now we have to propagate Task everywhere.
2. XUnit supports returning Task off the top of each of its test methods, but what about NUnit, MsTest, and other test frameworks that we've supported historically? Where does that leave them?
3. It's still possible that we could make all of these changes and not see much improvement in the test suite - because there are likely other factors at work that I'm discounting, such as how much work other processes are plying onto the build agent during our test runs.

If we were going to introduce this, here is what I'd propose doing:

1. Leave all existing public TestKit APIs as-is for backwards compatibility, mark them as deprecated.
2. Add Async versions that return Task - don't need to rename the method necessarily with the Async prefix, just need to add some overloads that return them.
3. Replace the default method implementations to all call down to the async versions of these.

Either way, wanted to throw this idea out there for consideration in Akka.NET 1.5.0 and later. What are your thoughts?

[Horusiath]

Honestly, I think we could greatly simplify the testkit AP - for example TestProbe could adopt IAsyncEnumerator or even ChannelReader API:

// equivalent of probe.ExpectNext
(await probe.NextAsync(cancellationToken)).Should().Be(new ExpectedMessage())

// equivalent of probe.FishForMessage
while (await probe.TryNextAsync(cancellationToken) && probe.LastMessage == new ExpectedMessage());

Of course, we could create a helper methods to simplify those patterns, but I've noticed that people on gitter are often asking how to do something, because the patterns and test kit methods don't feel familiar to them. Using async enumeration-like API is IMO very intuitive for them. There are also additional simplifications:

• Since probe just returns a message but doesn't do assertions, we won't need separate test probes for every assertion library.
• No need for methods like ReceiveOne/ReceiveMany.
• No need for Inbox type - probe already would provide the same features.

[Aaronontheweb]

@Horusiath I agree that returning something that is more LINQ-like might be helpful, since that's familiar, intuitive, and flexible enough to support a wide variety of use cases. That in combination with something like FluentAssertions for doing nasty stuff like checking that the messages returned are all in a specific order, or at least 1 of them are found inside a collection, etc.

I also think we'd need to launch a Roslyn analyzer embedded into the Akka.TestKit assembly to stop us from giving end-users enough rope to hang themselves. The fact that these methods will need to be await-ed inside the main test method or the test will fail isn't going to be immediately obvious to end-users. One of the great strengths of the current TestKit is that it automatically blocks the flow, which is what we want. If we have to rely on end-users do to that in their own tests it's going to open up a new can of worms. Therefore, needs to be more powerful warnings than the built-in one from XUnit - like a full-blown build error unless that Roslyn warning is explicitly disabled. That should cut down any "the TestKit is broken!!!1!" end-user complaints significantly, I expect.

Alternatively, if there's a way of forcing the current TestKit API to just await without propagating a Task and without blocking the thread all the way up to each test method themselves, that would also be ideal as it won't require users to change any existing code - but as far as I know that's not possible.