Regression for event subscription between 4.10 and 4.11

[chrisaut]

The following seems to be a regression between 4.10 and 4.11 (4.12 behaves like 4.11):

    public class C
    {
        public bool Raised { get; private set; }
        public virtual event Action E;
        public C()
        {
            E += () => Raised = true;
        }
    }
    public class UnitTest1
    {
        [Fact]
        public void Test1()
        {
            var c = new Moq.Mock<C>();
            c.Raise(x => x.E += () => { });
            Assert.True(c.Object.Raised);
        }
    }

Additional information, if the event subscription happens outside the mocked object, it still behaves as expected:

    public class C
    {
        public virtual event Action E;
    }
    public class UnitTest1
    {
        [Fact]
        public void Test1()
        {
            var raised = false;
            var c = new Moq.Mock<C>();            
             c.Object.E += () => raised = true; // this still does

            c.Raise(x => x.E += () => { });            

            Assert.True(raised);
        }
    }

or if it happens in any of the object's methods:

    public class C
    {
        public bool Raised { get; private set; }
        public virtual event Action E;
        public void M()
        {
            E += () => Raised = true;
        }
    }
    public class UnitTest1
    {
        [Fact]
        public void Test1()
        {
            var c = new Moq.Mock<C>();
            c.Object.M();
            c.Raise(x => x.E += () => { });            

            Assert.True(c.Object.Raised);
        }
    }

The subscription to the event "E" seems to no longer take effect if the event is being subscripted to in the mocked object's ctor.
Is this expected? I was unable to find an issue regarding this change.

[stakx]

@chrisaut: Thank you for reporting this. I'll add (a slightly modified version of) your tests to the regresstion test suite.

Is this expected?

It's somewhat an edge case, but yes, I believe this is to be expected. You are raising the event before the mocked object is created:

var c = new Moq.Mock<C>();
c.Raise(x => x.E += () => { });
Assert.True(c.Object.Raised);

Note how, in your code, c.Object is called after c.Raise(…). That is, the mocked object hasn't yet been created, therefore the constructor hasn't run and no event handler has been subscribed.

If you insert a _ = c.Object; before c.Raise(…);, your test will succeed.

While .Raise() could ensure that the mocked object is instantiated, I am not (yet) convinced that this would make sense in all cases. In general, I think it better not to cause side effects behind the client code's back.

You might wonder why this worked in previous versions of Moq. This was in fact a (most probably unintended) side effect. When c.Raise() is given a delegate x => x.E += …, it has to first reconstruct a LINQ expression tree from that delegate which it can then analyse to figure out what event on which mock it is referring to.

• Previous versions of Moq achieved this reconstruction by invoking the delegate on the mock object, then observing the side effects. In order to run the delegate on the mock object, Moq had to retrieve it via c.Object (without you knowing). That's precisely the kind of hidden side effects I referred to above. (Unfortunately, your first unit test relies on that hidden side effect.)

• Nowadays, Moq takes more care and performs the reconstruction by invoking the delegate on a different, completely separate recorder-like mock object. It does so because LINQ expression tree reconstruction shouldn't cause state changes (side effects) in your mock.

[chrisaut]

@stakx thnx for the explanation, that makes perfect sense, and is easy to fixup in the couple of tests I ran into it while upgrading.

However, I find the behavior a bit unexpected, I would expect a call to Raise to operate against a fully initialized object.

Since its easy to workaround, kind of an edge case to add an event subscriber inside the ctor, and you indicated you probably don't want to change this behavior, I'm closing this issue.

thnx for the help.

[stakx]

However, I find the behavior a bit unexpected, I would expect a call to Raise to operate against a fully initialized object.

Yes, what you're saying seems sensible. What I'm still trying to wrap my head around is the fact that most Moq API calls work perfectly fine without an actual mocked object—say, the .Setup*() and .Verify*() method groups—but .Raise() just may to be an exception despite the fact that it also sits on Mock<T>, and not on the mocked T.

I'll leave my PR open and reflect on this some more, perhaps Raise really should ensure that the mocked object has been instantiated. But for now, you'll be on the safe side by ensuring that in your own code.