-
-
Notifications
You must be signed in to change notification settings - Fork 803
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
How can I mock a function with ref parameter? #105
Comments
Questions should be directed to moqdisc@googlegroups.com |
Yes @kzu , but is possible or not? If is not possible it is an issue. |
You can mock, yes. You can't capture the value via a callback though. |
@kzu but how? It throws an exception. |
Hi, Thread necromancy. Is there any chance of progress on this? I love Moq and I've used it exclusively on many projects spanning several years but I've just bumped up against this issue and it's quite frustrating. There's now a chunk of my project I can't cover, and I can't just pull in a new mocking framework :( Cheers, |
I'd gladly accept a PR if this is critically important to you. Thanks! On Mon, Jun 20, 2016, 12:49 PM mattthr notifications@github.com wrote:
|
@kzu PR ... is that a pull request? Forgive my ignorance, I'm not a regular user of GitHub. Since I'm not writing a fork I'm not sure how best to do that, so I'll frame my problem below. Given a setup such as this:
I want to be able to write a unit test that can check CountFields is working correctly. As it stands, if I do this:
The method will error because the ref values will come out as null. If I change the mock setup to
I get the error "Invalid callback. Setup on method with parameters (String[]&,String[]&) cannot invoke callback with parameters (String[],String[])". I don't believe there's a way round this scenario using Moq as it stands, although I'd love to be proved wrong. |
Yup, seems like a bug. I wonder why they have to be ref if they are purely I hardly use ref parameters myself, which is why I suggested a PR if this Thanks On Tue, Jun 21, 2016, 6:32 AM mattthr notifications@github.com wrote:
|
@kzu The real code is more complex, but in truth they probably don't need to be ref. I can certainly try to refactor using Just seems like a curious oversight and obviously I didn't know this would be an issue when I wrote the code (I write test after rather than TDD, don't shoot me!). |
Hello everyone, I use a extensive's method, it is called IgnoreRefMatching. I found that class at Stackoverflow, but I lost the reference. Here is the class, I only use with ref parameter: ` namespace Test.Common.Moq
} And part of my test is: ` _mockRepository.Setup(x => x.List(It.IsAny<Int32?>(), It.IsAny(), It.IsAny(), It.IsAny<Boolean?>(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), ref total)).Returns(lst).Callback<Int32?, Int32, Int32, Boolean?, Int32, Int32, String, String, Int32>((CallParamIdCliente, CallParamIdUsuario, CallParamIdRegra, CallParamAssociado, CallParamStartRowIndex, CallParamPageSize, CallParamFiltro, CallParamOrderBy, CallParamTotal) => ValidCallBackTipoAssociacao(CallParamFiltro)).IgnoreRefMatching(); P.s.: I use .Net Core 1.1 |
Thanks. For those who (will) struggle like me, you can use @jeremybeavon's gist as well; RefCallback was not working for me. However, you can combine Jeremy's callback and the IgnoreRefMatching function from above. https://gist.github.com/jeremybeavon/36334a937d6c405831b9 |
While looking into #445, I've stumbled upon an inconsistency in Moq that could help us resolve this issue here. We know that Suggested changes:
Any opinions on this? |
|
@ohadschn, thanks for the feedback.
In principle, I totally agree with you. It would be logically correct if the
The last point (convenience) is why I think that not matching I agree that this "feature" can lead to confusion. One way out of this might be to create some Roslyn analyzers that would give informational hints or warnings if you try to use the API incorrectly, such as when you're trying to assign to a by-value callback parameter... but I'm daydreaming now. :)
I think we can do better than requiring users to pass around
That's a tough one. There's two aspects here:
|
|
It seems the expression tree API hasn't received a lot of love in the past. Nevertheless, that's good idea. 👍
OK, then I suggest the following for Moq 4.8:
|
I agree for the sake of backwards compatibility (as an MS employee I know a thing or two about that 😅). That being said, I hope you'll consider consolidating the |
@ohadschn - Yes, I would really like I'm not exactly a fan of the following suggestion, but perhaps we could make I'll get working on these changes in the next few days (excluding the compat switch). |
I think the merit for the switch is more on the grounds of backwards compatibility than user convenience (they don't really know what's good for them, just look at all the JS developers 😉). |
@ohadschn - the new method overloads are pretty much ready (see PR above). There's one unfortunate thing. Ideally, the new overloads would've looked like this:
This would've allowed us to do this: delegate void ExecuteCallback(ref Command _);
Command _;
mock.Setup(m => m.Execute(ref _)).Callback<ExecuteCallback>((ref Command command) => …);
^^^^^^^^^^^^^^^^^ Unfortunately, the C# compiler doesn't allow that type constraint. Unless we start playing around with Fody or another IL weaver, we need to drop it. Without the constraint, however, the methods compete with e.g.
mock.Setup(m => m.Execute(ref _)).Callback(new ExecuteCallback((ref Command command) => …));
^^^^^^^^^^^^^^^^^^^^ ^ Not quite as terse, but still working. |
Since this is essentially a new method, how about keeping the generics and simply changing the method name (e.g. As for the constraint, IMO this would be good enough: https://stackoverflow.com/a/191949/67824 (basically have a |
I've tried this, too. It didn't work, there were still occurrences in the test suite where the C# compiler would still pick the wrong overload. Definitely safer to either give the new method a different name, or give up on the generic parameter altogether.
Thanks for pointing out this possibility, I never even thought of this. :) I would however much prefer, and advise, that we stay with
So I think we should continue with |
@ohadschn - regarding C# missing ability to deal with
Done. See dotnet/csharplang#158 (comment). |
Well you could argue that this is a slightly different use case that merits maybe something like Since type safety isn't static either way (you have to check the Nice find on the csharplang issue BTW! |
@ohadschn - All done. Will start on your issue about @danicomas, @mattthr - It's been a while, but FYI the upcoming Moq version 4.8 will allow you do set up methods with public class Hola
{
// that's what you want to set up a callback for:
public virtual void DoSomething(ref string a) { }
}
// in order to do so, you will need a delegate with a matching signature:
delegate void DoSomethingAction(ref string a);
[Fact]
public void Test()
{
string o = "1";
var mock = new Mock<Hola>();
mock.Setup(x => x.DoSomething(ref o))
.Callback(new DoSomethingAction((ref string a) => a = "2"));
mock.Object.DoSomething(ref o);
Assert.Equal("2", o)
} The syntax is admittedly not super nice; the discussion above shows how we got here. Hope this helps! |
Good stuff @stakx ! I take it the sample works because Did you end up implementing |
That's correct. Think of it like this: var trigger = "0"; // holds the value that the setup will try to match against
mock.Setup(m => m.Method(ref trigger))
.Callback(new MethodHandler((ref string x) => x = "1"));
var value = trigger; // use trigger as input so setup is matched
Assert.Equal("0", value); // before
mock.Object.Method(ref value); // value gets aliased as parameter x in callback
Assert.Equal("1", value); // after
No, for the simple reason that I don't see a way to do it, given the current language / compiler limitations. The best I could think of is something like: mock.Setup(m => m.MethodWithByRefParams(...))
.DoNotMatchByRefParams()
...; I guess it'd be best to sketch out, discuss, and track such feature (whatever its concrete form) in a separate, new issue. |
Oh right, I forgot about the expression tree issue. While Do you want me to open an issue or would you prefer to open it? |
@ohadschn: Go right ahead. :-) |
When will version 4.8 be released? :) |
No fixed date yet; my guess is that it'll take another few weeks. I'd like to be extra careful with the planned features, as they could easily cause breaking changes. |
@asyle83 - A pre-release of Moq 4.8.0 (4.8.0-rc1) is now available on NuGet. |
I have this simple code, but I can't do it work
The error is this:
I prefer to use the Moq library. But only I get it on the Rhino Mocks library, with the function OutRef:
Thanks in advance!
The text was updated successfully, but these errors were encountered: