Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mock<T>.Raise only raises events on root object #166

Closed
hallipr opened this issue Apr 21, 2015 · 11 comments
Closed

Mock<T>.Raise only raises events on root object #166

hallipr opened this issue Apr 21, 2015 · 11 comments

Comments

@hallipr
Copy link

hallipr commented Apr 21, 2015

https://github.com/Moq/moq4/wiki/Quickstart#events
Example 2, "descendant events", doesn't function as expected

Events on the root object are raised when trying to raise events on the child.

This is noticed in 1/17/2013
http://stackoverflow.com/questions/14386540/mocked-inner-objects-events-will-not-fire

As tested in LINQPad:

void Main()
{
    var mock = new Mock<IFoo> { DefaultValue = DefaultValue.Mock };
    mock.Object.Event += (s,e) => "FIRED FOO".Dump();
    mock.Object.Bar.Event += (s,e) => "FIRED BAR".Dump();

    mock.Raise(x => x.Event += null, EventArgs.Empty);
    mock.Raise(x => x.Bar.Event += null, EventArgs.Empty);
}

public interface IFoo
{
    IBar Bar { get; }
    event EventHandler Event;
}

public interface IBar
{
    event EventHandler Event;
}

Expected output:

FIRED FOO
FIRED BAR

Actual output:

FIRED FOO
FIRED FOO

All events named "Event" raise IFoo.Event.

mock.Raise(x => x.Bar.Child.Another.Event += null, EventArgs.Empty);

Returns:

FIRED FOO
@BrunoJuchli
Copy link

This issue also exists when the root mock and the child mock don't have events with the same name.
'Raise' doesn't throw an exception but no event is raised.

Workaround

Retrieve the child mock from the root mock and raise the event on the child mock:

void Main()
{
    var mock = new Mock<IFoo> { DefaultValue = DefaultValue.Mock };
    mock.Object.Event += (s,e) => "FIRED FOO".Dump();
    mock.Object.Bar.Event += (s,e) => "FIRED BAR".Dump();

    mock.Raise(x => x.Event += null, EventArgs.Empty);
            Mock.Get(mock.Object.Bar).Raise(x => x.Event += null, EventArgs.Empty);
}

This works and leads to the expected results.
Edit: i see now that this workaround was also posted as an answer to this SO question mentioned by @hallipr

Issue Analysis

