An unmocked method leaves its out parameter value untouched

[davidnemeti]

Describe the bug

An unmocked method leaves its out parameter value untouched, i.e. it does not set it to default value.

Now, that seems a bit unorthodox, so I believe that it should set it to the default value. A normal (non-mocked) method cannot behave like this, i.e. it cannot leave its out parameter value as not set.

Handling out parameters should be similar to handling the return value: an unmocked method returns the default value.

To Reproduce

void Main()
{
    var foo = Substitute.For<IFoo>();

    int number1, number2;

    number1 = 11;
    number2 = 22;

    foo.Baz(out number1);
    foo.Bar(out number2);

    Console.WriteLine(number1);
    Console.WriteLine(number2);
}

public interface IFoo
{
    void Bar(out int number);
    int Baz(out int number);
}

The above code produces the following output:

11
22

Expected behaviour

The above code should produce the following output:

0
0

Environment

• NSubstitute version: 4.2.1
• Platform: .NET Framework 4.8

[zvirja]

@stakx @blairconrad Could you please describe the behavior of Moq and FakeItEasy in this regards? I could see value in both behaviors, so it would be interesting to see your conclusions ;-)

[stakx]

@zvirja: Moq 4's behavior is as follows:

When you don't set up the invoked method:

Moq will behave in the same way as was described above for NSubstitute: The out arguments are left untouched.

[Fact]
public void Moq_will_leave_out_arguments_untouched_if_invoked_method_not_set_up()
{
    var foo = new Mock<IFoo>().Object;

    int number1, number2;

    number1 = 11;
    number2 = 22;

    foo.Baz(out number1);
    foo.Bar(out number2);

    Assert.Equal(11, number1);
    Assert.Equal(22, number2);
}

I tend to agree with OP that this could be considered a bug, since in C# methods must assign out parameters before returning (except when they throw)—and ideally, mocks should follow suit for consistency.

That being said, I'm not sure we'll change this behavior in Moq—it would be a breaking change, though admittedly not a very risky one, and noone's complained about it so far. The last is not surprising, since there's usually no point in manually initializing the variables that will be used as an argument to out parameters; the compiler & runtime will take care of initializing locals to their default value.

(P.S.: While Moq has other APIs and extensibility points like Mock.SetReturnsDefault<T>(T defaultValue) and Mock.DefaultValueProvider, those are currently only invoked for producing default return values; these APIs aren't invoked for producing default out argument values. This might possibly change, depending on how AutoFixture/AutoFixture#1139 goes.)

When you set up the invoked method:

Moq will read the out argument used in the setup expression to determine what the out arguments of mock method invocations should be set to (which IMHO is a rather "interesting" way of going about this):

[Fact]
public void Moq_will_set_out_arguments_to_value_provided_in_out_parameter_at_setup_time()
{
    var fooMock = new Mock<IFoo>();

    int desiredNumber1 = 1111, desiredNumber2 = 2222;

    fooMock.Setup(f => f.Baz(out desiredNumber1));
    fooMock.Setup(f => f.Bar(out desiredNumber2));

    var foo = fooMock.Object;

    int number1, number2;

    number1 = 11;
    number2 = 22;

    foo.Baz(out number1);
    foo.Bar(out number2);

    Assert.Equal(1111, number1);
    Assert.Equal(2222, number2);
}

Let me know if you'd like to know more.

[blairconrad]

Hey, folks. FakeItEasy is no different—the value in the out argument will be left as it is.

I don't mind this behaviour, to tell the truth. Sure, a hand-coded method with an out parameter must assign a value, but there's nothing that requires it to be equivalent to default. Maybe the method just happens to always assign the same value that was passed in. By luck. 😉

@davidnemeti, may I ask what unwanted behaviour occurred because of this? Did you have a test that failed (or passed when it shouldn't)? Can you give any details? If you have production code that is expecting the faked substituted collaborator to provide a particular value, I would consider explicitly configuring it to do so rather than relying on default behaviour.

That being said, if a change were to be made, I very much like your suggestion of providing the same value as would be returned from an unconfigured method.

/cc @thomaslevesque

[thomaslevesque]

A normal (non-mocked) method cannot behave like this, i.e. it cannot leave its out parameter value as not set.

That's true in C#, because the compiler enforces it. But for the runtime, there's no difference between ref and out parameters, and initializing out parameters is not enforced. Other .NET languages, such as VB.NET, don't have the concept of out parameters. There's an <Out> attribute you can apply to ByRef parameters, which will be recognized by C#, but in VB.NET it won't force you to initialize the parameter. This code is valid and doesn't change the value passed for x

Sub Main
    Dim x As Integer = 42
    Test(x)
    Console.WriteLine(x) ' Prints 42
End Sub

Sub Test(<Out> ByRef x As Integer)
End Sub

Given that mocking libraries such as NSubstitute, Moq or FakeItEasy are not specific to C#, I'd say the current behavior is fine.

[davidnemeti]

@blairconrad, I just simply had a the test method in which I reused the variable which was an out parameter for the mocked method. Previously I had mocked that method so that it returned that specific value in the out parameter.

Then I called CallRouter.Clear, and I saw that the mocked method still return the same value in the out parameter, and I thought that maybe the CallRouter.Clear method did not work properly (#589).

Something like this:

var provider = Substitute.For<IFooProvider>();
provider.When(dtp => dtp.Do(out Arg.Any<int>())).Do(callInfo => callInfo[0] = 44);

int outValue;
provider.Do(out outValue);
// outValue is 44

callRouter = SubstitutionContext.Current.GetCallRouterFor(provider);
callRouter.Clear(ClearOptions.All);

provider.Do(out outValue);
// outValue is still 44

Now, I know that NSubstitute leaves the out parameter untouched in case of an unmocked method. Which I still find unnatural in C#.

However, after reading @thomaslevesque's comment about VB.NET, I must say that this behavior should not be changed.

[zvirja]

Hi @stakx @blairconrad @thomaslevesque,

Thank you very much for rich feedback you provided - that's very cool and helpful 😎 Take my apologize for the belated reply.

Moq will read the out argument used in the setup expression to determine what the out arguments of mock method invocations should be set to (which IMHO is a rather "interesting" way of going about this):

@stakx I've also found that it behaves like that even if you use argument specifier when configure a call (out It.Ref<string>.IsAny) - in this case it assigns "default" value (which technically is most likely what argument specifier returns). So your strategy is basically "if you touch this method, we start to return something". In NSubstitute it's not like that and we affect value only if you explicitly configure it - most likely same way it happens in FakeItEasy.

I love the @thomaslevesque's argument about VB.NET and fact that our libraries are in fact multi-target. It sounds like a good excuse to continue behaving in a way we do 😉 Especially, given that NSubstitute officially provides analyzers for VB.NET, same as FakeItEasy.

So I suggest to just keep things like they are, unless more people are concerned about this 😊