Merge pull request #854 from stakx/issue-733

Let `mock.Invocations.Clear()` remove traces of earlier invocations more thoroughly

Author: stakx

CHANGELOG.md

@@ -11,6 +11,8 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 * Improved error message that is supplied with `ArgumentException` thrown when `Setup` or `Verify` are called on a protected method if the method could not be found with both the name and compatible argument types specified (@thomasfdm, #852).
 
+* `mock.Invocations.Clear()` now removes traces of previous invocations more thoroughly by additionally resetting all setups to an "unmatched" state. (@stakx, #854)
+
 * Consistent `Callback` delegate validation regardless of whether or not `Callback` is preceded by a `Returns`: Validation for post-`Returns` callback delegates used to be very relaxed, but is now equally strict as in the pre-`Returns` case.) (@stakx, #876)
 
 #### Added
@@ -19,7 +21,10 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 #### Fixed
 
+* `Invocations.Clear()` does not cause `Verify` to fail (@jchessir, #733)
+
 * Regression: `SetupAllProperties` can no longer set up properties whose names start with `Item`. (@mattzink, #870; @kaan-kaya, #869)
+
 * Regression: `MockDefaultValueProvider` will no longer attempt to set `CallBase` to true for mocks generated for delegates. (@dammejed, #874)
 
 

src/Moq/InvocationCollection.cs

@@ -4,7 +4,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-using System.Linq;
+using System.Diagnostics;
 
 namespace Moq
 {
@@ -16,6 +16,14 @@ internal sealed class InvocationCollection : IInvocationList
 		private int count = 0;
 
 		private readonly object invocationsLock = new object();
+		private readonly Mock owner;
+
+		public InvocationCollection(Mock owner)
+		{
+			Debug.Assert(owner != null);
+
+			this.owner = owner;
+		}
 
 		public int Count
 		{
@@ -68,6 +76,9 @@ public void Clear()
 				this.invocations = null;
 				this.count = 0;
 				this.capacity = 0;
+
+				this.owner.Setups.UninvokeAll();
+				// ^ TODO: Currently this could cause a deadlock as another lock will be taken inside this one!
 			}
 		}
 

src/Moq/MethodCall.cs

@@ -329,6 +329,12 @@ public override MockException TryVerifyAll()
 			return (this.flags & Flags.Invoked) == 0 ? MockException.UnmatchedSetup(this) : null;
 		}
 
+		public override void Uninvoke()
+		{
+			this.flags &= ~Flags.Invoked;
+			this.limitInvocationCountResponse?.Reset();
+		}
+
 		public void Verifiable()
 		{
 			this.flags |= Flags.Verifiable;
@@ -388,6 +394,11 @@ public LimitInvocationCountResponse(MethodCall setup, int maxCount)
 				this.count = 0;
 			}
 
+			public void Reset()
+			{
+				this.count = 0;
+			}
+
 			public void RespondTo(Invocation invocation)
 			{
 				++this.count;

src/Moq/Mock.Generic.cs

@@ -139,7 +139,7 @@ public Mock(MockBehavior behavior, params object[] args)
 			this.constructorArguments = args;
 			this.defaultValueProvider = DefaultValueProvider.Empty;
 			this.eventHandlers = new EventHandlerCollection();
-			this.invocations = new InvocationCollection();
+			this.invocations = new InvocationCollection(this);
 			this.name = CreateUniqueDefaultMockName();
 			this.setups = new SetupCollection();
 			this.switches = Switches.Default;

src/Moq/Setup.cs

@@ -109,5 +109,9 @@ public MockException TryVerifyInnerMock(Func<Mock, MockException> verify)
 
 			return null;
 		}
+
+		public virtual void Uninvoke()
+		{
+		}
 	}
 }

src/Moq/SetupCollection.cs

@@ -119,6 +119,17 @@ public IEnumerable<Setup> GetInnerMockSetups()
 			return this.ToArrayLive(setup => setup.ReturnsInnerMock(out _));
 		}
 
+		public void UninvokeAll()
+		{
+			lock (this.setups)
+			{
+				foreach (var setup in this.setups)
+				{
+					setup.Uninvoke();
+				}
+			}
+		}
+
 		public Setup[] ToArrayLive(Func<Setup, bool> predicate)
 		{
 			var matchingSetups = new Stack<Setup>();

tests/Moq.Tests/InvocationsFixture.cs

@@ -128,5 +128,32 @@ public void Invocations_for_object_methods_on_interface_proxy_record_return_valu
 			var invocation = mock.MutableInvocations.ToArray()[0];
 			Assert.Equal(42, invocation.ReturnValue);
 		}
+
+		[Fact]
+		public void Invocations_Clear_also_resets_setup_verification_state()
+		{
+			var mock = new Mock<IComparable>();
+			mock.Setup(m => m.CompareTo(default));
+			_ = mock.Object.CompareTo(default);
+			mock.VerifyAll();  // ensure setup has been matched
+
+			mock.Invocations.Clear();
+			var ex = Assert.Throws<MockException>(() => mock.VerifyAll());
+			Assert.Equal(MockExceptionReasons.UnmatchedSetup, ex.Reasons);
+		}
+
+		[Fact]
+		[Obsolete()]
+		public void Invocations_Clear_resets_count_kept_by_setup_AtMost()
+		{
+			var mock = new Mock<IComparable>();
+			mock.Setup(m => m.CompareTo(default)).Returns(0).AtMostOnce();
+			_ = mock.Object.CompareTo(default);
+
+			mock.Invocations.Clear();
+			_ = mock.Object.CompareTo(default);  // this second call should now count as the first
+			var ex = Assert.Throws<MockException>(() => mock.Object.CompareTo(default));
+			Assert.Equal(MockExceptionReasons.MoreThanOneCall, ex.Reasons);
+		}
 	}
 }