trjhtr

I have a project where I want to build a more sophisticated ToDo list - basically a personal project management system. I'm just starting out with the project, and I'd like some feedback on whether my test methods are OK, as I'm pretty new to TDD. So far, for the main page that lists the daily ToDo items, I've got the following ViewModel:

public class MainVM : ViewModelBase

 private IEnumerable<ToDoItem> _toDoItems;
 private IRepository<ToDoItem> _toDoItemRepo;
 private bool _dataIsLoaded;

 public bool DataIsLoaded
 
 get return _dataIsLoaded; 
 set Set(ref _dataIsLoaded, value, true); 
 

 public IEnumerable<ToDoItem> ToDoItems
 
 get return _toDoItems; 
 set Set(ref _toDoItems, value); 
 

 public MainVM(IRepository<ToDoItem> toDoItemRepo)
 
 _toDoItemRepo= toDoItemRepo;
 LoadData().ContinueWith(t => FinishedLoadingData(t));
 

 private void FinishedLoadingData(Task loadTask)
 
 switch (loadTask.Status)
 
 case TaskStatus.RanToCompletion:
 DataIsLoaded = true;
 break;
 default:
 DataIsLoaded = false;
 break;
 
 

 public async Task LoadData()
 
 if (!DataIsLoaded)
 ToDoItems= await _toDoItemRepo.GetAsync();
 

As you can see, I want to load my data when the ViewModel is created.

I'm using the Set method from the MVVMLight toolkit to set the DataIsLoaded property- it sets the field behind and raises the PropertyChanged event for me.

I have the following test set up to test for a successful load:

 public void DataIsLoaded_is_true_if_loading_task_ran_to_completion()
 
 AutoResetEvent testTrigger = new AutoResetEvent(false);

 TaskCompletionSource<IEnumerable<ToDoItem>> taskCompletion = new TaskCompletionSource<IEnumerable<ToDoItem>>();
 taskCompletion.SetResult(null);

 Mock<IRepository<ToDoItem>> repoMock = new Mock<IRepository<ToDoItem>>();
 repoMock.Setup(s => s.GetAsync()).Returns(taskCompletion.Task);

 MainVM vm = new MainVM(repoMock.Object);
 vm.PropertyChanged += (s, e) =>
 
 if (e.PropertyName == nameof(MainVM.DataIsLoaded))
 testTrigger.Set();
 ;

 testTrigger.WaitOne(5000);

 Assert.IsTrue(vm.DataIsLoaded);
 

Now, I can see that there is a potential race condition here. The LoadData method could complete before the event handler is attached. As an alternative, I could use the Set method that broadcasts a message to MVVMLight's message handler in my ViewModel, and change my test method to this:

 [Test]
 public void DataIsLoaded_is_true_if_loading_task_ran_to_completion()
 
 AutoResetEvent testTrigger = new AutoResetEvent(false);

 TaskCompletionSource<IEnumerable<ToDoItem>> taskCompletion = new TaskCompletionSource<IEnumerable<ToDoItem>>();
 taskCompletion.SetResult(null);

 Mock<IRepository<ToDoItem>> repoMock = new Mock<IRepository<ToDoItem>>();
 repoMock.Setup(s => s.GetAsync()).Returns(taskCompletion.Task);

 Messenger.Default.Register<PropertyChangedMessage<bool>>(
 this,
 message =>
 
 if (message.PropertyName == nameof(MainVM.DataIsLoaded))
 
 testTrigger.Set();
 
 );
 MainVM vm = new MainVM(repoMock.Object);

 testTrigger.WaitOne(5000);

 Assert.IsTrue(vm.DataIsLoaded);
 

Now I don't have a race condition, but I'm a bit annoyed that I'm broadcasting a message that I don't really need for my code to work. I could wrap the whole thing in an pre-processor directive, of course, but that's just clunky.

Is there a better way to go about the whole thing?

Trying to load the data asynchronously in the constructor is a bad idea and in my opinion goes against what the constructor is for.

Constructors cannot be async, and asynchronous initialization can be seen as an implementation detail.

I suggest separating the initialization and data loading out ...

public class MainVM : ViewModelBase 
 private readonly IRepository<ToDoItem> repository;
 private IEnumerable<ToDoItem> items; 
 private bool _dataIsLoaded;
 private bool loading = false;

 public bool DataIsLoaded 
 get return _dataIsLoaded; 
 set Set(ref _dataIsLoaded, value, true); 
 

 public IEnumerable<ToDoItem> ToDoItems 
 get return items; 
 set Set(ref items, value); 
 

 public MainVM(IRepository<ToDoItem> repository) 
 this.repository = repository;
 

 public async Task LoadData() 
 if (loading) return;

 if (!DataIsLoaded) 
 try 
 loading = true;
 ToDoItems = await repository.GetAsync();
 loading = false;
 DataIsLoaded = true; // task Ran To Completion
 catch (Exception ex) 
 loading = false;
 //TODO: Log error
 DataIsLoaded = false; // task did not complete
 
 
 

... and have whatever is binding/using to the view model (ie the view, another model, a test, etc) load the data after initialization.

This allows the testing to be much simpler and to the point

For example

[TestClass]
public class MainVmTests 
 [TestMethod]
 public async Task DataIsLoaded_is_true_if_loading_task_ran_to_completion() 
 //Arrange
 var repoMock = new Mock<IRepository<ToDoItem>>();
 repoMock.Setup(_ => _.GetAsync()).Returns(Task.FromResult(Enumerable.Empty<ToDoItem>()));
 var subject = new MainVM(repoMock.Object);

 //Act
 await subject.LoadData();

 //Assert
 Assert.IsTrue(subject.DataIsLoaded);
 Assert.IsNotNull(subject.ToDoItems);
 

 [TestMethod]
 public async Task DataIsLoaded_is_false_if_loading_task_failed() 
 //Arrange
 var repoMock = new Mock<IRepository<ToDoItem>>();
 repoMock.Setup(_ => _.GetAsync()).Throws(new AggregateException());
 var subject = new MainVM(repoMock.Object);

 //Act
 await subject.LoadData();

 //Assert
 Assert.IsFalse(subject.DataIsLoaded);
 Assert.IsNull(subject.ToDoItems);