2

I have a simple class that implements INotifyPropertyChanged, I invoke the property change on another thread, and I had a pretty hard time getting FluentAsserts to see that the propertyChanged was invoked. It does not seem to happen if I use a Task.Delay in an async Task method. But it does if I just sleep the thread.

The SimpleNotify class:

namespace FluentAssertPropertyThreads
{
    class SimpleNotify : System.ComponentModel.INotifyPropertyChanged
    {
        public event System.ComponentModel.PropertyChangedEventHandler PropertyChanged;
        private void onChange(string name)
        {
            this.PropertyChanged?.Invoke(this, new System.ComponentModel.PropertyChangedEventArgs(name));
        }

        private int count = 0;
        public int Count
        {
            get
            {
                return this.count;
            }

            set
            {
                if (this.count != value)
                {
                    this.count = value;
                    this.onChange(nameof(this.Count));
                }
            }
        }
    }
}

and here are my unit tests:

using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.Threading;
using System.Threading.Tasks;

namespace FluentAssertPropertyThreads
{
    [TestClass]
    public class UnitTest1
    {
        private SimpleNotify simpleNotify;
        private int notifyCount;
        private void bumpCount()
        {
            this.simpleNotify.Count++;
        }

        private void SimpleNotify_PropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
        {
            SimpleNotify simpleNotify = sender as SimpleNotify;
            if (simpleNotify == null)
            {
                throw new ArgumentNullException(nameof(sender), "sender should be " + nameof(SimpleNotify));
            }

            if (e.PropertyName.Equals(nameof(SimpleNotify.Count), StringComparison.InvariantCultureIgnoreCase))
            {
                this.notifyCount++;
            }
        }

        [TestInitialize]
        public void TestSetup()
        {
            this.notifyCount = 0;
            this.simpleNotify = new SimpleNotify();
            this.simpleNotify.PropertyChanged += SimpleNotify_PropertyChanged;
            this.simpleNotify.MonitorEvents();

            Thread thread = new Thread(this.bumpCount)
            {
                IsBackground = true,
                Name = @"My Background Thread",
                Priority = ThreadPriority.Normal
            };
            thread.Start();
        }

        [TestMethod]
        public async Task TestMethod1()
        {
            await Task.Delay(100);
            this.notifyCount.Should().Be(1);        //this passes, so I know that my notification has be executed.
            this.simpleNotify.ShouldRaisePropertyChangeFor(x => x.Count);       //but this fails, saying that I need to be monitoring the events (which I am above)
        }

        [TestMethod]
        public void TestMethod2()
        {
            Thread.Sleep(100);
            this.notifyCount.Should().Be(1);        //this passes, so I know that my notification has be executed.
            this.simpleNotify.ShouldRaisePropertyChangeFor(x => x.Count);       //this passes as I expected
        }
    }
}

The exact error is:

System.InvalidOperationException: Object is not being monitored for events or has already been garbage collected. Use the MonitorEvents() extension method to start monitoring events.

I don't see how MonitorEvents would care if I use await or Thread.Sleep. What am I missing? I get that await leaves the method and comes back in, whereas Thread.Sleep does not.

So when it leaves the TestMethod1 during the await, it is hitting a dispose on an object that FluentAsserts is using to track the properties? Could it? Should it?

VMAtm
  • 27,943
  • 17
  • 79
  • 125
Jason Bemis
  • 161
  • 1
  • 11

2 Answers2

2

Yes, the things is like you said: await pauses the execution of current method and create a state machine to get back to the method after the Delay is done. But the caller (a UnitTest engine) doesn't expect your tests to be asynchronous and simply ends the execution, which leads to the disposal of the objects.

Stephen Cleary wrote a brilliant MSDN article about Unit testing and async/await keywords, you probably should move your code out to the method returning the Task and wait for all of it in test, something like this:

async Task Testing()
{
    await Task.Delay(100);
    this.notifyCount.Should().Be(1);
    this.simpleNotify.ShouldRaisePropertyChangeFor(x => x.Count);
}

[TestMethod]
public async Task TestMethod1()
{
    await Testing();
}

but this still may fail as the logic after await may execute after the disposable being disposed.

VMAtm
  • 27,943
  • 17
  • 79
  • 125
2

Looks like version 5.0.0 will include support for Async tests with monitoring.

This Issue was raised and the work completed just waiting on the documentation to be updated and version 5.0.0 to be released.

But for the time being there is a prerelease with the code changes 5.0.0-beta2 can get it from the prerelease versions on NuGet

From the change log:

{Breaking} Replaced the old thread-unsafe MonitorEvents API with a new Monitor extension method that will return a thread-safe monitoring scope that exposes methods like Should().Raise() and metadata such as OccurredEvents and MonitoredEvents

So the code with the updated NuGet will look like this:

[TestInitialize]
public void TestSetup()
{
    this.notifyCount = 0;
    this.simpleNotify = new SimpleNotify();
    this.simpleNotify.PropertyChanged += SimpleNotify_PropertyChanged;

    Thread thread = new Thread(this.bumpCount)
    {
        IsBackground = true,
        Name = @"My Background Thread",
        Priority = ThreadPriority.Normal
    };
    thread.Start();
}

[TestMethod]
public async Task TestMethod1()
{
    using (var MonitoredSimpleNotify = this.simpleNotify.Monitor())
    {
        await Task.Delay(100);
        this.notifyCount.Should().Be(1);
        MonitoredSimpleNotify.Should().RaisePropertyChangeFor(x => x.Count); // New API for Monitoring
    }
}
Iain Smith
  • 9,230
  • 4
  • 50
  • 61