From 25895f1ab36f2deb662780f87bd404e0cea0b496 Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 12:44:36 +0100 Subject: [PATCH 01/10] Add tests for invocations collection --- Moq.Tests/InvocationsFixture.cs | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 Moq.Tests/InvocationsFixture.cs diff --git a/Moq.Tests/InvocationsFixture.cs b/Moq.Tests/InvocationsFixture.cs new file mode 100644 index 000000000..7fc1597a2 --- /dev/null +++ b/Moq.Tests/InvocationsFixture.cs @@ -0,0 +1,52 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace Moq.Tests +{ + public class InvocationsFixture + { + [Fact] + public void MockInvocationsAreRecorded() + { + var mock = new Mock(); + + mock.Object.CompareTo(new object()); + + Assert.Equal(1, mock.Invocations.Count); + } + + [Fact] + public void MockInvocationsIncludeInvokedMethod() + { + var mock = new Mock(); + + mock.Object.CompareTo(new object()); + + var invocation = mock.Invocations[0]; + + var expectedMethod = typeof(IComparable).GetMethod(nameof(mock.Object.CompareTo)); + + Assert.Equal(expectedMethod, invocation.Method); + } + + [Fact] + public void MockInvocationsIncludeArguments() + { + var mock = new Mock(); + + var obj = new object(); + + mock.Object.CompareTo(obj); + + var invocation = mock.Invocations[0]; + + var expectedArguments = new[] {obj}; + + Assert.Equal(expectedArguments, invocation.Arguments); + } + } +} From 11f1027d0395f14b5014648e442d8af53a92d69a Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 13:32:13 +0100 Subject: [PATCH 02/10] Add thread-safety improvements to InvocationCollection --- Moq.Tests/InvocationsFixture.cs | 14 ++++ Source/InvocationCollection.cs | 109 ++++++++++++++++++++++++++------ 2 files changed, 104 insertions(+), 19 deletions(-) diff --git a/Moq.Tests/InvocationsFixture.cs b/Moq.Tests/InvocationsFixture.cs index 7fc1597a2..6bfea4f5d 100644 --- a/Moq.Tests/InvocationsFixture.cs +++ b/Moq.Tests/InvocationsFixture.cs @@ -48,5 +48,19 @@ public void MockInvocationsIncludeArguments() Assert.Equal(expectedArguments, invocation.Arguments); } + + [Fact] + public void MockInvocationsCanBeEnumerated() + { + var mock = new Mock(); + + mock.Object.CompareTo(-1); + mock.Object.CompareTo(0); + mock.Object.CompareTo(1); + + var count = mock.Invocations.Count(); + + Assert.Equal(mock.Invocations.Count, count); + } } } diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index dcbf35564..093cf60e3 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -47,20 +47,20 @@ namespace Moq { internal sealed class InvocationCollection : IReadOnlyList { - private List invocations; + private Invocation[] invocations; - public InvocationCollection() - { - this.invocations = new List(); - } + private int capacity = 0; + private int count = 0; + + private readonly object invocationsLock = new object(); public int Count { get { - lock (this.invocations) + lock (this.invocationsLock) { - return this.invocations.Count; + return count; } } } @@ -69,7 +69,7 @@ public IReadOnlyInvocation this[int index] { get { - lock (this.invocations) + lock (this.invocationsLock) { return this.invocations[index]; } @@ -78,49 +78,120 @@ public IReadOnlyInvocation this[int index] public void Add(Invocation invocation) { - lock (this.invocations) + lock (this.invocationsLock) { - this.invocations.Add(invocation); + if (this.count == this.capacity) + { + var targetCapacity = this.capacity == 0 ? 4 : (this.capacity * 2); + Array.Resize(ref this.invocations, targetCapacity); + this.capacity = targetCapacity; + } + + this.invocations[this.count] = invocation; + this.count++; } } public bool Any() { - return this.invocations.Count > 0; + lock (this.invocationsLock) + { + return this.count > 0; + } } public void Clear() { - lock (this.invocations) + lock (this.invocationsLock) { - this.invocations.Clear(); + // Replace the collection so readers with a reference to the old collection aren't interrupted + this.invocations = null; + this.count = 0; + this.capacity = 0; } } public Invocation[] ToArray() { - lock (this.invocations) + lock (this.invocationsLock) { - return this.invocations.ToArray(); + if (this.count == 0) + { + return new Invocation[0]; + } + + return this.invocations.Take(this.count).ToArray(); } } public Invocation[] ToArray(Func predicate) { - lock (this.invocations) + lock (this.invocationsLock) { - return this.invocations.Where(predicate).ToArray(); + if (this.count == 0) + { + return new Invocation[0]; + } + + return this.invocations.Take(this.count).Where(predicate).ToArray(); } } public IEnumerator GetEnumerator() { - lock (this.invocations) + lock (this.invocationsLock) { - return this.invocations.ToList().GetEnumerator(); + return new Enumerator(this.invocations, this.count); } } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + /// + /// Custom enumerator which allows enumeration over the invocation collection, + /// even when the underlying collection is modified. + /// + /// + /// The enumerator insulates the caller from seeing any changes to the collection by only providing + /// access to the range which was present when the enumerator was created. The invocation collection + /// only appends new items, never removing or modifying. + /// + private class Enumerator : IEnumerator + { + private readonly int count; + private readonly Invocation[] collection; + private int index = 0; + + public Enumerator(Invocation[] collection, int count) + { + this.collection = collection; + this.count = count; + } + + public IReadOnlyInvocation Current { get; private set; } + + object IEnumerator.Current => this.Current; + + public bool MoveNext() + { + if (this.index < this.count) + { + this.Current = this.collection[this.index]; + this.index++; + return true; + } + + return false; + } + + public void Reset() + { + this.index = 0; + } + + public void Dispose() + { + } + } } } From 5738a90ca9eeb97e9dc98f7e84124256ec1290e0 Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 14:53:39 +0100 Subject: [PATCH 03/10] Improve enumeration test and add clear test --- Moq.Tests/InvocationsFixture.cs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/Moq.Tests/InvocationsFixture.cs b/Moq.Tests/InvocationsFixture.cs index 6bfea4f5d..4f514ea93 100644 --- a/Moq.Tests/InvocationsFixture.cs +++ b/Moq.Tests/InvocationsFixture.cs @@ -1,8 +1,4 @@ using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; using Xunit; namespace Moq.Tests @@ -58,9 +54,31 @@ public void MockInvocationsCanBeEnumerated() mock.Object.CompareTo(0); mock.Object.CompareTo(1); - var count = mock.Invocations.Count(); + var count = 0; - Assert.Equal(mock.Invocations.Count, count); + using (var enumerator = mock.Invocations.GetEnumerator()) + { + while (enumerator.MoveNext()) + { + Assert.NotNull(enumerator.Current); + + count++; + } + } + + Assert.Equal(3, count); + } + + [Fact] + public void MockInvocationsCanBeCleared() + { + var mock = new Mock(); + + mock.Object.CompareTo(new object()); + + mock.ResetCalls(); + + Assert.Equal(0, mock.Invocations.Count); } } } From 8dc2ce341e8bf9f34060a251fa3c0dbaeca65fbf Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 16:57:38 +0100 Subject: [PATCH 04/10] Remove Linq calls from ToArray methods --- Source/InvocationCollection.cs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index 093cf60e3..77813f2bf 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -120,7 +120,11 @@ public Invocation[] ToArray() return new Invocation[0]; } - return this.invocations.Take(this.count).ToArray(); + var result = new Invocation[this.count]; + + Array.Copy(this.invocations, result, this.count); + + return result; } } @@ -133,7 +137,24 @@ public Invocation[] ToArray(Func predicate) return new Invocation[0]; } - return this.invocations.Take(this.count).Where(predicate).ToArray(); + var result = new Invocation[this.count]; + var resultIndex = 0; + var resultSize = 0; + + for (var i = 0; i < this.count; i++) + { + var invocation = this.invocations[i]; + if (predicate(invocation)) + { + result[resultIndex] = invocation; + resultIndex++; + resultSize++; + } + } + + Array.Resize(ref result, resultSize); + + return result; } } From 61cdb4dd53808413c58883dce3205324c5c52323 Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 17:08:51 +0100 Subject: [PATCH 05/10] Switch custom Enumerator to using yield return --- Source/InvocationCollection.cs | 57 ++++++---------------------------- 1 file changed, 10 insertions(+), 47 deletions(-) diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index 77813f2bf..e0f818afe 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -160,59 +160,22 @@ public Invocation[] ToArray(Func predicate) public IEnumerator GetEnumerator() { - lock (this.invocationsLock) - { - return new Enumerator(this.invocations, this.count); - } - } - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - - /// - /// Custom enumerator which allows enumeration over the invocation collection, - /// even when the underlying collection is modified. - /// - /// - /// The enumerator insulates the caller from seeing any changes to the collection by only providing - /// access to the range which was present when the enumerator was created. The invocation collection - /// only appends new items, never removing or modifying. - /// - private class Enumerator : IEnumerator - { - private readonly int count; - private readonly Invocation[] collection; - private int index = 0; - - public Enumerator(Invocation[] collection, int count) - { - this.collection = collection; - this.count = count; - } + // Take local copies of collection and count so they are isolated from changes by other threads. + Invocation[] collection; + int count; - public IReadOnlyInvocation Current { get; private set; } - - object IEnumerator.Current => this.Current; - - public bool MoveNext() - { - if (this.index < this.count) - { - this.Current = this.collection[this.index]; - this.index++; - return true; - } - - return false; - } - - public void Reset() + lock (this.invocationsLock) { - this.index = 0; + collection = this.invocations; + count = this.count; } - public void Dispose() + for (var i = 0; i < count; i++) { + yield return collection[i]; } } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } } From 0dbb1bbded34dec0d46e2e6e77045a1e0bf1d31c Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 18:42:35 +0100 Subject: [PATCH 06/10] Add tests and check for indexer --- Moq.Tests/InvocationsFixture.cs | 24 ++++++++++++++++++++++++ Source/InvocationCollection.cs | 6 ++++++ 2 files changed, 30 insertions(+) diff --git a/Moq.Tests/InvocationsFixture.cs b/Moq.Tests/InvocationsFixture.cs index 4f514ea93..fd22eea23 100644 --- a/Moq.Tests/InvocationsFixture.cs +++ b/Moq.Tests/InvocationsFixture.cs @@ -80,5 +80,29 @@ public void MockInvocationsCanBeCleared() Assert.Equal(0, mock.Invocations.Count); } + + [Fact] + public void MockInvocationsCanBeRetrievedByIndex() + { + var mock = new Mock(); + + mock.Object.CompareTo(-1); + mock.Object.CompareTo(0); + mock.Object.CompareTo(1); + + var invocation = mock.Invocations[1]; + + var arg = invocation.Arguments[0]; + + Assert.Equal(0, arg); + } + + [Fact] + public void MockInvocationsIndexerThrowsIndexOutOfRangeWhenCollectionIsEmpty() + { + var mock = new Mock(); + + Assert.Throws(() => mock.Invocations[0]); + } } } diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index e0f818afe..f32206cbf 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -69,6 +69,12 @@ public IReadOnlyInvocation this[int index] { get { + // Test + if (this.invocations == null) + { + throw new IndexOutOfRangeException(); + } + lock (this.invocationsLock) { return this.invocations[index]; From c9dd227b94514fb1a1252b90d028be514964d57a Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 18:52:30 +0100 Subject: [PATCH 07/10] Remove obsolete Any method --- Source/InvocationCollection.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index f32206cbf..8ef384332 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -69,7 +69,6 @@ public IReadOnlyInvocation this[int index] { get { - // Test if (this.invocations == null) { throw new IndexOutOfRangeException(); @@ -98,14 +97,6 @@ public void Add(Invocation invocation) } } - public bool Any() - { - lock (this.invocationsLock) - { - return this.count > 0; - } - } - public void Clear() { lock (this.invocationsLock) From ef18220b8f311fd42a7b7f1779bdae9c99a4df9b Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 19:02:19 +0100 Subject: [PATCH 08/10] Fix broken indexer check --- Source/InvocationCollection.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index 8ef384332..bf20a2f44 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -69,13 +69,13 @@ public IReadOnlyInvocation this[int index] { get { - if (this.invocations == null) - { - throw new IndexOutOfRangeException(); - } - lock (this.invocationsLock) { + if (this.count >= index || index < 0) + { + throw new IndexOutOfRangeException(); + } + return this.invocations[index]; } } From 108688dfd8e4fe6da1cbb65e487078e80dc8f104 Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 19:13:57 +0100 Subject: [PATCH 09/10] Fix index check --- Source/InvocationCollection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index bf20a2f44..74c2bb7a4 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -71,7 +71,7 @@ public IReadOnlyInvocation this[int index] { lock (this.invocationsLock) { - if (this.count >= index || index < 0) + if (this.count <= index || index < 0) { throw new IndexOutOfRangeException(); } From aebd6dbea3877c0ec61a2a84aaaf45e444bfd91f Mon Sep 17 00:00:00 2001 From: Paul Turner Date: Sun, 10 Jun 2018 19:21:38 +0100 Subject: [PATCH 10/10] Simplified ToArray-with-predicate overload --- Source/InvocationCollection.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/Source/InvocationCollection.cs b/Source/InvocationCollection.cs index 74c2bb7a4..c75c85225 100644 --- a/Source/InvocationCollection.cs +++ b/Source/InvocationCollection.cs @@ -133,25 +133,19 @@ public Invocation[] ToArray(Func predicate) { return new Invocation[0]; } - - var result = new Invocation[this.count]; - var resultIndex = 0; - var resultSize = 0; + + var result = new List(this.count); for (var i = 0; i < this.count; i++) { var invocation = this.invocations[i]; if (predicate(invocation)) { - result[resultIndex] = invocation; - resultIndex++; - resultSize++; + result.Add(invocation); } } - Array.Resize(ref result, resultSize); - - return result; + return result.ToArray(); } }