diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f935cab6..00818f5a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1 #### Changed -* Event accessor calls (`+=` and `-=`) now get consistently recorded in `Mock.Invocations`. This previously wasn't the case for backwards compatibility with `VerifyNoOtherCalls` (which got implemented before it was possible to check them using `Verify{Add,Remove}`). You now need to explicitly verify expected calls to event accessors prior to `VerifyNoOtherCalls`. Verification of `+=` and `-=` now works regardless of whether or not you set those up (which makes it consistent with how verification usually works). (@80O, @stakx, #1058) +* Event accessor calls (`+=` and `-=`) now get consistently recorded in `Mock.Invocations`. This previously wasn't the case for backwards compatibility with `VerifyNoOtherCalls` (which got implemented before it was possible to check them using `Verify{Add,Remove}`). You now need to explicitly verify expected calls to event accessors prior to `VerifyNoOtherCalls`. Verification of `+=` and `-=` now works regardless of whether or not you set those up (which makes it consistent with how verification usually works). (@80O, @stakx, #1058, #1084) #### Fixed diff --git a/src/Moq/Interception/InterceptionAspects.cs b/src/Moq/Interception/InterceptionAspects.cs index 35f618ed0..114c2b3dd 100644 --- a/src/Moq/Interception/InterceptionAspects.cs +++ b/src/Moq/Interception/InterceptionAspects.cs @@ -132,21 +132,16 @@ public static bool Handle(Invocation invocation, Mock mock) var @event = implementingMethod.DeclaringType.GetEvents(bindingFlags).SingleOrDefault(e => e.GetAddMethod(true) == implementingMethod); if (@event != null) { - bool doesntHaveEventSetup = !mock.MutableSetups.HasEventSetup; - if (mock.CallBase && !invocation.Method.IsAbstract) { - if (doesntHaveEventSetup) - { - invocation.ReturnValue = invocation.CallBase(); - } + invocation.ReturnValue = invocation.CallBase(); + return true; } else if (invocation.Arguments.Length > 0 && invocation.Arguments[0] is Delegate delegateInstance) { mock.EventHandlers.Add(@event, delegateInstance); + return true; } - - return doesntHaveEventSetup; } } else if (methodName[0] == 'r' && methodName.Length > 7 && methodName[6] == '_' && invocation.Method.IsEventRemoveAccessor()) @@ -155,21 +150,16 @@ public static bool Handle(Invocation invocation, Mock mock) var @event = implementingMethod.DeclaringType.GetEvents(bindingFlags).SingleOrDefault(e => e.GetRemoveMethod(true) == implementingMethod); if (@event != null) { - bool doesntHaveEventSetup = !mock.MutableSetups.HasEventSetup; - if (mock.CallBase && !invocation.Method.IsAbstract) { - if (doesntHaveEventSetup) - { - invocation.ReturnValue = invocation.CallBase(); - } + invocation.ReturnValue = invocation.CallBase(); + return true; } else if (invocation.Arguments.Length > 0 && invocation.Arguments[0] is Delegate delegateInstance) { mock.EventHandlers.Remove(@event, delegateInstance); + return true; } - - return doesntHaveEventSetup; } } } diff --git a/src/Moq/Interception/Mock.cs b/src/Moq/Interception/Mock.cs index 60d3f1afc..20baedbde 100644 --- a/src/Moq/Interception/Mock.cs +++ b/src/Moq/Interception/Mock.cs @@ -14,11 +14,6 @@ void IInterceptor.Intercept(Invocation invocation) RecordInvocation.Handle(invocation, this); - if (HandleEventSubscription.Handle(invocation, this)) - { - return; - } - if (FindAndExecuteMatchingSetup.Handle(invocation, this)) { return; @@ -29,6 +24,11 @@ void IInterceptor.Intercept(Invocation invocation) return; } + if (HandleEventSubscription.Handle(invocation, this)) + { + return; + } + FailForStrictMock.Handle(invocation, this); Return.Handle(invocation, this); diff --git a/src/Moq/MethodCall.cs b/src/Moq/MethodCall.cs index 9967c5f73..02209c4e4 100644 --- a/src/Moq/MethodCall.cs +++ b/src/Moq/MethodCall.cs @@ -105,6 +105,10 @@ protected override void ExecuteCore(Invocation invocation) new ReturnBaseOrDefaultValue(this.Mock).Execute(invocation); } } + else + { + HandleEventSubscription.Handle(invocation, this.Mock); // no-op for everything other than event accessors + } this.afterReturnCallback?.Execute(invocation); } diff --git a/src/Moq/SetupCollection.cs b/src/Moq/SetupCollection.cs index a022e476a..c80dac9aa 100644 --- a/src/Moq/SetupCollection.cs +++ b/src/Moq/SetupCollection.cs @@ -11,12 +11,10 @@ namespace Moq internal sealed class SetupCollection : ISetupList { private List setups; - private volatile bool hasEventSetup; public SetupCollection() { this.setups = new List(); - this.hasEventSetup = false; } public int Count @@ -30,14 +28,6 @@ public int Count } } - public bool HasEventSetup - { - get - { - return this.hasEventSetup; - } - } - public ISetup this[int index] { get @@ -53,11 +43,6 @@ public void Add(Setup setup) { lock (this.setups) { - if (setup.Method.IsEventAddAccessor() || setup.Method.IsEventRemoveAccessor()) - { - this.hasEventSetup = true; - } - this.setups.Add(setup); this.MarkOverriddenSetups(); @@ -116,7 +101,6 @@ public void Clear() lock (this.setups) { this.setups.Clear(); - this.hasEventSetup = false; } } diff --git a/tests/Moq.Tests/EventHandlersFixture.cs b/tests/Moq.Tests/EventHandlersFixture.cs index e79526986..8a591c4fd 100644 --- a/tests/Moq.Tests/EventHandlersFixture.cs +++ b/tests/Moq.Tests/EventHandlersFixture.cs @@ -82,15 +82,15 @@ public void Raising_event__using_Raise__triggers_handler__if_setup_without_CallB Assert.True(handled); } - [Fact] // this is inconsistent with the above test `Raising_event__using_Raise__does_not_trigger_handler__if_CallBase_true` - public void Raising_event__using_Raise__triggers_handler__if_setup_with_CallBase_present() + [Fact] + public void Raising_event__using_Raise__does_not_trigger_handler__if_setup_with_CallBase_present() { var handled = false; var mock = new Mock(); mock.SetupAdd(m => m.Event += It.IsAny()).CallBase(); mock.Object.Event += () => handled = true; mock.Raise(m => m.Event += null); - Assert.True(handled); + Assert.False(handled); } [Fact] diff --git a/tests/Moq.Tests/MockFixture.cs b/tests/Moq.Tests/MockFixture.cs index 5793b7f25..6c7fdb152 100644 --- a/tests/Moq.Tests/MockFixture.cs +++ b/tests/Moq.Tests/MockFixture.cs @@ -1279,20 +1279,6 @@ public void Reset_clears_configured_default_return_values() Assert.Empty(mock.ConfiguredDefaultValues); } - [Fact] - public void Reset_clears_event_setup_flag() - { - var mock = new Mock(); - mock.SetupAdd(m => m.EventHandler += It.IsAny()); - - var before = mock.MutableSetups.HasEventSetup; - mock.Reset(); - var after = mock.MutableSetups.HasEventSetup; - - Assert.True(before, "Before reset"); - Assert.False(after, "After reset"); - } - #if FEATURE_DYNAMICPROXY_SERIALIZABLE_PROXIES [Serializable] public class BadSerializable : ISerializable