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

Feature request: Promote Invocation.ReturnValue to IInvocation #920

Closed
MaStr11 opened this issue Sep 5, 2019 · 1 comment · Fixed by #921
Closed

Feature request: Promote Invocation.ReturnValue to IInvocation #920

MaStr11 opened this issue Sep 5, 2019 · 1 comment · Fixed by #921
Milestone

Comments

@MaStr11
Copy link
Contributor

MaStr11 commented Sep 5, 2019

The Invocation class has a property ReturnValue:
https://github.com/moq/moq4/blob/a12fe5b1fc2eb7857799d5a7bb5cc9e18f61aace/src/Moq/Invocation.cs#L69

The IIvocation interface does not include this property:

namespace Moq
{
	public interface IInvocation
	{
		MethodInfo Method { get; }
		IReadOnlyList<object> Arguments { get; }
	}
}

While the argument values of the invocation are accessible, the return value is not. Is it possible to make the return value part of the interface?

namespace Moq
{
	public interface IInvocation
	{
		MethodInfo Method { get; }
		IReadOnlyList<object> Arguments { get; }
		/// <summary>
		/// Gets the return value of the invocation.
		/// </summary>                
                object ReturnValue { get; }
	}
}

A related discussion of why the return value wasn't included in the first place can be found here:
#560 (comment)

I currently use reflection to access the Invocation.ReturnValue and the value is set to the correct value.

@stakx
Copy link
Contributor

stakx commented Sep 5, 2019

Yes, Moq does record the return value these days. See my comment in your PR for the reason why I haven't yet added ReturnValue to the public API despite that fact.

@stakx stakx added this to the 4.15.0 milestone Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants