dotnet / reactive

The Reactive Extensions for .NET
http://reactivex.io
MIT License
6.66k stars 747 forks source link

Add tests to AsyncRx.NET #1900

Open idg10 opened 1 year ago

idg10 commented 1 year ago

The AsyncRx.NET code is currently experimental. Although it was added to this repository several years ago, it was not published to NuGet until the very recent 6.0.0-alpha.3 release.

It is labelled as alpha to indicate that this is currently a long way from being considered of production quality. In particular, there are currently no tests at all for this code

We would like to be able to share test code with the main Rx.NET repository. (A secondary goal for this sharing is to make it possible for other repositories to run the entire Rx test suite against either Rx.NET, AsyncRx.NET, or both, to make it possible for us to push support for UI frameworks out into separate repos. So a repo with WPF-specific extensions could, in this proposed world, execute all the parts of the test suite that will be affected by WPF-specific concerns, e.g., tests involving schedulers.) The majority of what the Rx.NET tests express is also applicable to AsyncRx.NET. Take this test of the Rx Average operator, for example:

https://github.com/dotnet/reactive/blob/2a8c658d8ba1af1fe21dd70e73fba4d12a85f04f/Rx.NET/Source/tests/Tests.System.Reactive/Tests/Linq/Observable/AverageTest.cs#L101-L126

This describes a sequence of values that an observable source should emit, along with the schedule on which they should be emitted (in virtual time), tells the TestScheduler to send the sequence just described through the Average operator. (Something not immediately obvious from this code is that by default, the TestScheduler.Start method invokes the callback that subscribes to the source at virtual time 200, meaning that the first value specified by this test—value 1 at virtual time 150—will be emitted before the xs.Average() observable is subscribed to.) It then verifies that the expected output is produced at the expected virtual time. (Specifically, it expects a single output immediately after the source completes, whose value is 3.0, the average of the three input values that are produced after subscription, 3, 4, and 2.)

We would want a test for the asynchronous Average operator to test all the same things. There is exactly one detail of this test that makes it specific to the non-async version of Rx, and it's this call to Start:

var res = scheduler.Start(() =>
    xs.Average()
);

That Start method's signature looks like this:

public ITestableObserver<T> Start<T>(Func<IObservable<T>> create)

so the call cs.Average() resolves to this method of the Observable class:

public static IObservable<double> Average(this IObservable<int> source)

What we would like to do is be able to generalize tests of this kind. We have some ideas about how this might be achieved, but haven't yet worked any of the ideas out thoroughly. Suggestions include:

We have yet to prototype any of these, and it's not immediately obvious which approaches will work best. We would want to ensure that whatever we did didn't produce tests that are difficult to debug. We are also expecting that one tricky part to get right is handling cases where there do need to be differences. For example, ordinary Rx.NET's Select operator comes in two forms:

// Just the element:
public static IObservable<TResult> Select<TSource, TResult>(this IObservable<TSource> source, Func<TSource, TResult> selector);

// Element and index:
public static IObservable<TResult> Select<TSource, TResult>(this IObservable<TSource> source, Func<TSource, int, TResult> selector);

but in AsyncRx.NET, there are four:

// Just the element (non-async projection):
public static IAsyncObservable<TResult> Select<TSource, TResult>(this IAsyncObservable<TSource> source, Func<TSource, TResult> selector);

// Just the element (async projection):
public static IAsyncObservable<TResult> Select<TSource, TResult>(this IAsyncObservable<TSource> source, Func<TSource, ValueTask<TResult>> selector);

// Element and index (non-async projection):
public static IAsyncObservable<TResult> Select<TSource, TResult>(this IAsyncObservable<TSource> source, Func<TSource, int, TResult> selector);

// Element and index (async projection):
public static IAsyncObservable<TResult> Select<TSource, TResult>(this IAsyncObservable<TSource> source, Func<TSource, int, ValueTask<TResult>> selector);

More generally, any operator that accepts a callback is likely to have flavours that accept both an ordinary and an async callback. It's not obvious how we might take existing tests for Select like this:

https://github.com/dotnet/reactive/blob/2a8c658d8ba1af1fe21dd70e73fba4d12a85f04f/Rx.NET/Source/tests/Tests.System.Reactive/Tests/Linq/Observable/SelectTest.cs#L530-L532

and augment them to cover the versions that accept async projection callbacks.

It's possible that the kind of reuse we're hoping for is too ambitious. A fallback position is to use clipboard inheritance, but we're hoping to avoid that because it makes ongoing maintenance harder. We would much prefer it for there to be a single source of truth about what constitutes correctness for each operator.

quinmars commented 1 year ago

While both methods might look very similar, the underlying types are completely different. So I'm skeptically that you can solve this problem with language means. Source generators work on class level and not on method level. But there is a sixth options: using simple pattern replace. A short sed script could generate all (at least many) async counter parts:

