Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exception property to IInvocation #1077

Merged
merged 23 commits into from Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cc90a0f
Add Exception property to IInvocation
MaStr11 Oct 14, 2020
2a6a5f4
Introduce InvocationExceptionWrapper and add logic to IINvocation.Ret…
MaStr11 Oct 14, 2020
01b9329
Add inheritdoc doc comments.
MaStr11 Oct 14, 2020
dd5c654
Add tests for IInvocation.ReturnValue
MaStr11 Oct 14, 2020
1936182
Add test MockInvocationsIncludeException_Setup
MaStr11 Oct 14, 2020
67793f3
Add exception handling to Interceptor.Intercept
MaStr11 Oct 14, 2020
e49273f
Add test for "CallBase = true" and method returning an exception type.
MaStr11 Oct 14, 2020
a66310e
Add code comment why mock.Invocations is empty.
MaStr11 Oct 14, 2020
1e18aff
Add header to InvocationExceptionWrapper
MaStr11 Oct 14, 2020
988feb0
Change doc comments on IInvocation.
MaStr11 Oct 14, 2020
a18d167
Remove "inhertitdoc" from internal type.
MaStr11 Oct 14, 2020
31f5ec7
Harden test by replacing hard coded value by calculated random.Next
MaStr11 Oct 14, 2020
09816d0
Add MockInvocationsIncludeException_BaseCall_Virtual test
MaStr11 Oct 14, 2020
a064c98
Add test for behavior of MockException and remove exclusion of catch(…
MaStr11 Oct 14, 2020
22f3575
Update changelog
MaStr11 Oct 14, 2020
f2cfc4c
Revert whitespace change
MaStr11 Oct 14, 2020
0ae9189
Revert whitespace change
MaStr11 Oct 14, 2020
6055015
Use Assert.Throws instead of try/catch
MaStr11 Oct 14, 2020
e1fa793
Formatting
MaStr11 Oct 14, 2020
b2c07f3
Whitespace
MaStr11 Oct 14, 2020
b9a4d63
Fix failing test
MaStr11 Oct 14, 2020
3f2a43b
Remove misleading test
MaStr11 Oct 15, 2020
3b5be64
Assert `thrown.Reasons` instead of `thrown.Message`
MaStr11 Oct 15, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -10,7 +10,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
#### Added

* New method overloads for `It.Is`, `It.IsIn`, and `It.IsNotIn` that compare values using a custom `IEqualityComparer<T>` (@weitzhandler, #1064)
* New property `IInvocation.ReturnValue` to query recorded invocations return values (@MaStr11, #921)
* New properties `ReturnValue` and `Exception` on `IInvocation` to query recorded invocations return values or exceptions (@MaStr11, #921, #1077)


## 4.14.7 (2020-10-14)
Expand Down
8 changes: 7 additions & 1 deletion src/Moq/IInvocation.cs
@@ -1,6 +1,7 @@
// Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD, and Contributors.
// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.

using System;
using System.Collections.Generic;
using System.Reflection;

Expand Down Expand Up @@ -32,8 +33,13 @@ public interface IInvocation
bool IsVerified { get; }

/// <summary>
/// Gets the return value of the invocation.
/// The value being returned for a non-void method if no exception was thrown.
/// </summary>
object ReturnValue { get; }
MaStr11 marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Optional exception if the method invocation results in an exception being thrown.
/// </summary>
Exception Exception { get; }
}
}
9 changes: 8 additions & 1 deletion src/Moq/Invocation.cs
Expand Up @@ -71,14 +71,21 @@ public MethodInfo MethodImplementation

public object ReturnValue
{
get => this.returnValue;
get => this.returnValue is InvocationExceptionWrapper
? null
: this.returnValue;
set
{
Debug.Assert(this.returnValue == null);
this.returnValue = value;
}
}

public Exception Exception
=> this.returnValue is InvocationExceptionWrapper wrapper
? wrapper.Exception
: null;

public bool IsVerified => this.verified;

/// <summary>
Expand Down
22 changes: 22 additions & 0 deletions src/Moq/InvocationExceptionWrapper.cs
@@ -0,0 +1,22 @@
// Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD, and Contributors.
// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.

using System;

namespace Moq
{
/// <summary>
/// Internal type to mark invocation results as "exception occurred during execution". The type just
/// wraps the Exception so a thrown exception can be distinguished from an <see cref="System.Exception"/>
/// return by the invocation.
/// </summary>
internal readonly struct InvocationExceptionWrapper
{
public InvocationExceptionWrapper(Exception exception)
{
Exception = exception;
}

public Exception Exception { get; }
}
}
5 changes: 5 additions & 0 deletions src/Moq/ProxyFactories/CastleProxyFactory.cs
Expand Up @@ -101,6 +101,11 @@ public void Intercept(Castle.DynamicProxy.IInvocation underlying)
this.interceptor.Intercept(invocation);
underlying.ReturnValue = invocation.ReturnValue;
}
catch (Exception ex)
stakx marked this conversation as resolved.
Show resolved Hide resolved
{
invocation.ReturnValue = new InvocationExceptionWrapper(ex);
throw;
stakx marked this conversation as resolved.
Show resolved Hide resolved
}
finally
{
invocation.DetachFromUnderlying();
Expand Down
129 changes: 128 additions & 1 deletion tests/Moq.Tests/InvocationsFixture.cs
Expand Up @@ -2,6 +2,8 @@
// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

using Xunit;
Expand Down Expand Up @@ -45,11 +47,131 @@ public void MockInvocationsIncludeArguments()

var invocation = mock.Invocations[0];

var expectedArguments = new[] {obj};
var expectedArguments = new[] { obj };

Assert.Equal(expectedArguments, invocation.Arguments);
}

[Fact]
public void MockInvocationsIncludeReturnValue_NoSetup()
{
var mock = new Mock<IComparable>();

var obj = new object();

mock.Object.CompareTo(obj);

var invocation = mock.Invocations[0];

Assert.Equal(0, invocation.ReturnValue);
Assert.Null(invocation.Exception);
}

[Fact]
public void MockInvocationsIncludeReturnValue_Setup()
{
var mock = new Mock<IComparable>();
var obj = new object();
mock.Setup(c => c.CompareTo(obj)).Returns(42);

mock.Object.CompareTo(obj);

var invocation = mock.Invocations[0];

Assert.Equal(42, invocation.ReturnValue);
Assert.Null(invocation.Exception);
}

[Fact]
public void MockInvocationsIncludeReturnValue_BaseCall()
{
var mock = new Mock<Random>(1) // seed: 1
{
CallBase = true,
};

mock.Object.Next();

var invocation = mock.Invocations[0];

Assert.Equal(new Random(Seed: 1).Next(), invocation.ReturnValue);
Assert.Null(invocation.Exception);
}

[Fact]
public void MockInvocationsIncludeReturnValue_ReturnsException()
{
var mock = new Mock<ICloneable>();
var returnValue = new Exception();
mock.Setup(c => c.Clone()).Returns(returnValue);

mock.Object.Clone();

var invocation = mock.Invocations[0];

Assert.Equal(returnValue, invocation.ReturnValue);
Assert.Null(invocation.Exception);
}

[Fact]
public void MockInvocationsIncludeException_Setup()
{
var mock = new Mock<IComparable>();
var exception = new Exception("Message");
mock.Setup(c => c.CompareTo(It.IsAny<object>())).Throws(exception);

var thrown = Assert.Throws<Exception>(() => mock.Object.CompareTo(null));

Assert.Equal(exception.Message, thrown.Message);

var invocation = mock.Invocations[0];
Assert.Same(thrown, invocation.Exception);
}

[Fact]
public void MockInvocationsIncludeException_BaseCall_Nonvirtual()
{
var mock = new Mock<List<int>>()
{
CallBase = true,
};

Assert.Throws<ArgumentException>(() => mock.Object.GetRange(1, 1));
// Base-call exceptions are not recorded
Assert.Empty(mock.Invocations);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good of you for thinking of this test case! (I didn't.)

Something's odd here—invocations should get recorded even when mock.CallBase == true. I think the reason why no invocations get recorded here is that List<>.GetRange is not virtual.

Could you change this test so it calls some overridable method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't think of the virtual requirement. I will add another test and change the code comment accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added another test 09816d0

Copy link
Contributor

@stakx stakx Oct 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this test completely. It really targets List<>.GetRange, and nothing from Moq. What's more, it contains a factually incorrect statement in the comment, // Base-call exceptions are not recorded. All things considered, this test as it stands will potentially raise more questions than it answers.

(The newly added test is fine, though!)

}

[Fact]
public void MockInvocationsIncludeException_BaseCall_Virtual()
{
var mock = new Mock<Test>()
{
CallBase = true,
};

var thrown = Assert.Throws<InvalidOperationException>(() => mock.Object.ThrowingVirtualMethod());

Assert.Equal("Message", thrown.Message);

var invocation = mock.Invocations[0];
Assert.Same(thrown, invocation.Exception);
}

[Fact]
public void MockInvocationsIncludeException_MockException()
{
var mock = new Mock<ICloneable>(MockBehavior.Strict);

var thrown = Assert.Throws<MockException>(() => mock.Object.Clone());

Assert.Equal(
@"ICloneable.Clone() invocation failed with mock behavior Strict.
All invocations on the mock must have a corresponding setup.", thrown.Message);
stakx marked this conversation as resolved.
Show resolved Hide resolved

var invocation = mock.Invocations[0];
Assert.Same(thrown, invocation.Exception);
}

[Fact]
public void MockInvocationsCanBeEnumerated()
{
Expand Down Expand Up @@ -269,5 +391,10 @@ public FlagInitiallySetToTrue()

public virtual bool Flag { get; set; }
}

public class Test
{
public virtual int ThrowingVirtualMethod() => throw new InvalidOperationException("Message");
}
}
}