ReactiveUI icon indicating copy to clipboard operation
ReactiveUI copied to clipboard

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

Open zamzowd opened this issue 5 years ago • 9 comments

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.

zamzowd avatar Mar 26 '21 18:03 zamzowd

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

Also mind updating this is for blazor earlier on?

glennawatson avatar Mar 26 '21 20:03 glennawatson

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.

zamzowd avatar Mar 26 '21 21:03 zamzowd

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;

felipeleon73 avatar Jun 10 '21 14:06 felipeleon73

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 avatar Aug 10 '21 20:08 ChrisPulman

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.

ChrisPulman avatar Aug 11 '21 07:08 ChrisPulman

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 avatar Aug 11 '21 13:08 zamzowd

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

zamzowd avatar Aug 11 '21 13:08 zamzowd

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

glennawatson avatar Aug 17 '21 02:08 glennawatson

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.

ChrisPulman avatar Aug 22 '21 18:08 ChrisPulman