4

I have a viewModel with async Task. I don't now how to test it.

public class MyViewModel : BindableBase
{
    public MyViewModel()
    {
        this.PropertyChanged += MyViewModel_PropertyChanged;
    }

    private void MyViewModel_PropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        Action action = async () => await DoSomething();
        action();
    }

    public const string BeforeKey = "before";
    public const string AfterKey = "After";

    public string Status { get; private set; } = BeforeKey;

    public async Task DoSomething()
    {
        await Task.Delay(3000);
        Status = AfterKey;
    }

    string bindagleProp;
    public string BindagleProp
    {
        get { return bindagleProp; }
        set { SetProperty(ref bindagleProp, value); }
    }
}

Here is my test:

[TestMethod]
public async Task TestMyViewModel()
{
    MyViewModel viewModel = new MyViewModel();
    Assert.AreEqual(viewModel.Status, MyViewModel.BeforeKey, "before check");

    viewModel.BindagleProp = "abc";
    Assert.AreEqual(viewModel.Status, MyViewModel.AfterKey, "after check");
}

The test failed because it's not waiting to completion of the task. I DON'T want to use Task.Delay in the unit test, because it's not safety. DoSomething method can has unknown duration time.

Thank you for any help.


Edit:

In fact, The issue is not specific for MVVM, but for any async event handler. For example:

// class with some logic, can be UI or whatever.
public class MyClassA
{
    Size size;

    public Size Size
    {
        get { return size; }
        set
        {
            size = value;
            SizeChanged?.Invoke(this, EventArgs.Empty);
        }
    }

    public event EventHandler SizeChanged;
}

// this class uses the MyClassA class.
public class MyCunsomerClass
{
    readonly MyClassA myClassA = new MyClassA();

    public MyCunsomerClass()
    {
        myClassA.SizeChanged += MyClassA_SizeChanged;
    }

    public string Status { get; private set; } = "BEFORE";

    private async void MyClassA_SizeChanged(object sender, EventArgs e)
    {
        await LongRunningTaskAsync();
        Status = "AFTER";
    }

    public async Task LongRunningTaskAsync()
    {
        await Task.Delay(3000);
        ///await XYZ....;
    }

    public void SetSize()
    {
        myClassA.Size = new Size(20, 30);
    }
}

Now, I want to test it:

    [TestMethod]
    public void TestMyClass()
    {
        var cunsomerClass = new MyCunsomerClass();
        cunsomerClass.SetSize();
        Assert.AreEqual(cunsomerClass.Status, "AFTER");
    }

The test failed.

