diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dd1ee76c..76b29a327 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1 * New SetupSequence verbs .PassAsync() and .ThrowsAsync(...) for async methods with void return type (@fuzzybair, #993) +#### Fixed + +* `StackOverflowException` on `VerifyAll` when mocked method returns mocked object (@hotchkj, #1012) + + ## 4.14.0 (2020-04-24) #### Added diff --git a/src/Moq/Mock.cs b/src/Moq/Mock.cs index cec7f1e6a..e426d618b 100644 --- a/src/Moq/Mock.cs +++ b/src/Moq/Mock.cs @@ -295,14 +295,24 @@ public void VerifyAll() private void Verify(Func predicate) { - if (!this.TryVerify(predicate, out var error) && error.IsVerificationError) + var verifiedMocks = new HashSet(); + + if (!this.TryVerify(predicate, verifiedMocks, out var error) && error.IsVerificationError) { throw error; } } - internal bool TryVerify(Func predicate, out MockException error) + internal bool TryVerify(Func predicate, HashSet verifiedMocks, out MockException error) { + if (verifiedMocks.Add(this) == false) + { + // This mock has already been verified; don't verify it again. + // (We can end up here e.g. when there are loops in the inner mock object graph.) + error = null; + return true; + } + foreach (Invocation invocation in this.MutableInvocations) { invocation.MarkAsVerifiedIfMatchedBy(predicate); @@ -312,7 +322,7 @@ internal bool TryVerify(Func predicate, out MockException error) foreach (var setup in this.MutableSetups.ToArray(predicate)) { - if (predicate(setup) && !setup.TryVerify(recursive: true, predicate, out var e) && e.IsVerificationError) + if (predicate(setup) && !setup.TryVerify(recursive: true, predicate, verifiedMocks, out var e) && e.IsVerificationError) { errors.Add(e); } diff --git a/src/Moq/Setup.cs b/src/Moq/Setup.cs index f1f8d9765..f6ea7c38f 100644 --- a/src/Moq/Setup.cs +++ b/src/Moq/Setup.cs @@ -2,6 +2,7 @@ // All rights reserved. Licensed under the BSD 3-Clause License; see License.txt. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq.Expressions; using System.Reflection; @@ -127,6 +128,9 @@ public override string ToString() /// /// Specifies which setups should be verified. /// + /// + /// The set of mocks that have already been verified. + /// /// /// If this setup or any of its inner mock (if present and known) failed verification, /// this parameter will receive a describing the verification error(s). @@ -135,7 +139,7 @@ public override string ToString() /// if verification succeeded without any errors; /// otherwise, . /// - public bool TryVerify(bool recursive, Func predicate, out MockException error) + internal bool TryVerify(bool recursive, Func predicate, HashSet verifiedMocks, out MockException error) { MockException e; @@ -147,7 +151,7 @@ public bool TryVerify(bool recursive, Func predicate, out MockExce } // optionally verify setups of inner mock (if present and known): - if (recursive && this.InnerMock?.TryVerify(predicate, out e) == false && e.IsVerificationError) + if (recursive && this.InnerMock?.TryVerify(predicate, verifiedMocks, out e) == false && e.IsVerificationError) { error = MockException.FromInnerMockOf(this, e); return false; @@ -186,12 +190,14 @@ public void VerifyAll() private void Verify(bool recursive, Func predicate) { + var verifiedMocks = new HashSet(); + foreach (Invocation invocation in this.mock.MutableInvocations) { invocation.MarkAsVerifiedIfMatchedBy(setup => setup == this); } - if (!this.TryVerify(recursive, predicate, out var error) && error.IsVerificationError) + if (!this.TryVerify(recursive, predicate, verifiedMocks, out var error) && error.IsVerificationError) { throw error; } diff --git a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs b/tests/Moq.Tests/Regressions/IssueReportsFixture.cs index b6941a281..4b41ba9bd 100644 --- a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs +++ b/tests/Moq.Tests/Regressions/IssueReportsFixture.cs @@ -3279,6 +3279,40 @@ public interface IQueryOver #endregion + #region 1012 + + public class Issue1012 + { + [Fact] + public void Verification_can_deal_with_cycles_in_inner_mock_object_graph_1() + { + var mock = new Mock(); + mock.Setup(m => m.X).Returns(mock.Object); + _ = mock.Object.X; + mock.VerifyAll(); + } + + [Fact] + public void Verification_can_deal_with_cycles_in_inner_mock_object_graph_2() + { + var mock = new Mock(); + var otherMock = new Mock(); + mock.Setup(m => m.X).Returns(otherMock.Object); + otherMock.Setup(m => m.X).Returns(mock.Object); + _ = mock.Object.X; + _ = otherMock.Object.X; + mock.VerifyAll(); + otherMock.VerifyAll(); + } + + public interface IX + { + IX X { get; } + } + } + + #endregion + // Old @ Google Code #region #47