reactiveui / ReactiveUI

An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms that is inspired by functional reactive programming. ReactiveUI allows you to abstract mutable state away from your user interfaces, express the idea around a feature in one readable place and improve the testability of your application.
https://www.reactiveui.net
MIT License
8.04k stars 1.12k forks source link

[BUG] IsExecuting doesn't update in unit tests mocking a task with scheduler duration #2716

Open zamzowd opened 3 years ago

zamzowd commented 3 years ago

Describe the bug When unit testing with a TestScheduler to control timing, the IsExecuting of a ReactiveCommand created from a task does not emit new values after awaiting scheduler.Sleep(...).

Using the VirtualTimeSchedulerBase override of scheduler.Sleep(...) misaligns the expected timing.

Steps To Reproduce An example can be seen here: https://github.com/zamzowd/ZamzowD.ExampleBlazor

  1. Create a ViewModel with an injected service.
  2. Add a ReactiveCommand created from a task, including awaiting a task from the injected service.
  3. Add a property created with .IsExecuting.ToProperty(...) from the command.
  4. Create a unit test project to test the ViewModel
  5. mock the injected service. mock the task to include await scheduler.Sleep(TimeSpan.FromTicks(50)).
  6. With a TestScheduler, schedule one or more executions of the command
  7. Start the scheduler for either the IsExecuting observable or changes to the property from PropertyChanged events.
  8. Run the test

Expected The IsExecuting and PropertyChanged indicate an update to true when the ReactiveCommand starts and false when the ReactiveCommand completes.

[OnNext(True)@211, OnNext(False)@261, OnNext(True)@311, OnNext(False)@361]

Actual The IsExecuting and PropertyChanged indicate an update to true once, but never an update to false nor subsequent updates to true.

With await scheduler.Sleep(TimeSpan.FromTicks(50)):

[OnNext(True)@211]

With scheduler.Sleep(50):

[OnNext(True)@260, OnNext(False)@262, OnNext(True)@360, OnNext(False)@362]

Breakpoints are still hit through each scheduled execution. Running the Blazor application normally demonstrates the expected behavior. Properties updated in the command (Todo in the example project) have the expected timing in PropertyChanged events. It is only IsExecuting that falls silent, and only in unit tests.

glennawatson commented 3 years ago

So blazor app. Are you running the state update method on the form?

Also mind updating this is for blazor earlier on?

zamzowd commented 3 years ago

If you mean StateHasChanged, then yes. I am using ReactiveInjectableComponentBase, which does that.

Running the actual blazor app seems to work as expected. It's the unit test in which the mocked task includes an await scheduler.Sleep(...) where the IsExecuting becomes silent. I didn't specify blazor in the description since it seemed like the ViewModel and the unit testing class didn't use anything from ReactiveUI.Blazor.

felipeleon73 commented 3 years ago

I think it's a scheduling problem. I am having a similar error in a Visual Studio Extension project where a command opens a WPF window. I this window I have a button with binding on a ReactiveCommand:

Save = ReactiveCommand.CreateFromTask(async () =>
  {
     if (!ItemVM.Modified || await Interactions.RequestConfirm.Handle("Are you sure you want to save the changes?"))
       {
           var newAppInfo = ItemVM.ToAppInfo();
           await Task.Run( () => throw new Exception("EXXXX"));
           return newAppInfo;
       }
       return null;
  }, ItemVM.IsValid()
);

ThrownExcpetions kick an interaction Save.ThrownExceptions.SelectMany(x => Interactions.Errors.Handle(x)).Subscribe();

And I have an interaction handler

Interactions.Errors.RegisterHandler(i =>
  {
     MessageBox.Show($"Something went wrong\n{i.Input.Message}\n{i.Input.InnerException?.Message}", "Agrolab Apps Management", MessageBoxButton.OK, MessageBoxImage.Error);
     i.SetOutput(Unit.Default);
  }
);

Everything works properly and the messagebox appears, but the button remains grayed as if the command were still running.

PS In order to avoid errors in creating the RxApp I had to manually set the scheduler: RxApp.MainThreadScheduler = Scheduler.Immediate;

ChrisPulman commented 3 years ago

Is this still an issue? Unit tests for a ReactiveCommand normally require you to set the Scheduler to ImmediateScheduler.Instance for both Schedulers that now exist, execution Scheduler and canExecute Scheduler

ChrisPulman commented 3 years ago

I just checked against the repo posted and this is still not functioning as per the test in the repo with the latest build as of today. I have a thought that its to do with Observable.FromAsync and the way its working but will dig deeper and try to find out.

zamzowd commented 3 years ago

Thanks for following up.

I updated the repo with latest Blazor and ReactiveUI nuget packages. As you said, it doesn't address this issue.

I also created an override-scheduler branch, following the guidance in ReactiveUI - Testing (and also setting canExecuteScheduler, unspecified in the article).

It does have an impact on the LoadTodos_UpdatesLoading test, but not to the desired result.

In master branch: image

In override-scheduler branch: image

zamzowd commented 3 years ago

I'm not sure where I would reference Scheduler.Immediate or ImmediateScheduler.Instance, and couldn't find guidance on it.

glennawatson commented 3 years ago

@zamzowd It's part of the ReactiveX/System.Reactive that ReactiveUI is based on.

ChrisPulman commented 3 years ago

We are trying to come up with a resolve for this but the root cause is in the System.Reactive code https://github.com/dotnet/reactive/issues/1256 This issue has been outstanding for quite some time.