Yehudah asher
  • 107
  • 1
  • 7
  • Will the finished code have `await Task.Delay(3000);` in it, or are you using that to simulate a long-running task? – Chris Mantle Nov 06 '15 at 09:53
  • It's for simulation only. The real method can do any long-running task. – Yehudah asher Nov 06 '15 at 10:36
  • This may already have an answer [here](http://stackoverflow.com/questions/27178806/unit-test-async-eventhandler) – Mike Eason Nov 06 '15 at 12:25
  • Your edit doesn't invoke the private event, so there is no reason to test the event handler. – Kcvin Nov 06 '15 at 15:14
  • Additionally, your event handler is supposed to handle an event when it occurs, like a property change (as seen in original Q). The only thing you need to test is that the handler is getting called assuming you have another test, testing that DoSomething works. Because of the nature of MVVM, that event handler could get called a lot when you have many bindable props. From the outside looking in, since the event is private, callers have no idea that tasks are getting fired every time they set a property (think about OnPropertyChanged, how often setter gets called with TextBox). – Kcvin Nov 06 '15 at 15:18
  • Also, since you're trying to test a private event handler/method, I [advise](http://stackoverflow.com/a/250719/1144624) you to rethink your approach. – Kcvin Nov 06 '15 at 15:34
  • 1
    As pointed out by Netscape, you're doing it wrong. This whole discussion is pointless. OP clearly has no idea how events work or how async workflow is supposed to work. He needs to learn those before anything about testing can be discussed. As it stands right now, there's no hope for this question IMHO – Maverik Nov 06 '15 at 15:55
  • @Maverik I see nothing wrong in starting an asynchronous operation in reaction on an event. Especially in UI environment. Yeah, I too would recommend not using events at all, but use direct interface calls. But the event invoker doesn't need to care that it is invoking asynchronous operation. – Euphoric Nov 06 '15 at 15:59
  • My problem is with the fact that he's not subscribing to the event when he triggers property changed yet he expects to have the value there. As for async, he could have awaited it, .Result, .Wait() or some other form of synchronisation. Clearly he's not aware of that so he needs a quick lesson in synchronisation before he can test these things. – Maverik Nov 06 '15 at 16:01
  • 1
    @Maverik .Result and .Wait() are hacks. Not solution. Their usage is generally discouraged. – Euphoric Nov 06 '15 at 16:02
  • We should probably open a ticket on connect then for them to be deprecated but in the absence of any other synchronisation context, I don't see how this can work. Again this discussion isn't about our disagreement on the usage of await/Wait/Result but the fact OP doesn't know how to use things. Until he's able to synchronise, tests can't be done. – Maverik Nov 06 '15 at 16:04
  • @Maverik, I think your answer is not helpful. I presented my idea without getting into details. If you insist, here's a replication done in two seconds: – Yehudah asher Nov 07 '15 at 23:46
  • I put the replication above, instead of the original code. – Yehudah asher Nov 07 '15 at 23:58
  • @Yehudahasher I have NOT presented any answer. The question had multiple basic errors that just state the obvious: you need to learn more about the basic c# language behaviours before you dive into TDD. – Maverik Nov 10 '15 at 18:44

2 Answers2

3

I asked Stehphen Cleary [The famous professor of asynchronous], and he answered me:

If by "async event handler" you mean an async void event handler, then no, those aren't testable. However, they are often useful in a UI application. So what I usually end up doing is having all my async void methods be exactly one line long. They all look like this:

async void SomeEventHandler(object sender, EventArgsOrWhatever args)
{
     await SomeEventHandlerAsync(sender, args);
}

async Task SomeEventHandlerAsync(object sender, EventArgsOrWhatever args)
{
      ... // Actual handling logic
}

Then the async Task version is unit testable, composable, etc. The async void handler isn't, but that's acceptable since it no longer has any real logic at all.

Thanks Stephen! Your idea is excellent!

Yehudah asher
  • 107
  • 1
  • 7
  • Ok, I think that you lost me somewhere.. In your initial question (and in my answer) there are no `async void` anywhere. And it's still not clear to me if you want to run the long-running-task in your unit test, and if so why would you want that? But those things aside, I don't really understand how your answer would help you in your question? I would really appreciate if you could show it with your own initial example code. – Markus Nov 13 '15 at 08:06
  • For future reference, the answer Stephen provided is in the comments of his [Blog, over here](http://blog.stephencleary.com/2013/02/async-oop-5-events.html) – StuartLC Oct 02 '17 at 19:29
2

Ok So first of all, I would move the worker out to an other class and make an interface to it. So that when I run the test I can inject another worker!

public class MyViewModel : BindableBase
{
    private IWorker _worker;

    private readonly DataHolder _data = new DataHolder(){Test = DataHolder.BeforeKey};
    public string Status { get { return _data.Status; } }

    public MyViewModel(IWorker worker = null)
    {
        _worker = worker;
        if (_worker == null)
        {
            _worker = new Worker();
        }

        this.PropertyChanged += MyViewModel_PropertyChanged;
    }

    private void MyViewModel_PropertyChanged(object sender, PropertyChangedEventArgs e)
    {

        Action action = async () => await _worker.DoSomething(_data);
        action();
    }


    string bindagleProp;
    public string BindagleProp
    {
        get { return bindagleProp; }
        set { SetProperty(ref bindagleProp, value); }
    }
}

public class DataHolder
{
    public const string BeforeKey = "before";
    public const string AfterKey = "After";

    public string Status;
}

public interface IWorker
{
    Task DoSomething(DataHolder data);
}

public class Worker : IWorker
{
    public async Task DoSomething(DataHolder data)
    {
        await Task.Delay(3000);
        data.Status = DataHolder.AfterKey;
    }
}

Now the inject code would look something like:

[TestMethod]
public async Task TestMyViewModel()
{
    TestWorker w = new TestWorker();

    MyViewModel viewModel = new MyViewModel(w);
    Assert.AreEqual(viewModel.Status, DataHolder.BeforeKey, "before check");

    viewModel.BindagleProp = "abc";
    Assert.AreEqual(viewModel.Status, DataHolder.AfterKey, "after check");
}

public class TestWorker : IWorker
{
    public Task DoSomething(DataHolder data)
    {
        data.Status = DataHolder.BeforeKey;
        return null; //you maybe should return something else here...
    }
}
Markus
  • 3,297
  • 4
  • 31
  • 52
  • Please write a comment if it doesn't work, or why it's a bad answer – Markus Nov 06 '15 at 13:20
  • You are missing _worker field and call to _worker.DoSomething(). I was wondering where is _worker called. – Euphoric Nov 06 '15 at 13:22
  • Also, I don't think extraction DoSomething for test makes sense, because I'm assuming it is code that is actually being tested. – Euphoric Nov 06 '15 at 13:23
  • Thanks @Euphoric, fixed. About the extraction part, that depends on what the code does.. for example of the `DoSomething` writes to a large file, or some database stuff, then that should probably not be tested in a unit test. – Markus Nov 06 '15 at 13:29
  • @Euphoric it's a view model, why wouldn't you extract `DoSomething` into a model or have it part of a service layer/extension method or something? I don't think Markus is extracting for test, rather extracting it out of VM entirely. – Kcvin Nov 06 '15 at 15:30
  • In your exact example, the Data.Status IMO should be set from the view model. It is VM's responsibility to update itself after that the long-running task finished. – Euphoric Nov 06 '15 at 15:40
  • @Euphoric and in OP's exact example, Status setter isn't calling property changed, so clearly VM isn't updating the view unless the MVVM framework in use is doing it magically. – Kcvin Nov 06 '15 at 15:59
  • @Netscape The "property changed" is called in SetProperty – Euphoric Nov 06 '15 at 16:00
  • @Euphoric which there isn't one for Status. – Kcvin Nov 06 '15 at 16:00
  • @Markus There are many disadvantages to this method, because it is very complicated code. But after much searching I think there is no other solution. async void run independently and there is no way to wait for him. – Yehudah asher Nov 08 '15 at 15:25