s/Start(/StartAsync(/g
s/IObservable/IAsyncObservable/g
idg10 commented 1 year ago

the underlying types are completely different. So I'm skeptically that you can solve this problem with language means

We do this kind of type substitution all the time in https://github.com/reaqtive/reaqtor - there's code in there that takes Rx expressions written in one flavour (e.g., IAsyncReactiveQbservable<T>) and rewrites them in some other space (e.g. IObservable<T>.).

To show how this can work, I've put together a rough and ready illustration of the approach at https://github.com/idg10/rx-testing-experiments

If you look at https://github.com/idg10/rx-testing-experiments/blob/main/Solutions/RxGenericTestSpike.Tests/RxGenericTestSpike.SharedTests/AverageTests.cs you'll see an implementation of one of the test cases for Average (the same as the example from my comment above).

The test project in that solution executes the same test twice, once against System.Reactive (i.e., with the source being an IObservable<int> and the second against System.Reactive.Async

The key to this is that the part of the shared test that calls xs.Average() is now doing so in a lambda that's producing an expression tree. And that happens because it goes via this parameter:

https://github.com/idg10/rx-testing-experiments/blob/8000050fe526bae95832582ca0b459652386c3bd/Solutions/RxGenericTestSpike.Tests/RxGenericTestSpike.SharedTests/AverageTests.cs#L11

So that queryRewriter argument is a callback that expects an expression tree whose input type is some IObservable<TIn> and whose output is IObservable<TOut>. And its output is a delegate of the same type.

The crucial point here is that this queryRewriter argument provides a way for whatever code calls this test to cause the Rx query to be rewritten before it is executed.

The non-async version doesn't actually rewrite it at all. It uses https://github.com/idg10/rx-testing-experiments/blob/8000050fe526bae95832582ca0b459652386c3bd/Solutions/RxGenericTestSpike.Tests/RxGenericTestSpike.Tests/ObservableQueryToSync.cs which just compiles the incoming expression to a delegate without modification. That's because the test is already producing Rx queries that work with IObservable<T> so no modification is required.

The interesting one is https://github.com/idg10/rx-testing-experiments/blob/8000050fe526bae95832582ca0b459652386c3bd/Solutions/RxGenericTestSpike.Tests/RxGenericTestSpike.Tests/ObservableQueryToAync.cs

Let's look at what happens when the shared Average test calls that rewriter here:

queryRewriter(xs =>
    xs.Average()
    )(xs)

Remember the queryRewriter expects an expression tree, so C# will compile that lambda not into runnable code, but into an expression tree describing the call. It's going to be something roughly like this:

The expression rewriter produces a new lambda based on that which looks like this:

Note that the rewriter just quietly assumes that any method defined by Observable whose first argument is of type IObservable<T> will have a counterpart with the same name defined by AsyncObservable where the first argument is of type IAsyncObservable<T>. That particular assumption is embodied here:

https://github.com/idg10/rx-testing-experiments/blob/8000050fe526bae95832582ca0b459652386c3bd/Solutions/RxGenericTestSpike.Tests/RxGenericTestSpike.Tests/ObservableQueryToAync.cs#L103-L105

This is just a proof of concept, so there are plenty of problems with it, but it does illustrate that we definitely can solve this without resorting to anything as grotty as text replacement. (If we were to solve it with text-based mangling, I think I'd prefer more T4 templates. But this example shows that you don't need to do it that way.)

BalassaMarton commented 7 months ago

How about reversing the problem: IObservable/IObserver are just special cases of IAsyncObservable/IAsyncObserver that always return synchronously. A test suite written for the async version can be applied to the synchronous one by just wrapping it into something that implements the async interfaces.

idg10 commented 1 week ago

I've finally got a bit of time to experiment with this. Here's an illustration of how things could work with a source generator:

image

On the left we have one of the tests from the normal Rx.NET test suite. (I've copied it out into an experimental project, but we have that test with that name in the real project.) And then on the right is a class generated from that test by a source generator.

@quinmars wrote:

Source generators work on class level and not on method level

I don't know what you mean by that. (And I've now written several source generators.)

Source generators get access to the entire source code (in the form of Roslyn syntax trees for each file) of whatever project they are being used in. So on the input side, they can work at any level you like: you can work with everything from entire source files down to individual syntax tokens. And on the output side, you simply get to add new source files to the project. So a source generator can do anything that can be achieved by adding a source file to a project.

This means that the only real limitation is that you can't change existing source files. (Even then, interceptors can enable you do some of the things you might have wanted to achieve by modifying existing files.)

And that limitation doesn't really matter to us. We want to generate whole new test types that exist in addition to the existing classes. And when it comes to adding completely new types to a project, there are effectively no limitations on what a source generator can do.

A big advantage of this over a basic regular expression based search and replace is that this can work out where it needs to inject await statements, and also when it needs to adjust the entire method to be async to accommodate that. I suspect you can't do that with sed, but even if you could, I suspect the necessary inputs to sed would be pretty horrific.

I've verified that when we generate test types in this way, the newly-generated types are also visible to the unit test tooling in Visual Studio. (I couldn't be 100% sure because the unit test discovery has always been a bit cranky, and source generators are also pretty cranky, so it wouldn't have surprised me if using the two in combination would break. But fortunately, it looks OK.)

So I'm hopeful that this technique could well be viable.

The next step will be to apply this to a wider range of tests to find out what other kinds of transformations need to be applied to make the tests work.

idg10 commented 1 week ago

Looking promising so far:

image

The SelectTest entry here shows tests from SelectTest.cs imported directly from Rx. (It's in the project as a linked file, so it's literally exactly the same set of tests.) And SelectTestAsync is the test class containing asyncified versions of the code, generated from that by my experimental code generator.

It's going to require the addition of a few hand-crafted tests, but it looks like this might be a good way to get a lot of the existing Rx tests to serve as AsyncRx tests as well.