Merge pull request #1121 from stakx/verify-conditional-setups

Allow marking conditional setups as `.Verifiable()` once again

CHANGELOG.md

@@ -7,10 +7,17 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 ## Unreleased
 
+#### Changed
+
+* Attempts to mark conditionals setup as verifiable are once again allowed; it turns out that forbidding it (as was done in #997 for version 4.14.0) is in fact a regression. (@stakx, #1121)
+
 #### Fixed
 
 * Performance regression: Adding setups to a mock becomes slower with each setup (@CeesKaas, #1110)
 
+* Regression: `mock.Verify[All]` no longer marks invocations as verified if they were matched by conditional setups. (@Lyra2108, #1114)
+
+
 ## 4.15.2 (2020-11-26)
 
 #### Changed

src/Moq/Mock.cs

@@ -258,7 +258,7 @@ public DefaultValue DefaultValue
 		/// </example>
 		public void Verify()
 		{
-			this.Verify(setup => !setup.IsOverridden && !setup.IsConditional && setup.IsVerifiable);
+			this.Verify(setup => setup.IsVerifiable);
 		}
 
 		/// <summary>
@@ -283,7 +283,7 @@ public void Verify()
 		/// </example>
 		public void VerifyAll()
 		{
-			this.Verify(setup => !setup.IsOverridden && !setup.IsConditional);
+			this.Verify(setup => true);
 		}
 
 		private void Verify(Func<ISetup, bool> predicate)
@@ -313,9 +313,9 @@ internal bool TryVerify(Func<ISetup, bool> predicate, HashSet<Mock> verifiedMock
 
 			var errors = new List<MockException>();
 
-			foreach (var setup in this.MutableSetups.ToArray(predicate))
+			foreach (var setup in this.MutableSetups.ToArray(setup => !setup.IsOverridden && !setup.IsConditional && predicate(setup)))
 			{
-				if (predicate(setup) && !setup.TryVerify(recursive: true, predicate, verifiedMocks, out var e) && e.IsVerificationError)
+				if (!setup.TryVerify(recursive: true, predicate, verifiedMocks, out var e) && e.IsVerificationError)
 				{
 					errors.Add(e);
 				}

src/Moq/Properties/Resources.resx

@@ -351,7 +351,4 @@ This could be caused by an unrecognized type conversion, coercion, narrowing, or
   <data name="UseItIsOtherOverload" xml:space="preserve">
     <value>It is impossible to call the provided strongly-typed predicate due to the use of a type matcher. Provide a weakly-typed predicate with two parameters (object, Type) instead.</value>
   </data>
-  <data name="ConditionalSetupsAreNotVerifiable" xml:space="preserve">
-    <value>This call to 'Verifiable' will have no effect because conditional setups are ignored by both 'Verify' and 'VerifyAll'. This might indicate an error in your code.</value>
-  </data>
-</root>
+</root>

src/Moq/Setup.cs

@@ -8,8 +8,6 @@
 using System.Reflection;
 using System.Text;
 
-using Moq.Properties;
-
 namespace Moq
 {
 	internal abstract class Setup : ISetup
@@ -89,11 +87,6 @@ public void MarkAsOverridden()
 
 		public void MarkAsVerifiable()
 		{
-			if (this.IsConditional)
-			{
-				throw new InvalidOperationException(Resources.ConditionalSetupsAreNotVerifiable);
-			}
-
 			this.flags |= Flags.Verifiable;
 		}
 
@@ -180,12 +173,12 @@ protected virtual void ResetCore()
 
 		public void Verify(bool recursive = true)
 		{
-			this.Verify(recursive, setup => !setup.IsOverridden && !setup.IsConditional && setup.IsVerifiable);
+			this.Verify(recursive, setup => setup.IsVerifiable);
 		}
 
 		public void VerifyAll()
 		{
-			this.Verify(recursive: true, setup => !setup.IsOverridden && !setup.IsConditional);
+			this.Verify(recursive: true, setup => true);
 		}
 
 		private void Verify(bool recursive, Func<ISetup, bool> predicate)

tests/Moq.Tests/Regressions/IssueReportsFixture.cs

@@ -3611,6 +3611,55 @@ public interface IClassA
 
 		#endregion
 
+		#region 1114
+
+		public class Issue1114
+		{
+			[Fact]
+			public void TestMockSequence()
+			{
+				var myMock = new Mock<MyClass>();
+				var myTestObject = new MyTestObject(myMock.Object);
+				var sequence = new MockSequence { Cyclic = false };
+				myMock.InSequence(sequence).Setup(x => x.FirstCall()).Verifiable();
+				myMock.InSequence(sequence).Setup(x => x.SecondCall()).Verifiable();
+
+				myTestObject.TestMe();
+
+				myMock.Verify();
+				myMock.VerifyNoOtherCalls();
+			}
+
+			public class MyTestObject
+			{
+				private readonly MyClass _myMockObject;
+
+				public MyTestObject(MyClass myMockObject)
+				{
+					_myMockObject = myMockObject;
+				}
+
+				public void TestMe()
+				{
+					_myMockObject.FirstCall();
+					_myMockObject.SecondCall();
+				}
+			}
+
+			public class MyClass
+			{
+				public virtual void FirstCall()
+				{
+				}
+
+				public virtual void SecondCall()
+				{
+				}
+			}
+		}
+
+		#endregion
+
 		// Old @ Google Code
 
 		#region #47

tests/Moq.Tests/VerifyFixture.cs

@@ -1047,16 +1047,17 @@ public void CanVerifyMethodThatIsNamedLikeEventRemoveAccessor()
 		}
 
 		[Fact]
-		public void Verify_ignores_conditional_setups_so_calling_Verifiable_is_a_user_error()
+		public void Verify_ignores_conditional_setups()
 		{
 			var mock = new Mock<IFoo>();
-			var setup = mock.When(() => true).Setup(m => m.Submit());
+			mock.When(() => true).Setup(m => m.Submit()).Verifiable();
 
-			// Conditional setups are completely ignored by `Verify[All]`.
-			// Making such a setup verifiable is therefore a no-op, but
-			// may be indicative of the user making an incorrect assumption;
-			// best to warn the user so they revise their code:
-			Assert.Throws<InvalidOperationException>(() => setup.Verifiable());
+			var exception = Record.Exception(() =>
+			{
+				mock.Verify();
+			});
+
+			Assert.Null(exception);
 		}
 
 		[Fact]
@@ -1502,6 +1503,16 @@ public void Verification_marks_invocations_of_inner_mocks_as_verified()
 			Mock.Get(mock.Object.Bar).VerifyNoOtherCalls();
 		}
 
+		[Fact]
+		public void Verify__marks_invocations_as_verified__even_if_the_setups_they_were_matched_by_were_conditional()
+		{
+			var mock = new Mock<IFoo>();
+			mock.When(() => true).Setup(m => m.Submit()).Verifiable();
+			mock.Object.Submit();
+			mock.Verify();
+			mock.VerifyNoOtherCalls();
+		}
+
 		public class Exclusion_of_unreachable_inner_mocks
 		{
 			[Fact]