Async method within constructor affecting unit tests

Question

Have a unit test that tests that navigation is only allowed if Jobs.Count > 0. GetJobsAsync() is called during construction and clears the Jobs list if not null (gets fresh list each time it is called). It appears that the Jobs list is cleared after I manually add a new job as a condition for the unit test to pass. How do I get this timing correct so that the Jobs list isn't cleared during the running of the test?

In MyClass constructor:

this.GetJobsAsync();

GetJobsAsync:

private async void GetJobsAsync()
{
    var jobs = await this.dataService.GetJobs();
    if (jobs != null)
    {
        this.Jobs.Clear();

        foreach (var job in jobs)
        {
            this.Jobs.Add(new JobViewModel(job));
        }
    }

    // have the select job command rerun its condition
    this.SelectJobCommand.RaiseCanExecuteChanged();
}

Unit test (must have at least one job for navigation, Jobs is cleared after job is added):

var vm = new MyClass();
vm.Jobs.Add(new JobVM(new JobModel()));
vm.SelectJobCommand.Execute(null);            
Assert.AreEqual(
  NavigationKeys.WizardJob,
  this.navigationService.CurrentPageKey);

This is why you don't have `async void` methods that aren't top level methods. — Servy, Mar 16 '15 at 19:50
Wouldn't the best practice be to set the DataService to return the Job? That way the Testing flow mimics the production flow as accurate as possible. Typically I would do that by have a Mock DataService object in my Unit Tests. — Silas Reinagel, Mar 16 '15 at 20:04
The issue is that in the real app you've got a SynchronizationContext that is keeping things single threaded and maintaining the order for you, while in a unit test you don't have one so multiple threads are running and you've got a race condition. — Mike Zboray, Mar 16 '15 at 20:11
@SilasReinagel - Not sure, either should be OK, but the problem still remains. — O.O, Mar 16 '15 at 20:23

score 2 · Accepted Answer · answered Mar 16 '15 at 20:07

2

It appears that the Jobs list is cleared after I manually add a new job as a condition for the unit test to pass.

It's clear that you understand that you have a race condition. You can't await on async void, and you're inside a constructor.

How do I get this timing correct so that the Jobs list isn't cleared during the running of the test?

Two things. First, change GetJobAsync to async Task instead of async void. Then, use the async initialize pattern:

public class MyClass
{
    public Task InitializeAsync()
    {
         return GetJobsAsync();
    }
}

And in your unit test:

public async Task TestNavigationAsync()
{
     var vm = new MyClass();
     await vm.InitializeAsync();

     vm.Jobs.Add(new JobVM(new JobModel()));
     vm.SelectJobCommand.Execute(null);       

     Assert.AreEqual(NavigationKeys.WizardJob,
                                  navigationService.CurrentPageKey); 
 }

This will assure the order of execution.

answered Mar 16 '15 at 20:07

Yuval Itzchakov

146,575
32
257
321

The problem with this is that it doesn't work very well with WPF's model. In a typical app, `MyClass` would be created and initialized from a property getter where you won't be able to await. – Mike Zboray Mar 16 '15 at 20:09
Who said anything about WPF and properties? Anyway, he can use any `Loaded` or `Navigated` event to trigger the initialization via `AsyncCommand` – Yuval Itzchakov Mar 16 '15 at 20:10
@YuvalItzchakov - I think that would work great for the unit test as the unit test waits for data before continuing, but I'm struggling with where InitializeAsync would be called in the actual code as it is currently in the constructor... oh wait you might have just answered that... – O.O Mar 16 '15 at 20:16
Obviously it doesn't explicitly say it, but the references to VMs, commands, and a navigation service strongly imply it or a similar MVVM framework. – Mike Zboray Mar 16 '15 at 20:17
@mikez - Then it should be properly stated as part of the problem. Currently it seems like the main concern is around the unit test. Anyway, WPF has plenty of ways to handle this. – Yuval Itzchakov Mar 16 '15 at 20:18
@mikez - yes, you are correct this is MVVM Light for windows store app and view model instances are initialized in XAML and view model locator, but I'll have to think about what he said using an event to load instead of the constructor. – O.O Mar 16 '15 at 20:19
1

@O.O - Exactly. Async event handler or async commands are what you're after. – Yuval Itzchakov Mar 16 '15 at 20:19

Async method within constructor affecting unit tests

1 Answers1