Upon a first look into the implementation, it seems that the EventInfo is correctly retrieved (if it wouldn't be, it should throw an exception). It fails at Mock.DoRaise(EventInfo ev, EventArgs args), more specifically retrieving the list of event subscription delegates returns no delegates: this.Interceptor.InterceptionContext.GetInvocationList(ev). I believe that this is because DoRaise is executed on the root mock instead of the child mock.

@stakx
Copy link
Contributor

stakx commented Jun 28, 2017

@BrunoJuchli: Thank you for the analysis, this is exactly the reason why events on child mocks don't get raised.

I had already prepared a fix for this when I discovered this test from 2009, which is now failing:

[Fact]
public void DoesNotRaiseEventOnSubObject()
{
    var mock = new Mock<IParent> { DefaultValue = DefaultValue.Mock };
    bool raised = false;
    mock.Object.Adder.Added += (sender, args) => raised = true;
    Assert.Same(mock.Object.Adder, mock.Object.Adder);
    mock.Raise(p => p.Adder.Added += null, EventArgs.Empty);
    Assert.False(raised);
}

According to this, not being able to raise events on child mocks is expected behavior. As mentioned by @hallipr, the quickstart guide (which was started in late 2013) says differently.

Something needs to be fixed here:

  1. If raising an event on a child mock really shouldn't be allowed, then any attempt to do so must not fail silently, nor raise the wrong event on the wrong mock. Instead, an exception (perhaps NotSupportedException or InvalidOperationException) should be thrown.

  2. We allow this scenario, apply the bugfix, and replace this unit test. This is a functional change and could potentially break client code, though I can't quite imagine why anyone would try to raise an event, knowing it won't work, and then rely on the unintuitive effect this feture has had until now.

I'd say we go ahead with (2).

@BrunoJuchli
Copy link

@stakx
I agree that it is very unlikely to break client code. I'd say if it's going to break client code, then it's probably not because the user was relying on this "feature" but rather because the test or implementation is actually incorrect. That, however, would be the case for both fixes, 1 and 2.
So no matter how this is fixed, for those wo "relied" on this "feature" it will make itself known by not working as before.

Also, I think there's some cases where supporting to raise events on child mocks would be helpful - i experienced it at least once. Therefore I'd, too, vote for number 2.

@stakx
Copy link
Contributor

stakx commented Jun 28, 2017

@BrunoJuchli, you're right on all accounts, I didn't phrase the breakage scenario very well. Glad to hear you agree on (2). I've just now merged the required changes, so this will be resolved in Moq >4.7.63.

@stakx stakx closed this as completed Jul 2, 2017
@yordans
Copy link

yordans commented Jan 30, 2019

This behavior is still present in 4.10.0. Is it going to be fixed?

@BrunoJuchli
Copy link

BrunoJuchli commented Jan 30, 2019

@yordans are you sure? You've just reported #745, which, to me, seems an entirely different case.

If you believe this issue here still is present please provide the code to reproduce the issue.

@miguelDeDiego
Copy link

I've found an extrange behavior on firing events with Moq.
If I declare the handler for an event with Action instead of EventHandler, the Raise method always throws an exception when the only argument is EventArgs, that is, with Action as the event's delegate throws the exception: System.Reflection.TargetParameterCountException: Parameter count mismatch
I have this code:

public interface IThermostat {
    event EventHandler TooHot;
    //event Action<EventArgs> TooHot;
    event Action LoadedNoParams;
    event Action<EventArgs> LoadedOneParam;
    event Action<HeaterEventArgs> LoadedHeaterEventArgsParam;
    event Action<DateTime> LoadedDateTimeParam;
    event Action<string> LoadedStringParam;
    event Action<object, EventArgs> LoadedTwoParams;

    void StartAsyncSelfCheck();
}

public class HeaterEventArgs : EventArgs {
    public string Message { get; set; }
}

public class ThermostatTemperatureTests {
    [Fact]
    public void TheHeaterIsSwitchedOffWhenTheHighTemperatureIsReached() {
        // Arrange
        var mockThermostat = new Mock<IThermostat>();
        var controller = new HeatingController( mockThermostat.Object);

        // Act
        mockThermostat.Raise(m => m.TooHot += null, new HeaterEventArgs() { Message = "Alfa" });
        mockThermostat.Raise(m => m.TooHot += null, this, new HeaterEventArgs() { Message = "Beta" });

        mockThermostat.Raise(m => m.LoadedNoParams += null);
        mockThermostat.Raise(m => m.LoadedStringParam += null, "LoadedOneParam");
        mockThermostat.Raise(m => m.LoadedOneParam += null, new HeaterEventArgs() { Message = "Error" });
        mockThermostat.Raise(m => m.LoadedTwoParams += null, this, new HeaterEventArgs() { Message = "LoadedTwoParams" });

        mockThermostat.Raise(m => m.LoadedHeaterEventArgsParam += null, new HeaterEventArgs() { Message = "Error" });
        mockThermostat.Raise(m => m.LoadedDateTimeParam += null, DateTime.Now);

        // Assert
        mockThermostat.Verify(m => m.StartAsyncSelfCheck());
    }
}

When mockThermostat.Raise(...) fires the event all is correct except the cases with event args as the only parameter, it always throws the previous mentioned exception:
mockThermostat.Raise(m => m.LoadedOneParam += null, EventArgs.Empty);
mockThermostat.Raise(m => m.LoadedOneParam += null, new HeaterEventArgs() { Message = "Error" });
mockThermostat.Raise(m => m.LoadedHeaterEventArgsParam += null, new HeaterEventArgs() { Message = "Error" });

Am I missing any thing?

@yordans
Copy link

yordans commented Feb 4, 2019

@BrunoJuchli Here is the code for reproducing the behavior:

public interface IEventOwner
{
	event EventHandler OwnerEvent;

	bool OwnerBool { get; set; }
}

public interface IFoo
{
	IEventOwner EventOwner { get; }

	bool FooBoolean { get; set; }
}

[Test]
public void TestEvent()
{
	var eventOwnerMock = new Mock<IEventOwner>();
	eventOwnerMock.SetupSet(owner => owner.OwnerBool = It.IsAny<bool>()).Raises(owner => owner.OwnerEvent += null, EventArgs.Empty);

	var fooMock = new Mock<IFoo>();
	fooMock.Setup(f => f.EventOwner).Returns(eventOwnerMock.Object);
	fooMock.SetupSet(f => f.FooBoolean = It.IsAny<bool>()).Raises(f => f.EventOwner.OwnerEvent += null, EventArgs.Empty);

	bool isEventRaised = false;
	fooMock.Object.EventOwner.OwnerEvent += (s, e) => isEventRaised = true;

	//eventOwnerMock.Object.OwnerBool = true;
	fooMock.Object.FooBoolean = true;

	Assert.IsTrue(isEventRaised);
}

In case the OwnerBool is changed (the commented line), the event is fired as expected. But when the FooBoolean is changed the event on IEventOwner is not fired as expected (I think that it is being searched for in IFoo...).

@stakx
Copy link
Contributor

stakx commented Feb 4, 2019

@yordans - I haven't digged very deeply yet, however it appears that the problem was fixed only for mock.Raise, but not for setup.Raises.

The following lines of code look relevant. Notice how the "target" (i.e. the object on which the event sits) simply get discarded:

https://github.com/moq/moq4/blob/e5197829d13567c7984362829db625da6662a32d/src/Moq/MethodCall.cs#L191-L194

At this point I am not yet sure whether and how to fix this (if a fix is feasible at all).

For now, I can only advise not to get too carried away with the possibilities Moq appears to offer here. Remember, under normal circumstances, you wouldn't be able either to just trigger an event from the outside. (I am not so sure that it was wise to add that ability to Moq, as it might steer people to less-than-ideal code patterns.)

@stakx
Copy link
Contributor

stakx commented Mar 7, 2019

@yordans - I have closed this issue because your test case is actually caused by a different problem. You manually set up the fooMock to return an sub-mock for .EventOwner. When the time comes to raise .EventOwner.OwnerEvent, Moq isn't smart enough to figure out that it could transition from fooMock to eventOwnerMock where the event sits. The reason is that a setup can be configured to return anything lazily, and Moq tries not to actually execute the setup to determine the return value since that could cause side effects. Instead, it takes the safer route and only transitions to an inner mock if it knows, without executing the setup, what that inner mock is. That's currently only the case when you let Moq set up the inner mock itself, using a fluent / recursive / multi-dot Setup expression.

So what you can do is this:

-var fooMock = new Mock<IFoo>();
+var fooMock = new Mock<IFoo>() { DefaultValue = DefaultValue.Mock };
-fooMock.Setup(f => f.EventOwner).Returns(eventOwnerMock.Object);
 fooMock.SetupSet(f => f.FooBoolean = It.IsAny<bool>()).Raises(f => f.EventOwner.OwnerEvent += null, EventArgs.Empty);

 bool isEventRaised = false;
 fooMock.Object.EventOwner.OwnerEvent += (s, e) => isEventRaised = true;

The last line shown above, together with DefaultValue.Mock, is the one that will cause Moq to create the inner mock for EventOwner automatically. When raising the event, it is then able transition from the root mock to the inner mock.

By the way, this limitation is likely going to be improved soonish.

@stakx stakx added this to the 4.11.0 milestone Apr 19, 2019
@stakx
Copy link
Contributor

stakx commented Apr 19, 2019

Resolved in 4.11.0-rc1.

@devlooped devlooped locked and limited conversation to collaborators Sep 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants