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

Setup cannot invoke callback with different number of parameters in Moq 4.8.1 #572

Closed
Caraul opened this issue Jan 9, 2018 · 7 comments
Closed
Assignees
Labels

Comments

@Caraul
Copy link
Contributor

Caraul commented Jan 9, 2018

Hi,

looks like adding ValidateReturnDelegate makes extension methods unusable in zero argument methods such as property getters. Here is small repro that fails at Setup phase with ArgumentException:

using Moq;

namespace TestMoq
{
	internal class Program
	{
		internal static void Main()
		{
			var a = new A();
			var mock = new Mock<A>();
			mock.SetupGet(aa => aa.P).Returns(a.Pext);
		}
	}

	public class A
	{
		public virtual string P { get; set; }
	}

	internal static class Aext
	{
		public static string Pext(this A a)
		{
			return "42";
		}
	}
}

Not sure if this is expected effect - please could anyone tell is such behavior correct?

@stakx
Copy link
Contributor

stakx commented Jan 9, 2018

@Caraul, thanks for reporting. Without looking into this in detail, this looks like a regression introduced in 4.8.0. This used to work in Moq 4.7.145, but I'm not sure whether that was by design or incidental. Will try to look into this in the next few days.

@stakx stakx added the bug label Jan 10, 2018
@stakx
Copy link
Contributor

stakx commented Jan 10, 2018

It's indeed a regression caused by #520 / commit 104c646 (which aimed to make delegate validation more strict, as you've already found out). We didn't have a test for the extension method case you've provided. I'll add a regression test for this to the test suite, and fix the problem sometime in the next few days... or would you prefer to submit a PR yourself?

@Caraul
Copy link
Contributor Author

Caraul commented Jan 11, 2018

Well, fix won't take weeks but one week maybe cause I need to go deeper inside Moq - if next week ASAP is acceptable then I'm in with PR.

@Caraul Caraul closed this as completed Jan 11, 2018
@Caraul Caraul reopened this Jan 11, 2018
@stakx
Copy link
Contributor

stakx commented Jan 11, 2018

@Caraul, that is fine. 👍

I expect that a local fix in just that validation method should be possible; check whether the callback is an extension method (IsStatic & IsDefined(typeof(ExtensionAttribute))), and if so, subtract 1 from the parameter count when matching. Something like that.

Please don't forget to also add one or more unit tests to the test suite. :)

Thank you!

stakx added a commit that referenced this issue Jan 17, 2018
Issue #572 Setup cannot invoke callback with different number of parameters in Moq 4.8.1
@stakx stakx closed this as completed Jan 17, 2018
@Caraul
Copy link
Contributor Author

Caraul commented Jan 18, 2018

Thanks for moving Moq forward - Good job! 👍

@stakx
Copy link
Contributor

stakx commented Feb 23, 2018

Moq 4.8.2 (which includes your patch, see changelog) has just been published on NuGet.

@Caraul
Copy link
Contributor Author

Caraul commented Feb 23, 2018

Great news, thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants