diff --git a/CHANGELOG.md b/CHANGELOG.md index f0dd3f393..5d54f77ea 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Moq/Mock.cs b/src/Moq/Mock.cs index c55126b3c..1727219b0 100644 --- a/src/Moq/Mock.cs +++ b/src/Moq/Mock.cs @@ -258,7 +258,7 @@ public DefaultValue DefaultValue /// public void Verify() { - this.Verify(setup => !setup.IsOverridden && !setup.IsConditional && setup.IsVerifiable); + this.Verify(setup => setup.IsVerifiable); } /// @@ -283,7 +283,7 @@ public void Verify() /// public void VerifyAll() { - this.Verify(setup => !setup.IsOverridden && !setup.IsConditional); + this.Verify(setup => true); } private void Verify(Func predicate) @@ -313,9 +313,9 @@ internal bool TryVerify(Func predicate, HashSet verifiedMock var errors = new List(); - 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); } diff --git a/src/Moq/Properties/Resources.Designer.cs b/src/Moq/Properties/Resources.Designer.cs index 626977b47..7fce6d353 100644 --- a/src/Moq/Properties/Resources.Designer.cs +++ b/src/Moq/Properties/Resources.Designer.cs @@ -114,15 +114,6 @@ internal static string CantSetReturnValueForVoid { } } - /// - /// Looks up a localized string similar to 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.. - /// - internal static string ConditionalSetupsAreNotVerifiable { - get { - return ResourceManager.GetString("ConditionalSetupsAreNotVerifiable", resourceCulture); - } - } - /// /// Looks up a localized string similar to Constructor arguments cannot be passed for delegate mocks.. /// diff --git a/src/Moq/Properties/Resources.resx b/src/Moq/Properties/Resources.resx index 6fb141e64..09e0223ca 100644 --- a/src/Moq/Properties/Resources.resx +++ b/src/Moq/Properties/Resources.resx @@ -351,7 +351,4 @@ This could be caused by an unrecognized type conversion, coercion, narrowing, or 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. - - 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. - - + \ No newline at end of file diff --git a/src/Moq/Setup.cs b/src/Moq/Setup.cs index 69a978712..633142390 100644 --- a/src/Moq/Setup.cs +++ b/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 predicate) diff --git a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs b/tests/Moq.Tests/Regressions/IssueReportsFixture.cs index 3937fffa1..1007351c3 100644 --- a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs +++ b/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(); + 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 diff --git a/tests/Moq.Tests/VerifyFixture.cs b/tests/Moq.Tests/VerifyFixture.cs index c247faaf0..4957ae937 100644 --- a/tests/Moq.Tests/VerifyFixture.cs +++ b/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(); - 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(() => 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(); + mock.When(() => true).Setup(m => m.Submit()).Verifiable(); + mock.Object.Submit(); + mock.Verify(); + mock.VerifyNoOtherCalls(); + } + public class Exclusion_of_unreachable_inner_mocks { [Fact]