Testing Async Commands?

Hi everyone,

How about testing an async command delegates?

public class ViewModel
{
  Command _DoWorkCommand;
  public Command DoWorkCommand =>
    _DoWorkCommand ?? (_DoWorkCommand = new Command(DoWork));
  async void DoWork()
  {
    await Task.Run(() => Thread.Sleep(5000));
    WorkDone = true;
  }

  public bool WorkDone { get; private set; }

  [Fact]
  public void TestDoWork()
  {
    DoWorkCommand.Execute(null);
    Assert.True(WorkDone);
  }
}

The assertion fails because assertion happens before the task in DoWork has completed and WorkDone has been set to true.

Am I wrong calling an async void from my command? Any nice general way to address this?

Best Answers

  • ShimmyWeitzhandlerShimmyWeitzhandler US ✭✭✭
    edited June 2017 Accepted Answer

    @Velocity

    1. Not so relevant, because of the pattern of the Xamarin Command, unless I want to dirtify my VMs.
    2. I thought about option 2, but
      a. It's gonna tremendously slow down the tests.
      b. Times are really unpredictable, how much should should I dedicate to each test? To each inner connection?

    Isn't there a general guidance for this?
    I'm thinking, since the long-operation in my app is mostly web-requests, maybe I should interfere with the inner HttpMessageHandler which I've anyway overridden and have access to, and expose a pair of events, something like Requested and Responded and a PendingRequestCount property, so to give an indication and track keeping of whether any requests are still undergoing. Is this a bad idea?

  • ShimmyWeitzhandlerShimmyWeitzhandler US ✭✭✭
    edited June 2017 Accepted Answer

    I managed to make up something that work, I'll facilitate it to a more reusable tool, but I hope to find a better way, so here it is:

    public class Client
    {
      public virtual async Task SendAsync() =>
        await Task.Run(() => Thread.Sleep(10000));
    }
    
    public class TestClient : Client
    {
      public event EventHandler<int> Completed;
      public int RequestCount { get; private set; }
      public override async Task SendAsync()
      {
        RequestCount++;
        await base.SendAsync();
        Completed?.Invoke(this, --RequestCount);
        return;
      }
    }
    
    public class ViewModel
    {
      public const string SuccessMessage = "I was downloaded from a remote server";
      TestClient Client = new TestClient();
    
      DelegateCommand _DoWorkCommand;
      public DelegateCommand DoWorkCommand => _DoWorkCommand ?? (_DoWorkCommand = new DelegateCommand(DoWork));
      async void DoWork()
      {
        await Client.SendAsync();
        Payload = SuccessMessage;
      }
      public string Payload { get; private set; } = "Nada";
    
      [Fact]
      public async void TestDoWork()
      {
        var task = new TaskCompletionSource<int>();
        void OnCompleted(object sender, int e)
        {
          if (e == 0)
          {
            Client.Completed -= OnCompleted;
            task.TrySetResult(e);
          }
        }
    
        Client.Completed += OnCompleted;
        await DoWorkCommand.Execute();      
    
        await task.Task;
    
        //allow for short local setup
        //(i.e. settings the payload property
        //with result from server or merging changes etc.)
        Thread.Sleep(500);
        Assert.Equal(SuccessMessage, Payload);
    
      }
    }
    
  • ShimmyWeitzhandlerShimmyWeitzhandler US ✭✭✭
    edited June 2017 Accepted Answer

    Actually even easier without changing the VM, what I had to do is call it as a task in the test method:

    public async Task TestMethod() =>
      await Task.Run(() => Vm.DoWorkCommand.Execute(null));
    

    Thank you very much @BrianLagunas!

Answers

  • VelocityVelocity NZMember ✭✭✭

    A couple of things here:
    1. Try to avoid use of async void, except maybe possibly for event handlers. Return a Task or Task<T>.
    2. Await a Task.Delay instead of using Task.Run with Thread.Sleep. ie. await Task.Delay(5000);

  • ShimmyWeitzhandlerShimmyWeitzhandler USMember ✭✭✭
    edited June 2017 Accepted Answer

    @Velocity

    1. Not so relevant, because of the pattern of the Xamarin Command, unless I want to dirtify my VMs.
    2. I thought about option 2, but
      a. It's gonna tremendously slow down the tests.
      b. Times are really unpredictable, how much should should I dedicate to each test? To each inner connection?

    Isn't there a general guidance for this?
    I'm thinking, since the long-operation in my app is mostly web-requests, maybe I should interfere with the inner HttpMessageHandler which I've anyway overridden and have access to, and expose a pair of events, something like Requested and Responded and a PendingRequestCount property, so to give an indication and track keeping of whether any requests are still undergoing. Is this a bad idea?

  • ShimmyWeitzhandlerShimmyWeitzhandler USMember ✭✭✭
    edited June 2017 Accepted Answer

    I managed to make up something that work, I'll facilitate it to a more reusable tool, but I hope to find a better way, so here it is:

    public class Client
    {
      public virtual async Task SendAsync() =>
        await Task.Run(() => Thread.Sleep(10000));
    }
    
    public class TestClient : Client
    {
      public event EventHandler<int> Completed;
      public int RequestCount { get; private set; }
      public override async Task SendAsync()
      {
        RequestCount++;
        await base.SendAsync();
        Completed?.Invoke(this, --RequestCount);
        return;
      }
    }
    
    public class ViewModel
    {
      public const string SuccessMessage = "I was downloaded from a remote server";
      TestClient Client = new TestClient();
    
      DelegateCommand _DoWorkCommand;
      public DelegateCommand DoWorkCommand => _DoWorkCommand ?? (_DoWorkCommand = new DelegateCommand(DoWork));
      async void DoWork()
      {
        await Client.SendAsync();
        Payload = SuccessMessage;
      }
      public string Payload { get; private set; } = "Nada";
    
      [Fact]
      public async void TestDoWork()
      {
        var task = new TaskCompletionSource<int>();
        void OnCompleted(object sender, int e)
        {
          if (e == 0)
          {
            Client.Completed -= OnCompleted;
            task.TrySetResult(e);
          }
        }
    
        Client.Completed += OnCompleted;
        await DoWorkCommand.Execute();      
    
        await task.Task;
    
        //allow for short local setup
        //(i.e. settings the payload property
        //with result from server or merging changes etc.)
        Thread.Sleep(500);
        Assert.Equal(SuccessMessage, Payload);
    
      }
    }
    
  • JimBennettJimBennett GBXamarin Team, Insider, University, Developer Group Leader ✭✭✭✭

    I use MvvmCross and that has an Async Command built in - this command has an ExecuteAsync method I can use in my unit tests. You could implement your own command to do something similar - check out the source at https://github.com/MvvmCross/MvvmCross/blob/develop/MvvmCross/Core/Core/ViewModels/MvxAsyncCommand.cs

  • BrianLagunasBrianLagunas USInsider ✭✭✭✭
    edited June 2017

    I would just be careful with those as those might not behave the way you want them to as the UI framework (button) does not invoke those async

  • ShimmyWeitzhandlerShimmyWeitzhandler USMember ✭✭✭
    edited June 2017

    @BrianLagunas

    var command = new Command(async () => await DoWork());

    A bit ugly (especially since you have to make your method public), and I prefer the ugly code to be in the tests, but I'll go for it.

  • ShimmyWeitzhandlerShimmyWeitzhandler USMember ✭✭✭
    edited June 2017 Accepted Answer

    Actually even easier without changing the VM, what I had to do is call it as a task in the test method:

    public async Task TestMethod() =>
      await Task.Run(() => Vm.DoWorkCommand.Execute(null));
    

    Thank you very much @BrianLagunas!

  • ShimmyWeitzhandlerShimmyWeitzhandler USMember ✭✭✭

    Still ran into some issues.

    Using @StephenCleary's AsyncContext solved them all. I hope I'm using the right approach. Anyway only using it in the testing so as long as the test is valid and passes I can live with it.

  • GaetanFGaetanF USMember ✭✭✭

    Side note:

    Do not use Thread.Sleep in any way in an async context. Do use Task.Delay.

    Thread.Sleep blocks the current executing thread and monopolises it whereas it could be used somewhere else. If you launch a lot of unit tests in parallel that uses this code, chances are the test will blow up because of threadpool thread starvation. Task.Delay is meant to "yield" the execution for a while and trigger the task to be put back to the threadpool task queue. It will be taken back when the delay elapses or is canceled.

    By doing await Task.Run(() => Thread.Sleep(5000));, you are launching a task (potentially stressing the threadpool by creating a new threadpool thread) on a background thread and ask for it to sit and do nothing, consuming CPU resource for nothing. Instead, await Task.Delay(5000).ConfigureAwait(false) would just yield the task, and consume nothing.

    Async/Await Best practices

  • GaetanFGaetanF USMember ✭✭✭

    This is a rework version of @ShimmyWeitzhandler that takes advantage of async guidelines (not tested)

    public class Client
    {
        public Client() : this(10000)
        {
        }
    
        public Client(int sendDelay_ms)
        {
            SendDelayMS = sendDelay_ms;
        }
    
        public int SendDelayMs { get; }
    
        //  Avoid async if you can, it adds overhead and GC stress. Returning a Task is perfectly fine if you don't await it
        //  Also passing a CancellationToken to a cancellable task is almost always a good idea.
        public virtual Task SendAsync() => Task.Delay(SendDelayMs);
    }
    
    public class TestClient : Client
    {
        public event EventHandler<int> Completed;
    
        private int _requestCount = 0;
    
        public int RequestCount => _requestCount;
    
        public override async Task SendAsync()
        {
            //  Because tests can be launched in parallel, use Interlocked method to provide atomic operation and all time up-to-date value of the request count.
            Interlocked.Increment(ref _requestCount);
    
            //  Use ConfigureAwait(false) if it does not matter wether the remaining work has to be done on the same synchronisation context (UI thread, APS.Net Classic Request thread).
            await base.SendAsync().ConfigureAwait(false);
    
            //  Dangerous if base.SendAsync() sends an exception. RequestCount will not be decremented.
            Completed?.Invoke(this, Interlocked.Decrement(ref _requestCount));
        }
    }
    
    public class ViewModel
    {
        public ViewModel(Client client)
        {
            Client = client;
        }
    
        public const string SuccessMessage = "I was downloaded from a remote server";
        Client Client {get;}
    
        DelegateCommand _doWorkCommand;
        public DelegateCommand DoWorkCommand => _doWorkCommand ?? (_doWorkCommand = new DelegateCommand(DoWork));
    
        async void DoWork()
        {
            await Client.SendAsync().ConfigureAwait(false);
            Payload = SuccessMessage;
        }
    
        public string Payload { get; private set; } = "Nada";
    }
    
    [TestUnit]  //  Or equivalent
    public class ViewModelTest
    {
        [TestFixture]
        public void Setup()
        {
            Client = new Client(10000);
        }
    
        private Client Client {get; set;}
    
        [Fact]
        public async void ExecuteAsync_DoWorkCommand()
        {
            var uut = new ViewModel(Client);
    
            await uut.DoWorkCommand.Execute().ConfigureAwait(false);
            //allow for short local setup
            //(i.e. settings the payload property
            //with result from server or merging changes etc.)
            await Task.Delay(500).ConfigureAwait(false);
    
            Assert.Equal(ViewModel.SuccessMessage, uut.Payload);
        }
    }
    

    Feel free to comment/correct/do any constructive action. :)

  • NMackayNMackay GBInsider, University mod

    I'd suggest Brian's accepted answer from 2017 is the best approach, that's what we do.

Sign In or Register to comment.